reduce Surface Pro 3 multitouch jitter

The firmware found in the touch screen of the Surface Pro 3 is slightly buggy and occasionally doesn't send lift off reports for contacts; add MT_QUIRK_NOT_SEEN_MEANS_UP to .quirks to compensate for the missed reports.

Signed-off-by Joey Pabalinas <[email protected]>

 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/hid/hid-multitouch.c +0 -0

index 45968f7970f87775fa..a793076139d7d0db9b 100644

drivers/hid/hid-multitouch.c +0 -1

        { .name = MT_CLS_EXPORT_ALL_INPUTS,
                .quirks = MT_QUIRK_ALWAYS_VALID |
                        MT_QUIRK_CONTACT_CNT_ACCURATE,
                .export_all_inputs = true },
        { .name = MT_CLS_WIN_8_DUAL,
--

2.18.0

NACK on this.
If the Surface has a buggy firmware, we should handle it as a special case, not as a common failure.
Also, I am not sure this quirk is compatible with Win 8 specification. Last, we now have a timeout for unreleased touches, so I really don't think you need that in recent kernels.

Cheers,
Benjamin

Quoted 8 lines

{ .name = MT_CLS_EXPORT_ALL_INPUTS,
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_CONTACT_CNT_ACCURATE,
.export_all_inputs = true },
{ .name = MT_CLS_WIN_8_DUAL,
--
2.18.0

The firmware found in the touch screen of the Surface Pro 3 is slightly buggy and occasionally doesn't send lift off reports for contacts; add MT_QUIRK_NOT_SEEN_MEANS_UP to .quirks to compensate for the missed reports.

Signed-off-by Joey Pabalinas <[email protected]>

The HID_GROUP_MULTITOUCH_WIN_8 group never needs to check for the serial protocol, so avoid setting td->serial_maybe = true; in order to avoid an unnecessary mt_post_parse_default_settings() call

Signed-off-by Joey Pabalinas <[email protected]>

 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/hid/hid-multitouch.c +0 -0

index a793076139d7d0db9b..c0654db0b736543ca0 100644

drivers/hid/hid-multitouch.c +0 -1

                td->serial_maybe = true;
 
        /* This allows the driver to correctly support devices
         * that emit events over several HID messages.
         */
--

2.18.0

There is a tiny difference between the HID group (this device looks like it is used as a Win 8 device) and the device class (this is effectively a Win8 device)

It makes sense to remove this check for Win8 devices, but I don't think it should be done for all devices.

All in all, it won't change much.

Avoid processing reports containing invalid values to reduce multitouch input stutter.

Signed-off-by Joey Pabalinas <[email protected]>

 1 file changed, 9 insertions(+)

drivers/hid/hid-multitouch.c +0 -0

index c0654db0b736543ca0..08b50e5908cecdda66 100644

drivers/hid/hid-multitouch.c +4 -1

+               td->num_received++;
+               return;
+       }
+
        if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
                int active;
                int slotnum = mt_compute_slot(td, input);
                struct mt_slot *s = &td->curdata;
                struct input_mt *mt = input->mt;
--

2.18.0

Hi Joey,
You can't really use plain values like that. There is a tiny chance these values are valid on an other device.
IIRC, MS spec says that we should ignore out of band values if they are tagged as such. Such input are tagged with NULL values
(http://www.usb.org/developers/hidpage/HID1_11.pdf page 31) and MS spec mentioned this.

All in all, if you have this bit set, you need to compare the value with the logical_max/min for each field.

I never encountered a device that required this, so you are probably the lucky one :)

Elide lone else cases and replace else if clauses with plain if conditionals when they occur immediately after return statements.

Signed-off-by Joey Pabalinas <[email protected]>

 1 file changed, 6 insertions(+), 5 deletions(-)

drivers/hid/hid-multitouch.c +6 -6

index 08b50e5908cecdda66..30b1a2c67f39a41325 100644
@@ -205,12 +205,12 @@ static void mt_post_parse(struct mt_device *td);
 
 static int cypress_compute_slot(struct mt_device *td)
 {
        if (td->curdata.contactid != 0 || td->num_received == 0)
                return td->curdata.contactid;
-       else
-               return -1;
+
+       return -1;
 }
 
 static struct mt_class mt_classes[] = {
        { .name = MT_CLS_DEFAULT,
                .quirks = MT_QUIRK_ALWAYS_VALID |

@@ -803,12 +803,12 @@ static int mt_compute_timestamp(struct mt_device *td, struct hid_field *field,
        delta *= 100;
 
        if (jdelta > MAX_TIMESTAMP_INTERVAL)
                /* No data received for a while, resync the timestamp. */
                return 0;
-       else
-               return td->timestamp + delta;
+
+       return td->timestamp + delta;
 }
 
 static int mt_touch_event(struct hid_device *hid, struct hid_field *field,
                                struct hid_usage *usage, __s32 value)
 {

@@ -1110,11 +1110,12 @@ static int mt_input_mapping(struct hid_device *hdev, struct hid_input *hi,
         * HID_DG_CONTACTCOUNT from the pen report as it is outside the physical
         * collection, but within the report ID.
         */
        if (field->physical == HID_DG_STYLUS)
                return 0;
-       else if ((field->physical == 0) &&
+
+       if ((field->physical == 0) &&
                 (field->report->id != td->mt_report_id) &&
                 (td->mt_report_id != -1))
                return 0;
 
        if (field->application == HID_DG_TOUCHSCREEN ||
--

2.18.0

This one looks good.

Reviewed-by Benjamin Tissoires <[email protected]>

Just FYI, I sent out a big refactor of the hid-multitouch code. Jiri should be still reviewing it, so I am not so sure who will have to rebase which series on top of the other, but someone between us will have to do it :)