[PATCH 0/4] Input: synaptics - add semi-mt support

[PATCH 0/4] Input: synaptics - add semi-mt support

[Henrik Rydberg]

Hi all,

Here is what I hope to be a final version of the synaptics MT
patches. The first patch is unchanged except for a fixed typo. It does
not work very well on some clickpads, but this could arguably be fixed
in userspace. There is a dkms package out there with similar code,
which generally seems to work for people. The second patch fixes an
odd behavior found on my test machine. Does it make a difference to
anyone else? The third patch utilizes the new property interface to
flag the clickpad property. The fourth patch implements the semi-mt
support, this time flagged using the property interface.

Chase, Chris?

Thanks,
Henrik

Chase Douglas (1):
  Input: synaptics - add multitouch packet support

Henrik Rydberg (3):
  Input: synaptics - ignore bogus mt packet
  Input: synaptics - report clickpad property
  Input: synaptics - emit multitouch data

 drivers/input/mouse/synaptics.c |  124 ++++++++++++++++++++++++++++++++++-----
 drivers/input/mouse/synaptics.h |    4 +
 2 files changed, 112 insertions(+), 16 deletions(-)

-- 
1.7.2.3

[PATCH 1/4] 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..8a769e9 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.2.3

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

[Chris Bagwell]

On Sat, Dec 18, 2010 at 8:52 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> 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.

We should probably update this comment and remove parts that mention
stationary and movement.  It appears to be doing finger tracking and
first packet is always first touch... except we are getting reports
that clickpads/integrated button pads may not be consistently like
this.

This tracking is partly why type B protocol is being used in later patches.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Chris Bagwell]

On Sat, Dec 18, 2010 at 10:55 AM, Chris Bagwell <chris@cnpbagwell.com> wrote:
> On Sat, Dec 18, 2010 at 8:52 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> 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.
>
> We should probably update this comment and remove parts that mention
> stationary and movement.  It appears to be doing finger tracking and
> first packet is always first touch... except we are getting reports
> that clickpads/integrated button pads may not be consistently like
> this.
>

Sorry.  Not first packet but higher resolution/second packet.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Henrik Rydberg]

On Sat, Dec 18, 2010 at 10:56:59AM -0600, Chris Bagwell wrote:
> On Sat, Dec 18, 2010 at 10:55 AM, Chris Bagwell <chris@cnpbagwell.com> wrote:
> > On Sat, Dec 18, 2010 at 8:52 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> >> 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.
> >
> > We should probably update this comment and remove parts that mention
> > stationary and movement.  It appears to be doing finger tracking and
> > first packet is always first touch... except we are getting reports
> > that clickpads/integrated button pads may not be consistently like
> > this.
> >
> 
> Sorry.  Not first packet but higher resolution/second packet.

Not for all devices, it seems - at least not mine, which is not a clickpad.

Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Henrik Rydberg]

Hi Chris,

Thanks for you comments.

> > 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.
> 
> We should probably update this comment and remove parts that mention
> stationary and movement.  It appears to be doing finger tracking and
> first packet is always first touch... except we are getting reports
> that clickpads/integrated button pads may not be consistently like
> this.

Yes, good point. Will update.
 
> This tracking is partly why type B protocol is being used in later patches.

The original thought was that the two points corresponded well to the
two fingers, only sometimes in the wrong order. The solution then was
to use the tracking module to order them appropriately. As it turned
out that the fingers are not followed that well after all, the whole
idea fell flat, so we resorted to only sending the bounding rectangle,
which requires no tracking.

All in all, this patchset contains enough information to resolve
jumping and clickpad issues in user space, at the same time as it
keeps compatibility with mousedev and older hardware. We know there
are still issues, but I see no real reason to postpone these changes
any further.

Thanks,
Henrik

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

[Chris Bagwell]

On Sat, Dec 18, 2010 at 11:54 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Chris,
>
> Thanks for you comments.
>
>> > 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.
>>
>> We should probably update this comment and remove parts that mention
>> stationary and movement.  It appears to be doing finger tracking and
>> first packet is always first touch... except we are getting reports
>> that clickpads/integrated button pads may not be consistently like
>> this.
>
> Yes, good point. Will update.
>
>> This tracking is partly why type B protocol is being used in later patches.
>
> The original thought was that the two points corresponded well to the
> two fingers, only sometimes in the wrong order. The solution then was
> to use the tracking module to order them appropriately. As it turned
> out that the fingers are not followed that well after all, the whole
> idea fell flat, so we resorted to only sending the bounding rectangle,
> which requires no tracking.
>
> All in all, this patchset contains enough information to resolve
> jumping and clickpad issues in user space, at the same time as it
> keeps compatibility with mousedev and older hardware. We know there
> are still issues, but I see no real reason to postpone these changes
> any further.
>

