[PATCH synaptics] Increase post-motion-event timeout from 13 to 50 ms.

[Peter Hutterer]

On Tue, Mar 01, 2011 at 09:40:38AM +0800, Daniel Kurtz wrote:
> Honestly, the purpose of this timer baffles me a bit.  AFAICT, it should
> never fire.

note that this timer has been present for years now while large parts of the
driver evolved around it. it is quite possible that it has outlived its
original purpose.

> The comment says:
> 
> "to create fluid edge motion, call back 'soon' even in the absence of
> new hardware events"

this comment was only added Feb 21 this year and replaced the old "WTF?
what's with 13?" comment :)

> However, my understanding is that edge motion occurs when a drag or move
> hits an edge, but the finger stays in contact with the pad.  Thus, there
> should always still be hardware events during an edge motion.
> 
> This timer, instead, should only fire after the finger is raised immediately
> after a valid motion packet (a hardware sample that generated a delta
> resulting in pointer motion).  Actually, since there is no way of
> distinguishing between timer-events and hardware-events (since timerFunc copies
> previous priv->hwState), what this means is, once fired, this timer will
> keep firing forever.
> 
> However, at least on the trackpads that I have tested, this doesn't happen
> for a very different reason.  When the finger is lifted from the pad, the
> touchpad/kernel-driver doesn't immediately stop sending packets.  Instead,
> it sends about 1 second worth of dummy "x=0,y=0,z=0..." packets at regular
> intervals.
> These packets are recognized as "!insidearea", which resets
> priv->count_packet_finger and therefore doesn't schedule the edge_motion
> timer, effectively cancelling it.
> 
> The only time this timer actual fires, then, is in this case that I am
> trying to fix - when it fires in between touchpad hardware samples because
> the touchpad generates samples slower than 13 ms apart.

you could easily add a field to hwState to indicate if it's a timer event or
not. but I think that just removing this timer may be the best approach.
If it really breaks on some pads we should have people yelling at us quite
quickly.

My suspicion is that the kernel filters for us - the backends that may be
affected are the non-evdev ones but I have no setups to test these.

Cheers,
  Peter

> On Tue, Mar 1, 2011 at 9:12 AM, Peter Hutterer <peter.hutterer at who-t.net>wrote:
> 
> > On Mon, Feb 28, 2011 at 08:56:27PM +0800, Daniel Kurtz wrote:
> > > Some Synaptics image sensors report samples at less than 80 Samples/sec.
> > > Thus, the inter-sample gap is longer than 13 ms (more like 17-25 ms).
> > >
> > > With a 13ms timeout, every sample was processed twice:
> > >   1) Once when ReadHwState() returned valid data.
> > >   2) 13ms later, when the scheduled timer would expire.
> > >
> > > The value 50ms is chosen arbitrarily higher than the expected slowest
> > > trackpad reporting interval, but short enough not to be noticeable by
> > > a human user.
> > >
> > > Signed-off-by: Daniel Kurtz <djkurtz at google.com>
> > > ---
> > >  src/synaptics.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/src/synaptics.c b/src/synaptics.c
> > > index 56ce725..0b2d7a1 100644
> > > --- a/src/synaptics.c
> > > +++ b/src/synaptics.c
> > > @@ -1870,7 +1870,7 @@ ComputeDeltas(SynapticsPrivate *priv, const struct
> > SynapticsHwState *hw,
> > >
> > >      /* to create fluid edge motion, call back 'soon'
> > >       * even in the absence of new hardware events */
> > > -    delay = MIN(delay, 13);
> > > +    delay = MIN(delay, 50);
> > >
> > >      if (priv->count_packet_finger <= 3) /* min. 3 packets, see
> > get_delta() */
> > >          goto skip; /* skip the lot */
> > > --
> > > 1.7.3.1
> >
> > I wonder - wouldn't the better fix be to cancel the timer if the data was
> > processed or if new data would come in?
> >
> > Cheers,
> >   Peter
> >