linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
@ 2022-07-23  2:14 Stefan Hansson
  2022-07-23 11:40 ` Jiri Kosina
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Hansson @ 2022-07-23  2:14 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, linux-input, linux-kernel; +Cc: newbie13xd

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-23  2:14 PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet Stefan Hansson
@ 2022-07-23 11:40 ` Jiri Kosina
  2022-07-24 11:48   ` José Expósito
  0 siblings, 1 reply; 15+ messages in thread
From: Jiri Kosina @ 2022-07-23 11:40 UTC (permalink / raw)
  To: Stefan Hansson
  Cc: benjamin.tissoires, linux-input, linux-kernel, José Expósito

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-23 11:40 ` Jiri Kosina
@ 2022-07-24 11:48   ` José Expósito
  2022-07-25 22:48     ` José Expósito
  0 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-24 11:48 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: Stefan Hansson, benjamin.tissoires, linux-input, linux-kernel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-24 11:48   ` José Expósito
@ 2022-07-25 22:48     ` José Expósito
  2022-07-26 17:58       ` Stefan Hansson
  0 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-25 22:48 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Peter Hutterer
  Cc: Stefan Hansson, benjamin.tissoires, linux-input, linux-kernel

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@redhat.com/
(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));

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-25 22:48     ` José Expósito
@ 2022-07-26 17:58       ` Stefan Hansson
  2022-07-26 21:48         ` José Expósito
  2022-08-04 18:24         ` José Expósito
  0 siblings, 2 replies; 15+ messages in thread
From: Stefan Hansson @ 2022-07-26 17:58 UTC (permalink / raw)
  To: José Expósito, Jiri Kosina, Benjamin Tissoires,
	Ping Cheng, Peter Hutterer
  Cc: linux-input, linux-kernel

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/20220203143226.4023622-11-benjamin.tissoires@redhat.com/
> (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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-26 17:58       ` Stefan Hansson
@ 2022-07-26 21:48         ` José Expósito
  2022-07-27  2:56           ` Stefan Hansson
  2022-08-04 18:24         ` José Expósito
  1 sibling, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-07-26 21:48 UTC (permalink / raw)
  To: Stefan Hansson
  Cc: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Peter Hutterer,
	linux-input, linux-kernel

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/20220203143226.4023622-11-benjamin.tissoires@redhat.com/
> > (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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-26 21:48         ` José Expósito
@ 2022-07-27  2:56           ` Stefan Hansson
  2022-07-27 16:27             ` José Expósito
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Hansson @ 2022-07-27  2:56 UTC (permalink / raw)
  To: José Expósito
  Cc: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Peter Hutterer,
	linux-input, linux-kernel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-27  2:56           ` Stefan Hansson
@ 2022-07-27 16:27             ` José Expósito
  0 siblings, 0 replies; 15+ messages in thread
From: José Expósito @ 2022-07-27 16:27 UTC (permalink / raw)
  To: Stefan Hansson
  Cc: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Peter Hutterer,
	linux-input, linux-kernel

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-07-26 17:58       ` Stefan Hansson
  2022-07-26 21:48         ` José Expósito
