Re: Status on hid xppen patch

Re: Status on hid xppen patch

[José Expósito]

> Hello everynyan!
>
> A while ago I sent in a patch to add support for newer Xp-pen tablets 
> that even made it into patchwork:
>
> https://patchwork.kernel.org/project/linux-input/patch/b401e453-9c66-15e3-1a1d-21f33b7a64e8@gmail.com/
>
> I have never actually gotten any feedback on it though and am wondering 
> if everything is to your liking. Anyway it doesn't build against the 
> current tree anymore. Assuming there is any value it, should I rewrite 
> it against hid master or hid uclogic?
>
> Many thanks
>
> Stefan Berzl

Hi Stefan,

I just saw your email in the mailing list.

Your patch doesn't apply against the current tree because of the changes
from Nikolai/the DIGImend project I'm sending upstream. For reference,
here is the latest batch of patches, with links to the previous ones:

https://lore.kernel.org/linux-input/20220421175052.911446-1-jose.exposito89@gmail.com/T/

Please note that I'm not the maintainer of the driver, I'm just a web
developer who does free software as a hobby, i.e., this is not my area
of expertise, so take my words as suggestions, not as the path to
follow ;)

The development of the uclogic driver takes place on the DIGImend
project (inactive right now):
https://github.com/DIGImend/digimend-kernel-drivers

Like you, I wanted to add support for my tablet/improve my kernel
dev skills, but I noticed that I needed some patches from DIGImend, so,
instead of taking what I needed, I decided to upstream all the patches.

At the moment of writing this email, 24 patches from DIGImend have been
merged, 5 are under review and 7 more need to be sent. We are close to
the end.

My tablet (Parblo A610 PLUS V2) also needs some magic data to be enabled.
Actually, the data is pretty similar to the one in your patch.
You can see my implementation here:
https://github.com/JoseExposito/linux/commit/f1f24e57fab45a2bcf4e0af5ba9d8f5a2245670b

I just refactored my patch and extracted the code to send the magic data
to its own function (uclogic_probe_interface), so we can share it.

I'd suggest rebasing your code on top of DIGImend's code + my patch
so we can share some code and once all patches are upstream, you can
send it with minor or without conflicts.

What do you think?

José Expósito

Re: Status on hid xppen patch

[Stefan Berzl]

>> Hello everynyan!
>>
>> A while ago I sent in a patch to add support for newer Xp-pen tablets 
>> that even made it into patchwork:
>>
>> https://patchwork.kernel.org/project/linux-input/patch/b401e453-9c66-15e3-1a1d-21f33b7a64e8@gmail.com/
>>
>> I have never actually gotten any feedback on it though and am wondering 
>> if everything is to your liking. Anyway it doesn't build against the 
>> current tree anymore. Assuming there is any value it, should I rewrite 
>> it against hid master or hid uclogic?
>>
>> Many thanks
>>
>> Stefan Berzl
> 
> Hi Stefan,
> 
> I just saw your email in the mailing list.
> 
> Your patch doesn't apply against the current tree because of the changes
> from Nikolai/the DIGImend project I'm sending upstream. For reference,
> here is the latest batch of patches, with links to the previous ones:
> 
> https://lore.kernel.org/linux-input/20220421175052.911446-1-jose.exposito89@gmail.com/T/
> 
> Please note that I'm not the maintainer of the driver, I'm just a web
> developer who does free software as a hobby, i.e., this is not my area
> of expertise, so take my words as suggestions, not as the path to
> follow ;)
> 
> The development of the uclogic driver takes place on the DIGImend
> project (inactive right now):
> https://github.com/DIGImend/digimend-kernel-drivers
> 
> Like you, I wanted to add support for my tablet/improve my kernel
> dev skills, but I noticed that I needed some patches from DIGImend, so,
> instead of taking what I needed, I decided to upstream all the patches.
> 
> At the moment of writing this email, 24 patches from DIGImend have been
> merged, 5 are under review and 7 more need to be sent. We are close to
> the end.
> 
> My tablet (Parblo A610 PLUS V2) also needs some magic data to be enabled.
> Actually, the data is pretty similar to the one in your patch.
> You can see my implementation here:
> https://github.com/JoseExposito/linux/commit/f1f24e57fab45a2bcf4e0af5ba9d8f5a2245670b
> 
> I just refactored my patch and extracted the code to send the magic data
> to its own function (uclogic_probe_interface), so we can share it.
> 
> I'd suggest rebasing your code on top of DIGImend's code + my patch
> so we can share some code and once all patches are upstream, you can
> send it with minor or without conflicts.
> 
> What do you think?
> 
> José Expósito
> 

