[PATCH 0/8 v3] Synaptics image sensor support

[PATCH 0/8 v3] Synaptics image sensor support

[Daniel Kurtz]

Hello,

----
For v3:
  Merged Patch 5 & 6:
     BTN_TOOL_*TAP always reports correct number of fingers.
     During certain ambiguous number-of-finger-transitions, there may be less
     than 2 valid MT-B slots, even if the BTN_TOOL_*TAP value indicates two or
     more fingers are present on the pad.
----

This patch set (against next) is intended to add support for synaptics
"image sensor" touchpads.

Patches 1-3 clean up the current driver slightly and prepare for the image
sensor patches which follow.

Patches 4-6 add 3 finger support for image sensor touchpads.
Image sensors do not suffer from the finger tracking issues that plagued
the earlier "profile sensors", and which required the invention of "semi-mt"
(Semi-mt reports a bounding box around two fingers instead of the fingers
themselves).  Instead, the image sensors report the actual positions of two
fingers using the same "Advanced Gesture Mode".  This driver uses two MT-B slots
to report these two fingers to userspace.  In addition, it will also report
the total number of fingers using BTN_TOOL_*TAP EV_KEY events.

Userspace drivers should be aware that the number of fingers reported via
BTN_TOOL_*TAP can be greater than the total number MT-B slots with non-negative
track_ids.  Upon opening the device node, userspace should query the maximum
values supported ABS_MT_SLOT, and note the number of supported BTN_TOOL_*TAP
events.

Patches 7-8 add 4 and 5 finger support.
In fact, the image sensor touchpads actually track 5 fingers while reporting
just 2 finger positions.  These patches add support for properly tracking the
reported slots through 4 and 5 finger transitions, while always reporting two of
them via 2 MT-B slots.  In addition, a new event, EV_KEY/BTN_TOOL_QUINTTAP, is
added to the event subsystem to allow reporting up to 5 fingers.

These patches are similar to, and inspired by, a similar patchset recently
submitted by Derek Foreman and Daniel Stone.  However, it is not directly built
upon, nor is it compatible with, those patches.

Thanks,
Daniel

Daniel Kurtz (8):
  Input: synaptics - refactor y inversion
  Input: synaptics - refactor agm packet parsing
  Input: synaptics - refactor initialization of abs position axes
  Input: synaptics - add image sensor support
  Input: synaptics - decode AGM packet types
  Input: synaptics - process finger (<=3) transitions
  Input: add BTN_TOOL_QUINTTAP for reporting 5 fingers on touchpad
  Input: synaptics - process finger (<=5) transitions

 drivers/input/input-mt.c        |    1 +
 drivers/input/mouse/synaptics.c |  506 ++++++++++++++++++++++++++++++++++++---
 drivers/input/mouse/synaptics.h |   27 ++-
 include/linux/input.h           |    1 +
 4 files changed, 498 insertions(+), 37 deletions(-)

-- 
1.7.3.1

[PATCH 1/8 v3] Input: synaptics - refactor y inversion

[Daniel Kurtz]

Synaptics touchpads report increasing y from bottom to top.
This is inverted from normal userspace "top of screen is 0" coordinates.
Thus, the kernel driver reports inverted y coordinates to userspace.

This patch refactors this inversion.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Acked-by: Chase Douglas <chase.douglas@canonical.com>
---
 drivers/input/mouse/synaptics.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 5538fc6..297e88f 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -44,6 +44,16 @@
 #define YMIN_NOMINAL 1408
 #define YMAX_NOMINAL 4448
 
+/*
+ * Synaptics touchpads report the y coordinate from bottom to top, which is
+ * opposite from what userspace expects.
+ * This function is used to invert y before reporting.
+ */
+static int invert_y(int y)
+{
+	return YMAX_NOMINAL + YMIN_NOMINAL - y;
+}
+
 
 /*****************************************************************************
  *	Stuff we need even when we do not want native Synaptics support
@@ -502,8 +512,7 @@ static void synaptics_report_semi_mt_slot(struct input_dev *dev, int 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);
+		input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(y));
 	}
 }
 
@@ -597,7 +606,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 
 	if (num_fingers > 0) {
 		input_report_abs(dev, ABS_X, hw.x);
-		input_report_abs(dev, ABS_Y, YMAX_NOMINAL + YMIN_NOMINAL - hw.y);
+		input_report_abs(dev, ABS_Y, invert_y(hw.y));
 	}
 	input_report_abs(dev, ABS_PRESSURE, hw.z);
 
-- 
1.7.3.1

[PATCH 2/8 v3] Input: synaptics - refactor agm packet parsing

[Daniel Kurtz]

When a Synaptics touchpad is in "AGM" mode, and multiple fingers are
detected, the touchpad sends alternating "Advanced Gesture Mode" (AGM) and
"Simple Gesture Mode" (SGM) packets.
  The AGM packets have w=2, and contain reduced resolution finger data.
  The SGM packets have w={0,1} and contain full resolution finger data.

Refactor the parsing of agm packets to its own function, and rename the
synaptics_data.mt field to .agm to indicate that it contains the contents of
the last agm packet.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Acked-by: Chase Douglas <chase.douglas@canonical.com>
---
 drivers/input/mouse/synaptics.c |   19 ++++++++++++++-----
 drivers/input/mouse/synaptics.h |    6 +++++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 297e88f..e966894 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -419,6 +419,17 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
+static void synaptics_parse_agm(const unsigned char buf[],
+				struct synaptics_data *priv)
+{
+	struct synaptics_hw_state *agm = &priv->agm;
+
+	/* Gesture packet: (x, y, z) at half resolution */
+	agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
+	agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
+	agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+}
+
 static int synaptics_parse_hw_state(const unsigned char buf[],
 				    struct synaptics_data *priv,
 				    struct synaptics_hw_state *hw)
@@ -453,10 +464,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 		}
 
 		if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) && hw->w == 2) {
-			/* Gesture packet: (x, y, z) at half resolution */
-			priv->mt.x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
-			priv->mt.y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
-			priv->mt.z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+			synaptics_parse_agm(buf, priv);
 			return 1;
 		}
 
@@ -595,7 +603,8 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	}
 
 	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c))
-		synaptics_report_semi_mt_data(dev, &hw, &priv->mt, num_fingers);
+		synaptics_report_semi_mt_data(dev, &hw, &priv->agm,
+					      num_fingers);
 
 	/* Post events
 	 * BTN_TOUCH has to be first as mousedev relies on it when doing
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index ca040aa..a9efbf3 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -146,7 +146,11 @@ struct synaptics_data {
 
 	struct serio *pt_port;			/* Pass-through serio port */
 
-	struct synaptics_hw_state mt;		/* current gesture packet */
+	/*
+	 * Last received Advanced Gesture Mode (AGM) packet. An AGM packet
+	 * contains position data for a second contact, at half resolution.
+	 */
+	struct synaptics_hw_state agm;
 };
 
 void synaptics_module_init(void);
-- 
1.7.3.1

[PATCH 3/8 v3] Input: synaptics - refactor initialization of abs position axes

[Daniel Kurtz]

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Acked-by: Chase Douglas <chase.douglas@canonical.com>
---
 drivers/input/mouse/synaptics.c |   44 +++++++++++++++++---------------------
 1 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index e966894..30d05fd 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -712,39 +712,38 @@ static psmouse_ret_t synaptics_process_byte(struct psmouse *psmouse)
 /*****************************************************************************
  *	Driver initialization/cleanup functions
  ****************************************************************************/
