From 175c20d95487c8997b2019b3a53db718b8865157 Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Sun, 3 Aug 2025 13:00:55 +0100 Subject: [PATCH] Revert removal of automatic drm reset on VT switch ...on the basis that it reduces user-space breakage with respect to layer-shell clients disappearing on suspend/resume and on VT change. Revert the following two commits: - 1edd5e2 "backend/drm: Remove reset from interface" - 0f255b4 "backend/drm: Remove automatic reset on VT switch" This it not intended to be reverted on the `master` branch. Instead, the next wlroots release is anticipated to handle the situation differently, possibly by splitting some objects and leaving output-related wl_globals which have been announced to clients. See discussions on [issue-3944]. [issue-3944]: https://gitlab.freedesktop.org/wlroots/wlroots/-/issues/3944#note_3030632 Background: With [MR-4878] on wlroots-0.19.0, the DRM backend destroys/recreates outputs on VT switch and in some cases on suspend/resume too. The reason for this change was that (i) the KMS state is undefined when a VT is switched away; and (ii) the previous outputs had issues with restoration, particularly when the output configuration had changed whilst switched away. This change causes two issues for users: - Some layer-shell clients do not re-appear on output re-connection, or may appear on a different output. Whilst this has always been the case, it will now also happen in said situations. It is technically possible for layer-shell clients to deal with this more thoughtfully by handling the new-output and surface-destroy signals to achieve desired behaviours, but such changes take time and meanwhile Desktop Environment teams and other users consider this a serious regression. - Some Gtk clients issue critical warnings on some compositors as they assume that at least one output is always available. This will be fixed in `Gtk-3.24.50` and is believed to be a harmless warning, [MR-4878]: https://gitlab.freedesktop.org/wlroots/wlroots/-/merge_requests/4878 Testing: 1. This reversion has been tested with [conky] and [GlassyClock]. With the reversion applied, the clients remain visible after VT switching. 2. It was also tested with labwc on tty1; and sway on tty2. No adverse affects were observed when changing output scales on either compositor. [conky]: https://github.com/brndnmtthws/conky [GlassyClock]: https://github.com/tsujan/GlassyClock --- backend/drm/atomic.c | 27 ++++++++++ backend/drm/backend.c | 9 +--- backend/drm/drm.c | 102 ++++++++++++++++++++++++++++++++++++ backend/drm/legacy.c | 14 +++++ backend/drm/libliftoff.c | 1 + include/backend/drm/drm.h | 1 + include/backend/drm/iface.h | 2 + 7 files changed, 148 insertions(+), 8 deletions(-) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index 037c3c23a..c1e5b8026 100644 --- a/backend/drm/atomic.c +++ b/backend/drm/atomic.c @@ -525,6 +525,33 @@ out: return ok; } +bool drm_atomic_reset(struct wlr_drm_backend *drm) { + struct atomic atom; + atomic_begin(&atom); + + for (size_t i = 0; i < drm->num_crtcs; i++) { + struct wlr_drm_crtc *crtc = &drm->crtcs[i]; + atomic_add(&atom, crtc->id, crtc->props.mode_id, 0); + atomic_add(&atom, crtc->id, crtc->props.active, 0); + } + + struct wlr_drm_connector *conn; + wl_list_for_each(conn, &drm->connectors, link) { + atomic_add(&atom, conn->id, conn->props.crtc_id, 0); + } + + for (size_t i = 0; i < drm->num_planes; i++) { + plane_disable(&atom, &drm->planes[i]); + } + + uint32_t flags = DRM_MODE_ATOMIC_ALLOW_MODESET; + bool ok = atomic_commit(&atom, drm, NULL, NULL, flags); + atomic_finish(&atom); + + return ok; +} + const struct wlr_drm_interface atomic_iface = { .commit = atomic_device_commit, + .reset = drm_atomic_reset, }; diff --git a/backend/drm/backend.c b/backend/drm/backend.c index 5a545b192..3569885f9 100644 --- a/backend/drm/backend.c +++ b/backend/drm/backend.c @@ -112,18 +112,11 @@ static void handle_session_active(struct wl_listener *listener, void *data) { wlr_log(WLR_INFO, "DRM FD %s", session->active ? "resumed" : "paused"); if (!session->active) { - // Disconnect any active connectors so that the client will modeset and - // rerender when the session is activated again. - struct wlr_drm_connector *conn; - wl_list_for_each(conn, &drm->connectors, link) { - if (conn->status == DRM_MODE_CONNECTED) { - wlr_output_destroy(&conn->output); - } - } return; } scan_drm_connectors(drm, NULL); + restore_drm_device(drm); } static void handle_dev_change(struct wl_listener *listener, void *data) { diff --git a/backend/drm/drm.c b/backend/drm/drm.c index e9e4c6db2..9fd753f90 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -1888,6 +1888,108 @@ void scan_drm_leases(struct wlr_drm_backend *drm) { drmFree(list); } +static void build_current_connector_state(struct wlr_output_state *state, + struct wlr_drm_connector *conn) { + bool enabled = conn->status != DRM_MODE_DISCONNECTED && conn->output.enabled; + + wlr_output_state_init(state); + wlr_output_state_set_enabled(state, enabled); + if (!enabled) { + return; + } + + if (conn->output.current_mode != NULL) { + wlr_output_state_set_mode(state, conn->output.current_mode); + } else { + wlr_output_state_set_custom_mode(state, + conn->output.width, conn->output.height, conn->output.refresh); + } +} + +/** + * Check whether we need to perform a full reset after a VT switch. + * + * If any connector or plane has a different CRTC, we need to perform a full + * reset to restore our mapping. We couldn't avoid a full reset even if we + * used a single KMS atomic commit to apply our state: the kernel rejects + * commits which migrate a plane from one CRTC to another without going through + * an intermediate state where the plane is disabled. + */ +static bool skip_reset_for_restore(struct wlr_drm_backend *drm) { + struct wlr_drm_connector *conn; + wl_list_for_each(conn, &drm->connectors, link) { + drmModeConnector *drm_conn = drmModeGetConnectorCurrent(drm->fd, conn->id); + if (drm_conn == NULL) { + return false; + } + struct wlr_drm_crtc *crtc = connector_get_current_crtc(conn, drm_conn); + drmModeFreeConnector(drm_conn); + + if (crtc != NULL && conn->crtc != crtc) { + return false; + } + } + + for (size_t i = 0; i < drm->num_planes; i++) { + struct wlr_drm_plane *plane = &drm->planes[i]; + + drmModePlane *drm_plane = drmModeGetPlane(drm->fd, plane->id); + if (drm_plane == NULL) { + return false; + } + uint32_t crtc_id = drm_plane->crtc_id; + drmModeFreePlane(drm_plane); + + struct wlr_drm_crtc *crtc = NULL; + for (size_t i = 0; i < drm->num_crtcs; i++) { + if (drm->crtcs[i].id == crtc_id) { + crtc = &drm->crtcs[i]; + break; + } + } + if (crtc == NULL) { + continue; + } + + bool ok = false; + switch (plane->type) { + case DRM_PLANE_TYPE_PRIMARY: + ok = crtc->primary == plane; + break; + case DRM_PLANE_TYPE_CURSOR: + ok = crtc->cursor == plane; + break; + } + if (!ok) { + return false; + } + } + + return true; +} + +void restore_drm_device(struct wlr_drm_backend *drm) { + // The previous DRM master leaves KMS in an undefined state. We need + // to restore our own state, but be careful to avoid invalid + // configurations. The connector/CRTC mapping may have changed, so + // first disable all CRTCs, then light up the ones we were using + // before the VT switch. + // TODO: better use the atomic API to improve restoration after a VT switch + if (!skip_reset_for_restore(drm) && !drm->iface->reset(drm)) { + wlr_log(WLR_ERROR, "Failed to reset state after VT switch"); + } + + struct wlr_drm_connector *conn; + wl_list_for_each(conn, &drm->connectors, link) { + struct wlr_output_state state; + build_current_connector_state(&state, conn); + if (!drm_connector_commit_state(conn, &state, false)) { + wlr_drm_conn_log(conn, WLR_ERROR, "Failed to restore state after VT switch"); + } + wlr_output_state_finish(&state); + } +} + bool commit_drm_device(struct wlr_drm_backend *drm, const struct wlr_backend_output_state *output_states, size_t output_states_len, bool test_only) { diff --git a/backend/drm/legacy.c b/backend/drm/legacy.c index d77c9d342..0e1230bfc 100644 --- a/backend/drm/legacy.c +++ b/backend/drm/legacy.c @@ -272,6 +272,20 @@ bool drm_legacy_crtc_set_gamma(struct wlr_drm_backend *drm, return true; } +static bool legacy_reset(struct wlr_drm_backend *drm) { + bool ok = true; + for (size_t i = 0; i < drm->num_crtcs; i++) { + struct wlr_drm_crtc *crtc = &drm->crtcs[i]; + if (drmModeSetCrtc(drm->fd, crtc->id, 0, 0, 0, NULL, 0, NULL) != 0) { + wlr_log_errno(WLR_ERROR, "Failed to disable CRTC %"PRIu32, + crtc->id); + ok = false; + } + } + return ok; +} + const struct wlr_drm_interface legacy_iface = { .commit = legacy_commit, + .reset = legacy_reset, }; diff --git a/backend/drm/libliftoff.c b/backend/drm/libliftoff.c index 12761afd4..042de940b 100644 --- a/backend/drm/libliftoff.c +++ b/backend/drm/libliftoff.c @@ -507,4 +507,5 @@ const struct wlr_drm_interface liftoff_iface = { .init = init, .finish = finish, .commit = commit, + .reset = drm_atomic_reset, }; diff --git a/include/backend/drm/drm.h b/include/backend/drm/drm.h index 9409b7b10..a100aebe4 100644 --- a/include/backend/drm/drm.h +++ b/include/backend/drm/drm.h @@ -225,6 +225,7 @@ void scan_drm_connectors(struct wlr_drm_backend *state, void scan_drm_leases(struct wlr_drm_backend *drm); bool commit_drm_device(struct wlr_drm_backend *drm, const struct wlr_backend_output_state *states, size_t states_len, bool test_only); +void restore_drm_device(struct wlr_drm_backend *drm); int handle_drm_event(int fd, uint32_t mask, void *data); void destroy_drm_connector(struct wlr_drm_connector *conn); bool drm_connector_is_cursor_visible(struct wlr_drm_connector *conn); diff --git a/include/backend/drm/iface.h b/include/backend/drm/iface.h index bc9610589..1ac1095a8 100644 --- a/include/backend/drm/iface.h +++ b/include/backend/drm/iface.h @@ -22,6 +22,8 @@ struct wlr_drm_interface { bool (*commit)(struct wlr_drm_backend *drm, const struct wlr_drm_device_state *state, struct wlr_drm_page_flip *page_flip, uint32_t flags, bool test_only); + // Turn off everything + bool (*reset)(struct wlr_drm_backend *drm); }; extern const struct wlr_drm_interface atomic_iface;