<div dir="ltr"><div>So you're saying that every time we're suspended, we simply throw out the entire context and drop all the devices on the floor, as if you just unplugged all of them? </div><div>I suppose I just never thought of that. On first though, I don't see anything wrong with that. If that's what we should do, should we remove libinput_suspend / libinput_resume then? </div></div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 17, 2014 at 11:21 PM, Peter Hutterer <<a href="mailto:peter.hutterer@who-t.net" target="_blank">peter.hutterer@who-t.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">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 <<a href="mailto:peter.hutterer@who-t.net">peter.hutterer@who-t.net</a>> > > --- > > 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 </div></div>Jasper? you can answer that better than me <div class=""> > 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 </div>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 <div class="HOEnZb"><div class="h5"> > > 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 > > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> </div></div></blockquote></div> -- Jasper </div>