+static void set_abs_position_params(struct input_dev *dev,
+				    struct synaptics_data *priv, int x_code,
+				    int y_code)
+{
+	int x_min = priv->x_min ?: XMIN_NOMINAL;
+	int x_max = priv->x_max ?: XMAX_NOMINAL;
+	int y_min = priv->y_min ?: YMIN_NOMINAL;
+	int y_max = priv->y_max ?: YMAX_NOMINAL;
+	int fuzz = SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) ?
+			SYN_REDUCED_FILTER_FUZZ : 0;
+
+	input_set_abs_params(dev, x_code, x_min, x_max, fuzz, 0);
+	input_set_abs_params(dev, y_code, y_min, y_max, fuzz, 0);
+	input_abs_set_res(dev, x_code, priv->x_res);
+	input_abs_set_res(dev, y_code, priv->y_res);
+}
+
 static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 {
 	int i;
-	int fuzz = SYN_CAP_REDUCED_FILTERING(priv->ext_cap_0c) ?
-			SYN_REDUCED_FILTER_FUZZ : 0;
 
 	__set_bit(INPUT_PROP_POINTER, dev->propbit);
 
 	__set_bit(EV_ABS, dev->evbit);
-	input_set_abs_params(dev, ABS_X,
-			     priv->x_min ?: XMIN_NOMINAL,
-			     priv->x_max ?: XMAX_NOMINAL,
-			     fuzz, 0);
-	input_set_abs_params(dev, ABS_Y,
-			     priv->y_min ?: YMIN_NOMINAL,
-			     priv->y_max ?: YMAX_NOMINAL,
-			     fuzz, 0);
+	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
 	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
 		__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
 		input_mt_init_slots(dev, 2);
-		input_set_abs_params(dev, ABS_MT_POSITION_X,
-				     priv->x_min ?: XMIN_NOMINAL,
-				     priv->x_max ?: XMAX_NOMINAL,
-				     fuzz, 0);
-		input_set_abs_params(dev, ABS_MT_POSITION_Y,
-				     priv->y_min ?: YMIN_NOMINAL,
-				     priv->y_max ?: YMAX_NOMINAL,
-				     fuzz, 0);
-
-		input_abs_set_res(dev, ABS_MT_POSITION_X, priv->x_res);
-		input_abs_set_res(dev, ABS_MT_POSITION_Y, priv->y_res);
+		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
+					ABS_MT_POSITION_Y);
 	}
 
 	if (SYN_CAP_PALMDETECT(priv->capabilities))
@@ -777,9 +776,6 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	__clear_bit(REL_X, dev->relbit);
 	__clear_bit(REL_Y, dev->relbit);
 
-	input_abs_set_res(dev, ABS_X, priv->x_res);
-	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 */
-- 
1.7.3.1

[PATCH 4/8 v3] Input: synaptics - add image sensor support

[Daniel Kurtz]

Synaptics makes (at least) two kinds of touchpad sensors:
 * Older pads use a profile sensor that could only infer the location
   of individual fingers based on the projection of their profiles
   onto row and column sensors.
 * Newer pads use an image sensor that can track true finger position
   using a two-dimensional sensor grid.

Both sensor types support an "Advanced Gesture Mode":
 When multiple fingers are detected, the touchpad sends alternating
 "Advanced Gesture Mode" (AGM) and "Simple Gesture Mode" (SGM)
 packets.
 The AGM packets have w=2, and contain reduced resolution finger data
 The SGM packets have w={0,1} and contain full resolution finger data

Profile sensors try to report the "upper" (larger y value) finger in
the SGM packet, and the lower (smaller y value) in the AGM packet.
However, due to the nature of the profile sensor, they easily get
confused when fingers cross, and can start reporting the x-coordinate
of one with the y-coordinate of the other.  Thus, for profile
sensors, "semi-mt" was created, which reports a "bounding box"
created by pairing min and max coordinates of the two pairs of
reported fingers.

Image sensors can report the actual coordinates of two of the fingers
present.  This patch detects if the touchpad is an image sensor and
reports finger data using the MT-B protocol.

NOTE: This patch only adds partial support for 2-finger gestures.
      The proper interpretation of the slot contents when more than
      two fingers are present is left to later patches.  Also,
      handling of 'number of fingers' transitions is incomplete.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |  104 +++++++++++++++++++++++++++++++++++++-
 drivers/input/mouse/synaptics.h |    3 +
 2 files changed, 104 insertions(+), 3 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 30d05fd..a9960d2 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -304,7 +304,8 @@ static int synaptics_set_advanced_gesture_mode(struct psmouse *psmouse)
 	static unsigned char param = 0xc8;
 	struct synaptics_data *priv = psmouse->private;
 
-	if (!SYN_CAP_ADV_GESTURE(priv->ext_cap_0c))
+	if (!(SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) ||
+			SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)))
 		return 0;
 
 	if (psmouse_sliced_command(psmouse, SYN_QUE_MODEL))
@@ -463,7 +464,9 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 			hw->down = ((buf[0] ^ buf[3]) & 0x02) ? 1 : 0;
 		}
 
-		if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c) && hw->w == 2) {
+		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)
+				|| SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
+				&& hw->w == 2) {
 			synaptics_parse_agm(buf, priv);
 			return 1;
 		}
@@ -543,6 +546,87 @@ static void synaptics_report_semi_mt_data(struct input_dev *dev,
 	}
 }
 
+static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
+}
+
+static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
+				      const struct synaptics_hw_state *sgm)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
+	input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
+	input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
+	input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
+	/* SGM can sometimes contain valid width */
+	if (sgm->w >= 4)
+		input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
+}
+
+static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
+				      const struct synaptics_hw_state *agm)
+{
+	input_mt_slot(dev, slot);
+	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
+	input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
+	input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
+	input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
+}
+
+static void synaptics_report_mt(struct psmouse *psmouse,
+				int count,
+				const struct synaptics_hw_state *sgm)
+{
+	struct input_dev *dev = psmouse->dev;
+	struct synaptics_data *priv = psmouse->private;
+	struct synaptics_hw_state *agm = &priv->agm;
+
+	switch (count) {
+	case 0:
+		synaptics_report_slot_empty(dev, 0);
+		synaptics_report_slot_empty(dev, 1);
+		break;
+	case 1:
+		synaptics_report_slot_sgm(dev, 0, sgm);
+		synaptics_report_slot_empty(dev, 1);
+		break;
+	case 2:
+	case 3: /* Fall-through case */
+		synaptics_report_slot_sgm(dev, 0, sgm);
+		synaptics_report_slot_agm(dev, 1, agm);
+		break;
+	}
+
+	/* Don't use active slot count to generate BTN_TOOL events. */
+	input_mt_report_pointer_emulation(dev, false);
+
+	/* Send the number of fingers reported by touchpad itself. */
+	input_mt_report_finger_count(dev, count);
+
+	input_report_key(dev, BTN_LEFT, sgm->left);
+	input_sync(dev);
+}
+
+static void synaptics_image_sensor_process(struct psmouse *psmouse,
+					   struct synaptics_hw_state *sgm)
+{
+	int count;
+
+	if (sgm->z == 0)
+		count = 0;
+	else if (sgm->w >= 4)
+		count = 1;
+	else if (sgm->w == 0)
+		count = 2;
+	else
+		count = 3;
+
+	/* Send resulting input events to user space */
+	synaptics_report_mt(psmouse, count, sgm);
+}
+
 /*
  *  called for each full received packet from the touchpad
  */
@@ -558,6 +642,11 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
 		return;
 
+	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
+		synaptics_image_sensor_process(psmouse, &hw);
+		return;
+	}
+
 	if (hw.scroll) {
 		priv->scroll += hw.scroll;
 
@@ -739,7 +828,16 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
 	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
 
-	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
+		input_mt_init_slots(dev, 2);
+		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
+					ABS_MT_POSITION_Y);
+		/* Image sensors can report per-contact pressure */
+		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
+		/* Image sensors can sometimes report per-contact width */
+		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
+	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+		/* Non-image sensors with AGM use semi-mt */
 		__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
 		input_mt_init_slots(dev, 2);
 		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index a9efbf3..0ea7616 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -74,6 +74,8 @@
  * 2	0x04	reduced filtering	firmware does less filtering on
  *					position data, driver should watch
  *					for noise.
+ * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
+ *					reports 2.
  * 2	0x20	report min		query 0x0f gives min coord reported
  */
 #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
@@ -82,6 +84,7 @@
 #define SYN_CAP_MIN_DIMENSIONS(ex0c)	((ex0c) & 0x002000)
 #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
 #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
+#define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
 
 /* synaptics modes query bits */
 #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
-- 
1.7.3.1

[PATCH 5/8 v3] Input: synaptics - decode AGM packet types

[Daniel Kurtz]

A Synaptics image sensor tracks 5 fingers, but can only report 2.

The algorithm for choosing which 2 fingers to report and in which packet:
  Touchpad maintains 5 slots, numbered 0 to 4
  Initially all slots are empty
  As new fingers are detected, assign them to the lowest available slots
  The touchpad always reports:
    SGM: lowest numbered non-empty slot
    AGM: highest numbered non-empty slot, if there is one

In addition, these touchpads have a special AGM packet type which reports
the number of fingers currently being tracked, and which finger is in
each of the two slots.  Unfortunately, these "TYPE=2" packets are only used
when more than 3 fingers are being tracked.  When less than 4 fingers
are present, the 'w' value must be used to track how many fingers are
present, and knowing which fingers are being reported is much more
difficult, if not impossible.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |   38 +++++++++++++++++++++++++++++++-------
 drivers/input/mouse/synaptics.h |   14 +++++++++++++-
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index a9960d2..ff8c839 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -420,15 +420,39 @@ static void synaptics_pt_create(struct psmouse *psmouse)
  *	Functions to interpret the absolute mode packets
  ****************************************************************************/
 
