Linux Input/HID development
 help / color / mirror / Atom feed
From: Benjamin Tissoires <bentiss@kernel.org>
To: Dave Carey <carvsdriver@gmail.com>
Cc: linux-input@vger.kernel.org, jikos@kernel.org
Subject: Re: [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero
Date: Tue, 25 Aug 2026 11:32:27 +0200	[thread overview]
Message-ID: <ao1eFuXAdIehpWUM@beelink> (raw)
In-Reply-To: <20260730124336.637339-1-carvsdriver@gmail.com>

Hi Dave,

On Jul 30 2026, Dave Carey wrote:
> The INGENIC 17EF:6161 touchscreen (Lenovo Yoga Book 9 14IAH10) reports
> HID_DG_CONTACTCOUNT=0 in the frame immediately following the last finger
> lift rather than omitting the frame entirely.  In mt_touch_report() the
> existing code only updates num_expected when contact_count is non-zero,
> so a zero contact count on the first packet of a new frame leaves
> num_expected at its previous value (e.g. 2 for a two-finger gesture).
> The sync check "num_received >= num_expected" then evaluates "0 >= 2"
> and never fires, preventing INPUT_MT_DROP_UNUSED from releasing the
> stale slots.  Those slots remain active in the kernel MT layer until the
> next touch, at which point they are released in a batch alongside the
> new contact — causing the userspace event consumer to miss the intervening
> finger-up sequence and corrupt its gesture session state.
> 
> Fix by resetting num_expected to 0 when contact_count is zero and
> num_received is still 0 (i.e., this is the first and only packet of the
> frame, not a continuation packet in a multi-packet sequence).  With
> num_expected=0 the sync check "0 >= 0" fires immediately, calling
> input_mt_sync_frame() which drops the stale slots via INPUT_MT_DROP_UNUSED.
> 
> The num_received==0 guard is critical: continuation packets in a
> multi-packet frame arrive after at least one contact has already been
> processed (num_received>0), so they are correctly excluded from this
> path and the existing multi-packet logic is unaffected.
> 
> Signed-off-by: Dave Carey <carvsdriver@gmail.com>
> Tested-by: Dave Carey <carvsdriver@gmail.com>
> ---
> v3:
> - Resend as standalone patch; v2 was sent with incorrect subject "2/5"
>   (as a ping reply to the original series) so it was not recognized as
>   a versioned respin by the maintainers.  No code changes from v2.
> 
> v2:
> - Restructured contact_count block per Benjamin Tissoires' v1 review:
>   replace three-branch if/else-if/else-if with a cleaner two-branch
>   form, dropping the outer if (contact_count >= 0) wrapper.
> - Add prev_scantime != scantime guard to the zero-contact sentinel case.
> 
>  drivers/hid/hid-multitouch.c | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index ec04dbafbd99..56a3f29d4938 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1321,21 +1321,18 @@ static void mt_touch_report(struct hid_device *hid,
>  	 * Includes multi-packet support where subsequent
>  	 * packets are sent with zero contactcount.
>  	 */
> -	if (contact_count >= 0) {
> +	if (contact_count > 0)
> +		app->num_expected = contact_count;
> +	else if (app->num_received == 0 && app->prev_scantime != scantime) {

Sorry, I couldn't get back to you with this. I had second thoughts on
this patch, and ended up asking claude whether we were having any
potential issues. Turns out that we are fine according to all code path
it could find to one tiny exception: if there is no HID_DG_CONTACTCOUNT
in the report, contact_count is initialized to -1 but will not get a
positive or null value. So semantically, the code is misleading, but
it's harmless because num_expected would still stay at the value 0.

On the other hand, if the device reports a signed contact count and a
negative value, the code with your patch would fix the negative value to
be 0, so that's an improvement actually.

Cheers,
Benjamin

>  		/*
> +		 * New multi-report frame:
> +		 *
>  		 * For Win8 PTPs the first packet (td->num_received == 0) may
>  		 * have a contactcount of 0 if there only is a button event.
> -		 * We double check that this is not a continuation packet
> -		 * of a possible multi-packet frame be checking that the
> -		 * timestamp has changed.
> +		 *
> +		 * Some other devices use a sentinel frame with 0 to release all contacts
>  		 */
> -		if ((app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
> -		    app->num_received == 0 &&
> -		    app->prev_scantime != scantime)
> -			app->num_expected = contact_count;
> -		/* A non 0 contact count always indicates a first packet */
> -		else if (contact_count)
> -			app->num_expected = contact_count;
> +		app->num_expected = 0;
>  	}
>  	app->prev_scantime = scantime;
>  
> -- 
> 2.55.0
> 
> 

  parent reply	other threads:[~2026-08-25  9:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-30 12:43 [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero Dave Carey
2026-07-30 12:55 ` sashiko-bot
2026-08-25  9:32 ` Benjamin Tissoires [this message]
2026-08-25 10:17 ` Benjamin Tissoires

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=ao1eFuXAdIehpWUM@beelink \
    --to=bentiss@kernel.org \
    --cc=carvsdriver@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.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