Agree.  I've found no issues in daily usage for a couple of weeks now
and with an unmodified xf86-input-synaptics (newer then 1.3.0 btw).

I can't speak for clickpads since I do not have access to one but I'm
happy to help narrow down any problems.  I really suspect any
remaining issues are on user land side.

The fact that this patch allows BTN_TOOL_DOUBLETAP to finally be sent
will surely help user land filter out cursor jumps much easier as
well.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Chase Douglas]

On 12/18/2010 12:54 PM, Henrik Rydberg wrote:
> All in all, this patchset contains enough information to resolve
> jumping and clickpad issues in user space, at the same time as it
> keeps compatibility with mousedev and older hardware. We know there
> are still issues, but I see no real reason to postpone these changes
> any further.

My only concern is that by enabling MT mode we may get different ST
values out of the device. However, I don't have concrete evidence that
it is worse, and we can probably fix it up, maybe with tracking, after
these patches are applied.

-- Chase

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

[Dmitry Torokhov]

On Sat, Dec 18, 2010 at 03:52:58PM +0100, Henrik Rydberg wrote:
> 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..8a769e9 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;

That should be bool/true. Also, do we need to have this variable at all
since we seem to abort initialization if multitouch mode fails?

> +	printk(KERN_INFO "Synaptics: Multitouch mode enabled\n");

I'd rather only reported if we failed to activate multitouch mode on
devices that ought support it.

> +	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 calls this bit "Advanced Gesture", I think we should do the
same as they might produce full-MT devices in the future.

Also, how useful is this patch without the 4th one? Why aren't they
folded together?

Thanks.

-- 
Dmitry

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

[Henrik Rydberg]

> > @@ -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;
> 
> That should be bool/true. Also, do we need to have this variable at all
> since we seem to abort initialization if multitouch mode fails?

The logic of the all-patches-applied version does depend on the
advanced gesture flag, but sure, instead of this variable, one could
set the SYN_CAP_MULTIFINGER bit, and use SYN_CAP_MULTITOUCH explicitly
in remaining places. And, of course, another round of testing.

> 
> > +	printk(KERN_INFO "Synaptics: Multitouch mode enabled\n");
> 
> I'd rather only reported if we failed to activate multitouch mode on
> devices that ought support it.

Ok.

> > @@ -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 calls this bit "Advanced Gesture", I think we should do the
> same as they might produce full-MT devices in the future.
> 
> Also, how useful is this patch without the 4th one? Why aren't they
> folded together?

This first patch implements multi-finger support in the same fashion
as older hardware, thus enabling for instance hw-finger scroll in
synaptics.

Thanks,
Henrik

[PATCH 2/4] Input: synaptics - ignore bogus mt packet

[Henrik Rydberg]

In multitouch mode, at least one device (fw: 7.4 id: 0x1c0b1) sometimes
sends a final main packet with x == 1. Since the normal values are above
1472, this is clearly bogus. At the same time, a two-finger touch is
signaled, even though only one finger was on the pad to begin with. This
patch ignores the packet altogether, removing the problem.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 8a769e9..4744064 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -526,7 +526,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 		return;
 	}
 