+static void synaptics_mt_state_set(struct synaptics_mt_state *state, int count,
+				   int sgm, int agm)
+{
+	state->count = count;
+	state->sgm = sgm;
+	state->agm = agm;
+}
+
 static void synaptics_parse_agm(const unsigned char buf[],
-				struct synaptics_data *priv)
+				struct synaptics_data *priv,
+				struct synaptics_hw_state *hw)
 {
 	struct synaptics_hw_state *agm = &priv->agm;
+	int agm_packet_type;
+
+	agm_packet_type = (buf[5] & 0x30) >> 4;
+	switch (agm_packet_type) {
+	case 1:
+		/* Gesture packet: (x, y, z) half resolution */
+		agm->w = hw->w;
+		agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
+		agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
+		agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+		break;
 
-	/* Gesture packet: (x, y, z) at half resolution */
-	agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
-	agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
-	agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+	case 2:
+		/* AGM-CONTACT packet: (count, sgm, agm) */
+		synaptics_mt_state_set(&agm->mt_state, buf[1], buf[2], buf[4]);
+		break;
+
+	default:
+		break;
+	}
 }
 
 static int synaptics_parse_hw_state(const unsigned char buf[],
@@ -467,7 +491,7 @@ static int synaptics_parse_hw_state(const unsigned char buf[],
 		if ((SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)
 				|| SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c))
 				&& hw->w == 2) {
-			synaptics_parse_agm(buf, priv);
+			synaptics_parse_agm(buf, priv, hw);
 			return 1;
 		}
 
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 0ea7616..20f57df 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -115,9 +115,18 @@
 #define SYN_REDUCED_FILTER_FUZZ		8
 
 /*
- * A structure to describe the state of the touchpad hardware (buttons and pad)
+ * A structure to describe which internal touchpad finger slots are being
+ * reported in raw packets.
  */
+struct synaptics_mt_state {
+	int count;			/* num fingers being tracked */
+	int sgm;			/* which slot is reported by sgm pkt */
+	int agm;			/* which slot is reported by agm pkt*/
+};
 
+/*
+ * A structure to describe the state of the touchpad hardware (buttons and pad)
+ */
 struct synaptics_hw_state {
 	int x;
 	int y;
@@ -130,6 +139,9 @@ struct synaptics_hw_state {
 	unsigned int down:1;
 	unsigned char ext_buttons;
 	signed char scroll;
+
+	/* As reported in last AGM-CONTACT packets */
+	struct synaptics_mt_state mt_state;
 };
 
 struct synaptics_data {
-- 
1.7.3.1

[PATCH 6/8 v3] Input: synaptics - process finger (<=3) transitions

[Daniel Kurtz]

Synaptics image sensor touchpads track 5 fingers, but only report 2.
This patch attempts to deal with some idiosyncrasies of these touchpads:

 * When there are 3 or more fingers, only two are reported.
 * The touchpad tracks the 5 fingers in slot[0] through slot[4].
 * It always reports the lowest and highest valid slots in SGM and AGM
   packets, respectively.
 * The number of fingers is only reported in the SGM packet.  However,
   the number of fingers can change either before or after an AGM
   packet.
 * Thus, if an SGM reports a different number of fingers than the last
   SGM, it is impossible to tell whether the intervening AGM corresponds
   to the old number of fingers or the new number of fingers.
 * For example, when going from 2->3 fingers, it is not possible to tell
   whether tell AGM contains slot[1] (old 2nd finger) or slot[2] (new
   3rd finger).
 * When fingers are added one at at time, from 1->2->3, it is possible to
   track which slots are contained in the SGM and AGM packets:
     1 finger:  SGM = slot[0], no AGM
     2 fingers: SGM = slot[0], AGM = slot[1]
     3 fingers: SGM = slot[0], AGM = slot[2]
 * It is also possible to track which slot is contained in the SGM when 1
   of 2 fingers is removed.  This is because the touchpad sends a special
   (0,0,0) AGM packet whenever all fingers are removed except slot[0]:
     Last AGM == (0,0,0): SGM contains slot[1]
     Else: SGM contains slot[0]
 * However, once there are 3 fingers, if exactly 1 finger is removed, it
   is impossible to tell which 2 slots are contained in SGM and AGM.
   The (SGM,AGM) could be (0,1), (0,2), or (1,2). There is no way to know.
 * Similarly, if two fingers are simultaneously removed (3->1), then it
   is only possible to know if SGM still contains slot[0].
 * Since it is not possible to reliably track which slot is being
   reported, we invalidate the tracking_id every time the number of
   fingers changes until this ambiguity is resolved when:
     a) All fingers are removed.
     b) 4 or 5 fingers are touched, generates an AGM-CONTACT packet.
     c) All fingers are removed except slot[0].  In this special case, the
        ambiguity is resolved since by the (0,0,0) AGM packet.

Behavior of the driver:

When 2 or more fingers are present on the touchpad, the kernel reports
up to two MT-B slots containing the position data for two of the fingers
reported by the touchpad.  If the identity of a finger cannot be tracked
when the number-of-fingers changes, the corresponding MT-B slot will be
invalidated (track_id set to -1), and a new track_id will be assigned in
a subsequent input event report.

The driver always reports the total number of fingers using one of the
EV_KEY/BTN_TOOL_*TAP events. This could differ from the number of valid
MT-B slots for two reasons:
 a) There are more than 2 fingers on the pad.
 b) During ambiguous number-of-fingers transitions, the correct track_id
    for one or both of the slots cannot be determined, so the slots are
    invalidated.

Thus, this is a hybrid singletouch/MT-B scheme. Userspace can detect
this behavior by noting that the driver supports more EV_KEY/BTN_TOOL_*TAP
events than its maximum EV_ABS/ABS_MT_SLOT.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |  290 ++++++++++++++++++++++++++++++++++++--
 drivers/input/mouse/synaptics.h |    4 +
 2 files changed, 278 insertions(+), 16 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index ff8c839..57104ed 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -453,6 +453,9 @@ static void synaptics_parse_agm(const unsigned char buf[],
 	default:
 		break;
 	}
