Re: [PATCH v2 0/7] Improve latency of IR decoding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Apr 10, 2018 at 07:53:43PM +0200, Matthias Reichl wrote:
> Hi Sean,
> 
> On Sun, Apr 08, 2018 at 10:19:35PM +0100, Sean Young wrote:
> > The current IR decoding is much too slow. Many IR protocols rely on
> > a trailing space for decoding (e.g. rc-6 needs to know when the bits
> > end). The trailing space is generated by the IR timeout, and if this
> > is longer than required, buttons can feel slow to respond.
> > 
> > The other issue is the keyup timer. IR has no concept of a keyup message,
> > this is implied by the absence of IR. So, minimising the timeout for
> > this makes buttons less "sticky"; the are released much quicker.
> > 
> > With these patches in place, using IR with the builtin decoders is much
> > improved and feels very snappy.
> > 
> > Changes since v1:
> >  - lost more testing
> >  - fixed various issues with mce decoder
> >  - fixed mceusb so it can use better timeout too
> 
> thanks, this version is working fine with meson-ir and gpio-rc-recv
> (latter on RPi). I mainly tested it with rc-5 remotes so far, more
> will follow and I'll update LibreELEC in a day or two to include
> the v2 series.

Thanks again for testing!

> Also thanks a lot for the mce_kbd fixes, I was just going to dig
> into the decoder (we received a bug report about stuck keys with
> mce_kbd last week), your patches can in just at the right time :)
> 
> I had a closer look at ir-mce_kbd-decoder.c and noticed 2 things
> (which can be handled separately):
> 
> It looks like the input_sync call in the state machine error
> path is not necessary:
> 
> out:
> 	dev_dbg(&dev->dev, "failed at state %i (%uus %s)\n",
> 		data->state, TO_US(ev.duration), TO_STR(ev.pulse));
> 	data->state = STATE_INACTIVE;
> 	input_sync(data->idev);
> 	return -EINVAL;
> 
> If I followed the code paths correctly states that generate
> input events won't go to out, only paths were no events are
> generated jump to out - but better verify that, I could have
> missed something.

The input_sync() patch is in mce_kbd_rx_timeout() code path, which
doesn't go through this. Hopefully it should fix the keys stuck
issue.

> The other thing I noticed is that there's no spinlock synchronizing
> the events from the timeout callback with the ones from the state
> machine - like keylock in rc-main. So the code could potentially
> be racy (if the timeout fires while the state machine is outputting
> events).

This timeout is for sending keyups in case no keyup is received from
the keyboard via input_report_key(). The input system does locking
for us (see drivers/input/input.c:436).


Sean



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux