From 037b21647b06957682ff8a0a1b24dcbc49e1c357 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Tue, 6 Dec 2022 17:39:23 +0100 Subject: [PATCH] backend/drm: store pending FB in state Instead of having a pending_fb field on the struct wlr_drm_plane, move it to struct wlr_drm_connector_state. That way, there's no risk having a stale pending FB around: the state doesn't survive across tests and commits. The cursor is a special case because it's disconnected from the atomic state: the wlr_backend_impl.set_cursor hook sets the cursor for the next commit. Move the field to wlr_drm_connector.cursor_pending_fb. --- backend/drm/atomic.c | 4 +- backend/drm/drm.c | 97 ++++++++++++++++++++-------------- backend/drm/legacy.c | 7 ++- backend/drm/renderer.c | 6 ++- include/backend/drm/drm.h | 4 +- include/backend/drm/renderer.h | 1 + 6 files changed, 69 insertions(+), 50 deletions(-) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index 1ca1467c..c69d6abd 100644 --- a/backend/drm/atomic.c +++ b/backend/drm/atomic.c @@ -294,8 +294,8 @@ 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, plane_get_next_fb(crtc->primary), - crtc->id, 0, 0); + set_plane_props(&atom, drm, crtc->primary, state->primary_fb, crtc->id, + 0, 0); if (crtc->primary->props.fb_damage_clips != 0) { atomic_add(&atom, crtc->primary->id, crtc->primary->props.fb_damage_clips, fb_damage_clips); diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 73920f0e..9b728bbb 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -332,12 +332,14 @@ static bool drm_crtc_commit(struct wlr_drm_connector *conn, struct wlr_drm_crtc *crtc = conn->crtc; bool ok = drm->iface->crtc_commit(conn, state, flags, test_only); if (ok && !test_only) { - drm_fb_move(&crtc->primary->queued_fb, &crtc->primary->pending_fb); + drm_fb_clear(&crtc->primary->queued_fb); + if (state->primary_fb != NULL) { + crtc->primary->queued_fb = drm_fb_lock(state->primary_fb); + } if (crtc->cursor != NULL) { drm_fb_move(&crtc->cursor->queued_fb, &conn->cursor_pending_fb); } } else { - drm_fb_clear(&crtc->primary->pending_fb); // The set_cursor() hook is a bit special: it's not really synchronized // to commit() or test(). Once set_cursor() returns true, the new // cursor is effectively committed. So don't roll it back here, or we @@ -377,19 +379,32 @@ static void drm_connector_state_init(struct wlr_drm_connector_state *state, assert(mode != NULL); state->mode = mode->drm_mode; } + + if (conn->crtc != NULL) { + struct wlr_drm_plane *primary = conn->crtc->primary; + if (primary->queued_fb != NULL) { + state->primary_fb = drm_fb_lock(primary->queued_fb); + } else if (primary->current_fb != NULL) { + state->primary_fb = drm_fb_lock(primary->current_fb); + } + } } -static bool drm_connector_set_pending_fb(struct wlr_drm_connector *conn, - const struct wlr_output_state *state) { +static void drm_connector_state_finish(struct wlr_drm_connector_state *state) { + drm_fb_clear(&state->primary_fb); +} + +static bool drm_connector_state_update_primary_fb(struct wlr_drm_connector *conn, + struct wlr_drm_connector_state *state) { struct wlr_drm_backend *drm = conn->backend; + assert(state->base->committed & WLR_OUTPUT_STATE_BUFFER); + struct wlr_drm_crtc *crtc = conn->crtc; - if (!crtc) { - return false; - } - struct wlr_drm_plane *plane = crtc->primary; + assert(crtc != NULL); - assert(state->committed & WLR_OUTPUT_STATE_BUFFER); + struct wlr_drm_plane *plane = crtc->primary; + struct wlr_buffer *source_buf = state->base->buffer; struct wlr_buffer *local_buf; if (drm->parent) { @@ -402,22 +417,22 @@ static bool drm_connector_set_pending_fb(struct wlr_drm_connector *conn, // TODO: fallback to modifier-less buffer allocation bool ok = init_drm_surface(&plane->mgpu_surf, &drm->mgpu_renderer, - state->buffer->width, state->buffer->height, format); + source_buf->width, source_buf->height, format); free(format); if (!ok) { return false; } - local_buf = drm_surface_blit(&plane->mgpu_surf, state->buffer); + local_buf = drm_surface_blit(&plane->mgpu_surf, source_buf); if (local_buf == NULL) { return false; } } else { - local_buf = wlr_buffer_lock(state->buffer); + local_buf = wlr_buffer_lock(source_buf); } - bool ok = drm_fb_import(&plane->pending_fb, drm, local_buf, - &crtc->primary->formats); + bool ok = drm_fb_import(&state->primary_fb, drm, local_buf, + &plane->formats); wlr_buffer_unlock(local_buf); if (!ok) { wlr_drm_conn_log(conn, WLR_DEBUG, @@ -459,6 +474,7 @@ static bool drm_connector_test(struct wlr_output *output, } } + bool ok = false; struct wlr_drm_connector_state pending = {0}; drm_connector_state_init(&pending, conn, state); @@ -468,41 +484,47 @@ static bool drm_connector_test(struct wlr_output *output, !(state->committed & WLR_OUTPUT_STATE_BUFFER)) { wlr_drm_conn_log(conn, WLR_DEBUG, "Can't enable an output without a buffer"); - return false; + goto out; } if (!drm_connector_alloc_crtc(conn)) { wlr_drm_conn_log(conn, WLR_DEBUG, "No CRTC available for this connector"); - return false; + goto out; } } if ((state->committed & WLR_OUTPUT_ADAPTIVE_SYNC_ENABLED) && state->adaptive_sync_enabled && !drm_connector_supports_vrr(conn)) { - return false; + goto out; } if (conn->backend->parent) { // If we're running as a secondary GPU, we can't perform an atomic // commit without blitting a buffer. - return true; + ok = true; + goto out; } if (!conn->crtc) { // If the output is disabled, we don't have a crtc even after // reallocation - return true; + ok = true; + goto out; } if (state->committed & WLR_OUTPUT_STATE_BUFFER) { - if (!drm_connector_set_pending_fb(conn, pending.base)) { - return false; + if (!drm_connector_state_update_primary_fb(conn, &pending)) { + goto out; } } - return drm_crtc_commit(conn, &pending, 0, true); + ok = drm_crtc_commit(conn, &pending, 0, true); + +out: + drm_connector_state_finish(&pending); + return ok; } bool drm_connector_supports_vrr(struct wlr_drm_connector *conn) { @@ -544,26 +566,28 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn, return true; } + bool ok = false; struct wlr_drm_connector_state pending = {0}; drm_connector_state_init(&pending, conn, base); if (!pending.active && conn->crtc == NULL) { // Disabling an already-disabled connector - return true; + ok = true; + goto out; } if (pending.active) { if (!drm_connector_alloc_crtc(conn)) { wlr_drm_conn_log(conn, WLR_ERROR, "No CRTC available for this connector"); - return false; + goto out; } } uint32_t flags = 0; if (pending.base->committed & WLR_OUTPUT_STATE_BUFFER) { - if (!drm_connector_set_pending_fb(conn, pending.base)) { - return false; + if (!drm_connector_state_update_primary_fb(conn, &pending)) { + goto out; } flags |= DRM_MODE_PAGE_FLIP_EVENT; @@ -574,7 +598,7 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn, 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"); - return false; + goto out; } } if (pending.modeset && pending.active) { @@ -591,8 +615,9 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn, } } - if (!drm_crtc_commit(conn, &pending, flags, false)) { - return false; + ok = drm_crtc_commit(conn, &pending, flags, false); + if (!ok) { + goto out; } if (pending.base->committed & WLR_OUTPUT_STATE_MODE) { @@ -617,7 +642,9 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn, conn->output.frame_pending = true; } - return true; +out: + drm_connector_state_finish(&pending); + return ok; } static bool drm_connector_commit(struct wlr_output *output, @@ -672,16 +699,6 @@ struct wlr_drm_fb *get_next_cursor_fb(struct wlr_drm_connector *conn) { return conn->crtc->cursor->current_fb; } -struct wlr_drm_fb *plane_get_next_fb(struct wlr_drm_plane *plane) { - if (plane->pending_fb) { - return plane->pending_fb; - } - if (plane->queued_fb) { - return plane->queued_fb; - } - return plane->current_fb; -} - static void realloc_crtcs(struct wlr_drm_backend *drm, struct wlr_drm_connector *want_conn); diff --git a/backend/drm/legacy.c b/backend/drm/legacy.c index 6e6b33dd..a8f618cf 100644 --- a/backend/drm/legacy.c +++ b/backend/drm/legacy.c @@ -38,7 +38,7 @@ static bool legacy_crtc_test(struct wlr_drm_connector *conn, struct wlr_drm_crtc *crtc = conn->crtc; if ((state->base->committed & WLR_OUTPUT_STATE_BUFFER) && !state->modeset) { - struct wlr_drm_fb *pending_fb = crtc->primary->pending_fb; + struct wlr_drm_fb *pending_fb = state->primary_fb; struct wlr_drm_fb *prev_fb = crtc->primary->queued_fb; if (!prev_fb) { @@ -74,13 +74,12 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn, uint32_t fb_id = 0; if (state->active) { - struct wlr_drm_fb *fb = plane_get_next_fb(crtc->primary); - if (fb == NULL) { + if (state->primary_fb == NULL) { wlr_log(WLR_ERROR, "%s: failed to acquire primary FB", conn->output.name); return false; } - fb_id = fb->id; + fb_id = state->primary_fb->id; } if (state->modeset) { diff --git a/backend/drm/renderer.c b/backend/drm/renderer.c index 50be0a96..4a9181e4 100644 --- a/backend/drm/renderer.c +++ b/backend/drm/renderer.c @@ -130,7 +130,6 @@ void drm_plane_finish_surface(struct wlr_drm_plane *plane) { return; } - drm_fb_clear(&plane->pending_fb); drm_fb_clear(&plane->queued_fb); drm_fb_clear(&plane->current_fb); @@ -194,6 +193,11 @@ void drm_fb_clear(struct wlr_drm_fb **fb_ptr) { *fb_ptr = NULL; } +struct wlr_drm_fb *drm_fb_lock(struct wlr_drm_fb *fb) { + wlr_buffer_lock(fb->wlr_buf); + return fb; +} + static void drm_fb_handle_destroy(struct wlr_addon *addon) { struct wlr_drm_fb *fb = wl_container_of(addon, fb, addon); drm_fb_destroy(fb); diff --git a/include/backend/drm/drm.h b/include/backend/drm/drm.h index 365f1ad2..fa18225c 100644 --- a/include/backend/drm/drm.h +++ b/include/backend/drm/drm.h @@ -22,8 +22,6 @@ struct wlr_drm_plane { /* Only initialized on multi-GPU setups */ struct wlr_drm_surface mgpu_surf; - /* Buffer to be submitted to the kernel on the next page-flip */ - struct wlr_drm_fb *pending_fb; /* Buffer submitted to the kernel, will be presented on next vblank */ struct wlr_drm_fb *queued_fb; /* Buffer currently displayed on screen */ @@ -99,6 +97,7 @@ struct wlr_drm_connector_state { bool modeset; bool active; drmModeModeInfo mode; + struct wlr_drm_fb *primary_fb; }; struct wlr_drm_connector { @@ -153,7 +152,6 @@ size_t drm_crtc_get_gamma_lut_size(struct wlr_drm_backend *drm, struct wlr_drm_crtc *crtc); void drm_lease_destroy(struct wlr_drm_lease *lease); -struct wlr_drm_fb *plane_get_next_fb(struct wlr_drm_plane *plane); struct wlr_drm_fb *get_next_cursor_fb(struct wlr_drm_connector *conn); #define wlr_drm_conn_log(conn, verb, fmt, ...) \ diff --git a/include/backend/drm/renderer.h b/include/backend/drm/renderer.h index 77ed4127..5851d228 100644 --- a/include/backend/drm/renderer.h +++ b/include/backend/drm/renderer.h @@ -45,6 +45,7 @@ void drm_fb_destroy(struct wlr_drm_fb *fb); void drm_fb_clear(struct wlr_drm_fb **fb); void drm_fb_move(struct wlr_drm_fb **new, struct wlr_drm_fb **old); +struct wlr_drm_fb *drm_fb_lock(struct wlr_drm_fb *fb); struct wlr_buffer *drm_surface_blit(struct wlr_drm_surface *surf, struct wlr_buffer *buffer);