input: wacom - Pass touch resolution to clients through input_absinfo

PATCH Applied Ping Cheng 3 participants 5 messages Jan 23, 2011 – Jan 24, 2011

Subsystems (2)

HID Input drivers

lore

Share & tools

lore

Ping Cheng

author

2011-01-23 23:27 UTC #0 lore raw

PATCH 2/2 input: wacom - Pass touch resolution to clients through input_absinfo

Fixed the workaround used for kernels older than 2.6.35.

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

---
 drivers/input/tablet/wacom_wac.c |   18 +++++++++++++++---
 1 files changed, 15 insertions(+), 3 deletions(-)

drivers/input/tablet/wacom_wac.c +15 -3

index f44c822..497d0ba 100644
@@ -1105,7 +1105,7 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 				    struct wacom_wac *wacom_wac)
 {
 	struct wacom_features *features = &wacom_wac->features;
-	int i;
+	int i, res;
 
 	input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);

@@ -1228,8 +1228,13 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 	case TABLETPC:
 		if (features->device_type == BTN_TOOL_DOUBLETAP ||
 		    features->device_type == BTN_TOOL_TRIPLETAP) {
-			input_set_abs_params(input_dev, ABS_RX, 0, features->x_phy, 0, 0);
-			input_set_abs_params(input_dev, ABS_RY, 0, features->y_phy, 0, 0);
+			/* set touch resolution in points/mm */
+			res = features->x_phy/100;
+			input_abs_set_res(input_dev, ABS_X,
+					    features->x_max/res);
+			res = features->y_phy/100;
+			input_abs_set_res(input_dev, ABS_Y,
+					    features->y_max/res);
 			__set_bit(BTN_TOOL_DOUBLETAP, input_dev->keybit);
 		}

