Merge branch 'drm-pending-crtc-state' into 'master'

Draft: backend/drm: move pending CRTC to connector state

See merge request wlroots/wlroots!3928

backend/drm/atomic.c

@@ -171,7 +171,7 @@ static void plane_disable(struct atomic *atom, struct wlr_drm_plane *plane) {
 }
 
 static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm,
-		struct wlr_drm_plane *plane, struct wlr_drm_fb *fb, uint32_t crtc_id,
+		struct wlr_drm_plane *plane, struct wlr_drm_fb *fb, struct wlr_drm_crtc *crtc,
 		int32_t x, int32_t y) {
 	uint32_t id = plane->id;
 	const union wlr_drm_plane_props *props = &plane->props;
@@ -193,7 +193,7 @@ static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm,
 	atomic_add(atom, id, props->crtc_w, width);
 	atomic_add(atom, id, props->crtc_h, height);
 	atomic_add(atom, id, props->fb_id, fb->id);
-	atomic_add(atom, id, props->crtc_id, crtc_id);
+	atomic_add(atom, id, props->crtc_id, crtc->id);
 	atomic_add(atom, id, props->crtc_x, (uint64_t)x);
 	atomic_add(atom, id, props->crtc_y, (uint64_t)y);
 }
@@ -203,10 +203,10 @@ static bool atomic_crtc_commit(struct wlr_drm_connector *conn,
 		bool test_only) {
 	struct wlr_drm_backend *drm = conn->backend;
 	struct wlr_output *output = &conn->output;
-	struct wlr_drm_crtc *crtc = conn->crtc;
 
 	bool modeset = state->modeset;
 	bool active = state->active;
+	struct wlr_drm_crtc *crtc = state->crtc;
 
 	uint32_t mode_id = crtc->mode_id;
 	if (modeset) {
@@ -294,7 +294,7 @@ static bool atomic_crtc_commit(struct wlr_drm_connector *conn,
 		if (crtc->props.vrr_enabled != 0) {
 			atomic_add(&atom, crtc->id, crtc->props.vrr_enabled, vrr_enabled);
 		}
-		set_plane_props(&atom, drm, crtc->primary, state->primary_fb, crtc->id,
+		set_plane_props(&atom, drm, crtc->primary, state->primary_fb, crtc,
 			0, 0);
 		if (crtc->primary->props.fb_damage_clips != 0) {
 			atomic_add(&atom, crtc->primary->id,
@@ -303,7 +303,7 @@ static bool atomic_crtc_commit(struct wlr_drm_connector *conn,
 		if (crtc->cursor) {
 			if (drm_connector_is_cursor_visible(conn)) {
 				set_plane_props(&atom, drm, crtc->cursor, get_next_cursor_fb(conn),
-					crtc->id, conn->cursor_x, conn->cursor_y);
+					crtc, conn->cursor_x, conn->cursor_y);
 			} else {
 				plane_disable(&atom, crtc->cursor);
 			}

backend/drm/backend.c

@@ -33,7 +33,7 @@ static void backend_destroy(struct wlr_backend *backend) {
 	struct wlr_drm_backend *drm = get_drm_backend_from_backend(backend);
 
 	struct wlr_drm_connector *conn, *next;
-	wl_list_for_each_safe(conn, next, &drm->outputs, link) {
+	wl_list_for_each_safe(conn, next, &drm->connectors, link) {
 		destroy_drm_connector(conn);
 	}
 
@@ -104,7 +104,7 @@ static void handle_session_active(struct wl_listener *listener, void *data) {
 		scan_drm_connectors(drm, NULL);
 
 		struct wlr_drm_connector *conn;
-		wl_list_for_each(conn, &drm->outputs, link) {
+		wl_list_for_each(conn, &drm->connectors, link) {
 			struct wlr_output_mode *mode = NULL;
 			uint32_t committed = WLR_OUTPUT_STATE_ENABLED;
 			if (conn->status != DRM_MODE_DISCONNECTED && conn->output.enabled
@@ -195,7 +195,7 @@ struct wlr_backend *wlr_drm_backend_create(struct wl_display *display,
 
 	drm->session = session;
 	wl_list_init(&drm->fbs);
-	wl_list_init(&drm->outputs);
+	wl_list_init(&drm->connectors);
 
 	drm->dev = dev;
 	drm->fd = dev->fd;

backend/drm/drm.c

@@ -329,7 +329,7 @@ static bool drm_crtc_commit(struct wlr_drm_connector *conn,
 	assert((flags & ~DRM_MODE_PAGE_FLIP_FLAGS) == 0);
 
 	struct wlr_drm_backend *drm = conn->backend;
-	struct wlr_drm_crtc *crtc = conn->crtc;
+	struct wlr_drm_crtc *crtc = state->crtc;
 	bool ok = drm->iface->crtc_commit(conn, state, flags, test_only);
 	if (ok && !test_only) {
 		drm_fb_clear(&crtc->primary->queued_fb);
@@ -354,10 +354,12 @@ static void drm_connector_state_init(struct wlr_drm_connector_state *state,
 		struct wlr_drm_connector *conn,
 		const struct wlr_output_state *base) {
 	memset(state, 0, sizeof(*state));
+	state->conn = conn;
 	state->base = base;
 	state->modeset = base->allow_artifacts;
 	state->active = (base->committed & WLR_OUTPUT_STATE_ENABLED) ?
 		base->enabled : conn->output.enabled;
+	state->crtc = conn->crtc;
 
 	if (base->committed & WLR_OUTPUT_STATE_MODE) {
 		switch (base->mode_type) {
@@ -400,9 +402,7 @@ static bool drm_connector_state_update_primary_fb(struct wlr_drm_connector *conn
 
 	assert(state->base->committed & WLR_OUTPUT_STATE_BUFFER);
 
-	struct wlr_drm_crtc *crtc = conn->crtc;
+	struct wlr_drm_crtc *crtc = state->crtc;
-	assert(crtc != NULL);
-
 	struct wlr_drm_plane *plane = crtc->primary;
 	struct wlr_buffer *source_buf = state->base->buffer;
 
@@ -443,7 +443,15 @@ static bool drm_connector_state_update_primary_fb(struct wlr_drm_connector *conn
 	return true;
 }
 
-static bool drm_connector_alloc_crtc(struct wlr_drm_connector *conn);
+static void realloc_crtcs(struct wlr_drm_backend *drm,
+	struct wlr_drm_connector_state *want_conn_state);
+
+static bool drm_connector_state_alloc_crtc(struct wlr_drm_connector_state *state) {
+	if (state->crtc == NULL) {
+		realloc_crtcs(state->conn->backend, state);
+	}
+	return state->crtc != NULL;
+}
 
 static bool drm_connector_test(struct wlr_output *output,
 		const struct wlr_output_state *state) {
@@ -487,7 +495,7 @@ static bool drm_connector_test(struct wlr_output *output,
 			goto out;
 		}
 
-		if (!drm_connector_alloc_crtc(conn)) {
+		if (!drm_connector_state_alloc_crtc(&pending)) {
 			wlr_drm_conn_log(conn, WLR_DEBUG,
 				"No CRTC available for this connector");
 			goto out;
@@ -507,7 +515,7 @@ static bool drm_connector_test(struct wlr_output *output,
 		goto out;
 	}
 
-	if (!conn->crtc) {
+	if (!pending.crtc) {
 		// If the output is disabled, we don't have a crtc even after
 		// reallocation
 		ok = true;
@@ -577,7 +585,7 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn,
 	}
 
 	if (pending.active) {
-		if (!drm_connector_alloc_crtc(conn)) {
+		if (!drm_connector_state_alloc_crtc(&pending)) {
 			wlr_drm_conn_log(conn, WLR_ERROR,
 				"No CRTC available for this connector");
 			goto out;
@@ -699,16 +707,6 @@ struct wlr_drm_fb *get_next_cursor_fb(struct wlr_drm_connector *conn) {
 	return conn->crtc->cursor->current_fb;
 }
 
-static void realloc_crtcs(struct wlr_drm_backend *drm,
-	struct wlr_drm_connector *want_conn);
-
-static bool drm_connector_alloc_crtc(struct wlr_drm_connector *conn) {
-	if (conn->crtc == NULL) {
-		realloc_crtcs(conn->backend, conn);
-	}
-	return conn->crtc != NULL;
-}
-
 static struct wlr_drm_mode *drm_mode_create(const drmModeModeInfo *modeinfo) {
 	struct wlr_drm_mode *mode = calloc(1, sizeof(*mode));
 	if (!mode) {
@@ -1026,18 +1024,20 @@ static void dealloc_crtc(struct wlr_drm_connector *conn) {
 }
 
 static void realloc_crtcs(struct wlr_drm_backend *drm,
-		struct wlr_drm_connector *want_conn) {
+		struct wlr_drm_connector_state *want_conn_state) {
 	assert(drm->num_crtcs > 0);
 
-	size_t num_outputs = wl_list_length(&drm->outputs);
+	struct wlr_drm_connector *want_conn = want_conn_state != NULL ? want_conn_state->conn : NULL;
-	if (num_outputs == 0) {
+
+	size_t num_connectors = wl_list_length(&drm->connectors);
+	if (num_connectors == 0) {
 		return;
 	}
 
 	wlr_log(WLR_DEBUG, "Reallocating CRTCs");
 
-	struct wlr_drm_connector *connectors[num_outputs];
+	struct wlr_drm_connector *connectors[num_connectors];
-	uint32_t connector_constraints[num_outputs];
+	uint32_t connector_constraints[num_connectors];
 	uint32_t previous_match[drm->num_crtcs];
 	uint32_t new_match[drm->num_crtcs];
 
@@ -1048,7 +1048,7 @@ static void realloc_crtcs(struct wlr_drm_backend *drm,
 	wlr_log(WLR_DEBUG, "State before reallocation:");
 	size_t i = 0;
 	struct wlr_drm_connector *conn;
-	wl_list_for_each(conn, &drm->outputs, link) {
+	wl_list_for_each(conn, &drm->connectors, link) {
 		connectors[i] = conn;
 
 		if (conn->crtc) {
@@ -1059,9 +1059,9 @@ static void realloc_crtcs(struct wlr_drm_backend *drm,
 		// connector the user wants to enable
 		bool want_crtc = conn == want_conn || conn->output.enabled;
 
-		wlr_log(WLR_DEBUG, "  '%s' crtc=%d status=%d want_crtc=%d",
+		wlr_log(WLR_DEBUG, "  '%s': crtc=%d status=%s want_crtc=%d",
 			conn->name, conn->crtc ? (int)(conn->crtc - drm->crtcs) : -1,
-			conn->status, want_crtc);
+			drm_connector_status_str(conn->status), want_crtc);
 
 		if (conn->status == DRM_MODE_CONNECTED && want_crtc) {
 			connector_constraints[i] = conn->possible_crtcs;
@@ -1073,12 +1073,12 @@ static void realloc_crtcs(struct wlr_drm_backend *drm,
 		++i;
 	}
 
-	match_obj(num_outputs, connector_constraints,
+	match_obj(num_connectors, connector_constraints,
 		drm->num_crtcs, previous_match, new_match);
 
 	// Converts our crtc=>connector result into a connector=>crtc one.
-	ssize_t connector_match[num_outputs];
+	ssize_t connector_match[num_connectors];
-	for (size_t i = 0 ; i < num_outputs; ++i) {
+	for (size_t i = 0 ; i < num_connectors; ++i) {
 		connector_match[i] = -1;
 	}
 	for (size_t i = 0; i < drm->num_crtcs; ++i) {
@@ -1087,46 +1087,40 @@ static void realloc_crtcs(struct wlr_drm_backend *drm,
 		}
 	}
 
-	/*
+	// Refuse to remove a CRTC from an enabled connector, and refuse to
-	 * In the case that we add a new connector (hotplug) and we fail to
+	// change the CRTC of an enabled connector.
-	 * match everything, we prefer to fail the new connector and keep all
+	for (size_t i = 0; i < num_connectors; ++i) {
-	 * of the old mappings instead.
-	 */
-	for (size_t i = 0; i < num_outputs; ++i) {
 		struct wlr_drm_connector *conn = connectors[i];
-		if (conn->status == DRM_MODE_CONNECTED && conn->output.enabled &&
+		if (conn->status != DRM_MODE_CONNECTED || !conn->output.enabled) {
-				connector_match[i] == -1) {
+			continue;
+		}
+		if (connector_match[i] == -1) {
 			wlr_log(WLR_DEBUG, "Could not match a CRTC for previously connected output; "
-					"keeping old configuration");
+				"keeping old configuration");
+			return;
+		}
+		assert(conn->crtc != NULL);
+		if (connector_match[i] != conn->crtc - drm->crtcs) {
+			wlr_log(WLR_DEBUG, "Cannot switch CRTC for enabled output; "
+				"keeping old configuration");
 			return;
 		}
 	}
-	wlr_log(WLR_DEBUG, "State after reallocation:");
 
 	// Apply new configuration
-	for (size_t i = 0; i < num_outputs; ++i) {
+	wlr_log(WLR_DEBUG, "State after reallocation:");
+	for (size_t i = 0; i < num_connectors; ++i) {
 		struct wlr_drm_connector *conn = connectors[i];
-		bool prev_enabled = conn->crtc;
 
-		wlr_log(WLR_DEBUG, "  '%s' crtc=%zd status=%d",
+		wlr_log(WLR_DEBUG, "  '%s': crtc=%zd",
-			conn->name, connector_match[i], conn->status);
+			conn->name, connector_match[i]);
 
-		// We don't need to change anything.
+		if (conn->crtc != NULL && connector_match[i] == conn->crtc - drm->crtcs) {
-		if (prev_enabled && connector_match[i] == conn->crtc - drm->crtcs) {
+			// We don't need to change anything
 			continue;
 		}
 
 		dealloc_crtc(conn);
-
-		if (connector_match[i] == -1) {
-			if (prev_enabled) {
-				wlr_drm_conn_log(conn, WLR_DEBUG, "Output has lost its CRTC");
-				wlr_output_update_enabled(&conn->output, false);
-				wlr_output_update_mode(&conn->output, NULL);
-			}
-			continue;
-		}
-
 		conn->crtc = &drm->crtcs[connector_match[i]];
 	}
 }
@@ -1200,7 +1194,7 @@ static struct wlr_drm_connector *create_drm_connector(struct wlr_drm_backend *dr
 
 	wlr_conn->crtc = connector_get_current_crtc(wlr_conn, drm_conn);
 
-	wl_list_insert(drm->outputs.prev, &wlr_conn->link);
+	wl_list_insert(drm->connectors.prev, &wlr_conn->link);
 	return wlr_conn;
 }
 
@@ -1364,7 +1358,7 @@ void scan_drm_connectors(struct wlr_drm_backend *drm,
 		return;
 	}
 
-	size_t seen_len = wl_list_length(&drm->outputs);
+	size_t seen_len = wl_list_length(&drm->connectors);
 	// +1 so length can never be 0, which is undefined behaviour.
 	// Last element isn't used.
 	bool seen[seen_len + 1];
@@ -1377,7 +1371,7 @@ void scan_drm_connectors(struct wlr_drm_backend *drm,
 
 		ssize_t index = -1;
 		struct wlr_drm_connector *c, *wlr_conn = NULL;
-		wl_list_for_each(c, &drm->outputs, link) {
+		wl_list_for_each(c, &drm->connectors, link) {
 			index++;
 			if (c->id == conn_id) {
 				wlr_conn = c;
@@ -1448,8 +1442,8 @@ void scan_drm_connectors(struct wlr_drm_backend *drm,
 	// Iterate in reverse order because we'll remove items from the list and
 	// still want indices to remain correct.
 	struct wlr_drm_connector *conn, *tmp_conn;
-	size_t index = wl_list_length(&drm->outputs);
+	size_t index = wl_list_length(&drm->connectors);
-	wl_list_for_each_reverse_safe(conn, tmp_conn, &drm->outputs, link) {
+	wl_list_for_each_reverse_safe(conn, tmp_conn, &drm->connectors, link) {
 		index--;
 		if (index >= seen_len || seen[index]) {
 			continue;
@@ -1478,7 +1472,7 @@ void scan_drm_leases(struct wlr_drm_backend *drm) {
 	}
 
 	struct wlr_drm_connector *conn;
-	wl_list_for_each(conn, &drm->outputs, link) {
+	wl_list_for_each(conn, &drm->connectors, link) {
 		if (conn->lease == NULL) {
 			continue;
 		}
@@ -1510,7 +1504,7 @@ static void handle_page_flip(int fd, unsigned seq,
 
 	bool found = false;
 	struct wlr_drm_connector *conn;
-	wl_list_for_each(conn, &drm->outputs, link) {
+	wl_list_for_each(conn, &drm->connectors, link) {
 		if (conn->pending_page_flip_crtc == crtc_id) {
 			found = true;
 			break;
@@ -1721,7 +1715,7 @@ void drm_lease_destroy(struct wlr_drm_lease *lease) {
 	wl_signal_emit_mutable(&lease->events.destroy, NULL);
 
 	struct wlr_drm_connector *conn;
-	wl_list_for_each(conn, &drm->outputs, link) {
+	wl_list_for_each(conn, &drm->connectors, link) {
 		if (conn->lease == lease) {
 			conn->lease = NULL;
 		}

backend/drm/legacy.c

@@ -69,7 +69,7 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn,
 
 	struct wlr_drm_backend *drm = conn->backend;
 	struct wlr_output *output = &conn->output;
-	struct wlr_drm_crtc *crtc = conn->crtc;
+	struct wlr_drm_crtc *crtc = state->crtc;
 	struct wlr_drm_plane *cursor = crtc->cursor;
 
 	uint32_t fb_id = 0;

backend/drm/util.c

@@ -119,6 +119,18 @@ void parse_edid(struct wlr_drm_connector *conn, size_t len, const uint8_t *data)
 	}
 }
 
+const char *drm_connector_status_str(drmModeConnection status) {
+	switch (status) {
+	case DRM_MODE_CONNECTED:
+		return "connected";
+	case DRM_MODE_DISCONNECTED:
+		return "disconnected";
+	case DRM_MODE_UNKNOWNCONNECTION:
+		return "unknown";
+	}
+	return "<unsupported>";
+}
+
 static bool is_taken(size_t n, const uint32_t arr[static n], uint32_t key) {
 	for (size_t i = 0; i < n; ++i) {
 		if (arr[i] == key) {

include/backend/drm/drm.h

@@ -75,7 +75,7 @@ struct wlr_drm_backend {
 	struct wl_listener dev_remove;
 
 	struct wl_list fbs; // wlr_drm_fb.link
-	struct wl_list outputs;
+	struct wl_list connectors; // wlr_drm_connector.link
 
 	/* Only initialized on multi-GPU setups */
 	struct wlr_drm_renderer mgpu_renderer;
@@ -93,10 +93,12 @@ struct wlr_drm_mode {
 };
 
 struct wlr_drm_connector_state {
+	struct wlr_drm_connector *conn;
 	const struct wlr_output_state *base;
 	bool modeset;
 	bool active;
 	drmModeModeInfo mode;
+	struct wlr_drm_crtc *crtc;
 	struct wlr_drm_fb *primary_fb;
 };
 
@@ -122,7 +124,7 @@ struct wlr_drm_connector {
 	/* Buffer to be submitted to the kernel on the next page-flip */
 	struct wlr_drm_fb *cursor_pending_fb;
 
-	struct wl_list link;
+	struct wl_list link; // wlr_drm_backend.connectors
 
 	/* CRTC ID if a page-flip is pending, zero otherwise.
 	 *

include/backend/drm/util.h

@@ -14,6 +14,7 @@ enum wlr_output_mode_aspect_ratio get_picture_aspect_ratio(const drmModeModeInfo
 const char *get_pnp_manufacturer(uint16_t code);
 // Populates the make/model/phys_{width,height} of output from the edid data
 void parse_edid(struct wlr_drm_connector *conn, size_t len, const uint8_t *data);
+const char *drm_connector_status_str(drmModeConnection status);
 
 // Part of match_obj
 enum {