[PATCH] input : wacom - report resolution for ABS_MT events

[PATCH] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

This is mainly required by the serial pen and touch devices since
both pen and touch are on the same port. Updated the USB ones to
keep MT event definition consistent among serial and USB devices.

Signed-off-by: Ping Cheng <pingc@wacom.com>
---
 drivers/input/tablet/wacom_wac.c        |    6 ++++++
 drivers/input/touchscreen/wacom_w8001.c |   23 ++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletions(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 367fa82..f129440 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -1283,6 +1283,12 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 			input_set_abs_params(input_dev, ABS_MT_PRESSURE,
 					     0, features->pressure_max,
 					     features->pressure_fuzz, 0);
+			input_abs_set_res(input_dev, ABS_MT_POSITION_X,
+				wacom_calculate_touch_res(features->x_max,
+							features->x_phy));
+			input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
+				wacom_calculate_touch_res(features->y_max,
+							features->y_phy));
 			input_abs_set_res(input_dev, ABS_X,
 				wacom_calculate_touch_res(features->x_max,
 							features->x_phy));
diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 5cb8449..f54da5c 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -51,6 +51,11 @@ MODULE_LICENSE("GPL");
 #define W8001_PKTLEN_TPCCTL	11	/* control packet */
 #define W8001_PKTLEN_TOUCH2FG	13
 
+/* resolution in points/mm */
+#define W8001_PEN_RESOLUTION    100
+#define W8001_TOUCH_RESOLUTION  10
+
+
 struct w8001_coord {
 	u8 rdy;
 	u8 tsw;
@@ -92,6 +97,8 @@ struct w8001 {
 	u16 max_touch_y;
 	u16 max_pen_x;
 	u16 max_pen_y;
+	u16 touch_res_x;
+	u16 touch_res_y;
 	char name[64];
 };
 
@@ -198,7 +205,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
 		query->y = 1024;
 		if (query->panel_res)
 			query->x = query->y = (1 << query->panel_res);
-		query->panel_res = 10;
+		query->panel_res = W8001_TOUCH_RESOLUTION;
 	}
 }
 
@@ -394,6 +401,8 @@ static int w8001_setup(struct w8001 *w8001)
 
 		input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
 		input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
+		input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
+		input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
 		input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
 		if (coord.tilt_x && coord.tilt_y) {
 			input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
@@ -411,6 +420,9 @@ static int w8001_setup(struct w8001 *w8001)
 	 * second byte is empty, which indicates touch is not supported.
 	 */
 	if (!error && w8001->response[1]) {
+		u16 touch_res_x  = touch.panel_res;
+		u16 touch_res_y = touch.panel_res;
+
 		__set_bit(BTN_TOUCH, dev->keybit);
 		__set_bit(BTN_TOOL_FINGER, dev->keybit);
 
@@ -422,10 +434,17 @@ static int w8001_setup(struct w8001 *w8001)
 		if (w8001->max_pen_x && w8001->max_pen_y) {
 			touch.x = w8001->max_pen_x;
 			touch.y = w8001->max_pen_y;
+
+			/* scale resolution as well */
+			touch_res_x *= w8001->max_pen_x / w8001->max_touch_x;
+			touch_res_y *= w8001->max_pen_y / w8001->max_touch_y;
 		}
 
 		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
 		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
+		/* don't report touch resolution for ST since pen resolution
+		 * is more important
+		 */
 
 		switch (touch.sensor_id) {
 		case 0:
@@ -453,6 +472,8 @@ static int w8001_setup(struct w8001 *w8001)
 						0, touch.y, 0, 0);
 			input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
 						0, MT_TOOL_MAX, 0, 0);
+			input_abs_set_res(dev, ABS_MT_POSITION_X, touch_res_x);
+			input_abs_set_res(dev, ABS_MT_POSITION_Y, touch_res_y);
 
 			strlcat(w8001->name, " 2FG", sizeof(w8001->name));
 			if (w8001->max_pen_x && w8001->max_pen_y)
-- 
1.7.3.5

Re: [PATCH] input : wacom - report resolution for ABS_MT events

[Henrik Rydberg]

Hi Ping,

> This is mainly required by the serial pen and touch devices since
> both pen and touch are on the same port. Updated the USB ones to
> keep MT event definition consistent among serial and USB devices.

Why is this change performed? What does the patch do? Please elaborate
a little bit.

> 
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
>  drivers/input/tablet/wacom_wac.c        |    6 ++++++
>  drivers/input/touchscreen/wacom_w8001.c |   23 ++++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index 367fa82..f129440 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -1283,6 +1283,12 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>  			input_set_abs_params(input_dev, ABS_MT_PRESSURE,
>  					     0, features->pressure_max,
>  					     features->pressure_fuzz, 0);
> +			input_abs_set_res(input_dev, ABS_MT_POSITION_X,
> +				wacom_calculate_touch_res(features->x_max,
> +							features->x_phy));
> +			input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
> +				wacom_calculate_touch_res(features->y_max,
> +							features->y_phy));

