Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

[José Expósito]

Hi everyone,

On Sun, Jul 24, 2022 at 01:48:49PM +0200, José Expósito wrote:
> On Fri, 22 Jul 2022, Stefan Hansson wrote:
> > Hi!
> >
> > Somewhere between Linux 5.17.6 and 5.18.11 the Huion tablet I have stopped
> > working properly. In GNOME Control Center it is identified as Huion New 1060
> > Plus, however that's a different tablet than the one I have. Mine is a Huion
> > Inspiroy H640, and it uses the hid_uclogic driver.
> >
> > With Linux 5.17.6, the tablet works as expected with all the buttons being
> > detected and the stylus being usable. With 5.18.11, the buttons work fine but
> > the stylus does not work correctly. The first time I approach the tablet with
> > the stylus it works properly, i.e., the cursor on my screen moves around and
> > follows the stylus around the tablet as expected. It continues working like
> > this until I remove the stylus from the tablet. After I remove it from the
> > tablet, the cursor never gets controlled by the stylus again. I can see that
> > the tablet detects the stylus (it has a small indicator light), but the cursor
> > doesn't move when I approach the tablet again. To clarify, with Linux 5.17.6,
> > the cursor moves around just fine when I remove and then put it back to the
> > tablet, just as you would expected.
> >
> > It may also be worth noting that it worked fine when I previously used it
> > around six months ago, although I'm not sure what version of Linux I was using
> > at that time (whatever Fedora shipped back then). I also tried reproducing it
> > with yesterday's linux-next and Linux 5.19.0-RC7, and the behaviour was the
> > same as 5.18.11. I am currently trying to bisect this, but it's not going very
> > fast as I currently only have access to a dual core laptop from 2014, so
> > building Linux takes a good while.
> 
> Thanks a lot for reporting the issue.
> 
> HUION and other non-Wacom tablets are handled by the UCLogic driver.
> This driver is present in the kernel but its changes were deployed
> and tested first in the DIGImend driver:
> https://github.com/DIGImend/digimend-kernel-drivers
> 
> A while ago, I started including in the kernel the code present in
> DIGImend. At this moment, 5.19.0-RC7 and DIGImend have the same code
> (well, 5.19 has more features, but they don't affect your tablet).
> 
> I'm telling you this because it might be easier for you to bisect the
> changes in the DIGImend driver as it builds way faster than the kernel.
> Let me know if you need help bisecting it and I'll do my best to help
> you.
> 
> Is this your device?
> https://www.huion.com/pen_tablet/Inspiroy/H640P.html
> 
> It is affordable, so I ordered it. I don't have any HUION devices to
> debug and this is a good excuse to buy one ;)
> I'll let you know how it goes once I receive it.

The tablet arrived today and it is a bank holiday in Spain, so I had
some time to bisect the bug.

The first bad commit is 87562fcd1342 ("HID: input: remove the need for
HID_QUIRK_INVERT"):
https://lore.kernel.org/all/20220203143226.4023622-11-benjamin.tissoires@xxxxxxxxxx/
(CCing the folks whose email is in the patch tags)

I reverted the patch on hid/for-next and, after fixing a tiny conflict,
I can confirm that the tablet works again as expected.

I'd need to investigate a bit more over the weekend, but I think that
all HUION tablets with the latest firmware (internally, v2) are
affected.

Those tablets do not set the inrange bit (UCLOGIC_PARAMS_PEN_INRANGE_NONE).
The driver sets it and uses a timer to remove it.
See drivers/hid/hid-uclogic-core.c, function uclogic_raw_event_pen().

However, at least the Huion Inspiroy H640, sends a 0x00 byte when the
tool is removed, making it possible to fix it in the driver [1].

Unfortunately, the affected code path is used by many tablets and I
can not test them, so I'd prefer to hear Benjamin's opinion and see if
this should be fixed in hid-input rather than in the driver before
sending a fix.

Best wishes,
José Expósito

[1] Diff of a possible fix:

diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index 47a17375c7fc..bdcbbd57d0fc 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -316,8 +316,11 @@ static int uclogic_raw_event_pen(struct uclogic_drvdata *drvdata,
        }
        /* If we need to emulate in-range detection */
        if (pen->inrange == UCLOGIC_PARAMS_PEN_INRANGE_NONE) {
                /* Set in-range bit */
-               data[1] |= 0x40;
+               if (data[1])
+                       data[1] |= 0x40;
+
                /* (Re-)start in-range timeout */
                mod_timer(&drvdata->inrange_timer,
                                jiffies + msecs_to_jiffies(100));