From 15d8f1806e9debd11a1c306e76c97b535e708766 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Mon, 18 May 2020 14:27:42 +0200 Subject: [PATCH] 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 | 18 +++++++++--------- backend/drm/drm.c | 23 ++++++++++++++++------- backend/drm/legacy.c | 12 +++++++----- include/backend/drm/drm.h | 10 +++++----- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index 3c8f5e62..22d8e69b 100644 --- a/backend/drm/atomic.c +++ b/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 (modeset) { + if (crtc->pending_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); - if (modeset && crtc->active && conn->props.link_status != 0) { + crtc->pending.active ? crtc->id : 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); - if (crtc->active) { + atomic_add(&atom, crtc->id, crtc->props.active, crtc->pending.active); + if (crtc->pending.active) { if (crtc->props.gamma_lut != 0) { atomic_add(&atom, crtc->id, crtc->props.gamma_lut, gamma_lut); } diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 62a94ae0..b13d048d 100644 --- a/backend/drm/drm.c +++ b/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; - memcpy(&crtc->mode, &mode->drm_mode, sizeof(drmModeModeInfo)); - crtc->active = true; + crtc->pending_modeset = true; + crtc->pending.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 |= WLR_DRM_CRTC_MODE; + conn->crtc->pending_modeset = true; + conn->crtc->pending.active = false; if (!drm_crtc_commit(conn, 0)) { return false; } diff --git a/backend/drm/legacy.c b/backend/drm/legacy.c index d708e2b3..61309475 100644 --- a/backend/drm/legacy.c +++ b/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; diff --git a/include/backend/drm/drm.h b/include/backend/drm/drm.h index dc34c681..7755da59 100644 --- a/include/backend/drm/drm.h +++ b/include/backend/drm/drm.h @@ -42,16 +42,16 @@ struct wlr_drm_plane { union wlr_drm_plane_props props; }; -enum wlr_drm_crtc_field { - WLR_DRM_CRTC_MODE = 1 << 0, +struct wlr_drm_crtc_state { + 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; - drmModeModeInfo mode; + bool pending_modeset; + struct wlr_drm_crtc_state pending, current; // Atomic modesetting only uint32_t mode_id;