+
+	/* Record that at least one AGM has been received since last SGM */
+	priv->agm_pending = true;
 }
 
 static int synaptics_parse_hw_state(const unsigned char buf[],
@@ -600,26 +603,53 @@ static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
 }
 
 static void synaptics_report_mt(struct psmouse *psmouse,
-				int count,
+				struct synaptics_mt_state *mt_state,
 				const struct synaptics_hw_state *sgm)
 {
 	struct input_dev *dev = psmouse->dev;
 	struct synaptics_data *priv = psmouse->private;
 	struct synaptics_hw_state *agm = &priv->agm;
+	struct synaptics_mt_state *old = &priv->mt_state;
 
-	switch (count) {
+	switch (mt_state->count) {
 	case 0:
 		synaptics_report_slot_empty(dev, 0);
 		synaptics_report_slot_empty(dev, 1);
 		break;
 	case 1:
-		synaptics_report_slot_sgm(dev, 0, sgm);
-		synaptics_report_slot_empty(dev, 1);
+		if (mt_state->sgm == -1) {
+			synaptics_report_slot_empty(dev, 0);
+			synaptics_report_slot_empty(dev, 1);
+		} else if (mt_state->sgm == 0) {
+			synaptics_report_slot_sgm(dev, 0, sgm);
+			synaptics_report_slot_empty(dev, 1);
+		} else {
+			synaptics_report_slot_empty(dev, 0);
+			synaptics_report_slot_sgm(dev, 1, sgm);
+		}
 		break;
-	case 2:
-	case 3: /* Fall-through case */
-		synaptics_report_slot_sgm(dev, 0, sgm);
-		synaptics_report_slot_agm(dev, 1, agm);
+	default:
+		/*
+		 * If the finger slot contained in SGM is valid, and either
+		 * hasn't changed, or is new, then report SGM in MTB slot 0.
+		 * Otherwise, empty MTB slot 0.
+		 */
+		if (mt_state->sgm != -1 &&
+		    (mt_state->sgm == old->sgm || old->sgm == -1))
+			synaptics_report_slot_sgm(dev, 0, sgm);
+		else
+			synaptics_report_slot_empty(dev, 0);
+
+		/*
+		 * If the finger slot contained in AGM is valid, and either
+		 * hasn't changed, or is new, then report AGM in MTB slot 1.
+		 * Otherwise, empty MTB slot 1.
+		 */
+		if (mt_state->agm != -1 &&
+		    (mt_state->agm == old->agm || old->agm == -1))
+			synaptics_report_slot_agm(dev, 1, agm);
+		else
+			synaptics_report_slot_empty(dev, 1);
 		break;
 	}
 
@@ -627,28 +657,256 @@ static void synaptics_report_mt(struct psmouse *psmouse,
 	input_mt_report_pointer_emulation(dev, false);
 
 	/* Send the number of fingers reported by touchpad itself. */
-	input_mt_report_finger_count(dev, count);
+	input_mt_report_finger_count(dev, mt_state->count);
 
 	input_report_key(dev, BTN_LEFT, sgm->left);
 	input_sync(dev);
 }
 
+/* Handle case where mt_state->count = 0 */
+static void synaptics_image_sensor_0f(struct synaptics_data *priv,
+				      struct synaptics_mt_state *mt_state)
+{
+	synaptics_mt_state_set(mt_state, 0, -1, -1);
+	priv->mt_state_lost = false;
+}
+
+/* Handle case where mt_state->count = 1 */
+static void synaptics_image_sensor_1f(struct synaptics_data *priv,
+				      struct synaptics_mt_state *mt_state)
+{
+	struct synaptics_hw_state *agm = &priv->agm;
+	struct synaptics_mt_state *old = &priv->mt_state;
+
+	/*
+	 * If the last AGM was (0,0,0), and there is only one finger left,
+	 * then we absolutely know that SGM contains slot 0, and all other
+	 * fingers have been removed.
+	 */
+	if (priv->agm_pending && agm->z == 0) {
+		synaptics_mt_state_set(mt_state, 1, 0, -1);
+		priv->mt_state_lost = false;
+		return;
+	}
+
+	switch (old->count) {
+	case 0:
+		synaptics_mt_state_set(mt_state, 1, 0, -1);
+		break;
+	case 1:
+		/*
+		 * If mt_state_lost, then the previous transition was 3->1,
+		 * and SGM now contains either slot 0 or 1, but we don't know
+		 * which.  So, we just assume that the SGM now contains slot 1.
+		 *
+		 * If pending AGM and either:
+		 *   (a) the previous SGM slot contains slot 0, or
+		 *   (b) there was no SGM slot
+		 * then, the SGM now contains slot 1
+		 *
+		 * Case (a) happens with very rapid "drum roll" gestures, where
+		 * slot 0 finger is lifted and a new slot 1 finger touches
+		 * within one reporting interval.
+		 *
+		 * Case (b) happens if initially two or more fingers tap
+		 * briefly, and all but one lift before the end of the first
+		 * reporting interval.
+		 *
+		 * (In both these cases, slot 0 will becomes empty, so SGM
+		 * contains slot 1 with the new finger)
+		 *
+		 * Else, if there was no previous SGM, it now contains slot 0.
+		 *
+		 * Otherwise, SGM still contains the same slot.
+		 */
+		if (priv->mt_state_lost ||
+		    (priv->agm_pending && old->sgm <= 0))
+			synaptics_mt_state_set(mt_state, 1, 1, -1);
+		else if (old->sgm == -1)
+			synaptics_mt_state_set(mt_state, 1, 0, -1);
+		break;
+	case 2:
+		/*
+		 * If mt_state_lost, we don't know which finger SGM contains.
+		 *
+		 * So, report 1 finger, but with both slots empty.
+		 * We will use slot 1 on subsequent 1->1
+		 */
+		if (priv->mt_state_lost) {
+			synaptics_mt_state_set(mt_state, 1, -1, -1);
+			break;
+		}
+		/*
+		 * Since the last AGM was NOT (0,0,0), it was the finger in
+		 * slot 0 that has been removed.
+		 * So, SGM now contains previous AGM's slot, and AGM is now
+		 * empty.
+		 */
+		synaptics_mt_state_set(mt_state, 1, old->agm, -1);
+		break;
+	case 3:
+		/*
+		 * Since last AGM was not (0,0,0), we don't know which finger
+		 * is left.
+		 *
+		 * So, report 1 finger, but with both slots empty.
+		 * We will use slot 1 on subsequent 1->1
+		 */
+		synaptics_mt_state_set(mt_state, 1, -1, -1);
+		priv->mt_state_lost = true;
+		break;
+	}
+}
+
+/* Handle case where mt_state->count = 2 */
+static void synaptics_image_sensor_2f(struct synaptics_data *priv,
+				      struct synaptics_mt_state *mt_state)
+{
+	struct synaptics_mt_state *old = &priv->mt_state;
+
+	switch (old->count) {
+	case 0:
+		synaptics_mt_state_set(mt_state, 2, 0, 1);
+		break;
+	case 1:
+		/*
+		 * If previous SGM contained slot 1 or higher, SGM now contains
+		 * slot 0 (the newly touching finger) and AGM contains SGM's
+		 * previous slot.
+		 *
+		 * Otherwise, SGM still contains slot 0 and AGM now contains
+		 * slot 1.
+		 */
+		if (old->sgm >= 1)
+			synaptics_mt_state_set(mt_state, 2, 0, old->sgm);
+		else
+			synaptics_mt_state_set(mt_state, 2, 0, 1);
+		break;
+	case 2:
+		/*
+		 * If mt_state_lost, SGM now contains either finger 1 or 2, but
+		 * we don't know which.
+		 * So, we just assume that the SGM contains slot 0 and AGM 1.
+		 */
+		if (priv->mt_state_lost)
+			synaptics_mt_state_set(mt_state, 2, 0, 1);
+		/*
+		 * Otherwise, use the same mt_state, since it either hasn't
+		 * changed, or was updated by a recently received AGM-CONTACT
+		 * packet.
+		 */
+		break;
+	case 3:
+		/*
+		 * 3->2 transitions have two unsolvable problems:
+		 *  1) no indication is given which finger was removed
+		 *  2) no way to tell if agm packet was for finger 3
+		 *     before 3->2, or finger 2 after 3->2.
+		 *
+		 * So, report 2 fingers, but empty all slots.
+		 * We will guess slots [0,1] on subsequent 2->2.
+		 */
+		synaptics_mt_state_set(mt_state, 2, -1, -1);
+		priv->mt_state_lost = true;
+		break;
+	}
+}
+
+/* Handle case where mt_state->count = 3 */
+static void synaptics_image_sensor_3f(struct synaptics_data *priv,
+				      struct synaptics_mt_state *mt_state)
+{
+	struct synaptics_mt_state *old = &priv->mt_state;
+
+	switch (old->count) {
+	case 0:
+		synaptics_mt_state_set(mt_state, 3, 0, 2);
+		break;
+	case 1:
+		/*
+		 * If previous SGM contained slot 2 or higher, SGM now contains
+		 * slot 0 (one of the newly touching fingers) and AGM contains
+		 * SGM's previous slot.
+		 *
+		 * Otherwise, SGM now contains slot 0 and AGM contains slot 2.
+		 */
+		if (old->sgm >= 2)
+			synaptics_mt_state_set(mt_state, 3, 0, old->sgm);
+		else
+			synaptics_mt_state_set(mt_state, 3, 0, 2);
+		break;
+	case 2:
+		/*
+		 * After some 3->1 and all 3->2 transitions, we lose track
+		 * of which slot is reported by SGM and AGM.
+		 *
+		 * For 2->3 in this state, report 3 fingers, but empty all
+		 * slots, and we will guess (0,2) on a subsequent 0->3.
+		 *
+		 * To userspace, the resulting transition will look like:
+		 *    2:[0,1] -> 3:[-1,-1] -> 3:[0,2]
+		 */
+		if (priv->mt_state_lost) {
+			synaptics_mt_state_set(mt_state, 3, -1, -1);
+			break;
+		}
+
+		/*
+		 * If the (SGM,AGM) really previously contained slots (0, 1),
+		 * then we cannot know what slot was just reported by the AGM,
+		 * because the 2->3 transition can occur either before or after
+		 * the AGM packet. Thus, this most recent AGM could contain
+		 * either the same old slot 1 or the new slot 2.
+		 * Subsequent AGMs will be reporting slot 2.
+		 *
+		 * To userspace, the resulting transition will look like:
+		 *    2:[0,1] -> 3:[0,-1] -> 3:[0,2]
+		 */
+		synaptics_mt_state_set(mt_state, 3, 0, -1);
+		break;
+	case 3:
+		/*
+		 * If, for whatever reason, the previous agm was invalid,
+		 * Assume SGM now contains slot 0, AGM now contains slot 2.
+		 */
+		if (old->agm <= 2)
+			synaptics_mt_state_set(mt_state, 3, 0, 2);
+		/*
+		 * mt_state either hasn't changed, or was updated by a recently
+		 * received AGM-CONTACT packet.
+		 */
+		break;
+	}
+}
+
 static void synaptics_image_sensor_process(struct psmouse *psmouse,
 					   struct synaptics_hw_state *sgm)
 {
-	int count;
+	struct synaptics_data *priv = psmouse->private;
+	struct synaptics_hw_state *agm = &priv->agm;
+	struct synaptics_mt_state mt_state;
 
+	/* Initialize using current mt_state (as updated by last agm) */
+	mt_state = agm->mt_state;
+
+	/*
+	 * Update mt_state using the new finger count and current mt_state.
+	 */
 	if (sgm->z == 0)
-		count = 0;
+		synaptics_image_sensor_0f(priv, &mt_state);
 	else if (sgm->w >= 4)
-		count = 1;
+		synaptics_image_sensor_1f(priv, &mt_state);
 	else if (sgm->w == 0)
-		count = 2;
-	else
-		count = 3;
+		synaptics_image_sensor_2f(priv, &mt_state);
+	else if (sgm->w == 1)
+		synaptics_image_sensor_3f(priv, &mt_state);
 
 	/* Send resulting input events to user space */
-	synaptics_report_mt(psmouse, count, sgm);
+	synaptics_report_mt(psmouse, &mt_state, sgm);
+
+	/* Store updated mt_state */
+	priv->mt_state = agm->mt_state = mt_state;
+	priv->agm_pending = false;
 }
 
 /*
diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
index 20f57df..622aea8 100644
--- a/drivers/input/mouse/synaptics.h
+++ b/drivers/input/mouse/synaptics.h
@@ -161,11 +161,15 @@ struct synaptics_data {
 
 	struct serio *pt_port;			/* Pass-through serio port */
 
+	struct synaptics_mt_state mt_state;	/* Current mt finger state */
+	bool mt_state_lost;			/* mt_state may be incorrect */
+
 	/*
 	 * Last received Advanced Gesture Mode (AGM) packet. An AGM packet
 	 * contains position data for a second contact, at half resolution.
 	 */
 	struct synaptics_hw_state agm;
+	bool agm_pending;			/* new AGM packet received */
 };
 
 void synaptics_module_init(void);
-- 
1.7.3.1

[PATCH 7/8 v3] Input: add BTN_TOOL_QUINTTAP for reporting 5 fingers on touchpad

[Daniel Kurtz]

"4-finger scroll" is a gesture supported by some applications and
operating systems.

"Resting thumb" is when a clickpad user rests a finger (e.g., a
thumb), in a "click zone" (typically the bottom of the touchpad) in
anticipation of click+move=select gestures.

Thus, "4-finger scroll + resting thumb" is a 5-finger gesture.
To allow userspace to detect this gesture, we send BTN_TOOL_QUINTTAP.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
Acked-by: Chase Douglas <chase.douglas@canonical.com>
---
 drivers/input/input-mt.c |    1 +
 include/linux/input.h    |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index c48c81f..9150ee7 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -117,6 +117,7 @@ void input_mt_report_finger_count(struct input_dev *dev, int count)
 	input_event(dev, EV_KEY, BTN_TOOL_DOUBLETAP, count == 2);
 	input_event(dev, EV_KEY, BTN_TOOL_TRIPLETAP, count == 3);
 	input_event(dev, EV_KEY, BTN_TOOL_QUADTAP, count == 4);
+	input_event(dev, EV_KEY, BTN_TOOL_QUINTTAP, count == 5);
 }
 EXPORT_SYMBOL(input_mt_report_finger_count);
 
