2022-07-23 02:34:41

by Stefan Hansson

[permalink] [raw]
Subject: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

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.

Regards,
Stefan Hansson


2022-07-23 11:45:07

by Jiri Kosina

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

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 for the report. CCing José who has been doing a lot of work in this
area recently, maybe he has an immediate idea.

If not, perhaps bisecting the hid-uclogic.c commits between the two
kernels would be the quickest option.

--
Jiri Kosina
SUSE Labs

2022-07-24 11:59:58

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

Hi!

Thanks for CCing me Jiří.

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.

Jose

2022-07-25 23:03:52

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

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/[email protected]/
(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));

2022-07-26 18:25:27

by Stefan Hansson

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

Hi again!

On 2022-07-25 18:48, José Expósito wrote:
> 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

Yes :)

>> 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/[email protected]/
> (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.

Thanks for looking into this! Bisecting has been slow on my end
unfortunately. I built today's linux-next (20220726) with your proposed
patch below and my drawing tablet curiously still does not work as
expected. The stylus works a couple of times, but eventually stops
working (unlike prior where it always seemed to only work once). Do I
need both your revert and this diff for it to work properly?

Also, do you know whether the revert be backported to stable 5.18?

> 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));
Regards,
Stefan Hansson

2022-07-26 21:51:27

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

Hi!