Hello José,

nice hearing from you. I was actually thinking of just asking if you
want to take my patch under your wing, since you seem to be doing a lot
of uclogic related work. But your idea is even better, just submitting  
my patch once everything has settled down seems to be the way to go.

You are actually the first person to ever contact me about this. I was  
pretty worried that the patch is no good, but then I saw that there is  
quite a backlog in the maintainers patchwork and some of your patches
haven't been reviewed either. Originally I tried showing it to Nikolai,
but he said he was rather busy too and then some guys in the Digimend
project tried to get me interested in some userspace driver that works  
around all the issues of like init-packets and stuff. I always imagined
the kernel as this highly organzied, well-structured endeavor where
things would be quickly scrutinized and suggestions offered to assure 
the highest quality. Yet here we have two web developers working on the
graphics tablet driver. Let me say that it's really great you take the
time and effort to mainline these patches and help everyone enjoy their
tablets on linux!

On the technical side, the magic bytes really are very similar, the only
difference being the two additional trailing 0x00 for mine. Also my
tablet sends a response after activating this new interface which can 
get interpreted as a button press if it's not discarded. Is there a nice
way to work around this, with subreports or anything?

Lastly, do you have an idea in which kernel version the bulk of your
patches will have been merged, just so I know when to start looking at
this again? I don't mean to belittle the kernel maintainers of course,  
for I am sure they have more pressing and difficult issues on their
plate, like eBPF and such.   

Lets fixie wixie this fucksy uppsy!

Bye


Stefan Berzl

Re: Status on hid xppen patch

[José Expósito]

Hi Stefan,

On Sun, Apr 24, 2022 at 01:19:30AM +0200, Stefan Berzl wrote:
> >> Hello everynyan!
> >>
> >> A while ago I sent in a patch to add support for newer Xp-pen tablets 
> >> that even made it into patchwork:
> >>
> >> https://patchwork.kernel.org/project/linux-input/patch/b401e453-9c66-15e3-1a1d-21f33b7a64e8@gmail.com/
> >>
> >> I have never actually gotten any feedback on it though and am wondering 
> >> if everything is to your liking. Anyway it doesn't build against the 
> >> current tree anymore. Assuming there is any value it, should I rewrite 
> >> it against hid master or hid uclogic?
> >>
> >> Many thanks
> >>
> >> Stefan Berzl
> > 
> > Hi Stefan,
> > 
> > I just saw your email in the mailing list.
> > 
> > Your patch doesn't apply against the current tree because of the changes
> > from Nikolai/the DIGImend project I'm sending upstream. For reference,
> > here is the latest batch of patches, with links to the previous ones:
> > 
> > https://lore.kernel.org/linux-input/20220421175052.911446-1-jose.exposito89@gmail.com/T/
> > 
> > Please note that I'm not the maintainer of the driver, I'm just a web
> > developer who does free software as a hobby, i.e., this is not my area
> > of expertise, so take my words as suggestions, not as the path to
> > follow ;)
> > 
> > The development of the uclogic driver takes place on the DIGImend
> > project (inactive right now):
> > https://github.com/DIGImend/digimend-kernel-drivers
> > 
> > Like you, I wanted to add support for my tablet/improve my kernel
> > dev skills, but I noticed that I needed some patches from DIGImend, so,
> > instead of taking what I needed, I decided to upstream all the patches.
> > 
> > At the moment of writing this email, 24 patches from DIGImend have been
> > merged, 5 are under review and 7 more need to be sent. We are close to
> > the end.
> > 
> > My tablet (Parblo A610 PLUS V2) also needs some magic data to be enabled.
> > Actually, the data is pretty similar to the one in your patch.
> > You can see my implementation here:
> > https://github.com/JoseExposito/linux/commit/f1f24e57fab45a2bcf4e0af5ba9d8f5a2245670b
> > 
> > I just refactored my patch and extracted the code to send the magic data
> > to its own function (uclogic_probe_interface), so we can share it.
> > 
> > I'd suggest rebasing your code on top of DIGImend's code + my patch
> > so we can share some code and once all patches are upstream, you can
> > send it with minor or without conflicts.
> > 
> > What do you think?
> > 
> > José Expósito
> > 
> 
> Hello José,
> 
> nice hearing from you. I was actually thinking of just asking if you
> want to take my patch under your wing, since you seem to be doing a lot
> of uclogic related work. But your idea is even better, just submitting  
> my patch once everything has settled down seems to be the way to go.