diff --git a/include/linux/input.h b/include/linux/input.h
index 068784e..4de4b46 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -503,6 +503,7 @@ struct input_keymap_entry {
 #define BTN_TOOL_FINGER		0x145
 #define BTN_TOOL_MOUSE		0x146
 #define BTN_TOOL_LENS		0x147
+#define BTN_TOOL_QUINTTAP	0x148	/* Five fingers on trackpad */
 #define BTN_TOUCH		0x14a
 #define BTN_STYLUS		0x14b
 #define BTN_STYLUS2		0x14c
-- 
1.7.3.1

[PATCH 8/8 v3] Input: synaptics - process finger (<=5) transitions

[Daniel Kurtz]

Synaptics image sensor touchpads track up to 5 fingers, but only report 2.
They use a special "TYPE=2" (AGM-CONTACT) packet type that reports
the number of tracked fingers and which finger is reported in the SGM
and AGM packets.

With this new packet type, it is possible to tell userspace when 4 or 5
fingers are touching.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/input/mouse/synaptics.c |   44 ++++++++++++++++++++++++++++++++++++++-
 1 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 57104ed..6f71cb7 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -755,6 +755,10 @@ static void synaptics_image_sensor_1f(struct synaptics_data *priv,
 		synaptics_mt_state_set(mt_state, 1, -1, -1);
 		priv->mt_state_lost = true;
 		break;
+	case 4:
+	case 5:
+		/* mt_state was updated by AGM-CONTACT packet */
+		break;
 	}
 }
 
@@ -809,6 +813,10 @@ static void synaptics_image_sensor_2f(struct synaptics_data *priv,
 		synaptics_mt_state_set(mt_state, 2, -1, -1);
 		priv->mt_state_lost = true;
 		break;
+	case 4:
+	case 5:
+		/* mt_state was updated by AGM-CONTACT packet */
+		break;
 	}
 }
 
@@ -837,6 +845,22 @@ static void synaptics_image_sensor_3f(struct synaptics_data *priv,
 		break;
 	case 2:
 		/*
+		 * If the AGM previously contained slot 3 or higher, then the
+		 * newly touching finger is in the lowest available slot.
+		 *
+		 * If SGM was previously 1 or higher, then the new SGM is
+		 * now slot 0 (with a new finger), otherwise, the new finger
+		 * is now in a hidden slot between 0 and AGM's slot.
+		 *
+		 * In all such cases, the SGM now contains slot 0, and the AGM
+		 * continues to contain the same slot as before.
+		 */
+		if (old->agm >= 3) {
+			synaptics_mt_state_set(mt_state, 3, 0, old->agm);
+			break;
+		}
+
+		/*
 		 * After some 3->1 and all 3->2 transitions, we lose track
 		 * of which slot is reported by SGM and AGM.
 		 *
@@ -876,9 +900,22 @@ static void synaptics_image_sensor_3f(struct synaptics_data *priv,
 		 * received AGM-CONTACT packet.
 		 */
 		break;
+
+	case 4:
+	case 5:
+		/* mt_state was updated by AGM-CONTACT packet */
+		break;
 	}
 }
 
+/* Handle case where mt_state->count = 4, or = 5 */
+static void synaptics_image_sensor_45f(struct synaptics_data *priv,
+				       struct synaptics_mt_state *mt_state)
+{
+	/* mt_state was updated correctly by AGM-CONTACT packet */
+	priv->mt_state_lost = false;
+}
+
 static void synaptics_image_sensor_process(struct psmouse *psmouse,
 					   struct synaptics_hw_state *sgm)
 {
@@ -898,8 +935,10 @@ static void synaptics_image_sensor_process(struct psmouse *psmouse,
 		synaptics_image_sensor_1f(priv, &mt_state);
 	else if (sgm->w == 0)
 		synaptics_image_sensor_2f(priv, &mt_state);
-	else if (sgm->w == 1)
+	else if (sgm->w == 1 && mt_state.count <= 3)
 		synaptics_image_sensor_3f(priv, &mt_state);
+	else
+		synaptics_image_sensor_45f(priv, &mt_state);
 
 	/* Send resulting input events to user space */
 	synaptics_report_mt(psmouse, &mt_state, sgm);
@@ -1118,6 +1157,9 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
 		/* Image sensors can sometimes report per-contact width */
 		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
+		/* Image sensors can signal 4 and 5 finger clicks */
+		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
+		__set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
 	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
 		/* Non-image sensors with AGM use semi-mt */
 		__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
-- 
1.7.3.1

Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support

[Henrik Rydberg]

Hi Daniel,

looks good, some details below.

> +static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
> +}
> +
> +static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
> +				      const struct synaptics_hw_state *sgm)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> +	input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
> +	input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
> +	input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
> +	/* SGM can sometimes contain valid width */
> +	if (sgm->w >= 4)
> +		input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
> +}

The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
compatible with user space handling of type A devices. Also, the scale
should match the screen/pad size, such that the actual size of the
touch area can be deduced. In addition, based on the current sensor
technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
mutually exclusive.

All in all, I would prefer to only report width via (the single-finger
axis) ABS_TOOL_WIDTH, and only for compatibility reasons.