@@ -1272,6 +1277,13 @@ 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);
+			/* set touch resolution in points/mm */
+			res = features->x_phy/100;
+			input_abs_set_res(input_dev, ABS_X,
+					    features->x_max/res);
+			res = features->y_phy/100;
+			input_abs_set_res(input_dev, ABS_Y,
+					    features->y_max/res);
 		} else if (features->device_type == BTN_TOOL_PEN) {
 			__set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
 			__set_bit(BTN_TOOL_PEN, input_dev->keybit);

Signature · 1 line

1.7.3.4

Henrik Rydberg

2011-01-24 14:45 UTC #1 lore raw

Hi Ping,

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

Quoted 10 lines

drivers/input/tablet/wacom_wac.c +4 -2

@@ -1228,8 +1228,13 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
 	case TABLETPC:
 		if (features->device_type == BTN_TOOL_DOUBLETAP ||
 		    features->device_type == BTN_TOOL_TRIPLETAP) {
-			input_set_abs_params(input_dev, ABS_RX, 0, features->x_phy, 0, 0);
-			input_set_abs_params(input_dev, ABS_RY, 0, features->y_phy, 0, 0);
+			/* set touch resolution in points/mm */
+			res = features->x_phy/100;
+			input_abs_set_res(input_dev, ABS_X,
+					    features->x_max/res);

It seems to me the construction above looses precision twice for no apparent reason. How about instead use "100 * features->x_max / features->x_phy"? Besides, it is a bit unfortunate that "res" is short for both result and resolution. Finally, are the physical dimensions actually presented in 100ths of millimeters?

Thanks,
Henrik

Ping Cheng

author

2011-01-24 18:18 UTC #2 lore raw

Quoted 15 lines · Henrik Rydberg

Hi Ping,
Quoted 10 lines
(quoted diff) +0 -0
@@ -1228,8 +1228,13 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
case TABLETPC:
if (features->device_type == BTN_TOOL_DOUBLETAP || features->device_type == BTN_TOOL_TRIPLETAP) {
- input_set_abs_params(input_dev, ABS_RX, 0, features->x_phy, 0, 0);
- input_set_abs_params(input_dev, ABS_RY, 0, features->y_phy, 0, 0);
+ /* set touch resolution in points/mm */
+ res = features->x_phy/100;
+ input_abs_set_res(input_dev, ABS_X,
+ features->x_max/res);
It seems to me the construction above looses precision twice for no apparent reason.

Well, there was a reason (logically instead of mathematically though):
features->x_phy/100 gives us the physical size of the touch area in mm.

Quoted 2 lines

How about instead use "100 * features->x_max /
features->x_phy"?

Sure.

Quoted 2 lines

Besides, it is a bit unfortunate that "res" is short for both result and resolution.

Resolution is defined as __s32 in input_absinfo. How about we use __s32 then?

Quoted 2 lines

Finally, are the physical dimensions
actually presented in 100ths of millimeters?

Well, the protocol says it is in thousandth of a centimeter. So, 100th of a mm is right too.

Thank you for the comments.

Ping
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Dmitry Torokhov

maintainer

2011-01-24 18:37 UTC #3 lore raw

Quoted 24 lines · Ping Cheng

Quoted 15 lines · Henrik Rydberg
Hi Ping,
Quoted 10 lines
(quoted diff) +0 -0
@@ -1228,8 +1228,13 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
case TABLETPC:
if (features->device_type == BTN_TOOL_DOUBLETAP || features->device_type == BTN_TOOL_TRIPLETAP) {
- input_set_abs_params(input_dev, ABS_RX, 0, features->x_phy, 0, 0);
- input_set_abs_params(input_dev, ABS_RY, 0, features->y_phy, 0, 0);
+ /* set touch resolution in points/mm */
+ res = features->x_phy/100;
+ input_abs_set_res(input_dev, ABS_X,
+ features->x_max/res);
It seems to me the construction above looses precision twice for no apparent reason.
Well, there was a reason (logically instead of mathematically though):
features->x_phy/100 gives us the physical size of the touch area in mm.
Quoted 2 lines
How about instead use "100 * features->x_max /
features->x_phy"?

Quoted 6 lines

Sure.
Quoted 2 lines
Besides, it is a bit unfortunate that "res" is short for both result and resolution.
Resolution is defined as __s32 in input_absinfo. How about we use __s32 then?

input_abs_set_res() takes and int argument so keep it int (or just embed the expression into the call and forgo 'res').

Signature · 5 lines

Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ping Cheng

author

2011-01-24 20:09 UTC #4 lore raw

Quoted 27 lines · Dmitry Torokhov

Quoted 26 lines · Ping Cheng
Quoted 15 lines · Henrik Rydberg
Hi Ping,
Quoted 10 lines
(quoted diff) +0 -0
@@ -1228,8 +1228,13 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
case TABLETPC:
if (features->device_type == BTN_TOOL_DOUBLETAP || features->device_type == BTN_TOOL_TRIPLETAP) {
- input_set_abs_params(input_dev, ABS_RX, 0, features->x_phy, 0, 0);
- input_set_abs_params(input_dev, ABS_RY, 0, features->y_phy, 0, 0);
+ /* set touch resolution in points/mm */
+ res = features->x_phy/100;
+ input_abs_set_res(input_dev, ABS_X,
+ features->x_max/res);
It seems to me the construction above looses precision twice for no apparent reason.
Well, there was a reason (logically instead of mathematically though):
features->x_phy/100 gives us the physical size of the touch area in mm.
Quoted 2 lines
How about instead use "100 * features->x_max /
features->x_phy"?
Sure.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Besides, it is a bit unfortunate that "res" is short for both result and resolution.
Resolution is defined as __s32 in input_absinfo. How about we use __s32 then?
input_abs_set_res() takes and int argument so keep it int (or just embed the expression into the call and forgo 'res').

I added 'res' to avoid confusion hence questions about the formula. I am still questioned ;).

I'll forgo 'res' in v2. Thank you for the suggestions.

Ping
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html