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?
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? On Mon, Mar 17, 2014 at 11:21 PM, Peter Hutterer <[email protected]>wrote: > 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 <[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 > > 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 > > > [email protected] > > > http://lists.freedesktop.org/mailman/listinfo/wayland-devel > -- Jasper
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
