Hi Daniel,
On Thu, Sep 17, 2015 at 11:06 AM, Daniel Martin
<daniel.martin@xxxxxxxxxxx> wrote:
> The N-trig (1b96:1B05) is an I2C device. It can be found in a
> Microsoft Surface Pro 3. Users reported that sometimes the touschscreen
> gets stuck during work - not responding to touches anymore.
>
> Under certain circumstances the touschscreen sends "ghost" reports.
> Reports for contacts that have been lifted prior and with suspicious
> data (ContactID == X == Y == W == H == 0xffff and Tipswitch == true). In
> such a case evemu-record would report something like:
>
> E: 44.290448 0003 002f 0006 # EV_ABS / ABS_MT_SLOT 6
> E: 44.290448 0003 0039 0024 # EV_ABS / ABS_MT_TRACKING_ID 24
> E: 44.290448 0003 0035 65535 # EV_ABS / ABS_MT_POSITION_X 65535
> E: 44.290448 0003 0036 65535 # EV_ABS / ABS_MT_POSITION_Y 65535
> E: 44.290448 0003 003c 65535 # EV_ABS / ABS_MT_TOOL_X 65535
> E: 44.290448 0003 003d 65535 # EV_ABS / ABS_MT_TOOL_Y 65535
> E: 44.290448 0003 0034 0000 # EV_ABS / ABS_MT_ORIENTATION 0
> E: 44.290448 0003 0030 32767 # EV_ABS / ABS_MT_TOUCH_MAJOR 32767
> E: 44.290448 0003 0031 32767 # EV_ABS / ABS_MT_TOUCH_MINOR 32767
>
> We never see that such a contact gets lifted (Tipswitch == false) and
> therefore have an active unused slot. If we keep processing we end up
> with all slots active, but none in use. Then input_mt_get_slot_by_key()
> always returns -1 (can't get a slot) and we never send any event
> anymore. MT_QUIRK_NOT_SEEN_MEANS_UP could fix this, but we don't want
> to send such suspicious values to user-land.
> Instead, we add MT_QUIRK_INVALID_CONTACTID_FFFF for this device and
> don't try to get a slot for a report with ContactID=0xffff. Though, now,
> we may miss a one valid contact (per input descriptor a contact id of
> 0xffff is valid, that's its logic maximum).
Facepalm. How did they managed to have such a bug in their protocol on
their own device???? This is really depressing.
This fix is fine IMO. 0xffff seems unlikely to be used before long
after boot, and I wouldn't be surprised if they just roll back to 0
when they arrive at 0xff for the contact ID.
Small remark, we may not need to add the device ID to hid-ids.h given
that it is not used anywhere else than in hid-multitouch.c.
Besides this:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
Thanks for the analysis and the patch!
Cheers,
Benjamin
>
> Lets look at input reports unveiling the problem. First 3 bytes
> (Length=29 and ReportId=3) and last 4 bytes (ScanTime) have been
> stripped.
>
> 1. frame - 10 reports (10 contacts active):
> ca 01 e7 31 00 3b 12 3b 12 15 05 15 05 69 02 76 03 ff ff ff ff 0a
> ca 01 e7 32 00 ba 0c ba 0c a1 01 a1 01 d3 01 c5 02 ff ff ff ff 00
> ca 01 e7 33 00 a8 07 a8 07 5b 03 5b 03 d3 01 14 02 ff ff ff ff 00
> ca 01 e7 37 00 92 0f 92 0f 79 01 79 01 d3 01 77 03 ff ff ff ff 00
> ca 01 e7 3d 00 82 09 82 09 2b 18 2b 18 cf 04 72 03 ff ff ff ff 00
> ca 01 e7 38 00 a9 21 a9 21 80 07 80 07 6f 02 14 02 ff ff ff ff 00
> ca 01 e7 39 00 c0 1c c0 1c 2c 05 2c 05 6f 02 c5 02 ff ff ff ff 00
> ca 01 e7 3a 00 a7 19 a7 19 55 05 55 05 e0 01 22 02 ff ff ff ff 00
> ca 01 e7 3b 00 bd 15 bd 15 80 09 80 09 db 01 1d 02 ff ff ff ff 00
> ca 01 e7 3c 00 f2 16 f2 16 56 13 56 13 76 01 a4 01 ff ff ff ff 00
> v---- v- v---- v---- v---- v---- v---- v---- v---- v---------- v-
> V1 CT CID X X Y Y W H V2 CC
>
> V1 ... Usage Page (Vendor Defined 0xFF00), Usage (0x01) [1x2 bytes]
> Frame sequence id?
> TC ... Tipswitch (bit 0) and Confidence (bit 2)
> (Confidence bit is always set)
> CID .. ContactID
> X/Y .. X/Y coordinate (yes, always twice the same value)
> W/H .. Width/Height
> V2 ... Usage Page (Vendor Defined 0xFF00), Usage (0x02) [4x1 byte]
> (always ff, not const, valid from 0 to 255?)
> CC ... ContactCount
>
> We see that initially ContactCount is 10 and we get 10 reports. Each
> report shows an active slot (Tipswitch bit in TC is set, TC=e7).
> The contact ids are 31, 32, 33, 37, 38, 39, 3a, 3b, 3c and 3d.
>
> 2. frame - 10 reports (5 contacts active, 5 becoming inactive):
> cb 01 e4 31 00 3b 12 3b 12 15 05 15 05 69 02 76 03 ff ff ff ff 0a
> cb 01 e4 32 00 ba 0c ba 0c a1 01 a1 01 d3 01 c5 02 ff ff ff ff 00
> cb 01 e4 33 00 a8 07 a8 07 5b 03 5b 03 d3 01 14 02 ff ff ff ff 00
> cb 01 e4 37 00 92 0f 92 0f 79 01 79 01 d3 01 77 03 ff ff ff ff 00
> cb 01 e4 3d 00 82 09 82 09 2b 18 2b 18 cf 04 72 03 ff ff ff ff 00
> cb 01 e7 38 00 a9 21 a9 21 80 07 80 07 6f 02 14 02 ff ff ff ff 00
> cb 01 e7 39 00 c0 1c c0 1c 2c 05 2c 05 6f 02 c5 02 ff ff ff ff 00
> cb 01 e7 3a 00 a7 19 a7 19 57 05 57 05 e0 01 25 02 ff ff ff ff 00
> cb 01 e7 3b 00 bd 15 bd 15 81 09 81 09 dd 01 1f 02 ff ff ff ff 00
> cb 01 e7 3c 00 f6 16 f6 16 53 13 53 13 7e 01 ad 01 ff ff ff ff 00
>
> 5 contacts (31, 32, 33, 37 and 3d) become inactive (Tipswitch bit
> in TC is not set, TC=e4). Remaining contacts (38 to 3c) stay active.
>
> 3. frame - 10 reports (5 contacts active, 5 "ghosts"???)
> cc 01 e7 38 00 a9 21 a9 21 80 07 80 07 6f 02 14 02 ff ff ff ff 0a
> cc 01 e7 39 00 c0 1c c0 1c 2c 05 2c 05 6f 02 c5 02 ff ff ff ff 00
> cc 01 e7 3a 00 a6 19 a6 19 58 05 58 05 e2 01 27 02 ff ff ff ff 00
> cc 01 e7 3b 00 bc 15 bc 15 83 09 83 09 df 01 21 02 ff ff ff ff 00
> cc 01 e7 3c 00 f8 16 f8 16 4f 13 4f 13 85 01 b5 01 ff ff ff ff 00
> cc 01 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00
> cc 01 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00
> cc 01 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00
> cc 01 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00
> cc 01 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff 00
>
> The 5 active contacts (38 to 3c) from the previous frame stay active.
> Though, we get 5 suspicious reports! Ghosts from the last frame?
>
> 4. frame - 5 reports (5 contacts active)
> cd 01 e7 38 00 a9 21 a9 21 80 07 80 07 6f 02 14 02 ff ff ff ff 05
> cd 01 e7 39 00 c0 1c c0 1c 2c 05 2c 05 6f 02 c5 02 ff ff ff ff 00
> cd 01 e7 3a 00 a6 19 a6 19 5a 05 5a 05 e2 01 29 02 ff ff ff ff 00
> cd 01 e7 3b 00 bc 15 bc 15 84 09 84 09 df 01 24 02 ff ff ff ff 00
> cd 01 e7 3c 00 fb 16 fb 16 4c 13 4c 13 8b 01 bd 01 ff ff ff ff 00
>
> Next frame and we're back to normal mode/data.
>
> Signed-off-by: Daniel Martin <consume.noise@xxxxxxxxx>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-multitouch.c | 16 ++++++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b3b225b..f297416 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -708,6 +708,7 @@
> #define USB_DEVICE_ID_NOVATEK_MOUSE 0x1602
>
> #define USB_VENDOR_ID_NTRIG 0x1b96
> +#define I2C_DEVICE_ID_NTRIG_TOUCH_SCREEN 0x1B05
> #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN 0x0001
> #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_1 0x0003
> #define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_2 0x0004
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 7c81125..8547b2e 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -67,6 +67,7 @@ MODULE_LICENSE("GPL");
> #define MT_QUIRK_HOVERING (1 << 11)
> #define MT_QUIRK_CONTACT_CNT_ACCURATE (1 << 12)
> #define MT_QUIRK_FORCE_GET_FEATURE (1 << 13)
> +#define MT_QUIRK_INVALID_CONTACTID_FFFF (1 << 14)
>
> #define MT_INPUTMODE_TOUCHSCREEN 0x02
> #define MT_INPUTMODE_TOUCHPAD 0x03
> @@ -155,6 +156,7 @@ static void mt_post_parse(struct mt_device *td);
> #define MT_CLS_GENERALTOUCH_TWOFINGERS 0x0108
> #define MT_CLS_GENERALTOUCH_PWT_TENFINGERS 0x0109
> #define MT_CLS_VTL 0x0110
> +#define MT_CLS_NTRIG 0x0111
>
> #define MT_DEFAULT_MAXCONTACT 10
> #define MT_MAX_MAXCONTACT 250
> @@ -265,6 +267,10 @@ static struct mt_class mt_classes[] = {
> MT_QUIRK_CONTACT_CNT_ACCURATE |
> MT_QUIRK_FORCE_GET_FEATURE,
> },
> + { .name = MT_CLS_NTRIG,
> + .quirks = MT_QUIRK_ALWAYS_VALID |
> + MT_QUIRK_INVALID_CONTACTID_FFFF,
> + },
> { }
> };
>
> @@ -546,6 +552,10 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
> if (quirks & MT_QUIRK_SLOT_IS_CONTACTID_MINUS_ONE)
> return td->curdata.contactid - 1;
>
> + if (quirks & MT_QUIRK_INVALID_CONTACTID_FFFF &&
> + td->curdata.contactid == 0xffff)
> + return -1;
> +
> return input_mt_get_slot_by_key(input, td->curdata.contactid);
> }
>
> @@ -1277,6 +1287,12 @@ static const struct hid_device_id mt_devices[] = {
> MT_USB_DEVICE(USB_VENDOR_ID_TURBOX,
> USB_DEVICE_ID_TURBOX_TOUCHSCREEN_MOSART) },
>
> + /* N-trig */
> + { .driver_data = MT_CLS_NTRIG,
> + HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
> + USB_VENDOR_ID_NTRIG,
> + I2C_DEVICE_ID_NTRIG_TOUCH_SCREEN) },
> +
> /* Panasonic panels */
> { .driver_data = MT_CLS_PANASONIC,
> MT_USB_DEVICE(USB_VENDOR_ID_PANASONIC,
> --
> 2.1.4
>
--
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
