[PATCH v3 0/5] input: synaptics - report correct width and pressure values

[PATCH v3 0/5] input: synaptics - report correct width and pressure values

[Gabriele Mazzotta]

Hi,

I updated the series fixing the error reported by Shunsuke Shimizu that
I made when I rebased v1. I've also included a change to make input
devices correctly report their capabilities and included a change to
make MT devices report widths as ABS_TOOL_WIDTH.

Gabriele Mazzotta (5):
  input: synaptics - fix width values calculation on image sensors
  input: synaptics - change default width value of cr48 sensors
  input: synaptics - setup devices depending on their capabilities
  input: synaptics - make image sensors and cr48 sensors report widths
  input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation

 drivers/input/input-mt.c        | 12 +++++++--
 drivers/input/mouse/synaptics.c | 55 +++++++++++++++++++++++++++--------------
 2 files changed, 46 insertions(+), 21 deletions(-)

-- 
2.1.4

[PATCH v3 1/5] input: synaptics - fix width values calculation on image sensors

[Gabriele Mazzotta]

When multiple fingers are on the touchpad, W holds 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 d73a942..133e488 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -661,10 +661,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)) << 1;
+		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:
@@ -960,6 +962,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 v3 2/5] 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 133e488..a7a0e73 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1018,7 +1018,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

