LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/3] Balanced slots, attempt #2
@ 2015-03-30 22:09 Benjamin Tissoires
  2015-03-30 22:09 ` [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot Benjamin Tissoires
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2015-03-30 22:09 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Hans de Goede, Peter Hutterer
  Cc: linux-input, linux-kernel

Hi,

So, it occurs that the balanced slots assignment introduced in v4.0 had some
troubles and the Synaptics part was reverted.
I tried to spend some time today to figure out a solution, and the only way
I can get something from it is to add a band aid in input_mt_set_slots().

It should not be very costly, but I think it is required: I can guarantee that
the path is hit if the band aid is not here (pr_err was my friend).

So the series is:
- band aid
- revert the revert
- small fix in synaptics.c to keep the slots stable when landing a third finger.

Cheers,
Benjamin

Benjamin Tissoires (3):
  Input: mt - prevent balanced slot assignment to assign twice the slot
  Revert "Revert "Input: synaptics - use dmax in input_mt_assign_slots""
  Input: synaptics - allocate 3 slots to keep stability in image sensors

 drivers/input/input-mt.c        | 7 ++++++-
 drivers/input/mouse/synaptics.c | 7 +++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.3.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot
  2015-03-30 22:09 [PATCH 0/3] Balanced slots, attempt #2 Benjamin Tissoires
@ 2015-03-30 22:09 ` Benjamin Tissoires
  2015-03-31  7:24   ` Henrik Rydberg
  2015-03-30 22:09 ` [PATCH 2/3] Revert "Revert "Input: synaptics - use dmax in input_mt_assign_slots"" Benjamin Tissoires
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2015-03-30 22:09 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Hans de Goede, Peter Hutterer
  Cc: linux-input, linux-kernel

If two touches are under the dmax distance, it looks like they can now
be assigned to the same slot. Add a band aid to prevent such situation
and be able to use the balanced slot assignment.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/input-mt.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index cb150a1..ad74f64 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -371,11 +371,16 @@ static void input_mt_set_slots(struct input_mt *mt,
 		*p = -1;
 
 	for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
+		bool found = false;
+
 		if (!input_mt_is_active(s))
 			continue;
 		for (p = slots; p != slots + num_pos; p++)
-			if (*w++ < 0)
+			if ((*w++ < 0) && !found) {
 				*p = s - mt->slots;
+				/* no break here: w won't be incremented */
+				found = true;
+			}
 	}
 
 	for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
-- 
2.3.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] Revert "Revert "Input: synaptics - use dmax in input_mt_assign_slots""
  2015-03-30 22:09 [PATCH 0/3] Balanced slots, attempt #2 Benjamin Tissoires
  2015-03-30 22:09 ` [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot Benjamin Tissoires
@ 2015-03-30 22:09 ` Benjamin Tissoires
  2015-03-30 22:09 ` [PATCH 3/3] Input: synaptics - allocate 3 slots to keep stability in image sensors Benjamin Tissoires
  2015-03-31  7:11 ` [PATCH 0/3] Balanced slots, attempt #2 Hans de Goede
  3 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2015-03-30 22:09 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Hans de Goede, Peter Hutterer
  Cc: linux-input, linux-kernel

This reverts commit 09d042a2eb90 ("Revert "Input: synaptics - use dmax in input_mt_assign_slots"")

Now that balanced slots assignments seem to be fixed, let's
reenable the use in synaptics.c and wait for users to complain
if there are still problems.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/synaptics.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 1d3d11d..02a12b694 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -67,6 +67,9 @@
 #define X_MAX_POSITIVE 8176
 #define Y_MAX_POSITIVE 8176
 
+/* maximum ABS_MT_POSITION displacement (in mm) */
+#define DMAX 10
+
 /*****************************************************************************
  *	Stuff we need even when we do not want native Synaptics support
  ****************************************************************************/
@@ -922,7 +925,7 @@ static void synaptics_report_mt_data(struct psmouse *psmouse,
 		pos[i].y = synaptics_invert_y(hw[i]->y);
 	}
 
-	input_mt_assign_slots(dev, slot, pos, nsemi, 0);
+	input_mt_assign_slots(dev, slot, pos, nsemi, DMAX * priv->x_res);
 
 	for (i = 0; i < nsemi; i++) {
 		input_mt_slot(dev, slot[i]);
-- 
2.3.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] Input: synaptics - allocate 3 slots to keep stability in image sensors
  2015-03-30 22:09 [PATCH 0/3] Balanced slots, attempt #2 Benjamin Tissoires
  2015-03-30 22:09 ` [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot Benjamin Tissoires
  2015-03-30 22:09 ` [PATCH 2/3] Revert "Revert "Input: synaptics - use dmax in input_mt_assign_slots"" Benjamin Tissoires
