[PATCH 0/7] elantech: various improvements for 6-byte protocol

[PATCH 0/7] elantech: various improvements for 6-byte protocol

[Éric Piel]

Hello,

Following the better understanding of the 6-byte protocol for the
elantech touchpad by reading the Dell/Ubuntu driver, here comes a series
of patches. It should bring support for more or less all the features
supported by the hardware.

All the patches have been tested with my elantech touchpad reporting
firmware version 2.8.1, and with nothing else. Hopefully, more people
can test and confirm it works fine with them (any other firmware version
is useful to test, including hardware vesrion 1).

The first two patches are unmodified from the one sent 3 weeks ago. I
haven't added yet the MT support with the MT protocol, but it should be
very easy, and will try to do it soon.

It's against 2.6.35-rc3. Dmitry, would it be possible to have them as
2.6.36 merge window material? Hopefully we can catch all the potential
regressions during 2.6.36 development.

Cheers,
Eric

[PATCH 1/7] elantech: Describe further the protocol

[Éric Piel]

For some Dell laptops, Ubuntu had a special version of the elantech
driver with more knowledge on the devices. It can be found there:
http://zinc.ubuntu.com/git?p=mid-team/hardy-netbook.git;a=blob;f=drivers/input/mouse/elantech.c;h=d0e2cafed162428f72e3654f4dda85e08ea486b3;hb=refs/heads/abi-22

By inspecting the source code, and doing some test on a real hardware, I
have completed the protocol specification (especially for the 6 bytes
protocol). It also adds information about the mapping between the
version reported by the device and the protocol to use.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 Documentation/input/elantech.txt |  122 ++++++++++++++++++++++++++++++-------
 1 files changed, 99 insertions(+), 23 deletions(-)

diff --git a/Documentation/input/elantech.txt b/Documentation/input/elantech.txt
index 56941ae..242f69c 100644
--- a/Documentation/input/elantech.txt
+++ b/Documentation/input/elantech.txt
@@ -34,7 +34,8 @@ Contents
 Currently the Linux Elantech touchpad driver is aware of two different
 hardware versions unimaginatively called version 1 and version 2. Version 1
 is found in "older" laptops and uses 4 bytes per packet. Version 2 seems to
-be introduced with the EeePC and uses 6 bytes per packet.
+be introduced with the EeePC and uses 6 bytes per packet, and provides
+additional features such as position of two fingers, and width of the touch.
 
 The driver tries to support both hardware versions and should be compatible
 with the Xorg Synaptics touchpad driver and its graphical configuration
@@ -94,18 +95,44 @@ Currently the Linux Elantech touchpad driver provides two extra knobs under
    can check these bits and reject any packet that appears corrupted. Using
    this knob you can bypass that check.
 
-   It is not known yet whether hardware version 2 provides the same parity
-   bits. Hence checking is disabled by default. Currently even turning it on
-   will do nothing.
-
+   Hardware version 2 does not provide the same parity bits. Only some basic
+   data consistency checking can be done. For now checking is disabled by
+   default. Currently even turning it on will do nothing.
 
 /////////////////////////////////////////////////////////////////////////////
 
+3. Differentiating hardware versions
+   =================================
+   
+To detect the hardware version, read the version number as param[0].param[1].param[2]
+
+ 4 bytes version: (after the arrow is the name given in the Dell-provided driver)
+ 02.00.22 => EF013 
+ 02.06.00 => EF019
+In the wild, there appear to be more versions, such as 00.01.64, 01.00.21,
+02.00.00, 02.00.04, 02.00.06. 
+
+ 6 bytes:
+ 02.00.30 => EF113
+ 02.08.00 => EF023
+ 02.08.XX => EF123 
+ 02.0B.00 => EF215
+ 04.01.XX => Scroll_EF051
+ 04.02.XX => EF051 
+In the wild, there appear to be more versions, such as 04.03.01, 04.04.11. There
+appear to be almost no difference excepted the EF113 which do not report
+pressure/width and has different data consistency checking.
+
+Probably all the versions with param[0] <= 01 can be considered as 
+4 bytes/firmware 1. The versions < 02.08.00, with the exception of 02.00.30, as 
+4 bytes/firmware 2. Everything >= 02.08.00 can be considered as 6 bytes.
+
+/////////////////////////////////////////////////////////////////////////////
 
-3. Hardware version 1
+4. Hardware version 1
    ==================
 
-3.1 Registers
+4.1 Registers
     ~~~~~~~~~
 
 By echoing a hexadecimal value to a register it contents can be altered.
@@ -168,7 +195,7 @@ For example:
          smart edge activation area width?
 
 
-3.2 Native relative mode 4 byte packet format
+4.2 Native relative mode 4 byte packet format
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 byte 0:
@@ -226,9 +253,13 @@ byte 3:
                        positive = down
 
 
-3.3 Native absolute mode 4 byte packet format
+4.3 Native absolute mode 4 byte packet format
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+EF013 and EF019 have a special behaviour (due to a bug in the firmware?), and
+when 1 finger is touching, the first 2 position reports must be discarded.
+This counting is reset whenever a different number of fingers is reported.
+
 byte 0:
    firmware version 1.x:
 
@@ -279,11 +310,11 @@ byte 3:
 /////////////////////////////////////////////////////////////////////////////
 
 
-4. Hardware version 2
+5. Hardware version 2
    ==================
 
 
-4.1 Registers
+5.1 Registers
     ~~~~~~~~~
 
 By echoing a hexadecimal value to a register it contents can be altered.
@@ -316,16 +347,41 @@ For example:
                                    0x7f = never i.e. tap again to release)
 
 
-4.2 Native absolute mode 6 byte packet format
+5.2 Native absolute mode 6 byte packet format
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-4.2.1 One finger touch
+5.2.1 Parity checking and packet re-synchronization
+There is no parity checking, however some consistency checks can be performed.
+
+For instance for EF113:
+        SA1= packet[0];
+        A1 = packet[1];
+        B1 = packet[2];
+        SB1= packet[3];
+        C1 = packet[4];
+        D1 = packet[5];
+        if( (((SA1 & 0x3C) != 0x3C) && ((SA1 & 0xC0) != 0x80)) || // check Byte 1
+            (((SA1 & 0x0C) != 0x0C) && ((SA1 & 0xC0) == 0x80)) || // check Byte 1 (one finger pressed)
+            (((SA1 & 0xC0) != 0x80) && (( A1 & 0xF0) != 0x00)) || // check Byte 2
+            (((SB1 & 0x3E) != 0x38) && ((SA1 & 0xC0) != 0x80)) || // check Byte 4
+            (((SB1 & 0x0E) != 0x08) && ((SA1 & 0xC0) == 0x80)) || // check Byte 4 (one finger pressed)
+            (((SA1 & 0xC0) != 0x80) && (( C1 & 0xF0) != 0x00))  ) // check Byte 5
+		// error detected
+
+For all the other ones, there are just a few constant bits:
+        if( ((packet[0] & 0x0C) != 0x04) ||
+            ((packet[3] & 0x0f) != 0x02) )
+		// error detected
+
+
+In case an error is detected, all the packets are shifted by one (and packet[0] is discarded).
+
+5.2.1 One/Three finger touch
       ~~~~~~~~~~~~~~~~
 
 byte 0:
 
    bit   7   6   5   4   3   2   1   0
-        n1  n0   .   .   .   .   R   L
+        n1  n0  w3  w2   .   .   R   L
 
          L, R = 1 when Left, Right mouse button pressed
          n1..n0 = numbers of fingers on touchpad
@@ -333,24 +389,39 @@ byte 0:
 byte 1:
 
    bit   7   6   5   4   3   2   1   0
-         .   .   .   .   .  x10 x9  x8
+         p7  p6  p5  p4  .  x10 x9  x8
 
 byte 2:
 
    bit   7   6   5   4   3   2   1   0
-        x7  x6  x5  x4  x4  x2  x1  x0
+        x7  x6  x5  x4  x3  x2  x1  x0
 
          x10..x0 = absolute x value (horizontal)
 
 byte 3:
 
    bit   7   6   5   4   3   2   1   0
-         .   .   .   .   .   .   .   .
+        n4  vf  w1  w0   .   .   .  b2
+
+         n4 = set if more than 3 fingers (only in 3 fingers mode)
+	 vf = a kind of flag ? (only on EF123, 0 when finger is over one of the buttons, 1 otherwise)
+	 w3..w0 = width of the finger touch (not EF113)
+	 b2 (on EF113 only, 0 otherwise), b2.R.L indicates one button pressed:
+		0 = none
+		1 = Left
+		2 = Right
+		3 = Middle (Left and Right)
+		4 = Forward
+		5 = Back
+		6 = Another one
+		7 = Another one
 
 byte 4:
 
    bit   7   6   5   4   3   2   1   0
-         .   .   .   .   .   .  y9  y8
+        p3  p1  p2  p0   .   .  y9  y8
+
+	 p7..p0 = pressure (not EF113)
 
 byte 5:
 
@@ -363,6 +434,11 @@ byte 5:
 4.2.2 Two finger touch
       ~~~~~~~~~~~~~~~~
 
+Note that the two pairs of coordinates are not exactly the coordinates of the
+two fingers, but only the pair of the lower-left and upper-right coordinates.
+So the actual fingers might be situated on the other diagonal of the square
+defined by these two points.
+
 byte 0:
 
    bit   7   6   5   4   3   2   1   0
@@ -376,14 +452,14 @@ byte 1:
    bit   7   6   5   4   3   2   1   0
         ax7 ax6 ax5 ax4 ax3 ax2 ax1 ax0
 
-         ax8..ax0 = first finger absolute x value
+         ax8..ax0 = lower-left finger absolute x value
 
 byte 2:
 
    bit   7   6   5   4   3   2   1   0
         ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0
 
-         ay8..ay0 = first finger absolute y value
+         ay8..ay0 = lower-left finger absolute y value
 
 byte 3:
 
@@ -395,11 +471,11 @@ byte 4:
    bit   7   6   5   4   3   2   1   0
         bx7 bx6 bx5 bx4 bx3 bx2 bx1 bx0
 
-         bx8..bx0 = second finger absolute x value
+         bx8..bx0 = upper-right finger absolute x value
 
 byte 5:
 
    bit   7   6   5   4   3   2   1   0
         by7 by8 by5 by4 by3 by2 by1 by0
 
-         by8..by0 = second finger absolute y value
+         by8..by0 = upper-right finger absolute y value
-- 
1.7.1


--
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 2/7] [NEEDS TEST] elantech: discard the first 2 positions reports for some firmwares

[Éric Piel]

According to the Dell/Ubuntu driver, what was previously observed as "jumpy cursor"
corresponds to the hardware sending incorrect data for the first two reports of a 
one touch finger. So let's use the same workaround as in the other driver. Also,
detect another firmware version with the same behaviour, as in the other driver.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   23 +++++++++++------------
 1 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index b18862b..1c1d065 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -185,7 +185,7 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	struct elantech_data *etd = psmouse->private;
 	unsigned char *packet = psmouse->packet;
 	int fingers;
-	static int old_fingers;
+	static int one_finger_reports;
 
 	if (etd->fw_version < 0x020000) {
 		/*
@@ -203,11 +203,13 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	}
 
 	if (etd->jumpy_cursor) {
-		/* Discard packets that are likely to have bogus coordinates */
-		if (fingers > old_fingers) {
+		if ((fingers == 1) && (one_finger_reports < 2)) {
+			/* Discard first 2 reports of one finger, bogus */
+			one_finger_reports++;
 			elantech_debug("discarding packet\n");
-			goto discard_packet_v1;
-		}
+			return;
+		} else if (fingers != 1)
+			one_finger_reports = 0;
 	}
 
 	input_report_key(dev, BTN_TOUCH, fingers != 0);
@@ -238,9 +240,6 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	}
 
 	input_sync(dev);
-
- discard_packet_v1:
-	old_fingers = fingers;
 }
 
 /*
@@ -733,12 +732,12 @@ int elantech_init(struct psmouse *psmouse)
 	etd->capabilities = param[0];
 
 	/*
-	 * This firmware seems to suffer from misreporting coordinates when
+	 * This firmware suffers from misreporting coordinates when
 	 * a touch action starts causing the mouse cursor or scrolled page
 	 * to jump. Enable a workaround.
 	 */
-	if (etd->fw_version == 0x020022) {
-		pr_info("firmware version 2.0.34 detected, enabling jumpy cursor workaround\n");
+	if ((etd->fw_version == 0x020022) || (etd->fw_version == 0x020600)) {
+		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
 		etd->jumpy_cursor = 1;
 	}
 
-- 
1.7.1


--
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/7] elantech: distinguish various hardware/firmware versions

[Éric Piel]

According to the protocol document, there are a couple of different
versions of the hardware and firmware. Using the version number, it
should be possible to distinguish between them, at least for the
properties we care about.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   34 +++++++++++++++++++++-------------
 drivers/input/mouse/elantech.h |   10 +++++++++-
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 1c1d065..f32ffda 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -709,15 +709,32 @@ int elantech_init(struct psmouse *psmouse)
 	 * Assume every version greater than this is new EeePC style
 	 * hardware with 6 byte packets
 	 */
-	if (etd->fw_version >= 0x020030) {
+	if (etd->fw_version >= 0x020800) {
 		etd->hw_version = 2;
 		/* For now show extra debug information */
 		etd->debug = 1;
-		/* Parity check is only for 4 byte protocol */
-		etd->paritycheck = 0;
+		etd->paritycheck = ETP_CONST_CHECK;
+		etd->reports_pres = 1;
+	} else if (etd->fw_version == 0x020030) {
+		etd->hw_version = 2;
+		/* For now show extra debug information */
+		etd->debug = 1;
+		etd->paritycheck = ETP_EF113_CHECK;
+		etd->reports_pres = 0;
+	} else if ((etd->fw_version == 0x020022) || (etd->fw_version == 0x020600)) {
+		/*
+		 * This firmware suffers from misreporting coordinates when
+		 * a touch action starts causing the mouse cursor or scrolled page
+		 * to jump. Enable a workaround.
+		 */
+		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
+		etd->jumpy_cursor = 1;
+		etd->debug = 1;
+		etd->hw_version = 1;
+		etd->paritycheck = ETP_FULL_PC;
 	} else {
 		etd->hw_version = 1;
-		etd->paritycheck = 1;
+		etd->paritycheck = ETP_FULL_PC;
 	}
 
 	pr_info("assuming hardware version %d, firmware version %d.%d.%d\n",
@@ -731,15 +748,6 @@ int elantech_init(struct psmouse *psmouse)
 		param[0], param[1], param[2]);
 	etd->capabilities = param[0];
 
-	/*
-	 * This firmware suffers from misreporting coordinates when
-	 * a touch action starts causing the mouse cursor or scrolled page
-	 * to jump. Enable a workaround.
-	 */
-	if ((etd->fw_version == 0x020022) || (etd->fw_version == 0x020600)) {
-		pr_info("firmware version 2.0.34/2.6.0 detected, enabling jumpy cursor workaround\n");
-		etd->jumpy_cursor = 1;
-	}
 
 	if (elantech_set_absolute_mode(psmouse)) {
 		pr_err("failed to put touchpad into absolute mode.\n");
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index ac57bde..b98c3c2 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -101,12 +101,20 @@ struct elantech_data {
 	unsigned char debug;
 	unsigned char capabilities;
 	unsigned char paritycheck;
-	unsigned char jumpy_cursor;
+	unsigned char jumpy_cursor :1;
+	unsigned char reports_pres :1;
 	unsigned char hw_version;
 	unsigned int  fw_version;
 	unsigned char parity[256];
 };
 
+enum paritycheck_types {
+	ETP_NOT_CHECK = 0,
+	ETP_FULL_PC,	 /* used in 4-byte protocol */
+	ETP_EF113_CHECK, /* check used only on the EF113 */
+	ETP_CONST_CHECK, /* used in 6-byte protocol, only checking constant bits */
+};
+
 #ifdef CONFIG_MOUSE_PS2_ELANTECH
 int elantech_detect(struct psmouse *psmouse, bool set_properties);
 int elantech_init(struct psmouse *psmouse);
-- 
1.7.1


--
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 4/7] elantech: implement data check for 6-byte protocol

[Éric Piel]

Based on the protocol description, and the source code of the
Dell/Ubuntu driver, the two additional types of data checking for the
6-byte protocol are added. Technically, this is not parity checking, but
to avoid too much change, "paritycheck" is kept to talk about the checks.

In addition, as in the Dell/Ubuntu driver, instead of droping all the
bytes when an error is detected, just drop the first byte, so that
it's possible to re-synchronize quickly.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   61 +++++++++++++++++++++++++++++++++++++--
 1 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index f32ffda..b09b458 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -339,6 +339,57 @@ static int elantech_check_parity_v1(struct psmouse *psmouse)
 	       etd->parity[packet[3]] == p3;
 }
 
