Re: ALPS v4 Semi-mt Support

[George Pantalos <gpantalos@gmail.com>]

> On Tue, Apr 10, 2012 at 10:21:13AM -0500, Seth Forshee wrote:
> > I definitely think your state processing could be improved. My
> > suggestion would be to treat multi_packet as a counter. Reset it to 0
> > when you see the sync bit is set, and increment it for each packet until
> > you have a full set of MT data. That way you know for sure which section
> > of the MT data you're working with for any given packet. But be prepared
> > to handle an incomplete packet sequence as well (I'm pretty sure I saw
> > some of these when I was working with a v4 touchpad).
> 
> I found an old patch I had from working on the semi-MT support
> previously that demonstrates the multi_packet-as-counter approach. I
> ported the relevant code on top of 3.4-rc2, cleaned it up, compile
> tested it, and pasted the resulting patch below.
> 
> Note that this is ignoring the ST coordinates except from the final
> packet of the MT sequence, which isn't ideal. A better approach might be
> to stash the ST data from each packet in alps_data, then when you have
> all three packets process the bitmaps and report all three ST points
> with the same set of MT data.

Hello Seth,

First of all, sorry for answering so late, I had no internet access for the 
past week.

Thank you for your comments and for posting your patch. You are of course 
correct in your remarks.

 I decided to try and improve upon your patch which has superior state 
processing. I followed your proposal initially and stashed the first and 
second packet ST data in alps_data and if bitmap fingers < 2 report all the ST 
data.
The problem with this approach is that it introduces a noticeable delay/lag in 
mouse pointer movement.

I then tried using the last_fingers approach.  This way we report ST data as 
they come but only if the last MT report had 1 finger present and we always 
(as far as I can tell) must have calculated bitmap finger count before 
reporting ST events. I have posted this patch below. I also observed jumpy 
behavior with the xf86-input-multitouch driver when the MT report had 1 
finger, so if bitmap fingers are 1 , it uses x,y instead of x1,y1.
This approach has the benefit of producing smooth pointer movement and 
accurately reporting MT events like the other approach.

To handle inconsistent data I used your patch to dump raw packets.  I noticed 
,as your documentation also states, that the condition (packet[6] & 0x40) 
could also be triggered by the second MT packet and then reset 
multi_packet to 0.
So I used the fact that byte 5 of the MT data, priv->multi_data [4] must 
always be 0x00. A kind of second sync byte to ensure we have correct MT 
reports. Unless the MT data is consistent we report nothing.

Thanks for your help and guidance.

