On Wed, Oct 20, 2021 at 10:04 PM Benjamin Tissoires
[off-list ref] wrote:

On Wed, Oct 20, 2021 at 1:34 PM Alistair Francis [off-list ref] wrote:

quoted

On Wed, Oct 20, 2021 at 5:40 PM Benjamin Tissoires
[off-list ref] wrote:

quoted

On Wed, Oct 20, 2021 at 4:14 AM Dmitry Torokhov
[off-list ref] wrote:

quoted

On Wed, Oct 20, 2021 at 11:44:50AM +1000, Alistair Francis wrote:

quoted

On Wed, Oct 20, 2021 at 11:05 AM Dmitry Torokhov
[off-list ref] wrote:

quoted

On Wed, Oct 20, 2021 at 09:33:13AM +1000, Alistair Francis wrote:

quoted

On Tue, Oct 19, 2021 at 11:51 AM Dmitry Torokhov
[off-list ref] wrote:

quoted

We already have touchscreen-inverted-x/y defined in
Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml,
why are they not sufficient?

The touchscreen-* properties aren't applied to HID devices though, at
least not that I can tell.

No, they are not currently, but that does not mean we need to establish
a new set of properties (property names) for HID case.

I can update the names to use the existing touchscreen ones.

Do you have a hint of where this should be implemented though?

Right now (without "HID: wacom: Add support for the AG14 Wacom
device") the wacom touchscreen is just registered as a generic HID
device. I don't see any good place in hid-core, hid-input or
hid-generic to invert the input values for this.

I think the transformation should happen in
hid-multitouch.c::mt_process_slot() using helpers from
include/linux/input/touchscreen.h

I think the more challenging question is to how pass/attach struct
touchscreen_properties * to the hid device (i expect the properties will
be attached to i2c-hid device, but maybe we could create a sub-node of
it and attach properties there.

Sorry but I don't like that very much. This would mean that we have an
out of band information that needs to be carried over to
HID-generic/multitouch and having tests for it is going to be harder.
I would rather have userspace deal with the rotation if we do not have
the information from the device itself.

My 2c below

quoted

Foreword: I have been given a hammer, so I see nails everywhere.

The past 3 weeks I have been working on implementing some eBPF hooks
in the HID subsystem. This would IMO be the best solution here: a udev
hwdb rule sees that there is the not-wacom PID/VID (and maybe the
platform or parses the OF properties if they are available in the

I'm not sure we have a specific VID/PID to work with here. The VID is
generic AFAIK, not sure about the PID though. Maybe someone from Wacom
could confirm either way.

It actually doesn't really matter. What matters is that there is a way
to know that this device needs to be rotated, being through DT
properties that would be exported through sysfs, or a hwdb entry that
matches on the product, the platform or something else.

quoted

quoted

sysfs) and adds a couple of functions in the HID stack to rotate the
screen. The advantage is that we do not need to add a new kernel API

I would say that touchscreen-inverted-x/y isn't a new API, it's
commonly used. To me it makes sense that it's supported for all
touchscreens.

Well, it's new in the HID world, and this is opening the pandora box:
the patch adds only the equivalent of touchscreen-inverted-x/y, but
not touchscreen-swapped-x-y. So you can not actually rotate a screen
by 90 degrees.

Inverting a value on an axis is easy. Swapping 2 axes is way harder in
the HID world, because you have to interpret the report descriptor
differently.

Also, the patch adds 3 new properties: flip-tilt-x/y and flip-distance.

This patch does yes, but I'm happy to just drop this to the invert
touchscreen properties.

The tilt and distance would be easy, but suddenly we need to also add
pressure, and all of the other HID definitions. This is going to be
endless. It took me a while to understand Rob's point regarding
generic properties, but we are exactly entering this territory: this
is an endless chase and will never end.

I would much rather have a device specific quirk that would be
triggered by the DT than adding generic properties like that.

That works for me!

A HID_QUIRK_XY_INVERT would work for me and seems useful for others in
the future as well.

I managed to figure out how to do this, I'll send a patch soon.

Also, hid-multitouch is the most tested driver through the hid-tools
test suite: https://gitlab.freedesktop.org/libevdev/hid-tools
I am not sure how I can add tests for those properties in a generic
way (the creation of the "virtual DT" is going to be problematic).

On the contrary, a device specific quirk can easily be tested without
having to mess too much with the hid subsystem.

Great!

Alistair

quoted

quoted

anymore, the disadvantage is that we need userspace to "fix" the
kernel behaviour (so at boot, this might be an issue).

That's a pain for me. I'm still stuck with the vendors userspace as I
need their propiritory eInk driver code. It also seems like a hassle
for different distros to handle this (compared to just in the DT).

I understand the pain. But I am not talking about a 1 kernel cycle
release timeframe. More like 6-12 months to bring in all the pieces
together. Distributions have no issues with udev most of the time
(even those that stuck to the old pre-systemd fork), and it would not
be different than having a udev intrinsic that tags the pen with
ID_INPUT_TABLET so libinput and others can deal with it.

Cheers,
Benjamin

quoted

Alistair

quoted

I am not at the point where I can share the code as there is a lot of
rewriting and my last attempt is resulting in a page fault, but I'd be
happy to share it more once that hiccup is solved.

Cheers,
Benjamin