+static int elantech_check_parity_ef113(struct psmouse *psmouse)
+{
+	unsigned char *packet = psmouse->packet;
+
+	/* Consistency checks as in the Dell/Ubuntu driver */
+	return !((((packet[0] & 0x3c) != 0x3c) && ((packet[0] & 0xc0) != 0x80)) ||
+		 (((packet[0] & 0x0c) != 0x0c) && ((packet[0] & 0xc0) == 0x80)) ||
+		 (((packet[0] & 0xc0) != 0x80) && ((packet[1] & 0xf0) != 0x00)) ||
+		 (((packet[3] & 0x3e) != 0x38) && ((packet[0] & 0xc0) != 0x80)) ||
+		 (((packet[3] & 0x0e) != 0x08) && ((packet[0] & 0xc0) == 0x80)) ||
+		 (((packet[0] & 0xc0) != 0x80) && ((packet[4] & 0xf0) != 0x00)));
+}
+
+static int elantech_check_parity_const(struct psmouse *psmouse)
+{
+	unsigned char *packet = psmouse->packet;
+
+	/* Some bits which are constant in the 6 packets */
+	return (((packet[0] & 0x0c) == 0x04) && ((packet[3] & 0x0f) == 0x02));
+}
+
+/*
+ * Returns 0 if an error is detected in the packets.
+ */
+static int elantech_check_parity(struct psmouse *psmouse)
+{
+	struct elantech_data *etd = psmouse->private;
+
+	switch (etd->paritycheck) {
+	case ETP_FULL_PC:
+		return elantech_check_parity_v1(psmouse);
+	case ETP_EF113_CHECK:
+		return elantech_check_parity_ef113(psmouse);
+	case ETP_CONST_CHECK:
+		return elantech_check_parity_const(psmouse);
+	case ETP_NOT_CHECK:
+	default:
+		return 1;
+	}
+}
+
+static void elantech_shift_packets(struct psmouse *psmouse)
+{
+	unsigned char *packet = psmouse->packet;
+	int i;
+
+	for (i = 0; i < (psmouse->pktsize - 1); i++)
+		packet[i] = packet[i + 1];
+	psmouse->pktcnt--;
+}
+
 /*
  * Process byte stream from mouse and handle complete packets
  */
@@ -352,16 +403,18 @@ static psmouse_ret_t elantech_process_byte(struct psmouse *psmouse)
 	if (etd->debug > 1)
 		elantech_packet_dump(psmouse->packet, psmouse->pktsize);
 
+	if (!elantech_check_parity(psmouse)) {
+		/* Try to re-synchronize */
+		elantech_shift_packets(psmouse);
+		return PSMOUSE_GOOD_DATA;
+	}
+
 	switch (etd->hw_version) {
 	case 1:
-		if (etd->paritycheck && !elantech_check_parity_v1(psmouse))
-			return PSMOUSE_BAD_DATA;
-
 		elantech_report_absolute_v1(psmouse);
 		break;
 
 	case 2:
-		/* We don't know how to check parity in protocol v2 */
 		elantech_report_absolute_v2(psmouse);
 		break;
 	}
-- 
1.7.1


--
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 6/7] elantech: export pressure and width when supported

[Éric Piel]

Using the info of the Dell/Ubuntu driver, described in the protocol
document, report both width and pressure when pressing 1 and 3
fingers, for the versions of the touchpad which support it.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   19 ++++++++++++++++++-
 drivers/input/mouse/elantech.h |    5 +++++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 633f100..c84f741 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -248,9 +248,10 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
  */
 static void elantech_report_absolute_v2(struct psmouse *psmouse)
 {
+	struct elantech_data *etd = psmouse->private;
 	struct input_dev *dev = psmouse->dev;
 	unsigned char *packet = psmouse->packet;
-	int fingers, x1, y1, x2, y2;
+	int fingers, x1, y1, x2, y2, width = 0, pres = 0;
 
 	/* byte 0: n1  n0   .   .   .   .   R   L */
 	fingers = (packet[0] & 0xc0) >> 6;
@@ -278,6 +279,8 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		 */
 		input_report_abs(dev, ABS_Y,
 			ETP_YMAX_V2 - (((packet[4] & 0x03) << 8) | packet[5]));
+		pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
+		width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
 		break;
 
 	case 2:
@@ -311,6 +314,10 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		input_report_abs(dev, ABS_HAT0Y, y1);
 		input_report_abs(dev, ABS_HAT1X, x2);
 		input_report_abs(dev, ABS_HAT1Y, y2);
+
+		/* Unknown so just report sensible values */
+		pres = 127;
+		width = 7;
 		break;
 	}
 
@@ -320,6 +327,10 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 	input_report_key(dev, BTN_TOOL_QUADTAP, fingers == 4);
 	input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
 	input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
+	if (etd->reports_pres) {
+		input_report_abs(dev, ABS_PRESSURE, pres);
+		input_report_abs(dev, ABS_TOOL_WIDTH, width);
+	}
 
 	input_sync(dev);
 }
@@ -531,6 +542,12 @@ static void elantech_set_input_params(struct psmouse *psmouse)
 		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
 		input_set_abs_params(dev, ABS_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);
 		input_set_abs_params(dev, ABS_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);
+		if (etd->reports_pres) {
+			input_set_abs_params(dev, ABS_PRESSURE, ETP_PMIN_V2,
+					     ETP_PMAX_V2, 0, 0);
+			input_set_abs_params(dev, ABS_TOOL_WIDTH, ETP_WMIN_V2,
+					     ETP_WMAX_V2, 0, 0);
+		}
 		input_set_abs_params(dev, ABS_HAT0X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
 		input_set_abs_params(dev, ABS_HAT0Y, ETP_2FT_YMIN, ETP_2FT_YMAX, 0, 0);
 		input_set_abs_params(dev, ABS_HAT1X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index b98c3c2..f6e3be5 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -77,6 +77,11 @@
 #define ETP_YMIN_V2			(   0 + ETP_EDGE_FUZZ_V2)
 #define ETP_YMAX_V2			( 768 - ETP_EDGE_FUZZ_V2)
 
+#define ETP_PMIN_V2			0
+#define ETP_PMAX_V2			255
+#define ETP_WMIN_V2			0
+#define ETP_WMAX_V2			15
+
 /*
  * For two finger touches the coordinate of each finger gets reported
  * separately but with reduced resolution.
-- 
1.7.1


--
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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

When 2 fingers are pressed, the low and high coordinates are given.
Instead of reporting just the low coordinate as the normal position,
report an average of both. This allow to have the cursor always move,
whichever finger moves. In practice, this improves the support of
two-fingers scolling with the synaptics X driver.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index c84f741..d1b505a 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -301,11 +301,11 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
 		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
 		/*
-		 * For compatibility with the X Synaptics driver scale up
-		 * one coordinate and report as ordinary mouse movent
+		 * For compatibility with non-multitouch userspace apps
+		 * report the average of both coordinates and scale up.
 		 */
-		input_report_abs(dev, ABS_X, x1 << 2);
-		input_report_abs(dev, ABS_Y, y1 << 2);
+		input_report_abs(dev, ABS_X, (x1 + x2) << 1);
+		input_report_abs(dev, ABS_Y, (y1 + y2) << 1);
 		/*
 		 * For compatibility with the proprietary X Elantech driver
 		 * report both coordinates as hat coordinates
-- 
1.7.1


--
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 5/7] elantech: report position also with 3 fingers

[Éric Piel]

The 6-byte protocol supports reporting the position when three fingers
are pressed, exactly like when one finger is pressed. Report this.

In addition, it is also distinguishes between 3 and 4 fingers pressed.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index b09b458..633f100 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -257,6 +257,14 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 	input_report_key(dev, BTN_TOUCH, fingers != 0);
  	switch (fingers) {
+	case 3:
+		/*
+		 * Same as one finger, except report of more than 3 fingers:
+		 * byte 3:  n4  .   w1  w0   .   .   .   .
+		 */
+		if (packet[3] & 0x80)
+			fingers = 4;
+		/* pass through... */
 	case 1:
 		/*
 		 * byte 1:  .   .   .   .   .  x10 x9  x8
@@ -309,6 +317,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 	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, packet[0] & 0x01);
 	input_report_key(dev, BTN_RIGHT, packet[0] & 0x02);
 @@ -519,6 +528,7 @@ static void elantech_set_input_params(struct psmouse *psmouse)
 		break;
  	case 2:
+		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
 		input_set_abs_params(dev, ABS_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);
 		input_set_abs_params(dev, ABS_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);
 		input_set_abs_params(dev, ABS_HAT0X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
-- 
1.7.1

--
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 7/7] elantech: average the two coordinates when 2 fingers

[Dmitry Torokhov]

On Mon, Jun 21, 2010 at 11:06:28PM +0200, Éric Piel wrote:
> When 2 fingers are pressed, the low and high coordinates are given.
> Instead of reporting just the low coordinate as the normal position,
> report an average of both. This allow to have the cursor always move,
> whichever finger moves. In practice, this improves the support of
> two-fingers scolling with the synaptics X driver.
> 
> Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
> ---
>  drivers/input/mouse/elantech.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index c84f741..d1b505a 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -301,11 +301,11 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
>  		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
>  		/*
> -		 * For compatibility with the X Synaptics driver scale up
> -		 * one coordinate and report as ordinary mouse movent
> +		 * For compatibility with non-multitouch userspace apps
> +		 * report the average of both coordinates and scale up.
>  		 */
> -		input_report_abs(dev, ABS_X, x1 << 2);
> -		input_report_abs(dev, ABS_Y, y1 << 2);
> +		input_report_abs(dev, ABS_X, (x1 + x2) << 1);
> +		input_report_abs(dev, ABS_Y, (y1 + y2) << 1);

I am concerned what happens when you put one finger first and then add
another one. Would not that couse the cursor to jump?

I'd say we need to obey MT protocol and continue reporting the first
contact in non-MT protocol and rely on MT-aware drivers to do better job
of contact tracking. This is also consistent with how other drivers,
such as bcm5974, report coordinates.

-- 
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 5/7] elantech: report position also with 3 fingers

[Dmitry Torokhov]

Hi Eric,

On Mon, Jun 21, 2010 at 11:07:24PM +0200, Éric Piel wrote:
> 
> The 6-byte protocol supports reporting the position when three fingers
> are pressed, exactly like when one finger is pressed. Report this.
> 
> In addition, it is also distinguishes between 3 and 4 fingers pressed.
> 
> Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
> ---
>  drivers/input/mouse/elantech.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index b09b458..633f100 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -257,6 +257,14 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  	input_report_key(dev, BTN_TOUCH, fingers != 0);
>   	switch (fingers) {
> +	case 3:
> +		/*
> +		 * Same as one finger, except report of more than 3 fingers:
> +		 * byte 3:  n4  .   w1  w0   .   .   .   .
> +		 */
> +		if (packet[3] & 0x80)
> +			fingers = 4;
> +		/* pass through... */

Are we certain it is 4-finger and not equivalent of "palm detect" flag?
I could not locate the equivalent in Ubutu sources, where did you get
this data?

-- 
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 5/7] elantech: report position also with 3 fingers

[Éric Piel]

On 21-07-10 05:38, Dmitry Torokhov wrote:
:
>> +	case 3:
>> +		/*
>> +		 * Same as one finger, except report of more than 3 fingers:
>> +		 * byte 3:  n4  .   w1  w0   .   .   .   .
>> +		 */
>> +		if (packet[3]&  0x80)
>> +			fingers = 4;
>> +		/* pass through... */
>
> Are we certain it is 4-finger and not equivalent of "palm detect" flag?
> I could not locate the equivalent in Ubutu sources, where did you get
> this data?
Hi,

I've just tried it on my hardware, when putting the palm, only one 
finger is actually reported, with a large width. This flag is definitely 
only when 4 (or more fingers) are reported.

It's one of the small things not found in the ubuntu/dell driver, but 
that I just found out by observing the output.

Eric

Re: [PATCH 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

On 21-07-10 05:36, Dmitry Torokhov wrote:
> On Mon, Jun 21, 2010 at 11:06:28PM +0200, Éric Piel wrote:
>> When 2 fingers are pressed, the low and high coordinates are given.
>> Instead of reporting just the low coordinate as the normal position,
>> report an average of both. This allow to have the cursor always move,
>> whichever finger moves. In practice, this improves the support of
>> two-fingers scolling with the synaptics X driver.
:
>> -		input_report_abs(dev, ABS_X, x1<<  2);
>> -		input_report_abs(dev, ABS_Y, y1<<  2);
>> +		input_report_abs(dev, ABS_X, (x1 + x2)<<  1);
>> +		input_report_abs(dev, ABS_Y, (y1 + y2)<<  1);
>
> I am concerned what happens when you put one finger first and then add
> another one. Would not that couse the cursor to jump?
>
> I'd say we need to obey MT protocol and continue reporting the first
> contact in non-MT protocol and rely on MT-aware drivers to do better job
> of contact tracking. This is also consistent with how other drivers,
> such as bcm5974, report coordinates.
If the MT protocol says we should always report on the single-touch side 
the first contact, then I guess it's better to do so. However, I've just 
read the latest version of the mt-protocol document, and couldn't see 
anything saying that. Are you sure this is the official behavior? I'd 
tend to think that single-touch and multi-touch are never read together, 
so they can be different.

More specifically, on this hardware, when there are two fingers, we get 
only the lower coordinates and the higher coordinates, but we don't know 
exactly where are the fingers. As there is no tracking, the lower 
coordinate might be the second finger applied, so even if we report just 
the lower coordinates, there is 75% chance that the cursor will jump.

That's actually the reason I prefer the average: it gives a 100% 
expectable behavior (because the average is always correct). In 
practice, when using the xorg synaptics driver, with the default 
2-fingers-scrolling mode, the cursor never jumps because as soon as the 
second finger is applied, X and Y are not used to move the cursor.

So, I think this patch is an improvement for the user, but if you think 
that it would not respect the MT protocol, just drop it.

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 07/30/2010 08:55 PM, Éric Piel wrote:

> On 21-07-10 05:36, Dmitry Torokhov wrote:
>> On Mon, Jun 21, 2010 at 11:06:28PM +0200, Éric Piel wrote:
>>> When 2 fingers are pressed, the low and high coordinates are given.
>>> Instead of reporting just the low coordinate as the normal position,
>>> report an average of both. This allow to have the cursor always move,
>>> whichever finger moves. In practice, this improves the support of
>>> two-fingers scolling with the synaptics X driver.
> :
>>> -        input_report_abs(dev, ABS_X, x1<<  2);
>>> -        input_report_abs(dev, ABS_Y, y1<<  2);
>>> +        input_report_abs(dev, ABS_X, (x1 + x2)<<  1);
>>> +        input_report_abs(dev, ABS_Y, (y1 + y2)<<  1);
>>
>> I am concerned what happens when you put one finger first and then add
>> another one. Would not that couse the cursor to jump?


I agree, I have seen this behavior before and it is not quite what you want. The
average point does jump.

>> I'd say we need to obey MT protocol and continue reporting the first
>> contact in non-MT protocol and rely on MT-aware drivers to do better job
>> of contact tracking. This is also consistent with how other drivers,
>> such as bcm5974, report coordinates.
> If the MT protocol says we should always report on the single-touch side
> the first contact, then I guess it's better to do so. However, I've just
> read the latest version of the mt-protocol document, and couldn't see
> anything saying that. Are you sure this is the official behavior? I'd
> tend to think that single-touch and multi-touch are never read together,
> so they can be different.


It is true that there are no specific section on how to handle the ABS_X/Y
events, and different drivers have slightly different strategies. However,
common to them all is that a single finger is consistently used to report the
events.

> More specifically, on this hardware, when there are two fingers, we get
> only the lower coordinates and the higher coordinates, but we don't know
> exactly where are the fingers. As there is no tracking, the lower
> coordinate might be the second finger applied, so even if we report just
> the lower coordinates, there is 75% chance that the cursor will jump.


Perhaps it is better to not report ABS_X/Y at all in the case of two fingers on
this trackpad. With any reasonable userspace driver, the ABS_X/Y will be masked
out anyways.

> That's actually the reason I prefer the average: it gives a 100%
> expectable behavior (because the average is always correct). In
> practice, when using the xorg synaptics driver, with the default
> 2-fingers-scrolling mode, the cursor never jumps because as soon as the
> second finger is applied, X and Y are not used to move the cursor.


It is predictable, but it is still bad behavior. ;-)

> So, I think this patch is an improvement for the user, but if you think
> that it would not respect the MT protocol, just drop it.


Either dropping the patch or muting ABS_X/Y in case of multiple fingers is what
I would recommend.

Cheers,
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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 30-07-10 23:01, Henrik Rydberg schreef:
> On 07/30/2010 08:55 PM, Éric Piel wrote:
:
>>> I am concerned what happens when you put one finger first and then add
>>> another one. Would not that couse the cursor to jump?
> 
> 
> I agree, I have seen this behavior before and it is not quite what you want. The
> average point does jump.
> 
>>> I'd say we need to obey MT protocol and continue reporting the first
>>> contact in non-MT protocol and rely on MT-aware drivers to do better job
>>> of contact tracking. This is also consistent with how other drivers,
>>> such as bcm5974, report coordinates.
>> If the MT protocol says we should always report on the single-touch side
>> the first contact, then I guess it's better to do so. However, I've just
>> read the latest version of the mt-protocol document, and couldn't see
>> anything saying that. Are you sure this is the official behavior? I'd
>> tend to think that single-touch and multi-touch are never read together,
>> so they can be different.
> 
> 
> It is true that there are no specific section on how to handle the ABS_X/Y
> events, and different drivers have slightly different strategies. However,
> common to them all is that a single finger is consistently used to report the
> events.
Maybe it would be useful to add a paragraph to the mt-protocol document
about the relation with the single-touch events.

Well as an anecdote, on my old laptop with a single-touch synaptics
hardware, when I press 2 fingers, the average point is reported. That's
all in hardware, though so it probably doesn't count as a typical driver
behaviour ;-)

