diff mbox series

hid-mt: Fix Cirque 121f touch release

Message ID 20201019130532.19959-1-jandryuk@gmail.com
State New
Headers show
Series hid-mt: Fix Cirque 121f touch release | expand

Commit Message

Jason Andryuk Oct. 19, 2020, 1:05 p.m. UTC
We're seeing the touchpad not send all the touch release events.  This
can result in the cursor getting stuck generating scroll events instead
of cursor movement for single finger motion.  With the cursor not
moving, users think the trackpad is broken.  With libinput-record, you
can see that it doesn't always get to a neutral state when there are no
fingers on the touchpad.

MT_QUIRK_STICKY_FINGERS was insufficient alone.  The timer often didn't
fire to release all the contacts.  MT_QUIRK_NOT_SEEN_MEANS_UP seems to
help with tracking the touches, and allows the timer to fire properly
when needed.

You can reproduce by touching the trackpad with 4 fingers spread out,
then pulling them all together and removing from the track pad.

libinput-record output without the patch:
  - evdev:
    - [  2, 591028,   4,   5, 2561700] # EV_MSC / MSC_TIMESTAMP        2561700
    - [  2, 591028,   0,   0,       0] # ------------ SYN_REPORT (0) ---------- +7ms
  - evdev:
    - [  2, 603307,   3,  57,      -1] # EV_ABS / ABS_MT_TRACKING_ID       -1
    - [  2, 603307,   1, 334,       1] # EV_KEY / BTN_TOOL_TRIPLETAP        1
    - [  2, 603307,   1, 335,       0] # EV_KEY / BTN_TOOL_QUADTAP          0
    - [  2, 603307,   4,   5, 2570700] # EV_MSC / MSC_TIMESTAMP        2570700
    - [  2, 603307,   0,   0,       0] # ------------ SYN_REPORT (0) ---------- +12ms

Note the lack of "# Touch device in neutral state".  There has been no
activity for 2 seconds, but the tripletap has not cleared.  If you place
a finger back on at this point, it gets interpreted as a quadtap.

With this patch:
  - evdev:
    - [1123, 442602,   3,  47,       3] # EV_ABS / ABS_MT_SLOT               3
    - [1123, 442602,   3,  54,     133] # EV_ABS / ABS_MT_POSITION_Y       133 (+4)
    - [1123, 442602,   3,  47,       2] # EV_ABS / ABS_MT_SLOT               2
    - [1123, 442602,   3,  53,     667] # EV_ABS / ABS_MT_POSITION_X       667 (-2)
    - [1123, 442602,   3,  54,     121] # EV_ABS / ABS_MT_POSITION_Y       121 (+4)
    - [1123, 442602,   3,  47,       1] # EV_ABS / ABS_MT_SLOT               1
    - [1123, 442602,   3,  57,      -1] # EV_ABS / ABS_MT_TRACKING_ID       -1
    - [1123, 442602,   1, 333,       1] # EV_KEY / BTN_TOOL_DOUBLETAP        1
    - [1123, 442602,   1, 334,       0] # EV_KEY / BTN_TOOL_TRIPLETAP        0
    - [1123, 442602,   3,   0,     667] # EV_ABS / ABS_X                   667 (+321)
    - [1123, 442602,   3,   1,     121] # EV_ABS / ABS_Y                   121 (-204)
    - [1123, 442602,   4,   5,  196500] # EV_MSC / MSC_TIMESTAMP        196500
    - [1123, 442602,   0,   0,       0] # ------------ SYN_REPORT (0) ---------- +5ms
  - evdev:
    - [1123, 544680,   3,  47,       2] # EV_ABS / ABS_MT_SLOT               2
    - [1123, 544680,   3,  57,      -1] # EV_ABS / ABS_MT_TRACKING_ID       -1
    - [1123, 544680,   3,  47,       3] # EV_ABS / ABS_MT_SLOT               3
    - [1123, 544680,   3,  57,      -1] # EV_ABS / ABS_MT_TRACKING_ID       -1
    - [1123, 544680,   1, 330,       0] # EV_KEY / BTN_TOUCH                 0
    - [1123, 544680,   1, 333,       0] # EV_KEY / BTN_TOOL_DOUBLETAP        0
    - [1123, 544680,   0,   0,       0] # ------------ SYN_REPORT (0) ---------- +102ms
                                     # Touch device in neutral state

Here the timer fired and cleared the lingering doubletap.

Create a new MT_CLS_CIRQUE by copying MT_CLS_WIN_8 and replacing
MT_QUIRK_ALWAYS_VALID with MT_QUIRK_NOT_SEEN_MEANS_UP
(MT_QUIRK_STICKY_FINGERS is already in MT_CLS_WIN_8).

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Cc: <stable@vger.kernel.org>

---
This is developed and tested against 5.4 and forward ported to latest
upstream.

I don't know if it needs to be quirked more specifically, but the
modalias is:
input:b0018v0488p121Fe0100-e0,1,3,4,k110,111,145,148,14A,14D,14E,14F,ra0,1,2F,35,36,37,39,m5,lsfw