-	if (hw.z > 0) {
+	if (hw.z > 0 && hw.x > 1) {
 		num_fingers = 1;
 		finger_width = 5;
 		if (SYN_CAP_EXTENDED(priv->capabilities)) {
@@ -558,7 +558,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
 	if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
 
-	if (hw.z > 0) {
+	if (hw.z > 0 && hw.x > 1) {
 		input_report_abs(dev, ABS_X, hw.x);
 		input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
 	}
-- 
1.7.2.3

Re: [PATCH 2/4] Input: synaptics - ignore bogus mt packet

[Chris Bagwell]

On Sat, Dec 18, 2010 at 8:52 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> In multitouch mode, at least one device (fw: 7.4 id: 0x1c0b1) sometimes
> sends a final main packet with x == 1. Since the normal values are above
> 1472, this is clearly bogus. At the same time, a two-finger touch is
> signaled, even though only one finger was on the pad to begin with. This
> patch ignores the packet altogether, removing the problem.

I've seen this behavior as well although I thought it always came when
hw.z == 0 so was masked.  I guess its not to surprising its sneaking
threw with low pressures.

>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/input/mouse/synaptics.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 8a769e9..4744064 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -526,7 +526,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
>                return;
>        }
>
> -       if (hw.z > 0) {
> +       if (hw.z > 0 && hw.x > 1) {
>                num_fingers = 1;
>                finger_width = 5;
>                if (SYN_CAP_EXTENDED(priv->capabilities)) {
> @@ -558,7 +558,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
>        if (hw.z > 30) input_report_key(dev, BTN_TOUCH, 1);
>        if (hw.z < 25) input_report_key(dev, BTN_TOUCH, 0);
>
> -       if (hw.z > 0) {
> +       if (hw.z > 0 && hw.x > 1) {
>                input_report_abs(dev, ABS_X, hw.x);
>                input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
>        }

This might read better if we change to "if (num_fingers) {" and then
we do not need to repeat the hw work around logic twice.

Also, this snippet shows I may be seeing it as well but the
BTN_TOUCH=0 when < 25 is probably masking it for most cases.

Either way:

Acked-by: Chris Bagwell <chris@cnpbagwell.com>

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] Input: synaptics - report clickpad property

[Henrik Rydberg]

With the new input property interface, it is possible to report
the special quirks of a device using ioctl/sysfs. This patch
sets up the device as a pointer, and reports the clickpad
functionality via the INPUT_PROP_BUTTONPAD property.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 4744064..01302db 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -661,6 +661,8 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 {
 	int i;
 
+	__set_bit(INPUT_PROP_POINTER, dev->propbit);
+
 	__set_bit(EV_ABS, dev->evbit);
 	input_set_abs_params(dev, ABS_X,
 			     XMIN_NOMINAL, priv->x_max ?: XMAX_NOMINAL, 0, 0);
@@ -702,6 +704,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	input_abs_set_res(dev, ABS_Y, priv->y_res);
 
 	if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
+		__set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
 		/* Clickpads report only left button */
 		__clear_bit(BTN_RIGHT, dev->keybit);
 		__clear_bit(BTN_MIDDLE, dev->keybit);
-- 
1.7.2.3

[PATCH 4/4] 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 rectangle corners, allowing userspace to track and produce
appropriate gestures. The number of fingers are still reported as before.
The INPUT_PROP_SEMI_MT quirk is set to signal this special behavior.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---
 drivers/input/mouse/synaptics.c |   42 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 41 insertions(+), 1 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 01302db..1ae5c29 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,34 @@ 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)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, 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);
+	}
+}
+
+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));
+		set_slot(dev, 1, true, max(a->x, b->x), max(a->y, b->y));
+	} else if (num_fingers == 1) {
+		set_slot(dev, 0, true, a->x, a->y);
+		set_slot(dev, 1, false, 0, 0);
+	} else {
+		set_slot(dev, 0, false, 0, 0);
+		set_slot(dev, 1, false, 0, 0);
+	}
+}
+
 /*
  *  called for each full received packet from the touchpad
  */
@@ -551,6 +579,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
@@ -662,6 +693,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	int i;
 
 	__set_bit(INPUT_PROP_POINTER, dev->propbit);
+	__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
 
 	__set_bit(EV_ABS, dev->evbit);
 	input_set_abs_params(dev, ABS_X,
@@ -670,6 +702,14 @@ 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);
+	}
+
 	if (SYN_CAP_PALMDETECT(priv->capabilities))
 		input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
 
-- 
1.7.2.3

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

[Chris Bagwell]

I haven't tested this one but it looks like only 2 lines different
from one I did (report as FINGER again and new cap).  So you still
have my:

Acked-by: Chris Bagwell <chris@cnpbagwell.com>

On Sat, Dec 18, 2010 at 8:53 AM, Henrik Rydberg <rydberg@euromail.se> 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 rectangle corners, allowing userspace to track and produce
> appropriate gestures. The number of fingers are still reported as before.
> The INPUT_PROP_SEMI_MT quirk is set to signal this special behavior.
>
> Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
> ---
>  drivers/input/mouse/synaptics.c |   42 ++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 41 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 01302db..1ae5c29 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,34 @@ 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)
> +{
> +       input_mt_slot(dev, slot);
> +       input_mt_report_slot_state(dev, MT_TOOL_FINGER, 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);
> +       }
> +}
> +
> +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));
> +               set_slot(dev, 1, true, max(a->x, b->x), max(a->y, b->y));
> +       } else if (num_fingers == 1) {
> +               set_slot(dev, 0, true, a->x, a->y);
> +               set_slot(dev, 1, false, 0, 0);
> +       } else {
> +               set_slot(dev, 0, false, 0, 0);
> +               set_slot(dev, 1, false, 0, 0);
> +       }
> +}
> +
>  /*
>  *  called for each full received packet from the touchpad
>  */
> @@ -551,6 +579,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
> @@ -662,6 +693,7 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>        int i;
>
>        __set_bit(INPUT_PROP_POINTER, dev->propbit);
> +       __set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
>
>        __set_bit(EV_ABS, dev->evbit);
>        input_set_abs_params(dev, ABS_X,
> @@ -670,6 +702,14 @@ 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);
> +       }
> +
>        if (SYN_CAP_PALMDETECT(priv->capabilities))
>                input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
>
> --
> 1.7.2.3
>
>