>> More specifically, on this hardware, when there are two fingers, we get
>> only the lower coordinates and the higher coordinates, but we don't know
>> exactly where are the fingers. As there is no tracking, the lower
>> coordinate might be the second finger applied, so even if we report just
>> the lower coordinates, there is 75% chance that the cursor will jump.
> 
> 
> Perhaps it is better to not report ABS_X/Y at all in the case of two fingers on
> this trackpad. With any reasonable userspace driver, the ABS_X/Y will be masked
> out anyways.
What do you mean? ABS_X/Y is still very useful, for example for gesture,
or to manage horizontal/vertical scrolling. In the xorg synaptics
driver, it's not used to move the cursor position, but it's definitely
not masked out!

>> That's actually the reason I prefer the average: it gives a 100%
>> expectable behavior (because the average is always correct). In
>> practice, when using the xorg synaptics driver, with the default
>> 2-fingers-scrolling mode, the cursor never jumps because as soon as the
>> second finger is applied, X and Y are not used to move the cursor.
> 
> 
> It is predictable, but it is still bad behavior. ;-)
:
I'm completely against not reporting the position with 2 fingers,
because definitely, users would lose some features. With 3 and 4
fingers, we report the lower coordinates (the only ones provided by the
hardware). So it'd be extremely weird not to report any for 2 fingers!

As a user, I enjoy better when the average is reported, because
whichever finger I move, I get a feedback, there is never a finger
"hidden" behind the other one. Nevertheless, if consistency matters,
let's just leave it as is.

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 07/30/2010 11:41 PM, Éric Piel wrote:

[...]

>> It is true that there are no specific section on how to handle the ABS_X/Y

>> events, and different drivers have slightly different strategies. However,
>> common to them all is that a single finger is consistently used to report the
>> events.
> Maybe it would be useful to add a paragraph to the mt-protocol document
> about the relation with the single-touch events.


Indeed.

> Well as an anecdote, on my old laptop with a single-touch synaptics
> hardware, when I press 2 fingers, the average point is reported. That's
> all in hardware, though so it probably doesn't count as a typical driver
> behaviour ;-)


Those devices have no other information to report.

>>> More specifically, on this hardware, when there are two fingers, we get

>>> only the lower coordinates and the higher coordinates, but we don't know
>>> exactly where are the fingers. As there is no tracking, the lower
>>> coordinate might be the second finger applied, so even if we report just
>>> the lower coordinates, there is 75% chance that the cursor will jump.
>>
>>
>> Perhaps it is better to not report ABS_X/Y at all in the case of two fingers on
>> this trackpad. With any reasonable userspace driver, the ABS_X/Y will be masked
>> out anyways.
> What do you mean? ABS_X/Y is still very useful, for example for gesture,
> or to manage horizontal/vertical scrolling. In the xorg synaptics
> driver, it's not used to move the cursor position, but it's definitely
> not masked out!


Maybe the idea sounds radical, but there is a rationale.

The semantics of ABS_X/Y is a singular, well-defined point. In the trackpad
case, the movement of that point is used to guide a pointer on screen, but there
are other scenarios.

In the presence of multiple touch points, both ABS_X/Y and the pointer are
ill-defined. This simple fact has far-reaching implications; in Xorg land, there
is a whole new input protocol cooking to deal with it. For all practical
purposes, the traditional ABS_X/Y point has no meaning in the multitouch case.

Instead of extending the semantics of ABS_X/Y, one could simply omit the event
during a multi-finger touch. In practice, this would most likely confuse some
applications. Sending the last known one-finger touch point should be less
confusing. Not quite equivalent, but close enough.

>>> That's actually the reason I prefer the average: it gives a 100%
>>> expectable behavior (because the average is always correct). In
>>> practice, when using the xorg synaptics driver, with the default
>>> 2-fingers-scrolling mode, the cursor never jumps because as soon as the
>>> second finger is applied, X and Y are not used to move the cursor.
>>
>>
>> It is predictable, but it is still bad behavior. ;-)
> :
> I'm completely against not reporting the position with 2 fingers,
> because definitely, users would lose some features. With 3 and 4
> fingers, we report the lower coordinates (the only ones provided by the
> hardware). So it'd be extremely weird not to report any for 2 fingers!


This sounds like a misunderstanding. To rephrase: the suggestion is to always
send the last known one-finger touch point, and to update that point only when
there is a single finger on the pad.


> As a user, I enjoy better when the average is reported, because
> whichever finger I move, I get a feedback, there is never a finger
> "hidden" behind the other one. Nevertheless, if consistency matters,
> let's just leave it as is.


This behavior can be created in userland using the MT touch points, if so preferred.

Thanks,
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 7/7] elantech: average the two coordinates when 2 fingers

[Dmitry Torokhov]

On Sat, Jul 31, 2010 at 11:28:20AM +0200, Henrik Rydberg wrote:
> On 07/30/2010 11:41 PM, Éric Piel wrote:
> 
> [...]
> 
> >> It is true that there are no specific section on how to handle the ABS_X/Y
> 
> >> events, and different drivers have slightly different strategies. However,
> >> common to them all is that a single finger is consistently used to report the
> >> events.
> > Maybe it would be useful to add a paragraph to the mt-protocol document
> > about the relation with the single-touch events.
> 
> 
> Indeed.
> 
> > Well as an anecdote, on my old laptop with a single-touch synaptics
> > hardware, when I press 2 fingers, the average point is reported. That's
> > all in hardware, though so it probably doesn't count as a typical driver
> > behaviour ;-)
> 
> 
> Those devices have no other information to report.
> 
> >>> More specifically, on this hardware, when there are two fingers, we get
> 
> >>> only the lower coordinates and the higher coordinates, but we don't know
> >>> exactly where are the fingers. As there is no tracking, the lower
> >>> coordinate might be the second finger applied, so even if we report just
> >>> the lower coordinates, there is 75% chance that the cursor will jump.
> >>
> >>
> >> Perhaps it is better to not report ABS_X/Y at all in the case of two fingers on
> >> this trackpad. With any reasonable userspace driver, the ABS_X/Y will be masked
> >> out anyways.
> > What do you mean? ABS_X/Y is still very useful, for example for gesture,
> > or to manage horizontal/vertical scrolling. In the xorg synaptics
> > driver, it's not used to move the cursor position, but it's definitely
> > not masked out!
> 
> 
> Maybe the idea sounds radical, but there is a rationale.
> 
> The semantics of ABS_X/Y is a singular, well-defined point. In the trackpad
> case, the movement of that point is used to guide a pointer on screen, but there
> are other scenarios.
> 
> In the presence of multiple touch points, both ABS_X/Y and the pointer are
> ill-defined. This simple fact has far-reaching implications; in Xorg land, there
> is a whole new input protocol cooking to deal with it. For all practical
> purposes, the traditional ABS_X/Y point has no meaning in the multitouch case.
> 
> Instead of extending the semantics of ABS_X/Y, one could simply omit the event
> during a multi-finger touch. In practice, this would most likely confuse some
> applications. Sending the last known one-finger touch point should be less
> confusing. Not quite equivalent, but close enough.
> 
> >>> That's actually the reason I prefer the average: it gives a 100%
> >>> expectable behavior (because the average is always correct). In
> >>> practice, when using the xorg synaptics driver, with the default
> >>> 2-fingers-scrolling mode, the cursor never jumps because as soon as the
> >>> second finger is applied, X and Y are not used to move the cursor.
> >>
> >>
> >> It is predictable, but it is still bad behavior. ;-)
> > :
> > I'm completely against not reporting the position with 2 fingers,
> > because definitely, users would lose some features. With 3 and 4
> > fingers, we report the lower coordinates (the only ones provided by the
> > hardware). So it'd be extremely weird not to report any for 2 fingers!
> 
> 
> This sounds like a misunderstanding. To rephrase: the suggestion is to always
> send the last known one-finger touch point, and to update that point only when
> there is a single finger on the pad.
> 

This would break two-finger scrolling mode of synaptics X driver.

> 
> > As a user, I enjoy better when the average is reported, because
> > whichever finger I move, I get a feedback, there is never a finger
> > "hidden" behind the other one. Nevertheless, if consistency matters,
> > let's just leave it as is.
> 
> 
> This behavior can be created in userland using the MT touch points, if so preferred.
> 

-- 
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 07/31/2010 11:33 AM, Dmitry Torokhov wrote:
[...]

>> This sounds like a misunderstanding. To rephrase: the suggestion is to always
>> send the last known one-finger touch point, and to update that point only when
>> there is a single finger on the pad.
>>
> 
> This would break two-finger scrolling mode of synaptics X driver.


I envision synaptics being MT-aware at some point, but alright, the idea is too
radical at this stage.

Back to the original problem. When adding the second finger, the first finger
will actually be in one of the four corners of the detected rectangle, correct?
So how about keeping as a state what corner of the rectangle represents the
original touch, and continue to report that corner as ABS_X/Y during a
two-finger touch?

Cheers,
Henrik

Re: [PATCH 7/7] elantech: average the two coordinates when 2 fingers

[Chris Bagwell]

On Fri, Jul 30, 2010 at 4:41 PM, Éric Piel <E.A.B.Piel@tudelft.nl> wrote:
>
> Op 30-07-10 23:01, Henrik Rydberg schreef:
> > On 07/30/2010 08:55 PM, Éric Piel wrote:
> :
> >>> I am concerned what happens when you put one finger first and then add
> >>> another one. Would not that couse the cursor to jump?
> >
> >
> > I agree, I have seen this behavior before and it is not quite what you want. The
> > average point does jump.
> >

This is basically same approach as what synaptics hardware is
reporting.  Their  Synaptics PS/2 TouchPad Interfacing Guide mentions
current hardware reports an x/y closer to first finger during multiple
fingers and that older hardware reports a value closer to center.
Both cases, you can see movement when either finger is moving.

There are a few existing X side bug reports I've seen related to this
behaviour.  On newer synaptics pad its easiest to see if you touch 1st
finger on left side of pad, 2nd finger on right side of pad, and then
release 1st finger then you will see a big cursor jump as hardware is
force to transition over to 2nd finger x/y (make sure 2 finger
scrolling is disabled as that feature can fix issue).

I've seen xf86-input-synaptics patches floating around that try to
detect jumps-because-of-averaging from the size of delta and discard
but it is easy to be mistaken.  Probably its better to detect
BTN_TOOL_DOUBLETAP transitions back to 0 and treating similar to both
fingers being lifted to account for expected jump.

So an option is to do something similar to below and clearly document
that user space needs to account for known reporting behaviour when
BTN_TOOL_DOUBLETOUCH transitions back to 0.

