backend/drm: introduce pending and current CRTC state

Previously, we only had the pending state (crtc->pending, crtc->mode and
crtc->active). This causes issues when a commit fails: the pending state
is left as-is, and the next commit may read stale data from it.

This will also cause issues when implementing test-only commits: we need
to rollback the pending CRTC state after a test-only commit.

Introduce separate pending and current CRTC states. Properly update the
current state after a commit.

backend/drm/atomic.c

@@ -56,12 +56,12 @@ static void atomic_add(struct atomic *atom, uint32_t id, uint32_t prop, uint64_t
 
 static bool create_mode_blob(struct wlr_drm_backend *drm,
 		struct wlr_drm_crtc *crtc, uint32_t *blob_id) {
-	if (!crtc->active) {
+	if (!crtc->pending.active) {
 		*blob_id = 0;
 		return true;
 	}
 
-	if (drmModeCreatePropertyBlob(drm->fd, &crtc->mode,
+	if (drmModeCreatePropertyBlob(drm->fd, &crtc->pending.mode->drm_mode,
 			sizeof(drmModeModeInfo), blob_id)) {
 		wlr_log_errno(WLR_ERROR, "Unable to create mode property blob");
 		return false;
@@ -171,7 +171,7 @@ static bool atomic_crtc_commit(struct wlr_drm_backend *drm,
 	struct wlr_drm_crtc *crtc = conn->crtc;
 
 	uint32_t mode_id = crtc->mode_id;
-	if (crtc->pending & WLR_DRM_CRTC_MODE) {
+	if (crtc->pending_modeset) {
 		if (!create_mode_blob(drm, crtc, &mode_id)) {
 			return false;
 		}
@@ -196,8 +196,7 @@ static bool atomic_crtc_commit(struct wlr_drm_backend *drm,
 		}
 	}
 
-	bool modeset = crtc->pending & WLR_DRM_CRTC_MODE;
+	if (crtc->pending_modeset) {
-	if (modeset) {
 		flags |= DRM_MODE_ATOMIC_ALLOW_MODESET;
 	} else {
 		flags |= DRM_MODE_ATOMIC_NONBLOCK;
@@ -206,14 +205,15 @@ static bool atomic_crtc_commit(struct wlr_drm_backend *drm,
 	struct atomic atom;
 	atomic_begin(&atom);
 	atomic_add(&atom, conn->id, conn->props.crtc_id,
-		crtc->active ? crtc->id : 0);
+		crtc->pending.active ? crtc->id : 0);
-	if (modeset && crtc->active && conn->props.link_status != 0) {
+	if (crtc->pending_modeset && crtc->pending.active &&
+			conn->props.link_status != 0) {
 		atomic_add(&atom, conn->id, conn->props.link_status,
 			DRM_MODE_LINK_STATUS_GOOD);
 	}
 	atomic_add(&atom, crtc->id, crtc->props.mode_id, mode_id);
-	atomic_add(&atom, crtc->id, crtc->props.active, crtc->active);
+	atomic_add(&atom, crtc->id, crtc->props.active, crtc->pending.active);
-	if (crtc->active) {
+	if (crtc->pending.active) {
 		if (crtc->props.gamma_lut != 0) {
 			atomic_add(&atom, crtc->id, crtc->props.gamma_lut, gamma_lut);
 		}

backend/drm/drm.c

@@ -336,7 +336,12 @@ static bool drm_crtc_commit(struct wlr_drm_connector *conn, uint32_t flags) {
 		get_drm_backend_from_backend(conn->output.backend);
 	struct wlr_drm_crtc *crtc = conn->crtc;
 	bool ok = drm->iface->crtc_commit(drm, conn, flags);
-	crtc->pending = 0;
+	if (ok) {
+		memcpy(&crtc->current, &crtc->pending, sizeof(struct wlr_drm_crtc_state));
+	} else {
+		memcpy(&crtc->pending, &crtc->current, sizeof(struct wlr_drm_crtc_state));
+	}
+	crtc->pending_modeset = false;
 	return ok;
 }
 
@@ -349,7 +354,7 @@ static bool drm_crtc_page_flip(struct wlr_drm_connector *conn) {
 		return false;
 	}
 
-	assert(crtc->active);
+	assert(crtc->pending.active);
 	assert(plane_get_next_fb(crtc->primary)->type != WLR_DRM_FB_TYPE_NONE);
 	if (!drm_crtc_commit(conn, DRM_MODE_PAGE_FLIP_EVENT)) {
 		return false;
@@ -692,9 +697,9 @@ static bool drm_connector_init_renderer(struct wlr_drm_connector *conn,
 	}
 	struct wlr_drm_plane *plane = crtc->primary;
 
-	crtc->pending |= WLR_DRM_CRTC_MODE;
+	crtc->pending_modeset = true;
-	memcpy(&crtc->mode, &mode->drm_mode, sizeof(drmModeModeInfo));
+	crtc->pending.active = true;
-	crtc->active = true;
+	crtc->pending.mode = mode;
 
 	int width = mode->wlr_mode.width;
 	int height = mode->wlr_mode.height;
@@ -723,6 +728,10 @@ static bool drm_connector_init_renderer(struct wlr_drm_connector *conn,
 			"retrying without modifiers");
 		modifiers = false;
 
+		crtc->pending_modeset = true;
+		crtc->pending.active = true;
+		crtc->pending.mode = mode;
+
 		if (!drm_plane_init_surface(plane, drm, width, height, format,
 				0, modifiers)) {
 			return false;
@@ -767,8 +776,8 @@ bool drm_connector_set_mode(struct wlr_drm_connector *conn,
 
 	if (wlr_mode == NULL) {
 		if (conn->crtc != NULL) {
-			conn->crtc->active = false;
+			conn->crtc->pending_modeset = true;
-			conn->crtc->pending |= WLR_DRM_CRTC_MODE;
+			conn->crtc->pending.active = false;
 			if (!drm_crtc_commit(conn, 0)) {
 				return false;
 			}

backend/drm/legacy.c

@@ -15,7 +15,7 @@ static bool legacy_crtc_commit(struct wlr_drm_backend *drm,
 	struct wlr_drm_plane *cursor = crtc->cursor;
 
 	uint32_t fb_id = 0;
-	if (crtc->active) {
+	if (crtc->pending.active) {
 		struct wlr_drm_fb *fb = plane_get_next_fb(crtc->primary);
 		struct gbm_bo *bo = drm_fb_acquire(fb, drm, &crtc->primary->mgpu_surf);
 		if (!bo) {
@@ -28,18 +28,20 @@ static bool legacy_crtc_commit(struct wlr_drm_backend *drm,
 		}
 	}
 
-	if (crtc->pending & WLR_DRM_CRTC_MODE) {
+	if (crtc->pending_modeset) {
 		uint32_t *conns = NULL;
 		size_t conns_len = 0;
 		drmModeModeInfo *mode = NULL;
-		if (crtc->active) {
+		if (crtc->pending.mode != NULL) {
 			conns = &conn->id;
 			conns_len = 1;
-			mode = &crtc->mode;
+			mode = &crtc->pending.mode->drm_mode;
 		}
 
+		uint32_t dpms = crtc->pending.active ?
+			DRM_MODE_DPMS_ON : DRM_MODE_DPMS_OFF;
 		if (drmModeConnectorSetProperty(drm->fd, conn->id, conn->props.dpms,
-				crtc->active ? DRM_MODE_DPMS_ON : DRM_MODE_DPMS_OFF) != 0) {
+				dpms) != 0) {
 			wlr_log_errno(WLR_ERROR, "%s: failed to set DPMS property",
 				conn->output.name);
 			return false;

include/backend/drm/drm.h

@@ -42,16 +42,16 @@ struct wlr_drm_plane {
 	union wlr_drm_plane_props props;
 };
 
-enum wlr_drm_crtc_field {
+struct wlr_drm_crtc_state {
-	WLR_DRM_CRTC_MODE = 1 << 0,
+	bool active;
+	struct wlr_drm_mode *mode;
 };
 
 struct wlr_drm_crtc {
 	uint32_t id;
-	uint32_t pending; // bitfield of enum wlr_drm_crtc_field
 
-	bool active;
+	bool pending_modeset;
-	drmModeModeInfo mode;
+	struct wlr_drm_crtc_state pending, current;
 
 	// Atomic modesetting only
 	uint32_t mode_id;