Bug 207235

Summary: Pointingstick doesn't work on Synaptics touchpad
Product: Drivers Reporter: Kai-Heng Feng (kai.heng.feng)
Component: Input DevicesAssignee: drivers_input-devices
Status: RESOLVED CODE_FIX    
Severity: normal CC: benjamin.tissoires, kai.heng.feng
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: mainline Subsystem:
Regression: No Bisected commit-id:
Attachments: dmesg with i2c-hid.debug=1
output of hid-recorder
proposed solution
0001-HID-multitouch-enable-multi-input-as-a-quirk-for-som.patch

Description Kai-Heng Feng 2020-04-14 08:30:54 UTC
There's input report (with length 6) coming from i2c-hid. However HID layer doesn't seem to send those report to userspace.
Comment 1 Kai-Heng Feng 2020-04-14 08:31:47 UTC
Created attachment 288435 [details]
dmesg with i2c-hid.debug=1
Comment 2 Kai-Heng Feng 2020-04-14 08:32:25 UTC
In addition to pointing stick, buttons for pointing stick also doesn't work.
Comment 3 Kai-Heng Feng 2020-04-14 08:34:28 UTC
Furthermore, pointing stick works fine under Windows.
Comment 4 Benjamin Tissoires 2020-04-14 08:41:06 UTC
The logs are incomplete (not your fault, the debug option truncates the data). Can you attach the output of hid-recorder from https://gitlab.freedesktop.org/libevdev/hid-tools/ ?
Comment 5 Kai-Heng Feng 2020-04-14 09:04:02 UTC
Created attachment 288437 [details]
output of hid-recorder
Comment 6 Benjamin Tissoires 2020-04-14 09:29:12 UTC
Thanks for the logs.

I can not come up with a quick solution for this one. The problem is that we have 2 mouse collections, and of course, the trackstick is using the second one.

It is rather weird that the input report gets entirely omitted by the kernel, so I would need to debug it further.
The problem is that we are making the assumption that splitting by application is a good thing, when clearly Windows doesn't do that, and this bites us hard on this one.
Comment 7 Kai-Heng Feng 2020-04-14 09:39:55 UTC
I think I am not up to the task :)
But if I want to resolve this issue, is the fix in hid-multitouch, or we also need to modify hid-core?
Comment 8 Benjamin Tissoires 2020-04-14 09:47:37 UTC
> But if I want to resolve this issue, is the fix in hid-multitouch, or we also
> need to modify hid-core?

I think it would be a hid-core issue. We should find a middle point between HID_MULTI_INPUT and HID_QUIRK_INPUT_PER_APP to not break any devices and to not have to manually quirk some others.

Maybe as a quick workaround you can try if setting HID_MULTI_INPUT instead of HID_QUIRK_INPUT_PER_APP in hid-multitouch.c would solve the issue here, but this can not be the final solution because defaulting to multi_input is not good too :/
Comment 9 Kai-Heng Feng 2020-04-14 10:15:03 UTC
Yes, use HID_QUIRK_MULTI_INPUT instead of HID_QUIRK_INPUT_PER_APP can workaround the issue.
Comment 10 Kai-Heng Feng 2020-04-14 18:40:41 UTC
Created attachment 288453 [details]
proposed solution

This patch can solve the issue.
It uses application id to differentiate application usages.
Comment 11 Benjamin Tissoires 2020-05-14 12:47:06 UTC
Thanks for the patch. I am trying to get my head around it, and I don't think this solution will work. Basically, this patch converts HID_QUIRK_INPUT_PER_APP into HID_MULTI_INPUT.

The problem is that some devices (Advanced Silicons and Ilitek) have the multitouch feature to control the touchscreen/touchpad mode on a different report ID.

So this patch would break those devices.

I am currently planning to fix that in a 2 steps way:

- firstly, fix the bogus behavior. If 2 reports are sharing the same hidinput, we should not ignore the second. This should at least give us a patch we can backport to stable and that will not be too intrusive

- then, secondly, change the default HID_QUIRK_INPUT_PER_APP to merge it with HID_MULTI_INPUT. We need to split by report ID, even in the same application usage, unless there is only on Input report ID in the given application. If there is one report ID, then any output/feature report matching the application should be associated with this hidinput.
Comment 12 Kai-Heng Feng 2020-05-14 14:07:18 UTC
Thanks for your explanation. I'll wait for the patch to test.
Comment 13 Benjamin Tissoires 2020-05-26 08:59:32 UTC
(In reply to Benjamin Tissoires from comment #11)
> - firstly, fix the bogus behavior. If 2 reports are sharing the same
> hidinput, we should not ignore the second. This should at least give us a
> patch we can backport to stable and that will not be too intrusive
> 

That part tends to be harder than what I initially thought.
The reason we ignore the events is not because the entire collection is ignored, but because the X and Y axis, and left and right buttons are already mapped to the mouse emulation mode.

So a dirty workaround would be to not ignore new axes, but overwrite them. It could be OK in this situation (relative axis and buttons), but will likely not work properly for absolute axes.

So I am leaning towards a quirk solution right now. I can't really switch back to the solution of using MULTI_INPUT as default on hid-multitouch. There is at least one device that really doesn't like it in my test suite, and that means it is likely that others would be broken if we change this behaviour and backport it to stable.

The quirk solution has the advantage of being able to be backported to stable, so I'll likely submit that solution in the interim.
Comment 14 Benjamin Tissoires 2020-05-26 09:06:46 UTC
Created attachment 289283 [details]
0001-HID-multitouch-enable-multi-input-as-a-quirk-for-som.patch

proposed patch
Comment 15 Kai-Heng Feng 2020-05-27 06:21:12 UTC
This patch works for me. Thanks!
Comment 16 Kai-Heng Feng 2020-06-02 09:37:52 UTC
commit 40d5bb87377a599d0405af765290f28aaa6abb1e
Author: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Date:   Tue May 26 17:07:17 2020 +0200

    HID: multitouch: enable multi-input as a quirk for some devices