[RFC PATCH libinput] udev: add libinput_udev_rescan_devices()
Peter Hutterer
peter.hutterer at who-t.net
Mon Mar 17 20:21:27 PDT 2014
On Sat, Mar 15, 2014 at 07:59:29PM +0100, Jonas Ådahl wrote:
> On Thu, Mar 13, 2014 at 04:18:20PM +1000, Peter Hutterer wrote:
> > When a libinput context for a given seat is initialized, not all devices may
> > be available. Some or all devices may be paused by systemd-logind. Waiting for
> > a unplug event is not appropriate here as the devices are physically
> > available, just prevented from getting access.
> >
> > Add a libinput_udev_rescan_devices() function that triggers a scan of all
> > devices on the current udev seat. Devices that do not exist on the seat are
> > added, devices that already exist on the seat but have been revoked are
> > removed.
> >
> > Note that devices that have been physically removed are not removed, instead
> > we wait for the udev event to handle this for us.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> > The idea here is basically to start a udev context as usual. If the
> > compositor doesn't have the session, open_restricted will fail. Once the
> > ResumeDevice signals are handled by the compositor it can ask libinput to
> > rescan the device list to add the ones that failed before (or remove revoked
> > ones).
> >
> > Notes on why this approach:
> > * libinput_device_suspend()/resume() seems nicer at first but if a device
> > fails to open, we don't have a libinput_device context. handling that
> > would make the API complicated since we cannot guarantee that all
> > libinput_device_* functions (specificall has_capability) work on all
> > devices anymore.
> > * I suspect in the 90% case the the PauseDevice/ResumeDevice signals come in
> > in a batch anyway, so the compositor should be able to optimise this to
> > one single call
> > * this is a udev specific call, for the path backend the compositor can and
> > should maintain the devices manually anyway
> > * EVIOCGVERSION was picked because it always succeeds, except after revoke
> >
> > This is an RFC at this point, let me know if that approach works. Still need
> > to write tests and better evdev duplicate detection - right now there is a
> > race condition that could remove the wrong device.
>
> Hi,
>
> So what this patch is trying to solve is handling the following flow:
>
> * create libinput udev context
> - some or all devices will fail to open due to being paused
> * devices are resumed
>
> What stops us from simply doing
>
> * devices are resumed
> * create libinput udev context
Jasper? you can answer that better than me
> As you say, a compositor should be able to know when it should rescan,
> and in most cases (?) before this, we won't get a single device anyway
> so whats the point of creating earlier than that? For resuming after
> session switch I suppose we'd have the same problem, but this would then
> just work the same:
>
> * devices are resumed
> * resume libinput context
the question here is: is there a use-case for a single device to be
paused/resumed outside of the usual process? David?
We're struggling with this in X but that's caused by a completely different
problem and is rather orthogonal to this.
Cheers,
Peter
> > src/evdev.c | 15 +++++++++++++++
> > src/evdev.h | 2 ++
> > src/libinput.h | 21 +++++++++++++++++++++
> > src/udev-seat.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
> > 4 files changed, 79 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/evdev.c b/src/evdev.c
> > index ba7c8b3..018fbb1 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -790,3 +790,18 @@ evdev_device_destroy(struct evdev_device *device)
> > free(device->sysname);
> > free(device);
> > }
> > +
> > +int
> > +evdev_device_is_alive(struct evdev_device *device)
> > +{
> > + int rc;
> > + int version;
> > +
> > + rc = ioctl(device->fd, EVIOCGVERSION, &version);
> > +
> > + if (rc < 0 && errno != ENODEV)
> > + log_info("evdev: %s failed with errno %d (%s)\n",
> > + __func__, errno, strerror(errno));
> > +
> > + return rc != -1;
> > +}
> > diff --git a/src/evdev.h b/src/evdev.h
> > index b83a2f9..82a3873 100644
> > --- a/src/evdev.h
> > +++ b/src/evdev.h
> > @@ -156,4 +156,6 @@ evdev_device_remove(struct evdev_device *device);
> > void
> > evdev_device_destroy(struct evdev_device *device);
> >
> > +int
> > +evdev_device_is_alive(struct evdev_device *device);
> > #endif /* EVDEV_H */
> > diff --git a/src/libinput.h b/src/libinput.h
> > index 3e09871..dadcac2 100644
> > --- a/src/libinput.h
> > +++ b/src/libinput.h
> > @@ -715,6 +715,27 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface,
> > /**
> > * @ingroup base
> > *
> > + * Re-scan the list of devices available to this context. Devices in the
> > + * seat specified in libinput_udev_create_for_seat() that previously have
> > + * failed to initialize are re-initialized. Devices that have successfully
> > + * re-initialized but are now revoked are removed.
> > + *
> > + * Calling libinput_udev_rescan_devices() on a context suspended with
> > + * libinput_suspend() does nothing.
> > + *
> > + * @note This function should not be used for detection of physically added
> > + * or removed devices, libinput_dispatch() detects those. This function
> > + * should only be used to re-open or close existing devices, e.g. if
> > + * systemd-logind prevented access to a device before.
> > + *
> > + * @param libinput The previously initialized libinput context
> > + */
> > +void
> > +libinput_udev_rescan_devices(struct libinput *libinput);
> > +
> > +/**
> > + * @ingroup base
> > + *
> > * Create a new libinput context that requires the caller to manually add or
> > * remove devices with libinput_path_add_device() and
> > * libinput_path_remove_device().
> > diff --git a/src/udev-seat.c b/src/udev-seat.c
> > index 366c92b..5b09e4b 100644
> > --- a/src/udev-seat.c
> > +++ b/src/udev-seat.c
> > @@ -136,12 +136,28 @@ device_removed(struct udev_device *udev_device, struct udev_input *input)
> > }
> > }
> >
> > +static struct evdev_device*
> > +udev_input_find_device_by_sysname(struct udev_input *input, const char *sysname)
> > +{
> > + struct udev_seat *seat;
> > + struct evdev_device *device;
> > +
> > + list_for_each(seat, &input->base.seat_list, base.link) {
> > + list_for_each(device, &seat->base.devices_list, base.link)
> > + if (!strcmp(device->sysname, sysname)) {
> > + return device;
> > + }
> > + }
> > + return NULL;
> > +}
> > +
> > static int
> > udev_input_add_devices(struct udev_input *input, struct udev *udev)
> > {
> > struct udev_enumerate *e;
> > struct udev_list_entry *entry;
> > struct udev_device *device;
> > + struct evdev_device *evdev;
> > const char *path, *sysname;
> >
> > e = udev_enumerate_new(udev);
> > @@ -157,11 +173,15 @@ udev_input_add_devices(struct udev_input *input, struct udev *udev)
> > continue;
> > }
> >
> > - if (device_added(device, input) < 0) {
> > - udev_device_unref(device);
> > - udev_enumerate_unref(e);
> > - return -1;
> > - }
> > + evdev = udev_input_find_device_by_sysname(input, sysname);
> > + if (!evdev) {
> > + if (device_added(device, input) < 0) {
> > + udev_device_unref(device);
> > + udev_enumerate_unref(e);
> > + return -1;
> > + }
> > + } else if (!evdev_device_is_alive(evdev))
> > + device_removed(device, input);
> >
> > udev_device_unref(device);
> > }
> > @@ -364,3 +384,19 @@ libinput_udev_create_for_seat(const struct libinput_interface *interface,
> >
> > return &input->base;
> > }
> > +
> > +LIBINPUT_EXPORT void
> > +libinput_udev_rescan_devices(struct libinput *libinput)
> > +{
> > + struct udev_input *udev_input = (struct udev_input*)libinput;
> > +
> > + if (libinput->interface_backend != &interface_backend) {
> > + log_error("Mismatching backends. This is an application bug.\n");
> > + return;
> > + }
> > +
> > + if (udev_input->udev_monitor == NULL)
> > + return;
> > +
> > + udev_input_add_devices(udev_input, udev_input->udev);
> > +}
> > --
> > 1.8.5.3
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list