Cool, let's do that. I'll cc you on the last batch of patches so you
get notified when they get merged.
 
> You are actually the first person to ever contact me about this. I was  
> pretty worried that the patch is no good, but then I saw that there is  
> quite a backlog in the maintainers patchwork and some of your patches
> haven't been reviewed either. Originally I tried showing it to Nikolai,
> but he said he was rather busy too and then some guys in the Digimend
> project tried to get me interested in some userspace driver that works  
> around all the issues of like init-packets and stuff. I always imagined
> the kernel as this highly organzied, well-structured endeavor where
> things would be quickly scrutinized and suggestions offered to assure 
> the highest quality. Yet here we have two web developers working on the
> graphics tablet driver. Let me say that it's really great you take the
> time and effort to mainline these patches and help everyone enjoy their
> tablets on linux!

Well, the kernel is well-structured, but as you mentioned, there is a
*lot* of work on the maintainers side and I think there is not a big
community supporting this driver. It's normal that it takes some time
to get everything reviewed.

> On the technical side, the magic bytes really are very similar, the only
> difference being the two additional trailing 0x00 for mine. Also my
> tablet sends a response after activating this new interface which can 
> get interpreted as a button press if it's not discarded. Is there a nice
> way to work around this, with subreports or anything?

Same here, my tablet responds with: 02 b1 04 00 00 00 00 00 00 00.

In my case, button presses look like: 02 F0 XX XX 00 00 00 00 00 00.
Because of the 0xF0 subreport, I think this data gets ignored, but I'll
do some extra testing just in case.

How did you test it?

> Lastly, do you have an idea in which kernel version the bulk of your
> patches will have been merged, just so I know when to start looking at
> this again? I don't mean to belittle the kernel maintainers of course,  
> for I am sure they have more pressing and difficult issues on their
> plate, like eBPF and such.   

I couldn't tell, I can imagine that 5.19 or 5.20. Anyway, I'll cc you
so you can have a good idea about what's going on.

Best wishes,
José Expósito

> Lets fixie wixie this fucksy uppsy!
> 
> Bye
> 
> 
> Stefan Berzl

Re: Status on hid xppen patch

[José Expósito]

Hi Stefan,

A quick update on my work on XP-PEN tablets.

> > nice hearing from you. I was actually thinking of just asking if you
> > want to take my patch under your wing, since you seem to be doing a lot
> > of uclogic related work. But your idea is even better, just submitting  
> > my patch once everything has settled down seems to be the way to go.
> 
> Cool, let's do that. I'll cc you on the last batch of patches so you
> get notified when they get merged.

As promised, I cc'ed you on the last patchset, which has been merged
already.
At this point, the kernel and DIGImend have the same code.
  
In the meantime, in order to make the driver code as generic as
possible, I bought a couple of XP-PEN tablets. 

The tablets are the Deco Mini 4 and the Deco L, both of them are UGEE
tablets. I already had a UGEE Parblo A610 Pro tablet and after having a
look to the Windows driver traffic, I found out that after sending a
chunk of magic data to enable the tablet, it requests a string
descriptor ("uclogic_params_get_str_desc" can be used here) and the
tablets respond with their parameters.