> +
> +static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
> +				      const struct synaptics_hw_state *agm)
> +{
> +	input_mt_slot(dev, slot);
> +	input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> +	input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> +	input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> +	input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> +}

With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...

> +
> +static void synaptics_report_mt(struct psmouse *psmouse,
> +				int count,
> +				const struct synaptics_hw_state *sgm)
> +{
> +	struct input_dev *dev = psmouse->dev;
> +	struct synaptics_data *priv = psmouse->private;
> +	struct synaptics_hw_state *agm = &priv->agm;
> +
> +	switch (count) {
> +	case 0:
> +		synaptics_report_slot_empty(dev, 0);
> +		synaptics_report_slot_empty(dev, 1);
> +		break;
> +	case 1:
> +		synaptics_report_slot_sgm(dev, 0, sgm);
> +		synaptics_report_slot_empty(dev, 1);
> +		break;
> +	case 2:
> +	case 3: /* Fall-through case */
> +		synaptics_report_slot_sgm(dev, 0, sgm);
> +		synaptics_report_slot_agm(dev, 1, agm);
> +		break;
> +	}
> +
> +	/* Don't use active slot count to generate BTN_TOOL events. */
> +	input_mt_report_pointer_emulation(dev, false);
> +
> +	/* Send the number of fingers reported by touchpad itself. */
> +	input_mt_report_finger_count(dev, count);
> +
> +	input_report_key(dev, BTN_LEFT, sgm->left);
> +	input_sync(dev);
> +}
> +
> +static void synaptics_image_sensor_process(struct psmouse *psmouse,
> +					   struct synaptics_hw_state *sgm)
> +{
> +	int count;
> +
> +	if (sgm->z == 0)
> +		count = 0;
> +	else if (sgm->w >= 4)
> +		count = 1;
> +	else if (sgm->w == 0)
> +		count = 2;
> +	else
> +		count = 3;
> +
> +	/* Send resulting input events to user space */
> +	synaptics_report_mt(psmouse, count, sgm);
> +}
> +
>  /*
>   *  called for each full received packet from the touchpad
>   */
> @@ -558,6 +642,11 @@ static void synaptics_process_packet(struct psmouse *psmouse)
>  	if (synaptics_parse_hw_state(psmouse->packet, priv, &hw))
>  		return;
>  
> +	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> +		synaptics_image_sensor_process(psmouse, &hw);
> +		return;
> +	}
> +
>  	if (hw.scroll) {
>  		priv->scroll += hw.scroll;
>  
> @@ -739,7 +828,16 @@ static void set_input_params(struct input_dev *dev, struct synaptics_data *priv)
>  	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
>  	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
>  
> -	if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
> +	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> +		input_mt_init_slots(dev, 2);
> +		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> +					ABS_MT_POSITION_Y);
> +		/* Image sensors can report per-contact pressure */
> +		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +		/* Image sensors can sometimes report per-contact width */
> +		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> +	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
> +		/* Non-image sensors with AGM use semi-mt */
>  		__set_bit(INPUT_PROP_SEMI_MT, dev->propbit);
>  		input_mt_init_slots(dev, 2);
>  		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> diff --git a/drivers/input/mouse/synaptics.h b/drivers/input/mouse/synaptics.h
> index a9efbf3..0ea7616 100644
> --- a/drivers/input/mouse/synaptics.h
> +++ b/drivers/input/mouse/synaptics.h
> @@ -74,6 +74,8 @@
>   * 2	0x04	reduced filtering	firmware does less filtering on
>   *					position data, driver should watch
>   *					for noise.
> + * 2	0x08	image sensor		image sensor tracks 5 fingers, but only
> + *					reports 2.
>   * 2	0x20	report min		query 0x0f gives min coord reported
>   */
>  #define SYN_CAP_CLICKPAD(ex0c)		((ex0c) & 0x100000) /* 1-button ClickPad */
> @@ -82,6 +84,7 @@
>  #define SYN_CAP_MIN_DIMENSIONS(ex0c)	((ex0c) & 0x002000)
>  #define SYN_CAP_ADV_GESTURE(ex0c)	((ex0c) & 0x080000)
>  #define SYN_CAP_REDUCED_FILTERING(ex0c)	((ex0c) & 0x000400)
> +#define SYN_CAP_IMAGE_SENSOR(ex0c)	((ex0c) & 0x000800)
>  
>  /* synaptics modes query bits */
>  #define SYN_MODE_ABSOLUTE(m)		((m) & (1 << 7))
> -- 
> 1.7.3.1
> 

Looks good otherwise.

Thanks,
Henrik

Re: [PATCH 6/8 v3] Input: synaptics - process finger (<=3) transitions

[Henrik Rydberg]

Hi Daniel,

> Synaptics image sensor touchpads track 5 fingers, but only report 2.
> This patch attempts to deal with some idiosyncrasies of these
> touchpads:

I appreciate the complexity of the protocol at hand, and the patch is
doing an excellent job documenting it. However, it does constitute a
fair deal of code... What about the statements below:

1. When two subsequent sgm packets report the same number of fingers
(and unless told otherwise in some other fashion?), the sgm and the
previous agm contains data for the same fingers as the previous (agm,
sgm) pair, i.e., there is no transition.

2. There is no general way to know which fingers are being reported
after a transition.

If both points above are true, how about something like this:

a) Always report slots zero and one (for two or more fingers)

b) Keep a global number, trackid

c) Parse are buffer agm

d) Parse sgm

e) If a transition occured, increase trackid by two

f) Set TRACKING_ID to trackid in slot zero and trackid+1 in slot one.

It seems to me the above will result in contiuous handling of two
unknown contacts in between transitions, and during transitions, new
contacts will be generated.

The algorithm above could possibly be improved for the 2->1
transition, but from what I gather, 1->2 and 2->3 as well as 3->2 does
not seem to be generally solvable, hence resulting in the above
anyways.

If any of points 1 and 2 are wrong, I am sure you will
enlighten/remind me. ;-)

Cheers,
Henrik

Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support

[Daniel Kurtz]

Hi Henrik,

Thanks for the great feedback.

On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Daniel,
>
> looks good, some details below.
>
>> +static void synaptics_report_slot_empty(struct input_dev *dev, int slot)
>> +{
>> +     input_mt_slot(dev, slot);
>> +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, false);
>> +}
>> +
>> +static void synaptics_report_slot_sgm(struct input_dev *dev, int slot,
>> +                                   const struct synaptics_hw_state *sgm)
>> +{
>> +     input_mt_slot(dev, slot);
>> +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
>> +     input_report_abs(dev, ABS_MT_POSITION_X, sgm->x);
>> +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(sgm->y));
>> +     input_report_abs(dev, ABS_MT_PRESSURE, sgm->z);
>> +     /* SGM can sometimes contain valid width */
>> +     if (sgm->w >= 4)
>> +             input_report_abs(dev, ABS_MT_TOUCH_MAJOR, sgm->w);
>> +}
>
> The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
> compatible with user space handling of type A devices. Also, the scale
> should match the screen/pad size, such that the actual size of the
> touch area can be deduced. In addition, based on the current sensor
> technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
> mutually exclusive.
>
> All in all, I would prefer to only report width via (the single-finger
> axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
>
> +
> +static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
> +                                   const struct synaptics_hw_state *agm)
> +{
> +     input_mt_slot(dev, slot);
> +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> +     input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> +     input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> +}

With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
....
>
> Looks good otherwise.
>
> Thanks,
> Henrik
>

Ahh....  ok, this makes sense.  I missed this detail previously.
Basically:
  * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards compatibility)
  * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really
'width of finger in surface units'), and ABS_PRESSURE (for backwards
compatibility).
  * Don't use ABS_MT_PRESSURE.
  * This will simplify agm/sgm reporting by making them both reoprt
the same axes.

I will rework these patches to do this.

Thanks,
-Dan
--
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 6/8 v3] Input: synaptics - process finger (<=3) transitions

[Daniel Kurtz]

Hi Henrik,

Thanks for the feedback.

