Bumping since it seems to have been lost between the cracks. 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.... On Wed, Apr 8, 2020 at 7:59 AM Gerecke, Jason <killertofu@xxxxxxxxx> wrote: > > From: Jason Gerecke <killertofu@xxxxxxxxx> > > This reverts commit 15893fa40109f5e7c67eeb8da62267d0fdf0be9d. > > The referenced commit broke pen and touch input for a variety of devices > such as the Cintiq Pro 32. Affected devices may appear to work normally > for a short amount of time, but eventually loose track of actual touch > state and can leave touch arbitration enabled which prevents the pen > from working. The commit is not itself required for any currently-available > Bluetooth device, and so we revert it to correct the behavior of broken > devices. > > This breakage occurs due to a mismatch between the order of collections > and the order of usages on some devices. This commit tries to read the > contact count before processing events, but will fail if the contact > count does not occur prior to the first logical finger collection. This > is the case for devices like the Cintiq Pro 32 which place the contact > count at the very end of the report. > > Without the contact count set, touches will only be partially processed. > The `wacom_wac_finger_slot` function will not open any slots since the > number of contacts seen is greater than the expectation of 0, but we will > still end up calling `input_mt_sync_frame` for each finger anyway. This > can cause problems for userspace separate from the issue currently taking > place in the kernel. Only once all of the individual finger collections > have been processed do we finally get to the enclosing collection which > contains the contact count. The value ends up being used for the *next* > report, however. > > This delayed use of the contact count can cause the driver to loose track > of the actual touch state and believe that there are contacts down when > there aren't. This leaves touch arbitration enabled and prevents the pen > from working. It can also cause userspace to incorrectly treat single- > finger input as gestures. > > Link: https://github.com/linuxwacom/input-wacom/issues/146 > Signed-off-by: Jason Gerecke <jason.gerecke@xxxxxxxxx> > Reviewed-by: Aaron Armstrong Skomra <aaron.skomra@xxxxxxxxx> > Fixes: 15893fa40109 ("HID: wacom: generic: read the number of expected touches on a per collection basis") > Cc: stable@xxxxxxxxxxxxxxx # 5.3+ > --- > drivers/hid/wacom_wac.c | 79 +++++++++-------------------------------- > 1 file changed, 16 insertions(+), 63 deletions(-) > > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c > index d99a9d407671..96d00eba99c0 100644 > --- a/drivers/hid/wacom_wac.c > +++ b/drivers/hid/wacom_wac.c > @@ -2637,9 +2637,25 @@ static void wacom_wac_finger_pre_report(struct hid_device *hdev, > case HID_DG_TIPSWITCH: > hid_data->last_slot_field = equivalent_usage; > break; > + case HID_DG_CONTACTCOUNT: > + hid_data->cc_report = report->id; > + hid_data->cc_index = i; > + hid_data->cc_value_index = j; > + break; > } > } > } > + > + if (hid_data->cc_report != 0 && > + hid_data->cc_index >= 0) { > + struct hid_field *field = report->field[hid_data->cc_index]; > + int value = field->value[hid_data->cc_value_index]; > + if (value) > + hid_data->num_expected = value; > + } > + else { > + hid_data->num_expected = wacom_wac->features.touch_max; > + } > } > > static void wacom_wac_finger_report(struct hid_device *hdev, > @@ -2649,7 +2665,6 @@ static void wacom_wac_finger_report(struct hid_device *hdev, > struct wacom_wac *wacom_wac = &wacom->wacom_wac; > struct input_dev *input = wacom_wac->touch_input; > unsigned touch_max = wacom_wac->features.touch_max; > - struct hid_data *hid_data = &wacom_wac->hid_data; > > /* If more packets of data are expected, give us a chance to > * process them rather than immediately syncing a partial > @@ -2663,7 +2678,6 @@ static void wacom_wac_finger_report(struct hid_device *hdev, > > input_sync(input); > wacom_wac->hid_data.num_received = 0; > - hid_data->num_expected = 0; > > /* keep touch state for pen event */ > wacom_wac->shared->touch_down = wacom_wac_finger_count_touches(wacom_wac); > @@ -2738,73 +2752,12 @@ static void wacom_report_events(struct hid_device *hdev, > } > } > > -static void wacom_set_num_expected(struct hid_device *hdev, > - struct hid_report *report, > - int collection_index, > - struct hid_field *field, > - int field_index) > -{ > - struct wacom *wacom = hid_get_drvdata(hdev); > - struct wacom_wac *wacom_wac = &wacom->wacom_wac; > - struct hid_data *hid_data = &wacom_wac->hid_data; > - unsigned int original_collection_level = > - hdev->collection[collection_index].level; > - bool end_collection = false; > - int i; > - > - if (hid_data->num_expected) > - return; > - > - // find the contact count value for this segment > - for (i = field_index; i < report->maxfield && !end_collection; i++) { > - struct hid_field *field = report->field[i]; > - unsigned int field_level = > - hdev->collection[field->usage[0].collection_index].level; > - unsigned int j; > - > - if (field_level != original_collection_level) > - continue; > - > - for (j = 0; j < field->maxusage; j++) { > - struct hid_usage *usage = &field->usage[j]; > - > - if (usage->collection_index != collection_index) { > - end_collection = true; > - break; > - } > - if (wacom_equivalent_usage(usage->hid) == HID_DG_CONTACTCOUNT) { > - hid_data->cc_report = report->id; > - hid_data->cc_index = i; > - hid_data->cc_value_index = j; > - > - if (hid_data->cc_report != 0 && > - hid_data->cc_index >= 0) { > - > - struct hid_field *field = > - report->field[hid_data->cc_index]; > - int value = > - field->value[hid_data->cc_value_index]; > - > - if (value) > - hid_data->num_expected = value; > - } > - } > - } > - } > - > - if (hid_data->cc_report == 0 || hid_data->cc_index < 0) > - hid_data->num_expected = wacom_wac->features.touch_max; > -} > - > static int wacom_wac_collection(struct hid_device *hdev, struct hid_report *report, > int collection_index, struct hid_field *field, > int field_index) > { > struct wacom *wacom = hid_get_drvdata(hdev); > > - if (WACOM_FINGER_FIELD(field)) > - wacom_set_num_expected(hdev, report, collection_index, field, > - field_index); > wacom_report_events(hdev, report, collection_index, field_index); > > /* > -- > 2.26.0 >