input_report_abs(dev, ABS_X, x1 + (x2 & 7));
input_report_abs(dev, ABS_Y, y1 + (y2 & 7));

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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 31-07-10 14:49, Henrik Rydberg schreef:
> On 07/31/2010 11:33 AM, Dmitry Torokhov wrote:
> [...]
> 
>>> This sounds like a misunderstanding. To rephrase: the suggestion is to always
>>> send the last known one-finger touch point, and to update that point only when
>>> there is a single finger on the pad.
>>>
>>
>> This would break two-finger scrolling mode of synaptics X driver.
> 
> 
> I envision synaptics being MT-aware at some point, but alright, the idea is too
> radical at this stage.
> 
> Back to the original problem. When adding the second finger, the first finger
> will actually be in one of the four corners of the detected rectangle, correct?
> So how about keeping as a state what corner of the rectangle represents the
> original touch, and continue to report that corner as ABS_X/Y during a
> two-finger touch?
That could work, but in general the policy is to not do finger tracking
in the kernel. And still, if you leave this first finger before the
second finger, you'll necessarily see this "jump" as well.

Eric

Re: [PATCH 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 31-07-10 21:53, Chris Bagwell schreef:
> 
> 
> On Fri, Jul 30, 2010 at 4:01 PM, Henrik Rydberg <rydberg@euromail.se
> <mailto:rydberg@euromail.se>> wrote:
> 
>     On 07/30/2010 08:55 PM, Éric Piel wrote:
> 
>     > On 21-07-10 05:36, Dmitry Torokhov wrote:
>     >> On Mon, Jun 21, 2010 at 11:06:28PM +0200, Éric Piel wrote:
>     >>> When 2 fingers are pressed, the low and high coordinates are given.
>     >>> Instead of reporting just the low coordinate as the normal position,
>     >>> report an average of both. This allow to have the cursor always
>     move,
>     >>> whichever finger moves. In practice, this improves the support of
>     >>> two-fingers scolling with the synaptics X driver.
>     > :
>     >>> -        input_report_abs(dev, ABS_X, x1<<  2);
>     >>> -        input_report_abs(dev, ABS_Y, y1<<  2);
>     >>> +        input_report_abs(dev, ABS_X, (x1 + x2)<<  1);
>     >>> +        input_report_abs(dev, ABS_Y, (y1 + y2)<<  1);
>     >>
>     >> I am concerned what happens when you put one finger first and
>     then add
>     >> another one. Would not that couse the cursor to jump?
> 
> 
>     I agree, I have seen this behavior before and it is not quite what
>     you want. The
>     average point does jump.
> 
> 
> This is basically same approach as what synaptics hardware is
> reporting.  Their  Synaptics PS/2 TouchPad Interfacing Guide mentions
> current hardware reports an x/y closer to first finger during multiple
> fingers and that older hardware reports a value closer to center.  Both
> cases, you can see movement when either finger is moving.
> 
> There are a few existing X side bug reports I've seen related to this
> behaviour.  On newer synaptics pad its easiest to see if you touch 1st
> finger on left side of pad, 2nd finger on right side of pad, and then
> release 1st finger then you will see a big cursor jump as hardware is
> force to transition over to 2nd finger x/y (make sure 2 finger scrolling
> is disabled as that feature can fix issue).
> 
> I've seen xf86-input-synaptics patches floating around that try to
> detect jumps-because-of-averaging from the size of delta and discard but
> it is easy to be mistaken.  Probably its better to detect
> BTN_TOOL_DOUBLETAP transitions back to 0 and treating similar to both
> fingers being lifted to account for expected jump.
> 
> So an option is to do something similar to below and clearly document
> that user space needs to account for known reporting behaviour when
> BTN_TOOL_DOUBLETOUCH transitions back to 0.
I've got some fairly new versions of everything, but at least here, this
is already happening with the X synaptics driver: in practice there is
not jump at all, which ever finger is reported in doubletap. In
doubletap it moves the scrolling axes, and changing to singletap, it
resets to the new current position. No jump, both with my old synaptics
hardware and with the elantech.

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 07/31/2010 09:53 PM, Chris Bagwell wrote:

[...]
> I've seen xf86-input-synaptics patches floating around that try to detect
> jumps-because-of-averaging from the size of delta and discard but it is easy
> to be mistaken.  Probably its better to detect BTN_TOOL_DOUBLETAP
> transitions back to 0 and treating similar to both fingers being lifted to
> account for expected jump.


Looks good. Detecting "finger configuration changed" based on the number of
fingers is probably the closest one can get to MT behavior without actually
using it.

Henrik

Re: [PATCH 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/01/2010 01:00 AM, Éric Piel wrote:

> Op 31-07-10 14:49, Henrik Rydberg schreef:
>> On 07/31/2010 11:33 AM, Dmitry Torokhov wrote:
>> [...]
>>
>>>> This sounds like a misunderstanding. To rephrase: the suggestion is to always
>>>> send the last known one-finger touch point, and to update that point only when
>>>> there is a single finger on the pad.
>>>>
>>>
>>> This would break two-finger scrolling mode of synaptics X driver.
>>
>>
>> I envision synaptics being MT-aware at some point, but alright, the idea is too
>> radical at this stage.
>>
>> Back to the original problem. When adding the second finger, the first finger
>> will actually be in one of the four corners of the detected rectangle, correct?
>> So how about keeping as a state what corner of the rectangle represents the
>> original touch, and continue to report that corner as ABS_X/Y during a
>> two-finger touch?
> That could work, but in general the policy is to not do finger tracking
> in the kernel. And still, if you leave this first finger before the
> second finger, you'll necessarily see this "jump" as well.


The difference being that all positions reported correspond to real fingers.

The problem with this particular device is that it is more primitive than type
A, so some method to produce correct MT information would be desirable. Keeping
track of what corners of the rectangle correspond to fingers does not tell which
finger is which (thus not finger tracking), but makes this device comply with
type A, which is capable of detecting an unordered set of actual touch points.

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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/01/2010 01:04 AM, Éric Piel wrote:

[...]

> I've got some fairly new versions of everything, but at least here, this
> is already happening with the X synaptics driver: in practice there is
> not jump at all, which ever finger is reported in doubletap. In
> doubletap it moves the scrolling axes, and changing to singletap, it
> resets to the new current position. No jump, both with my old synaptics
> hardware and with the elantech.


You are right. Jumping does not generally occur in synaptics when the number of
fingers changes.

The overall solution is still not quite satisfactory, since the MT points
reported are the lower-left and upper-right corners of a rectangle, instead of
the actual touch points. As mentioned in another mail, it would make sense to
add logic to bring this device up to the type A level. If the needed logic turns
out to be equivalent to the type B level, what can you do. :-)

How about this as a stab at the MT point problem. The idea here is that
get_corner_id() returns the rectangle corner which minimizes the change in
motion direction, according to the principle of inertia. For all cases,
position[0] is reported as the ABS_X/Y point.

state = (num_finger, corner_id, motion_id, position[n])

At each touch frame update, do

1. num_finger == 1

Set
  position[0] = finger-position

2. num_finger == 2

If last frame was one finger, do
  Find corner id such that position change is minimized
    id = get_initial_corner_id(position[0], corner-position[id])

Else
  Find temporary motion id
    mid = get_motion_id(position[0], corner-position[corner_id])
  Find corner id such that motion change is minimized
    id = get_corner_id(corner_id, motion_id, mid)

Set
  corner_id = id
  motion_id = get_motion_id(position[0], corner-position[id])
  position[0] = corner-position[id],
  position[1] = corner-position[opposite[id]]

Report position[0] and position[1] as MT points

Since this is all integer logic, it should be possible to detect motion changes
as sign changes, such that get_motion_id() and get_corner_id() can be
implemented as constant arrays.

Cheers,
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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 01-08-10 11:37, Henrik Rydberg schreef:
> On 08/01/2010 01:04 AM, Éric Piel wrote:
> 
> [...]
> 
>> I've got some fairly new versions of everything, but at least here, this
>> is already happening with the X synaptics driver: in practice there is
>> not jump at all, which ever finger is reported in doubletap. In
>> doubletap it moves the scrolling axes, and changing to singletap, it
>> resets to the new current position. No jump, both with my old synaptics
>> hardware and with the elantech.
> 
> 
> You are right. Jumping does not generally occur in synaptics when the number of
> fingers changes.
> 
> The overall solution is still not quite satisfactory, since the MT points
> reported are the lower-left and upper-right corners of a rectangle, instead of
> the actual touch points. As mentioned in another mail, it would make sense to
> add logic to bring this device up to the type A level. If the needed logic turns
> out to be equivalent to the type B level, what can you do. :-)
> 
> How about this as a stab at the MT point problem. The idea here is that
> get_corner_id() returns the rectangle corner which minimizes the change in
> motion direction, according to the principle of inertia. For all cases,
> position[0] is reported as the ABS_X/Y point.
> 
> state = (num_finger, corner_id, motion_id, position[n])
> 
> At each touch frame update, do
> 
> 1. num_finger == 1
> 
> Set
>   position[0] = finger-position
> 
> 2. num_finger == 2
> 
> If last frame was one finger, do
>   Find corner id such that position change is minimized
>     id = get_initial_corner_id(position[0], corner-position[id])
> 
> Else
>   Find temporary motion id
>     mid = get_motion_id(position[0], corner-position[corner_id])
>   Find corner id such that motion change is minimized
>     id = get_corner_id(corner_id, motion_id, mid)
> 
> Set
>   corner_id = id
>   motion_id = get_motion_id(position[0], corner-position[id])
>   position[0] = corner-position[id],
>   position[1] = corner-position[opposite[id]]
> 
> Report position[0] and position[1] as MT points
> 
> Since this is all integer logic, it should be possible to detect motion changes
> as sign changes, such that get_motion_id() and get_corner_id() can be
> implemented as constant arrays.
Hi, I've tried, with a simplified version, and it works!

Basically, I track both coordinates independently, as the rectangle
coordinates are independent anyway. Then I associate to position[0]
always the closest coordinates compared to the previous frame.

It works pretty fine. At least it avoid the jump between one and two
fingers. Of course, when the two fingers become horizontally or
vertically aligned (eg: in a rotation gesture), it's not possible to
differentiate anymore when they dis-align. And I don't think using motion
will help because the hardware tends to report _exactly_ the same value
as soon the two fingers become close (= a segment instead of a
rectangle).  Or motion tracking would require a long window of past
frames.

I still think that for the very specific use case of scrolling when
pressing one finger and moving up and dow the other one, reporting the
average works better than the first finger. However, I guess this can be
considered just as a drawback of the ST protocol, and fixed in userspace
by using the MT protocol.

What do you think? Does it look fine to you? Below is the code.

Eric


8<--------------------------------------------------------------------
The elantech hardware is limited when reporting two fingers. It just
reports the minimum and maximum coordinates of the rectangle containing
the fingers.

As multitouch protocol requires at least to report the correct position
of each finger, we track the finger position when there is only one
finger to distinguish which corner of the rectangle corresponds to the
actual position of the fingers. When there are three fingers, only the
minimum position is reported, so we just don't track it and hope the
last tracked position has the most likelihood to be correct.

It works fine, at least as long as the two fingers don't get
horizontally or vertically aligned, after what it's back to random.
This also allows to avoid so "jumps" in the single-touch protocol.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   51 ++++++++++++++++++++++++++++++++--------
 drivers/input/mouse/elantech.h |    2 +
 2 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index bb66ae8..a3a3d8d 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -242,6 +242,33 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+
+static void track_one_finger(struct elantech_data *etd, int x, int y)
+{
+	etd->prev_x = x;
+	etd->prev_y = y;
+}
+
+static void find_closest_and_swap(int *ref, int *a, int *b)
+{
+	int diff_a = abs(*ref - *a);
+	int diff_b = abs(*ref - *b);
+	int tmp;
+	if (diff_b < diff_a) {
+		tmp = *a;
+		*a = *b;
+		*b = tmp;
+	}
+	*ref = *a;
+}
+
+static void track_two_fingers(struct elantech_data *etd, int *x1, int *y1, int *x2, int *y2)
+{
+	find_closest_and_swap(&etd->prev_x, x1, x2);
+	find_closest_and_swap(&etd->prev_y, y1, y2);
+}
+
+
 /*
  * Interpret complete data packets and report absolute mode input events for
  * hardware version 2. (6 byte packets)
@@ -284,6 +311,8 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 
 		input_report_abs(dev, ABS_X, x1);
 		input_report_abs(dev, ABS_Y, y1);
+		if (fingers == 1)
+			track_one_finger(etd, x1, y1);
 
 		pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
 		width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
@@ -296,29 +325,31 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		 * byte 0:  .   .  ay8 ax8  .   .   .   .
 		 * byte 1: ax7 ax6 ax5 ax4 ax3 ax2 ax1 ax0
 		 */
-		x1 = ((packet[0] & 0x10) << 4) | packet[1];
+		x1 = (((packet[0] & 0x10) << 4) | packet[1]) << 2;
 		/* byte 2: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0 */
-		y1 = ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2]);
+		y1 = (ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2])) << 2;
 		/*
 		 * byte 3:  .   .  by8 bx8  .   .   .   .
 		 * byte 4: bx7 bx6 bx5 bx4 bx3 bx2 bx1 bx0
 		 */
-		x2 = ((packet[3] & 0x10) << 4) | packet[4];
+		x2 = (((packet[3] & 0x10) << 4) | packet[4]) << 2;
 		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
-		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
+		y2 = (ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5])) << 2;
+
+		track_two_fingers(etd, &x1, &y1, &x2, &y2);
 		/* Multitouch */
-		input_report_abs(dev, ABS_MT_POSITION_X, x1 << 2);
-		input_report_abs(dev, ABS_MT_POSITION_Y, y1 << 2);
+		input_report_abs(dev, ABS_MT_POSITION_X, x1);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y1);
 		input_mt_sync(dev);
-		input_report_abs(dev, ABS_MT_POSITION_X, x2 << 2);
-		input_report_abs(dev, ABS_MT_POSITION_Y, y2 << 2);
+		input_report_abs(dev, ABS_MT_POSITION_X, x2);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y2);
 		input_mt_sync(dev);
 		/*
 		 * For compatibility with non-multitouch userspace apps
 		 * report the average of both coordinates and scale up.
 		 */
