Re: [PATCH 2/2] Input: atmel_mxt_ts: atmel_mxt_ts: Fix event loss

[Peter Hutterer <peter.hutterer@who-t.net>]

On Wed, Jun 23, 2021 at 05:48:49PM -0700, Dmitry Torokhov wrote:
> On Wed, Jun 23, 2021 at 03:56:37PM +0200, Loic Poulain wrote:
> > If both touch events and release are part of the same report,
> > userspace will not consider it as a touch-down & touch-up but as
> > a non-action. That can happen on resume when 'buffered' events are
> > dequeued in a row.
> > 
> > Make sure that release always causes previous events to be synced
> > before being reported.
> > 
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > ---
> >  drivers/input/touchscreen/atmel_mxt_ts.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> > index 807f449..e05ec30 100644
> > --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> > @@ -990,6 +990,13 @@ static void mxt_proc_t100_message(struct mxt_data *data, u8 *message)
> >  		input_report_abs(input_dev, ABS_MT_DISTANCE, distance);
> >  		input_report_abs(input_dev, ABS_MT_ORIENTATION, orientation);
> >  	} else {
> > +		/*
> > +		 * Always sync input before reporting release, to be sure
> > +		 * previous event(s) are taking into account by user side.
> > +		 */
> > +		if (data->update_input)
> > +			mxt_input_sync(data);
> 
> That means we sync for every contact release, whereas I think ideal
> would be to only sync when we observe touch-down and touch-up in the
> same slot.
> 
> Let's also add Peter to the conversation...

Thanks for the CC.

FTR, this is expected userspace behaviour, the device state is only looked
at during SYN_REPORT. Where you send event E=1 and E=0 in the same frame,
the state at SYN_REPORT time is 0, the 1 never happened.

The only device we (as in: libinput) make an exception for here are
keyboards because too many drivers get it wrong and it's too hard to fix all
of them. But especially for touch devices (and tablets!) we don't really
have any choice but to look at the state of the device at the end of the
frame.

So, yes, this patch is needed but I agree with Dmitry that you should only
send this for the special case that requires it.

Cheers,
   Peter