Is this for the BAMBOO_PT case? What about the extra x/y scaling for
some of those devices?

>  			input_abs_set_res(input_dev, ABS_X,
>  				wacom_calculate_touch_res(features->x_max,
>  							features->x_phy));
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 5cb8449..f54da5c 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -51,6 +51,11 @@ MODULE_LICENSE("GPL");
>  #define W8001_PKTLEN_TPCCTL	11	/* control packet */
>  #define W8001_PKTLEN_TOUCH2FG	13
>  
> +/* resolution in points/mm */
> +#define W8001_PEN_RESOLUTION    100
> +#define W8001_TOUCH_RESOLUTION  10
> +
> +
>  struct w8001_coord {
>  	u8 rdy;
>  	u8 tsw;
> @@ -92,6 +97,8 @@ struct w8001 {
>  	u16 max_touch_y;
>  	u16 max_pen_x;
>  	u16 max_pen_y;
> +	u16 touch_res_x;
> +	u16 touch_res_y;

Are these used anywhere?

>  	char name[64];
>  };
>  
> @@ -198,7 +205,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>  		query->y = 1024;
>  		if (query->panel_res)
>  			query->x = query->y = (1 << query->panel_res);
> -		query->panel_res = 10;
> +		query->panel_res = W8001_TOUCH_RESOLUTION;
>  	}
>  }
>  
> @@ -394,6 +401,8 @@ static int w8001_setup(struct w8001 *w8001)
>  
>  		input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
>  		input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> +		input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
> +		input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
>  		input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
>  		if (coord.tilt_x && coord.tilt_y) {
>  			input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
> @@ -411,6 +420,9 @@ static int w8001_setup(struct w8001 *w8001)
>  	 * second byte is empty, which indicates touch is not supported.
>  	 */
>  	if (!error && w8001->response[1]) {
> +		u16 touch_res_x  = touch.panel_res;
> +		u16 touch_res_y = touch.panel_res;
> +
>  		__set_bit(BTN_TOUCH, dev->keybit);
>  		__set_bit(BTN_TOOL_FINGER, dev->keybit);
>  
> @@ -422,10 +434,17 @@ static int w8001_setup(struct w8001 *w8001)
>  		if (w8001->max_pen_x && w8001->max_pen_y) {
>  			touch.x = w8001->max_pen_x;
>  			touch.y = w8001->max_pen_y;
> +
> +			/* scale resolution as well */
> +			touch_res_x *= w8001->max_pen_x / w8001->max_touch_x;
> +			touch_res_y *= w8001->max_pen_y / w8001->max_touch_y;
>  		}
>  
>  		input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
>  		input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
> +		/* don't report touch resolution for ST since pen resolution
> +		 * is more important
> +		 */
>  
>  		switch (touch.sensor_id) {
>  		case 0:
> @@ -453,6 +472,8 @@ static int w8001_setup(struct w8001 *w8001)
>  						0, touch.y, 0, 0);
>  			input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
>  						0, MT_TOOL_MAX, 0, 0);
> +			input_abs_set_res(dev, ABS_MT_POSITION_X, touch_res_x);
> +			input_abs_set_res(dev, ABS_MT_POSITION_Y, touch_res_y);
>  
>  			strlcat(w8001->name, " 2FG", sizeof(w8001->name));
>  			if (w8001->max_pen_x && w8001->max_pen_y)
> -- 
> 1.7.3.5
> 

Thanks,
Henrik

Re: [PATCH] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

On Fri, Jan 28, 2011 at 9:30 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Ping,
>
>> This is mainly required by the serial pen and touch devices since
>> both pen and touch are on the same port. Updated the USB ones to
>> keep MT event definition consistent among serial and USB devices.
>
> Why is this change performed? What does the patch do? Please elaborate
> a little bit.

Let's see if I can explain it clearer or not.

We need to report resolution to userland for wacom_w8001.c. Since pen
and touch data are reported through the same logical port for serial
devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
resolution for pen or touch. So, I let ABS_X/Y report pen resolution
since most legacy ST clients process pen data over touch. However, for
multi-touch clients, especially for those that support gestures, touch
resolution is needed to calculate the physical distance between two
touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
it is quite intuitive.

This is not an issue for USB devices since pen and touch data are
reported on separate logical port to the userland. I added
ABS_MT_POSITION_X/Y for Bamboo touch only to make the interface
consistent between USB and serial.

Please see more comments inline.

>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>> ---
>>  drivers/input/tablet/wacom_wac.c        |    6 ++++++
>>  drivers/input/touchscreen/wacom_w8001.c |   23 ++++++++++++++++++++++-
>>  2 files changed, 28 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
>> index 367fa82..f129440 100644
>> --- a/drivers/input/tablet/wacom_wac.c
>> +++ b/drivers/input/tablet/wacom_wac.c
>> @@ -1283,6 +1283,12 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
>>                       input_set_abs_params(input_dev, ABS_MT_PRESSURE,
>>                                            0, features->pressure_max,
>>                                            features->pressure_fuzz, 0);
>> +                     input_abs_set_res(input_dev, ABS_MT_POSITION_X,
>> +                             wacom_calculate_touch_res(features->x_max,
>> +                                                     features->x_phy));
>> +                     input_abs_set_res(input_dev, ABS_MT_POSITION_Y,
>> +                             wacom_calculate_touch_res(features->y_max,
>> +                                                     features->y_phy));
>
> Is this for the BAMBOO_PT case? What about the extra x/y scaling for
> some of those devices?

Yes, it is for Bamboo. We do not need to scale the value here since
touch and pen are on different ports. We had to scale the value for
wacom_w8001 since both pen and touch are on the same port. Without
scaling the data, the pen/finger tip will not follow the cursor on the
screen (wacom_w8001 is a touchscren device).


>>                       input_abs_set_res(input_dev, ABS_X,
>>                               wacom_calculate_touch_res(features->x_max,
>>                                                       features->x_phy));
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index 5cb8449..f54da5c 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -51,6 +51,11 @@ MODULE_LICENSE("GPL");
>>  #define W8001_PKTLEN_TPCCTL  11      /* control packet */
>>  #define W8001_PKTLEN_TOUCH2FG        13
>>
>> +/* resolution in points/mm */
>> +#define W8001_PEN_RESOLUTION    100
>> +#define W8001_TOUCH_RESOLUTION  10
>> +
>> +
>>  struct w8001_coord {
>>       u8 rdy;
>>       u8 tsw;
>> @@ -92,6 +97,8 @@ struct w8001 {
>>       u16 max_touch_y;
>>       u16 max_pen_x;
>>       u16 max_pen_y;
>> +     u16 touch_res_x;
>> +     u16 touch_res_y;
>
> Are these used anywhere?

Nice catch! I thought we needed to keep those two values when I worked
on it. But the final patch turned out they are only used once. I will
remove them with version 2 after I hear from you about the other
comments.

Thank you.

Ping

>>       char name[64];
>>  };
>>
>> @@ -198,7 +205,7 @@ static void parse_touchquery(u8 *data, struct w8001_touch_query *query)
>>               query->y = 1024;
>>               if (query->panel_res)
>>                       query->x = query->y = (1 << query->panel_res);
>> -             query->panel_res = 10;
>> +             query->panel_res = W8001_TOUCH_RESOLUTION;
>>       }
>>  }
>>
>> @@ -394,6 +401,8 @@ static int w8001_setup(struct w8001 *w8001)
>>
>>               input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
>>               input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
>> +             input_abs_set_res(dev, ABS_X, W8001_PEN_RESOLUTION);
>> +             input_abs_set_res(dev, ABS_Y, W8001_PEN_RESOLUTION);
>>               input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
>>               if (coord.tilt_x && coord.tilt_y) {
>>                       input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
>> @@ -411,6 +420,9 @@ static int w8001_setup(struct w8001 *w8001)
>>        * second byte is empty, which indicates touch is not supported.
>>        */
>>       if (!error && w8001->response[1]) {
>> +             u16 touch_res_x  = touch.panel_res;
>> +             u16 touch_res_y = touch.panel_res;
>> +
>>               __set_bit(BTN_TOUCH, dev->keybit);
>>               __set_bit(BTN_TOOL_FINGER, dev->keybit);
>>
>> @@ -422,10 +434,17 @@ static int w8001_setup(struct w8001 *w8001)
>>               if (w8001->max_pen_x && w8001->max_pen_y) {
>>                       touch.x = w8001->max_pen_x;
>>                       touch.y = w8001->max_pen_y;
>> +
>> +                     /* scale resolution as well */
>> +                     touch_res_x *= w8001->max_pen_x / w8001->max_touch_x;
>> +                     touch_res_y *= w8001->max_pen_y / w8001->max_touch_y;
>>               }
>>
>>               input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
>>               input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
>> +             /* don't report touch resolution for ST since pen resolution
>> +              * is more important
>> +              */
>>
>>               switch (touch.sensor_id) {
>>               case 0:
>> @@ -453,6 +472,8 @@ static int w8001_setup(struct w8001 *w8001)
>>                                               0, touch.y, 0, 0);
>>                       input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
>>                                               0, MT_TOOL_MAX, 0, 0);
>> +                     input_abs_set_res(dev, ABS_MT_POSITION_X, touch_res_x);
>> +                     input_abs_set_res(dev, ABS_MT_POSITION_Y, touch_res_y);
>>
>>                       strlcat(w8001->name, " 2FG", sizeof(w8001->name));
>>                       if (w8001->max_pen_x && w8001->max_pen_y)
>> --
>> 1.7.3.5
>>
>
> 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] input : wacom - report resolution for ABS_MT events