-		input_report_abs(dev, ABS_X, (x1 + x2) << 1);
-		input_report_abs(dev, ABS_Y, (y1 + y2) << 1);
+		input_report_abs(dev, ABS_X, (x1 + x2) >> 1);
+		input_report_abs(dev, ABS_Y, (y1 + y2) >> 1);
 		/*
 		 * For compatibility with the proprietary X Elantech driver
 		 * report both coordinates as hat coordinates
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index f6e3be5..fa9a6b4 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -111,6 +111,8 @@ struct elantech_data {
 	unsigned char hw_version;
 	unsigned int  fw_version;
 	unsigned char parity[256];
+	unsigned int prev_x;
+	unsigned int prev_y;
 };
 
 enum paritycheck_types {
-- 
1.7.2.1



--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/01/2010 01:28 PM, Éric Piel wrote:
[...]

>> How about this as a stab at the MT point problem. The idea here is that
>> get_corner_id() returns the rectangle corner which minimizes the change in
>> motion direction, according to the principle of inertia. For all cases,
>> position[0] is reported as the ABS_X/Y point.
>>
>> state = (num_finger, corner_id, motion_id, position[n])
>>
>> At each touch frame update, do
>>
>> 1. num_finger == 1
>>
>> Set
>>   position[0] = finger-position
>>
>> 2. num_finger == 2
>>
>> If last frame was one finger, do
>>   Find corner id such that position change is minimized
>>     id = get_initial_corner_id(position[0], corner-position[id])
>>
>> Else
>>   Find temporary motion id
>>     mid = get_motion_id(position[0], corner-position[corner_id])
>>   Find corner id such that motion change is minimized
>>     id = get_corner_id(corner_id, motion_id, mid)
>>
>> Set
>>   corner_id = id
>>   motion_id = get_motion_id(position[0], corner-position[id])
>>   position[0] = corner-position[id],
>>   position[1] = corner-position[opposite[id]]
>>
>> Report position[0] and position[1] as MT points
>>
>> Since this is all integer logic, it should be possible to detect motion changes
>> as sign changes, such that get_motion_id() and get_corner_id() can be
>> implemented as constant arrays.
> Hi, I've tried, with a simplified version, and it works!


:-)

> Basically, I track both coordinates independently, as the rectangle
> coordinates are independent anyway. Then I associate to position[0]
> always the closest coordinates compared to the previous frame.


Yep, looks equivalent to the first part of the pseudo code.

> It works pretty fine. At least it avoid the jump between one and two
> fingers. Of course, when the two fingers become horizontally or
> vertically aligned (eg: in a rotation gesture), it's not possible to
> differentiate anymore when they dis-align. And I don't think using motion
> will help because the hardware tends to report _exactly_ the same value
> as soon the two fingers become close (= a segment instead of a
> rectangle).  Or motion tracking would require a long window of past
> frames.


The case when the fingers are first resting in horizontal/vertical position,
then moving, is, as you say, not possible to resolve. I agree, the version you
implemented is probably as far as one gets without more information from the
device. Too bad. :-)

> I still think that for the very specific use case of scrolling when
> pressing one finger and moving up and dow the other one, reporting the
> average works better than the first finger. However, I guess this can be
> considered just as a drawback of the ST protocol, and fixed in userspace
> by using the MT protocol.
> 
> What do you think? Does it look fine to you? Below is the code.


I might have lost track of what problem needs to be solved. The current patch
seems to implement tracking, but still does not solve the individual MT finger
problem. And, it uses the same definition of ABS_X/Y as before. I was also under
the impression that synaptics needs fixing, anyways. All of this taken together
sadly suggests that this patch could just as well be reverted to the original
one. Or? Alternatively, one could switch to the type B protocol, since no
further tracking improvement is possible in userspace. The implementation is
tidy and simple enough, I think.

> Eric
> 
> 
> 8<--------------------------------------------------------------------
> The elantech hardware is limited when reporting two fingers. It just
> reports the minimum and maximum coordinates of the rectangle containing
> the fingers.
> 
> As multitouch protocol requires at least to report the correct position
> of each finger, we track the finger position when there is only one
> finger to distinguish which corner of the rectangle corresponds to the
> actual position of the fingers. When there are three fingers, only the
> minimum position is reported, so we just don't track it and hope the
> last tracked position has the most likelihood to be correct.
> 
> It works fine, at least as long as the two fingers don't get
> horizontally or vertically aligned, after what it's back to random.
> This also allows to avoid so "jumps" in the single-touch protocol.
> 
> Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
> ---
>  drivers/input/mouse/elantech.c |   51 ++++++++++++++++++++++++++++++++--------
>  drivers/input/mouse/elantech.h |    2 +
>  2 files changed, 43 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index bb66ae8..a3a3d8d 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -242,6 +242,33 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
>  	input_sync(dev);
>  }
>  
> +
> +static void track_one_finger(struct elantech_data *etd, int x, int y)
> +{
> +	etd->prev_x = x;
> +	etd->prev_y = y;
> +}
> +
> +static void find_closest_and_swap(int *ref, int *a, int *b)
> +{
> +	int diff_a = abs(*ref - *a);
> +	int diff_b = abs(*ref - *b);
> +	int tmp;
> +	if (diff_b < diff_a) {
> +		tmp = *a;
> +		*a = *b;
> +		*b = tmp;
> +	}
> +	*ref = *a;
> +}


Use swap(a, b);

> +
> +static void track_two_fingers(struct elantech_data *etd, int *x1, int *y1, int *x2, int *y2)
> +{
> +	find_closest_and_swap(&etd->prev_x, x1, x2);
> +	find_closest_and_swap(&etd->prev_y, y1, y2);
> +}
> +
> +
>  /*
>   * Interpret complete data packets and report absolute mode input events for
>   * hardware version 2. (6 byte packets)
> @@ -284,6 +311,8 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  
>  		input_report_abs(dev, ABS_X, x1);
>  		input_report_abs(dev, ABS_Y, y1);
> +		if (fingers == 1)
> +			track_one_finger(etd, x1, y1);
>  
>  		pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
>  		width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
> @@ -296,29 +325,31 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
>  		 * byte 0:  .   .  ay8 ax8  .   .   .   .
>  		 * byte 1: ax7 ax6 ax5 ax4 ax3 ax2 ax1 ax0
>  		 */
> -		x1 = ((packet[0] & 0x10) << 4) | packet[1];
> +		x1 = (((packet[0] & 0x10) << 4) | packet[1]) << 2;
>  		/* byte 2: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0 */
> -		y1 = ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2]);
> +		y1 = (ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2])) << 2;
>  		/*
>  		 * byte 3:  .   .  by8 bx8  .   .   .   .
>  		 * byte 4: bx7 bx6 bx5 bx4 bx3 bx2 bx1 bx0
>  		 */
> -		x2 = ((packet[3] & 0x10) << 4) | packet[4];
> +		x2 = (((packet[3] & 0x10) << 4) | packet[4]) << 2;
>  		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
> -		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
> +		y2 = (ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5])) << 2;
> +
> +		track_two_fingers(etd, &x1, &y1, &x2, &y2);
>  		/* Multitouch */
> -		input_report_abs(dev, ABS_MT_POSITION_X, x1 << 2);
> -		input_report_abs(dev, ABS_MT_POSITION_Y, y1 << 2);
> +		input_report_abs(dev, ABS_MT_POSITION_X, x1);
> +		input_report_abs(dev, ABS_MT_POSITION_Y, y1);
>  		input_mt_sync(dev);
> -		input_report_abs(dev, ABS_MT_POSITION_X, x2 << 2);
> -		input_report_abs(dev, ABS_MT_POSITION_Y, y2 << 2);
> +		input_report_abs(dev, ABS_MT_POSITION_X, x2);
> +		input_report_abs(dev, ABS_MT_POSITION_Y, y2);
>  		input_mt_sync(dev);
>  		/*
>  		 * For compatibility with non-multitouch userspace apps
>  		 * report the average of both coordinates and scale up.
>  		 */
> -		input_report_abs(dev, ABS_X, (x1 + x2) << 1);
> -		input_report_abs(dev, ABS_Y, (y1 + y2) << 1);
> +		input_report_abs(dev, ABS_X, (x1 + x2) >> 1);
> +		input_report_abs(dev, ABS_Y, (y1 + y2) >> 1);
>  		/*
>  		 * For compatibility with the proprietary X Elantech driver
>  		 * report both coordinates as hat coordinates
> diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
> index f6e3be5..fa9a6b4 100644
> --- a/drivers/input/mouse/elantech.h
> +++ b/drivers/input/mouse/elantech.h
> @@ -111,6 +111,8 @@ struct elantech_data {
>  	unsigned char hw_version;
>  	unsigned int  fw_version;
>  	unsigned char parity[256];
> +	unsigned int prev_x;
> +	unsigned int prev_y;
>  };
>  
>  enum paritycheck_types {


Thanks,
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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 01-08-10 15:57, Henrik Rydberg schreef:
> On 08/01/2010 01:28 PM, Éric Piel wrote:
:
>> I still think that for the very specific use case of scrolling when
>> pressing one finger and moving up and dow the other one, reporting the
>> average works better than the first finger. However, I guess this can be
>> considered just as a drawback of the ST protocol, and fixed in userspace
>> by using the MT protocol.
>>
>> What do you think? Does it look fine to you? Below is the code.
> 
> 
> I might have lost track of what problem needs to be solved. The current patch
> seems to implement tracking, but still does not solve the individual MT finger
> problem. And, it uses the same definition of ABS_X/Y as before. I was also under
> the impression that synaptics needs fixing, anyways. All of this taken together
> sadly suggests that this patch could just as well be reverted to the original
> one. Or? Alternatively, one could switch to the type B protocol, since no
> further tracking improvement is possible in userspace. The implementation is
> tidy and simple enough, I think.
Yes, you're right, the patch I've sent was still with the "average of
the 2 fingers", but I'm now willing to drop it. With the tracking, at
least we can keep sending info about a real finger and avoid jumps at
the transition 1->2, so reporting the first finger might have advantages
over reporting the average :-) The improvement for the test case can
just go to userspace.

The tracking is still not so clever, so it's definitly not adapted to a
type B MT protocol (think transition 2->1).

Dmitry, if that's ok with you, you can drop this patch 7/7, and I'll
send you two new patches to add MT support and finger tracking?

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 10:17 AM, Éric Piel wrote:

> Op 01-08-10 15:57, Henrik Rydberg schreef:
>> On 08/01/2010 01:28 PM, Éric Piel wrote:
> :
>>> I still think that for the very specific use case of scrolling when
>>> pressing one finger and moving up and dow the other one, reporting the
>>> average works better than the first finger. However, I guess this can be
>>> considered just as a drawback of the ST protocol, and fixed in userspace
>>> by using the MT protocol.
>>>
>>> What do you think? Does it look fine to you? Below is the code.
>>
>>
>> I might have lost track of what problem needs to be solved. The current patch
>> seems to implement tracking, but still does not solve the individual MT finger
>> problem. And, it uses the same definition of ABS_X/Y as before. I was also under
>> the impression that synaptics needs fixing, anyways. All of this taken together
>> sadly suggests that this patch could just as well be reverted to the original
>> one. Or? Alternatively, one could switch to the type B protocol, since no
>> further tracking improvement is possible in userspace. The implementation is
>> tidy and simple enough, I think.
> Yes, you're right, the patch I've sent was still with the "average of
> the 2 fingers", but I'm now willing to drop it. With the tracking, at
> least we can keep sending info about a real finger and avoid jumps at
> the transition 1->2, so reporting the first finger might have advantages
> over reporting the average :-) The improvement for the test case can
> just go to userspace.
> 
> The tracking is still not so clever, so it's definitly not adapted to a
> type B MT protocol (think transition 2->1).


You need to add the tracking id and a couple of lines, but i do not see why the
2->1 transition would be treated any differently. The one-finger coordinate
would be close to either position[0] or position[1], which would determine the
tracking id to keep. Every time you add a finger you add a new tracking id. What
is your planned support for three fingers?

Henrik

> Dmitry, if that's ok with you, you can drop this patch 7/7, and I'll
> send you two new patches to add MT support and finger tracking?
> 
> Eric


--
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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 02-08-10 12:02, Henrik Rydberg schreef:
> On 08/02/2010 10:17 AM, Éric Piel wrote:
> 
>> Op 01-08-10 15:57, Henrik Rydberg schreef:
>>> On 08/01/2010 01:28 PM, Éric Piel wrote:
>> :
>>>> I still think that for the very specific use case of scrolling when
>>>> pressing one finger and moving up and dow the other one, reporting the
>>>> average works better than the first finger. However, I guess this can be
>>>> considered just as a drawback of the ST protocol, and fixed in userspace
>>>> by using the MT protocol.
>>>>
>>>> What do you think? Does it look fine to you? Below is the code.
>>>
>>>
>>> I might have lost track of what problem needs to be solved. The current patch
>>> seems to implement tracking, but still does not solve the individual MT finger
>>> problem. And, it uses the same definition of ABS_X/Y as before. I was also under
>>> the impression that synaptics needs fixing, anyways. All of this taken together
>>> sadly suggests that this patch could just as well be reverted to the original
>>> one. Or? Alternatively, one could switch to the type B protocol, since no
>>> further tracking improvement is possible in userspace. The implementation is
>>> tidy and simple enough, I think.
>> Yes, you're right, the patch I've sent was still with the "average of
>> the 2 fingers", but I'm now willing to drop it. With the tracking, at
>> least we can keep sending info about a real finger and avoid jumps at
>> the transition 1->2, so reporting the first finger might have advantages
>> over reporting the average :-) The improvement for the test case can
>> just go to userspace.
>>
>> The tracking is still not so clever, so it's definitly not adapted to a
>> type B MT protocol (think transition 2->1).
> 
> 
> You need to add the tracking id and a couple of lines, but i do not see why the
> 2->1 transition would be treated any differently. The one-finger coordinate
> would be close to either position[0] or position[1], which would determine the
> tracking id to keep. Every time you add a finger you add a new tracking id. What
> is your planned support for three fingers?
Yes, yes, it's probably fairly easy to do some kind of tracking. But I
think that as long as the hardware does not provide such a thing, it's
better to do the minimum in kernel space, just enough to be meaningful,
and leave the rest to userspace.

Concerning 3 fingers (and more), the hardware reports only one set of
coordinates, the lower ones. So in this case, tracking is definitely not
possible.

Eric

--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 01:12 PM, Éric Piel wrote:

> Op 02-08-10 12:02, Henrik Rydberg schreef:
>> On 08/02/2010 10:17 AM, Éric Piel wrote:
>>
>>> Op 01-08-10 15:57, Henrik Rydberg schreef:
>>>> On 08/01/2010 01:28 PM, Éric Piel wrote:
>>> :
>>>>> I still think that for the very specific use case of scrolling when
>>>>> pressing one finger and moving up and dow the other one, reporting the
>>>>> average works better than the first finger. However, I guess this can be
>>>>> considered just as a drawback of the ST protocol, and fixed in userspace
>>>>> by using the MT protocol.
>>>>>
>>>>> What do you think? Does it look fine to you? Below is the code.
>>>>
>>>>
>>>> I might have lost track of what problem needs to be solved. The current patch
>>>> seems to implement tracking, but still does not solve the individual MT finger
>>>> problem. And, it uses the same definition of ABS_X/Y as before. I was also under
>>>> the impression that synaptics needs fixing, anyways. All of this taken together
>>>> sadly suggests that this patch could just as well be reverted to the original
>>>> one. Or? Alternatively, one could switch to the type B protocol, since no
>>>> further tracking improvement is possible in userspace. The implementation is
>>>> tidy and simple enough, I think.
>>> Yes, you're right, the patch I've sent was still with the "average of
>>> the 2 fingers", but I'm now willing to drop it. With the tracking, at
>>> least we can keep sending info about a real finger and avoid jumps at
>>> the transition 1->2, so reporting the first finger might have advantages
>>> over reporting the average :-) The improvement for the test case can
>>> just go to userspace.
>>>
>>> The tracking is still not so clever, so it's definitly not adapted to a
>>> type B MT protocol (think transition 2->1).
>>
>>
>> You need to add the tracking id and a couple of lines, but i do not see why the
>> 2->1 transition would be treated any differently. The one-finger coordinate
>> would be close to either position[0] or position[1], which would determine the
>> tracking id to keep. Every time you add a finger you add a new tracking id. What
>> is your planned support for three fingers?
> Yes, yes, it's probably fairly easy to do some kind of tracking. But I
> think that as long as the hardware does not provide such a thing, it's
> better to do the minimum in kernel space, just enough to be meaningful,
> and leave the rest to userspace.


The implemented part could also be done in userspace. Going half-way just to
circumvent buggy behavior in synaptics is really not a good idea.

> Concerning 3 fingers (and more), the hardware reports only one set of
> coordinates, the lower ones. So in this case, tracking is definitely not
> possible.


Ok.

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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 02-08-10 13:22, Henrik Rydberg schreef:
> On 08/02/2010 01:12 PM, Éric Piel wrote:
:
>>> You need to add the tracking id and a couple of lines, but i do not see why the
>>> 2->1 transition would be treated any differently. The one-finger coordinate
>>> would be close to either position[0] or position[1], which would determine the
>>> tracking id to keep. Every time you add a finger you add a new tracking id. What
>>> is your planned support for three fingers?
>> Yes, yes, it's probably fairly easy to do some kind of tracking. But I
>> think that as long as the hardware does not provide such a thing, it's
>> better to do the minimum in kernel space, just enough to be meaningful,
>> and leave the rest to userspace.
> 
> 
> The implemented part could also be done in userspace. Going half-way just to
> circumvent buggy behavior in synaptics is really not a good idea.
No, we've been going from protocol 0.5 (report max/min coordinates) to
protocol A.5 (report finger positions, often with correct track ID). My
argument is that it's not because we are half-way to B, by chance, that
we should go up to it. We do just the minimum to respect the minimum
protocol. Once the driver respects that protocol, all the fancy stuff
has to stay in userspace. There is already mtdev (I'm sure I don't have
to tell you ;-) ), I don't see the point of doing some copy-pasting.

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 01:33 PM, Éric Piel wrote:

> Op 02-08-10 13:22, Henrik Rydberg schreef:
>> On 08/02/2010 01:12 PM, Éric Piel wrote:
> :
>>>> You need to add the tracking id and a couple of lines, but i do not see why the
>>>> 2->1 transition would be treated any differently. The one-finger coordinate
>>>> would be close to either position[0] or position[1], which would determine the
>>>> tracking id to keep. Every time you add a finger you add a new tracking id. What
>>>> is your planned support for three fingers?
>>> Yes, yes, it's probably fairly easy to do some kind of tracking. But I
>>> think that as long as the hardware does not provide such a thing, it's
>>> better to do the minimum in kernel space, just enough to be meaningful,
>>> and leave the rest to userspace.
>>
>>
>> The implemented part could also be done in userspace. Going half-way just to
>> circumvent buggy behavior in synaptics is really not a good idea.
> No, we've been going from protocol 0.5 (report max/min coordinates) to
> protocol A.5 (report finger positions, often with correct track ID). My
> argument is that it's not because we are half-way to B, by chance, that
> we should go up to it. We do just the minimum to respect the minimum
> protocol. Once the driver respects that protocol, all the fancy stuff
> has to stay in userspace. There is already mtdev (I'm sure I don't have
> to tell you ;-) ), I don't see the point of doing some copy-pasting.


Without this patch, the driver reports two points, the lower-left and
upper-right of a rectangle. With this patch, the driver reports two points,
which is either equal to the two actual fingers, or, after resting the fingers
horizontally, two random opposite corners of a rectangle.

Doing userspace tracking without the patch results in properly following the
lower-left and upper-right corners of a triangle. Doing userspace tracking with
the patch results in properly following two random opposite corners of a rectangle.

Without this patch, synaptics shows jerky behavior. With this patch, synaptics
works a bit better.

The above tells me that the MT situation did not improve much, and that the
major improvement is to paper over the synaptics problem.

The argument to go forward implementing protocol B is that the current patch
actually does proper two-finger tracking to the extent that it is possible.
Since mtdev cannot improve the fact that this device does not following fingers
but corners, it makes sense to treat this device specially, and implement the
extra lines in the kernel to make it as good as it can be. Alternatively, one
can give up the idea of using MT in this driver altogether, and just implement
the mean position ABS_X/Y, old-style.

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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 02-08-10 13:46, Henrik Rydberg schreef:
> On 08/02/2010 01:33 PM, Éric Piel wrote:
:
>>> The implemented part could also be done in userspace. Going half-way just to
>>> circumvent buggy behavior in synaptics is really not a good idea.
>> No, we've been going from protocol 0.5 (report max/min coordinates) to
>> protocol A.5 (report finger positions, often with correct track ID). My
>> argument is that it's not because we are half-way to B, by chance, that
>> we should go up to it. We do just the minimum to respect the minimum
>> protocol. Once the driver respects that protocol, all the fancy stuff
>> has to stay in userspace. There is already mtdev (I'm sure I don't have
>> to tell you ;-) ), I don't see the point of doing some copy-pasting.
> 
> 
> Without this patch, the driver reports two points, the lower-left and
> upper-right of a rectangle. With this patch, the driver reports two points,
> which is either equal to the two actual fingers, or, after resting the fingers
> horizontally, two random opposite corners of a rectangle.
Yes, exactly.

> Doing userspace tracking without the patch results in properly following the
> lower-left and upper-right corners of a triangle. Doing userspace tracking with
> the patch results in properly following two random opposite corners of a rectangle.
Yes, with "random opposite" being "quite often the actual corners where
the fingers are".

> Without this patch, synaptics shows jerky behavior. With this patch, synaptics
> works a bit better.
No. This patch, doing the tracking, doesn't change at all the behaviour
with synaptics.

Synaptics already detects the transitions between singletap/doubletap,
and avoid cursor jumps. So tracking the finger doesn't help. The only
patch that ever helped was the one in the subject of this email:
reporting the double touch in ST-protocol as the middle of the two
touches. That helps a lot when you do scrolling with one finger fixed
and one finger moving (because without it half of the time nothing
happens). If your gesture is to move both fingers at the same time, it
has always worked fine.

The tracking patch improves cursor jumps for other userspace apps which
do not detect singletap/doubletap transitions... but in practice I don't
know any other app that read the touchpad events! So that's not a big
improvement ;-)

> The above tells me that the MT situation did not improve much, and that the
> major improvement is to paper over the synaptics problem.
For me it has changed because my argument was "with two finger we report
crap anyway, so let's report the average". With the tracking, we
actually (often) report the real position of the fingers, so this
argument is not valid anymore. Now we can happily say we conform to the
official protocols: ST (which apparently requires to always reports an
actual position), and MT-A.


> The argument to go forward implementing protocol B is that the current patch
> actually does proper two-finger tracking to the extent that it is possible.
> Since mtdev cannot improve the fact that this device does not following fingers
> but corners, it makes sense to treat this device specially, and implement the
> extra lines in the kernel to make it as good as it can be. Alternatively, one
> can give up the idea of using MT in this driver altogether, and just implement
> the mean position ABS_X/Y, old-style.
Do you think that if we implement full tracking in kernel, we can do
_better_ than the small tracking + mtdev? I don't think so (but, hey,
I've been tinkering with multitouch for something like 20h in my life,
so I might have missed something ;-) ). If you have in mind a better
performing algorithm, then let me know, and I'll happily implement it in
kernel :-)

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 02:13 PM, Éric Piel wrote:

> Op 02-08-10 13:46, Henrik Rydberg schreef:
>> On 08/02/2010 01:33 PM, Éric Piel wrote:
> :
>>>> The implemented part could also be done in userspace. Going half-way just to
>>>> circumvent buggy behavior in synaptics is really not a good idea.
>>> No, we've been going from protocol 0.5 (report max/min coordinates) to
>>> protocol A.5 (report finger positions, often with correct track ID). My
>>> argument is that it's not because we are half-way to B, by chance, that
>>> we should go up to it. We do just the minimum to respect the minimum
>>> protocol. Once the driver respects that protocol, all the fancy stuff
>>> has to stay in userspace. There is already mtdev (I'm sure I don't have
>>> to tell you ;-) ), I don't see the point of doing some copy-pasting.
>>
>>
>> Without this patch, the driver reports two points, the lower-left and
>> upper-right of a rectangle. With this patch, the driver reports two points,
>> which is either equal to the two actual fingers, or, after resting the fingers
>> horizontally, two random opposite corners of a rectangle.
> Yes, exactly.
> 
>> Doing userspace tracking without the patch results in properly following the
>> lower-left and upper-right corners of a triangle. Doing userspace tracking with
>> the patch results in properly following two random opposite corners of a rectangle.
> Yes, with "random opposite" being "quite often the actual corners where
> the fingers are".


Right. It is not very compelling for someone actually trying to use the MT
points to anything useful.

>> Without this patch, synaptics shows jerky behavior. With this patch, synaptics
>> works a bit better.
> No. This patch, doing the tracking, doesn't change at all the behaviour
> with synaptics.
> 
> Synaptics already detects the transitions between singletap/doubletap,
> and avoid cursor jumps. So tracking the finger doesn't help. The only
> patch that ever helped was the one in the subject of this email:
> reporting the double touch in ST-protocol as the middle of the two
> touches. That helps a lot when you do scrolling with one finger fixed
> and one finger moving (because without it half of the time nothing
> happens). If your gesture is to move both fingers at the same time, it
> has always worked fine.


So "this patch" got intermingled. The part that you describe is the part that I
was referring to.

> The tracking patch improves cursor jumps for other userspace apps which
> do not detect singletap/doubletap transitions... but in practice I don't
> know any other app that read the touchpad events! So that's not a big
> improvement ;-)


Right.

>> The above tells me that the MT situation did not improve much, and that the
>> major improvement is to paper over the synaptics problem.
> For me it has changed because my argument was "with two finger we report
> crap anyway, so let's report the average". With the tracking, we
> actually (often) report the real position of the fingers, so this
> argument is not valid anymore. Now we can happily say we conform to the
> official protocols: ST (which apparently requires to always reports an
> actual position), and MT-A.


It does not comply with MT-A as long as two semi-random corners of a rectangle
are reported.

>> The argument to go forward implementing protocol B is that the current patch
>> actually does proper two-finger tracking to the extent that it is possible.
>> Since mtdev cannot improve the fact that this device does not following fingers
>> but corners, it makes sense to treat this device specially, and implement the
>> extra lines in the kernel to make it as good as it can be. Alternatively, one
>> can give up the idea of using MT in this driver altogether, and just implement
>> the mean position ABS_X/Y, old-style.
> Do you think that if we implement full tracking in kernel, we can do
> _better_ than the small tracking + mtdev? I don't think so (but, hey,
> I've been tinkering with multitouch for something like 20h in my life,
> so I might have missed something ;-) ). If you have in mind a better
> performing algorithm, then let me know, and I'll happily implement it in
> kernel :-)


We can do better in the sense that since the device cannot comply with MT-A, we
can do as close as possible to MT-B with less cpu usage. But frankly, the best
solution at this stage seems to be to drop MT handling altogether, since there
does not seem to be any plan to use it.

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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 02-08-10 14:29, Henrik Rydberg schreef:
> On 08/02/2010 02:13 PM, Éric Piel wrote:
:
>>> The above tells me that the MT situation did not improve much, and that the
>>> major improvement is to paper over the synaptics problem.
>> For me it has changed because my argument was "with two finger we report
>> crap anyway, so let's report the average". With the tracking, we
>> actually (often) report the real position of the fingers, so this
>> argument is not valid anymore. Now we can happily say we conform to the
>> official protocols: ST (which apparently requires to always reports an
>> actual position), and MT-A.
> 
> 
> It does not comply with MT-A as long as two semi-random corners of a rectangle
> are reported.
Fair enough. Without the tracking patch it complied with MT-A 50% of the
time, with it it's 75% of the time... I guess it's still not called
"complying" ;-)


>>> The argument to go forward implementing protocol B is that the current patch
>>> actually does proper two-finger tracking to the extent that it is possible.
>>> Since mtdev cannot improve the fact that this device does not following fingers
>>> but corners, it makes sense to treat this device specially, and implement the
>>> extra lines in the kernel to make it as good as it can be. Alternatively, one
>>> can give up the idea of using MT in this driver altogether, and just implement
>>> the mean position ABS_X/Y, old-style.
>> Do you think that if we implement full tracking in kernel, we can do
>> _better_ than the small tracking + mtdev? I don't think so (but, hey,
>> I've been tinkering with multitouch for something like 20h in my life,
>> so I might have missed something ;-) ). If you have in mind a better
>> performing algorithm, then let me know, and I'll happily implement it in
>> kernel :-)
> 
> 
> We can do better in the sense that since the device cannot comply with MT-A, we
> can do as close as possible to MT-B with less cpu usage. But frankly, the best
> solution at this stage seems to be to drop MT handling altogether, since there
> does not seem to be any plan to use it.
Ah, this alternative solution is a bit boring ;-) In addition it's not
compatible with my own master plan to eventually have the "pinch to
zoom" and "rotate to rotate" gestures work on my laptop, like in Windows ;-)

So, let's agree on just the two patches I'm going to send to support
"better than nothing" MT-protocol for now. When, later, someone has
better ideas, he can send patches to improve MT-protocol
support/compliance. Fine?

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 02:46 PM, Éric Piel wrote:

[...]

>>
>> We can do better in the sense that since the device cannot comply with MT-A, we
>> can do as close as possible to MT-B with less cpu usage. But frankly, the best
>> solution at this stage seems to be to drop MT handling altogether, since there
>> does not seem to be any plan to use it.
> Ah, this alternative solution is a bit boring ;-) In addition it's not
> compatible with my own master plan to eventually have the "pinch to
> zoom" and "rotate to rotate" gestures work on my laptop, like in Windows ;-)


Since rotational symmetry is what prevents us from implementing proper MT
support, it is not likely going to work anyways.

> So, let's agree on just the two patches I'm going to send to support
> "better than nothing" MT-protocol for now. When, later, someone has
> better ideas, he can send patches to improve MT-protocol
> support/compliance. Fine?


No. I think it is fair to say we tried to make it work properly, but the (known)
information about the device is simply not enough. As long as MT does not work
properly, there is no point in adding it at all - in particular not an elaborate
solution. I can see that zoom would work even with a poor MT implementation, but
that is not what the MT protocol is about. Maybe one day someone will find the
missing information somewhere in the packets.

For your usecase, perhaps one should add ABS_ZOOM etcetera instead, and patch up
synaptics to use those values.

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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 02-08-10 15:03, Henrik Rydberg schreef:
> On 08/02/2010 02:46 PM, Éric Piel wrote:
> 
> [...]
> 
>>>
>>> We can do better in the sense that since the device cannot comply with MT-A, we
>>> can do as close as possible to MT-B with less cpu usage. But frankly, the best
>>> solution at this stage seems to be to drop MT handling altogether, since there
>>> does not seem to be any plan to use it.
>> Ah, this alternative solution is a bit boring ;-) In addition it's not
>> compatible with my own master plan to eventually have the "pinch to
>> zoom" and "rotate to rotate" gestures work on my laptop, like in Windows ;-)
> 
> 
> Since rotational symmetry is what prevents us from implementing proper MT
> support, it is not likely going to work anyways.
Well, yes, in Windows to do the rotation gesture, they "suggest" you to
keep one finger on the bottom left, and rotate only the other finger.
It's kind of fake, but it works perfectly in practice. You just have to
know about it.

>> So, let's agree on just the two patches I'm going to send to support
>> "better than nothing" MT-protocol for now. When, later, someone has
>> better ideas, he can send patches to improve MT-protocol
>> support/compliance. Fine?
> 
> 
> No. I think it is fair to say we tried to make it work properly, but the (known)
> information about the device is simply not enough. As long as MT does not work
> properly, there is no point in adding it at all - in particular not an elaborate
> solution. I can see that zoom would work even with a poor MT implementation, but
> that is not what the MT protocol is about. Maybe one day someone will find the
> missing information somewhere in the packets.
> 
> For your usecase, perhaps one should add ABS_ZOOM etcetera instead, and patch up
> synaptics to use those values.
Detecting the pinch gesture in-kernel? In addition to me not knowing how
to do gesture recognition, I doubt Dmitry would ever accept such a
patch, and Peter Hutterer would probably not enjoy a lot either the
patch for synaptics.

That's why I like the idea of reporting MT events as well as we can (the
user will know that on his device some gestures are limited), and  the
rest of the stack can stay completely standard.

Eric
--
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 03:23 PM, Éric Piel wrote:

[...]

>> For your usecase, perhaps one should add ABS_ZOOM etcetera instead, and patch up

>> synaptics to use those values.
> Detecting the pinch gesture in-kernel? In addition to me not knowing how
> to do gesture recognition, I doubt Dmitry would ever accept such a
> patch, and Peter Hutterer would probably not enjoy a lot either the
> patch for synaptics.
> 
> That's why I like the idea of reporting MT events as well as we can (the
> user will know that on his device some gestures are limited), and  the
> rest of the stack can stay completely standard.


Using my offering of a different solution as an argument _for_ your solution is
incorrect. It is clear that you want zoom to work, and rotate to almost work. I
disagree with the notion of implementing a fairly complex but still incomplete
MT solution to that end alone. I am sure there is a third alternative here.

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 7/7] elantech: average the two coordinates when 2 fingers

[Dmitry Torokhov]

On Mon, Aug 02, 2010 at 01:22:15PM +0200, Henrik Rydberg wrote:
> On 08/02/2010 01:12 PM, Éric Piel wrote:
> 
> > Op 02-08-10 12:02, Henrik Rydberg schreef:
> >> On 08/02/2010 10:17 AM, Éric Piel wrote:
> >>
> >>> Op 01-08-10 15:57, Henrik Rydberg schreef:
> >>>> On 08/01/2010 01:28 PM, Éric Piel wrote:
> >>> :
> >>>>> I still think that for the very specific use case of scrolling when
> >>>>> pressing one finger and moving up and dow the other one, reporting the
> >>>>> average works better than the first finger. However, I guess this can be
> >>>>> considered just as a drawback of the ST protocol, and fixed in userspace
> >>>>> by using the MT protocol.
> >>>>>
> >>>>> What do you think? Does it look fine to you? Below is the code.
> >>>>
> >>>>
> >>>> I might have lost track of what problem needs to be solved. The current patch
> >>>> seems to implement tracking, but still does not solve the individual MT finger
> >>>> problem. And, it uses the same definition of ABS_X/Y as before. I was also under
> >>>> the impression that synaptics needs fixing, anyways. All of this taken together
> >>>> sadly suggests that this patch could just as well be reverted to the original
> >>>> one. Or? Alternatively, one could switch to the type B protocol, since no
> >>>> further tracking improvement is possible in userspace. The implementation is
> >>>> tidy and simple enough, I think.
> >>> Yes, you're right, the patch I've sent was still with the "average of
> >>> the 2 fingers", but I'm now willing to drop it. With the tracking, at
> >>> least we can keep sending info about a real finger and avoid jumps at
> >>> the transition 1->2, so reporting the first finger might have advantages
> >>> over reporting the average :-) The improvement for the test case can
> >>> just go to userspace.
> >>>
> >>> The tracking is still not so clever, so it's definitly not adapted to a
> >>> type B MT protocol (think transition 2->1).
> >>
> >>
> >> You need to add the tracking id and a couple of lines, but i do not see why the
> >> 2->1 transition would be treated any differently. The one-finger coordinate
> >> would be close to either position[0] or position[1], which would determine the
> >> tracking id to keep. Every time you add a finger you add a new tracking id. What
> >> is your planned support for three fingers?
> > Yes, yes, it's probably fairly easy to do some kind of tracking. But I
> > think that as long as the hardware does not provide such a thing, it's
> > better to do the minimum in kernel space, just enough to be meaningful,
> > and leave the rest to userspace.
> 
> 
> The implemented part could also be done in userspace. Going half-way just to
> circumvent buggy behavior in synaptics is really not a good idea.
> 

What synaptics you are talking about here? Userspace driver or the
in-kernel synaptics support? If the latter, then the device is not truly
multitouch device (latest versions of the hardware/firmware aside) as it
is capable of reportiong only one set of coordinates (X/Y) in addition
to number of fingers on the surface.

-- 
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 7/7] elantech: average the two coordinates when 2 fingers

[Dmitry Torokhov]

On Mon, Aug 02, 2010 at 03:03:15PM +0200, Henrik Rydberg wrote:
> On 08/02/2010 02:46 PM, Éric Piel wrote:
> 
> [...]
> 
> >>
> >> We can do better in the sense that since the device cannot comply with MT-A, we
> >> can do as close as possible to MT-B with less cpu usage. But frankly, the best
> >> solution at this stage seems to be to drop MT handling altogether, since there
> >> does not seem to be any plan to use it.
> > Ah, this alternative solution is a bit boring ;-) In addition it's not
> > compatible with my own master plan to eventually have the "pinch to
> > zoom" and "rotate to rotate" gestures work on my laptop, like in Windows ;-)
> 
> 
> Since rotational symmetry is what prevents us from implementing proper MT
> support, it is not likely going to work anyways.
> 

Not work at all or not work as good as with other devices? "Not as good"
is still OK, hardware is different and we have to make the best of it.
The same elantech does not export pressure (at lest the version in
mainline does not) and thus users are unable to adjust sensitivity.
Still the device is useable with Synaptics X.

> > So, let's agree on just the two patches I'm going to send to support
> > "better than nothing" MT-protocol for now. When, later, someone has
> > better ideas, he can send patches to improve MT-protocol
> > support/compliance. Fine?
> 
> 
> No. I think it is fair to say we tried to make it work properly, but the (known)
> information about the device is simply not enough. As long as MT does not work
> properly, there is no point in adding it at all - in particular not an elaborate
> solution. I can see that zoom would work even with a poor MT implementation, but
> that is not what the MT protocol is about. Maybe one day someone will find the
> missing information somewhere in the packets.
> 
> For your usecase, perhaps one should add ABS_ZOOM etcetera instead, and patch up
> synaptics to use those values.

No, I do not believe that this is the direcion we should be taking. I'd
prefer we export as much information as we can for MT to be useful. And
if we discover some missing pieces in proocol (or future firmware
revisioons will be smarter) we can improve MT support.

-- 
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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 06:26 PM, Dmitry Torokhov wrote:
> On Mon, Aug 02, 2010 at 01:22:15PM +0200, Henrik Rydberg wrote:
>> On 08/02/2010 01:12 PM, Éric Piel wrote:
>>
>>> Op 02-08-10 12:02, Henrik Rydberg schreef:
>>>> On 08/02/2010 10:17 AM, Éric Piel wrote:
>>>>
>>>>> Op 01-08-10 15:57, Henrik Rydberg schreef:
>>>>>> On 08/01/2010 01:28 PM, Éric Piel wrote:
>>>>> :
>>>>>>> I still think that for the very specific use case of scrolling when
>>>>>>> pressing one finger and moving up and dow the other one, reporting the
>>>>>>> average works better than the first finger. However, I guess this can be
>>>>>>> considered just as a drawback of the ST protocol, and fixed in userspace
>>>>>>> by using the MT protocol.
>>>>>>>
>>>>>>> What do you think? Does it look fine to you? Below is the code.
>>>>>>
>>>>>>
>>>>>> I might have lost track of what problem needs to be solved. The current patch
>>>>>> seems to implement tracking, but still does not solve the individual MT finger
>>>>>> problem. And, it uses the same definition of ABS_X/Y as before. I was also under
>>>>>> the impression that synaptics needs fixing, anyways. All of this taken together
>>>>>> sadly suggests that this patch could just as well be reverted to the original
>>>>>> one. Or? Alternatively, one could switch to the type B protocol, since no
>>>>>> further tracking improvement is possible in userspace. The implementation is
>>>>>> tidy and simple enough, I think.
>>>>> Yes, you're right, the patch I've sent was still with the "average of
>>>>> the 2 fingers", but I'm now willing to drop it. With the tracking, at
>>>>> least we can keep sending info about a real finger and avoid jumps at
>>>>> the transition 1->2, so reporting the first finger might have advantages
>>>>> over reporting the average :-) The improvement for the test case can
>>>>> just go to userspace.
>>>>>
>>>>> The tracking is still not so clever, so it's definitly not adapted to a
>>>>> type B MT protocol (think transition 2->1).
>>>>
>>>>
>>>> You need to add the tracking id and a couple of lines, but i do not see why the
>>>> 2->1 transition would be treated any differently. The one-finger coordinate
>>>> would be close to either position[0] or position[1], which would determine the
>>>> tracking id to keep. Every time you add a finger you add a new tracking id. What
>>>> is your planned support for three fingers?
>>> Yes, yes, it's probably fairly easy to do some kind of tracking. But I
>>> think that as long as the hardware does not provide such a thing, it's
>>> better to do the minimum in kernel space, just enough to be meaningful,
>>> and leave the rest to userspace.
>>
>>
>> The implemented part could also be done in userspace. Going half-way just to
>> circumvent buggy behavior in synaptics is really not a good idea.
>>
> 
> What synaptics you are talking about here? Userspace driver or the
> in-kernel synaptics support? If the latter, then the device is not truly
> multitouch device (latest versions of the hardware/firmware aside) as it
> is capable of reportiong only one set of coordinates (X/Y) in addition
> to number of fingers on the surface.
> 

I was referring to the user-space driver.

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 7/7] elantech: average the two coordinates when 2 fingers

[Henrik Rydberg]

On 08/02/2010 06:39 PM, Dmitry Torokhov wrote:
> On Mon, Aug 02, 2010 at 03:03:15PM +0200, Henrik Rydberg wrote:
>> On 08/02/2010 02:46 PM, Éric Piel wrote:
>>
>> [...]
>>
>>>>
>>>> We can do better in the sense that since the device cannot comply with MT-A, we
>>>> can do as close as possible to MT-B with less cpu usage. But frankly, the best
>>>> solution at this stage seems to be to drop MT handling altogether, since there
>>>> does not seem to be any plan to use it.
>>> Ah, this alternative solution is a bit boring ;-) In addition it's not
>>> compatible with my own master plan to eventually have the "pinch to
>>> zoom" and "rotate to rotate" gestures work on my laptop, like in Windows ;-)
>>
>>
>> Since rotational symmetry is what prevents us from implementing proper MT
>> support, it is not likely going to work anyways.
>>
> 
> Not work at all or not work as good as with other devices? "Not as good"
> is still OK, hardware is different and we have to make the best of it.
> The same elantech does not export pressure (at lest the version in
> mainline does not) and thus users are unable to adjust sensitivity.
> Still the device is useable with Synaptics X.

This whole thread started off as a question how the ABS_X/Y should be reported
in the presence of two fingers. Given that the elantech driver can report _some_
point via ABS_X/Y, and the number of fingers, the device should be usable with
Synaptics X. If there is a problem with a jumping cursor, I have a feeling this
is a bug that should be fixed in Synaptics X. As far as I can tell, the MT
events are not needed for this case.

>>> So, let's agree on just the two patches I'm going to send to support
>>> "better than nothing" MT-protocol for now. When, later, someone has
>>> better ideas, he can send patches to improve MT-protocol
>>> support/compliance. Fine?
>>
>>
>> No. I think it is fair to say we tried to make it work properly, but the (known)
>> information about the device is simply not enough. As long as MT does not work
>> properly, there is no point in adding it at all - in particular not an elaborate
>> solution. I can see that zoom would work even with a poor MT implementation, but
>> that is not what the MT protocol is about. Maybe one day someone will find the
>> missing information somewhere in the packets.
>>
>> For your usecase, perhaps one should add ABS_ZOOM etcetera instead, and patch up
>> synaptics to use those values.
> 
> No, I do not believe that this is the direcion we should be taking. I'd
> prefer we export as much information as we can for MT to be useful. And
> if we discover some missing pieces in proocol (or future firmware
> revisioons will be smarter) we can improve MT support.
> 

That is fine per se, I was simply looking for other options. The question then
is how much MT support is needed. For existing applications (read Synaptics X),
there should be no need for MT, so we should be able to wait with that part
until the firmware gets smarter. If the idea is to use something like the
Multitouch X Driver to enable zoom and rotate, this is where I am hesitant,
given the current capabilities of the device.

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 7/7] elantech: average the two coordinates when 2 fingers

[Éric Piel]

Op 02-08-10 18:39, Dmitry Torokhov schreef:
> No, I do not believe that this is the direcion we should be taking. I'd
> prefer we export as much information as we can for MT to be useful. And
> if we discover some missing pieces in proocol (or future firmware
> revisioons will be smarter) we can improve MT support.
> 
Hi Dimtry,

So I've implemented support for MT, with tracking to report as best as
possible the position of the 2 fingers and avoid jumps in the ST protocol.

So, please drop this 7/7 patch of this series, and instead apply the 4
coming patches 7 -> 10. Hopefully you manage to follow, otherwise just
tell me and I'll just resend the whole 10 patch series.

That's still against 2.6.35, but there shouldn't be any conflict with
the HEAD of your tree.

Cheers,
Eric

[PATCH 07/10] elantech: Report multitouch with proper ABS_MT messages

[Éric Piel]

Multitouch info was reported only via a old protocol used by the
proprietary X driver from elantech. Let's report the multitouch info
also following the official MT protocol.

This was done following the multi-touch-protocol.txt documentation, and
inspired by the bcm5974 implementation. Testing was light as there is
not many applications using this protocol yet, but the X synaptics
driver didn't complain and the X multitouch driver behaved correctly.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   23 +++++++++++++++++++----
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index c84f741..7fa4f29 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -271,14 +271,20 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		 * byte 1:  .   .   .   .   .  x10 x9  x8
 		 * byte 2: x7  x6  x5  x4  x4  x2  x1  x0
 		 */
