[PATCH 0/4] Stylus-on-touchscreen device support

[PATCH 0/4] Stylus-on-touchscreen device support

[наб]

[-- Attachment #1: Type: text/plain, Size: 982 bytes --]

This patchset adds support for stylus-on-touchscreen devices as found on
the OneMix 3 Pro and Dell Inspiron 15 7000 2-in-1 (7591), among others;
with it, they properly behave like a drawing tablet.

Patches 2 and 4 funxionally depend on patch 1.
Patch 4 needs patch 3 to apply.

The output of this patchset and the need for a kernel, rather than
userspace, patch was previously discussed here:
  https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/558#note_792834

Ahelenia Ziemiańska (4):
  HID: multitouch: require Finger field to mark Win8 reports as MT
  HID: multitouch: set Stylus suffix for Stylus-application devices, too
  HID: input: replace outdated HID numbers+comments with macros
  HID: input: work around Win8 stylus-on-touchscreen reporting

 drivers/hid/hid-input.c      | 47 +++++++++++++++++++++++++++++++++---
 drivers/hid/hid-multitouch.c | 18 ++++++++------
 2 files changed, 55 insertions(+), 10 deletions(-)

-- 
2.20.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/4] HID: multitouch: require Finger field to mark Win8 reports as MT

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 1297 bytes --]

This effectively changes collection_is_mt from
  contact ID in report->field
to
  (device is Win8 => collection is finger) && contact ID in report->field

Some devices erroneously report Pen for fingers,
and Win8 stylus-on-touchscreen devices report contact ID,
but mark the accompanying touchscreen device's collection correctly

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 drivers/hid/hid-multitouch.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 8429ebe7097e..8580ace596c2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -604,9 +604,13 @@ static struct mt_report_data *mt_allocate_report_data(struct mt_device *td,
 		if (!(HID_MAIN_ITEM_VARIABLE & field->flags))
 			continue;
 
-		for (n = 0; n < field->report_count; n++) {
-			if (field->usage[n].hid == HID_DG_CONTACTID)
-				rdata->is_mt_collection = true;
+		if (field->logical == HID_DG_FINGER || td->hdev->group != HID_GROUP_MULTITOUCH_WIN_8) {
+			for (n = 0; n < field->report_count; n++) {
+				if (field->usage[n].hid == HID_DG_CONTACTID) {
+					rdata->is_mt_collection = true;
+					break;
+				}
+			}
 		}
 	}
 
-- 
2.20.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 2/4] HID: multitouch: set Stylus suffix for Stylus-application devices, too

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

This re-adds the suffix to Win8 stylus-on-touchscreen devices,
now that they aren't erroneously marked as MT

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 drivers/hid/hid-multitouch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 8580ace596c2..e5a3704b9fe8 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1580,13 +1580,13 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 		/* we do not set suffix = "Touchscreen" */
 		hi->input->name = hdev->name;
 		break;
-	case HID_DG_STYLUS:
-		/* force BTN_STYLUS to allow tablet matching in udev */
-		__set_bit(BTN_STYLUS, hi->input->keybit);
-		break;
 	case HID_VD_ASUS_CUSTOM_MEDIA_KEYS:
 		suffix = "Custom Media Keys";
 		break;
+	case HID_DG_STYLUS:
+		/* force BTN_STYLUS to allow tablet matching in udev */
+		__set_bit(BTN_STYLUS, hi->input->keybit);
+		fallthrough;
 	case HID_DG_PEN:
 		suffix = "Stylus";
 		break;
-- 
2.20.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/4] HID: input: replace outdated HID numbers+comments with macros

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]

These were untouched since 2.3.99-pre3, and the explanatory comment for
HID_DG_TIPPRESSURE is TipPressure on other places

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 drivers/hid/hid-input.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index f23027d2795b..a5ba92978473 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1301,12 +1301,12 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 	}
 
