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).
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html