-		input_report_abs(dev, ABS_X,
-			((packet[1] & 0x07) << 8) | packet[2]);
+		x1 = ((packet[1] & 0x07) << 8) | packet[2];
 		/*
 		 * byte 4:  .   .   .   .   .   .  y9  y8
 		 * byte 5: y7  y6  y5  y4  y3  y2  y1  y0
 		 */
-		input_report_abs(dev, ABS_Y,
-			ETP_YMAX_V2 - (((packet[4] & 0x03) << 8) | packet[5]));
+		y1 = ETP_YMAX_V2 - (((packet[4] & 0x03) << 8) | packet[5]);
+		/* Multitouch */
+		input_report_abs(dev, ABS_MT_POSITION_X, x1);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y1);
+		input_mt_sync(dev);
+
+		input_report_abs(dev, ABS_X, x1);
+		input_report_abs(dev, ABS_Y, y1);
+
 		pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
 		width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
 		break;
@@ -300,6 +306,13 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		x2 = ((packet[3] & 0x10) << 4) | packet[4];
 		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
 		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
+		/* Multitouch */
+		input_report_abs(dev, ABS_MT_POSITION_X, x1 << 2);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y1 << 2);
+		input_mt_sync(dev);
+		input_report_abs(dev, ABS_MT_POSITION_X, x2 << 2);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y2 << 2);
+		input_mt_sync(dev);
 		/*
 		 * For compatibility with the X Synaptics driver scale up
 		 * one coordinate and report as ordinary mouse movent
@@ -548,6 +561,8 @@ static void elantech_set_input_params(struct psmouse *psmouse)
 			input_set_abs_params(dev, ABS_TOOL_WIDTH, ETP_WMIN_V2,
 					     ETP_WMAX_V2, 0, 0);
 		}
+		input_set_abs_params(dev, ABS_MT_POSITION_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);
+		input_set_abs_params(dev, ABS_MT_POSITION_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);
 		input_set_abs_params(dev, ABS_HAT0X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
 		input_set_abs_params(dev, ABS_HAT0Y, ETP_2FT_YMIN, ETP_2FT_YMAX, 0, 0);
 		input_set_abs_params(dev, ABS_HAT1X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
-- 
1.7.2.1

--
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 08/10] elantech: track finger to distinguish coordinates in 2-finger report

[Éric Piel]

The elantech hardware is limited when reporting two fingers. It just
reports the minimum and maximum coordinates of the rectangle containing
the fingers.

As multitouch protocol requires at least to report the correct position
of each finger, we track the finger position when there is only one
finger to distinguish which corner of the rectangle corresponds to the
actual position of the fingers. When there are three fingers, only the
minimum position is reported, so we just don't track it and hope the
last tracked position has the most likelihood to be correct.

It works fine, at least as long as the two fingers don't get
horizontally or vertically aligned, after what it's back to random.
This also allows to avoid so "jumps" in the single-touch protocol.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   75 ++++++++++++++++++++++++++++++++--------
 drivers/input/mouse/elantech.h |    2 +
 2 files changed, 62 insertions(+), 15 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 7fa4f29..1dd7e7c 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -242,6 +242,45 @@ static void elantech_report_absolute_v1(struct psmouse *psmouse)
 	input_sync(dev);
 }
 
+/* Store the current position of the finger, for tracking when 2 fingers */
+static void track_one_finger(struct elantech_data *etd, unsigned int x, unsigned int y)
+{
+	etd->prev_x = x;
+	etd->prev_y = y;
+}
+
+/* Ensure that argument a contains a value closest to ref than b */
+static void find_closest_and_swap(unsigned int *ref, unsigned int *a, unsigned int *b)
+{
+	int dist_a = abs(*ref - *a);
+	int dist_b = abs(*ref - *b);
+	int tmp;
+	if (dist_b < dist_a) {
+		tmp = *a;
+		*a = *b;
+		*b = tmp;
+	}
+	*ref = *a;
+}
+
+/*
+ * Track the coordinates reported and swap the values to improve the likelihood
+ * that the coordinates correspond to the real positions of the two fingers.
+ */
+static void
+track_two_fingers(struct elantech_data *etd,
+		  unsigned int *x1, unsigned int *y1, unsigned int *x2, unsigned int *y2)
+{
+	/*
+	 * As the hardware can only report the coordinates without knowing which one
+	 * corresponds to which finger, we try to guess according to the last known
+	 * position, when using only one finger.
+	 */
+	find_closest_and_swap(&etd->prev_x, x1, x2);
+	find_closest_and_swap(&etd->prev_y, y1, y2);
+}
+
+
 /*
  * Interpret complete data packets and report absolute mode input events for
  * hardware version 2. (6 byte packets)
@@ -251,7 +290,7 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 	struct elantech_data *etd = psmouse->private;
 	struct input_dev *dev = psmouse->dev;
 	unsigned char *packet = psmouse->packet;
-	int fingers, x1, y1, x2, y2, width = 0, pres = 0;
+	unsigned int fingers, x1, y1, x2, y2, width = 0, pres = 0;
 
 	/* byte 0: n1  n0   .   .   .   .   R   L */
 	fingers = (packet[0] & 0xc0) >> 6;
