On Fri, 9 Dec 2016 19:57:36 +0000 Daniel Stone <dani...@collabora.com> wrote:
> Now that we have better types in drm_fb, use it for cursor buffers as > well. > > Differential Revision: https://phabricator.freedesktop.org/D1493 > > Signed-off-by: Daniel Stone <dani...@collabora.com> > --- > libweston/compositor-drm.c | 73 > +++++++++++++++++++++++++++++++++------------- > 1 file changed, 53 insertions(+), 20 deletions(-) Hi, looks like this patch causes an FB to be created from cursor bo-s, and this was not done before and the FBs are not used yet. I think this requires an explanation in the commit message: what happens and why it will be useful later. I assume the purpose is to use them with universal planes, right? > > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index 950c265..557b97d 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -135,6 +135,7 @@ enum drm_fb_type { > BUFFER_CLIENT, /**< directly sourced from client */ > BUFFER_PIXMAN_DUMB, /**< internal Pixman rendering */ > BUFFER_GBM_SURFACE, /**< internal EGL rendering */ > + BUFFER_CURSOR, /**< internal cursor buffer */ > }; > > struct drm_fb { > @@ -183,11 +184,12 @@ struct drm_output { > int disable_pending; > > struct gbm_surface *gbm_surface; > - struct gbm_bo *gbm_cursor_bo[2]; > + struct drm_fb *gbm_cursor_fb[2]; > struct weston_plane cursor_plane; > - struct weston_plane fb_plane; > struct weston_view *cursor_view; > int current_cursor; > + > + struct weston_plane fb_plane; What is fb_plane doing here? > struct drm_fb *current, *next; > struct backlight *backlight; > > @@ -285,7 +287,8 @@ drm_fb_destroy_gbm(struct gbm_bo *bo, void *data) > { > struct drm_fb *fb = data; > > - assert(fb->type == BUFFER_GBM_SURFACE || fb->type == BUFFER_CLIENT); > + assert(fb->type == BUFFER_GBM_SURFACE || fb->type == BUFFER_CLIENT || > + fb->type == BUFFER_CURSOR); > drm_fb_destroy(fb); > } > > @@ -493,6 +496,7 @@ drm_fb_unref(struct drm_fb *fb) > case BUFFER_PIXMAN_DUMB: > drm_fb_destroy_dumb(fb); > break; > + case BUFFER_CURSOR: > case BUFFER_CLIENT: > gbm_bo_destroy(fb->bo); > break; > @@ -1281,7 +1285,7 @@ drm_output_set_cursor(struct drm_output *output) > pixman_region32_fini(&output->cursor_plane.damage); > pixman_region32_init(&output->cursor_plane.damage); > output->current_cursor ^= 1; > - bo = output->gbm_cursor_bo[output->current_cursor]; > + bo = output->gbm_cursor_fb[output->current_cursor]->bo; > > cursor_bo_update(b, bo, ev); > handle = gbm_bo_get_handle(bo).s32; > @@ -1867,6 +1871,48 @@ find_crtc_for_connector(struct drm_backend *b, > return -1; > } > > +static void drm_output_fini_cursor_egl(struct drm_output *output) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_LENGTH(output->gbm_cursor_fb); i++) { > + drm_fb_unref(output->gbm_cursor_fb[i]); Oh this is why drm_fb_unref() silently accepts NULL. I thought we had a habit of avoiding that, or are destructors an exception? > + output->gbm_cursor_fb[i] = NULL; > + } > +} > + > +static int > +drm_output_init_cursor_egl(struct drm_output *output, struct drm_backend *b) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_LENGTH(output->gbm_cursor_fb); i++) { > + struct gbm_bo *bo; > + > + bo = gbm_bo_create(b->gbm, b->cursor_width, b->cursor_height, > + GBM_FORMAT_ARGB8888, > + GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE); > + if (!bo) > + goto err; > + > + output->gbm_cursor_fb[i] = > + drm_fb_get_from_bo(bo, b, GBM_FORMAT_ARGB8888, > + BUFFER_CURSOR); > + if (!output->gbm_cursor_fb[i]) { > + gbm_bo_destroy(bo); > + goto err; > + } > + } > + > + return 0; > + > +err: > + weston_log("cursor buffers unavailable, using gl cursors\n"); > + b->cursors_are_broken = 1; > + drm_output_fini_cursor_egl(output); > + return -1; > +} > + > /* Init output state that depends on gl or gbm */ > static int > drm_output_init_egl(struct drm_output *output, struct drm_backend *b) > @@ -1875,7 +1921,7 @@ drm_output_init_egl(struct drm_output *output, struct > drm_backend *b) > output->gbm_format, > fallback_format_for(output->gbm_format), > }; > - int i, flags, n_formats = 1; > + int n_formats = 1; > > output->gbm_surface = gbm_surface_create(b->gbm, > output->base.current_mode->width, > @@ -1901,21 +1947,7 @@ drm_output_init_egl(struct drm_output *output, struct > drm_backend *b) > return -1; > } > > - flags = GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE; > - > - for (i = 0; i < 2; i++) { > - if (output->gbm_cursor_bo[i]) > - continue; > - > - output->gbm_cursor_bo[i] = > - gbm_bo_create(b->gbm, b->cursor_width, b->cursor_height, > - GBM_FORMAT_ARGB8888, flags); > - } > - > - if (output->gbm_cursor_bo[0] == NULL || output->gbm_cursor_bo[1] == > NULL) { > - weston_log("cursor buffers unavailable, using gl cursors\n"); > - b->cursors_are_broken = 1; > - } > + drm_output_init_cursor_egl(output, b); > > return 0; > } > @@ -1925,6 +1957,7 @@ drm_output_fini_egl(struct drm_output *output) > { > gl_renderer->output_destroy(&output->base); > gbm_surface_destroy(output->gbm_surface); > + drm_output_fini_cursor_egl(output); > } > > static int Anyway, with fb_plane dropped and commit message improved: Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> Thanks, pq
pgpsNeTHWXasK.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