[Dmitry Torokhov]

On Fri, Jan 28, 2011 at 10:23:59AM -0800, Ping Cheng wrote:
> On Fri, Jan 28, 2011 at 9:30 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> > Hi Ping,
> >
> >> This is mainly required by the serial pen and touch devices since
> >> both pen and touch are on the same port. Updated the USB ones to
> >> keep MT event definition consistent among serial and USB devices.
> >
> > Why is this change performed? What does the patch do? Please elaborate
> > a little bit.
> 
> Let's see if I can explain it clearer or not.
> 
> We need to report resolution to userland for wacom_w8001.c. Since pen
> and touch data are reported through the same logical port for serial
> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
> since most legacy ST clients process pen data over touch. However, for
> multi-touch clients, especially for those that support gestures, touch
> resolution is needed to calculate the physical distance between two
> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
> it is quite intuitive.

We just went through the round of why we should scale events from
different tools when they use the same event devices and I do not see
why MT data is any different.

Devices have only one set of physical axes (MT or not) so I believe
min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
you want to have this data in ABS_MT_POSITION, it should be trhe same.

ABS_MT_POSITION is just a vehicle of delivering multipe contacts, not a
new device axis.

Thanks.

-- 
Dmitry

Re: [PATCH] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

On Fri, Jan 28, 2011 at 10:46 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 28, 2011 at 10:23:59AM -0800, Ping Cheng wrote:
>> On Fri, Jan 28, 2011 at 9:30 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> > Hi Ping,
>> >
>> >> This is mainly required by the serial pen and touch devices since
>> >> both pen and touch are on the same port. Updated the USB ones to
>> >> keep MT event definition consistent among serial and USB devices.
>> >
>> > Why is this change performed? What does the patch do? Please elaborate
>> > a little bit.
>>
>> Let's see if I can explain it clearer or not.
>>
>> We need to report resolution to userland for wacom_w8001.c. Since pen
>> and touch data are reported through the same logical port for serial
>> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
>> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
>> since most legacy ST clients process pen data over touch. However, for
>> multi-touch clients, especially for those that support gestures, touch
>> resolution is needed to calculate the physical distance between two
>> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
>> it is quite intuitive.
>
> We just went through the round of why we should scale events from
> different tools when they use the same event devices and I do not see
> why MT data is any different.

