[PATCH] logind: delay wakeup until DRM-device is resumed
Kristian Høgsberg
krh at bitplanet.net
Tue Dec 3 13:42:57 PST 2013
On Sat, Nov 30, 2013 at 2:25 AM, David Herrmann <dh.herrmann at gmail.com> wrote:
> The logind API was designed to allow any kind of devices and any number of
> devices. It has no idea of "main DRM device" or similar. However, the
> weston DRM backend was designed with a single DRM device as master.
> Therefore, we wake it up unconditionally on session-wakeup. But this may
> fail with logind as a session may be awake, but not all devices have been
> resumed, yet.
>
> Therefore, we change the weston-logind backend to deal with this case
> correctly. Instead of waking up the compositor on session-wakeup, we wait
> for the main DRM device to wake up. Once we get the event, we notify the
> compositor.
>
> For sleep, we reverse this logic. On *any* of the following events we
> tell the compositor to go to sleep:
> - Session gets inactive
> - DRM device gets inactive
> - DRM device is removed
> This guarantees, that weston is only active if *both*, the session and the
> main DRM device are awake/active.
>
> Note that we could actually rely solely on the DRM-device Pause/Resume
> events from logind and drop all the Active-Prop-Changed handling. logind
> guarantees proper ordering of both. However, in case we ever change weston
> to support multiple GPUs, we need the per-device notification. Thus, keep
> the code. This also makes weston more fail-safe in case logind fails to
> send the PauseDevice event (for whatever reason..).
This explains the intermittent pageflip failures I was seeing. The
patch makes sense and makes VT switching work almost reliably. The
one missing thing is that occasionally my pointer doesn't work. It
looks like it remains locked by the X driver, and since it's only my
touchpad (keyboard always works) I'm guessing it's a
xf86-input-synaptics problem. It happens with weston-launch too, but
it doesn't look like a race. I can reproduce with just VT switching
from X to a VT and running evtest. That is, the device remains locked
long enough for me to type evtest, select the device and then see the
"Device is grabbed..." message. It doesn't get ungrabbed until I VT
switch back to X and then back to the VT.
At this point I'm calling it a synaptics bug and merging your patch.
Kristian
> ---
> src/logind-util.c | 58 +++++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 37 insertions(+), 21 deletions(-)
>
> diff --git a/src/logind-util.c b/src/logind-util.c
> index 5d163a6..40d811b 100644
> --- a/src/logind-util.c
> +++ b/src/logind-util.c
> @@ -44,6 +44,8 @@
> #include "dbus.h"
> #include "logind-util.h"
>
> +#define DRM_MAJOR 226
> +
> #ifndef KDSKBMUTE
> #define KDSKBMUTE 0x4B51
> #endif
> @@ -275,10 +277,10 @@ weston_logind_activate_vt(struct weston_logind *wl, int vt)
> static void
> weston_logind_set_active(struct weston_logind *wl, bool active)
> {
> - if (active)
> - wl->compositor->session_active = 1;
> - else
> - wl->compositor->session_active = 0;
> + if (!wl->compositor->session_active == !active)
> + return;
> +
> + wl->compositor->session_active = active;
>
> wl_signal_emit(&wl->compositor->session_signal,
> wl->compositor);
> @@ -314,7 +316,8 @@ get_active_cb(DBusPendingCall *pending, void *data)
> goto err_unref;
>
> dbus_message_iter_get_basic(&sub, &b);
> - weston_logind_set_active(wl, b);
> + if (!b)
> + weston_logind_set_active(wl, false);
>
> err_unref:
> dbus_message_unref(m);
> @@ -428,7 +431,8 @@ property_changed(struct weston_logind *wl, DBusMessage *m)
> if (!strcmp(name, "Active")) {
> if (dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_BOOLEAN) {
> dbus_message_iter_get_basic(&entry, &b);
> - weston_logind_set_active(wl, b);
> + if (!b)
> + weston_logind_set_active(wl, false);
> return;
> }
> }
> @@ -478,31 +482,43 @@ device_paused(struct weston_logind *wl, DBusMessage *m)
>
> /* "pause" means synchronous pausing. Acknowledge it unconditionally
> * as we support asynchronous device shutdowns, anyway.
> - * "force" means asynchronous pausing. We ignore it as the following
> - * session-deactivation will suffice as notification.
> - * "gone" means the device is gone. We ignore it as we receive a
> - * udev notification, anyway. */
> + * "force" means asynchronous pausing.
> + * "gone" means the device is gone. We handle it the same as "force" as
> + * a following udev event will be caught, too.
> + *
> + * If it's our main DRM device, tell the compositor to go asleep. */
>
> if (!strcmp(type, "pause"))
> weston_logind_pause_device_complete(wl, major, minor);
> +
> + if (major == DRM_MAJOR)
> + weston_logind_set_active(wl, false);
> }
>
> static void
> device_resumed(struct weston_logind *wl, DBusMessage *m)
> {
> - /*
> - * DeviceResumed messages provide us a new file-descriptor for
> + bool r;
> + uint32_t major;
> +
> + r = dbus_message_get_args(m, NULL,
> + DBUS_TYPE_UINT32, &major,
> + /*DBUS_TYPE_UINT32, &minor,
> + DBUS_TYPE_UNIX_FD, &fd,*/
> + DBUS_TYPE_INVALID);
> + if (!r) {
> + weston_log("logind: cannot parse ResumeDevice dbus signal\n");
> + return;
> + }
> +
> + /* DeviceResumed messages provide us a new file-descriptor for
> * resumed devices. For DRM devices it's the same as before, for evdev
> * devices it's a new open-file. As we reopen evdev devices, anyway,
> - * there is no need for us to handle this event. If we ever optimize
> - * our evdev code to support resuming devices, this event can be
> - * parsed like this:
> - * r = dbus_message_get_args(m, NULL,
> - * DBUS_TYPE_UINT32, &major,
> - * DBUS_TYPE_UINT32, &minor,
> - * DBUS_TYPE_UNIX_FD, &fd,
> - * DBUS_TYPE_INVALID);
> - */
> + * there is no need for us to handle this event for evdev. For DRM, we
> + * notify the compositor to wake up. */
> +
> + if (major == DRM_MAJOR)
> + weston_logind_set_active(wl, true);
> }
>
> static DBusHandlerResult
> --
> 1.8.4.2
>
More information about the wayland-devel
mailing list