On Sat, Aug 13, 2011 at 5:52 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>
> Hi Daniel,
>
> > Synaptics image sensor touchpads track 5 fingers, but only report 2.
> > This patch attempts to deal with some idiosyncrasies of these
> > touchpads:
>
> I appreciate the complexity of the protocol at hand, and the patch is
> doing an excellent job documenting it. However, it does constitute a
> fair deal of code... What about the statements below:
>
> 1. When two subsequent sgm packets report the same number of fingers
> (and unless told otherwise in some other fashion?), the sgm and the
> previous agm contains data for the same fingers as the previous (agm,
> sgm) pair, i.e., there is no transition.
>
> 2. There is no general way to know which fingers are being reported
> after a transition.
>
> If both points above are true, how about something like this:
>
> a) Always report slots zero and one (for two or more fingers)
>
> b) Keep a global number, trackid
>
> c) Parse are buffer agm
>
> d) Parse sgm
>
> e) If a transition occured, increase trackid by two
>
> f) Set TRACKING_ID to trackid in slot zero and trackid+1 in slot one.
>
> It seems to me the above will result in contiuous handling of two
> unknown contacts in between transitions, and during transitions, new
> contacts will be generated.
>
> The algorithm above could possibly be improved for the 2->1
> transition, but from what I gather, 1->2 and 2->3 as well as 3->2 does
> not seem to be generally solvable, hence resulting in the above
> anyways.
>
> If any of points 1 and 2 are wrong, I am sure you will
> enlighten/remind me. ;-)
>
> Cheers,
> Henrik

Actually, "in general", it is possible to know which fingers are being
reported _after_ a transition.

However:
  (1) Since the number of fingers is only reported in SGM packets, it
is not possible, _during_ a transition, to know whether the AGM was
sent for the old number of fingers, or the new number of fingers.
  (2) There is no way to tell which fingers remain when going from
3->2 fingers. Once we lose track, we can't really know what fingers we
are tracking until:
     (a) All fingers are removed
     (b) All but the 'primary' (finger slot[0]) are removed
     (c) 4 or 5 fingers are touched (this triggers an AGM_CONTACT
packet, which properly resets the driver's state)
  (3) Similarly, on 3->1 transitions, if the 'primary' (finger
slot[0]) finger is NOT the one that remains, we don't know which
finger remains.  Tracking state must be restored with (a), (b), or (c)
above.
  (4) A quick finger change, can generate a 1->1 (non-)transition with
an intervening AGM packet. In this case, the new SGM is actually
tracking slot[1], not slot[0].  If a second finger touches, this new
second finger will fill the now-empty slot[0], and, thus, be reported
in the SGM.

This results in a lot of the special handling in the switch/cases.

The resulting algorithm can accurately handle 1->2->1 transitions very
well (including drum roll, and no matter which of the two fingers is
being reported in the second '1' state).
It can deal with 1->2->3 properly in the typical "adding fingers" case.
Lastly, it also realizes when it has lost track (after 3->2 / 3->1
transition), and handles subsequent (1->2, 2->1, 2->3) transitions
carefully (by invalidating slots).

So, I think even if we started with a simpler "always use two new
slots" approach, once we added back the special casing, it would be
just as much code...

A simple example:
  On 2->3, even if we always used two new slots, we still couldn't
report them accurately, immediately, because we don't know whether the
AGM during the transition was for the "2" state, or the "3" state.
Instead, we'd need to do something like what I'm doing now: report
3:[0,-1] "three fingers down, the only one I can report is that the
one in slot[0]"

-Dan

Re: [PATCH 0/8 v3] Synaptics image sensor support

[Chase Douglas]

On 08/12/2011 10:16 AM, Daniel Kurtz wrote:
> Hello,
> 
> ----
> For v3:
>   Merged Patch 5 & 6:
>      BTN_TOOL_*TAP always reports correct number of fingers.
>      During certain ambiguous number-of-finger-transitions, there may be less
>      than 2 valid MT-B slots, even if the BTN_TOOL_*TAP value indicates two or
>      more fingers are present on the pad.

I am happy with the patch series. I'm not sure if anyone will actually
use the MT slots because of all the caveats :), but this driver appears
to do the best thing I can think of with the data from the device. It
looks like it was a lot of work to figure it out, so cheers to Daniel!

At this time I see Henrik has provided two replies and Daniel has
followed up. The first reply set will cause a small amount of axis
handling changes, and the second reply about the amount of code changes
and implementation details of the tracking ids has been rebuffed. If
this is the end result, you can add my Acked-by to all the patches:

Acked-by: Chase Douglas <chase.douglas@canonical.com>

Thanks again Daniel!

-- Chase

Re: [PATCH 0/8 v3] Synaptics image sensor support

[Chase Douglas]

On 08/16/2011 10:41 AM, Chase Douglas wrote:
> On 08/12/2011 10:16 AM, Daniel Kurtz wrote:
>> Hello,
>>
>> ----
>> For v3:
>>   Merged Patch 5 & 6:
>>      BTN_TOOL_*TAP always reports correct number of fingers.
>>      During certain ambiguous number-of-finger-transitions, there may be less
>>      than 2 valid MT-B slots, even if the BTN_TOOL_*TAP value indicates two or
>>      more fingers are present on the pad.
> 
> I am happy with the patch series. I'm not sure if anyone will actually
> use the MT slots because of all the caveats :), but this driver appears
> to do the best thing I can think of with the data from the device. It
> looks like it was a lot of work to figure it out, so cheers to Daniel!
> 
> At this time I see Henrik has provided two replies and Daniel has
> followed up. The first reply set will cause a small amount of axis
> handling changes, and the second reply about the amount of code changes
> and implementation details of the tracking ids has been rebuffed. If
> this is the end result, you can add my Acked-by to all the patches:
> 
> Acked-by: Chase Douglas <chase.douglas@canonical.com>

I forgot about the change in protocol aspect of this. We still need
documentation stating that when BTN_TOOL_*TAP > max mt slots, the
BTN_TOOL_*TAP events should be used to get the current number of active
touches instead of the number of open MT slots. Would you be able to
send a separate patch for that?

(I'm trying to coerce you into updating the docs partly because you're
touching this area and partly because I'm a bit weary of writing input
protocol docs myself :).

Thanks!

-- Chase

Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support

[Daniel Kurtz]

On Mon, Aug 15, 2011 at 3:17 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>
> Hi Henrik,
>
> Thanks for the great feedback.
>
> On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > Hi Daniel,
> >
> > looks good, some details below.
> >
> > The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
> > compatible with user space handling of type A devices. Also, the scale
> > should match the screen/pad size, such that the actual size of the
> > touch area can be deduced. In addition, based on the current sensor
> > technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
> > mutually exclusive.
> >
> > All in all, I would prefer to only report width via (the single-finger
> > axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
> >
> > +
> > +static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
> > +                                   const struct synaptics_hw_state *agm)
> > +{
> > +     input_mt_slot(dev, slot);
> > +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> > +     input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> > +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> > +     input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> > +}
>
> With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
> ....
> >
> > Looks good otherwise.
> >
> > Thanks,
> > Henrik
> >
>
> Ahh....  ok, this makes sense.  I missed this detail previously.
> Basically:
>  * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards compatibility)
>  * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really
> 'width of finger in surface units'), and ABS_PRESSURE (for backwards
> compatibility).
>  * Don't use ABS_MT_PRESSURE.
>  * This will simplify agm/sgm reporting by making them both reoprt
> the same axes.

Hi Henrik,

I think I was wrong in my previous comment.

I agree we should drop reporting synaptics 'w' (4-15, arbitrary units)
in ABS_MT_TOUCH_MAJOR.
Instead, to support legacy, report 'w' in ABS_TOOL_WIDTH, and do not
report ABS_MT_TOUCH_MAJOR at all.

However, I believe we should still report synaptics 'z' (0-255, in
arbitrary units) as ABS_MT_PRESSURE.
This will, in turn, be used by input_mt_report_pointer_emulation() to
determine the correct ABS_PRESSURE to report for legacy userspace
uses.

Do you agree with this?

Thanks,
-Dan

Re: [PATCH 4/8 v3] Input: synaptics - add image sensor support

[Dmitry Torokhov]

