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 <[email protected]>
> ---
> 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
?
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
Jonas
>
> 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
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