The information is encoded, in bytes, as:

 02 + 03 - UCLOGIC_RDESC_PEN_PH_ID_X_LM
 04 + 05 - UCLOGIC_RDESC_PEN_PH_ID_Y_LM
 06      - Number of buttons
 07      - Dial present or not
 08 + 09 - UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM
 10 + 11 - Resolution

Bytes 12 and 13 are present but set to 0, probably indicating my
tablets are lacking some feature.

Could you confirm that your tablet returns similar information, please?

In case you want to have a look to the implementation, I'm working on
this branch:
https://github.com/JoseExposito/linux/commits/patch-xppen-deco-l

I had to introduce new functionalities to the templating system of the
driver, that's the reason for the KUnit tests.
The last patch is work in progress (hopefully I'll have time to finish
it this weekend), only the HID descriptors are missing.

I'll cc you when in the patchset so you can add your IDs :)

Best wishes,
Jose

Re: Status on hid xppen patch

[Stefan Berzl]

Hello José,

> The tablets are the Deco Mini 4 and the Deco L, both of them are UGEE
> tablets. I already had a UGEE Parblo A610 Pro tablet and after having a
> look to the Windows driver traffic, I found out that after sending a
> chunk of magic data to enable the tablet, it requests a string
> descriptor ("uclogic_params_get_str_desc" can be used here) and the
> tablets respond with their parameters.
> 
> The information is encoded, in bytes, as:
> 
>  02 + 03 - UCLOGIC_RDESC_PEN_PH_ID_X_LM
>  04 + 05 - UCLOGIC_RDESC_PEN_PH_ID_Y_LM
>  06      - Number of buttons
>  07      - Dial present or not
>  08 + 09 - UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM
>  10 + 11 - Resolution
> 
> Bytes 12 and 13 are present but set to 0, probably indicating my
> tablets are lacking some feature.
> 
> Could you confirm that your tablet returns similar information, please?

yes, you can haz string descriptor:
0e 03 0b 8b cb 56 08 00 ff 1f d8 13

Byte 12 are 13 are set and zero, as in your case.

> In case you want to have a look to the implementation, I'm working on
> this branch:
> https://github.com/JoseExposito/linux/commits/patch-xppen-deco-l
> 
> I had to introduce new functionalities to the templating system of the
> driver, that's the reason for the KUnit tests.
> The last patch is work in progress (hopefully I'll have time to finish
> it this weekend), only the HID descriptors are missing.

I hope you make it, but otherwise you can always use mine, as it's quite
the same as yours. Only the logical minimum and maximum are -60 and 60.

> I'll cc you when in the patchset so you can add your IDs :)

Is that all you want me to do?

Kind regards,
Stefan Berzl

Re: Status on hid xppen patch

[José Expósito]

Hi Stefan,

On Fri, May 13, 2022 at 07:22:49PM +0200, Stefan Berzl wrote:
> Hello José,
> 
> > The tablets are the Deco Mini 4 and the Deco L, both of them are UGEE
> > tablets. I already had a UGEE Parblo A610 Pro tablet and after having a
> > look to the Windows driver traffic, I found out that after sending a
> > chunk of magic data to enable the tablet, it requests a string
> > descriptor ("uclogic_params_get_str_desc" can be used here) and the
> > tablets respond with their parameters.
> > 
> > The information is encoded, in bytes, as:
> > 
> >  02 + 03 - UCLOGIC_RDESC_PEN_PH_ID_X_LM
> >  04 + 05 - UCLOGIC_RDESC_PEN_PH_ID_Y_LM
> >  06      - Number of buttons
> >  07      - Dial present or not
> >  08 + 09 - UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM
> >  10 + 11 - Resolution
> > 
> > Bytes 12 and 13 are present but set to 0, probably indicating my
> > tablets are lacking some feature.
> > 
> > Could you confirm that your tablet returns similar information, please?
> 
> yes, you can haz string descriptor:
> 0e 03 0b 8b cb 56 08 00 ff 1f d8 13
> 
> Byte 12 are 13 are set and zero, as in your case.

Sweet, that means that we are on the right track and the implementation
can be generic... Meaning that we won't need to write and maintain a
bunch of HID descriptors :)
 