At the time when we worked on that, we did not consider resolution. We
only scaled the logical values.

> Devices have only one set of physical axes (MT or not)

You are right if the device only supports one type of tools. Yes, ST
or pen can go with ABS_X/Y. MT goes with MT. However, we are dealing
with a dual tool case: pen and touch on the same port. Using only one
set of axes would not be able to cover both tool events and pass the
resolution to the userland.

> so I believe
> min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
> you want to have this data in ABS_MT_POSITION, it should be trhe same.

max/min is different from resolution. We can scale max/min since they
are logical data. But we can not change the device's physical size.
That's the root cause of the "stubborn" nature of resolution.

> ABS_MT_POSITION is just a vehicle of delivering multipe contacts, not a
> new device axis.

That is true If we have other means to pass the resolution on. I am
open to other solutions.

Thanks,

Ping
--
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] input : wacom - report resolution for ABS_MT events

[Henrik Rydberg]

> >> This is mainly required by the serial pen and touch devices since
> >> both pen and touch are on the same port. Updated the USB ones to
> >> keep MT event definition consistent among serial and USB devices.
> >
> > Why is this change performed? What does the patch do? Please elaborate
> > a little bit.
> 
> Let's see if I can explain it clearer or not.
> 
> We need to report resolution to userland for wacom_w8001.c.

