[PATCH 1/2] Input: synaptics - add multitouch packet support

[PATCH 1/2] Input: synaptics - add multitouch packet support

[Henrik Rydberg]

From: Chase Douglas <chase.douglas@canonical.com>

Synaptics 2.7 series of touchpads support a mode for reporting 2 sets
of X/Y/Pressure data (multi-touch).  These same devices default mode
report single finger data and do not report finger counts.

Enabling MT mode makes finger count reporting start working in same
fashion as touchpads that claim that capability. Up to three fingers
can be reported this way.

While in MT mode and two or three fingers are touching, two sets of
data are sent.  The first is a new format buffer with lower resolution
reporting of stationary finger and the second is standard data format
reporting movement.

Work to enable MT and decoding its packet is from patch from Takashi Iwai.

Additional cleanup/testing of original patch was performed by Chase Douglas.

Minor cleanup and testing performed by Chris Bagwell.

Reported-by: Tobyn Bertram
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Chase Douglas <chase.douglas@canonical.com>
Signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |   75 ++++++++++++++++++++++++++++++++-------
 drivers/input/mouse/synaptics.h |    4 ++
 2 files changed, 66 insertions(+), 13 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 2e300a4..de08d77 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -279,6 +279,24 @@ static void synaptics_set_rate(struct psmouse *psmouse, unsigned int rate)
 	synaptics_mode_cmd(psmouse, priv->mode);
 }
 
+static int synaptics_set_multitouch_mode(struct psmouse *psmouse)
+{
+	static unsigned char param = 0xc8;
+	struct synaptics_data *priv = psmouse->private;
+
+	if (!SYN_CAP_MULTITOUCH(priv->ext_cap_0c))
+		return 0;
+
+	if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL))
+		return -1;
+	if (ps2_command(&psmouse->ps2dev, &param, PSMOUSE_CMD_SETRATE))
+		return -1;
+
+	priv->multitouch = 1;
+	printk(KERN_INFO "Synaptics: Multitouch mode enabled\n");
+	return 0;
+}
+
 /*****************************************************************************
  *	Synaptics pass-through PS/2 port support
  ****************************************************************************/