On Tue, Jul 26, 2022 at 01:58:24PM -0400, Stefan Hansson wrote:
> Hi again!
>
> On 2022-07-25 18:48, Jos? Exp?sito wrote:
> > Hi everyone,
> >
> > On Sun, Jul 24, 2022 at 01:48:49PM +0200, Jos? Exp?sito wrote:
> >
> > [...]
> >
> > The first bad commit is 87562fcd1342 ("HID: input: remove the need for
> > HID_QUIRK_INVERT"):
> > https://lore.kernel.org/all/[email protected]/
> > (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.
>
> Thanks for looking into this! Bisecting has been slow on my end
> unfortunately. I built today's linux-next (20220726) with your proposed
> patch below and my drawing tablet curiously still does not work as expected.
> The stylus works a couple of times, but eventually stops working (unlike
> prior where it always seemed to only work once). Do I need both your revert
> and this diff for it to work properly?

You are right, I just tested for a while with the diff applied (without
reverting the commit causing the issue) and after putting the pen in
and out proximity a fair amount of times (> 100) it stopped working.

I added some logs with the hope that they help to understand the issue:

Once the stylus stops working, hidinput_hid_event() is called with a
usage->hid of HID_DG_TIPSWITCH.
Next, it gets called again with HID_DG_INRANGE. At this point
report->tool_active and report->tool evaluate to true, i.e.,
hid_report_set_tool() is not invoked.

This succession of HID_DG_TIPSWITCH + HID_DG_INRANGE is repeated while
the stylus is in range and the tool used is never reported to user
space. In other words, using "libinput record" I can see ABS_* events
but without a leading and trailing BTN_TOOL_PEN event.

Notice that when the stylus works, report->tool evaluates to false and
hid_report_set_tool(), which calls hid_report_release_tool(), is
invoked.

> Also, do you know whether the revert be backported to stable 5.18?

Let's wait for Benjamin's opinion. I don't think that reverting the
commit is the best option in this case. While checking Benjamin's
series I remembered this libinput MR:
https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/724

And I think they are related. Ideally we'd like to keep that fix.

Usually, these kind of patches get backported eventually. I'm afraid I
can not tell you if it'd be the case this time.

Jose

2022-07-27 02:58:23

by Stefan Hansson

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

Hi!

>> Thanks for looking into this! Bisecting has been slow on my end
>> unfortunately. I built today's linux-next (20220726) with your proposed
>> patch below and my drawing tablet curiously still does not work as expected.
>> The stylus works a couple of times, but eventually stops working (unlike
>> prior where it always seemed to only work once). Do I need both your revert
>> and this diff for it to work properly?
>
> You are right, I just tested for a while with the diff applied (without
> reverting the commit causing the issue) and after putting the pen in
> and out proximity a fair amount of times (> 100) it stopped working.

This part is peculiar to me. When I said "a couple of times", I really
meant a couple of times. For me, this issue reproduces after maybe 10
times at most. I have never been able to do it for anything close to 100
times. I wonder what's up with this disparity?

Regards,
Stefan Hansson

2022-07-27 18:15:51

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

On Tue, Jul 26, 2022 at 10:56:33PM -0400, Stefan Hansson wrote:
> Hi!
>
> > > Thanks for looking into this! Bisecting has been slow on my end
> > > unfortunately. I built today's linux-next (20220726) with your proposed
> > > patch below and my drawing tablet curiously still does not work as expected.
> > > The stylus works a couple of times, but eventually stops working (unlike
> > > prior where it always seemed to only work once). Do I need both your revert
> > > and this diff for it to work properly?
> >
> > You are right, I just tested for a while with the diff applied (without
> > reverting the commit causing the issue) and after putting the pen in
> > and out proximity a fair amount of times (> 100) it stopped working.
>
> This part is peculiar to me. When I said "a couple of times", I really meant
> a couple of times. For me, this issue reproduces after maybe 10 times at
> most. I have never been able to do it for anything close to 100 times. I
> wonder what's up with this disparity?

We are most likely doing something different. Anyway, the important bit
is that with the current code present 5.18 the bug is easy to reproduce
in order to test fixes.

Jose

2022-08-04 18:50:33

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

Hi again,

On 2022-07-26 18:48, Jos? Exp?sito wrote:
> The first bad commit is 87562fcd1342 ("HID: input: remove the need for
> HID_QUIRK_INVERT"):
> https://lore.kernel.org/all/[email protected]/
> (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.

Indeed, it looks like v2 devices are affected. Similar reports:

- https://github.com/DIGImend/digimend-kernel-drivers/issues/626
- https://bugzilla.kernel.org/show_bug.cgi?id=216106

Kindly sending this thread back to your inbox to see if we could fix
this regression.

Best wishes,
Jose

> 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().
> [...]

2022-08-11 15:30:47

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

On Thu, Aug 4, 2022 at 8:24 PM José Expósito <[email protected]> wrote:
>
> Hi again,
>
> On 2022-07-26 18:48, José Expósito wrote:
> > The first bad commit is 87562fcd1342 ("HID: input: remove the need for
> > HID_QUIRK_INVERT"):
> > https://lore.kernel.org/all/[email protected]/
> > (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.
>
> Indeed, it looks like v2 devices are affected. Similar reports:
>
> - https://github.com/DIGImend/digimend-kernel-drivers/issues/626
> - https://bugzilla.kernel.org/show_bug.cgi?id=216106
>
> Kindly sending this thread back to your inbox to see if we could fix
> this regression.

[sorry, I was out on vacation the past 2 weeks and this week was the
usual "urgent" thing I have to day for yesterday]

Ideally, I'd like to not revert that commit. It solves a bunch of
issues on many devices, so that's maybe not the way forward.

FWIW, it was quite painful to tweak and that was a solution that
matches the hid-multitouch devices I could find.

I tried to process your email when you described the succession of
events without much success.

Would you mind dumping a hid-record when exposing the bug?

Cheers,
Benjamin


>
> Best wishes,
> Jose
>
> > 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().
> > [...]
>

2022-08-13 11:20:57

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

Hi Benjamin,

On Thu, Aug 11, 2022 at 05:23:52PM +0200, Benjamin Tissoires wrote:
> On Thu, Aug 4, 2022 at 8:24 PM Jos? Exp?sito <[email protected]> wrote:
> >
> > Hi again,
> >
> > On 2022-07-26 18:48, Jos? Exp?sito wrote:
> > > The first bad commit is 87562fcd1342 ("HID: input: remove the need for
> > > HID_QUIRK_INVERT"):
> > > https://lore.kernel.org/all/[email protected]/
> > > (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.
> >
> > Indeed, it looks like v2 devices are affected. Similar reports:
> >
> > - https://github.com/DIGImend/digimend-kernel-drivers/issues/626
> > - https://bugzilla.kernel.org/show_bug.cgi?id=216106
> >
> > Kindly sending this thread back to your inbox to see if we could fix
> > this regression.
>
> [sorry, I was out on vacation the past 2 weeks and this week was the
> usual "urgent" thing I have to day for yesterday]

No problem, I hope you enjoyed your holidays :D

> Ideally, I'd like to not revert that commit. It solves a bunch of
> issues on many devices, so that's maybe not the way forward.
>
> FWIW, it was quite painful to tweak and that was a solution that
> matches the hid-multitouch devices I could find.
>
> I tried to process your email when you described the succession of
> events without much success.
>
> Would you mind dumping a hid-record when exposing the bug?

Sure, I added as an attachment in the existing report in bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=216106#c2

I hope it helps you to debug the issue. Let me know if you need more
recordings, help testing patches or any other information.

In future changes to tablet code, feel free to cc me. I have a bunch of
non Wacom devices and I'll help you testing your changes.

Best wishes,
Jose

> Cheers,
> Benjamin
>
>
> >
> > Best wishes,
> > Jose
> >
> > > 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().
> > > [...]
> >
>

2022-08-19 14:22:40

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet



On Sat, Aug 13, 2022 at 1:09 PM José Expósito <[email protected]> wrote:
>
> Hi Benjamin,
>
> On Thu, Aug 11, 2022 at 05:23:52PM +0200, Benjamin Tissoires wrote:
> > On Thu, Aug 4, 2022 at 8:24 PM José Expósito <[email protected]> wrote:
> > >
> > > Hi again,
> > >
> > > On 2022-07-26 18:48, José Expósito wrote:
> > > > The first bad commit is 87562fcd1342 ("HID: input: remove the need for
> > > > HID_QUIRK_INVERT"):
> > > > https://lore.kernel.org/all/[email protected]/
> > > > (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.
> > >
> > > Indeed, it looks like v2 devices are affected. Similar reports:
> > >
> > > - https://github.com/DIGImend/digimend-kernel-drivers/issues/626
> > > - https://bugzilla.kernel.org/show_bug.cgi?id=216106
> > >
> > > Kindly sending this thread back to your inbox to see if we could fix
> > > this regression.
> >
> > [sorry, I was out on vacation the past 2 weeks and this week was the
> > usual "urgent" thing I have to day for yesterday]
>
> No problem, I hope you enjoyed your holidays :D
>
> > Ideally, I'd like to not revert that commit. It solves a bunch of
> > issues on many devices, so that's maybe not the way forward.
> >
> > FWIW, it was quite painful to tweak and that was a solution that
> > matches the hid-multitouch devices I could find.
> >
> > I tried to process your email when you described the succession of
> > events without much success.
> >
> > Would you mind dumping a hid-record when exposing the bug?
>
> Sure, I added as an attachment in the existing report in bugzilla:
> https://bugzilla.kernel.org/show_bug.cgi?id=216106#c2
>
> I hope it helps you to debug the issue. Let me know if you need more
> recordings, help testing patches or any other information.
>
> In future changes to tablet code, feel free to cc me. I have a bunch of
> non Wacom devices and I'll help you testing your changes.

FWIW, I found the issue: the hid-uclogic driver is emitting input data
behind hid-input, and the state between the 2 is desynchronized.

The following patch seems to be working (with the Huion v1 protocol I
have here that I have tweaked to resemble a v2):
---
From aeedd318e6cb4dbee551f67616302cc7c4308c58 Mon Sep 17 00:00:00 2001
From: Benjamin Tissoires <[email protected]>
Date: Thu, 18 Aug 2022 15:09:25 +0200
Subject: [PATCH] Fix uclogic

---
drivers/hid/hid-input.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index c6b27aab9041..a3e2397bb3a7 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1530,7 +1530,10 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
* assume ours
*/
if (!report->tool)
- hid_report_set_tool(report, input, usage->code);
+ report->tool = usage->code;
+
+ /* drivers may have changed the value behind our back, resend it */
+ hid_report_set_tool(report, input, report->tool);
} else {
hid_report_release_tool(report, input, usage->code);
}
--
2.36.1

---

Can someone with an affected device test it?

Cheers,
Benjamin

>
> Best wishes,
> Jose
>
> > Cheers,
> > Benjamin
> >
> >
> > >
> > > Best wishes,
> > > Jose
> > >
> > > > 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().
> > > > [...]
> > >
> >
>

2022-08-21 00:10:01

by Stefan Hansson

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

> FWIW, I found the issue: the hid-uclogic driver is emitting input data
> behind hid-input, and the state between the 2 is desynchronized.
>
> The following patch seems to be working (with the Huion v1 protocol I
> have here that I have tweaked to resemble a v2):
> ---
> From aeedd318e6cb4dbee551f67616302cc7c4308c58 Mon Sep 17 00:00:00 2001
> From: Benjamin Tissoires <[email protected]>
> Date: Thu, 18 Aug 2022 15:09:25 +0200
> Subject: [PATCH] Fix uclogic
>
> ---
>  drivers/hid/hid-input.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index c6b27aab9041..a3e2397bb3a7 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1530,7 +1530,10 @@ void hidinput_hid_event(struct hid_device *hid,
> struct hid_field *field, struct
>               * assume ours
>               */
>              if (!report->tool)
> -                hid_report_set_tool(report, input, usage->code);
> +                report->tool = usage->code;
> +
> +            /* drivers may have changed the value behind our back,
> resend it */
> +            hid_report_set_tool(report, input, report->tool);
>          } else {
>              hid_report_release_tool(report, input, usage->code);
>          }

What branch should this be applied on top of?

2022-08-22 06:48:36

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

On Sun, Aug 21, 2022 at 1:45 AM Stefan Hansson <[email protected]> wrote:
>
> > FWIW, I found the issue: the hid-uclogic driver is emitting input data
> > behind hid-input, and the state between the 2 is desynchronized.
> >
> > The following patch seems to be working (with the Huion v1 protocol I
> > have here that I have tweaked to resemble a v2):
> > ---
> > From aeedd318e6cb4dbee551f67616302cc7c4308c58 Mon Sep 17 00:00:00 2001
> > From: Benjamin Tissoires <[email protected]>
> > Date: Thu, 18 Aug 2022 15:09:25 +0200
> > Subject: [PATCH] Fix uclogic
> >
> > ---
> > drivers/hid/hid-input.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index c6b27aab9041..a3e2397bb3a7 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -1530,7 +1530,10 @@ void hidinput_hid_event(struct hid_device *hid,
> > struct hid_field *field, struct
> > * assume ours
> > */
> > if (!report->tool)
> > - hid_report_set_tool(report, input, usage->code);
> > + report->tool = usage->code;
> > +
> > + /* drivers may have changed the value behind our back,
> > resend it */
> > + hid_report_set_tool(report, input, report->tool);
> > } else {
> > hid_report_release_tool(report, input, usage->code);
> > }
>
> What branch should this be applied on top of?
>

Sorry for that. I had some local commits in my tree that made the
patch unusable. I just formally sent the patch [0] based on the
hid.git/for-next branch which is actually applying on top of v5.19 or
even v5.18.

Cheers,
Benjamin

[0] https://lore.kernel.org/linux-input/[email protected]/T/#u

2022-08-28 11:01:51

by José Expósito

[permalink] [raw]
Subject: Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet

On Mon, Aug 22, 2022 at 08:25:52AM +0200, Benjamin Tissoires wrote:
> On Sun, Aug 21, 2022 at 1:45 AM Stefan Hansson <[email protected]> wrote:
> >
> > > FWIW, I found the issue: the hid-uclogic driver is emitting input data
> > > behind hid-input, and the state between the 2 is desynchronized.
> > >
> > > The following patch seems to be working (with the Huion v1 protocol I
> > > have here that I have tweaked to resemble a v2):
> > > ---
> > > From aeedd318e6cb4dbee551f67616302cc7c4308c58 Mon Sep 17 00:00:00 2001
> > > From: Benjamin Tissoires <[email protected]>
> > > Date: Thu, 18 Aug 2022 15:09:25 +0200
> > > Subject: [PATCH] Fix uclogic
> > >
> > > ---
> > > drivers/hid/hid-input.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > > index c6b27aab9041..a3e2397bb3a7 100644
> > > --- a/drivers/hid/hid-input.c
> > > +++ b/drivers/hid/hid-input.c
> > > @@ -1530,7 +1530,10 @@ void hidinput_hid_event(struct hid_device *hid,
> > > struct hid_field *field, struct
> > > * assume ours
> > > */
> > > if (!report->tool)
> > > - hid_report_set_tool(report, input, usage->code);
> > > + report->tool = usage->code;
> > > +
> > > + /* drivers may have changed the value behind our back,
> > > resend it */
> > > + hid_report_set_tool(report, input, report->tool);
> > > } else {
> > > hid_report_release_tool(report, input, usage->code);
> > > }
> >
> > What branch should this be applied on top of?
> >
>
> Sorry for that. I had some local commits in my tree that made the
> patch unusable. I just formally sent the patch [0] based on the
> hid.git/for-next branch which is actually applying on top of v5.19 or
> even v5.18.
>
> Cheers,
> Benjamin
>
> [0] https://lore.kernel.org/linux-input/[email protected]/T/#u
>

As I already commented in the patch, the problem is now solved on
hid/for-next.

Thanks!