From 6977f3a843c7a9fc7b7cd4a5d75232c583132cf8 Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Thu, 2 Apr 2020 14:12:26 +0200 Subject: [PATCH] output: check buffer in wlr_output_test Check that buffer can be scanned out in wlr_output_test instead of wlr_output_attach_buffer. This allows the backend to have access to the whole pending state when performing the check. This brings the wlr_output API more in line with the KMS API. This removes the need for wlr_output_attach_buffer to return a value, and for wlr_output_impl.attach_buffer. --- backend/drm/drm.c | 121 +++++++++++++++------------- backend/wayland/output.c | 55 ++++++------- include/backend/drm/drm.h | 2 - include/backend/wayland.h | 1 - include/wlr/interfaces/wlr_output.h | 1 - include/wlr/types/wlr_output.h | 5 +- types/wlr_output.c | 52 ++++++------ 7 files changed, 122 insertions(+), 115 deletions(-) diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 53d49d17..47423dcb 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -334,7 +334,67 @@ static bool drm_connector_attach_render(struct wlr_output *output, return make_drm_surface_current(&conn->crtc->primary->surf, buffer_age); } +static uint32_t strip_alpha_channel(uint32_t format) { + switch (format) { + case DRM_FORMAT_ARGB8888: + return DRM_FORMAT_XRGB8888; + default: + return DRM_FORMAT_INVALID; + } +} + +static bool test_buffer(struct wlr_drm_connector *conn, + struct wlr_buffer *wlr_buffer) { + struct wlr_output *output = &conn->output; + struct wlr_drm_backend *drm = get_drm_backend_from_backend(output->backend); + + if (!drm->session->active) { + return false; + } + + struct wlr_drm_crtc *crtc = conn->crtc; + if (!crtc) { + return false; + } + + struct wlr_dmabuf_attributes attribs; + if (!wlr_buffer_get_dmabuf(wlr_buffer, &attribs)) { + return false; + } + + if (attribs.flags != 0) { + return false; + } + if (attribs.width != output->width || attribs.height != output->height) { + return false; + } + + if (!wlr_drm_format_set_has(&crtc->primary->formats, + attribs.format, attribs.modifier)) { + // The format isn't supported by the plane. Try stripping the alpha + // channel, if any. + uint32_t format = strip_alpha_channel(attribs.format); + if (format != DRM_FORMAT_INVALID && wlr_drm_format_set_has( + &crtc->primary->formats, format, attribs.modifier)) { + attribs.format = format; + } else { + return false; + } + } + + return true; +} + static bool drm_connector_test(struct wlr_output *output) { + struct wlr_drm_connector *conn = get_drm_connector_from_output(output); + + if ((output->pending.committed & WLR_OUTPUT_STATE_BUFFER) && + output->pending.buffer_type == WLR_OUTPUT_STATE_BUFFER_SCANOUT) { + if (!test_buffer(conn, output->pending.buffer)) { + return false; + } + } + return true; } @@ -377,8 +437,13 @@ static bool drm_connector_commit_buffer(struct wlr_output *output) { return false; } break; - case WLR_OUTPUT_STATE_BUFFER_SCANOUT: - bo = import_gbm_bo(&drm->renderer, &conn->pending_dmabuf); + case WLR_OUTPUT_STATE_BUFFER_SCANOUT:; + struct wlr_dmabuf_attributes attribs; + if (!wlr_buffer_get_dmabuf(output->pending.buffer, &attribs)) { + return false; + } + + bo = import_gbm_bo(&drm->renderer, &attribs); if (bo == NULL) { wlr_log(WLR_ERROR, "import_gbm_bo failed"); return false; @@ -1011,57 +1076,6 @@ static bool drm_connector_move_cursor(struct wlr_output *output, return ok; } -static uint32_t strip_alpha_channel(uint32_t format) { - switch (format) { - case DRM_FORMAT_ARGB8888: - return DRM_FORMAT_XRGB8888; - default: - return DRM_FORMAT_INVALID; - } -} - -static bool drm_connector_attach_buffer(struct wlr_output *output, - struct wlr_buffer *buffer) { - struct wlr_drm_connector *conn = get_drm_connector_from_output(output); - struct wlr_drm_backend *drm = get_drm_backend_from_backend(output->backend); - if (!drm->session->active) { - return false; - } - - struct wlr_drm_crtc *crtc = conn->crtc; - if (!crtc) { - return false; - } - - struct wlr_dmabuf_attributes attribs; - if (!wlr_buffer_get_dmabuf(buffer, &attribs)) { - return false; - } - - if (attribs.flags != 0) { - return false; - } - if (attribs.width != output->width || attribs.height != output->height) { - return false; - } - - if (!wlr_drm_format_set_has(&crtc->primary->formats, - attribs.format, attribs.modifier)) { - // The format isn't supported by the plane. Try stripping the alpha - // channel, if any. - uint32_t format = strip_alpha_channel(attribs.format); - if (format != DRM_FORMAT_INVALID && wlr_drm_format_set_has( - &crtc->primary->formats, format, attribs.modifier)) { - attribs.format = format; - } else { - return false; - } - } - - memcpy(&conn->pending_dmabuf, &attribs, sizeof(attribs)); - return true; -} - static void drm_connector_destroy(struct wlr_output *output) { struct wlr_drm_connector *conn = get_drm_connector_from_output(output); drm_connector_cleanup(conn); @@ -1081,7 +1095,6 @@ static const struct wlr_output_impl output_impl = { .set_gamma = set_drm_connector_gamma, .get_gamma_size = drm_connector_get_gamma_size, .export_dmabuf = drm_connector_export_dmabuf, - .attach_buffer = drm_connector_attach_buffer, }; bool wlr_output_is_drm(struct wlr_output *output) { diff --git a/backend/wayland/output.c b/backend/wayland/output.c index 1a1835e1..59804dcf 100644 --- a/backend/wayland/output.c +++ b/backend/wayland/output.c @@ -127,20 +127,35 @@ static const struct wl_buffer_listener buffer_listener = { .release = buffer_handle_release, }; -static struct wlr_wl_buffer *create_wl_buffer(struct wlr_wl_backend *wl, +static bool test_buffer(struct wlr_wl_backend *wl, struct wlr_buffer *wlr_buffer, int required_width, int required_height) { struct wlr_dmabuf_attributes attribs; if (!wlr_buffer_get_dmabuf(wlr_buffer, &attribs)) { - return NULL; + return false; } if (attribs.width != required_width || attribs.height != required_height) { - return NULL; + return false; } if (!wlr_drm_format_set_has(&wl->linux_dmabuf_v1_formats, attribs.format, attribs.modifier)) { + return false; + } + + return true; +} + +static struct wlr_wl_buffer *create_wl_buffer(struct wlr_wl_backend *wl, + struct wlr_buffer *wlr_buffer, + int required_width, int required_height) { + if (!test_buffer(wl, wlr_buffer, required_width, required_height)) { + return NULL; + } + + struct wlr_dmabuf_attributes attribs; + if (!wlr_buffer_get_dmabuf(wlr_buffer, &attribs)) { return NULL; } @@ -180,23 +195,6 @@ static struct wlr_wl_buffer *create_wl_buffer(struct wlr_wl_backend *wl, return buffer; } -static bool output_attach_buffer(struct wlr_output *wlr_output, - struct wlr_buffer *wlr_buffer) { - struct wlr_wl_output *output = - get_wl_output_from_output(wlr_output); - struct wlr_wl_backend *wl = output->backend; - - struct wlr_wl_buffer *buffer = create_wl_buffer(wl, wlr_buffer, - wlr_output->width, wlr_output->height); - if (buffer == NULL) { - return false; - } - - destroy_wl_buffer(output->pending_buffer); - output->pending_buffer = buffer; - return true; -} - static bool output_test(struct wlr_output *wlr_output) { if (wlr_output->pending.committed & WLR_OUTPUT_STATE_ENABLED) { wlr_log(WLR_DEBUG, "Cannot disable a Wayland output"); @@ -254,10 +252,14 @@ static bool output_commit(struct wlr_output *wlr_output) { return false; } break; - case WLR_OUTPUT_STATE_BUFFER_SCANOUT: - assert(output->pending_buffer != NULL); - wl_surface_attach(output->surface, - output->pending_buffer->wl_buffer, 0, 0); + case WLR_OUTPUT_STATE_BUFFER_SCANOUT:; + struct wlr_wl_buffer *buffer = create_wl_buffer(output->backend, + wlr_output->pending.buffer, wlr_output->width, wlr_output->height); + if (buffer == NULL) { + return false; + } + + wl_surface_attach(output->surface, buffer->wl_buffer, 0, 0); if (damage == NULL) { wl_surface_damage_buffer(output->surface, @@ -273,8 +275,6 @@ static bool output_commit(struct wlr_output *wlr_output) { } } wl_surface_commit(output->surface); - - output->pending_buffer = NULL; break; } @@ -400,8 +400,6 @@ static void output_destroy(struct wlr_output *wlr_output) { presentation_feedback_destroy(feedback); } - destroy_wl_buffer(output->pending_buffer); - wlr_egl_destroy_surface(&output->backend->egl, output->egl_surface); wl_egl_window_destroy(output->egl_window); if (output->zxdg_toplevel_decoration_v1) { @@ -429,7 +427,6 @@ static bool output_move_cursor(struct wlr_output *_output, int x, int y) { static const struct wlr_output_impl output_impl = { .destroy = output_destroy, .attach_render = output_attach_render, - .attach_buffer = output_attach_buffer, .test = output_test, .commit = output_commit, .set_cursor = output_set_cursor, diff --git a/include/backend/drm/drm.h b/include/backend/drm/drm.h index 5de4a7d1..ffd3138f 100644 --- a/include/backend/drm/drm.h +++ b/include/backend/drm/drm.h @@ -128,8 +128,6 @@ struct wlr_drm_connector { struct wl_event_source *retry_pageflip; struct wl_list link; - // DMA-BUF to be displayed on next commit - struct wlr_dmabuf_attributes pending_dmabuf; // Buffer submitted to the kernel but not yet displayed struct wlr_buffer *pending_buffer; struct gbm_bo *pending_bo; diff --git a/include/backend/wayland.h b/include/backend/wayland.h index 4e666c3a..9a8a404b 100644 --- a/include/backend/wayland.h +++ b/include/backend/wayland.h @@ -72,7 +72,6 @@ struct wlr_wl_output { struct zxdg_toplevel_decoration_v1 *zxdg_toplevel_decoration_v1; struct wl_egl_window *egl_window; EGLSurface egl_surface; - struct wlr_wl_buffer *pending_buffer; struct wl_list presentation_feedbacks; uint32_t enter_serial; diff --git a/include/wlr/interfaces/wlr_output.h b/include/wlr/interfaces/wlr_output.h index c99d0c93..53ff1310 100644 --- a/include/wlr/interfaces/wlr_output.h +++ b/include/wlr/interfaces/wlr_output.h @@ -28,7 +28,6 @@ struct wlr_output_impl { size_t (*get_gamma_size)(struct wlr_output *output); bool (*export_dmabuf)(struct wlr_output *output, struct wlr_dmabuf_attributes *attribs); - bool (*attach_buffer)(struct wlr_output *output, struct wlr_buffer *buffer); }; void wlr_output_init(struct wlr_output *output, struct wlr_backend *backend, diff --git a/include/wlr/types/wlr_output.h b/include/wlr/types/wlr_output.h index 0e2954b9..89ad509c 100644 --- a/include/wlr/types/wlr_output.h +++ b/include/wlr/types/wlr_output.h @@ -314,8 +314,11 @@ bool wlr_output_attach_render(struct wlr_output *output, int *buffer_age); /** * Attach a buffer to the output. Compositors should call `wlr_output_commit` * to submit the new frame. The output needs to be enabled. + * + * Not all backends support direct scan-out on all buffers. Compositors can + * check whether a buffer is supported by calling `wlr_output_test`. */ -bool wlr_output_attach_buffer(struct wlr_output *output, +void wlr_output_attach_buffer(struct wlr_output *output, struct wlr_buffer *buffer); /** * Get the preferred format for reading pixels. diff --git a/types/wlr_output.c b/types/wlr_output.c index 38df4c9f..dd6ede7a 100644 --- a/types/wlr_output.c +++ b/types/wlr_output.c @@ -474,10 +474,30 @@ static void output_state_clear(struct wlr_output_state *state) { } static bool output_basic_test(struct wlr_output *output) { - if ((output->pending.committed & WLR_OUTPUT_STATE_BUFFER) && - output->frame_pending) { - wlr_log(WLR_DEBUG, "Tried to commit a buffer while a frame is pending"); - return false; + if (output->pending.committed & WLR_OUTPUT_STATE_BUFFER) { + if (output->frame_pending) { + wlr_log(WLR_DEBUG, "Tried to commit a buffer while a frame is pending"); + return false; + } + + if (output->pending.buffer_type == WLR_OUTPUT_STATE_BUFFER_SCANOUT) { + if (output->attach_render_locks > 0) { + return false; + } + + // If the output has at least one software cursor, refuse to attach the + // buffer + struct wlr_output_cursor *cursor; + wl_list_for_each(cursor, &output->cursors, link) { + if (cursor->enabled && cursor->visible && + cursor != output->hardware_cursor) { + return false; + } + } + + // TOOD: check width/height matches the output's, since scaling + // isn't supported + } } bool enabled = output->enabled; @@ -588,34 +608,12 @@ void wlr_output_rollback(struct wlr_output *output) { output_state_clear(&output->pending); } -bool wlr_output_attach_buffer(struct wlr_output *output, +void wlr_output_attach_buffer(struct wlr_output *output, struct wlr_buffer *buffer) { - if (!output->impl->attach_buffer) { - return false; - } - if (output->attach_render_locks > 0) { - return false; - } - - // If the output has at least one software cursor, refuse to attach the - // buffer - struct wlr_output_cursor *cursor; - wl_list_for_each(cursor, &output->cursors, link) { - if (cursor->enabled && cursor->visible && - cursor != output->hardware_cursor) { - return false; - } - } - - if (!output->impl->attach_buffer(output, buffer)) { - return false; - } - output_state_clear_buffer(&output->pending); output->pending.committed |= WLR_OUTPUT_STATE_BUFFER; output->pending.buffer_type = WLR_OUTPUT_STATE_BUFFER_SCANOUT; output->pending.buffer = wlr_buffer_lock(buffer); - return true; } void wlr_output_send_frame(struct wlr_output *output) {