On Tue, Aug 29, 2017 at 11:34 AM, Ping Cheng <pinglinux@xxxxxxxxx> wrote:
> On Thu, Aug 24, 2017 at 4:09 PM, Jason Gerecke <killertofu@xxxxxxxxx> wrote:
>> The MobileStudio Pro, Cintiq Pro, and 2nd-gen Intuos Pro devices use a
>> different coordinate system for their touchring and pen twist than prior
>> devices. Prior devices had zero aligned to the tablet's left and would
>> increase clockwise. Userspace expects data from the kernel to be in this
>> old coordinate space, so adjustments are necessary.
>>
>> While the coordinate system for pen twist is formally defined by the HID
>> standard, no such definition existed for the touchring at the time these
>> tablets were introduced. Future tablets are expected to report touchring
>> data using the same "zero-up clockwise-increasing" coordinate system
>> defined for twist.
>>
>> Fixes: 60a2218698 ("HID: wacom: generic: add support for touchring")
>> Fixes: 50066a042d ("HID: wacom: generic: Add support for height, tilt, and twist usages")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Jason Gerecke <jason.gerecke@xxxxxxxxx>
>
> Thank you Jason for catching and correcting the issue.
>
> Intuos Pro 2 series supports wireless/bluetooth connection, which was
> implemented in its own routine. Should we also update that code path?
> Except that, the patch is:
>
> Reviewed-by: Ping Cheng <ping.cheng@xxxxxxxxx>
>
> Cheers,
> Ping
>
Good call. I've also found a second issue with the wireless codepath's
handling of negative numbers that also needs fixing. I'll send an
updated patchset shortly.
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
>> ---
>> drivers/hid/wacom_wac.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 51 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
>> index bb17d7bbefd3..7f0b3d497ecb 100644
>> --- a/drivers/hid/wacom_wac.c
>> +++ b/drivers/hid/wacom_wac.c
>> @@ -1616,6 +1616,20 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
>> return 0;
>> }
>>
>> +static int wacom_offset_rotation(struct input_dev *input, struct hid_usage *usage,
>> + int value, int num, int denom)
>> +{
>> + struct input_absinfo *abs = &input->absinfo[usage->code];
>> + int range = (abs->maximum - abs->minimum + 1);
>> +
>> + value += num*range/denom;
>> + if (value > abs->maximum)
>> + value -= range;
>> + else if (value < abs->minimum)
>> + value += range;
>> + return value;
>> +}
>> +
>> int wacom_equivalent_usage(int usage)
>> {
>> if ((usage & HID_USAGE_PAGE) == WACOM_HID_UP_WACOMDIGITIZER) {
>> @@ -1898,6 +1912,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
>> unsigned equivalent_usage = wacom_equivalent_usage(usage->hid);
>> int i;
>> bool is_touch_on = value;
>> + bool do_report = false;
>>
>> /*
>> * Avoid reporting this event and setting inrange_state if this usage
>> @@ -1912,6 +1927,29 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
>> }
>>
>> switch (equivalent_usage) {
>> + case WACOM_HID_WD_TOUCHRING:
>> + /*
>> + * Userspace expects touchrings to increase in value with
>> + * clockwise gestures and have their zero point at the
>> + * tablet's left. HID events "should" be clockwise-
>> + * increasing and zero at top, though the MobileStudio
>> + * Pro and 2nd-gen Intuos Pro don't do this...
>> + */
>> + if (hdev->vendor == 0x56a &&
>> + (hdev->product == 0x34d || hdev->product == 0x34e || /* MobileStudio Pro */
>> + hdev->product == 0x357 || hdev->product == 0x358)) { /* Intuos Pro 2 */
>> + value = (field->logical_maximum - value);
>> +
>> + if (hdev->product == 0x357 || hdev->product == 0x358)
>> + value = wacom_offset_rotation(input, usage, value, 3, 16);
>> + else if (hdev->product == 0x34d || hdev->product == 0x34e)
>> + value = wacom_offset_rotation(input, usage, value, 1, 2);
>> + }
>> + else {
>> + value = wacom_offset_rotation(input, usage, value, 1, 4);
>> + }
>> + do_report = true;
>> + break;
>> case WACOM_HID_WD_TOUCHRINGSTATUS:
>> if (!value)
>> input_event(input, usage->type, usage->code, 0);
>> @@ -1945,10 +1983,14 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
>> value, i);
>> /* fall through*/
>> default:
>> + do_report = true;
>> + break;
>> + }
>> +
>> + if (do_report) {
>> input_event(input, usage->type, usage->code, value);
>> if (value)
>> wacom_wac->hid_data.pad_input_event_flag = true;
>> - break;
>> }
>> }
>>
>> @@ -2089,6 +2131,14 @@ static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field
>> wacom_wac->serial[0] = (wacom_wac->serial[0] & ~0xFFFFFFFFULL);
>> wacom_wac->serial[0] |= (__u32)value;
>> return;
>> + case HID_DG_TWIST:
>> + /*
>> + * Userspace expects pen twist to have its zero point when
>> + * the buttons/finger is on the tablet's left. HID values
>> + * are zero when buttons are toward the top.
>> + */
>> + value = wacom_offset_rotation(input, usage, value, 1, 4);
>> + break;
>> case WACOM_HID_WD_SENSE:
>> wacom_wac->hid_data.sense_state = value;
>> return;
>> --
>> 2.14.1
>>