-	if (usage->hid == (HID_UP_DIGITIZER | 0x003c)) { /* Invert */
+	if (usage->hid == HID_DG_INVERT) {
 		*quirks = value ? (*quirks | HID_QUIRK_INVERT) : (*quirks & ~HID_QUIRK_INVERT);
 		return;
 	}
 
-	if (usage->hid == (HID_UP_DIGITIZER | 0x0032)) { /* InRange */
+	if (usage->hid == HID_DG_INRANGE) {
 		if (value) {
 			input_event(input, usage->type, (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code, 1);
 			return;
@@ -1316,7 +1316,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 	}
 
-	if (usage->hid == (HID_UP_DIGITIZER | 0x0030) && (*quirks & HID_QUIRK_NOTOUCH)) { /* Pressure */
+	if (usage->hid == HID_DG_TIPPRESSURE && (*quirks & HID_QUIRK_NOTOUCH)) {
 		int a = field->logical_minimum;
 		int b = field->logical_maximum;
 		input_event(input, EV_KEY, BTN_TOUCH, value > a + ((b - a) >> 3));
-- 
2.20.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 4/4] HID: input: work around Win8 stylus-on-touchscreen reporting

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 2318 bytes --]

With this, these devices now behave as tablets as expected by userspace

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
 drivers/hid/hid-input.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index a5ba92978473..b8813fc3e9d2 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1273,6 +1273,41 @@ static void hidinput_handle_scroll(struct hid_usage *usage,
 	input_event(input, EV_REL, usage->code, hi_res);
 }
 
+/*
+ * Win8 tablet stylus devices send, in order:
+ *   HID_DG_TIPSWITCH (BTN_TOUCH)
+ *   HID_DG_INVERT    (BTN_TOOL_RUBBER)
+ *   HID_DG_ERASER    (BTN_TOUCH)
+ *   HID_DG_INRANGE   (BTN_TOOL_PEN)
+ *
+ * For each of these states:
+ *   hover     :                         INRANGE
+ *   touching  : TIPSWITCH
+ *   hover+2   :           INVERT        INRANGE
+ *   touching+2:                  ERASER INRANGE
+ *
+ * Which means we'd send BTN_TOUCH=0 + BTN_TOOL_PEN=1 on proximity,
+ * then BTN_TOUCH=1 and BTN_TOOL_PEN=0 in consecutive groups when touched,
+ * indicating the stylus leaving the screen as soon as the two meet.
+ */
+static void hidinput_fixup_win8_inrange(struct hid_device *hid, struct hid_field *field, __s32 *value)
+{
+	unsigned f, u;
+	struct hid_field *rfield;
+
+	if (!*value) {
+		for (f = 0; f < field->report->maxfield; ++f) {
+			rfield = field->report->field[f];
+			for (u = 0; u < rfield->maxusage; ++u) {
+				if (rfield->usage[u].hid == HID_DG_TIPSWITCH) {
+					*value = rfield->value[u];
+					return;
+				}
+			}
+		}
+	}
+}
+
 void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, __s32 value)
 {
 	struct input_dev *input;
@@ -1306,7 +1341,13 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 	}
 
+	if (usage->hid == HID_DG_ERASER && value)
+		*quirks |= HID_QUIRK_INVERT;
+
 	if (usage->hid == HID_DG_INRANGE) {
+		if (hid->group == HID_GROUP_MULTITOUCH_WIN_8)
+			hidinput_fixup_win8_inrange(hid, field, &value);
+
 		if (value) {
 			input_event(input, usage->type, (*quirks & HID_QUIRK_INVERT) ? BTN_TOOL_RUBBER : usage->code, 1);
 			return;
-- 
2.20.1

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] Stylus-on-touchscreen device support

[Jiri Kosina]

On Wed, 17 Feb 2021, наб wrote:

> This patchset adds support for stylus-on-touchscreen devices as found on
> the OneMix 3 Pro and Dell Inspiron 15 7000 2-in-1 (7591), among others;
> with it, they properly behave like a drawing tablet.
> 
> Patches 2 and 4 funxionally depend on patch 1.
> Patch 4 needs patch 3 to apply.
> 
> The output of this patchset and the need for a kernel, rather than
> userspace, patch was previously discussed here:
>   https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/558#note_792834
> 
> Ahelenia Ziemiańska (4):
>   HID: multitouch: require Finger field to mark Win8 reports as MT
>   HID: multitouch: set Stylus suffix for Stylus-application devices, too
>   HID: input: replace outdated HID numbers+comments with macros
>   HID: input: work around Win8 stylus-on-touchscreen reporting
> 
>  drivers/hid/hid-input.c      | 47 +++++++++++++++++++++++++++++++++---
>  drivers/hid/hid-multitouch.c | 18 ++++++++------
>  2 files changed, 55 insertions(+), 10 deletions(-)

Benjamin, this patchset looks good to me; do you have any objections on 
queuing it for 5.13?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/4] Stylus-on-touchscreen device support

[Benjamin Tissoires]

Hi Jiri,

On Mon, Mar 8, 2021 at 11:15 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Wed, 17 Feb 2021, наб wrote:
>
> > This patchset adds support for stylus-on-touchscreen devices as found on
> > the OneMix 3 Pro and Dell Inspiron 15 7000 2-in-1 (7591), among others;
> > with it, they properly behave like a drawing tablet.
> >
> > Patches 2 and 4 funxionally depend on patch 1.
> > Patch 4 needs patch 3 to apply.
> >
> > The output of this patchset and the need for a kernel, rather than
> > userspace, patch was previously discussed here:
> >   https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/558#note_792834
> >
> > Ahelenia Ziemiańska (4):
> >   HID: multitouch: require Finger field to mark Win8 reports as MT
> >   HID: multitouch: set Stylus suffix for Stylus-application devices, too
> >   HID: input: replace outdated HID numbers+comments with macros
> >   HID: input: work around Win8 stylus-on-touchscreen reporting
> >
> >  drivers/hid/hid-input.c      | 47 +++++++++++++++++++++++++++++++++---
> >  drivers/hid/hid-multitouch.c | 18 ++++++++------
> >  2 files changed, 55 insertions(+), 10 deletions(-)
>
> Benjamin, this patchset looks good to me; do you have any objections on
> queuing it for 5.13?
>

Please hold on this one. I am pretty sure this should break the test
suite but couldn't have the chance to get to it. Will pop this one up
in TODO list.

Cheers,
Benjamin

Re: [PATCH 0/4] Stylus-on-touchscreen device support

[наб]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On Mon, Mar 08, 2021 at 11:21:56AM +0100, Benjamin Tissoires wrote:
> On Mon, Mar 8, 2021 at 11:15 AM Jiri Kosina <jikos@kernel.org> wrote:
> > On Wed, 17 Feb 2021, наб wrote:
> > > This patchset adds support for stylus-on-touchscreen devices as found on
> > > the OneMix 3 Pro and Dell Inspiron 15 7000 2-in-1 (7591), among others;
> > > with it, they properly behave like a drawing tablet.
> > >
> > > Patches 2 and 4 funxionally depend on patch 1.
> > > Patch 4 needs patch 3 to apply.
> > >
> > > The output of this patchset and the need for a kernel, rather than
> > > userspace, patch was previously discussed here:
> > >   https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/558#note_792834
> > >
> > > Ahelenia Ziemiańska (4):
> > >   HID: multitouch: require Finger field to mark Win8 reports as MT
> > >   HID: multitouch: set Stylus suffix for Stylus-application devices, too
> > >   HID: input: replace outdated HID numbers+comments with macros
> > >   HID: input: work around Win8 stylus-on-touchscreen reporting
> > >
> > >  drivers/hid/hid-input.c      | 47 +++++++++++++++++++++++++++++++++---
> > >  drivers/hid/hid-multitouch.c | 18 ++++++++------
> > >  2 files changed, 55 insertions(+), 10 deletions(-)
> >
> > Benjamin, this patchset looks good to me; do you have any objections on
> > queuing it for 5.13?
> >
> 
> Please hold on this one. I am pretty sure this should break the test
> suite but couldn't have the chance to get to it. Will pop this one up
> in TODO list.

This also has minor behavioural problems regarding BTN_STYLUS reporting
that came out in testing, and I didn't have the time to dig through
and verify the logs from pre-v2 a tester sent back last week.

I hope to send v2 today/tomorrow, energy and time permitting.

Best,
наб

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]