larixer / hid-asus-dkms

ASUS HID FTE100* DKMS Driver
GNU General Public License v2.0
68 stars 10 forks source link

(!!) BUG: Device "Asus TouchPad" received a double tracking ID 61 in slot 1. #54

Open larixer opened 6 years ago

larixer commented 6 years ago

I see the following errors in /var/log/lightdm/x-0.log:

(==) Using system config directory "/usr/share/X11/xorg.conf.d"
(!!) BUG: Device "Asus TouchPad" received a double tracking ID 60 in slot 1.
(!!) BUG: Device "Asus TouchPad" received a double tracking ID 61 in slot 1.
(!!) BUG: Device "Asus TouchPad" received a double tracking ID 62 in slot 1.
(!!) BUG: Device "Asus TouchPad" received a double tracking ID 63 in slot 1.
(!!) BUG: Device "Asus TouchPad" received a double tracking ID 64 in slot 1.
...

I'm using TouchPad driver from Ubuntu kernel 4.10.0-42-generic. My system is Ubuntu 16.04.3 LTS x-0.log

larixer commented 6 years ago

@redmcg @ain101 Guys, do you see these warning in your display manager logs? I wonder if these are the issues of TouchPad driver code or is it something else.

redmcg commented 6 years ago

I see it too. You can find the line of code that produces that output here: https://anonscm.debian.org/cgit/collab-maint/libevdev.git/tree/libevdev/libevdev.c#n1005

The output means that slot 1 is receiving a new tracking ID before being reset to '-1'.

The hid-asus driver doesn't manage these tracking numbers directly - it is using input_mt_report_slot_state from within drivers/input/input-mt.c to do so.

The only time I can see that it would create a new tracking ID for a slot without first going to '-1' is when the 'Tool Type' changes: https://github.com/torvalds/linux/blob/master/drivers/input/input-mt.c#L154

This can happen when you go from 'Finger' to 'Palm' - basically by starting with one finger on the touchpad and then without lifting that finger place your palm on the touchpad (probably easiest done by starting with your little finger).

Reading Documentation/input/multi-touch-protocol.txt I found the following:

For type B devices, the kernel driver should associate a slot with each identified contact, and use that slot to propagate changes for the contact. Creation, replacement and destruction of contacts is achieved by modifying the ABS_MT_TRACKING_ID of the associated slot. A non-negative tracking id is interpreted as a contact, and the value -1 denotes an unused slot.

So I would consider a change of tracking id (when the tool changes) valid usage of the input API. Hence I believe libevdev to be incorrect in assuming the tracking ID will change to '-1' before being reassigned (thus the 'BUG' report is incorrect).

larixer commented 6 years ago

@redmcg Thank you for taking time for deep insight. Both your understanding and how libevdev works make sense to me so I'm unsure. I have asked Benjamin Tissoires to step in and clarify how things are supposed to work in the kernel.

bentiss commented 6 years ago

adding @whot to the thread, as he is the main user of the mt protocol.

The problem here is that the doc is not always accurate, and when userspace starts interpreting the doc, and the kernel behaves properly with the userspace, we mostly have to change the doc.

And here, to me, the fact that we have in input-mt.c if (id < 0 || input_mt_get_value(slot, ABS_MT_TOOL_TYPE) != tool_type) is clearly a bug. Looking at the origin of this, it was a move from the original hid-3m-pct.c, which was later transformed into hid-multitouch among others.

At that time the MT_TOOL_TYPE flag was not used much and that's why we never realized this could happen.

IMO, input-mt.c should be fixed to remove the second part of the if statement after the logical OR.

One other thing that worries me, is that hid-asus seems to switch between palm and finger when I see the logs in comment one. Normally, userspace expects a finger to be switched into a palm, but not the other way around. If the hardware can not decide, hid-asus should be patched to tag the slots itself and when the palm is detected, stick to it, and not switch it back to a regular finger.

So IMO, hid-asus should also be patched to not trust the hardware for the palm misdetection and be more restrictive: if a contact is marked once as a palm, it should stay so.

whot commented 6 years ago

I think we had the discussion about whether you can directly switch tracking IDs a few years back and Henrik, the author of the protocol, says it was designed to do that but the basic problem is now: too much userspace expects the tracking ID to be -1 before you switch to something else. This isn't just libevdev, it merely warns about it. But all of the X drivers and libinput wouldn't notice a direct tracking ID change. We can update all those, but realistically that ship has sailed, you could not ship a kernel that does direct changes for years...

As for the palm/finger tool change, I'm less opposed to it than @bentiss. It's not ideal and we probably consider it "once palm, always palm" but that's a userspace decision. Still, it depends on the real events, if the same touch changes multiple times between finger and palm, then the detection is effectively worthless to us in userspace.