On Wed, Aug 17, 2011 at 10:05:48PM +0800, Daniel Kurtz wrote:
> On Mon, Aug 15, 2011 at 3:17 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> >
> > Hi Henrik,
> >
> > Thanks for the great feedback.
> >
> > On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > > Hi Daniel,
> > >
> > > looks good, some details below.
> > >
> > > The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
> > > compatible with user space handling of type A devices. Also, the scale
> > > should match the screen/pad size, such that the actual size of the
> > > touch area can be deduced. In addition, based on the current sensor
> > > technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
> > > mutually exclusive.
> > >
> > > All in all, I would prefer to only report width via (the single-finger
> > > axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
> > >
> > > +
> > > +static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
> > > +                                   const struct synaptics_hw_state *agm)
> > > +{
> > > +     input_mt_slot(dev, slot);
> > > +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> > > +     input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> > > +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> > > +     input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> > > +}
> >
> > With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
> > ....
> > >
> > > Looks good otherwise.
> > >
> > > Thanks,
> > > Henrik
> > >
> >
> > Ahh....  ok, this makes sense.  I missed this detail previously.
> > Basically:
> >  * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards compatibility)
> >  * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really
> > 'width of finger in surface units'), and ABS_PRESSURE (for backwards
> > compatibility).
> >  * Don't use ABS_MT_PRESSURE.
> >  * This will simplify agm/sgm reporting by making them both reoprt
> > the same axes.
> 
> Hi Henrik,
> 
> I think I was wrong in my previous comment.
> 
> I agree we should drop reporting synaptics 'w' (4-15, arbitrary units)
> in ABS_MT_TOUCH_MAJOR.
> Instead, to support legacy, report 'w' in ABS_TOOL_WIDTH, and do not
> report ABS_MT_TOUCH_MAJOR at all.
> 
> However, I believe we should still report synaptics 'z' (0-255, in
> arbitrary units) as ABS_MT_PRESSURE.
> This will, in turn, be used by input_mt_report_pointer_emulation() to
> determine the correct ABS_PRESSURE to report for legacy userspace
> uses.

Alternatively, you can use input_mt_set_value() to save Z value to be
used by input_mt_report_pointer_emulation() later, but not report it as
part of MT packet.

-- 
Dmitry
--
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 4/8 v3] Input: synaptics - add image sensor support

[Daniel Kurtz]

On Thu, Aug 18, 2011 at 12:32 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Wed, Aug 17, 2011 at 10:05:48PM +0800, Daniel Kurtz wrote:
> > On Mon, Aug 15, 2011 at 3:17 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> > >
> > > Hi Henrik,
> > >
> > > Thanks for the great feedback.
> > >
> > > On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > > > Hi Daniel,
> > > >
> > > > looks good, some details below.
> > > >
> > > > The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
> > > > compatible with user space handling of type A devices. Also, the scale
> > > > should match the screen/pad size, such that the actual size of the
> > > > touch area can be deduced. In addition, based on the current sensor
> > > > technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
> > > > mutually exclusive.
> > > >
> > > > All in all, I would prefer to only report width via (the single-finger
> > > > axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
> > > >
> > > > +
> > > > +static void synaptics_report_slot_agm(struct input_dev *dev, int slot,
> > > > +                                   const struct synaptics_hw_state *agm)
> > > > +{
> > > > +     input_mt_slot(dev, slot);
> > > > +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> > > > +     input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> > > > +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> > > > +     input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> > > > +}
> > >
> > > With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
> > > ....
> > > >
> > > > Looks good otherwise.
> > > >
> > > > Thanks,
> > > > Henrik
> > > >
> > >
> > > Ahh....  ok, this makes sense.  I missed this detail previously.
> > > Basically:
> > >  * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards compatibility)
> > >  * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really
> > > 'width of finger in surface units'), and ABS_PRESSURE (for backwards
> > > compatibility).
> > >  * Don't use ABS_MT_PRESSURE.
> > >  * This will simplify agm/sgm reporting by making them both reoprt
> > > the same axes.
> >
> > Hi Henrik,
> >
> > I think I was wrong in my previous comment.
> >
> > I agree we should drop reporting synaptics 'w' (4-15, arbitrary units)
> > in ABS_MT_TOUCH_MAJOR.
> > Instead, to support legacy, report 'w' in ABS_TOOL_WIDTH, and do not
> > report ABS_MT_TOUCH_MAJOR at all.
> >
> > However, I believe we should still report synaptics 'z' (0-255, in
> > arbitrary units) as ABS_MT_PRESSURE.
> > This will, in turn, be used by input_mt_report_pointer_emulation() to
> > determine the correct ABS_PRESSURE to report for legacy userspace
> > uses.
>
> Alternatively, you can use input_mt_set_value() to save Z value to be
> used by input_mt_report_pointer_emulation() later, but not report it as
> part of MT packet.
>
> --
> Dmitry

But I want userspace to get per-contact pressure, since it is used to
determine whether a given contact is a palm or finger, etc. The
trackpad does not report a useful per-contact "width" that can be used
to generate "TOUCH_MAJOR".
Does it therefore sound reasonable to report ABS_MT_PRESSURE?

-Dan
--
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 4/8 v3] Input: synaptics - add image sensor support

[Dmitry Torokhov]

On Thu, Aug 18, 2011 at 12:43:39AM +0800, Daniel Kurtz wrote:
> On Thu, Aug 18, 2011 at 12:32 AM, Dmitry Torokhov <dmitry.torokhov@gmail.com
> > wrote:
> 
> > On Wed, Aug 17, 2011 at 10:05:48PM +0800, Daniel Kurtz wrote:
> > > On Mon, Aug 15, 2011 at 3:17 PM, Daniel Kurtz <djkurtz@chromium.org>
> > wrote:
> > > >
> > > > Hi Henrik,
> > > >
> > > > Thanks for the great feedback.
> > > >
> > > > On Sat, Aug 13, 2011 at 5:09 AM, Henrik Rydberg <rydberg@euromail.se>
> > wrote:
> > > > > Hi Daniel,
> > > > >
> > > > > looks good, some details below.
> > > > >
> > > > > The ABS_MT_TOUCH_MAJOR is supposed to have zero intercept, to remain
> > > > > compatible with user space handling of type A devices. Also, the
> > scale
> > > > > should match the screen/pad size, such that the actual size of the
> > > > > touch area can be deduced. In addition, based on the current sensor
> > > > > technologies, ABS_MT_PRESSURE and ABS_MT_TOUCH_MAJOR are normally
> > > > > mutually exclusive.
> > > > >
> > > > > All in all, I would prefer to only report width via (the
> > single-finger
> > > > > axis) ABS_TOOL_WIDTH, and only for compatibility reasons.
> > > > >
> > > > > +
> > > > > +static void synaptics_report_slot_agm(struct input_dev *dev, int
> > slot,
> > > > > +                                   const struct synaptics_hw_state
> > *agm)
> > > > > +{
> > > > > +     input_mt_slot(dev, slot);
> > > > > +     input_mt_report_slot_state(dev, MT_TOOL_FINGER, true);
> > > > > +     input_report_abs(dev, ABS_MT_POSITION_X, agm->x);
> > > > > +     input_report_abs(dev, ABS_MT_POSITION_Y, invert_y(agm->y));
> > > > > +     input_report_abs(dev, ABS_MT_PRESSURE, agm->z);
> > > > > +}
> > > >
> > > > With ABS_MT_TOUCH_MAJOR dropped, sgm and agm seems to coincide...
> > > > ....
> > > > >
> > > > > Looks good otherwise.
> > > > >
> > > > > Thanks,
> > > > > Henrik
> > > > >
> > > >
> > > > Ahh....  ok, this makes sense.  I missed this detail previously.
> > > > Basically:
> > > >  * Only report synaptics "w" as ABS_TOOL_WIDTH (for backwards
> > compatibility)
> > > >  * Report synaptics "z" as ABS_MT_TOUCH_MAJOR (assuming it is really
> > > > 'width of finger in surface units'), and ABS_PRESSURE (for backwards
> > > > compatibility).
> > > >  * Don't use ABS_MT_PRESSURE.
> > > >  * This will simplify agm/sgm reporting by making them both reoprt
> > > > the same axes.
> > >
> > > Hi Henrik,
> > >
> > > I think I was wrong in my previous comment.
> > >
> > > I agree we should drop reporting synaptics 'w' (4-15, arbitrary units)
> > > in ABS_MT_TOUCH_MAJOR.
> > > Instead, to support legacy, report 'w' in ABS_TOOL_WIDTH, and do not
> > > report ABS_MT_TOUCH_MAJOR at all.
> > >
> > > However, I believe we should still report synaptics 'z' (0-255, in
> > > arbitrary units) as ABS_MT_PRESSURE.
> > > This will, in turn, be used by input_mt_report_pointer_emulation() to
> > > determine the correct ABS_PRESSURE to report for legacy userspace
> > > uses.
> >
> > Alternatively, you can use input_mt_set_value() to save Z value to be
> > used by input_mt_report_pointer_emulation() later, but not report it as
> > part of MT packet.
> >
> 
> But I want userspace to get per-contact pressure, since it is used to
> determine whether a given contact is a palm or finger, etc. The trackpad
> does not report a useful per-contact "width" that can be used to generate
> "TOUCH_MAJOR".

If the data reported is indeed pressure and it is indeed per-contact then yes,
it makes sense to report it as such. If the data reported is more like contact
size then report it as ABS_MT_TOUCH_MAJOR and use input_mt_set_value()
to "stash" it for legacy emulation.

-- 
Dmitry