Thanks, perfect - sentences like this one is great to put in the patch
description.

> Since pen
> and touch data are reported through the same logical port for serial
> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
> since most legacy ST clients process pen data over touch. However, for
> multi-touch clients, especially for those that support gestures, touch
> resolution is needed to calculate the physical distance between two
> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
> it is quite intuitive.

I thought this was solved by reporting both pen and touch on the same
scale, which would render this problem moot. Or?

[...]
> > Is this for the BAMBOO_PT case? What about the extra x/y scaling for
> > some of those devices?
> 
> Yes, it is for Bamboo. We do not need to scale the value here since
> touch and pen are on different ports. We had to scale the value for
> wacom_w8001 since both pen and touch are on the same port. Without
> scaling the data, the pen/finger tip will not follow the cursor on the
> screen (wacom_w8001 is a touchscren device).

I was thinking of the scaling quirk in effect for the Bamboo
Touch. The resolution would have to change as well.

Thanks,
Henrik

Re: [PATCH] input : wacom - report resolution for ABS_MT events

[Dmitry Torokhov]

On Fri, Jan 28, 2011 at 11:01:39AM -0800, Ping Cheng wrote:
> On Fri, Jan 28, 2011 at 10:46 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Jan 28, 2011 at 10:23:59AM -0800, Ping Cheng wrote:
> >> On Fri, Jan 28, 2011 at 9:30 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> >> > Hi Ping,
> >> >
> >> >> This is mainly required by the serial pen and touch devices since
> >> >> both pen and touch are on the same port. Updated the USB ones to
> >> >> keep MT event definition consistent among serial and USB devices.
> >> >
> >> > Why is this change performed? What does the patch do? Please elaborate
> >> > a little bit.
> >>
> >> Let's see if I can explain it clearer or not.
> >>
> >> We need to report resolution to userland for wacom_w8001.c. Since pen
> >> and touch data are reported through the same logical port for serial
> >> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
> >> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
> >> since most legacy ST clients process pen data over touch. However, for
> >> multi-touch clients, especially for those that support gestures, touch
> >> resolution is needed to calculate the physical distance between two
> >> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
> >> it is quite intuitive.
> >
> > We just went through the round of why we should scale events from
> > different tools when they use the same event devices and I do not see
> > why MT data is any different.
> 
> At the time when we worked on that, we did not consider resolution. We
> only scaled the logical values.
> 
> > Devices have only one set of physical axes (MT or not)
> 
> You are right if the device only supports one type of tools. Yes, ST
> or pen can go with ABS_X/Y. MT goes with MT. However, we are dealing
> with a dual tool case: pen and touch on the same port. Using only one
> set of axes would not be able to cover both tool events and pass the
> resolution to the userland.
> 
> > so I believe
> > min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
> > you want to have this data in ABS_MT_POSITION, it should be trhe same.
> 
> max/min is different from resolution. We can scale max/min since they
> are logical data. But we can not change the device's physical size.
> That's the root cause of the "stubborn" nature of resolution.

