On 07/12/2016 12:32 AM, Benjamin Tissoires wrote: > Hi Jason, > > On Jul 11 2016 or thereabouts, Jason Gerecke wrote: >> If a touchscreen contains both multitouch and single-touch reports in its >> descriptor in that order, the driver may overwrite information it saved >> about the format of the multitouch report. This can cause the report >> processing code to get tripped up and send an incorrect event stream to >> userspace. >> >> In particular, this can cause last_slot_field to be overwritten with the >> result that the driver prematurely assumes it has finished processing a >> slot and sending the ABS_MT_SLOT event at the wrong point in time, >> associating events for the current contact with the following contact >> instead. >> >> To prevent this from occuring, we stop mapping usages after having seen >> HID_DG_CONTACTCOUNT. This usage is only present in multitouch reports, >> so the format of any following single-touch reports will have no effect. > > I had a quick look at the driver, and it looks like the Cintiq Companion > 2 has more than one multitouch collection (see 499522c "HID: wacom: Tie > cached HID_DG_CONTACTCOUNT indices to report ID"). > > So if this doesn't break the cintiq companion 2, you have my > reviewed-by, but I'd rather be sure (and please see if we really need to > keep 499522c then). > > Cheers, > Benjamin > Thanks for the reminder about 499522c. This patch should supersede it in terms of functionality, so I can make a patch to remove it instead. As far as compatibility goes, I don't have a Companion 2 to actually test, but looking at the descriptor [1], it shouldn't pose any problems: the second report containing HID_DG_CONTACTCOUNT is the single-touch report that we don't want to use anyway. [1]: https://github.com/linuxwacom/wacom-hid-descriptors/blob/master/Wacom%20Cintiq%20Companion%202/0003:056A:0326.0002.hid.txt 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.... >> >> Signed-off-by: Jason Gerecke <jason.gerecke@xxxxxxxxx> >> --- >> drivers/hid/wacom_wac.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c >> index fcf2264..e499cdb 100644 >> --- a/drivers/hid/wacom_wac.c >> +++ b/drivers/hid/wacom_wac.c >> @@ -1560,6 +1560,11 @@ static void wacom_wac_finger_usage_mapping(struct hid_device *hdev, >> struct input_dev *input = wacom_wac->touch_input; >> unsigned touch_max = wacom_wac->features.touch_max; >> >> + /* stop processing after the first multitouch report */ >> + if (wacom_wac->hid_data.cc_report && >> + wacom_wac->hid_data.cc_report != field->report->id) >> + return; >> + >> switch (usage->hid) { >> case HID_GD_X: >> features->last_slot_field = usage->hid; >> -- >> 2.9.0 >> -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
