input : wacom - report resolution for ABS_MT events

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 <[email protected]>

---
 drivers/input/tablet/wacom_wac.c        |    6 ++++++
 drivers/input/touchscreen/wacom_w8001.c |   23 ++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletions(-)

drivers/input/tablet/wacom_wac.c +0 -0

index 367fa82..f129440 100644

drivers/input/tablet/wacom_wac.c +0 -0

 			input_abs_set_res(input_dev, ABS_X,
 				wacom_calculate_touch_res(features->x_max,
 							features->x_phy));

drivers/input/touchscreen/wacom_w8001.c +5 -0

index 5cb8449..f54da5c 100644
@@ -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;

drivers/input/touchscreen/wacom_w8001.c +15 -1

 	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)

Signature · 1 line

1.7.3.5

Hi Ping,

Quoted 3 lines

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.

Quoted 15 lines

Signed-off-by Ping Cheng <[email protected]>

---
 drivers/input/tablet/wacom_wac.c        |    6 ++++++ drivers/input/touchscreen/wacom_w8001.c |   23 ++++++++++++++++++++++-

 2 files changed, 28 insertions(+), 1 deletions(-)

drivers/input/tablet/wacom_wac.c +1 -0

index 367fa82..f129440 100644
@@ -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,

drivers/input/tablet/wacom_wac.c void wacom_setup_input_capabilities(struct input_dev *input_dev, · line 1291

Quoted 5 lines

drivers/input/tablet/wacom_wac.c +5 -0

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

Quoted 11 lines

drivers/input/tablet/wacom_wac.c +0 -0

                       input_abs_set_res(input_dev, ABS_X,
                               wacom_calculate_touch_res(features->x_max,
                                                       features->x_phy));

drivers/input/touchscreen/wacom_w8001.c +0 -0

index 5cb8449..f54da5c 100644
@@ -51,6 +51,11 @@ MODULE_LICENSE("GPL");
  #define W8001_PKTLEN_TPCCTL  11      /* control packet */
  #define W8001_PKTLEN_TOUCH2FG        13
 

Quoted 9 lines

drivers/input/touchscreen/wacom_w8001.c +5 -0

+/* resolution in points/mm */
+#define W8001_PEN_RESOLUTION    100
+#define W8001_TOUCH_RESOLUTION  10
+
+
  struct w8001_coord {
       u8 rdy;
       u8 tsw;

drivers/input/touchscreen/wacom_w8001.c struct w8001 { · line 101

Quoted 5 lines

drivers/input/touchscreen/wacom_w8001.c +2 -0

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

Quoted 56 lines

drivers/input/touchscreen/wacom_w8001.c +13 -1

       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:

drivers/input/touchscreen/wacom_w8001.c +2 -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);
 

Quoted 5 lines

drivers/input/touchscreen/wacom_w8001.c +0 -1

                       strlcat(w8001->name, " 2FG", sizeof(w8001->name));
                       if (w8001->max_pen_x && w8001->max_pen_y)
--

1.7.3.5

Thanks,
Henrik

Quoted 8 lines · Henrik Rydberg

Hi Ping,

Quoted 3 lines

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.

Quoted 12 lines · Ping Cheng

Quoted 8 lines · Henrik Rydberg

Hi Ping,

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 23 lines · Dmitry Torokhov

Quoted 12 lines · Ping Cheng

Quoted 8 lines · Henrik Rydberg

Hi Ping,

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 3 lines

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.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 44 lines · Ping Cheng

Quoted 23 lines · Dmitry Torokhov

Quoted 12 lines · Ping Cheng

Quoted 8 lines · Henrik Rydberg

Hi Ping,

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 3 lines

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.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 3 lines

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?

Quoted 45 lines · Dmitry Torokhov

Quoted 44 lines · Ping Cheng

Quoted 23 lines · Dmitry Torokhov

Quoted 12 lines · Ping Cheng

Quoted 8 lines · Henrik Rydberg

Hi Ping,

Quoted 3 lines

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.

Quoted 9 lines

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.

Quoted 3 lines

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.

Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

Quoted 3 lines

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?

Quoted 1 line

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.

Quoted 11 lines · Dmitry Torokhov

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

Quoted 3 lines

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?

Quoted 6 lines

Quoted 1 line

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?

Quoted 20 lines · Ping Cheng

Quoted 11 lines · Dmitry Torokhov

Quoted 11 lines

Quoted 10 lines

Quoted 7 lines

Quoted 3 lines

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?

Quoted 6 lines

Quoted 1 line

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.

Quoted 7 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

Quoted 2 lines

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

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 9 lines

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?

[...]

Quoted 3 lines

Quoted 2 lines

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

Quoted 5 lines

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.