This is the same device so it should have the same physical dimensions,
right?

-- 
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] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

On Fri, Jan 28, 2011 at 11:37 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 28, 2011 at 11:01:39AM -0800, Ping Cheng wrote:
>> On Fri, Jan 28, 2011 at 10:46 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> > On Fri, Jan 28, 2011 at 10:23:59AM -0800, Ping Cheng wrote:
>> >> On Fri, Jan 28, 2011 at 9:30 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> >> > Hi Ping,
>> >> >
>> >> >> This is mainly required by the serial pen and touch devices since
>> >> >> both pen and touch are on the same port. Updated the USB ones to
>> >> >> keep MT event definition consistent among serial and USB devices.
>> >> >
>> >> > Why is this change performed? What does the patch do? Please elaborate
>> >> > a little bit.
>> >>
>> >> Let's see if I can explain it clearer or not.
>> >>
>> >> We need to report resolution to userland for wacom_w8001.c. Since pen
>> >> and touch data are reported through the same logical port for serial
>> >> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
>> >> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
>> >> since most legacy ST clients process pen data over touch. However, for
>> >> multi-touch clients, especially for those that support gestures, touch
>> >> resolution is needed to calculate the physical distance between two
>> >> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
>> >> it is quite intuitive.
>> >
>> > We just went through the round of why we should scale events from
>> > different tools when they use the same event devices and I do not see
>> > why MT data is any different.
>>
>> At the time when we worked on that, we did not consider resolution. We
>> only scaled the logical values.
>>
>> > Devices have only one set of physical axes (MT or not)
>>
>> You are right if the device only supports one type of tools. Yes, ST
>> or pen can go with ABS_X/Y. MT goes with MT. However, we are dealing
>> with a dual tool case: pen and touch on the same port. Using only one
>> set of axes would not be able to cover both tool events and pass the
>> resolution to the userland.
>>
>> > so I believe
>> > min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
>> > you want to have this data in ABS_MT_POSITION, it should be trhe same.
>>
>> max/min is different from resolution. We can scale max/min since they
>> are logical data. But we can not change the device's physical size.
>> That's the root cause of the "stubborn" nature of resolution.
>
> This is the same device so it should have the same physical dimensions,
> right?

No, pen and touch use different chips. They can have different sizes.

Ping
--
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] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

On Fri, Jan 28, 2011 at 11:12 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
>> >> This is mainly required by the serial pen and touch devices since
>> >> both pen and touch are on the same port. Updated the USB ones to
>> >> keep MT event definition consistent among serial and USB devices.
>> >
>> > Why is this change performed? What does the patch do? Please elaborate
>> > a little bit.
>>
>> Let's see if I can explain it clearer or not.
>>
>> We need to report resolution to userland for wacom_w8001.c.
>
> Thanks, perfect - sentences like this one is great to put in the patch
> description.

Thanks for the encouragement ;).

>> Since pen
>> and touch data are reported through the same logical port for serial
>> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
>> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
>> since most legacy ST clients process pen data over touch. However, for
>> multi-touch clients, especially for those that support gestures, touch
>> resolution is needed to calculate the physical distance between two
>> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
>> it is quite intuitive.
>
> I thought this was solved by reporting both pen and touch on the same
> scale, which would render this problem moot. Or?

