[PATCH v2 0/4] input: synaptics - report correct width and pressure values

[PATCH v2 0/4] input: synaptics - report correct width and pressure values

[Gabriele Mazzotta]

Make image sensors and cr48 sensors report widths and fix the
calculation of width and pressure values in some cases.

Changes since v1:
 - Rebased on 35393dcb2ed3
 - Get widths from AGM packets too
 - Include support for cr48 sensors
 - Fix pressure values on image sensors

Gabriele Mazzotta (4):
  input: synaptics - fix pressure values calculation on image sensors
  input: synaptics - fix width values calculation on image sensors
  input: synaptics - change default width value of cr48 sensors
  input: synaptics - make semi-mt touchpads report widths

 drivers/input/mouse/synaptics.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

-- 
2.1.4

[PATCH v2 1/4] input: synaptics - fix pressure values calculation on image sensors

[Gabriele Mazzotta]

The pressure values retrieved from secondary packets was incorrectly
shifted, making them lower than what they actually were.
Since this only happened with secondary packets, the values reported
when only one finger was present on the touchpad were correct.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/input/mouse/synaptics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index f89de89..4d22ebd 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -588,7 +588,7 @@ static void synaptics_parse_agm(const unsigned char buf[],
 		agm->w = hw->w;
 		agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
 		agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
-		agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 1;
+		agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 2;
 		break;
 
 	case 2:
-- 
2.1.4

[PATCH v2 2/4] input: synaptics - fix width values calculation on image sensors

[Gabriele Mazzotta]

When multiple fingers are on the touchpad, W reports the finger
count rather than the width. Retrieve the correct value that is
encoded in X, Y and Z of AGM and SGM packets.

The minimum width reported is 8 rather than 4 in this case, while the
maximum remains 15.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/input/mouse/synaptics.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 4d22ebd..918a727 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -585,10 +585,12 @@ static void synaptics_parse_agm(const unsigned char buf[],
 	switch (agm_packet_type) {
 	case 1:
 		/* Gesture packet: (x, y, z) half resolution */
-		agm->w = hw->w;
-		agm->x = (((buf[4] & 0x0f) << 8) | buf[1]) << 1;
-		agm->y = (((buf[4] & 0xf0) << 4) | buf[2]) << 1;
-		agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0f)) << 2;
+		agm->w = ((buf[1] & 0x01) |
+			  ((buf[2] & 0x01) << 1) |
+			  ((buf[5] & 0x01) << 2)) + 8;
+		agm->x = (((buf[4] & 0x0f) << 8) | (buf[1] & 0xfe)) << 1;
+		agm->y = (((buf[4] & 0xf0) << 4) | (buf[2] & 0xfe)) << 1;
+		agm->z = ((buf[3] & 0x30) | (buf[5] & 0x0e)) << 2;
 		break;
 
 	case 2:
@@ -852,6 +854,17 @@ static void synaptics_image_sensor_process(struct psmouse *psmouse,
 	else
 		num_fingers = 4;
 
+	/* When multiple fingers are on the TouchPad, the width is encoded in
+	 * X, Y and Z */
+	if (sgm->w == 0 || sgm->w == 1) {
+		sgm->w = (((sgm->x & BIT(1)) >> 1) |
+			  (sgm->y & BIT(1)) |
+			  ((sgm->z & BIT(0)) << 2)) + 8;
+		sgm->x &= ~BIT(1);
+		sgm->y &= ~BIT(1);
+		sgm->z &= ~BIT(0);
+	}
+
 	/* Send resulting input events to user space */
 	synaptics_report_mt_data(psmouse, sgm, num_fingers);
 }
-- 
2.1.4

[PATCH v2 3/4] input: synaptics - change default width value of cr48 sensors

[Gabriele Mazzotta]

The minimum value these sensors can report is 4, so this should be the
value used when W is not reporting the width.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/input/mouse/synaptics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 918a727..5008e8c 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -910,7 +910,7 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 
 	if (hw.z > 0 && hw.x > 1) {
 		num_fingers = 1;
-		finger_width = 5;
+		finger_width = 4;
 		if (SYN_CAP_EXTENDED(priv->capabilities)) {
 			switch (hw.w) {
 			case 0 ... 1:
-- 
2.1.4

[PATCH v2 4/4] input: synaptics - make semi-mt touchpads report widths

[Gabriele Mazzotta]

Despite claiming to report finger widths, semi-mt touchpads were not
doing it. Make them report widths using ABS_MT_TOUCH_MAJOR instead
of ABS_TOOL_WIDTH.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/input/mouse/synaptics.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index 5008e8c..7b95a0b 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -819,6 +819,7 @@ static void synaptics_report_mt_data(struct psmouse *psmouse,
 		input_report_abs(dev, ABS_MT_POSITION_X, pos[i].x);
 		input_report_abs(dev, ABS_MT_POSITION_Y, pos[i].y);
 		input_report_abs(dev, ABS_MT_PRESSURE, hw[i]->z);
+		input_report_abs(dev, ABS_MT_TOUCH_MAJOR, hw[i]->w);
 	}
 
 	input_mt_drop_unused(dev);
@@ -954,8 +955,14 @@ static void synaptics_process_packet(struct psmouse *psmouse)
 	}
 	input_report_abs(dev, ABS_PRESSURE, hw.z);
 
-	if (SYN_CAP_PALMDETECT(priv->capabilities))
-		input_report_abs(dev, ABS_TOOL_WIDTH, finger_width);
+	if (SYN_CAP_PALMDETECT(priv->capabilities)) {
+		if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c) ||
+		    cr48_profile_sensor)
+			input_set_abs_params(dev,
+					     ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
+		else
+			input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
+	}
 
 	input_report_key(dev, BTN_TOOL_FINGER, num_fingers == 1);
 	if (SYN_CAP_MULTIFINGER(priv->capabilities)) {
-- 
2.1.4

Re: [PATCH v2 0/4] input: synaptics - report correct width and pressure values

[Oliver Neukum]

On Mon, 2015-01-05 at 23:28 +0100, Gabriele Mazzotta wrote:
> Make image sensors and cr48 sensors report widths and fix the
> calculation of width and pressure values in some cases.
> 
> Changes since v1:
>  - Rebased on 35393dcb2ed3
>  - Get widths from AGM packets too
>  - Include support for cr48 sensors
>  - Fix pressure values on image sensors

Hi,

it seems to me that all these fixes should go into stable.

	Regards
		Oliver

Re: [PATCH v2 0/4] input: synaptics - report correct width and pressure values

[Gabriele Mazzotta]

On Monday 05 January 2015 23:28:31 Gabriele Mazzotta wrote:
> Make image sensors and cr48 sensors report widths and fix the
> calculation of width and pressure values in some cases.
> 
> Changes since v1:
>  - Rebased on 35393dcb2ed3
>  - Get widths from AGM packets too
>  - Include support for cr48 sensors
>  - Fix pressure values on image sensors
> 
> Gabriele Mazzotta (4):
>   input: synaptics - fix pressure values calculation on image sensors
>   input: synaptics - fix width values calculation on image sensors
>   input: synaptics - change default width value of cr48 sensors
>   input: synaptics - make semi-mt touchpads report widths
> 
>  drivers/input/mouse/synaptics.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> 

Hi,

can this series be reviewed?

Regards,
Gabriele

Re: [PATCH v2 0/4] input: synaptics - report correct width and pressure values

[Gabriele Mazzotta]

It was pointed out that I made a terrible mistake in 4/4 [1] of this series
and I'm going to fix it. But I have a question first.

Is there any reason why the pointer emulation takes care of ABS_PRESSURE,
but not ABS_TOOL_WIDTH?
I know for example that xf86-input-synaptics relies on both (at least the
current stable release) for the palm detection and there might be some
other userspace applications relying on it. If there's no specific reason,
I could include in this series the changes to make input-mt.c send also
ABS_TOOL_WIDTH.

Regards,
Gabriele

[1] https://bugzilla.kernel.org/show_bug.cgi?id=77161#c17