diff options
author | Vincent Vanlaer <vincent.vanlaer@skynet.be> | 2019-02-04 20:47:07 +0100 |
---|---|---|
committer | Vincent Vanlaer <vincent.vanlaer@skynet.be> | 2019-02-04 20:47:07 +0100 |
commit | 7bc43413edc2db04bbfba395f9606957ff2fa21c (patch) | |
tree | 84d41957d4f0a9fe3d8ae7018b45beac6cf27e5c /backend/drm | |
parent | 28f11aec3106c5d0528d1835055c82c4d571c286 (diff) |
Allow cursor render surface to be used as fb
In order for a surface to be used as a cursor plane framebuffer, it
appears that requiring the buffer to be linear is sufficient.
GBM_BO_USE_SCANOUT is added in case GBM_BO_USE_LINEAR isn't sufficient
on untested hardware.
Fixes #1323
Removed wlr_drm_plane.cursor_bo as it does not serve any purpose
anymore.
Relevant analysis (taken from the PR description):
While trying to implement a fix for #1323, I found that when exporting
the rendered surface into a DMA-BUF and reimporting it with
`GBM_BO_USE_CURSOR`, the resulting object does not appear to be valid.
After some digging (turning on drm-kms debugging and switching to legacy
mode), I managed to extract the following error: ```
[drm:__setplane_check.isra.1 [drm]] Invalid pixel format AR24
little-endian (0x34325241), modifier 0x100000000000001 ``` The format
itself refers to ARGB8888 which is the same format as
`renderer->gbm_format` used in master to create the cursor bo. However,
using `gbm_bo_create` with `GBM_BO_USE_CURSOR` results in a modifier of
0. A modifier of zero represents a linear buffer while the modifier of
the surface that is rendered to is `I915_FORMAT_MOD_X_TILED` (see
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/include/uapi/drm/drm_fourcc.h?h=v4.20.6#n263).
In order to fix this mismatch in modifier, I added the
`GBM_BO_USE_LINEAR` to the render surface and everything started to work
just fine. I wondered however, whether the export and import is really
necessary. I then decided to test if the back buffer of the render
surface works as well, and at least on my hardware (Intel HD 530 and
Intel UHD 620) it does. This is the patch in this PR and this requires
no exporting and importing.
I have to note that I cheated in order to import DMA_BUFs into a cursor
bo when doing the first tests, since on import the Intel drivers check
that the cursor is 64x64. This is strange since cursor sizes other than
64x64 have been around for quite some time now
(https://lists.freedesktop.org/archives/mesa-commit/2014-June/050268.html).
Removing this check made everything work fine. I later (while writing
this PR) found out that `__DRI_IMAGE_USE_CURSOR` (to which
`GBM_BO_USE_CURSOR` translates) has been deprecated in mesa
(https://gitlab.freedesktop.org/mesa/mesa/blob/master/include/GL/internal/dri_interface.h#L1296),
which makes me wonder what the usecase of `GBM_BO_USE_CURSOR` is. The
reason we never encountered this is that when specifying
`GBM_BO_USE_WRITE`, a dumb buffer is created trough DRM and the usage
flag never reaches the Intel driver directly. The relevant code is in
https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gbm/backends/dri/gbm_dri.c#L1011-1089
. From this it seems that as long as the size, format and modifiers are
right, any surface can be used as a cursor.
Diffstat (limited to 'backend/drm')
-rw-r--r-- | backend/drm/backend.c | 2 | ||||
-rw-r--r-- | backend/drm/drm.c | 33 |
2 files changed, 3 insertions, 32 deletions
diff --git a/backend/drm/backend.c b/backend/drm/backend.c index a9082077..e1ec0966 100644 --- a/backend/drm/backend.c +++ b/backend/drm/backend.c @@ -104,7 +104,7 @@ static void session_signal(struct wl_listener *listener, void *data) { struct wlr_drm_plane *plane = conn->crtc->cursor; drm->iface->crtc_set_cursor(drm, conn->crtc, - (plane && plane->cursor_enabled) ? plane->cursor_bo : NULL); + (plane && plane->cursor_enabled) ? plane->surf.back : NULL); drm->iface->crtc_move_cursor(drm, conn->crtc, conn->cursor_x, conn->cursor_y); diff --git a/backend/drm/drm.c b/backend/drm/drm.c index 999ca494..619664d6 100644 --- a/backend/drm/drm.c +++ b/backend/drm/drm.c @@ -226,12 +226,6 @@ void finish_drm_resources(struct wlr_drm_backend *drm) { } free(crtc->gamma_table); } - for (size_t i = 0; i < drm->num_planes; ++i) { - struct wlr_drm_plane *plane = &drm->planes[i]; - if (plane->cursor_bo) { - gbm_bo_destroy(plane->cursor_bo); - } - } free(drm->crtcs); free(drm->planes); @@ -642,17 +636,10 @@ static bool drm_connector_set_cursor(struct wlr_output *output, drm->parent ? &drm->parent->renderer : &drm->renderer; if (!init_drm_surface(&plane->surf, renderer, w, h, - renderer->gbm_format, 0)) { + renderer->gbm_format, GBM_BO_USE_LINEAR | GBM_BO_USE_SCANOUT)) { wlr_log(WLR_ERROR, "Cannot allocate cursor resources"); return false; } - - plane->cursor_bo = gbm_bo_create(drm->renderer.gbm, w, h, - renderer->gbm_format, GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE); - if (!plane->cursor_bo) { - wlr_log_errno(WLR_ERROR, "Failed to create cursor bo"); - return false; - } } wlr_matrix_projection(plane->matrix, plane->surf.width, @@ -697,17 +684,6 @@ static bool drm_connector_set_cursor(struct wlr_output *output, return false; } - uint32_t bo_width = gbm_bo_get_width(plane->cursor_bo); - uint32_t bo_height = gbm_bo_get_height(plane->cursor_bo); - - uint32_t bo_stride; - void *bo_data; - if (!gbm_bo_map(plane->cursor_bo, 0, 0, bo_width, bo_height, - GBM_BO_TRANSFER_WRITE, &bo_stride, &bo_data)) { - wlr_log_errno(WLR_ERROR, "Unable to map buffer"); - return false; - } - make_drm_surface_current(&plane->surf, NULL); struct wlr_renderer *rend = plane->surf.renderer->wlr_rend; @@ -722,13 +698,8 @@ static bool drm_connector_set_cursor(struct wlr_output *output, wlr_render_texture_with_matrix(rend, texture, matrix, 1.0); wlr_renderer_end(rend); - wlr_renderer_read_pixels(rend, WL_SHM_FORMAT_ARGB8888, NULL, bo_stride, - plane->surf.width, plane->surf.height, 0, 0, 0, 0, bo_data); - swap_drm_surface_buffers(&plane->surf, NULL); - gbm_bo_unmap(plane->cursor_bo, bo_data); - plane->cursor_enabled = true; } @@ -736,7 +707,7 @@ static bool drm_connector_set_cursor(struct wlr_output *output, return true; // will be committed when session is resumed } - struct gbm_bo *bo = plane->cursor_enabled ? plane->cursor_bo : NULL; + struct gbm_bo *bo = plane->cursor_enabled ? plane->surf.back : NULL; bool ok = drm->iface->crtc_set_cursor(drm, crtc, bo); if (ok) { wlr_output_update_needs_swap(output); |