@ 2015-03-30 22:09 ` Benjamin Tissoires
  2015-03-31  7:11 ` [PATCH 0/3] Balanced slots, attempt #2 Hans de Goede
  3 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2015-03-30 22:09 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Hans de Goede, Peter Hutterer
  Cc: linux-input, linux-kernel

When slowly dropping 1, 2 and then 3 fingers on an image sensor touchpad,
we can see that the first finger gets reassigned a new slot while it did
not move. This is due to the kernel tracking algorithm which can not
assign correctly the 3 touches, being out of slots.

Declaring that we support 3 slots allows to actually forward:
slot 0 -> down, slot 1 -> up, slot 2 -> down

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/input/mouse/synaptics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 02a12b694..b4c6717 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1194,7 +1194,7 @@ static void set_input_params(struct psmouse *psmouse,
 					ABS_MT_POSITION_Y);
 		/* Image sensors can report per-contact pressure */
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
-		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
+		input_mt_init_slots(dev, 3, INPUT_MT_POINTER | INPUT_MT_TRACK);
 
 		/* Image sensors can signal 4 and 5 finger clicks */
 		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
-- 
2.3.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/3] Balanced slots, attempt #2
  2015-03-30 22:09 [PATCH 0/3] Balanced slots, attempt #2 Benjamin Tissoires
                   ` (2 preceding siblings ...)
  2015-03-30 22:09 ` [PATCH 3/3] Input: synaptics - allocate 3 slots to keep stability in image sensors Benjamin Tissoires