I don't know if MT_CLS_CIRQUE needs the HID_DG_CONFIDENCE change, but it
is included to keep it as close as possible to MT_CLS_WIN_8.

I opened this bug originally with some libinput debugging info:
https://gitlab.freedesktop.org/libinput/libinput/-/issues/531

This might be related:
https://bugs.launchpad.net/dell-sputnik/+bug/1651635

With this patch I occasionally see an MSC_TIMESTAMP/SYN_REPORT come
through after "# Touch device in neutral stat".  Is that a problem, or
will it just be ignored?
---
 drivers/hid/hid-ids.h        |  3 +++
 drivers/hid/hid-multitouch.c | 18 +++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

Comments

Jason Andryuk Oct. 21, 2020, 7:11 p.m. UTC | #1
On Mon, Oct 19, 2020 at 9:05 AM Jason Andryuk <jandryuk@gmail.com> wrote:
>

> We're seeing the touchpad not send all the touch release events.  This

> can result in the cursor getting stuck generating scroll events instead

> of cursor movement for single finger motion.  With the cursor not

> moving, users think the trackpad is broken.  With libinput-record, you

> can see that it doesn't always get to a neutral state when there are no

> fingers on the touchpad.

>

> MT_QUIRK_STICKY_FINGERS was insufficient alone.  The timer often didn't

> fire to release all the contacts.  MT_QUIRK_NOT_SEEN_MEANS_UP seems to

> help with tracking the touches, and allows the timer to fire properly

> when needed.

>

> You can reproduce by touching the trackpad with 4 fingers spread out,

> then pulling them all together and removing from the track pad.


<snip>

> ---

> This is developed and tested against 5.4 and forward ported to latest

> upstream.


I installed Fedora 32 with kernel 5.6.6 to test out some other stuff,
and it is not reproducing with the steps above.  Is there some other
change that may have fixed the release tracking?  I was definitely
seeing it with 5.4.72.

Regards,
Jason
diff mbox series

Patch

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 74fc1df6e3c2..7e153cf3ab16 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -278,6 +278,9 @@ 
 
 #define USB_VENDOR_ID_CIDC		0x1677
 
+#define I2C_VENDOR_ID_CIRQUE		0x0488
+#define I2C_PRODUCT_ID_CIRQUE_121F	0x121F
+
 #define USB_VENDOR_ID_CJTOUCH		0x24b8
 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0020	0x0020
 #define USB_DEVICE_ID_CJTOUCH_MULTI_TOUCH_0040	0x0040
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index e3152155c4b8..96e653e50e83 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -208,6 +208,7 @@  static void mt_post_parse(struct mt_device *td, struct mt_application *app);
 #define MT_CLS_GOOGLE				0x0111
 #define MT_CLS_RAZER_BLADE_STEALTH		0x0112
 #define MT_CLS_SMART_TECH			0x0113
+#define MT_CLS_CIRQUE				0x0114
 
 #define MT_DEFAULT_MAXCONTACT	10
 #define MT_MAX_MAXCONTACT	250
@@ -367,6 +368,14 @@  static const struct mt_class mt_classes[] = {
 			MT_QUIRK_CONTACT_CNT_ACCURATE |
 			MT_QUIRK_SEPARATE_APP_REPORT,
 	},
+	{ .name = MT_CLS_CIRQUE,
+		.quirks = MT_QUIRK_NOT_SEEN_MEANS_UP |
+			MT_QUIRK_IGNORE_DUPLICATES |
+			MT_QUIRK_HOVERING |
+			MT_QUIRK_CONTACT_CNT_ACCURATE |
+			MT_QUIRK_STICKY_FINGERS |
+			MT_QUIRK_WIN8_PTP_BUTTONS,
+		.export_all_inputs = true },
 	{ }
 };
 
@@ -758,7 +767,8 @@  static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
 			MT_STORE_FIELD(inrange_state);
 			return 1;
 		case HID_DG_CONFIDENCE:
-			if (cls->name == MT_CLS_WIN_8 &&
+			if ((cls->name == MT_CLS_WIN_8 ||
+			     cls->name == MT_CLS_CIRQUE) &&
 				(field->application == HID_DG_TOUCHPAD ||
 				 field->application == HID_DG_TOUCHSCREEN))
 				app->quirks |= MT_QUIRK_CONFIDENCE;
@@ -1828,6 +1838,12 @@  static const struct hid_device_id mt_devices[] = {
 		MT_USB_DEVICE(USB_VENDOR_ID_CHUNGHWAT,
 			USB_DEVICE_ID_CHUNGHWAT_MULTITOUCH) },
 
+	/* Cirque devices */
+	{ .driver_data = MT_CLS_CIRQUE,
+		HID_DEVICE(BUS_I2C, HID_GROUP_MULTITOUCH_WIN_8,
+			I2C_VENDOR_ID_CIRQUE,
+			I2C_PRODUCT_ID_CIRQUE_121F) },
+
 	/* CJTouch panels */
 	{ .driver_data = MT_CLS_NSMU,
 		MT_USB_DEVICE(USB_VENDOR_ID_CJTOUCH,