All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/5] input: synaptics - report correct width and pressure values
@ 2015-03-22 14:43 Gabriele Mazzotta
  2015-03-22 14:43 ` [PATCH v3 1/5] input: synaptics - fix width values calculation on image sensors Gabriele Mazzotta
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-22 14:43 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum,
	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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/5] input: synaptics - fix width values calculation on image sensors
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
@ 2015-03-22 14:43 ` Gabriele Mazzotta
  2015-03-22 14:43 ` [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors Gabriele Mazzotta
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-22 14:43 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum,
	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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
  2015-03-22 14:43 ` [PATCH v3 1/5] input: synaptics - fix width values calculation on image sensors Gabriele Mazzotta
@ 2015-03-22 14:43 ` Gabriele Mazzotta
  2015-03-23 20:48   ` Benjamin Tissoires
  2015-03-22 14:43 ` [PATCH v3 3/5] input: synaptics - setup devices depending on their capabilities Gabriele Mazzotta
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-22 14:43 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum,
	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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/5] input: synaptics - setup devices depending on their capabilities
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
  2015-03-22 14:43 ` [PATCH v3 1/5] input: synaptics - fix width values calculation on image sensors Gabriele Mazzotta
  2015-03-22 14:43 ` [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors Gabriele Mazzotta
@ 2015-03-22 14:43 ` Gabriele Mazzotta
  2015-03-22 14:43 ` [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths Gabriele Mazzotta
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-22 14:43 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum,
	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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
                   ` (2 preceding siblings ...)
  2015-03-22 14:43 ` [PATCH v3 3/5] input: synaptics - setup devices depending on their capabilities Gabriele Mazzotta
@ 2015-03-22 14:43 ` Gabriele Mazzotta
  2015-06-12  0:40   ` Dmitry Torokhov
  2015-03-22 14:43 ` [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation Gabriele Mazzotta
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-22 14:43 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum,
	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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
                   ` (3 preceding siblings ...)
  2015-03-22 14:43 ` [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths Gabriele Mazzotta
@ 2015-03-22 14:43 ` Gabriele Mazzotta
  2015-06-12  0:38   ` Dmitry Torokhov
  2015-04-27 20:01 ` [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
  2015-06-09 10:26 ` Gabriele Mazzotta
  6 siblings, 1 reply; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-22 14:43 UTC (permalink / raw)
  To: linux-input
  Cc: linux-kernel, dmitry.torokhov, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum,
	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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors
  2015-03-22 14:43 ` [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors Gabriele Mazzotta
@ 2015-03-23 20:48   ` Benjamin Tissoires
  2015-03-23 21:17     ` Gabriele Mazzotta
  0 siblings, 1 reply; 13+ messages in thread
From: Benjamin Tissoires @ 2015-03-23 20:48 UTC (permalink / raw)
  To: Gabriele Mazzotta
  Cc: linux-input, linux-kernel, Dmitry Torokhov, Henrik Rydberg,
	Maxwell Anselm, Peter Hutterer, Hans de Goede, grafi,
	Oliver Neukum

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
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors
  2015-03-23 20:48   ` Benjamin Tissoires
@ 2015-03-23 21:17     ` Gabriele Mazzotta
  0 siblings, 0 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-03-23 21:17 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: linux-input, linux-kernel, Dmitry Torokhov, Henrik Rydberg,
	Maxwell Anselm, Peter Hutterer, Hans de Goede, grafi,
	Oliver Neukum

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/5] input: synaptics - report correct width and pressure values
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
                   ` (4 preceding siblings ...)
  2015-03-22 14:43 ` [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation Gabriele Mazzotta
@ 2015-04-27 20:01 ` Gabriele Mazzotta
  2015-06-09 10:26 ` Gabriele Mazzotta
  6 siblings, 0 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-04-27 20:01 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: linux-input, linux-kernel, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 0/5] input: synaptics - report correct width and pressure values
  2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
                   ` (5 preceding siblings ...)
  2015-04-27 20:01 ` [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
@ 2015-06-09 10:26 ` Gabriele Mazzotta
  6 siblings, 0 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-06-09 10:26 UTC (permalink / raw)
  To: linux-input, dmitry.torokhov
  Cc: linux-kernel, rydberg, silverhammermba, peter.hutterer, hdegoede,
	benjamin.tissoires, grafi, oneukum

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation
  2015-03-22 14:43 ` [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation Gabriele Mazzotta
@ 2015-06-12  0:38   ` Dmitry Torokhov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Torokhov @ 2015-06-12  0:38 UTC (permalink / raw)
  To: Gabriele Mazzotta
  Cc: linux-input, linux-kernel, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths
  2015-03-22 14:43 ` [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths Gabriele Mazzotta
@ 2015-06-12  0:40   ` Dmitry Torokhov
  2015-06-12 18:39     ` Gabriele Mazzotta
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Torokhov @ 2015-06-12  0:40 UTC (permalink / raw)
  To: Gabriele Mazzotta
  Cc: linux-input, linux-kernel, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths
  2015-06-12  0:40   ` Dmitry Torokhov
@ 2015-06-12 18:39     ` Gabriele Mazzotta
  0 siblings, 0 replies; 13+ messages in thread
From: Gabriele Mazzotta @ 2015-06-12 18:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-kernel, rydberg, silverhammermba,
	peter.hutterer, hdegoede, benjamin.tissoires, grafi, oneukum

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-06-12 18:40 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-22 14:43 [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
2015-03-22 14:43 ` [PATCH v3 1/5] input: synaptics - fix width values calculation on image sensors Gabriele Mazzotta
2015-03-22 14:43 ` [PATCH v3 2/5] input: synaptics - change default width value of cr48 sensors Gabriele Mazzotta
2015-03-23 20:48   ` Benjamin Tissoires
2015-03-23 21:17     ` Gabriele Mazzotta
2015-03-22 14:43 ` [PATCH v3 3/5] input: synaptics - setup devices depending on their capabilities Gabriele Mazzotta
2015-03-22 14:43 ` [PATCH v3 4/5] input: synaptics - make image sensors and cr48 sensors report widths Gabriele Mazzotta
2015-06-12  0:40   ` Dmitry Torokhov
2015-06-12 18:39     ` Gabriele Mazzotta
2015-03-22 14:43 ` [PATCH v3 5/5] input: MT - add ABS_TOOL_WIDTH to the legacy pointer emulation Gabriele Mazzotta
2015-06-12  0:38   ` Dmitry Torokhov
2015-04-27 20:01 ` [PATCH v3 0/5] input: synaptics - report correct width and pressure values Gabriele Mazzotta
2015-06-09 10:26 ` Gabriele Mazzotta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.