From 3c74bd0c915e7ddd1b5dcc05d14f5555de10e980 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Tue, 10 Aug 2021 18:47:14 +0200 Subject: [PATCH] backend/drm: introduce wlr_drm_connector_state Previously, we were copying wlr_output_state on the stack and patching it up to be guaranteed to have a proper drmModeModeInfo stored in it (and not a custom mode). Also, we had a bunch of helpers deriving DRM-specific information from the generic wlr_output_state. Copying the wlr_output_state worked fine so far, but with output layers we'll be getting a wl_list in there. An empty wl_list stores two pointers to itself, copying it on the stack blindly results in infinite loops in wl_list_for_each. To fix this, rework our DRM backend to stop copying wlr_output_state, instead add a new struct wlr_drm_connector_state which holds both the wlr_output_state and additional DRM-specific information. --- backend/drm/atomic.c | 35 ++++---- backend/drm/drm.c | 164 ++++++++++++++++++------------------ backend/drm/legacy.c | 34 ++++---- include/backend/drm/drm.h | 13 +-- include/backend/drm/iface.h | 4 +- 5 files changed, 122 insertions(+), 128 deletions(-) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index ba493f6f..70f55aff 100644 --- a/backend/drm/atomic.c +++ b/backend/drm/atomic.c @@ -54,16 +54,14 @@ 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_connector *conn, const struct wlr_output_state *state, - uint32_t *blob_id) { - if (!drm_connector_state_active(conn, state)) { + struct wlr_drm_connector *conn, + const struct wlr_drm_connector_state *state, uint32_t *blob_id) { + if (!state->active) { *blob_id = 0; return true; } - drmModeModeInfo mode = {0}; - drm_connector_state_mode(conn, state, &mode); - if (drmModeCreatePropertyBlob(drm->fd, &mode, + if (drmModeCreatePropertyBlob(drm->fd, &state->mode, sizeof(drmModeModeInfo), blob_id)) { wlr_log_errno(WLR_ERROR, "Unable to create mode property blob"); return false; @@ -166,13 +164,14 @@ error: } static bool atomic_crtc_commit(struct wlr_drm_connector *conn, - const struct wlr_output_state *state, uint32_t flags, bool test_only) { + const struct wlr_drm_connector_state *state, uint32_t flags, + 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 = drm_connector_state_is_modeset(state); - bool active = drm_connector_state_active(conn, state); + bool modeset = state->modeset; + bool active = state->active; uint32_t mode_id = crtc->mode_id; if (modeset) { @@ -182,30 +181,30 @@ static bool atomic_crtc_commit(struct wlr_drm_connector *conn, } uint32_t gamma_lut = crtc->gamma_lut; - if (state->committed & WLR_OUTPUT_STATE_GAMMA_LUT) { + if (state->base->committed & WLR_OUTPUT_STATE_GAMMA_LUT) { // Fallback to legacy gamma interface when gamma properties are not // available (can happen on older Intel GPUs that support gamma but not // degamma). if (crtc->props.gamma_lut == 0) { if (!drm_legacy_crtc_set_gamma(drm, crtc, - state->gamma_lut_size, - state->gamma_lut)) { + state->base->gamma_lut_size, + state->base->gamma_lut)) { return false; } } else { - if (!create_gamma_lut_blob(drm, state->gamma_lut_size, - state->gamma_lut, &gamma_lut)) { + if (!create_gamma_lut_blob(drm, state->base->gamma_lut_size, + state->base->gamma_lut, &gamma_lut)) { return false; } } } uint32_t fb_damage_clips = 0; - if ((state->committed & WLR_OUTPUT_STATE_DAMAGE) && + if ((state->base->committed & WLR_OUTPUT_STATE_DAMAGE) && crtc->primary->props.fb_damage_clips != 0) { int rects_len; const pixman_box32_t *rects = pixman_region32_rectangles( - (pixman_region32_t *)&state->damage, &rects_len); + (pixman_region32_t *)&state->base->damage, &rects_len); if (drmModeCreatePropertyBlob(drm->fd, rects, sizeof(*rects) * rects_len, &fb_damage_clips) != 0) { wlr_log_errno(WLR_ERROR, "Failed to create FB_DAMAGE_CLIPS property blob"); @@ -215,9 +214,9 @@ static bool atomic_crtc_commit(struct wlr_drm_connector *conn, bool prev_vrr_enabled = output->adaptive_sync_status == WLR_OUTPUT_ADAPTIVE_SYNC_ENABLED; bool vrr_enabled = prev_vrr_enabled; - if ((state->committed & WLR_OUTPUT_STATE_ADAPTIVE_SYNC_ENABLED) && + if ((state->base->committed & WLR_OUTPUT_STATE_ADAPTIVE_SYNC_ENABLED) && drm_connector_supports_vrr(conn)) { - vrr_enabled = state->adaptive_sync_enabled; + vrr_enabled = state->base->adaptive_sync_enabled; } if (test_only) { diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 09e99eaf..13e9391f 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -329,7 +329,8 @@ static struct wlr_drm_connector *get_drm_connector_from_output( } static bool drm_crtc_commit(struct wlr_drm_connector *conn, - const struct wlr_output_state *state, uint32_t flags, bool test_only) { + const struct wlr_drm_connector_state *state, + uint32_t flags, bool test_only) { // Disallow atomic-only flags assert((flags & ~DRM_MODE_PAGE_FLIP_FLAGS) == 0); @@ -354,7 +355,7 @@ static bool drm_crtc_commit(struct wlr_drm_connector *conn, } static bool drm_crtc_page_flip(struct wlr_drm_connector *conn, - const struct wlr_output_state *state) { + const struct wlr_drm_connector_state *state) { struct wlr_drm_crtc *crtc = conn->crtc; assert(crtc != NULL); @@ -362,13 +363,13 @@ static bool drm_crtc_page_flip(struct wlr_drm_connector *conn, // page-flip, either a blocking modeset. When performing a blocking modeset // we'll wait for all queued page-flips to complete, so we don't need this // safeguard. - if (conn->pending_page_flip_crtc && !drm_connector_state_is_modeset(state)) { + if (conn->pending_page_flip_crtc && !state->modeset) { wlr_drm_conn_log(conn, WLR_ERROR, "Failed to page-flip output: " "a page-flip is already pending"); return false; } - assert(drm_connector_state_active(conn, state)); + assert(state->active); assert(plane_get_next_fb(crtc->primary)); if (!drm_crtc_commit(conn, state, DRM_MODE_PAGE_FLIP_EVENT, false)) { return false; @@ -384,6 +385,36 @@ static bool drm_crtc_page_flip(struct wlr_drm_connector *conn, return true; } +static void drm_connector_state_init(struct wlr_drm_connector_state *state, + struct wlr_drm_connector *conn, + const struct wlr_output_state *base) { + state->base = base; + state->modeset = base->committed & + (WLR_OUTPUT_STATE_ENABLED | WLR_OUTPUT_STATE_MODE); + state->active = (base->committed & WLR_OUTPUT_STATE_ENABLED) ? + base->enabled : conn->output.enabled; + + if (base->committed & WLR_OUTPUT_STATE_MODE) { + switch (base->mode_type) { + case WLR_OUTPUT_STATE_MODE_FIXED:; + struct wlr_drm_mode *mode = (struct wlr_drm_mode *)base->mode; + state->mode = mode->drm_mode; + break; + case WLR_OUTPUT_STATE_MODE_CUSTOM: + generate_cvt_mode(&state->mode, base->custom_mode.width, + base->custom_mode.height, + (float)base->custom_mode.refresh / 1000, false, false); + state->mode.type = DRM_MODE_TYPE_USERDEF; + break; + } + } else if (state->active) { + struct wlr_drm_mode *mode = + (struct wlr_drm_mode *)conn->output.current_mode; + assert(mode != NULL); + state->mode = mode->drm_mode; + } +} + static bool drm_connector_set_pending_fb(struct wlr_drm_connector *conn, const struct wlr_output_state *state) { struct wlr_drm_backend *drm = conn->backend; @@ -459,7 +490,10 @@ static bool drm_connector_test(struct wlr_output *output) { } } - if (drm_connector_state_active(conn, &output->pending)) { + struct wlr_drm_connector_state pending = {0}; + drm_connector_state_init(&pending, conn, &output->pending); + + if (pending.active) { if ((output->pending.committed & (WLR_OUTPUT_STATE_ENABLED | WLR_OUTPUT_STATE_MODE)) && !(output->pending.committed & WLR_OUTPUT_STATE_BUFFER)) { @@ -488,12 +522,12 @@ static bool drm_connector_test(struct wlr_output *output) { } if (output->pending.committed & WLR_OUTPUT_STATE_BUFFER) { - if (!drm_connector_set_pending_fb(conn, &output->pending)) { + if (!drm_connector_set_pending_fb(conn, pending.base)) { return false; } } - return drm_crtc_commit(conn, &output->pending, 0, true); + return drm_crtc_commit(conn, &pending, 0, true); } bool drm_connector_supports_vrr(struct wlr_drm_connector *conn) { @@ -523,18 +557,20 @@ bool drm_connector_supports_vrr(struct wlr_drm_connector *conn) { } static bool drm_connector_set_mode(struct wlr_drm_connector *conn, - const struct wlr_output_state *state); + const struct wlr_drm_connector_state *state); bool drm_connector_commit_state(struct wlr_drm_connector *conn, - const struct wlr_output_state *pending) { + const struct wlr_output_state *base) { struct wlr_drm_backend *drm = conn->backend; - struct wlr_output_state state = *pending; if (!drm->session->active) { return false; } - if (drm_connector_state_active(conn, &state)) { + struct wlr_drm_connector_state pending = {0}; + drm_connector_state_init(&pending, conn, base); + + if (pending.active) { if (!drm_connector_alloc_crtc(conn)) { wlr_drm_conn_log(conn, WLR_ERROR, "No CRTC available for this connector"); @@ -542,37 +578,25 @@ bool drm_connector_commit_state(struct wlr_drm_connector *conn, } } - if (state.committed & WLR_OUTPUT_STATE_BUFFER) { - if (!drm_connector_set_pending_fb(conn, &state)) { + if (pending.base->committed & WLR_OUTPUT_STATE_BUFFER) { + if (!drm_connector_set_pending_fb(conn, pending.base)) { return false; } } - if (state.committed & (WLR_OUTPUT_STATE_MODE | WLR_OUTPUT_STATE_ENABLED)) { - if ((state.committed & WLR_OUTPUT_STATE_MODE) && - state.mode_type == WLR_OUTPUT_STATE_MODE_CUSTOM) { - drmModeModeInfo mode = {0}; - drm_connector_state_mode(conn, &state, &mode); - - state.mode_type = WLR_OUTPUT_STATE_MODE_FIXED; - state.mode = wlr_drm_connector_add_mode(&conn->output, &mode); - if (state.mode == NULL) { - return false; - } - } - - if (!drm_connector_set_mode(conn, &state)) { + if (pending.modeset) { + if (!drm_connector_set_mode(conn, &pending)) { return false; } - } else if (state.committed & WLR_OUTPUT_STATE_BUFFER) { - if (!drm_crtc_page_flip(conn, &state)) { + } else if (pending.base->committed & WLR_OUTPUT_STATE_BUFFER) { + if (!drm_crtc_page_flip(conn, &pending)) { return false; } - } else if (state.committed & (WLR_OUTPUT_STATE_ADAPTIVE_SYNC_ENABLED | + } else if (pending.base->committed & (WLR_OUTPUT_STATE_ADAPTIVE_SYNC_ENABLED | WLR_OUTPUT_STATE_GAMMA_LUT)) { assert(conn->crtc != NULL); // TODO: maybe request a page-flip event here? - if (!drm_crtc_commit(conn, &state, 0, false)) { + if (!drm_crtc_commit(conn, &pending, 0, false)) { return false; } } @@ -629,7 +653,7 @@ struct wlr_drm_fb *plane_get_next_fb(struct wlr_drm_plane *plane) { } static bool drm_connector_init_renderer(struct wlr_drm_connector *conn, - const struct wlr_output_state *state) { + const struct wlr_drm_connector_state *state) { struct wlr_drm_backend *drm = conn->backend; if (conn->status != WLR_DRM_CONN_CONNECTED && @@ -642,12 +666,9 @@ static bool drm_connector_init_renderer(struct wlr_drm_connector *conn, if (drm->parent) { wlr_drm_conn_log(conn, WLR_DEBUG, "Initializing multi-GPU renderer"); - drmModeModeInfo mode = {0}; - drm_connector_state_mode(conn, state, &mode); - struct wlr_drm_plane *plane = conn->crtc->primary; - int width = mode.hdisplay; - int height = mode.vdisplay; + int width = state->mode.hdisplay; + int height = state->mode.vdisplay; struct wlr_drm_format *format = drm_plane_pick_render_format(plane, &drm->mgpu_renderer); @@ -705,14 +726,23 @@ static bool drm_connector_alloc_crtc(struct wlr_drm_connector *conn) { } static bool drm_connector_set_mode(struct wlr_drm_connector *conn, - const struct wlr_output_state *state) { + const struct wlr_drm_connector_state *state) { struct wlr_drm_backend *drm = conn->backend; struct wlr_output_mode *wlr_mode = NULL; - if (drm_connector_state_active(conn, state)) { - if (state->committed & WLR_OUTPUT_STATE_MODE) { - assert(state->mode_type == WLR_OUTPUT_STATE_MODE_FIXED); - wlr_mode = state->mode; + if (state->active) { + if (state->base->committed & WLR_OUTPUT_STATE_MODE) { + switch (state->base->mode_type) { + case WLR_OUTPUT_STATE_MODE_FIXED: + wlr_mode = state->base->mode; + break; + case WLR_OUTPUT_STATE_MODE_CUSTOM: + wlr_mode = wlr_drm_connector_add_mode(&conn->output, &state->mode); + if (wlr_mode == NULL) { + return false; + } + break; + } } else { wlr_mode = conn->output.current_mode; } @@ -1017,44 +1047,6 @@ uint32_t wlr_drm_connector_get_id(struct wlr_output *output) { return conn->id; } -bool drm_connector_state_is_modeset(const struct wlr_output_state *state) { - return state->committed & - (WLR_OUTPUT_STATE_ENABLED | WLR_OUTPUT_STATE_MODE); -} - -bool drm_connector_state_active(struct wlr_drm_connector *conn, - const struct wlr_output_state *state) { - if (state->committed & WLR_OUTPUT_STATE_ENABLED) { - return state->enabled; - } - return conn->output.enabled; -} - -void drm_connector_state_mode(struct wlr_drm_connector *conn, - const struct wlr_output_state *state, drmModeModeInfo *out) { - assert(drm_connector_state_active(conn, state)); - - struct wlr_output_mode *wlr_mode = conn->output.current_mode; - if (state->committed & WLR_OUTPUT_STATE_MODE) { - switch (state->mode_type) { - case WLR_OUTPUT_STATE_MODE_FIXED: - wlr_mode = state->mode; - break; - case WLR_OUTPUT_STATE_MODE_CUSTOM:; - drmModeModeInfo mode = {0}; - generate_cvt_mode(&mode, state->custom_mode.width, - state->custom_mode.height, - (float)state->custom_mode.refresh / 1000, true, false); - mode.type = DRM_MODE_TYPE_USERDEF; - memcpy(out, &mode, sizeof(drmModeModeInfo)); - return; - } - } - - struct wlr_drm_mode *mode = (struct wlr_drm_mode *)wlr_mode; - memcpy(out, &mode->drm_mode, sizeof(drmModeModeInfo)); -} - static const int32_t subpixel_map[] = { [DRM_MODE_SUBPIXEL_UNKNOWN] = WL_OUTPUT_SUBPIXEL_UNKNOWN, [DRM_MODE_SUBPIXEL_HORIZONTAL_RGB] = WL_OUTPUT_SUBPIXEL_HORIZONTAL_RGB, @@ -1073,11 +1065,13 @@ static void dealloc_crtc(struct wlr_drm_connector *conn) { wlr_drm_conn_log(conn, WLR_DEBUG, "De-allocating CRTC %zu", conn->crtc - drm->crtcs); - struct wlr_output_state state = { + struct wlr_output_state output_state = { .committed = WLR_OUTPUT_STATE_ENABLED, .enabled = false, }; - if (!drm_crtc_commit(conn, &state, 0, false)) { + struct wlr_drm_connector_state conn_state = {0}; + drm_connector_state_init(&conn_state, conn, &output_state); + if (!drm_crtc_commit(conn, &conn_state, 0, false)) { // On GPU unplug, disabling the CRTC can fail with EPERM wlr_drm_conn_log(conn, WLR_ERROR, "Failed to disable CRTC %"PRIu32, conn->crtc->id); @@ -1201,11 +1195,13 @@ static void realloc_crtcs(struct wlr_drm_backend *drm) { continue; } - struct wlr_output_state state = { + struct wlr_output_state output_state = { .committed = WLR_OUTPUT_STATE_ENABLED, .enabled = true, }; - if (!drm_connector_init_renderer(conn, &state)) { + struct wlr_drm_connector_state conn_state = {0}; + drm_connector_state_init(&conn_state, conn, &output_state); + if (!drm_connector_init_renderer(conn, &conn_state)) { wlr_drm_conn_log(conn, WLR_ERROR, "Failed to initialize renderer"); wlr_output_update_enabled(&conn->output, false); continue; diff --git a/backend/drm/legacy.c b/backend/drm/legacy.c index 5a9fbe24..37d8147a 100644 --- a/backend/drm/legacy.c +++ b/backend/drm/legacy.c @@ -34,11 +34,10 @@ static bool legacy_fb_props_match(struct wlr_drm_fb *fb1, } static bool legacy_crtc_test(struct wlr_drm_connector *conn, - const struct wlr_output_state *state) { + const struct wlr_drm_connector_state *state) { struct wlr_drm_crtc *crtc = conn->crtc; - if ((state->committed & WLR_OUTPUT_STATE_BUFFER) && - !drm_connector_state_is_modeset(state)) { + if ((state->base->committed & WLR_OUTPUT_STATE_BUFFER) && !state->modeset) { struct wlr_drm_fb *pending_fb = crtc->primary->pending_fb; struct wlr_drm_fb *prev_fb = crtc->primary->queued_fb; @@ -59,7 +58,8 @@ static bool legacy_crtc_test(struct wlr_drm_connector *conn, } static bool legacy_crtc_commit(struct wlr_drm_connector *conn, - const struct wlr_output_state *state, uint32_t flags, bool test_only) { + const struct wlr_drm_connector_state *state, + uint32_t flags, bool test_only) { if (!legacy_crtc_test(conn, state)) { return false; } @@ -72,10 +72,8 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn, struct wlr_drm_crtc *crtc = conn->crtc; struct wlr_drm_plane *cursor = crtc->cursor; - bool active = drm_connector_state_active(conn, state); - uint32_t fb_id = 0; - if (active) { + if (state->active) { struct wlr_drm_fb *fb = plane_get_next_fb(crtc->primary); if (fb == NULL) { wlr_log(WLR_ERROR, "%s: failed to acquire primary FB", @@ -85,19 +83,17 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn, fb_id = fb->id; } - if (drm_connector_state_is_modeset(state)) { + if (state->modeset) { uint32_t *conns = NULL; size_t conns_len = 0; drmModeModeInfo *mode = NULL; - drmModeModeInfo mode_info = {0}; - if (active) { + if (state->active) { conns = &conn->id; conns_len = 1; - drm_connector_state_mode(conn, state, &mode_info); - mode = &mode_info; + mode = (drmModeModeInfo *)&state->mode; } - uint32_t dpms = active ? DRM_MODE_DPMS_ON : DRM_MODE_DPMS_OFF; + uint32_t dpms = state->active ? DRM_MODE_DPMS_ON : DRM_MODE_DPMS_OFF; if (drmModeConnectorSetProperty(drm->fd, conn->id, conn->props.dpms, dpms) != 0) { wlr_drm_conn_log_errno(conn, WLR_ERROR, @@ -112,27 +108,27 @@ static bool legacy_crtc_commit(struct wlr_drm_connector *conn, } } - if (state->committed & WLR_OUTPUT_STATE_GAMMA_LUT) { + if (state->base->committed & WLR_OUTPUT_STATE_GAMMA_LUT) { if (!drm_legacy_crtc_set_gamma(drm, crtc, - state->gamma_lut_size, state->gamma_lut)) { + state->base->gamma_lut_size, state->base->gamma_lut)) { return false; } } - if ((state->committed & WLR_OUTPUT_STATE_ADAPTIVE_SYNC_ENABLED) && + if ((state->base->committed & WLR_OUTPUT_STATE_ADAPTIVE_SYNC_ENABLED) && drm_connector_supports_vrr(conn)) { if (drmModeObjectSetProperty(drm->fd, crtc->id, DRM_MODE_OBJECT_CRTC, crtc->props.vrr_enabled, - state->adaptive_sync_enabled) != 0) { + state->base->adaptive_sync_enabled) != 0) { wlr_drm_conn_log_errno(conn, WLR_ERROR, "drmModeObjectSetProperty(VRR_ENABLED) failed"); return false; } - output->adaptive_sync_status = state->adaptive_sync_enabled ? + output->adaptive_sync_status = state->base->adaptive_sync_enabled ? WLR_OUTPUT_ADAPTIVE_SYNC_ENABLED : WLR_OUTPUT_ADAPTIVE_SYNC_DISABLED; wlr_drm_conn_log(conn, WLR_DEBUG, "VRR %s", - state->adaptive_sync_enabled ? "enabled" : "disabled"); + state->base->adaptive_sync_enabled ? "enabled" : "disabled"); } if (cursor != NULL && drm_connector_is_cursor_visible(conn)) { diff --git a/include/backend/drm/drm.h b/include/backend/drm/drm.h index 87160ac6..69e563a9 100644 --- a/include/backend/drm/drm.h +++ b/include/backend/drm/drm.h @@ -104,6 +104,13 @@ struct wlr_drm_mode { drmModeModeInfo drm_mode; }; +struct wlr_drm_connector_state { + const struct wlr_output_state *base; + bool modeset; + bool active; + drmModeModeInfo mode; +}; + struct wlr_drm_connector { struct wlr_output output; // only valid if status != DISCONNECTED @@ -153,12 +160,6 @@ size_t drm_crtc_get_gamma_lut_size(struct wlr_drm_backend *drm, struct wlr_drm_fb *plane_get_next_fb(struct wlr_drm_plane *plane); -bool drm_connector_state_is_modeset(const struct wlr_output_state *state); -bool drm_connector_state_active(struct wlr_drm_connector *conn, - const struct wlr_output_state *state); -void drm_connector_state_mode(struct wlr_drm_connector *conn, - const struct wlr_output_state *state, drmModeModeInfo *mode); - #define wlr_drm_conn_log(conn, verb, fmt, ...) \ wlr_log(verb, "connector %s: " fmt, conn->name, ##__VA_ARGS__) #define wlr_drm_conn_log_errno(conn, verb, fmt, ...) \ diff --git a/include/backend/drm/iface.h b/include/backend/drm/iface.h index 98f7e06c..d52bbd3d 100644 --- a/include/backend/drm/iface.h +++ b/include/backend/drm/iface.h @@ -9,12 +9,14 @@ struct wlr_drm_backend; struct wlr_drm_connector; struct wlr_drm_crtc; +struct wlr_drm_connector_state; // Used to provide atomic or legacy DRM functions struct wlr_drm_interface { // Commit all pending changes on a CRTC. bool (*crtc_commit)(struct wlr_drm_connector *conn, - const struct wlr_output_state *state, uint32_t flags, bool test_only); + const struct wlr_drm_connector_state *state, uint32_t flags, + bool test_only); }; extern const struct wlr_drm_interface atomic_iface;