backend/drm: introduce page-flip tracking struct

Introduce a per-page-flip tracking struct passed to the kernel
when we request a page-flip event for an atomic commit. The kernel
will pass us back this pointer when delivering the event.

This eliminates any risk of mixing up events together. In particular,
if two events are pending, or if the CRTC of a connector is swapped,
we no longer blow up in the page-flip event handler.

Closes: https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3753

backend/drm/atomic.c

@@ -61,13 +61,14 @@ static void atomic_begin(struct atomic *atom) {
 }
 
 static bool atomic_commit(struct atomic *atom,
-		struct wlr_drm_connector *conn, uint32_t flags) {
+		struct wlr_drm_connector *conn, struct wlr_drm_page_flip *page_flip,
+		uint32_t flags) {
 	struct wlr_drm_backend *drm = conn->backend;
 	if (atom->failed) {
 		return false;
 	}
 
-	int ret = drmModeAtomicCommit(drm->fd, atom->req, flags, drm);
+	int ret = drmModeAtomicCommit(drm->fd, atom->req, flags, page_flip);
 	if (ret != 0) {
 		wlr_drm_conn_log_errno(conn,
 			(flags & DRM_MODE_ATOMIC_TEST_ONLY) ? WLR_DEBUG : WLR_ERROR,
@@ -257,8 +258,8 @@ static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm,
 }
 
 static bool atomic_crtc_commit(struct wlr_drm_connector *conn,
-		const struct wlr_drm_connector_state *state, uint32_t flags,
-		bool test_only) {
+		const struct wlr_drm_connector_state *state,
+		struct wlr_drm_page_flip *page_flip, uint32_t flags, bool test_only) {
 	struct wlr_drm_backend *drm = conn->backend;
 	struct wlr_output *output = &conn->output;
 	struct wlr_drm_crtc *crtc = conn->crtc;
@@ -367,7 +368,7 @@ static bool atomic_crtc_commit(struct wlr_drm_connector *conn,
 		}
 	}
 
-	bool ok = atomic_commit(&atom, conn, flags);
+	bool ok = atomic_commit(&atom, conn, page_flip, flags);
 	atomic_finish(&atom);
 
 	if (ok && !test_only) {

backend/drm/drm.c

@@ -412,15 +412,32 @@ static struct wlr_drm_layer *get_or_create_layer(struct wlr_drm_backend *drm,
 	return layer;
 }
 
+static void drm_connector_set_pending_page_flip(struct wlr_drm_connector *conn,
+		struct wlr_drm_page_flip *page_flip) {
+	if (conn->pending_page_flip != NULL) {
+		conn->pending_page_flip->conn = NULL;
+	}
+	conn->pending_page_flip = page_flip;
+}
+
 static bool drm_crtc_commit(struct wlr_drm_connector *conn,
 		const struct wlr_drm_connector_state *state,
 		uint32_t flags, bool test_only) {
 	// Disallow atomic-only flags
 	assert((flags & ~DRM_MODE_PAGE_FLIP_FLAGS) == 0);
 
+	struct wlr_drm_page_flip *page_flip = NULL;
+	if (flags & DRM_MODE_PAGE_FLIP_EVENT) {
+		page_flip = calloc(1, sizeof(*page_flip));
+		if (page_flip == NULL) {
+			return false;
+		}
+		page_flip->conn = conn;
+	}
+
 	struct wlr_drm_backend *drm = conn->backend;
 	struct wlr_drm_crtc *crtc = conn->crtc;
-	bool ok = drm->iface->crtc_commit(conn, state, flags, test_only);
+	bool ok = drm->iface->crtc_commit(conn, state, page_flip, flags, test_only);
 	if (ok && !test_only) {
 		drm_fb_clear(&crtc->primary->queued_fb);
 		if (state->primary_fb != NULL) {
@@ -434,6 +451,8 @@ static bool drm_crtc_commit(struct wlr_drm_connector *conn,
 		wl_list_for_each(layer, &crtc->layers, link) {
 			drm_fb_move(&layer->queued_fb, &layer->pending_fb);
 		}
+
+		drm_connector_set_pending_page_flip(conn, page_flip);
 	} else {
 		// The set_cursor() hook is a bit special: it's not really synchronized
 		// to commit() or test(). Once set_cursor() returns true, the new
@@ -446,6 +465,8 @@ static bool drm_crtc_commit(struct wlr_drm_connector *conn,
 		wl_list_for_each(layer, &crtc->layers, link) {
 			drm_fb_clear(&layer->pending_fb);
 		}
+
+		free(page_flip);
 	}
 	return ok;
 }
@@ -732,16 +753,6 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn,
 		if (!drm_connector_state_update_primary_fb(conn, &pending)) {
 			goto out;
 		}
-
-		// wlr_drm_interface.crtc_commit will perform either a non-blocking
-		// page-flip, either a blocking modeset. When performing a blocking modeset
-		// we'll wait for all queued page-flips to complete, so we don't need this
-		// safeguard.
-		if (conn->pending_page_flip_crtc && !pending.modeset) {
-			wlr_drm_conn_log(conn, WLR_ERROR, "Failed to page-flip output: "
-				"a page-flip is already pending");
-			goto out;
-		}
 	}
 	if (pending.base->committed & WLR_OUTPUT_STATE_LAYERS) {
 		if (!drm_connector_set_pending_layer_fbs(conn, pending.base)) {
@@ -759,7 +770,19 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn,
 		}
 	}
 
-	uint32_t flags = pending.active ? DRM_MODE_PAGE_FLIP_EVENT : 0;
+	uint32_t flags = 0;
+	if (pending.active) {
+		flags |= DRM_MODE_PAGE_FLIP_EVENT;
+		// wlr_drm_interface.crtc_commit will perform either a non-blocking
+		// page-flip, either a blocking modeset. When performing a blocking modeset
+		// we'll wait for all queued page-flips to complete, so we don't need this
+		// safeguard.
+		if (conn->pending_page_flip != NULL && !pending.modeset) {
+			wlr_drm_conn_log(conn, WLR_ERROR, "Failed to page-flip output: "
+				"a page-flip is already pending");
+			goto out;
+		}
+	}
 	if (pending.base->tearing_page_flip) {
 		flags |= DRM_MODE_PAGE_FLIP_ASYNC;
 	}
@@ -777,9 +800,6 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn,
 		conn->cursor_enabled = false;
 		conn->crtc = NULL;
 	}
-	if (flags & DRM_MODE_PAGE_FLIP_EVENT) {
-		conn->pending_page_flip_crtc = conn->crtc->id;
-	}
 
 out:
 	drm_connector_state_finish(&pending);
@@ -1029,7 +1049,7 @@ static void drm_connector_destroy_output(struct wlr_output *output) {
 	dealloc_crtc(conn);
 
 	conn->status = DRM_MODE_DISCONNECTED;
-	conn->pending_page_flip_crtc = 0;
+	drm_connector_set_pending_page_flip(conn, NULL);
 
 	struct wlr_drm_mode *mode, *mode_tmp;
 	wl_list_for_each_safe(mode, mode_tmp, &conn->output.modes, wlr_mode.link) {
@@ -1660,22 +1680,19 @@ static int mhz_to_nsec(int mhz) {
 
 static void handle_page_flip(int fd, unsigned seq,
 		unsigned tv_sec, unsigned tv_usec, unsigned crtc_id, void *data) {
-	struct wlr_drm_backend *drm = data;
+	struct wlr_drm_page_flip *page_flip = data;
 
-	bool found = false;
-	struct wlr_drm_connector *conn;
-	wl_list_for_each(conn, &drm->connectors, link) {
-		if (conn->pending_page_flip_crtc == crtc_id) {
-			found = true;
-			break;
-		}
+	struct wlr_drm_connector *conn = page_flip->conn;
+	if (conn != NULL) {
+		conn->pending_page_flip = NULL;
 	}
-	if (!found) {
-		wlr_log(WLR_DEBUG, "Unexpected page-flip event for CRTC %u", crtc_id);
+	free(page_flip);
+
+	if (conn == NULL) {
 		return;
 	}
 
-	conn->pending_page_flip_crtc = 0;
+	struct wlr_drm_backend *drm = conn->backend;
 
 	if (conn->status != DRM_MODE_CONNECTED || conn->crtc == NULL) {
 		wlr_drm_conn_log(conn, WLR_DEBUG,

backend/drm/legacy.c

@@ -59,7 +59,7 @@ static bool legacy_crtc_test(struct wlr_drm_connector *conn,
 
 static bool legacy_crtc_commit(struct wlr_drm_connector *conn,
 		const struct wlr_drm_connector_state *state,
-		uint32_t flags, bool test_only) {
+		struct wlr_drm_page_flip *page_flip, uint32_t flags, bool test_only) {
 	if (!legacy_crtc_test(conn, state)) {
 		return false;
 	}
@@ -181,7 +181,7 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn,
 		}
 
 		if (drmModePageFlip(drm->fd, crtc->id, fb_id,
-				page_flags, drm)) {
+				page_flags, page_flip)) {
 			wlr_drm_conn_log_errno(conn, WLR_ERROR, "drmModePageFlip failed");
 			return false;
 		}

backend/drm/libliftoff.c

@@ -301,8 +301,8 @@ static void update_layer_feedback(struct wlr_drm_backend *drm,
 }
 
 static bool crtc_commit(struct wlr_drm_connector *conn,
-		const struct wlr_drm_connector_state *state, uint32_t flags,
-		bool test_only) {
+		const struct wlr_drm_connector_state *state,
+		struct wlr_drm_page_flip *page_flip, uint32_t flags, bool test_only) {
 	struct wlr_drm_backend *drm = conn->backend;
 	struct wlr_output *output = &conn->output;
 	struct wlr_drm_crtc *crtc = conn->crtc;
@@ -455,7 +455,7 @@ static bool crtc_commit(struct wlr_drm_connector *conn,
 		goto out;
 	}
 
-	ret = drmModeAtomicCommit(drm->fd, req, flags, drm);
+	ret = drmModeAtomicCommit(drm->fd, req, flags, page_flip);
 	if (ret != 0) {
 		wlr_drm_conn_log_errno(conn, test_only ? WLR_DEBUG : WLR_ERROR,
 			"Atomic commit failed");

include/backend/drm/drm.h

@@ -129,6 +129,22 @@ struct wlr_drm_connector_state {
 	struct wlr_drm_fb *primary_fb;
 };
 
+/**
+ * Per-page-flip tracking struct.
+ *
+ * We've asked for a state change in the kernel, and yet to receive a
+ * notification for its completion. Currently, the kernel only has a queue
+ * length of 1, and no way to modify your submissions after they're sent.
+ *
+ * However, we might have multiple in-flight page-flip events, for instance
+ * when performing a non-blocking commit followed by a blocking commit. In
+ * that case, conn will be set to NULL on the non-blocking commit to indicate
+ * that it's been superseded.
+ */
+struct wlr_drm_page_flip {
+	struct wlr_drm_connector *conn;
+};
+
 struct wlr_drm_connector {
 	struct wlr_output output; // only valid if status != DISCONNECTED
 
@@ -153,14 +169,8 @@ struct wlr_drm_connector {
 
 	struct wl_list link; // wlr_drm_backend.connectors
 
-	/* CRTC ID if a page-flip is pending, zero otherwise.
-	 *
-	 * We've asked for a state change in the kernel, and yet to receive a
-	 * notification for its completion. Currently, the kernel only has a
-	 * queue length of 1, and no way to modify your submissions after
-	 * they're sent.
-	 */
-	uint32_t pending_page_flip_crtc;
+	// Last committed page-flip
+	struct wlr_drm_page_flip *pending_page_flip;
 };
 
 struct wlr_drm_backend *get_drm_backend_from_backend(

include/backend/drm/iface.h

@@ -12,6 +12,7 @@ struct wlr_drm_connector;
 struct wlr_drm_crtc;
 struct wlr_drm_connector_state;
 struct wlr_drm_fb;
+struct wlr_drm_page_flip;
 
 // Used to provide atomic or legacy DRM functions
 struct wlr_drm_interface {
@@ -19,8 +20,8 @@ struct wlr_drm_interface {
 	void (*finish)(struct wlr_drm_backend *drm);
 	// Commit all pending changes on a CRTC.
 	bool (*crtc_commit)(struct wlr_drm_connector *conn,
-		const struct wlr_drm_connector_state *state, uint32_t flags,
-		bool test_only);
+		const struct wlr_drm_connector_state *state,
+		struct wlr_drm_page_flip *page_flip, uint32_t flags, bool test_only);
 };
 
 extern const struct wlr_drm_interface atomic_iface;