Re: [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors

[Benjamin Tissoires]

On Sun, Mar 22, 2015 at 10:43 AM, Gabriele Mazzotta
<gabriele.mzt@gmail.com> wrote:
> 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 133e488..a7a0e73 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -1018,7 +1018,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;

I am not sure about this change. It looks benign, but I don't get how
changing the local variable finger_width can change anything in the
CR48 processing.

Except for this one (which could be dropped IMO), the *rest* of the series is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Thanks for pushing this Gabriele.

Cheers,
Benjamin

>                 if (SYN_CAP_EXTENDED(priv->capabilities)) {
>                         switch (hw.w) {
>                         case 0 ... 1:
> --
> 2.1.4
>

Re: [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors

[Gabriele Mazzotta]

On Monday 23 March 2015 16:48:15 Benjamin Tissoires wrote:
> On Sun, Mar 22, 2015 at 10:43 AM, Gabriele Mazzotta
> <gabriele.mzt@gmail.com> wrote:
> > 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 133e488..a7a0e73 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -1018,7 +1018,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;
> 
> I am not sure about this change. It looks benign, but I don't get how
> changing the local variable finger_width can change anything in the
> CR48 processing.

I fail to remember why I did this exactly. I think the reason why I did this
was that before e9e8520f229b ("Input: synaptics - use in-kernel tracking for
reporting mt data") cr48 sensors used ABS_TOOL_WIDTH for all the fingers. If
you had two fingers on the touchpad it was possible for the kernel to emit
fake width variations in case you lifted one (5->4) and brought it back
on the touchpad (4->5). I'm actually not sure of this as I have an Image
Sensor, but even if it's correct, it should no longer be a problem since
multiple slots are used and so 5 would only be used for the second slot.

> Except for this one (which could be dropped IMO), the *rest* of the series is:
> Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Thanks for pushing this Gabriele.
> 
> Cheers,
> Benjamin
> 
> >                 if (SYN_CAP_EXTENDED(priv->capabilities)) {
> >                         switch (hw.w) {
> >                         case 0 ... 1:
> > --
> > 2.1.4
> >

[PATCH v3 3/5] input: synaptics - setup devices depending on their capabilities

[Gabriele Mazzotta]

ABS_X, ABS_Y and ABS_PRESSURE were defined for all the devices, even
if not needed. Fix this by configuring each device depending on its
capabilities.

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

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index a7a0e73..ff47084 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -1188,11 +1188,6 @@ static void set_input_params(struct psmouse *psmouse,
 
 	/* Absolute mode */
 	__set_bit(EV_ABS, dev->evbit);
-	set_abs_position_params(dev, priv, ABS_X, ABS_Y);
-	input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
-
-	if (cr48_profile_sensor)
-		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
 
 	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
 		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
@@ -1200,26 +1195,32 @@ static void set_input_params(struct psmouse *psmouse,
 		/* Image sensors can report per-contact pressure */
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
 		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
-
 		/* Image sensors can signal 4 and 5 finger clicks */
 		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
 		__set_bit(BTN_TOOL_QUINTTAP, dev->keybit);
-	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+	} else if (cr48_profile_sensor) {
 		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
 					ABS_MT_POSITION_Y);
+		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
 		/*
 		 * Profile sensor in CR-48 tracks contacts reasonably well,
 		 * other non-image sensors with AGM use semi-mt.
 		 */
+		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
+	} else if (SYN_CAP_ADV_GESTURE(priv->ext_cap_0c)) {
+		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
+					ABS_MT_POSITION_Y);
+		input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
 		input_mt_init_slots(dev, 2,
-				    INPUT_MT_POINTER |
-				    (cr48_profile_sensor ?
-					INPUT_MT_TRACK : INPUT_MT_SEMI_MT));
+				    INPUT_MT_POINTER | INPUT_MT_SEMI_MT);
+	} else {
+		set_abs_position_params(dev, priv, ABS_X, ABS_Y);
+		input_set_abs_params(dev, ABS_PRESSURE, 0, 255, 0, 0);
+		if (SYN_CAP_PALMDETECT(priv->capabilities))
+			input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
 	}
 
-	if (SYN_CAP_PALMDETECT(priv->capabilities))
-		input_set_abs_params(dev, ABS_TOOL_WIDTH, 0, 15, 0, 0);
-
 	__set_bit(BTN_TOUCH, dev->keybit);
 	__set_bit(BTN_TOOL_FINGER, dev->keybit);
 
-- 
2.1.4

[PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths

[Gabriele Mazzotta]

The driver was not reporting widths for image sensors and cr48 sensors
despite it was calculating them.

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

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index ff47084..4e86ba6 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -927,6 +927,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);
@@ -1192,8 +1193,9 @@ static void set_input_params(struct psmouse *psmouse,
 	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
 		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
 					ABS_MT_POSITION_Y);
-		/* Image sensors can report per-contact pressure */
+		/* Image sensors can report per-contact pressure and width */
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
 		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
 		/* Image sensors can signal 4 and 5 finger clicks */
 		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
@@ -1202,6 +1204,7 @@ static void set_input_params(struct psmouse *psmouse,
 		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
 					ABS_MT_POSITION_Y);
 		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
+		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
 		/*
 		 * Profile sensor in CR-48 tracks contacts reasonably well,
 		 * other non-image sensors with AGM use semi-mt.
-- 
2.1.4

Re: [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths

[Dmitry Torokhov]

HI Gabriele,

On Sun, Mar 22, 2015 at 03:43:55PM +0100, Gabriele Mazzotta wrote:
> The driver was not reporting widths for image sensors and cr48 sensors
> despite it was calculating them.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
>  drivers/input/mouse/synaptics.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index ff47084..4e86ba6 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -927,6 +927,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);

As I mentioned in the other email ABS_MT_TOUCH_MAJOR should use surface
units for reporting.

>  	}
>  
>  	input_mt_drop_unused(dev);
> @@ -1192,8 +1193,9 @@ static void set_input_params(struct psmouse *psmouse,
>  	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
>  		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
>  					ABS_MT_POSITION_Y);
> -		/* Image sensors can report per-contact pressure */
> +		/* Image sensors can report per-contact pressure and width */
>  		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
>  		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
>  		/* Image sensors can signal 4 and 5 finger clicks */
>  		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> @@ -1202,6 +1204,7 @@ static void set_input_params(struct psmouse *psmouse,
>  		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
>  					ABS_MT_POSITION_Y);
>  		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
>  		/*
>  		 * Profile sensor in CR-48 tracks contacts reasonably well,
>  		 * other non-image sensors with AGM use semi-mt.
> -- 
> 2.1.4
> 

Thanks.

-- 
Dmitry

Re: [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths

[Gabriele Mazzotta]

On Thursday 11 June 2015 17:40:14 Dmitry Torokhov wrote:
> HI Gabriele,
> 
> On Sun, Mar 22, 2015 at 03:43:55PM +0100, Gabriele Mazzotta wrote:
> > The driver was not reporting widths for image sensors and cr48 sensors
> > despite it was calculating them.
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=77161
> > Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> > ---
> >  drivers/input/mouse/synaptics.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> > index ff47084..4e86ba6 100644
> > --- a/drivers/input/mouse/synaptics.c
> > +++ b/drivers/input/mouse/synaptics.c
> > @@ -927,6 +927,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);
> 
> As I mentioned in the other email ABS_MT_TOUCH_MAJOR should use surface
> units for reporting.

Sorry, I didn't notice this difference between ABS_MT_TOUCH_MAJOR and
ABS_TOOL_WIDTH. This makes things a bit more complicated as I don't
see any obvious way to convert these values.

This means that also hid-rmi.c should do some sort of conversion since
Synaptics touchpads using it (such as the one of my laptop) use
ABS_MT_TOUCH_MAJOR to report abstract values that are between 0 and 15.

Gabriele

> >  	}
> >  
> >  	input_mt_drop_unused(dev);
> > @@ -1192,8 +1193,9 @@ static void set_input_params(struct psmouse *psmouse,
> >  	if (SYN_CAP_IMAGE_SENSOR(priv->ext_cap_0c)) {
> >  		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> >  					ABS_MT_POSITION_Y);
> > -		/* Image sensors can report per-contact pressure */
> > +		/* Image sensors can report per-contact pressure and width */
> >  		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> > +		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> >  		input_mt_init_slots(dev, 2, INPUT_MT_POINTER | INPUT_MT_TRACK);
> >  		/* Image sensors can signal 4 and 5 finger clicks */
> >  		__set_bit(BTN_TOOL_QUADTAP, dev->keybit);
> > @@ -1202,6 +1204,7 @@ static void set_input_params(struct psmouse *psmouse,
> >  		set_abs_position_params(dev, priv, ABS_MT_POSITION_X,
> >  					ABS_MT_POSITION_Y);
> >  		input_set_abs_params(dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> > +		input_set_abs_params(dev, ABS_MT_TOUCH_MAJOR, 4, 15, 0, 0);
> >  		/*
> >  		 * Profile sensor in CR-48 tracks contacts reasonably well,
> >  		 * other non-image sensors with AGM use semi-mt.
> 
> Thanks.
> 
> 

[PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation

[Gabriele Mazzotta]

Userspace might still rely on ABS_TOOL_WIDTH to determine the width of
contacts, so add it to the legacy pointer emulation.

Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
---
 drivers/input/input-mt.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index cb150a1..bb4ca6d 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -65,6 +65,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
 		copy_abs(dev, ABS_X, ABS_MT_POSITION_X);
 		copy_abs(dev, ABS_Y, ABS_MT_POSITION_Y);
 		copy_abs(dev, ABS_PRESSURE, ABS_MT_PRESSURE);
+		copy_abs(dev, ABS_TOOL_WIDTH, ABS_MT_TOUCH_MAJOR);
 	}
 	if (flags & INPUT_MT_POINTER) {
 		__set_bit(BTN_TOOL_FINGER, dev->keybit);
@@ -182,8 +183,9 @@ EXPORT_SYMBOL(input_mt_report_finger_count);
  * @dev: input device with allocated MT slots
  * @use_count: report number of active contacts as finger count
  *
- * Performs legacy pointer emulation via BTN_TOUCH, ABS_X, ABS_Y and
- * ABS_PRESSURE. Touchpad finger count is emulated if use_count is true.
+ * Performs legacy pointer emulation via BTN_TOUCH, ABS_X, ABS_Y,
+ * ABS_PRESSURE and ABS_TOOL_WIDTH. Touchpad finger count is emulated
+ * if use_count is true.
  *
  * The input core ensures only the KEY and ABS axes already setup for
  * this device will produce output.
@@ -229,9 +231,15 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
 			int p = input_mt_get_value(oldest, ABS_MT_PRESSURE);
 			input_event(dev, EV_ABS, ABS_PRESSURE, p);
 		}
+		if (test_bit(ABS_MT_TOUCH_MAJOR, dev->absbit)) {
+			int w = input_mt_get_value(oldest, ABS_MT_TOUCH_MAJOR);
+			input_event(dev, EV_ABS, ABS_TOOL_WIDTH, w);
+		}
 	} else {
 		if (test_bit(ABS_MT_PRESSURE, dev->absbit))
 			input_event(dev, EV_ABS, ABS_PRESSURE, 0);
+		if (test_bit(ABS_MT_TOUCH_MAJOR, dev->absbit))
+			input_event(dev, EV_ABS, ABS_TOOL_WIDTH, 0);
 	}
 }
 EXPORT_SYMBOL(input_mt_report_pointer_emulation);
-- 
2.1.4

Re: [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation

[Dmitry Torokhov]

Hi Gabriele,

On Sun, Mar 22, 2015 at 03:43:56PM +0100, Gabriele Mazzotta wrote:
> Userspace might still rely on ABS_TOOL_WIDTH to determine the width of
> contacts, so add it to the legacy pointer emulation.

I do not think we can do that, at least not in such straightforward
manner, because while ABS_TOOL_WIDTH uses abstract units, normally in
the range of 0 - 15 (16 for bcm5974 driver) ABS_MT_TOUCH_MAJOR reports in
surface units. We would need to come up with a way to scale one into
another and make sure we are not stomping on whatever driver choses to
implement on its own.

In addition, the conversion only makes sense for touchpads, not tablets
or touchscreens.

> 
> Signed-off-by: Gabriele Mazzotta <gabriele.mzt@gmail.com>
> ---
>  drivers/input/input-mt.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
> index cb150a1..bb4ca6d 100644
> --- a/drivers/input/input-mt.c
> +++ b/drivers/input/input-mt.c
> @@ -65,6 +65,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
>  		copy_abs(dev, ABS_X, ABS_MT_POSITION_X);
>  		copy_abs(dev, ABS_Y, ABS_MT_POSITION_Y);
>  		copy_abs(dev, ABS_PRESSURE, ABS_MT_PRESSURE);
> +		copy_abs(dev, ABS_TOOL_WIDTH, ABS_MT_TOUCH_MAJOR);
>  	}
>  	if (flags & INPUT_MT_POINTER) {
>  		__set_bit(BTN_TOOL_FINGER, dev->keybit);
> @@ -182,8 +183,9 @@ EXPORT_SYMBOL(input_mt_report_finger_count);
>   * @dev: input device with allocated MT slots
>   * @use_count: report number of active contacts as finger count
>   *
> - * Performs legacy pointer emulation via BTN_TOUCH, ABS_X, ABS_Y and
> - * ABS_PRESSURE. Touchpad finger count is emulated if use_count is true.
> + * Performs legacy pointer emulation via BTN_TOUCH, ABS_X, ABS_Y,
> + * ABS_PRESSURE and ABS_TOOL_WIDTH. Touchpad finger count is emulated
> + * if use_count is true.
>   *
>   * The input core ensures only the KEY and ABS axes already setup for
>   * this device will produce output.
> @@ -229,9 +231,15 @@ void input_mt_report_pointer_emulation(struct input_dev *dev, bool use_count)
>  			int p = input_mt_get_value(oldest, ABS_MT_PRESSURE);
>  			input_event(dev, EV_ABS, ABS_PRESSURE, p);
>  		}
> +		if (test_bit(ABS_MT_TOUCH_MAJOR, dev->absbit)) {
> +			int w = input_mt_get_value(oldest, ABS_MT_TOUCH_MAJOR);
> +			input_event(dev, EV_ABS, ABS_TOOL_WIDTH, w);
> +		}
>  	} else {
>  		if (test_bit(ABS_MT_PRESSURE, dev->absbit))
>  			input_event(dev, EV_ABS, ABS_PRESSURE, 0);
> +		if (test_bit(ABS_MT_TOUCH_MAJOR, dev->absbit))
> +			input_event(dev, EV_ABS, ABS_TOOL_WIDTH, 0);
>  	}
>  }
>  EXPORT_SYMBOL(input_mt_report_pointer_emulation);
> -- 
> 2.1.4
> 

Thanks.

-- 
Dmitry

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

[Gabriele Mazzotta]

On Sunday 22 March 2015 15:43:51 Gabriele Mazzotta wrote:
> Hi,
> 
> I updated the series fixing the error reported by Shunsuke Shimizu that
> I made when I rebased v1. I've also included a change to make input
> devices correctly report their capabilities and included a change to
> make MT devices report widths as ABS_TOOL_WIDTH.
> 
> Gabriele Mazzotta (5):
>   input: synaptics - fix width values calculation on image sensors
>   input: synaptics - change default width value of cr48 sensors
>   input: synaptics - setup devices depending on their capabilities
>   input: synaptics - make image sensors and cr48 sensors report widths
>   input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation
> 
>  drivers/input/input-mt.c        | 12 +++++++--
>  drivers/input/mouse/synaptics.c | 55 +++++++++++++++++++++++++++--------------
>  2 files changed, 46 insertions(+), 21 deletions(-)
> 
> 

Hi Dmitry,

could you look into these?

Thanks,
Gabriele

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

[Gabriele Mazzotta]

On Sunday 22 March 2015 15:43:51 Gabriele Mazzotta wrote:
> Hi,
> 
> I updated the series fixing the error reported by Shunsuke Shimizu that
> I made when I rebased v1. I've also included a change to make input
> devices correctly report their capabilities and included a change to
> make MT devices report widths as ABS_TOOL_WIDTH.
> 
> Gabriele Mazzotta (5):
>   input: synaptics - fix width values calculation on image sensors
>   input: synaptics - change default width value of cr48 sensors
>   input: synaptics - setup devices depending on their capabilities
>   input: synaptics - make image sensors and cr48 sensors report widths
>   input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation
> 
>  drivers/input/input-mt.c        | 12 +++++++--
>  drivers/input/mouse/synaptics.c | 55 +++++++++++++++++++++++++++--------------
>  2 files changed, 46 insertions(+), 21 deletions(-)
> 
> 

Hi,

I'm sorry to bring this up again, but it's been a while.
Could this series be reviewed?

Thanks,
Gabriele