> > In case you want to have a look to the implementation, I'm working on
> > this branch:
> > https://github.com/JoseExposito/linux/commits/patch-xppen-deco-l
> > 
> > I had to introduce new functionalities to the templating system of the
> > driver, that's the reason for the KUnit tests.
> > The last patch is work in progress (hopefully I'll have time to finish
> > it this weekend), only the HID descriptors are missing.
> 
> I hope you make it, but otherwise you can always use mine, as it's quite
> the same as yours. Only the logical minimum and maximum are -60 and 60.

You are right, 60 and not 127 is the right value. Actually, I think
that -61 - 60 is the correct range, because of the 0.
Running "libinput-debug-tablet" makes it easier to debug.

I also had to fix the descriptor to avoid an issue with the pressure
causing issues with the Deco L, but other than that, it should be
correct now.

> > I'll cc you when in the patchset so you can add your IDs :)
> 
> Is that all you want me to do?

Code comments and suggestions are very welcome, as well as testing.

Also, I don't know if you have seen this error after connecting the
tablet:

  xhci_hcd 0000:2a:00.3: WARN urb submitted to disabled ep
  usb 3-2: reset full-speed USB device number 6 using xhci_hcd
  usb 3-2: reset full-speed USB device number 6 using xhci_hcd
  [...]

It happens with the Deco Mini 4, even when using the hid-generic
driver. I need to rebase my patches on 5.19 and test again, just to
make sure the problem is not somewhere else.

Jose

Re: Status on hid xppen patch

[Stefan Berzl]

Yo José,

>>> I had to introduce new functionalities to the templating system of the
>>> driver, that's the reason for the KUnit tests.
>>> The last patch is work in progress (hopefully I'll have time to finish
>>> it this weekend), only the HID descriptors are missing.
>>
>> I hope you make it, but otherwise you can always use mine, as it's quite
>> the same as yours. Only the logical minimum and maximum are -60 and 60.
> 
> You are right, 60 and not 127 is the right value. Actually, I think
> that -61 - 60 is the correct range, because of the 0.
> Running "libinput-debug-tablet" makes it easier to debug.
> 
> I also had to fix the descriptor to avoid an issue with the pressure
> causing issues with the Deco L, but other than that, it should be
> correct now.

I don't think the two's complement thing has any bearing here, because
there are still several numbers left in both directions. Also I couldn't
get the mini 7 to produce tilt values either >60 or <-60,
but I'm sure you'll get behind it.

> Also, I don't know if you have seen this error after connecting the
> tablet:
> 
>   xhci_hcd 0000:2a:00.3: WARN urb submitted to disabled ep
>   usb 3-2: reset full-speed USB device number 6 using xhci_hcd
>   usb 3-2: reset full-speed USB device number 6 using xhci_hcd
>   [...]
> 
> It happens with the Deco Mini 4, even when using the hid-generic
> driver. I need to rebase my patches on 5.19 and test again, just to
> make sure the problem is not somewhere else.

Can't say I have seen it. The only thing I can think of is that I was
using the async urb functions and that delay did something about it.

Kind regards

Stefan Berzl

Re: Status on hid xppen patch

[José Expósito]

On Mon, May 16, 2022 at 12:45:48PM +0200, Stefan Berzl wrote:
> Yo José,
> 
> > Also, I don't know if you have seen this error after connecting the
> > tablet:
> > 
> >   xhci_hcd 0000:2a:00.3: WARN urb submitted to disabled ep
> >   usb 3-2: reset full-speed USB device number 6 using xhci_hcd
> >   usb 3-2: reset full-speed USB device number 6 using xhci_hcd
> >   [...]
> > 
> > It happens with the Deco Mini 4, even when using the hid-generic
> > driver. I need to rebase my patches on 5.19 and test again, just to
> > make sure the problem is not somewhere else.
> 
> Can't say I have seen it. The only thing I can think of is that I was
> using the async urb functions and that delay did something about it.

Thanks, I'll give it a try. I hope is not some kind of time based
condition.

By the way, I only see the error when the tablet is connected. If I run
"usbhid-dump" or unload and load the driver, it works as expected.
I don't know if this could be related to the driver failing to process
the response to the magic data... Investigating.

Thanks,
Jose