@@ -380,23 +398,38 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
-static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)
+static int synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *priv, struct synaptics_hw_state *hw)
 {
 	memset(hw, 0, sizeof(struct synaptics_hw_state));
 
 	if (SYN_MODEL_NEWABS(priv->model_id)) {
-		hw->x = (((buf[3] & 0x10) << 8) |
-			 ((buf[1] & 0x0f) << 8) |
-			 buf[4]);
-		hw->y = (((buf[3] & 0x20) << 7) |
-			 ((buf[1] & 0xf0) << 4) |
-			 buf[5]);
-
-		hw->z = buf[2];
 		hw->w = (((buf[0] & 0x30) >> 2) |
 			 ((buf[0] & 0x04) >> 1) |
 			 ((buf[3] & 0x04) >> 2));
 
+		if (priv->multitouch && hw->w == 2) {
+			/* Multitouch data is half normal resolution */
+			hw->x = (((buf[4] & 0x0f) << 8) |
+				 buf[1]) << 1;
+			hw->y = (((buf[4] & 0xf0) << 4) |
+				 buf[2]) << 1;
+
+			hw->z = ((buf[3] & 0x30) |
+				 (buf[5] & 0x0f)) << 1;
+
+			/* Only look at x, y, and z for MT */
+			return 1;
+		} else {
+			hw->x = (((buf[3] & 0x10) << 8) |
+				 ((buf[1] & 0x0f) << 8) |
+				 buf[4]);
+			hw->y = (((buf[3] & 0x20) << 7) |
+				 ((buf[1] & 0xf0) << 4) |
+				 buf[5]);
+
+			hw->z = buf[2];
+		}
+
 		hw->left  = (buf[0] & 0x01) ? 1 : 0;
 		hw->right = (buf[0] & 0x02) ? 1 : 0;
 
@@ -452,6 +485,8 @@ static void synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data
 		hw->left  = (buf[0] & 0x01) ? 1 : 0;
 		hw->right = (buf[0] & 0x02) ? 1 : 0;
 	}
+
+	return 0;
 }
 
 /*
@@ -466,7 +501,10 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	int finger_width;
 	int i;
 
-	synaptics_parse_hw_state(psmouse->packet, priv, &hw);
+	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw)) {
+		priv->mt = hw;
+		return;
+	}
 
 	if (hw.scroll) {
 		priv->scroll += hw.scroll;
@@ -494,7 +532,8 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		if (SYN_CAP_EXTENDED(priv->capabilities)) {
 			switch (hw.w) {
 			case 0 ... 1:
-				if (SYN_CAP_MULTIFINGER(priv->capabilities))
+				if (SYN_CAP_MULTIFINGER(priv->capabilities) ||
+				    priv->multitouch)
 					num_fingers = hw.w + 2;
 				break;
 			case 2:
@@ -532,7 +571,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	input_report_key(dev, BTN_LEFT, hw.left);
 	input_report_key(dev, BTN_RIGHT, hw.right);
 
-	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
+	if (SYN_CAP_MULTIFINGER(priv->capabilities) || priv->multitouch) {
 		input_report_key(dev, BTN_TOOL_DOUBLETAP, num_fingers == 2);
 		input_report_key(dev, BTN_TOOL_TRIPLETAP, num_fingers == 3);
 	}
@@ -638,7 +677,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	__set_bit(BTN_LEFT, dev->keybit);
 	__set_bit(BTN_RIGHT, dev->keybit);
 
-	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
+	if (SYN_CAP_MULTIFINGER(priv->capabilities) | priv->multitouch) {
 		__set_bit(BTN_TOOL_DOUBLETAP, dev->keybit);
 		__set_bit(BTN_TOOL_TRIPLETAP, dev->keybit);
 	}
@@ -702,6 +741,11 @@ static int synaptics_reconnect(struct psmouse *psmouse)
 		return -1;
 	}
 
+	if (synaptics_set_multitouch_mode(psmouse)) {
+		printk(KERN_ERR "Unable to initialize Synaptics Multitouch.\n");
+		return -1;
+	}
+
 	return 0;
 }
 
@@ -769,6 +813,11 @@ int synaptics_init(struct psmouse *psmouse)
 		goto init_fail;
 	}
 
+	if (synaptics_set_multitouch_mode(psmouse)) {
+		printk(KERN_ERR "Unable to initialize Synaptics Multitouch.\n");
+		goto init_fail;
+	}
+
 	priv->pkt_type = SYN_MODEL_NEWABS(priv->model_id) ? SYN_NEWABS : SYN_OLDABS;
 
 	printk(KERN_INFO "Synaptics Touchpad, model: %ld, fw: %ld.%ld, id: %#lx, caps: %#lx/%#lx/%#lx\n",
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 613a365..4cb13b8 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -53,6 +53,7 @@
 #define SYN_CAP_PRODUCT_ID(ec)		(((ec) & 0xff0000) >> 16)
 #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100100)
 #define SYN_CAP_MAX_DIMENSIONS(ex0c)	((ex0c) & 0x020000)
+#define SYN_CAP_MULTITOUCH(ex0c)	((ex0c) & 0x080000)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
@@ -112,6 +113,9 @@ struct synaptics_data {
 	int scroll;
 
 	struct serio *pt_port;			/* Pass-through serio port */
+
+	int multitouch;				/* device provides MT data */
+	struct synaptics_hw_state mt;		/* current MT packet */
 };
 
 void synaptics_module_init(void);
-- 
1.7.1

[PATCH 2/2] Input: synaptics - emit multitouch data

[Henrik Rydberg]

In multitouch mode, two data packets are sent from the device.  Due to
limitations in the hardware detection mechanism, individual contacts
cannot be reliably extracted. However, the bounding rectangle of the
individual contacts can be accurately reproduced. This patch emits the
MT data as envelope coordinates, allowing userspace to track the
bounding rectangle and produce appropriate gestures.

Acked-by: Chris Bagwell <chris@cnpbagwell.com>
Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |   46 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index de08d77..95d0670 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -25,7 +25,7 @@
 
 #include <linux/module.h>
 #include <linux/dmi.h>
-#include <linux/input.h>
+#include <linux/input/mt.h>
 #include <linux/serio.h>
 #include <linux/libps2.h>
 #include <linux/slab.h>
