* wacom: Fixes for stylus pressure values for Thinkpad Yoga @ 2014-02-26 22:38 Carl Worth 2014-02-26 22:38 ` [PATCH 1/3] Input: wacom - EC tablet supports 1023 levels of pressure, not just 255 Carl Worth 2014-02-27 4:39 ` wacom: Fixes for stylus pressure values for Thinkpad Yoga Ping Cheng 0 siblings, 2 replies; 6+ messages in thread From: Carl Worth @ 2014-02-26 22:38 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input, linux-kernel 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). ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] Input: wacom - EC tablet supports 1023 levels of pressure, not just 255 2014-02-26 22:38 wacom: Fixes for stylus pressure values for Thinkpad Yoga Carl Worth @ 2014-02-26 22:38 ` Carl Worth 2014-02-26 22:38 ` [PATCH 2/3] Input: wacom - Don't discard bits from upper byte of pressure value Carl Worth 2014-02-27 4:39 ` wacom: Fixes for stylus pressure values for Thinkpad Yoga Ping Cheng 1 sibling, 1 reply; 6+ messages in thread From: Carl Worth @ 2014-02-26 22:38 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] Input: wacom - Don't discard bits from upper byte of pressure value 2014-02-26 22:38 ` [PATCH 1/3] Input: wacom - EC tablet supports 1023 levels of pressure, not just 255 Carl Worth @ 2014-02-26 22:38 ` Carl Worth 2014-02-26 22:38 ` [PATCH 3/3] Input: wacom - Avoid incorrect sign extension from pressure-value lower byte Carl Worth 0 siblings, 1 reply; 6+ messages in thread From: Carl Worth @ 2014-02-26 22:38 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] Input: wacom - Avoid incorrect sign extension from pressure-value lower byte 2014-02-26 22:38 ` [PATCH 2/3] Input: wacom - Don't discard bits from upper byte of pressure value Carl Worth @ 2014-02-26 22:38 ` Carl Worth 0 siblings, 0 replies; 6+ messages in thread From: Carl Worth @ 2014-02-26 22:38 UTC (permalink / raw) To: Ping Cheng; +Cc: linux-input, linux-kernel, 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: wacom: Fixes for stylus pressure values for Thinkpad Yoga 2014-02-26 22:38 wacom: Fixes for stylus pressure values for Thinkpad Yoga Carl Worth 2014-02-26 22:38 ` [PATCH 1/3] Input: wacom - EC tablet supports 1023 levels of pressure, not just 255 Carl Worth @ 2014-02-27 4:39 ` Ping Cheng 2014-02-28 18:26 ` Carl Worth 1 sibling, 1 reply; 6+ messages in thread From: Ping Cheng @ 2014-02-27 4:39 UTC (permalink / raw) To: Carl Worth, Dmitry Torokhov, Jason Gerecke Cc: linux-input, linux-kernel@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: wacom: Fixes for stylus pressure values for Thinkpad Yoga 2014-02-27 4:39 ` wacom: Fixes for stylus pressure values for Thinkpad Yoga Ping Cheng @ 2014-02-28 18:26 ` Carl Worth 0 siblings, 0 replies; 6+ messages in thread From: Carl Worth @ 2014-02-28 18:26 UTC (permalink / raw) To: Ping Cheng, Dmitry Torokhov, Jason Gerecke Cc: linux-input, linux-kernel@vger.kernel.org [-- 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 --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-02-28 18:26 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-26 22:38 wacom: Fixes for stylus pressure values for Thinkpad Yoga Carl Worth 2014-02-26 22:38 ` [PATCH 1/3] Input: wacom - EC tablet supports 1023 levels of pressure, not just 255 Carl Worth 2014-02-26 22:38 ` [PATCH 2/3] Input: wacom - Don't discard bits from upper byte of pressure value Carl Worth 2014-02-26 22:38 ` [PATCH 3/3] Input: wacom - Avoid incorrect sign extension from pressure-value lower byte Carl Worth 2014-02-27 4:39 ` wacom: Fixes for stylus pressure values for Thinkpad Yoga Ping Cheng 2014-02-28 18:26 ` Carl Worth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).