@@ -285,6 +324,10 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		input_report_abs(dev, ABS_X, x1);
 		input_report_abs(dev, ABS_Y, y1);
 
+		/* Only track if we are sure it's real finger coodinates */
+		if (fingers == 1)
+			track_one_finger(etd, x1, y1);
+
 		pres = (packet[1] & 0xf0) | ((packet[4] & 0xf0) >> 4);
 		width = ((packet[0] & 0x30) >> 2) | ((packet[3] & 0x30) >> 4);
 		break;
@@ -296,37 +339,39 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		 * byte 0:  .   .  ay8 ax8  .   .   .   .
 		 * byte 1: ax7 ax6 ax5 ax4 ax3 ax2 ax1 ax0
 		 */
-		x1 = ((packet[0] & 0x10) << 4) | packet[1];
+		x1 = (((packet[0] & 0x10) << 4) | packet[1]) << 2;
 		/* byte 2: ay7 ay6 ay5 ay4 ay3 ay2 ay1 ay0 */
-		y1 = ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2]);
+		y1 = (ETP_2FT_YMAX - (((packet[0] & 0x20) << 3) | packet[2])) << 2;
 		/*
 		 * byte 3:  .   .  by8 bx8  .   .   .   .
 		 * byte 4: bx7 bx6 bx5 bx4 bx3 bx2 bx1 bx0
 		 */
