[PATCH 0/2] 2 somewhat related input fixes

[PATCH 0/2] 2 somewhat related input fixes

[Benjamin Tissoires]

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

[PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW

[Benjamin Tissoires]

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

[PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Benjamin Tissoires]

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

Re: [PATCH 1/2] Input - elantech: fix semi-mt protocol for v3 HW

[Dmitry Torokhov]

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

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Dmitry Torokhov]

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

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Benjamin Tissoires]

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

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Benjamin Tissoires]

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) ----------

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Henrik Rydberg]

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

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Benjamin Tissoires]

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

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Benjamin Tissoires]

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) ----------
> 
> 

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Dmitry Torokhov]

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

Re: [PATCH 2/2] Input - synaptics: pin 3 touches when the firmware reports 3 fingers

[Benjamin Tissoires]

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