Re: [RFC RESEND 5/5] Input: evdev - add new EVIOCGABSRANGE ioctl

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Fri, Aug 08, 2014 at 03:26:56PM +0200, David Herrmann wrote:
> Hi
> 
> On Wed, Aug 6, 2014 at 3:35 AM, Peter Hutterer <peter.hutterer@who-t.net> wrote:
> >> +
> >> +/**
> >> + * EVIOCGABSRANGE - Fetch range of ABS values
> >> + *
> >> + * This fetches the current values of a range of ABS codes atomically. The range
> >> + * of codes to fetch and the buffer-types are passed as "struct input_absrange",
> >> + * which has the following fields:
> >> + *      slots: Number of MT slots to fetch data for.
> >> + *       code: First ABS axis to query.
> >> + *      count: Number of ABS axes to query starting at @code.
> >> + *     buffer: Pointer to a receive buffer where to store the fetched ABS
> >> + *             values. This buffer must be an array of __s32 with at least
> >> + *             (@slots * @code) elements. The buffer is interpreted as two
> >> + *             dimensional __s32 array, declared as: __s32[slots][codes]
> >
> > tbh this seems more complicated than necessary. Have you thought about
> > just dumping the events into the client buffer as if they came fresh in from
> > the device? So to sync, the client calls the ioctl with a buffer and a
> > buffer size, and the kernel simply writes a series of struct input_events
> > into that buffer, with ABS_MT_SLOT as required for all slots, (optionally?)
> > followed by a SYN_DROPPED. So the buffer afterwards could look like this:
> >    EV_ABS ABS_X 30
> >    EV_ABS ABS_X 1202
> >    EV_ABS ABS_MT_SLOT 0
> >    EV_ABS ABS_MT_POSITION_X 30
> >    EV_ABS ABS_MT_POSITION_Y 1202
> >    EV_ABS ABS_MT_SLOT 1
> >    EV_ABS ABS_MT_POSITION_X 80
> >    EV_ABS ABS_MT_POSITION_Y 1800
> >    EV_SYN SYN_REPORT 0
> >
> > the client can then go through and just process the events on-by-one as it
> > would otherwise with real events.
> >
> > This approach could be even extended to include EV_KEY, etc. providing a
> > single ioctl to sync the whole state of the device atomically.
> >
> > comments?

I like it.

> 
> So you mean instead of passing a __32 array we pass a "struct
> input_event" array and write it there? So bypassing the receive-queue?
> That does sound quite nice, indeed. We could replace all the other
> "sync" ioctls and just provide a way to receive all the events
> directly.
> 
> Something like:
> 
> EVIOCQUERY(struct input_query)
> 
> struct input_query {
>         __u16 type;
>         __u16 start_code;
>         __u16 end_code;
>         __u16 slots;
> 
>         struct input_event buffer[];
> };

No, it is more like EVIOCRESYNC(void) which makes input core to dump all
existing state into the client's standard event queue so that here is no
need to reconcile/reconstruct anything. We could give a new SYN marker
to indicate end-of-state boundary.

Thanks.

-- 
Dmitry