wacom: Fixes for stylus pressure values for Thinkpad Yoga

wacom: Fixes for stylus pressure values for Thinkpad Yoga

[Carl Worth]

This series of patches fixes the pressure values reported for the
wacom tablet built-in to a Lenovo ThinkPad Yoga laptop. Prior to this
patch series, if I slowly increased stylus pressure, (expecting a
gradual increase of values from 0 to 1023), I instead received values
that increased slowly to 255, then reset to 0 and increased slowly
again, etc.

The buggy arithmetic that is updated here appears to exist in
identical forms for other drivers. I did not update any code that I
was not able to test directly. But it looks like wacom_pl_irq and
wacom_dtu_irq potentially have similar bugs, (depending on the actual
pressure_max values encountered in practice).

[PATCH 1/3] Input: wacom - EC tablet supports 1023 levels of pressure, not just 255

[Carl Worth]

This was verified on a ThinkPad Yoga laptop with an integrated Wacom
tablet which reports itself with an ID of 0xEC.

Signed-off-by: Carl Worth <cworth@cworth.org>
---
 drivers/input/tablet/wacom_wac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 782c253..4958081 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -2109,7 +2109,7 @@ static const struct wacom_features wacom_features_0xE6 =
 	  0, TABLETPC2FG, WACOM_INTUOS_RES, WACOM_INTUOS_RES,
 	  .touch_max = 2 };
 static const struct wacom_features wacom_features_0xEC =
-	{ "Wacom ISDv4 EC",       WACOM_PKGLEN_GRAPHIRE,  25710, 14500,  255,
+	{ "Wacom ISDv4 EC",       WACOM_PKGLEN_GRAPHIRE,  25710, 14500,  1023,
 	  0, TABLETPC,    WACOM_INTUOS_RES, WACOM_INTUOS_RES };
 static const struct wacom_features wacom_features_0xED =
 	{ "Wacom ISDv4 ED",       WACOM_PKGLEN_GRAPHIRE,  26202, 16325,  255,
-- 
1.9.0

[PATCH 2/3] Input: wacom - Don't discard bits from upper byte of pressure value

[Carl Worth]

The mask here previously discarded all but one bit of data from the
upper byte of the pressure value. That would be correct for a tablet
with at most 512 pressure values, but is incorrect for a tablet with
1024 or more pressure values.

We can support as many tablets as possible by simply not discarding
any of the bits from this byte.

Signed-off-by: Carl Worth <cworth@cworth.org>
---
 drivers/input/tablet/wacom_wac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 4958081..563f197 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -1037,7 +1037,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
 		input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
 		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
 		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
-		pressure = ((data[7] & 0x01) << 8) | data[6];
+		pressure = (data[7] << 8) | data[6];
 		if (pressure < 0)
 			pressure = features->pressure_max + pressure + 1;
 		input_report_abs(input, ABS_PRESSURE, pressure);
-- 
1.9.0

[PATCH 3/3] Input: wacom - Avoid incorrect sign extension from pressure-value lower byte

[Carl Worth]

Previously, whenever the lower byte was greater than 127, the sign
extension of the "char" during integer promotion would result in a
negative value for pressure. There was code in place to adjust a
negative value back to positive by subtracting from pressure_max, but
this code was only correct with a tablet with a maximum of 256
pressure values.

By switching from "char" to "unsigned char" we can avoid the sign
extension altogether, eliminate the code to adjust values, and obtain
correct pressure results.

With this code in place, I am now obtaining correct pressure results
from the Wacom tablet built into a ThinkPad Yoga laptop. (Prior to
this fix, gradual increases of pressure would result in pressure
values that would reset from 255 to 0 rathern than simply increasing.)

Signed-off-by: Carl Worth <cworth@cworth.org>
---
 drivers/input/tablet/wacom_wac.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
index 563f197..93f7440 100644
--- a/drivers/input/tablet/wacom_wac.c
+++ b/drivers/input/tablet/wacom_wac.c
@@ -1018,8 +1018,7 @@ static int wacom_tpc_single_touch(struct wacom_wac *wacom, size_t len)
 
 static int wacom_tpc_pen(struct wacom_wac *wacom)
 {
-	struct wacom_features *features = &wacom->features;
-	char *data = wacom->data;
+	unsigned char *data = wacom->data;
 	struct input_dev *input = wacom->input;
 	int pressure;
 	bool prox = data[1] & 0x20;
@@ -1038,8 +1037,6 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
 		input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
 		input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
 		pressure = (data[7] << 8) | data[6];
-		if (pressure < 0)
-			pressure = features->pressure_max + pressure + 1;
 		input_report_abs(input, ABS_PRESSURE, pressure);
 		input_report_key(input, BTN_TOUCH, data[1] & 0x05);
 		input_report_key(input, wacom->tool[0], prox);
-- 
1.9.0

Re: wacom: Fixes for stylus pressure values for Thinkpad Yoga

[Ping Cheng]

Hi Carl,

Thank you for the heads up. I believe Jason's patchset 4 of 4
(http://www.spinics.net/lists/linux-input/msg29435.html) fixed the
issue for your device and for other's. The patch was submitted last
month. If you can test the set on your device and give us a Tested-by
here, it will help Dmitry to merge the patch upstream.

Thank you for your effort.

Ping

On Wed, Feb 26, 2014 at 2:38 PM, Carl Worth <cworth@cworth.org> wrote:
> This series of patches fixes the pressure values reported for the
> wacom tablet built-in to a Lenovo ThinkPad Yoga laptop. Prior to this
> patch series, if I slowly increased stylus pressure, (expecting a
> gradual increase of values from 0 to 1023), I instead received values
> that increased slowly to 255, then reset to 0 and increased slowly
> again, etc.
>
> The buggy arithmetic that is updated here appears to exist in
> identical forms for other drivers. I did not update any code that I
> was not able to test directly. But it looks like wacom_pl_irq and
> wacom_dtu_irq potentially have similar bugs, (depending on the actual
> pressure_max values encountered in practice).
>
> --
> 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: wacom: Fixes for stylus pressure values for Thinkpad Yoga

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

Ping Cheng <pinglinux@gmail.com> writes:
> Thank you for the heads up. I believe Jason's patchset 4 of 4
> (http://www.spinics.net/lists/linux-input/msg29435.html) fixed the
> issue for your device and for other's. The patch was submitted last
> month. If you can test the set on your device and give us a Tested-by
> here, it will help Dmitry to merge the patch upstream.
>
> Thank you for your effort.

Thanks, Ping.

Patches 2, 3 and 4 of Dmitry's series do everything that my series does,
(and a bit more since he also fixes the "unsigned char" for cases
besides the specific one I hit).

So those all get my:

Reviewed-by: Carl Worth <cworth@cworth.org>

I'll follow up more if I get a chance to test these as well.

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]