Bug 43591

Summary: Sentelic two finger scrolling/tapping incompatible
Product: Drivers Reporter: Jamie Kitson (jamie)
Component: Input DevicesAssignee: drivers_input-devices
Status: RESOLVED INVALID    
Severity: normal CC: christophe, dmitry.torokhov, kernel-bugzilla.3.atliang, ogoffart
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 3.4.2 Subsystem:
Regression: No Bisected commit-id:
Attachments: Patch working against 3.6 and 3.7
patch: fix buggy aboslute position events in sentelic.c

Description Jamie Kitson 2012-06-19 18:46:21 UTC
Since 3.4 Sentelic touchpads have had scrolling support. For me this works as expected at the edge, smoothly and accurately. However, two finger scrolling is erratic and inaccurate. Very short, gentle two finger scrolling works ok, however longer and faster two finger scrolling results in sudden jumps, sometimes in the wrong direction. Incidentally I previously the Saaros patch and didn't experience this erratic behaviour.

http://github.com/saaros/sentelic

[jamie@jamie-laptop ~]$ dmesg | grep Fing
[    1.649998] psmouse serio4: sentelic: Finger Sensing Pad, hw: 14.3.1, sw: 1.0.0-K
[    1.692960] input: FSPPS/2 Sentelic FingerSensingPad as /devices/platform/i8042/serio4/input/input7
[jamie@jamie-laptop ~]$ uname -a
Linux jamie-laptop 3.4.2-2-ARCH #1 SMP PREEMPT Mon Jun 11 22:27:17 CEST 2012 x86_64 GNU/Linux
Comment 1 Jamie Kitson 2012-06-19 22:01:22 UTC
After a bit of investigation I have found that two finger scrolling and two finger tapping interfere with each other. ie, if you disable one, the other works as expected.
Comment 2 Dmitry Torokhov 2012-06-25 07:55:13 UTC
The kernel does not process scrolling and tapping gestures, it simply emits stream of coordinates and it is responsibility of X driver to parse and interpret them. You should report this issue to xf86-input-synaptics.

Thanks.
Comment 3 Jamie Kitson 2012-06-25 10:00:25 UTC
I will do so, however I do believe that the issue is with the driver, since this issue seems specific to my Sentelic touchpad and wasn't an issue with the Saaros patch.
Comment 4 Jamie Kitson 2012-06-25 20:35:26 UTC
https://bugs.freedesktop.org/show_bug.cgi?id=51403
Comment 5 Olivier Goffart 2012-06-28 06:20:18 UTC
As I had the same problem, I investigated and fixed the problem.
Other mouse drivers such as elentec always send both touch point together, while the sentelic send them one by one.

I created a patch: https://lkml.org/lkml/2012/6/27/521
Comment 6 Jamie Kitson 2012-06-28 09:08:01 UTC
Thanks Olivier. Dmitry, are you happy to accept this bug as "two finger tapping doesn't work" or should I create a new item?
Comment 7 Dmitry Torokhov 2012-06-28 18:24:46 UTC
Once again, the issue seems to be in the X driver: if the protocol is defined such that it is acceptable to send only new data in the event packet (MT-B protocol) then the driver should handle that. The fact that the other drivers might sent data differently does not matter; also, input core will attempt filter out duplicate events when dealing with MT-B devices.

Of course there still may be issue in the kernel driver, but then your (Olivier's) description of the issue is not correct/complete.
Comment 8 Christophe Tordeux 2012-12-14 01:07:50 UTC
I have this issue on my Sentelic touchpad too. I'm sorry I created a duplicate of this bug. You can declare #51611 as a duplicate of this.

I also made a patch to fix it, you can get the patch at http://sentelic.pkbd.org. 

What happens is that the Sentelic kernel driver sends coordinates to the synaptics xorg driver without reworking the x and y absolute coordinates. This confuses the synaptics driver, horizontal two-finger scrolling is non-functional, vertical two-finger scrolling lacks smoothness.

The Synaptics kernel driver reworks x and y coordinates before sending them to the synaptics xorg driver. The first finger reported by the kernel is reported with x and y coordinates which are the min of the actual physical x and y coordinates of the user's two fingers. The reported second finger is the max of both physical fingers of the user. Apparently this is what the synaptics xorg driver needs to handle them properly.

On the other hand you don't need to report both fingers every time you report mt events. You can fix the bug simply by having the Sentelic kernel driver report min and max virtual fingers like the synaptics kernel driver does.

2 important considerations: 

1) I have not yet studied the code for the synaptics xorg driver. However, I suspect attempts to solve the issue in this driver for Sentelic touchpads will result in breaking the behavior of devices handled by the synaptics kernel driver.

2) Please note that, practically speaking, the MT-B protocol is totally irrelevant to the resolution of this bug using the approach I propose. The reason is that whatever events of the type ABS_MT_POSITION are reported by the Sentelic kernel driver are utterly ignored. What actually moves the cursor of multitouch Sentelic touchpads are plain and simple ABS_X and ABS_Y events, totally unrelated to the MT protocol. For consistency, my proposed patch alters both MT protocol events and regular absolute position events in the same way. However, should you change only the way ABS_X and ABS_Y events are reported, you'd get the same result.
Comment 9 Olivier Goffart 2012-12-14 09:19:58 UTC
Created attachment 89191 [details]
Patch working against 3.6 and 3.7

@Cristophe   I tried your patch. While it improves scrilling, it is still not working with double tap to simulate middle click.

I think the real issue is that we report the fingers one by one rather than together like other drivers do.

I submitted a patch long time ago already but it was not accepted upstream because they claim it is a userspace bug.  But I still think it deserve a workaround, because scrolling and middle-click just do not work currently.

I attached an updated version of my patch.
Comment 10 Christophe Tordeux 2012-12-15 07:46:56 UTC
Created attachment 89261 [details]
patch: fix buggy aboslute position events in sentelic.c
Comment 11 Christophe Tordeux 2012-12-15 07:47:14 UTC
Thanks for your feedback Olivier. Care to tell me what version is your touchpad?

I just tried your patch, and I confirm that it does bring the extra fix you claim it does, which makes it indeed better than mine. Alas, it has been refused, not on the basis that it does not respect the MT-B protocol, but on the basis that it does not respect the protocol more than existing code... which somehow makes any fixes and improved consistency in dealings with userspace irrelevant... if I follow well.

What I'm going to do, I'm going to submit a new version of my patch,which will be completely void of any impact on MT events, and therefore totally orthogonal to the MT protocol, and hope that we hear word from the maintainer.

As for describing the issue, the driver is buggy in that it reports non-MT absolute position events associated to the coordinates of each of 2 fingers alternatively, a situation which no sane userspace program can be expected to handle. My patch fixes that by having a unique and consistent virtual finger be reported by these events, and therefore constitutes an obvious bug fix.

Although we are probably wasting our time, since we should be fixing the "bug" of the xorg driver, and then we should be asking the guy who owns a touchpad from each and every vendor to check that we didn't break any non-Sentelic touchpad.
Comment 12 Christophe Tordeux 2012-12-15 07:51:25 UTC
(In reply to comment #2)
> The kernel does not process scrolling and tapping gestures, it simply emits
> stream of coordinates and it is responsibility of X driver to parse and
> interpret them. You should report this issue to xf86-input-synaptics.
> 
> Thanks.

The Sentelic driver indeed reports tapping gestures. It reports one-finger AND two-finger tappings as left clicks.
Comment 13 Christophe Tordeux 2012-12-19 23:40:18 UTC
You can find an updated and much simpler version of my patch, with the same effect, at this address:

https://lkml.org/lkml/2012/12/19/402