Yeah, see my reply to Dmitry for detail.

> [...]
>> > Is this for the BAMBOO_PT case? What about the extra x/y scaling for
>> > some of those devices?
>>
>> Yes, it is for Bamboo. We do not need to scale the value here since
>> touch and pen are on different ports. We had to scale the value for
>> wacom_w8001 since both pen and touch are on the same port. Without
>> scaling the data, the pen/finger tip will not follow the cursor on the
>> screen (wacom_w8001 is a touchscren device).
>
> I was thinking of the scaling quirk in effect for the Bamboo
> Touch. The resolution would have to change as well.

Yes, If we need to report the right values ;).

Ping
--
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] input : wacom - report resolution for ABS_MT events

[Dmitry Torokhov]

On Fri, Jan 28, 2011 at 12:08:18PM -0800, Ping Cheng wrote:
> On Fri, Jan 28, 2011 at 11:37 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Jan 28, 2011 at 11:01:39AM -0800, Ping Cheng wrote:
> >> On Fri, Jan 28, 2011 at 10:46 AM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >> > On Fri, Jan 28, 2011 at 10:23:59AM -0800, Ping Cheng wrote:
> >> >> On Fri, Jan 28, 2011 at 9:30 AM, Henrik Rydberg <rydberg@euromail.se> wrote:
> >> >> > Hi Ping,
> >> >> >
> >> >> >> This is mainly required by the serial pen and touch devices since
> >> >> >> both pen and touch are on the same port. Updated the USB ones to
> >> >> >> keep MT event definition consistent among serial and USB devices.
> >> >> >
> >> >> > Why is this change performed? What does the patch do? Please elaborate
> >> >> > a little bit.
> >> >>
> >> >> Let's see if I can explain it clearer or not.
> >> >>
> >> >> We need to report resolution to userland for wacom_w8001.c. Since pen
> >> >> and touch data are reported through the same logical port for serial
> >> >> devices, using input_abs_set_res  for ABS_X/Y,  we can only pass the
> >> >> resolution for pen or touch. So, I let ABS_X/Y report pen resolution
> >> >> since most legacy ST clients process pen data over touch. However, for
> >> >> multi-touch clients, especially for those that support gestures, touch
> >> >> resolution is needed to calculate the physical distance between two
> >> >> touch points. Using  ABS_MT_POSITION_X/Y does not cause confusion and
> >> >> it is quite intuitive.
> >> >
> >> > We just went through the round of why we should scale events from
> >> > different tools when they use the same event devices and I do not see
> >> > why MT data is any different.
> >>
> >> At the time when we worked on that, we did not consider resolution. We
> >> only scaled the logical values.
> >>
> >> > Devices have only one set of physical axes (MT or not)
> >>
> >> You are right if the device only supports one type of tools. Yes, ST
> >> or pen can go with ABS_X/Y. MT goes with MT. However, we are dealing
> >> with a dual tool case: pen and touch on the same port. Using only one
> >> set of axes would not be able to cover both tool events and pass the
> >> resolution to the userland.
> >>
> >> > so I believe
> >> > min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
> >> > you want to have this data in ABS_MT_POSITION, it should be trhe same.
> >>
> >> max/min is different from resolution. We can scale max/min since they
> >> are logical data. But we can not change the device's physical size.
> >> That's the root cause of the "stubborn" nature of resolution.
> >
> > This is the same device so it should have the same physical dimensions,
> > right?
> 
> No, pen and touch use different chips. They can have different sizes.

It does not matter how many chips you have inside. What matters whether
they share a working surface or not. If working surface is the same (and
consequentially you have single input_dev structure) then dimensions are
the same.

-- 
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] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

On Fri, Jan 28, 2011 at 12:54 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> >> > so I believe
>> >> > min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
>> >> > you want to have this data in ABS_MT_POSITION, it should be trhe same.
>> >>
>> >> max/min is different from resolution. We can scale max/min since they
>> >> are logical data. But we can not change the device's physical size.
>> >> That's the root cause of the "stubborn" nature of resolution.
>> >
>> > This is the same device so it should have the same physical dimensions,
>> > right?
>>
>> No, pen and touch use different chips. They can have different sizes.
>
> It does not matter how many chips you have inside. What matters whether
> they share a working surface or not. If working surface is the same (and
> consequentially you have single input_dev structure) then dimensions are
> the same.

