From: Peter Hutterer <peter.hutterer@who-t.net>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Jiri Kosina <jikos@kernel.org>,
Benjamin Tissoires <benjamin.tissoires@redhat.com>,
Henrik Rydberg <rydberg@bitmath.org>,
Jason Gerecke <killertofu@gmail.com>,
Dennis Kempin <denniskempin@google.com>,
Andrew de los Reyes <adlr@google.com>,
linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] HID: multitouch: report MT_TOOL_PALM for non-confident touches
Date: Thu, 31 May 2018 09:12:08 +1000 [thread overview]
Message-ID: <20180530231208.GA19976@jelly> (raw)
In-Reply-To: <20170811004500.13740-1-dmitry.torokhov@gmail.com>
Hi Dmitry,
On Thu, Aug 10, 2017 at 05:44:59PM -0700, Dmitry Torokhov wrote:
> According to Microsoft specification [1] for Precision Touchpads (and
> Touchscreens) the devices use "confidence" reports to signal accidental
> touches, or contacts that are "too large to be a finger". Instead of
> simply marking contact inactive in this case (which causes issues if
> contact was originally proper and we lost confidence in it later, as
> this results in accidental clicks, drags, etc), let's report such
> contacts as MT_TOOL_PALM and let userspace decide what to do.
> Additionally, let's report contact size for such touches as maximum
> allowed for major/minor, which should help userspace that is not yet
> aware of MT_TOOL_PALM to still perform palm rejection.
>
> An additional complication, is that some firmwares do not report
> non-confident touches as active. To cope with this we delay release of
> such contact (i.e. if contact was active we first report it as still
> active MT+TOOL_PALM and then synthesize the release event in a separate
> frame).
>
> [1] https://docs.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
This one popped up again in a bug report [1] and it looks like it never
got merged. fwiw, libinput does support ABS_MT_TOOL_PALM for touchpads as of
1.8.0 and just releasing the touch causes fake taps. So you have the green
light from me to merge this :)
Cheers,
Peter
[1] https://bugs.freedesktop.org/show_bug.cgi?id=106716
> ---
> drivers/hid/hid-multitouch.c | 86 +++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 77 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 440b999304a5..c28defe50a10 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -114,6 +114,8 @@ struct mt_device {
> struct timer_list release_timer; /* to release sticky fingers */
> struct mt_fields *fields; /* temporary placeholder for storing the
> multitouch fields */
> + unsigned long *pending_palm_slots; /* slots where we reported palm
> + and need to release */
> unsigned long mt_io_flags; /* mt flags (MT_IO_FLAGS_*) */
> int cc_index; /* contact count field index in the report */
> int cc_value_index; /* contact count value index in the field */
> @@ -543,8 +545,12 @@ static int mt_touch_input_mapping(struct hid_device *hdev, struct hid_input *hi,
> case HID_DG_CONFIDENCE:
> if ((cls->name == MT_CLS_WIN_8 ||
> cls->name == MT_CLS_WIN_8_DUAL) &&
> - field->application == HID_DG_TOUCHPAD)
> + field->application == HID_DG_TOUCHPAD) {
> cls->quirks |= MT_QUIRK_CONFIDENCE;
> + input_set_abs_params(hi->input,
> + ABS_MT_TOOL_TYPE,
> + MT_TOOL_FINGER, MT_TOOL_PALM, 0, 0);
> + }
> mt_store_field(usage, td, hi);
> return 1;
> case HID_DG_TIPSWITCH:
> @@ -657,6 +663,7 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>
> if (td->curvalid || (td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID)) {
> int active;
> + int tool;
> int slotnum = mt_compute_slot(td, input);
> struct mt_slot *s = &td->curdata;
> struct input_mt *mt = input->mt;
> @@ -671,24 +678,56 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> return;
> }
>
> + active = s->touch_state || s->inrange_state;
> +
> if (!(td->mtclass.quirks & MT_QUIRK_CONFIDENCE))
> s->confidence_state = 1;
> - active = (s->touch_state || s->inrange_state) &&
> - s->confidence_state;
> +
> + if (likely(s->confidence_state)) {
> + tool = MT_TOOL_FINGER;
> + } else {
> + tool = MT_TOOL_PALM;
> + if (!active &&
> + input_mt_is_active(&mt->slots[slotnum])) {
> + /*
> + * The non-confidence was reported for
> + * previously valid contact that is also no
> + * longer valid. We can't simply report
> + * lift-off as userspace will not be aware
> + * of non-confidence, so we need to split
> + * it into 2 events: active MT_TOOL_PALM
> + * and a separate liftoff.
> + */
> + active = true;
> + set_bit(slotnum, td->pending_palm_slots);
> + }
> + }
>
> input_mt_slot(input, slotnum);
> - input_mt_report_slot_state(input, MT_TOOL_FINGER, active);
> + input_mt_report_slot_state(input, tool, active);
> if (active) {
> /* this finger is in proximity of the sensor */
> int wide = (s->w > s->h);
> int major = max(s->w, s->h);
> int minor = min(s->w, s->h);
>
> - /*
> - * divided by two to match visual scale of touch
> - * for devices with this quirk
> - */
> - if (td->mtclass.quirks & MT_QUIRK_TOUCH_SIZE_SCALING) {
> + if (unlikely(!s->confidence_state)) {
> + /*
> + * When reporting palm, set contact to maximum
> + * size to help userspace that does not
> + * recognize MT_TOOL_PALM to reject contacts
> + * that are too large.
> + */
> + major = input_abs_get_max(input,
> + ABS_MT_TOUCH_MAJOR);
> + minor = input_abs_get_max(input,
> + ABS_MT_TOUCH_MINOR);
> + } else if (td->mtclass.quirks &
> + MT_QUIRK_TOUCH_SIZE_SCALING) {
> + /*
> + * divided by two to match visual scale of touch
> + * for devices with this quirk
> + */
> major = major >> 1;
> minor = minor >> 1;
> }
> @@ -711,6 +750,25 @@ static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
> td->num_received++;
> }
>
> +static void mt_release_pending_palms(struct mt_device *td,
> + struct input_dev *input)
> +{
> + int slotnum;
> + bool need_sync = false;
> +
> + for_each_set_bit(slotnum, td->pending_palm_slots, td->maxcontacts) {
> + clear_bit(slotnum, td->pending_palm_slots);
> +
> + input_mt_slot(input, slotnum);
> + input_mt_report_slot_state(input, MT_TOOL_PALM, false);
> +
> + need_sync = true;
> + }
> +
> + if (need_sync)
> + input_sync(input);
> +}
> +
> /*
> * this function is called when a whole packet has been received and processed,
> * so that it can decide what to send to the input layer.
> @@ -719,6 +777,9 @@ static void mt_sync_frame(struct mt_device *td, struct input_dev *input)
> {
> input_mt_sync_frame(input);
> input_sync(input);
> +
> + mt_release_pending_palms(td, input);
> +
> td->num_received = 0;
> if (test_bit(MT_IO_FLAGS_ACTIVE_SLOTS, &td->mt_io_flags))
> set_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags);
> @@ -903,6 +964,13 @@ static int mt_touch_input_configured(struct hid_device *hdev,
> if (td->is_buttonpad)
> __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
>
> + td->pending_palm_slots = devm_kcalloc(&hi->input->dev,
> + BITS_TO_LONGS(td->maxcontacts),
> + sizeof(long),
> + GFP_KERNEL);
> + if (!td->pending_palm_slots)
> + return -ENOMEM;
> +
> ret = input_mt_init_slots(input, td->maxcontacts, td->mt_flags);
> if (ret)
> return ret;
> --
> 2.14.0.434.g98096fd7a8-goog
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
next prev parent reply other threads:[~2018-05-30 23:12 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-11 0:44 [PATCH 1/2] HID: multitouch: report MT_TOOL_PALM for non-confident touches Dmitry Torokhov
2017-08-11 0:45 ` [PATCH 2/2] HID: multitouch: touchscreens also use confidence reports Dmitry Torokhov
2017-08-11 6:14 ` [PATCH 1/2] HID: multitouch: report MT_TOOL_PALM for non-confident touches Henrik Rydberg
2017-08-11 6:54 ` Dmitry Torokhov
2017-08-11 8:29 ` Henrik Rydberg
2017-08-18 3:08 ` Peter Hutterer
2018-05-30 23:12 ` Peter Hutterer [this message]
2018-06-01 9:31 ` Benjamin Tissoires
2018-06-01 14:16 ` Benjamin Tissoires
2018-06-01 18:43 ` Dmitry Torokhov
2018-06-01 19:03 ` Henrik Rydberg
2018-06-04 12:57 ` Benjamin Tissoires
2018-06-04 17:27 ` Dmitry Torokhov
2018-06-04 17:55 ` Henrik Rydberg
2018-06-04 18:26 ` Dmitry Torokhov
2018-06-04 20:59 ` Benjamin Tissoires
2018-06-04 21:32 ` Dmitry Torokhov
2018-06-04 22:14 ` Benjamin Tissoires
2018-06-04 23:06 ` Peter Hutterer
2018-06-04 23:28 ` Dmitry Torokhov
2018-06-04 23:51 ` Peter Hutterer
2018-06-04 23:54 ` Dmitry Torokhov
2018-06-04 13:18 ` Benjamin Tissoires
2018-06-04 17:33 ` Dmitry Torokhov
2018-06-04 20:42 ` Benjamin Tissoires
2018-06-04 21:19 ` Dmitry Torokhov
2018-06-04 22:03 ` Benjamin Tissoires
2018-06-04 22:55 ` Peter Hutterer
2018-06-05 13:50 ` Benjamin Tissoires
2018-06-05 17:05 ` Dmitry Torokhov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180530231208.GA19976@jelly \
--to=peter.hutterer@who-t.net \
--cc=adlr@google.com \
--cc=benjamin.tissoires@redhat.com \
--cc=denniskempin@google.com \
--cc=dmitry.torokhov@gmail.com \
--cc=jikos@kernel.org \
--cc=killertofu@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rydberg@bitmath.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).