@ 2022-08-04 18:24         ` José Expósito
  2022-08-11 15:23           ` Benjamin Tissoires
  1 sibling, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-08-04 18:24 UTC (permalink / raw)
  To: Stefan Hansson
  Cc: Jiri Kosina, Benjamin Tissoires, Ping Cheng, Peter Hutterer,
	linux-input, linux-kernel

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/20220203143226.4023622-11-benjamin.tissoires@redhat.com/
> (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().
> [...]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-08-04 18:24         ` José Expósito
@ 2022-08-11 15:23           ` Benjamin Tissoires
  2022-08-13 11:09             ` José Expósito
  0 siblings, 1 reply; 15+ messages in thread
From: Benjamin Tissoires @ 2022-08-11 15:23 UTC (permalink / raw)
  To: José Expósito
  Cc: Stefan Hansson, Jiri Kosina, Ping Cheng, Peter Hutterer,
	open list:HID CORE LAYER, lkml

On Thu, Aug 4, 2022 at 8:24 PM José Expósito <jose.exposito89@gmail.com> 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/20220203143226.4023622-11-benjamin.tissoires@redhat.com/
> > (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().
> > [...]
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-08-11 15:23           ` Benjamin Tissoires
@ 2022-08-13 11:09             ` José Expósito
  2022-08-19 14:15               ` Benjamin Tissoires
  0 siblings, 1 reply; 15+ messages in thread
From: José Expósito @ 2022-08-13 11:09 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Stefan Hansson, Jiri Kosina, Ping Cheng, Peter Hutterer,
	open list:HID CORE LAYER, lkml

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 <jose.exposito89@gmail.com> 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/20220203143226.4023622-11-benjamin.tissoires@redhat.com/
> > > (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().
> > > [...]
> >
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-08-13 11:09             ` José Expósito
@ 2022-08-19 14:15               ` Benjamin Tissoires
  2022-08-20 23:45                 ` Stefan Hansson
  0 siblings, 1 reply; 15+ messages in thread
From: Benjamin Tissoires @ 2022-08-19 14:15 UTC (permalink / raw)
  To: José Expósito
  Cc: Stefan Hansson, Jiri Kosina, Ping Cheng, Peter Hutterer,
	open list:HID CORE LAYER, lkml



On Sat, Aug 13, 2022 at 1:09 PM José Expósito <jose.exposito89@gmail.com> 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 <jose.exposito89@gmail.com> 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/20220203143226.4023622-11-benjamin.tissoires@redhat.com/
> > > > (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 <benjamin.tissoires@redhat.com>
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().
> > > > [...]
> > >
> >
>


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-08-19 14:15               ` Benjamin Tissoires
@ 2022-08-20 23:45                 ` Stefan Hansson
  2022-08-22  6:25                   ` Benjamin Tissoires
  0 siblings, 1 reply; 15+ messages in thread
From: Stefan Hansson @ 2022-08-20 23:45 UTC (permalink / raw)
  To: Benjamin Tissoires, José Expósito
  Cc: Jiri Kosina, Ping Cheng, Peter Hutterer, open list:HID CORE LAYER, lkml

> 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 <benjamin.tissoires@redhat.com>
> 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?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-08-20 23:45                 ` Stefan Hansson
@ 2022-08-22  6:25                   ` Benjamin Tissoires
  2022-08-28 10:07                     ` José Expósito
  0 siblings, 1 reply; 15+ messages in thread
From: Benjamin Tissoires @ 2022-08-22  6:25 UTC (permalink / raw)
  To: Stefan Hansson
  Cc: José Expósito, Jiri Kosina, Ping Cheng, Peter Hutterer,
	open list:HID CORE LAYER, lkml

On Sun, Aug 21, 2022 at 1:45 AM Stefan Hansson <newbie13xd@gmail.com> 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 <benjamin.tissoires@redhat.com>
> > 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/20220822062247.1146141-1-benjamin.tissoires@redhat.com/T/#u


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet
  2022-08-22  6:25                   ` Benjamin Tissoires
@ 2022-08-28 10:07                     ` José Expósito
  0 siblings, 0 replies; 15+ messages in thread
From: José Expósito @ 2022-08-28 10:07 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Stefan Hansson, Jiri Kosina, Ping Cheng, Peter Hutterer,
	open list:HID CORE LAYER, lkml

On Mon, Aug 22, 2022 at 08:25:52AM +0200, Benjamin Tissoires wrote:
> On Sun, Aug 21, 2022 at 1:45 AM Stefan Hansson <newbie13xd@gmail.com> 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 <benjamin.tissoires@redhat.com>
> > > 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/20220822062247.1146141-1-benjamin.tissoires@redhat.com/T/#u
> 

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

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-08-28 10:07 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-23  2:14 PROBLEM: Regression likely in hid_uclogic driver breaks Huion Inspiroy H640 drawing tablet Stefan Hansson
2022-07-23 11:40 ` Jiri Kosina
2022-07-24 11:48   ` José Expósito
2022-07-25 22:48     ` José Expósito
2022-07-26 17:58       ` Stefan Hansson
2022-07-26 21:48         ` José Expósito
2022-07-27  2:56           ` Stefan Hansson
2022-07-27 16:27             ` José Expósito
2022-08-04 18:24         ` José Expósito
2022-08-11 15:23           ` Benjamin Tissoires
2022-08-13 11:09             ` José Expósito
2022-08-19 14:15               ` Benjamin Tissoires
2022-08-20 23:45                 ` Stefan Hansson
2022-08-22  6:25                   ` Benjamin Tissoires
2022-08-28 10:07                     ` José Expósito

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).