They do not always share the exact same surface. Most of the devices
do have the same working surface. But not all of them although the
differences are normally small. To represent a finger touch, it may
not be too big a deal. But, making the assumption that they are the
same size doesn't sound right.

Do we ignore that small imprecision? Or do I submit a version 2 (I'll
update Bamboo resolution with Henrik's scale in the same patch)?

Thank you.

Ping

Re: [PATCH] input : wacom - report resolution for ABS_MT events

[Dmitry Torokhov]

On Fri, Jan 28, 2011 at 01:57:17PM -0800, Ping Cheng wrote:
> On Fri, Jan 28, 2011 at 12:54 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >> >> > so I believe
> >> >> > min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
> >> >> > you want to have this data in ABS_MT_POSITION, it should be trhe same.
> >> >>
> >> >> max/min is different from resolution. We can scale max/min since they
> >> >> are logical data. But we can not change the device's physical size.
> >> >> That's the root cause of the "stubborn" nature of resolution.
> >> >
> >> > This is the same device so it should have the same physical dimensions,
> >> > right?
> >>
> >> No, pen and touch use different chips. They can have different sizes.
> >
> > It does not matter how many chips you have inside. What matters whether
> > they share a working surface or not. If working surface is the same (and
> > consequentially you have single input_dev structure) then dimensions are
> > the same.
> 
> They do not always share the exact same surface. Most of the devices
> do have the same working surface. But not all of them although the
> differences are normally small. To represent a finger touch, it may
> not be too big a deal. But, making the assumption that they are the
> same size doesn't sound right.
> 
> Do we ignore that small imprecision?

I think we should consider these cases as having roughly the same
dimensions of working surfaces. Because if we do what you are proposing
the next thing we'll see an MT device that can report several tools
simultaneously and your scheme breaks apart.

Devices with surface characteristics so different that they can't be
handled well by scaling to common denominator should probably be
presented as 2 independent logical devices.

Thanks.

-- 
Dmitry

Re: [PATCH] input : wacom - report resolution for ABS_MT events

[Ping Cheng]

On Fri, Jan 28, 2011 at 4:05 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Jan 28, 2011 at 01:57:17PM -0800, Ping Cheng wrote:
>> On Fri, Jan 28, 2011 at 12:54 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> >> >> > so I believe
>> >> >> > min, max and resultion should be reported on ABS_X, Y, X, etc, or, if
>> >> >> > you want to have this data in ABS_MT_POSITION, it should be trhe same.
>> >> >>
>> >> >> max/min is different from resolution. We can scale max/min since they
>> >> >> are logical data. But we can not change the device's physical size.
>> >> >> That's the root cause of the "stubborn" nature of resolution.
>> >> >
>> >> > This is the same device so it should have the same physical dimensions,
>> >> > right?
>> >>
>> >> No, pen and touch use different chips. They can have different sizes.
>> >
>> > It does not matter how many chips you have inside. What matters whether
>> > they share a working surface or not. If working surface is the same (and
>> > consequentially you have single input_dev structure) then dimensions are
>> > the same.
>>
>> They do not always share the exact same surface. Most of the devices
>> do have the same working surface. But not all of them although the
>> differences are normally small. To represent a finger touch, it may
>> not be too big a deal. But, making the assumption that they are the
>> same size doesn't sound right.
>>
>> Do we ignore that small imprecision?
>
> I think we should consider these cases as having roughly the same
> dimensions of working surfaces. Because if we do what you are proposing
> the next thing we'll see an MT device that can report several tools
> simultaneously and your scheme breaks apart.
>
> Devices with surface characteristics so different that they can't be
> handled well by scaling to common denominator should probably be
> presented as 2 independent logical devices.

Dmitry,

Version 2 will be sent incorporating the above comments in a moment.

Henrik,

The Bamboo resolution has already been scaled since we calculated it
through the scaled logical maximum. So, no changes needed there.

Ping