@ 2015-03-31  7:11 ` Hans de Goede
  3 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2015-03-31  7:11 UTC (permalink / raw)
  To: Benjamin Tissoires, Dmitry Torokhov, Henrik Rydberg, Peter Hutterer
  Cc: linux-input, linux-kernel

Hi,

On 31-03-15 00:09, Benjamin Tissoires wrote:
> Hi,
>
> So, it occurs that the balanced slots assignment introduced in v4.0 had some
> troubles and the Synaptics part was reverted.
> I tried to spend some time today to figure out a solution, and the only way
> I can get something from it is to add a band aid in input_mt_set_slots().
>
> It should not be very costly, but I think it is required: I can guarantee that
> the path is hit if the band aid is not here (pr_err was my friend).
>
> So the series is:
> - band aid
> - revert the revert
> - small fix in synaptics.c to keep the slots stable when landing a third finger.
>
> Cheers,
> Benjamin

The entire set looks good to me:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot
  2015-03-30 22:09 ` [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot Benjamin Tissoires
@ 2015-03-31  7:24   ` Henrik Rydberg
  2015-03-31 14:58     ` Benjamin Tissoires
  0 siblings, 1 reply; 7+ messages in thread
From: Henrik Rydberg @ 2015-03-31  7:24 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Dmitry Torokhov, Hans de Goede, Peter Hutterer, linux-input,
	linux-kernel

Hi Benjamin,

> If two touches are under the dmax distance, it looks like they can now
> be assigned to the same slot. Add a band aid to prevent such situation
> and be able to use the balanced slot assignment.

Yes, great find. You patch is correct, but it is not a band aid, but a
result stemming from a limitation in how equality constraints (read
unique assignments) can be handled in the iterative algorithm. This
cannot happen in the original algorithm, because of the extra
penalization for overcovers.

Here is an alternative version which cleans up the loop logic
somewhat, together with a different commit message. I did not have any
opportunity to check this in hardware. I you like it and find it
working, then please take over the authorship and just add a
signed-off from me.

Thanks,
Henrik

---

>From e00d63f6cadf20d871bed763b3531428b34d785c Mon Sep 17 00:00:00 2001
From: Henrik Rydberg <rydberg@bitmath.org>
Date: Tue, 31 Mar 2015 09:10:02 +0200
Subject: [PATCH] Input: MT - make slot assignment work for overcovered
 solutions

The recent inclusion of a deassignment cost in the slot assignment
algorithm did not properly account for the corner cases where the
solutions are overcovered. This patch makes sure the resulting assignment
is unique, allocating new slots when necessary.

UNTESTED
---
 drivers/input/input-mt.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index fbe29fc..17e80a6 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -363,25 +363,28 @@ static void input_mt_set_slots(struct input_mt *mt,
 			       int *slots, int num_pos)
 {
 	struct input_mt_slot *s;
-	int *w = mt->red, *p;
+	int *w = mt->red, j;
 
-	for (p = slots; p != slots + num_pos; p++)
-		*p = -1;
+	for (j = 0; j != num_pos; j++)
+		slots[j] = -1;
 
 	for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
 		if (!input_mt_is_active(s))
 			continue;
-		for (p = slots; p != slots + num_pos; p++)
-			if (*w++ < 0)
-				*p = s - mt->slots;
+		for (j = 0; j != num_pos; j++)
+			if (w[j] < 0) {
+				slots[j] = s - mt->slots;
+				break;
+			}
+		w += num_pos;
 	}
 
 	for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
 		if (input_mt_is_active(s))
 			continue;
-		for (p = slots; p != slots + num_pos; p++)
-			if (*p < 0) {
-				*p = s - mt->slots;
+		for (j = 0; j != num_pos; j++)
+			if (slots[j] < 0) {
+				slots[j] = s - mt->slots;
 				break;
 			}
 	}
-- 
2.3.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot
  2015-03-31  7:24   ` Henrik Rydberg
@ 2015-03-31 14:58     ` Benjamin Tissoires
  0 siblings, 0 replies; 7+ messages in thread
From: Benjamin Tissoires @ 2015-03-31 14:58 UTC (permalink / raw)
  To: Henrik Rydberg
  Cc: Dmitry Torokhov, Hans de Goede, Peter Hutterer, linux-input,
	linux-kernel

Hi Henrik,

On Mar 31 2015 or thereabouts, Henrik Rydberg wrote:
> Hi Benjamin,
> 
> > If two touches are under the dmax distance, it looks like they can now
> > be assigned to the same slot. Add a band aid to prevent such situation
> > and be able to use the balanced slot assignment.
> 
> Yes, great find. You patch is correct, but it is not a band aid, but a
> result stemming from a limitation in how equality constraints (read
> unique assignments) can be handled in the iterative algorithm. This
> cannot happen in the original algorithm, because of the extra
> penalization for overcovers.
> 
> Here is an alternative version which cleans up the loop logic
> somewhat, together with a different commit message. I did not have any
> opportunity to check this in hardware. I you like it and find it
> working, then please take over the authorship and just add a
> signed-off from me.

OK, it works just fine. So I'll resend the series with Hans' ACK and
your v2. Thanks for your quick respin.

Cheers,
Benjamin

> 
> Thanks,
> Henrik
> 
> ---
> 
> From e00d63f6cadf20d871bed763b3531428b34d785c Mon Sep 17 00:00:00 2001
> From: Henrik Rydberg <rydberg@bitmath.org>
> Date: Tue, 31 Mar 2015 09:10:02 +0200
> Subject: [PATCH] Input: MT - make slot assignment work for overcovered
>  solutions
> 
> The recent inclusion of a deassignment cost in the slot assignment
> algorithm did not properly account for the corner cases where the
> solutions are overcovered. This patch makes sure the resulting assignment
> is unique, allocating new slots when necessary.
> 
> UNTESTED
> ---
>  drivers/input/input-mt.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index fbe29fc..17e80a6 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -363,25 +363,28 @@ static void input_mt_set_slots(struct input_mt *mt,
>  			       int *slots, int num_pos)
>  {
>  	struct input_mt_slot *s;
> -	int *w = mt->red, *p;
> +	int *w = mt->red, j;
>  
> -	for (p = slots; p != slots + num_pos; p++)
> -		*p = -1;
> +	for (j = 0; j != num_pos; j++)
> +		slots[j] = -1;
>  
>  	for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
>  		if (!input_mt_is_active(s))
>  			continue;
> -		for (p = slots; p != slots + num_pos; p++)
> -			if (*w++ < 0)
> -				*p = s - mt->slots;
> +		for (j = 0; j != num_pos; j++)
> +			if (w[j] < 0) {
> +				slots[j] = s - mt->slots;
> +				break;
> +			}
> +		w += num_pos;
>  	}
>  
>  	for (s = mt->slots; s != mt->slots + mt->num_slots; s++) {
>  		if (input_mt_is_active(s))
>  			continue;
> -		for (p = slots; p != slots + num_pos; p++)
> -			if (*p < 0) {
> -				*p = s - mt->slots;
> +		for (j = 0; j != num_pos; j++)
> +			if (slots[j] < 0) {
> +				slots[j] = s - mt->slots;
>  				break;
>  			}
>  	}
> -- 
> 2.3.0
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-03-31 14:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-30 22:09 [PATCH 0/3] Balanced slots, attempt #2 Benjamin Tissoires
2015-03-30 22:09 ` [PATCH 1/3] Input: mt - prevent balanced slot assignment to assign twice the slot Benjamin Tissoires
2015-03-31  7:24   ` Henrik Rydberg
2015-03-31 14:58     ` Benjamin Tissoires
2015-03-30 22:09 ` [PATCH 2/3] Revert "Revert "Input: synaptics - use dmax in input_mt_assign_slots"" Benjamin Tissoires
2015-03-30 22:09 ` [PATCH 3/3] Input: synaptics - allocate 3 slots to keep stability in image sensors Benjamin Tissoires
2015-03-31  7:11 ` [PATCH 0/3] Balanced slots, attempt #2 Hans de Goede

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