[PATCH] logind: delay wakeup until DRM-device is resumed

David Herrmann dh.herrmann at gmail.com
Sat Nov 30 02:25:45 PST 2013


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..).
---
 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