-		x2 = ((packet[3] & 0x10) << 4) | packet[4];
+		x2 = (((packet[3] & 0x10) << 4) | packet[4]) << 2;
 		/* byte 5: by7 by8 by5 by4 by3 by2 by1 by0 */
-		y2 = ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5]);
+		y2 = (ETP_2FT_YMAX - (((packet[3] & 0x20) << 3) | packet[5])) << 2;
+
+		track_two_fingers(etd, &x1, &y1, &x2, &y2);
 		/* Multitouch */
-		input_report_abs(dev, ABS_MT_POSITION_X, x1 << 2);
-		input_report_abs(dev, ABS_MT_POSITION_Y, y1 << 2);
+		input_report_abs(dev, ABS_MT_POSITION_X, x1);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y1);
 		input_mt_sync(dev);
-		input_report_abs(dev, ABS_MT_POSITION_X, x2 << 2);
-		input_report_abs(dev, ABS_MT_POSITION_Y, y2 << 2);
+		input_report_abs(dev, ABS_MT_POSITION_X, x2);
+		input_report_abs(dev, ABS_MT_POSITION_Y, y2);
 		input_mt_sync(dev);
 		/*
 		 * For compatibility with the X Synaptics driver scale up
 		 * one coordinate and report as ordinary mouse movent
 		 */
-		input_report_abs(dev, ABS_X, x1 << 2);
-		input_report_abs(dev, ABS_Y, y1 << 2);
+		input_report_abs(dev, ABS_X, x1);
+		input_report_abs(dev, ABS_Y, y1);
 		/*
 		 * For compatibility with the proprietary X Elantech driver
 		 * report both coordinates as hat coordinates
 		 */
-		input_report_abs(dev, ABS_HAT0X, x1);
-		input_report_abs(dev, ABS_HAT0Y, y1);
-		input_report_abs(dev, ABS_HAT1X, x2);
-		input_report_abs(dev, ABS_HAT1Y, y2);
+		input_report_abs(dev, ABS_HAT0X, x1 >> 2);
+		input_report_abs(dev, ABS_HAT0Y, y1 >> 2);
+		input_report_abs(dev, ABS_HAT1X, x2 >> 2);
+		input_report_abs(dev, ABS_HAT1Y, y2 >> 2);
 
 		/* Unknown so just report sensible values */
 		pres = 127;
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index f6e3be5..fa9a6b4 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -111,6 +111,8 @@ struct elantech_data {
 	unsigned char hw_version;
 	unsigned int  fw_version;
 	unsigned char parity[256];
+	unsigned int prev_x;
+	unsigned int prev_y;
 };
 
 enum paritycheck_types {
-- 
1.7.2.1


--
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 09/10] elantech: remove support for proprietary X driver

[Éric Piel]

Apparently somewhere someone had a proprietary X driver. To get the
multitouch info, it uses some hack on the normal API instead of using
the multitouch protocol. Now that the multitouch info is transmitted
correctly it makes not much sense to keep it. Especially because it's
impossible to find this proprietary X driver anywhere, so the number of
user must be pretty low.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   12 ------------
 1 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 1dd7e7c..ba1c3d8 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -364,14 +364,6 @@ static void elantech_report_absolute_v2(struct psmouse *psmouse)
 		 */
 		input_report_abs(dev, ABS_X, x1);
 		input_report_abs(dev, ABS_Y, y1);
-		/*
-		 * For compatibility with the proprietary X Elantech driver
-		 * report both coordinates as hat coordinates
-		 */
-		input_report_abs(dev, ABS_HAT0X, x1 >> 2);
-		input_report_abs(dev, ABS_HAT0Y, y1 >> 2);
-		input_report_abs(dev, ABS_HAT1X, x2 >> 2);
-		input_report_abs(dev, ABS_HAT1Y, y2 >> 2);
 
 		/* Unknown so just report sensible values */
 		pres = 127;
@@ -608,10 +600,6 @@ static void elantech_set_input_params(struct psmouse *psmouse)
 		}
 		input_set_abs_params(dev, ABS_MT_POSITION_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);
 		input_set_abs_params(dev, ABS_MT_POSITION_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);
-		input_set_abs_params(dev, ABS_HAT0X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
-		input_set_abs_params(dev, ABS_HAT0Y, ETP_2FT_YMIN, ETP_2FT_YMAX, 0, 0);
-		input_set_abs_params(dev, ABS_HAT1X, ETP_2FT_XMIN, ETP_2FT_XMAX, 0, 0);
-		input_set_abs_params(dev, ABS_HAT1Y, ETP_2FT_YMIN, ETP_2FT_YMAX, 0, 0);
 		break;
 	}
 }
-- 
1.7.2.1


--
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 10/10] elantech: don't take into account the border size in the calculations

[Éric Piel]

The min and max of the reported ABS_X/Y are slightly smaller than the
theoretical maximum due to the physical border of the touchpad. However,
in the calculations, the maximum is the theoretical maximum. Otherwise
we can end up sending negative coordinates if the user manages to touch
the very borders. In addition, the incorrect number prevented to align
perfectly the coordinates between 1 and 2 touches.

Signed-off-by: Éric Piel <eric.piel@tremplin-utc.net>
---
 drivers/input/mouse/elantech.c |   20 ++++++++++++++------
 drivers/input/mouse/elantech.h |   26 ++++++++++++--------------
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index ba1c3d8..bf95adf 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -584,22 +584,30 @@ static void elantech_set_input_params(struct psmouse *psmouse)
 			__set_bit(BTN_FORWARD, dev->keybit);
 			__set_bit(BTN_BACK, dev->keybit);
 		}
-		input_set_abs_params(dev, ABS_X, ETP_XMIN_V1, ETP_XMAX_V1, 0, 0);
-		input_set_abs_params(dev, ABS_Y, ETP_YMIN_V1, ETP_YMAX_V1, 0, 0);
+		input_set_abs_params(dev, ABS_X, ETP_XMIN_V1 + ETP_EDGE_FUZZ_V1,
+				     ETP_XMAX_V1 - ETP_EDGE_FUZZ_V1, 0, 0);
+		input_set_abs_params(dev, ABS_Y, ETP_YMIN_V1 + ETP_EDGE_FUZZ_V1,
+				     ETP_YMAX_V1 - ETP_EDGE_FUZZ_V1, 0, 0);
 		break;
 
 	case 2:
 		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
-		input_set_abs_params(dev, ABS_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);
-		input_set_abs_params(dev, ABS_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);
+		input_set_abs_params(dev, ABS_X, ETP_XMIN_V2 + ETP_EDGE_FUZZ_V2,
+				     ETP_XMAX_V2 - ETP_EDGE_FUZZ_V2, 0, 0);
+		input_set_abs_params(dev, ABS_Y, ETP_YMIN_V2 + ETP_EDGE_FUZZ_V2,
+				     ETP_YMAX_V2 - ETP_EDGE_FUZZ_V2, 0, 0);
 		if (etd->reports_pres) {
 			input_set_abs_params(dev, ABS_PRESSURE, ETP_PMIN_V2,
 					     ETP_PMAX_V2, 0, 0);
 			input_set_abs_params(dev, ABS_TOOL_WIDTH, ETP_WMIN_V2,
 					     ETP_WMAX_V2, 0, 0);
 		}
-		input_set_abs_params(dev, ABS_MT_POSITION_X, ETP_XMIN_V2, ETP_XMAX_V2, 0, 0);
-		input_set_abs_params(dev, ABS_MT_POSITION_Y, ETP_YMIN_V2, ETP_YMAX_V2, 0, 0);
+		input_set_abs_params(dev, ABS_MT_POSITION_X,
+				     ETP_XMIN_V2 + ETP_EDGE_FUZZ_V2,
+				     ETP_XMAX_V2 - ETP_EDGE_FUZZ_V2, 0, 0);
+		input_set_abs_params(dev, ABS_MT_POSITION_Y,
+				     ETP_YMIN_V2 + ETP_EDGE_FUZZ_V2,
+				     ETP_YMAX_V2 - ETP_EDGE_FUZZ_V2, 0, 0);
 		break;
 	}
 }
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index fa9a6b4..75b0546 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -60,10 +60,10 @@
  */
 #define ETP_EDGE_FUZZ_V1		32
 
-#define ETP_XMIN_V1			(  0 + ETP_EDGE_FUZZ_V1)
-#define ETP_XMAX_V1			(576 - ETP_EDGE_FUZZ_V1)
-#define ETP_YMIN_V1			(  0 + ETP_EDGE_FUZZ_V1)
-#define ETP_YMAX_V1			(384 - ETP_EDGE_FUZZ_V1)
+#define ETP_XMIN_V1			0
+#define ETP_XMAX_V1			576
+#define ETP_YMIN_V1			0
+#define ETP_YMAX_V1			384
 
 /*
  * It seems the resolution for hardware version 2 doubled.
@@ -72,10 +72,10 @@
  */
 #define ETP_EDGE_FUZZ_V2		8
 
-#define ETP_XMIN_V2			(   0 + ETP_EDGE_FUZZ_V2)
-#define ETP_XMAX_V2			(1152 - ETP_EDGE_FUZZ_V2)
-#define ETP_YMIN_V2			(   0 + ETP_EDGE_FUZZ_V2)
-#define ETP_YMAX_V2			( 768 - ETP_EDGE_FUZZ_V2)
+#define ETP_XMIN_V2			0
+#define ETP_XMAX_V2			1152
+#define ETP_YMIN_V2			0
+#define ETP_YMAX_V2			768
 
 #define ETP_PMIN_V2			0
 #define ETP_PMAX_V2			255
@@ -86,12 +86,10 @@
  * For two finger touches the coordinate of each finger gets reported
  * separately but with reduced resolution.
  */
-#define ETP_2FT_FUZZ			4
-
-#define ETP_2FT_XMIN			(  0 + ETP_2FT_FUZZ)
-#define ETP_2FT_XMAX			(288 - ETP_2FT_FUZZ)
-#define ETP_2FT_YMIN			(  0 + ETP_2FT_FUZZ)
-#define ETP_2FT_YMAX			(192 - ETP_2FT_FUZZ)
+#define ETP_2FT_XMIN			0
+#define ETP_2FT_XMAX			288
+#define ETP_2FT_YMIN			0
+#define ETP_2FT_YMAX			192
 
 struct elantech_data {
 	unsigned char reg_10;
-- 
1.7.2.1

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