--- linux-3.3/drivers/input/mouse/alps.c.orig	2012-04-15 21:45:18.083826446 
+0300
+++ linux-3.3/drivers/input/mouse/alps.c	2012-04-16 16:33:11.412181964 
+0300
@@ -604,10 +604,39 @@
 
 static void alps_process_packet_v4(struct psmouse *psmouse)
 {
+	struct alps_data *priv = psmouse->private;
 	unsigned char *packet = psmouse->packet;
 	struct input_dev *dev = psmouse->dev;
+	int offset;
 	int x, y, z;
 	int left, right;
+	int x1, y1, x2, y2;
+	int fingers = 0;
+	unsigned int x_bitmap, y_bitmap;
+
+	/*
+	 * v4 has a 6-byte encoding for bitmap data, but this data is
+	 * broken up between 3 normal packets. Use priv->multi_packet to
+	 * track our position in the bitmap packet.
+	 */
+	if ((packet[6] & 0x40) && (priv->multi_data[4] == 0x00)) {
+		/* sync, reset position */
+		priv->multi_packet = 0;
+	}
+
+	if (WARN_ON_ONCE(priv->multi_packet > 2))
+		return;
+
+	offset = 2 * priv->multi_packet;
+	priv->multi_data[offset] = packet[6];
+	priv->multi_data[offset + 1] = packet[7];
+
+	/*
+	 * The 5th byte of the MT data must always be 0x00.
+	 * Try to re-sync our position if MT data is inconsistent.
+	 */
+	if (priv->multi_data[4] != 0x00)
+		return;
 
 	left = packet[4] & 0x01;
 	right = packet[4] & 0x02;
@@ -617,22 +646,78 @@
 	y = ((packet[2] & 0x7f) << 4) | (packet[3] & 0x0f);
 	z = packet[5] & 0x7f;
 
-	if (z >= 64)
-		input_report_key(dev, BTN_TOUCH, 1);
-	else
-		input_report_key(dev, BTN_TOUCH, 0);
 
-	if (z > 0) {
-		input_report_abs(dev, ABS_X, x);
-		input_report_abs(dev, ABS_Y, y);
-	}
-	input_report_abs(dev, ABS_PRESSURE, z);
+	if (++priv->multi_packet > 2) {
+		priv->multi_packet = 0;
+
+		x_bitmap = ((priv->multi_data[2] & 0x1f) << 10) |
+			   ((priv->multi_data[3] & 0x60) << 3) |
+			   ((priv->multi_data[0] & 0x3f) << 2) |
+			   ((priv->multi_data[1] & 0x60) >> 5);
+		y_bitmap = ((priv->multi_data[5] & 0x01) << 10) |
+			   ((priv->multi_data[3] & 0x1f) << 5) |
+			   (priv->multi_data[1] & 0x1f);
+
+		fingers = alps_process_bitmap(x_bitmap, y_bitmap,
+					      &x1, &y1, &x2, &y2);
 
-	input_report_key(dev, BTN_TOOL_FINGER, z > 0);
-	input_report_key(dev, BTN_LEFT, left);
-	input_report_key(dev, BTN_RIGHT, right);
+		/*
+		 * If there were no contacts in the bitmap, use ST
+		 * points in MT reports.
+		 */
+		if (fingers < 2) {
+			x1 = x;
+			y1 = y;
+			fingers = z > 0 ? 1 : 0;
+		}
+
+		priv->last_fingers = fingers;
+
+		if (z >= 64)
+			input_report_key(dev, BTN_TOUCH, 1);
+		else
+			input_report_key(dev, BTN_TOUCH, 0);
+
+		alps_report_semi_mt_data(dev, fingers, x1, y1, x2, y2);
+
+		input_report_key(dev, BTN_TOOL_FINGER, fingers == 1);
+		input_report_key(dev, BTN_TOOL_DOUBLETAP, fingers == 2);
+		input_report_key(dev, BTN_TOOL_TRIPLETAP, fingers == 3);
+		input_report_key(dev, BTN_TOOL_QUADTAP, fingers == 4);
+
+		input_report_key(dev, BTN_LEFT, left);
+		input_report_key(dev, BTN_RIGHT, right);
+
+		if (z > 0) {
+			input_report_abs(dev, ABS_X, x);
+			input_report_abs(dev, ABS_Y, y);
+		}
+		input_report_abs(dev, ABS_PRESSURE, z);
+
+		input_sync(dev);
+	} else {
+		if (priv->last_fingers == 1) {
+			if (z >= 64)
+				input_report_key(dev, BTN_TOUCH, 1);
+			else
+				input_report_key(dev, BTN_TOUCH, 0);
+
+			alps_report_semi_mt_data(dev, 1, x, y, 0, 0);
+
+			input_report_key(dev, BTN_TOOL_FINGER, z > 0);
+
+			input_report_key(dev, BTN_LEFT, left);
+			input_report_key(dev, BTN_RIGHT, right);
+
+			if (z > 0) {
+				input_report_abs(dev, ABS_X, x);
+				input_report_abs(dev, ABS_Y, y);
+			}
+			input_report_abs(dev, ABS_PRESSURE, z);
 
-	input_sync(dev);
+			input_sync(dev);
+		}
+	}
 }
 
 static void alps_process_packet(struct psmouse *psmouse)
@@ -1557,6 +1642,7 @@
 		input_set_abs_params(dev1, ABS_Y, 0, 767, 0, 0);
 		break;
 	case ALPS_PROTO_V3:
+	case ALPS_PROTO_V4:
 		set_bit(INPUT_PROP_SEMI_MT, dev1->propbit);
 		input_mt_init_slots(dev1, 2);
 		input_set_abs_params(dev1, ABS_MT_POSITION_X, 0, 
ALPS_V3_X_MAX, 0, 0);
@@ -1565,8 +1651,7 @@
 		set_bit(BTN_TOOL_DOUBLETAP, dev1->keybit);
 		set_bit(BTN_TOOL_TRIPLETAP, dev1->keybit);
 		set_bit(BTN_TOOL_QUADTAP, dev1->keybit);
-		/* fall through */
-	case ALPS_PROTO_V4:
+
 		input_set_abs_params(dev1, ABS_X, 0, ALPS_V3_X_MAX, 0, 0);
 		input_set_abs_params(dev1, ABS_Y, 0, ALPS_V3_Y_MAX, 0, 0);
 		break;
--- linux-3.3/drivers/input/mouse/alps.h.orig	2012-04-16 
16:39:49.948823942 +0300
+++ linux-3.3/drivers/input/mouse/alps.h	2012-04-16 16:41:28.228817760 
+0300
@@ -39,6 +39,7 @@
 	int prev_fin;			/* Finger bit from previous packet */
 	int multi_packet;		/* Multi-packet data in progress */
 	unsigned char multi_data[6];	/* Saved multi-packet data */
+	int last_fingers;		/* Number of fingers from MT report*/
 	u8 quirks;
 	struct timer_list timer;
 };