@@ -489,6 +489,36 @@ static int synaptics_parse_hw_state(unsigned char buf[], struct synaptics_data *
 	return 0;
 }
 
+static void set_slot(struct input_dev *dev, int slot, bool active,
+		     int x, int y, int z)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_ENVELOPE, active);
+	if (active) {
+		input_report_abs(dev, ABS_MT_POSITION_X, x);
+		input_report_abs(dev, ABS_MT_POSITION_Y,
+				 YMAX_NOMINAL + YMIN_NOMINAL - y);
+		input_report_abs(dev, ABS_MT_PRESSURE, z);
+	}
+}
+
+static void synaptics_report_mt_data(struct input_dev *dev,
+				     const struct synaptics_hw_state *a,
+				     const struct synaptics_hw_state *b,
+				     int num_fingers)
+{
+	if (num_fingers >= 2) {
+		set_slot(dev, 0, true, min(a->x, b->x), min(a->y, b->y), a->z);
+		set_slot(dev, 1, true, max(a->x, b->x), max(a->y, b->y), a->z);
+	} else if (num_fingers == 1) {
+		set_slot(dev, 0, true, a->x, a->y, a->z);
+		set_slot(dev, 1, false, 0, 0, 0);
+	} else {
+		set_slot(dev, 0, false, 0, 0, 0);
+		set_slot(dev, 1, false, 0, 0, 0);
+	}
+}
+
 /*
  *  called for each full received packet from the touchpad
  */
@@ -551,6 +581,9 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		finger_width = 0;
 	}
 
