* [PATCH 0/2] 2 somewhat related input fixes @ 2015-04-22 15:45 Benjamin Tissoires 2015-04-22 15:45 ` [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW Benjamin Tissoires 2015-04-22 15:45 ` [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers Benjamin Tissoires 0 siblings, 2 replies; 12+ messages in thread From: Benjamin Tissoires @ 2015-04-22 15:45 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, linux-kernel Hi Dmitry, here are 2 somewhat related patches that are needed when the hardware reports a number of fingers greater than max slot while not filling all of the available slots. In such cases, libinput relies on the actual slot number (which should be correct) and this is not what the hardware says. The first one is marked for stable given that it has been there forever (2011) and seems simple enough to be in stable. The second one is a regression introduced in 4.1-rc0, so that would be sweet to have it in 4.1 final. Cheers, Benjamin Benjamin Tissoires (2): Input - elantech: fix semi-mt protocol for v3 HW Input - synaptics: pin 3 touches when the firmware reports 3 fingers drivers/input/mouse/elantech.c | 2 +- drivers/input/mouse/synaptics.c | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW 2015-04-22 15:45 [PATCH 0/2] 2 somewhat related input fixes Benjamin Tissoires @ 2015-04-22 15:45 ` Benjamin Tissoires 2015-04-23 16:09 ` Dmitry Torokhov 2015-04-22 15:45 ` [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers Benjamin Tissoires 1 sibling, 1 reply; 12+ messages in thread From: Benjamin Tissoires @ 2015-04-22 15:45 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, linux-kernel When the v3 hardware sees more than one finger, it uses the semi-mt protocol to report the touches. However, it currently works when num_fingers is 0, 1 or 2, but when it is 3 and above, it sends only 1 finger as if num_fingers was 1. This confuses userspace which knows how to deal with extra fingers when all the slots are used, but not when some are missing. Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=90101 CC: stable@vger.kernel.org Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/input/mouse/elantech.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c index 991dc6b..79363b6 100644 --- a/drivers/input/mouse/elantech.c +++ b/drivers/input/mouse/elantech.c @@ -315,7 +315,7 @@ static void elantech_report_semi_mt_data(struct input_dev *dev, unsigned int x2, unsigned int y2) { elantech_set_slot(dev, 0, num_fingers != 0, x1, y1); - elantech_set_slot(dev, 1, num_fingers == 2, x2, y2); + elantech_set_slot(dev, 1, num_fingers >= 2, x2, y2); } /* -- 2.1.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW 2015-04-22 15:45 ` [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW Benjamin Tissoires @ 2015-04-23 16:09 ` Dmitry Torokhov 0 siblings, 0 replies; 12+ messages in thread From: Dmitry Torokhov @ 2015-04-23 16:09 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: linux-input, linux-kernel On Wed, Apr 22, 2015 at 11:45:08AM -0400, Benjamin Tissoires wrote: > When the v3 hardware sees more than one finger, it uses > the semi-mt protocol to report the touches. However, it > currently works when num_fingers is 0, 1 or 2, but when > it is 3 and above, it sends only 1 finger as if num_fingers > was 1. > > This confuses userspace which knows how to deal with extra > fingers when all the slots are used, but not when some are > missing. > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=90101 > > CC: stable@vger.kernel.org > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Applied, thank you. > --- > drivers/input/mouse/elantech.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c > index 991dc6b..79363b6 100644 > --- a/drivers/input/mouse/elantech.c > +++ b/drivers/input/mouse/elantech.c > @@ -315,7 +315,7 @@ static void elantech_report_semi_mt_data(struct input_dev *dev, > unsigned int x2, unsigned int y2) > { > elantech_set_slot(dev, 0, num_fingers != 0, x1, y1); > - elantech_set_slot(dev, 1, num_fingers == 2, x2, y2); > + elantech_set_slot(dev, 1, num_fingers >= 2, x2, y2); > } > > /* > -- > 2.1.0 > -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-22 15:45 [PATCH 0/2] 2 somewhat related input fixes Benjamin Tissoires 2015-04-22 15:45 ` [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW Benjamin Tissoires @ 2015-04-22 15:45 ` Benjamin Tissoires 2015-04-23 16:38 ` Dmitry Torokhov 1 sibling, 1 reply; 12+ messages in thread From: Benjamin Tissoires @ 2015-04-22 15:45 UTC (permalink / raw) To: Dmitry Torokhov, linux-input, linux-kernel Synaptics PS/2 touchpad can send only 2 touches in a report. They can detect 4 or 5 and this information is valuable. In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep stability in image sensors"), we allocate 3 slots, but we still continue to report the 2 available fingers. That means that the client sees 2 used slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. For old kernels this is not a problem because max_slots was 2 and libinput/ xorg-synaptics knew how to deal with that. Now that max_slot is 3, the clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP information, and goes wild. We can pin the 3 slots until we get a total number of fingers below 2. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/input/mouse/synaptics.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c index 630af73..c69b308 100644 --- a/drivers/input/mouse/synaptics.c +++ b/drivers/input/mouse/synaptics.c @@ -935,6 +935,14 @@ static void synaptics_report_mt_data(struct psmouse *psmouse, input_report_abs(dev, ABS_MT_PRESSURE, hw[i]->z); } + /* keep (slot count <= num_fingers) by pinning all slots */ + if (num_fingers >= 3) { + for (i = 0; i < 3; i++) { + input_mt_slot(dev, i); + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true); + } + } + input_mt_drop_unused(dev); /* Don't use active slot count to generate BTN_TOOL events. */ -- 2.1.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-22 15:45 ` [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers Benjamin Tissoires @ 2015-04-23 16:38 ` Dmitry Torokhov 2015-04-23 18:48 ` Benjamin Tissoires 0 siblings, 1 reply; 12+ messages in thread From: Dmitry Torokhov @ 2015-04-23 16:38 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: linux-input, linux-kernel On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote: > Synaptics PS/2 touchpad can send only 2 touches in a report. They can > detect 4 or 5 and this information is valuable. > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep > stability in image sensors"), we allocate 3 slots, but we still continue > to report the 2 available fingers. That means that the client sees 2 used > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. > > For old kernels this is not a problem because max_slots was 2 and libinput/ > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > information, and goes wild. > > We can pin the 3 slots until we get a total number of fingers below 2. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 Benjamin, I do not quite like it. It seems that original patch was not quite right and we are adding more workarounds. Synaptics can only track 2 contacts, correct? Why 2 slots to track them is not enough? > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/input/mouse/synaptics.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > index 630af73..c69b308 100644 > --- a/drivers/input/mouse/synaptics.c > +++ b/drivers/input/mouse/synaptics.c > @@ -935,6 +935,14 @@ static void synaptics_report_mt_data(struct psmouse *psmouse, > input_report_abs(dev, ABS_MT_PRESSURE, hw[i]->z); > } > > + /* keep (slot count <= num_fingers) by pinning all slots */ > + if (num_fingers >= 3) { > + for (i = 0; i < 3; i++) { > + input_mt_slot(dev, i); > + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true); > + } > + } > + > input_mt_drop_unused(dev); > > /* Don't use active slot count to generate BTN_TOOL events. */ > -- > 2.1.0 > -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-23 16:38 ` Dmitry Torokhov @ 2015-04-23 18:48 ` Benjamin Tissoires 2015-04-24 22:50 ` Benjamin Tissoires 0 siblings, 1 reply; 12+ messages in thread From: Benjamin Tissoires @ 2015-04-23 18:48 UTC (permalink / raw) To: Dmitry Torokhov; +Cc: linux-input, linux-kernel On Apr 23 2015 or thereabouts, Dmitry Torokhov wrote: > On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote: > > Synaptics PS/2 touchpad can send only 2 touches in a report. They can > > detect 4 or 5 and this information is valuable. > > > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep > > stability in image sensors"), we allocate 3 slots, but we still continue > > to report the 2 available fingers. That means that the client sees 2 used > > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. > > > > For old kernels this is not a problem because max_slots was 2 and libinput/ > > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > > information, and goes wild. > > > > We can pin the 3 slots until we get a total number of fingers below 2. > > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 > > Benjamin, I do not quite like it. It seems that original patch was not > quite right and we are adding more workarounds. Agree. And I am starting to hate more and more the synaptics PS/2 and all the PS/2 drivers to be honest :) - trying to fit a heavy load data like multitouch in a simple and lightweight protocol like PS/2 is insane... We are internally trying to figure out if we can finally take advantage of the SMBus/RMI4 protocol, but we tried for one year without much success. > > Synaptics can only track 2 contacts, correct? Why 2 slots to track them > is not enough? IIRC, the problem was that upon a third finger down, with only 2 slots, the fingers were silently inverted in most cases. The thing is that the firmware forwards 2 fingers, but not necessarily the two first. So you generally get fingers 1+3 so the slot 2 needs to be removed. And that means the kernel tracking has to track 3 fingers upon transitions. This may be completely bullshit and we might not need to use 3 slots at all. I'll need to do further experiments to validate which one is best then. I am perfectly fine holding this one up for a little bit more testings and then we can decide which one needs to be done (revert or an other band-aid). Cheers, Benjamin > > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > drivers/input/mouse/synaptics.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c > > index 630af73..c69b308 100644 > > --- a/drivers/input/mouse/synaptics.c > > +++ b/drivers/input/mouse/synaptics.c > > @@ -935,6 +935,14 @@ static void synaptics_report_mt_data(struct psmouse *psmouse, > > input_report_abs(dev, ABS_MT_PRESSURE, hw[i]->z); > > } > > > > + /* keep (slot count <= num_fingers) by pinning all slots */ > > + if (num_fingers >= 3) { > > + for (i = 0; i < 3; i++) { > > + input_mt_slot(dev, i); > > + input_mt_report_slot_state(dev, MT_TOOL_FINGER, true); > > + } > > + } > > + > > input_mt_drop_unused(dev); > > > > /* Don't use active slot count to generate BTN_TOOL events. */ > > -- > > 2.1.0 > > > > -- > Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-23 18:48 ` Benjamin Tissoires @ 2015-04-24 22:50 ` Benjamin Tissoires 2015-04-25 9:40 ` Henrik Rydberg 2015-06-11 17:29 ` Benjamin Tissoires 0 siblings, 2 replies; 12+ messages in thread From: Benjamin Tissoires @ 2015-04-24 22:50 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, linux-kernel, Peter Hutterer, Hans de Goede, Henrik Rydberg Hi Dmitry, [ adding more relevant people to the discussion ] On Apr 23 2015 or thereabouts, Benjamin Tissoires wrote: > On Apr 23 2015 or thereabouts, Dmitry Torokhov wrote: > > On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote: > > > Synaptics PS/2 touchpad can send only 2 touches in a report. They can > > > detect 4 or 5 and this information is valuable. > > > > > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep > > > stability in image sensors"), we allocate 3 slots, but we still continue > > > to report the 2 available fingers. That means that the client sees 2 used > > > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. > > > > > > For old kernels this is not a problem because max_slots was 2 and libinput/ > > > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > > > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > > > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > > > information, and goes wild. > > > > > > We can pin the 3 slots until we get a total number of fingers below 2. > > > > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 > > > > Benjamin, I do not quite like it. It seems that original patch was not > > quite right and we are adding more workarounds. > > Agree. And I am starting to hate more and more the synaptics PS/2 and all > the PS/2 drivers to be honest :) - trying to fit a heavy load data like > multitouch in a simple and lightweight protocol like PS/2 is insane... > > We are internally trying to figure out if we can finally take advantage > of the SMBus/RMI4 protocol, but we tried for one year without much > success. > > > > > Synaptics can only track 2 contacts, correct? Why 2 slots to track them > > is not enough? > > IIRC, the problem was that upon a third finger down, with only 2 slots, > the fingers were silently inverted in most cases. The thing is that the > firmware forwards 2 fingers, but not necessarily the two first. So you > generally get fingers 1+3 so the slot 2 needs to be removed. And that > means the kernel tracking has to track 3 fingers upon transitions. > > This may be completely bullshit and we might not need to use 3 slots at > all. I'll need to do further experiments to validate which one is best > then. > > I am perfectly fine holding this one up for a little bit more testings > and then we can decide which one needs to be done (revert or an other > band-aid). > So I carefully recorded each situation (initial with 2 slots, 2 slots and then with the pinning in this patch*), and I am now convinced that the pinning is the best sequence that we forward to the user space (best among the 3). With 2 slots declared, there are 2 problems: - the first finger jumps to the position of the 3rd when it lands - the transition between 2 to 3 fingers goes to a state where the kernel removes the second finger (while jumping the first to the position of the 3rd finger), send a sync and then reallocate the first finger position as the second slot in use -> that means that user space sees a small transition where the slots count is 1 while the BTN_TOOL advertise triple tap :/ With 3 slots, we have the problem reported in the rhbz bug #1212230: - during the transition, the fingers are stable, but we have at most 2 active slots in one frame, which confuses libinput/xorg-synaptics. With the pinning, the user space is no more confused because BTN_TOOL is always greater or equal than the active slots. So I think for now we have 3 possibilities: 1. Just carry this patch, and hope that we will be able to switch the synaptics device in the non-PS/2 mode 2. Revert to 2 patches and fix the kernel tracking to accept 3 fingers and return the 2 best matches 3. Revert the use of the kernel tracking at all and re-introduce the spaghetti code that was here before and hope that all cases where properly handled. IMO that the solution 2. is the best, but I can not do it because I don't understand what the code does. I can guess things but I can not accurately change it because it is not readable IMO. (yes, there is also the solution 4: "screw up and let the user space deal with it", but I'd rather not do that given the history of the multitouch protocol) Cheers, Benjamin * I tried to put side by side the 3 test cases in the following logs: (init slots 2, no pinning) | (init slots 3, no pinning) | (init slots 3, pinning) ----------------------------- | ----------------------------- | --------------------------- ------------------------------|----- one finger down -------|---------------------------- | | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 ABS_MT_TRACKING_ID 53 | ABS_MT_TRACKING_ID 53 | ABS_MT_TRACKING_ID 53 ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 ABS_MT_PRESSURE 68 | ABS_MT_PRESSURE 68 | ABS_MT_PRESSURE 68 BTN_TOUCH 1 | BTN_TOUCH 1 | BTN_TOUCH 1 ABS_X 2000 | ABS_X 2000 | ABS_X 2000 ABS_Y 2000 | ABS_Y 2000 | ABS_Y 2000 ABS_PRESSURE 68 | ABS_PRESSURE 68 | ABS_PRESSURE 68 BTN_TOOL_FINGER 1 | BTN_TOOL_FINGER 1 | BTN_TOOL_FINGER 1 --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | | ... | ... | ... | | ------------------------------------ second finger down ------------------------------------ | | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 ABS_MT_SLOT 1 | ABS_MT_SLOT 1 | ABS_MT_SLOT 1 ABS_MT_TRACKING_ID 54 | ABS_MT_TRACKING_ID 54 | ABS_MT_TRACKING_ID 54 ABS_MT_POSITION_X 3000 | ABS_MT_POSITION_X 3000 | ABS_MT_POSITION_X 3000 ABS_MT_POSITION_Y 3000 | ABS_MT_POSITION_Y 3000 | ABS_MT_POSITION_Y 3000 ABS_MT_PRESSURE 64 | ABS_MT_PRESSURE 64 | ABS_MT_PRESSURE 64 ABS_X 2000 | ABS_X 2000 | ABS_X 2000 ABS_Y 2000 | ABS_Y 2000 | ABS_Y 2000 ABS_PRESSURE 78 | ABS_PRESSURE 78 | ABS_PRESSURE 78 BTN_TOOL_FINGER 0 | BTN_TOOL_FINGER 0 | BTN_TOOL_FINGER 0 BTN_TOOL_DOUBLETAP 1 | BTN_TOOL_DOUBLETAP 1 | BTN_TOOL_DOUBLETAP 1 --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | | ... | ... | ... | | ------------------------------------ third finger down ------------------------------------ | | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 ABS_MT_POSITION_Y 4000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 ABS_MT_SLOT 1 | ABS_MT_SLOT 2 | ABS_MT_SLOT 2 ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID 55 | ABS_MT_TRACKING_ID 55 | ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_Y 4000 | ABS_MT_POSITION_Y 4000 | ABS_MT_PRESSURE 72 | ABS_MT_PRESSURE 72 | ABS_MT_SLOT 1 | | ABS_MT_TRACKING_ID -1 | ABS_X 4000 | ABS_X 2000 | ABS_X 2000 ABS_Y 4000 | ABS_Y 2000 | ABS_Y 2000 ABS_PRESSURE 78 | ABS_PRESSURE 78 | ABS_PRESSURE 78 BTN_TOOL_DOUBLETAP 0 | BTN_TOOL_DOUBLETAP 0 | BTN_TOOL_DOUBLETAP 0 BTN_TOOL_TRIPLETAP 1 | BTN_TOOL_TRIPLETAP 1 | BTN_TOOL_TRIPLETAP 1 --- SYN_REPORT (0) ---------- | | ABS_MT_SLOT 0 | | ABS_MT_POSITION_X 4000 | | ABS_MT_POSITION_Y 4000 | | ABS_MT_PRESSURE 78 | | ABS_MT_SLOT 1 | | ABS_MT_TRACKING_ID 55 | | ABS_MT_POSITION_X 2000 | | ABS_MT_POSITION_Y 2000 | | ABS_MT_PRESSURE 72 | | ABS_X 4000 | | ABS_Y 4000 | | ABS_PRESSURE 78 | | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | | ... | ... | ... | | ------------------------------------ 3 fingers release ------------------------------------ | | | | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | | ABS_MT_SLOT 2 | | ABS_MT_POSITION_X 4000 | | ABS_MT_POSITION_Y 4000 | | ABS_MT_PRESSURE 45 ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 ABS_MT_SLOT 1 | ABS_MT_SLOT 2 | ABS_MT_SLOT 1 ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | | ABS_X 4000 | | ABS_Y 4000 BTN_TOUCH 0 | BTN_TOUCH 0 | ABS_PRESSURE 45 ABS_PRESSURE 0 | ABS_PRESSURE 0 | BTN_TOOL_FINGER 1 BTN_TOOL_TRIPLETAP 0 | BTN_TOOL_TRIPLETAP 0 | BTN_TOOL_TRIPLETAP 0 --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | | ABS_MT_SLOT 2 | | ABS_MT_TRACKING_ID -1 | | BTN_TOUCH 0 | | ABS_PRESSURE 0 | | BTN_TOOL_FINGER 0 | | --- SYN_REPORT (0) ---------- ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-24 22:50 ` Benjamin Tissoires @ 2015-04-25 9:40 ` Henrik Rydberg 2015-04-27 17:48 ` Benjamin Tissoires 2015-06-11 17:29 ` Benjamin Tissoires 1 sibling, 1 reply; 12+ messages in thread From: Henrik Rydberg @ 2015-04-25 9:40 UTC (permalink / raw) To: Benjamin Tissoires, Dmitry Torokhov Cc: linux-input, linux-kernel, Peter Hutterer, Hans de Goede Benjamin, >>>> For old kernels this is not a problem because max_slots was 2 and libinput/ >>>> xorg-synaptics knew how to deal with that. Now that max_slot is 3, the >>>> clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). >>>> It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP >>>> information, and goes wild. Maybe the cr48 sensor should be classified as MT_SEMI instead. Henrik ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-25 9:40 ` Henrik Rydberg @ 2015-04-27 17:48 ` Benjamin Tissoires 0 siblings, 0 replies; 12+ messages in thread From: Benjamin Tissoires @ 2015-04-27 17:48 UTC (permalink / raw) To: Henrik Rydberg Cc: Dmitry Torokhov, linux-input, linux-kernel, Peter Hutterer, Hans de Goede Hi Henrik, On Apr 25 2015 or thereabouts, Henrik Rydberg wrote: > Benjamin, > > >>>> For old kernels this is not a problem because max_slots was 2 and libinput/ > >>>> xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > >>>> clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > >>>> It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > >>>> information, and goes wild. > > Maybe the cr48 sensor should be classified as MT_SEMI instead. > That's not a cr48 issue (actually the cr48 is also affected given that it allocates 2 slots). To be able to fix the cursors jumps that we see with any regular image sensor touchpad, I cleaned up all the code and relied on the kernel tracking to provide an actual tracking. So now, either the touchpad is old and it does forwards SEMI_MT, either it is new enough and uses the in-kernel tracking. Cheers, Benjamin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-04-24 22:50 ` Benjamin Tissoires 2015-04-25 9:40 ` Henrik Rydberg @ 2015-06-11 17:29 ` Benjamin Tissoires 2015-07-01 0:26 ` Dmitry Torokhov 1 sibling, 1 reply; 12+ messages in thread From: Benjamin Tissoires @ 2015-06-11 17:29 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, linux-kernel, Peter Hutterer, Hans de Goede, Henrik Rydberg On Apr 24 2015 or thereabouts, Benjamin Tissoires wrote: > Hi Dmitry, > > [ adding more relevant people to the discussion ] > > On Apr 23 2015 or thereabouts, Benjamin Tissoires wrote: > > On Apr 23 2015 or thereabouts, Dmitry Torokhov wrote: > > > On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote: > > > > Synaptics PS/2 touchpad can send only 2 touches in a report. They can > > > > detect 4 or 5 and this information is valuable. > > > > > > > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep > > > > stability in image sensors"), we allocate 3 slots, but we still continue > > > > to report the 2 available fingers. That means that the client sees 2 used > > > > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. > > > > > > > > For old kernels this is not a problem because max_slots was 2 and libinput/ > > > > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > > > > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > > > > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > > > > information, and goes wild. > > > > > > > > We can pin the 3 slots until we get a total number of fingers below 2. > > > > > > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 > > > > > > Benjamin, I do not quite like it. It seems that original patch was not > > > quite right and we are adding more workarounds. > > > > Agree. And I am starting to hate more and more the synaptics PS/2 and all > > the PS/2 drivers to be honest :) - trying to fit a heavy load data like > > multitouch in a simple and lightweight protocol like PS/2 is insane... > > > > We are internally trying to figure out if we can finally take advantage > > of the SMBus/RMI4 protocol, but we tried for one year without much > > success. > > > > > > > > Synaptics can only track 2 contacts, correct? Why 2 slots to track them > > > is not enough? > > > > IIRC, the problem was that upon a third finger down, with only 2 slots, > > the fingers were silently inverted in most cases. The thing is that the > > firmware forwards 2 fingers, but not necessarily the two first. So you > > generally get fingers 1+3 so the slot 2 needs to be removed. And that > > means the kernel tracking has to track 3 fingers upon transitions. > > > > This may be completely bullshit and we might not need to use 3 slots at > > all. I'll need to do further experiments to validate which one is best > > then. > > > > I am perfectly fine holding this one up for a little bit more testings > > and then we can decide which one needs to be done (revert or an other > > band-aid). > > > > So I carefully recorded each situation (initial with 2 slots, 2 slots > and then with the pinning in this patch*), and I am now convinced that > the pinning is the best sequence that we forward to the user space (best > among the 3). > > With 2 slots declared, there are 2 problems: > - the first finger jumps to the position of the 3rd when it lands > - the transition between 2 to 3 fingers goes to a state where the kernel > removes the second finger (while jumping the first to the position of > the 3rd finger), send a sync and then reallocate the first finger > position as the second slot in use > > -> that means that user space sees a small transition where the slots > count is 1 while the BTN_TOOL advertise triple tap :/ > > With 3 slots, we have the problem reported in the rhbz bug #1212230: > - during the transition, the fingers are stable, but we have at most 2 > active slots in one frame, which confuses libinput/xorg-synaptics. > > With the pinning, the user space is no more confused because BTN_TOOL is > always greater or equal than the active slots. > > So I think for now we have 3 possibilities: > 1. Just carry this patch, and hope that we will be able to switch the > synaptics device in the non-PS/2 mode > 2. Revert to 2 patches and fix the kernel tracking to accept 3 fingers > and return the 2 best matches > 3. Revert the use of the kernel tracking at all and re-introduce the > spaghetti code that was here before and hope that all cases where > properly handled. > > IMO that the solution 2. is the best, but I can not do it because I > don't understand what the code does. I can guess things but I can not > accurately change it because it is not readable IMO. > > (yes, there is also the solution 4: "screw up and let the user space deal > with it", but I'd rather not do that given the history of the multitouch > protocol) Dmitry, I feel like this discussion fell a little bit between the cracks and that we all forgot about it. I still believe that the patch is needed (even if it is not the best solution), so I am sending a gently ping on this one :) Cheers, Benjamin > * I tried to put side by side the 3 test cases in the following logs: > > > (init slots 2, no pinning) | (init slots 3, no pinning) | (init slots 3, pinning) > ----------------------------- | ----------------------------- | --------------------------- > ------------------------------|----- one finger down -------|---------------------------- > | | > ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 > ABS_MT_TRACKING_ID 53 | ABS_MT_TRACKING_ID 53 | ABS_MT_TRACKING_ID 53 > ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 > ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 > ABS_MT_PRESSURE 68 | ABS_MT_PRESSURE 68 | ABS_MT_PRESSURE 68 > BTN_TOUCH 1 | BTN_TOUCH 1 | BTN_TOUCH 1 > ABS_X 2000 | ABS_X 2000 | ABS_X 2000 > ABS_Y 2000 | ABS_Y 2000 | ABS_Y 2000 > ABS_PRESSURE 68 | ABS_PRESSURE 68 | ABS_PRESSURE 68 > BTN_TOOL_FINGER 1 | BTN_TOOL_FINGER 1 | BTN_TOOL_FINGER 1 > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > | | > ... | ... | ... > | | > ------------------------------------ second finger down ------------------------------------ > | | > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 > ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 > ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 > ABS_MT_SLOT 1 | ABS_MT_SLOT 1 | ABS_MT_SLOT 1 > ABS_MT_TRACKING_ID 54 | ABS_MT_TRACKING_ID 54 | ABS_MT_TRACKING_ID 54 > ABS_MT_POSITION_X 3000 | ABS_MT_POSITION_X 3000 | ABS_MT_POSITION_X 3000 > ABS_MT_POSITION_Y 3000 | ABS_MT_POSITION_Y 3000 | ABS_MT_POSITION_Y 3000 > ABS_MT_PRESSURE 64 | ABS_MT_PRESSURE 64 | ABS_MT_PRESSURE 64 > ABS_X 2000 | ABS_X 2000 | ABS_X 2000 > ABS_Y 2000 | ABS_Y 2000 | ABS_Y 2000 > ABS_PRESSURE 78 | ABS_PRESSURE 78 | ABS_PRESSURE 78 > BTN_TOOL_FINGER 0 | BTN_TOOL_FINGER 0 | BTN_TOOL_FINGER 0 > BTN_TOOL_DOUBLETAP 1 | BTN_TOOL_DOUBLETAP 1 | BTN_TOOL_DOUBLETAP 1 > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > | | > ... | ... | ... > | | > ------------------------------------ third finger down ------------------------------------ > | | > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 > ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_X 2000 | ABS_MT_POSITION_X 2000 > ABS_MT_POSITION_Y 4000 | ABS_MT_POSITION_Y 2000 | ABS_MT_POSITION_Y 2000 > ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 | ABS_MT_PRESSURE 78 > ABS_MT_SLOT 1 | ABS_MT_SLOT 2 | ABS_MT_SLOT 2 > ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID 55 | ABS_MT_TRACKING_ID 55 > | ABS_MT_POSITION_X 4000 | ABS_MT_POSITION_X 4000 > | ABS_MT_POSITION_Y 4000 | ABS_MT_POSITION_Y 4000 > | ABS_MT_PRESSURE 72 | ABS_MT_PRESSURE 72 > | ABS_MT_SLOT 1 | > | ABS_MT_TRACKING_ID -1 | > ABS_X 4000 | ABS_X 2000 | ABS_X 2000 > ABS_Y 4000 | ABS_Y 2000 | ABS_Y 2000 > ABS_PRESSURE 78 | ABS_PRESSURE 78 | ABS_PRESSURE 78 > BTN_TOOL_DOUBLETAP 0 | BTN_TOOL_DOUBLETAP 0 | BTN_TOOL_DOUBLETAP 0 > BTN_TOOL_TRIPLETAP 1 | BTN_TOOL_TRIPLETAP 1 | BTN_TOOL_TRIPLETAP 1 > --- SYN_REPORT (0) ---------- | | > ABS_MT_SLOT 0 | | > ABS_MT_POSITION_X 4000 | | > ABS_MT_POSITION_Y 4000 | | > ABS_MT_PRESSURE 78 | | > ABS_MT_SLOT 1 | | > ABS_MT_TRACKING_ID 55 | | > ABS_MT_POSITION_X 2000 | | > ABS_MT_POSITION_Y 2000 | | > ABS_MT_PRESSURE 72 | | > ABS_X 4000 | | > ABS_Y 4000 | | > ABS_PRESSURE 78 | | > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > | | > ... | ... | ... > | | > ------------------------------------ 3 fingers release ------------------------------------ > | | > | | > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > | | ABS_MT_SLOT 2 > | | ABS_MT_POSITION_X 4000 > | | ABS_MT_POSITION_Y 4000 > | | ABS_MT_PRESSURE 45 > ABS_MT_SLOT 0 | ABS_MT_SLOT 0 | ABS_MT_SLOT 0 > ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 > ABS_MT_SLOT 1 | ABS_MT_SLOT 2 | ABS_MT_SLOT 1 > ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 | ABS_MT_TRACKING_ID -1 > | | ABS_X 4000 > | | ABS_Y 4000 > BTN_TOUCH 0 | BTN_TOUCH 0 | ABS_PRESSURE 45 > ABS_PRESSURE 0 | ABS_PRESSURE 0 | BTN_TOOL_FINGER 1 > BTN_TOOL_TRIPLETAP 0 | BTN_TOOL_TRIPLETAP 0 | BTN_TOOL_TRIPLETAP 0 > --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- | --- SYN_REPORT (0) ---------- > | | ABS_MT_SLOT 2 > | | ABS_MT_TRACKING_ID -1 > | | BTN_TOUCH 0 > | | ABS_PRESSURE 0 > | | BTN_TOOL_FINGER 0 > | | --- SYN_REPORT (0) ---------- > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-06-11 17:29 ` Benjamin Tissoires @ 2015-07-01 0:26 ` Dmitry Torokhov 2015-07-07 14:02 ` Benjamin Tissoires 0 siblings, 1 reply; 12+ messages in thread From: Dmitry Torokhov @ 2015-07-01 0:26 UTC (permalink / raw) To: Benjamin Tissoires Cc: linux-input, linux-kernel, Peter Hutterer, Hans de Goede, Henrik Rydberg Hi Benjamin, On Thu, Jun 11, 2015 at 01:29:09PM -0400, Benjamin Tissoires wrote: > On Apr 24 2015 or thereabouts, Benjamin Tissoires wrote: > > Hi Dmitry, > > > > [ adding more relevant people to the discussion ] > > > > On Apr 23 2015 or thereabouts, Benjamin Tissoires wrote: > > > On Apr 23 2015 or thereabouts, Dmitry Torokhov wrote: > > > > On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote: > > > > > Synaptics PS/2 touchpad can send only 2 touches in a report. They can > > > > > detect 4 or 5 and this information is valuable. > > > > > > > > > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep > > > > > stability in image sensors"), we allocate 3 slots, but we still continue > > > > > to report the 2 available fingers. That means that the client sees 2 used > > > > > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. > > > > > > > > > > For old kernels this is not a problem because max_slots was 2 and libinput/ > > > > > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > > > > > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > > > > > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > > > > > information, and goes wild. > > > > > > > > > > We can pin the 3 slots until we get a total number of fingers below 2. > > > > > > > > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 > > > > > > > > Benjamin, I do not quite like it. It seems that original patch was not > > > > quite right and we are adding more workarounds. > > > > > > Agree. And I am starting to hate more and more the synaptics PS/2 and all > > > the PS/2 drivers to be honest :) - trying to fit a heavy load data like > > > multitouch in a simple and lightweight protocol like PS/2 is insane... > > > > > > We are internally trying to figure out if we can finally take advantage > > > of the SMBus/RMI4 protocol, but we tried for one year without much > > > success. > > > > > > > > > > > Synaptics can only track 2 contacts, correct? Why 2 slots to track them > > > > is not enough? > > > > > > IIRC, the problem was that upon a third finger down, with only 2 slots, > > > the fingers were silently inverted in most cases. The thing is that the > > > firmware forwards 2 fingers, but not necessarily the two first. So you > > > generally get fingers 1+3 so the slot 2 needs to be removed. And that > > > means the kernel tracking has to track 3 fingers upon transitions. > > > > > > This may be completely bullshit and we might not need to use 3 slots at > > > all. I'll need to do further experiments to validate which one is best > > > then. > > > > > > I am perfectly fine holding this one up for a little bit more testings > > > and then we can decide which one needs to be done (revert or an other > > > band-aid). > > > > > > > So I carefully recorded each situation (initial with 2 slots, 2 slots > > and then with the pinning in this patch*), and I am now convinced that > > the pinning is the best sequence that we forward to the user space (best > > among the 3). > > > > With 2 slots declared, there are 2 problems: > > - the first finger jumps to the position of the 3rd when it lands > > - the transition between 2 to 3 fingers goes to a state where the kernel > > removes the second finger (while jumping the first to the position of > > the 3rd finger), send a sync and then reallocate the first finger > > position as the second slot in use > > > > -> that means that user space sees a small transition where the slots > > count is 1 while the BTN_TOOL advertise triple tap :/ > > > > With 3 slots, we have the problem reported in the rhbz bug #1212230: > > - during the transition, the fingers are stable, but we have at most 2 > > active slots in one frame, which confuses libinput/xorg-synaptics. > > > > With the pinning, the user space is no more confused because BTN_TOOL is > > always greater or equal than the active slots. > > > > So I think for now we have 3 possibilities: > > 1. Just carry this patch, and hope that we will be able to switch the > > synaptics device in the non-PS/2 mode > > 2. Revert to 2 patches and fix the kernel tracking to accept 3 fingers > > and return the 2 best matches > > 3. Revert the use of the kernel tracking at all and re-introduce the > > spaghetti code that was here before and hope that all cases where > > properly handled. > > > > IMO that the solution 2. is the best, but I can not do it because I > > don't understand what the code does. I can guess things but I can not > > accurately change it because it is not readable IMO. > > > > (yes, there is also the solution 4: "screw up and let the user space deal > > with it", but I'd rather not do that given the history of the multitouch > > protocol) > > Dmitry, I feel like this discussion fell a little bit between the cracks > and that we all forgot about it. > > I still believe that the patch is needed (even if it is not the best > solution), so I am sending a gently ping on this one :) Sorry I lost track of this, but I still believe that introducing the 3rd slot is not the right solution as is evidenced by the need of more workarounds. If the hardware is only capable of tracking the 2 contacts then we should be using 2 slots. It seems that userspace (and maybe the kernel as well?) is not quite prepared to handle change of contact's identity in a slot (i.e. assigning new tracking id to a slot without transitioning through -1), but that is what we need to fix then. I think we should revert 63c4fda3c0bb. Thanks. -- Dmitry ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers 2015-07-01 0:26 ` Dmitry Torokhov @ 2015-07-07 14:02 ` Benjamin Tissoires 0 siblings, 0 replies; 12+ messages in thread From: Benjamin Tissoires @ 2015-07-07 14:02 UTC (permalink / raw) To: Dmitry Torokhov Cc: linux-input, linux-kernel, Peter Hutterer, Hans de Goede, Henrik Rydberg On Jun 30 2015 or thereabouts, Dmitry Torokhov wrote: > Hi Benjamin, > > On Thu, Jun 11, 2015 at 01:29:09PM -0400, Benjamin Tissoires wrote: > > On Apr 24 2015 or thereabouts, Benjamin Tissoires wrote: > > > Hi Dmitry, > > > > > > [ adding more relevant people to the discussion ] > > > > > > On Apr 23 2015 or thereabouts, Benjamin Tissoires wrote: > > > > On Apr 23 2015 or thereabouts, Dmitry Torokhov wrote: > > > > > On Wed, Apr 22, 2015 at 11:45:09AM -0400, Benjamin Tissoires wrote: > > > > > > Synaptics PS/2 touchpad can send only 2 touches in a report. They can > > > > > > detect 4 or 5 and this information is valuable. > > > > > > > > > > > > In commit 63c4fda3c0bb ("Input: synaptics - allocate 3 slots to keep > > > > > > stability in image sensors"), we allocate 3 slots, but we still continue > > > > > > to report the 2 available fingers. That means that the client sees 2 used > > > > > > slots while there is a total of 3 fingers advertised by BTN_TOOL_TRIPLETAP. > > > > > > > > > > > > For old kernels this is not a problem because max_slots was 2 and libinput/ > > > > > > xorg-synaptics knew how to deal with that. Now that max_slot is 3, the > > > > > > clients ignore BTN_TOOL_TRIPLETAP and count the actual used slots (so 2). > > > > > > It then gets confused when receiving the BTN_TOOL_TRIPLETAP and DOUBLETAP > > > > > > information, and goes wild. > > > > > > > > > > > > We can pin the 3 slots until we get a total number of fingers below 2. > > > > > > > > > > > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1212230 > > > > > > > > > > Benjamin, I do not quite like it. It seems that original patch was not > > > > > quite right and we are adding more workarounds. > > > > > > > > Agree. And I am starting to hate more and more the synaptics PS/2 and all > > > > the PS/2 drivers to be honest :) - trying to fit a heavy load data like > > > > multitouch in a simple and lightweight protocol like PS/2 is insane... > > > > > > > > We are internally trying to figure out if we can finally take advantage > > > > of the SMBus/RMI4 protocol, but we tried for one year without much > > > > success. > > > > > > > > > > > > > > Synaptics can only track 2 contacts, correct? Why 2 slots to track them > > > > > is not enough? > > > > > > > > IIRC, the problem was that upon a third finger down, with only 2 slots, > > > > the fingers were silently inverted in most cases. The thing is that the > > > > firmware forwards 2 fingers, but not necessarily the two first. So you > > > > generally get fingers 1+3 so the slot 2 needs to be removed. And that > > > > means the kernel tracking has to track 3 fingers upon transitions. > > > > > > > > This may be completely bullshit and we might not need to use 3 slots at > > > > all. I'll need to do further experiments to validate which one is best > > > > then. > > > > > > > > I am perfectly fine holding this one up for a little bit more testings > > > > and then we can decide which one needs to be done (revert or an other > > > > band-aid). > > > > > > > > > > So I carefully recorded each situation (initial with 2 slots, 2 slots > > > and then with the pinning in this patch*), and I am now convinced that > > > the pinning is the best sequence that we forward to the user space (best > > > among the 3). > > > > > > With 2 slots declared, there are 2 problems: > > > - the first finger jumps to the position of the 3rd when it lands > > > - the transition between 2 to 3 fingers goes to a state where the kernel > > > removes the second finger (while jumping the first to the position of > > > the 3rd finger), send a sync and then reallocate the first finger > > > position as the second slot in use > > > > > > -> that means that user space sees a small transition where the slots > > > count is 1 while the BTN_TOOL advertise triple tap :/ > > > > > > With 3 slots, we have the problem reported in the rhbz bug #1212230: > > > - during the transition, the fingers are stable, but we have at most 2 > > > active slots in one frame, which confuses libinput/xorg-synaptics. > > > > > > With the pinning, the user space is no more confused because BTN_TOOL is > > > always greater or equal than the active slots. > > > > > > So I think for now we have 3 possibilities: > > > 1. Just carry this patch, and hope that we will be able to switch the > > > synaptics device in the non-PS/2 mode > > > 2. Revert to 2 patches and fix the kernel tracking to accept 3 fingers > > > and return the 2 best matches > > > 3. Revert the use of the kernel tracking at all and re-introduce the > > > spaghetti code that was here before and hope that all cases where > > > properly handled. > > > > > > IMO that the solution 2. is the best, but I can not do it because I > > > don't understand what the code does. I can guess things but I can not > > > accurately change it because it is not readable IMO. > > > > > > (yes, there is also the solution 4: "screw up and let the user space deal > > > with it", but I'd rather not do that given the history of the multitouch > > > protocol) > > > > Dmitry, I feel like this discussion fell a little bit between the cracks > > and that we all forgot about it. > > > > I still believe that the patch is needed (even if it is not the best > > solution), so I am sending a gently ping on this one :) > > Sorry I lost track of this, but I still believe that introducing the 3rd > slot is not the right solution as is evidenced by the need of more > workarounds. If the hardware is only capable of tracking the 2 contacts > then we should be using 2 slots. It seems that userspace (and maybe the > kernel as well?) is not quite prepared to handle change of contact's > identity in a slot (i.e. assigning new tracking id to a slot without > transitioning through -1), but that is what we need to fix then. > > I think we should revert 63c4fda3c0bb. > It seems you are right: https://bugzilla.redhat.com/show_bug.cgi?id=1236540 shows that the fix is not good enough. I am not sure the kernel tracking is able to correctly track and implement the identity change within the same slot, but we should be able to remove these limitations by switching to RMI4 in the near future. Anyway, ACK for the revert. Back from my PTOs, Chandler sent us a mail saying that he managed to get the PS/2 pass-through for the trackstick working which means that we have in our trees a fully working solution for the Lenovo 40 and 50 series, and potentially all other RMI4 over SMBus ones (except ForcePads). Cheers, Benjamin ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2015-07-07 14:02 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-22 15:45 [PATCH 0/2] 2 somewhat related input fixes Benjamin Tissoires 2015-04-22 15:45 ` [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW Benjamin Tissoires 2015-04-23 16:09 ` Dmitry Torokhov 2015-04-22 15:45 ` [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers Benjamin Tissoires 2015-04-23 16:38 ` Dmitry Torokhov 2015-04-23 18:48 ` Benjamin Tissoires 2015-04-24 22:50 ` Benjamin Tissoires 2015-04-25 9:40 ` Henrik Rydberg 2015-04-27 17:48 ` Benjamin Tissoires 2015-06-11 17:29 ` Benjamin Tissoires 2015-07-01 0:26 ` Dmitry Torokhov 2015-07-07 14:02 ` Benjamin Tissoires
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).