+	if (priv->multitouch)
+		synaptics_report_mt_data(dev, &hw, &priv->mt, num_fingers);
+
 	/* Post events
 	 * BTN_TOUCH has to be first as mousedev relies on it when doing
 	 * absolute -> relative conversion
@@ -668,6 +701,17 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 			     YMIN_NOMINAL, priv->y_max ?: YMAX_NOMINAL, 0, 0);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
+	if (priv->multitouch) {
+		input_mt_init_slots(dev, 2);
+		input_set_abs_params(dev, ABS_MT_POSITION_X, XMIN_NOMINAL,
+				     priv->x_max ?: XMAX_NOMINAL, 0, 0);
+		input_set_abs_params(dev, ABS_MT_POSITION_Y, YMIN_NOMINAL,
+				     priv->y_max ?: YMAX_NOMINAL, 0, 0);
+		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX,
+				     0, 0);
+	}
+
 	if (SYN_CAP_PALMDETECT(priv->capabilities))
 		input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
 
-- 
1.7.1

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Chase Douglas]

On 12/13/2010 02:55 PM, Henrik Rydberg wrote:
> @@ -638,7 +677,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>  	__set_bit(BTN_LEFT, dev->keybit);
>  	__set_bit(BTN_RIGHT, dev->keybit);
>  
> -	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
> +	if (SYN_CAP_MULTIFINGER(priv->capabilities) | priv->multitouch) {

^^ Although I think this is functionally correct, it is nevertheless a
typo. Note that there's only one '|' where there should be two.

Henrik, do you want to fix this, or would you like me to fix and resend?
I've got no problems either way :).

Thanks,

-- Chase

Re: [PATCH 2/2] Input: synaptics - emit multitouch data

[Chase Douglas]

On 12/13/2010 02:55 PM, Henrik Rydberg wrote:
> In multitouch mode, two data packets are sent from the device.  Due to
> limitations in the hardware detection mechanism, individual contacts
> cannot be reliably extracted. However, the bounding rectangle of the
> individual contacts can be accurately reproduced. This patch emits the
> MT data as envelope coordinates, allowing userspace to track the
> bounding rectangle and produce appropriate gestures.
> 
> Acked-by: Chris Bagwell <chris@cnpbagwell.com>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>

I agree with the patch in theory, but I still have reservations about
MT_TOOL_ENVELOPE. Once that's sorted out I'll add my ACK.

Thanks,

-- Chase

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Henrik Rydberg]

On 12/14/2010 12:09 AM, Chase Douglas wrote:

> On 12/13/2010 02:55 PM, Henrik Rydberg wrote:
>> @@ -638,7 +677,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>>  	__set_bit(BTN_LEFT, dev->keybit);
>>  	__set_bit(BTN_RIGHT, dev->keybit);
>>  
>> -	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
>> +	if (SYN_CAP_MULTIFINGER(priv->capabilities) | priv->multitouch) {
> 
> ^^ Although I think this is functionally correct, it is nevertheless a
> typo. Note that there's only one '|' where there should be two.


Ah, this one was removed but kept popping back in subsequent versions. Will fix,
but not resending now.

Thanks,
Henrik

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Chase Douglas]

On 12/13/2010 03:15 PM, Henrik Rydberg wrote:
> On 12/14/2010 12:09 AM, Chase Douglas wrote:
> 
>> On 12/13/2010 02:55 PM, Henrik Rydberg wrote:
>>> @@ -638,7 +677,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>>>  	__set_bit(BTN_LEFT, dev->keybit);
>>>  	__set_bit(BTN_RIGHT, dev->keybit);
>>>  
>>> -	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
>>> +	if (SYN_CAP_MULTIFINGER(priv->capabilities) | priv->multitouch) {
>>
>> ^^ Although I think this is functionally correct, it is nevertheless a
>> typo. Note that there's only one '|' where there should be two.
> 
> 
> Ah, this one was removed but kept popping back in subsequent versions. Will fix,
> but not resending now.

Henrik, Chris,

After some testing this is mostly fine, but I have one of those terrible
"integrated buttons" (or whatever we call it) trackpads. When switching
to multitouch mode, the cursor will sometimes jump when I go to push the
button.

Take the following sequence:

1. Touch in top right corner of pad to position cursor
2. Touch in bottom left corner over button
3. Press button, but finger moves a little

Step 3 causes the primary coordinates in the synaptics MT protocol to
flip to the button-pressing touch. This causes a cursor jump. *Many*
times I have gone to press an "Ok" dialog button and found that I
accidentally launched an application from my dock :).

I think we should perform some rudimentary touch tracking to ensure the
same touch is always used for reporting ABS_X/ABS_Y. A simple distance
comparison between the two touches, as I implemented in one of my other
patches, would suffice.

What do you think?

Thanks,

-- Chase

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Henrik Rydberg]

>

> After some testing this is mostly fine,

 but I have one of those terrible
> "integrated buttons" (or whatever we call it) trackpads. When switching
> to multitouch mode, the cursor will sometimes jump when I go to push the
> button.
> 
> Take the following sequence:
> 
> 1. Touch in top right corner of pad to position cursor
> 2. Touch in bottom left corner over button
> 3. Press button, but finger moves a little
> 
> Step 3 causes the primary coordinates in the synaptics MT protocol to
> flip to the button-pressing touch. This causes a cursor jump. *Many*
> times I have gone to press an "Ok" dialog button and found that I
> accidentally launched an application from my dock :).


I see - the behavior of the primary coordinates seems to vary between models,
then. On the other hand, if you point and click with the same finger, one could
possibly go around this problem, even though it means less precision. On my MT
laptop, I can click at the very edge of the pad without the cursor moving.

> I think we should perform some rudimentary touch tracking to ensure the
> same touch is always used for reporting ABS_X/ABS_Y. A simple distance
> comparison between the two touches, as I implemented in one of my other
> patches, would suffice.


One can certainly decide on the "best" coordinates when putting down the second
finger, as we tried for elantech. However, after some movement, when the second
finger is lifted, chances are you get a jump then instead.

> What do you think?


The general approach we have taken for the kernel is to provide a left button
for both the macbooks and these clickpads, and in addition provide enough
information (read mt data) to solve this problem in userspace. In other words,
one should probably see what additions are needed in the common X drivers to
make the experience a pleasant one.

Thanks,
Henrik

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Chase Douglas]

On 12/15/2010 09:21 AM, Henrik Rydberg wrote:
>> After some testing this is mostly fine,
> 
>  but I have one of those terrible
>> "integrated buttons" (or whatever we call it) trackpads. When switching
>> to multitouch mode, the cursor will sometimes jump when I go to push the
>> button.
>>
>> Take the following sequence:
>>
>> 1. Touch in top right corner of pad to position cursor
>> 2. Touch in bottom left corner over button
>> 3. Press button, but finger moves a little
>>
>> Step 3 causes the primary coordinates in the synaptics MT protocol to
>> flip to the button-pressing touch. This causes a cursor jump. *Many*
>> times I have gone to press an "Ok" dialog button and found that I
>> accidentally launched an application from my dock :).
> 
> 
> I see - the behavior of the primary coordinates seems to vary between models,
> then. On the other hand, if you point and click with the same finger, one could
> possibly go around this problem, even though it means less precision. On my MT
> laptop, I can click at the very edge of the pad without the cursor moving.

Because of the mechanical properties of my particular touchpad, it's
nearly impossible to click without dragging. My touchpad isn't one of
the new clickpads where the entire pad hinges. On a clickpad, I would
hope depress without movement is easy. On my trackpad, the buttons are
somewhat stiff and flex non-uniformly, causing the unwanted movement.

So, in Ubuntu we mask out the area of the touchpad where the buttons
are. Any movement in that area is discarded. Thus, the only way to be
sure of a click is to position with one touch, then lift the touch, then
click one of the buttons. Annoying :).

>> I think we should perform some rudimentary touch tracking to ensure the
>> same touch is always used for reporting ABS_X/ABS_Y. A simple distance
>> comparison between the two touches, as I implemented in one of my other
>> patches, would suffice.
> 
> 
> One can certainly decide on the "best" coordinates when putting down the second
> finger, as we tried for elantech. However, after some movement, when the second
> finger is lifted, chances are you get a jump then instead.

Chris, isn't this filtered out in xf86-input-synaptics based on the
change in the number of touches?

>> What do you think?
> 
> 
> The general approach we have taken for the kernel is to provide a left button
> for both the macbooks and these clickpads, and in addition provide enough
> information (read mt data) to solve this problem in userspace. In other words,
> one should probably see what additions are needed in the common X drivers to
> make the experience a pleasant one.

I think we're confusing clickpads and my "integrated buttons" trackpad
(I know we settled on a different name, but I can't remember it now :).
I believe my issue is purely due to the primary coordinates switching
back and forth between two touches as I position and click with two fingers.

Also, I have been told by an OEM partner that my trackpad will *never*
be used again :). We could consider quirking it if what makes this
trackpad work is detrimental to other trackpads.

Thanks,

-- Chase

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Chris Bagwell]

On Wed, Dec 15, 2010 at 11:47 AM, Chase Douglas
<chase.douglas@canonical.com> wrote:
> On 12/15/2010 09:21 AM, Henrik Rydberg wrote:
>>> After some testing this is mostly fine,
>>
>>  but I have one of those terrible
>>> "integrated buttons" (or whatever we call it) trackpads. When switching
>>> to multitouch mode, the cursor will sometimes jump when I go to push the
>>> button.
>>>
>>> Take the following sequence:
>>>
>>> 1. Touch in top right corner of pad to position cursor
>>> 2. Touch in bottom left corner over button
>>> 3. Press button, but finger moves a little
>>>
>>> Step 3 causes the primary coordinates in the synaptics MT protocol to
>>> flip to the button-pressing touch. This causes a cursor jump. *Many*
>>> times I have gone to press an "Ok" dialog button and found that I
>>> accidentally launched an application from my dock :).
>>
>>
>> I see - the behavior of the primary coordinates seems to vary between models,
>> then. On the other hand, if you point and click with the same finger, one could
>> possibly go around this problem, even though it means less precision. On my MT
>> laptop, I can click at the very edge of the pad without the cursor moving.
>
> Because of the mechanical properties of my particular touchpad, it's
> nearly impossible to click without dragging. My touchpad isn't one of
> the new clickpads where the entire pad hinges. On a clickpad, I would
> hope depress without movement is easy. On my trackpad, the buttons are
> somewhat stiff and flex non-uniformly, causing the unwanted movement.
>
> So, in Ubuntu we mask out the area of the touchpad where the buttons
> are. Any movement in that area is discarded. Thus, the only way to be
> sure of a click is to position with one touch, then lift the touch, then
> click one of the buttons. Annoying :).

Is this custom code or in upstream (are you talking about
inside_active_area logic?)?  I'm not sure why your seeing a jump if
its being discarded.  There is a chance that something related to this
discard logic is defeating the other logic that handles jumps caused
during finger transitions.

>
>>> I think we should perform some rudimentary touch tracking to ensure the
>>> same touch is always used for reporting ABS_X/ABS_Y. A simple distance
>>> comparison between the two touches, as I implemented in one of my other
>>> patches, would suffice.
>>
>>
>> One can certainly decide on the "best" coordinates when putting down the second
>> finger, as we tried for elantech. However, after some movement, when the second
>> finger is lifted, chances are you get a jump then instead.
>
> Chris, isn't this filtered out in xf86-input-synaptics based on the
> change in the number of touches?

Yes, in version 1.30 or newer xf86-input-synaptics anyways.  And of
course the *TAP transition needs to come in same sync window as jump
for it to be helpful.  It may be able to handle if *TAP comes 1 sync
window before jump but that would be about the limit.

Mind sending me a evtest log snippet during
touch-then-click-with-2nd-finger (with MT mode enabled)?  Seeing those
tends to be better for understanding then words for me.  I'm
interested for more then just this specific issue, btw.

I'm really interested in where the *TAP come (before or after)
relative to cursor jump and how close it was to being filtered out by
xf86-input-synaptics.

>
>>> What do you think?
>>
>>
>> The general approach we have taken for the kernel is to provide a left button
>> for both the macbooks and these clickpads, and in addition provide enough
>> information (read mt data) to solve this problem in userspace. In other words,
>> one should probably see what additions are needed in the common X drivers to
>> make the experience a pleasant one.
>
> I think we're confusing clickpads and my "integrated buttons" trackpad
> (I know we settled on a different name, but I can't remember it now :).
> I believe my issue is purely due to the primary coordinates switching
> back and forth between two touches as I position and click with two fingers.

I would think they both have same issue... Just yours may have some
extra movement and pressure changes compared to the other.

Chris

>
> Also, I have been told by an OEM partner that my trackpad will *never*
> be used again :). We could consider quirking it if what makes this
> trackpad work is detrimental to other trackpads.
>
> Thanks,
>
> -- Chase
>

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Chase Douglas]

On 12/15/2010 02:14 PM, Chris Bagwell wrote:
> On Wed, Dec 15, 2010 at 11:47 AM, Chase Douglas
> <chase.douglas@canonical.com> wrote:
>> On 12/15/2010 09:21 AM, Henrik Rydberg wrote:
>>>> After some testing this is mostly fine,
>>>
>>>  but I have one of those terrible
>>>> "integrated buttons" (or whatever we call it) trackpads. When switching
>>>> to multitouch mode, the cursor will sometimes jump when I go to push the
>>>> button.
>>>>
>>>> Take the following sequence:
>>>>
>>>> 1. Touch in top right corner of pad to position cursor
>>>> 2. Touch in bottom left corner over button
>>>> 3. Press button, but finger moves a little
>>>>
>>>> Step 3 causes the primary coordinates in the synaptics MT protocol to
>>>> flip to the button-pressing touch. This causes a cursor jump. *Many*
>>>> times I have gone to press an "Ok" dialog button and found that I
>>>> accidentally launched an application from my dock :).
>>>
>>>
>>> I see - the behavior of the primary coordinates seems to vary between models,
>>> then. On the other hand, if you point and click with the same finger, one could
>>> possibly go around this problem, even though it means less precision. On my MT
>>> laptop, I can click at the very edge of the pad without the cursor moving.
>>
>> Because of the mechanical properties of my particular touchpad, it's
>> nearly impossible to click without dragging. My touchpad isn't one of
>> the new clickpads where the entire pad hinges. On a clickpad, I would
>> hope depress without movement is easy. On my trackpad, the buttons are
>> somewhat stiff and flex non-uniformly, causing the unwanted movement.
>>
>> So, in Ubuntu we mask out the area of the touchpad where the buttons
>> are. Any movement in that area is discarded. Thus, the only way to be
>> sure of a click is to position with one touch, then lift the touch, then
>> click one of the buttons. Annoying :).
> 
> Is this custom code or in upstream (are you talking about
> inside_active_area logic?)?  I'm not sure why your seeing a jump if
> its being discarded.  There is a chance that something related to this
> discard logic is defeating the other logic that handles jumps caused
> during finger transitions.

In Ubuntu 10.10 I think we are using an in-house patched hack. It was
necessary for an Dell Minis, which was a paid OEM services project, so
we needed a fix ASAP at the time. I believe xf86-input-synaptics has an
option for this now, so we'll probably transition whenever someone gets
a chance to take another look. It may be something that would be handled
better by the upstream logic. When I get a chance I'll try the upstream
logic instead.

>>>> I think we should perform some rudimentary touch tracking to ensure the
>>>> same touch is always used for reporting ABS_X/ABS_Y. A simple distance
>>>> comparison between the two touches, as I implemented in one of my other
>>>> patches, would suffice.
>>>
>>>
>>> One can certainly decide on the "best" coordinates when putting down the second
>>> finger, as we tried for elantech. However, after some movement, when the second
>>> finger is lifted, chances are you get a jump then instead.
>>
>> Chris, isn't this filtered out in xf86-input-synaptics based on the
>> change in the number of touches?
> 
> Yes, in version 1.30 or newer xf86-input-synaptics anyways.  And of
> course the *TAP transition needs to come in same sync window as jump
> for it to be helpful.  It may be able to handle if *TAP comes 1 sync
> window before jump but that would be about the limit.

Ahh! I'm only running 1.22.2, so maybe I should try out the newer
synaptics too.

> Mind sending me a evtest log snippet during
> touch-then-click-with-2nd-finger (with MT mode enabled)?  Seeing those
> tends to be better for understanding then words for me.  I'm
> interested for more then just this specific issue, btw.
> 
> I'm really interested in where the *TAP come (before or after)
> relative to cursor jump and how close it was to being filtered out by
> xf86-input-synaptics.

I'll get this when I get a chance. I'm really crazy backed up with work
right now, and holidays are coming up. I'm supposed to be off for the
rest of the year, but I doubt that will hold perfectly :). So, if I
forget to get you data in the next week or two, feel free to ping me
privately.

>>>> What do you think?
>>>
>>>
>>> The general approach we have taken for the kernel is to provide a left button
>>> for both the macbooks and these clickpads, and in addition provide enough
>>> information (read mt data) to solve this problem in userspace. In other words,
>>> one should probably see what additions are needed in the common X drivers to
>>> make the experience a pleasant one.
>>
>> I think we're confusing clickpads and my "integrated buttons" trackpad
>> (I know we settled on a different name, but I can't remember it now :).
>> I believe my issue is purely due to the primary coordinates switching
>> back and forth between two touches as I position and click with two fingers.
> 
> I would think they both have same issue... Just yours may have some
> extra movement and pressure changes compared to the other.

Ok.

Thanks,

-- Chase

Re: [PATCH 1/2] Input: synaptics - add multitouch packet support

[Peter Hutterer]

On Wed, Dec 15, 2010 at 04:24:40PM -0500, Chase Douglas wrote:
> > Is this custom code or in upstream (are you talking about
> > inside_active_area logic?)?  I'm not sure why your seeing a jump if
> > its being discarded.  There is a chance that something related to this
> > discard logic is defeating the other logic that handles jumps caused
> > during finger transitions.
> 
> In Ubuntu 10.10 I think we are using an in-house patched hack. It was
> necessary for an Dell Minis, which was a paid OEM services project, so
> we needed a fix ASAP at the time. I believe xf86-input-synaptics has an
> option for this now, so we'll probably transition whenever someone gets
> a chance to take another look. It may be something that would be handled
> better by the upstream logic. When I get a chance I'll try the upstream
> logic instead.

mostly identical logic, but the Ubuntu patches only covered the bottom edge
(MovementBottomEdge option, IIRC). The upstream version covers all four
edges with AreaLeftEdge and friends.
recent X servers also support a percentage as option, so instead of the
hardcoded value for the edge, you can say Option "AreaBottomEdge" "20%"

I've had a few attempts to fix this touchpad to work slighlty better but
the box died on me before I could finish it. Feel free to send me one of
these machines if you want it fixed, it's been bugging me for ages :)

Cheers,
  Peter