On Fri, 9 Dec 2016 19:57:34 +0000 Daniel Stone <dani...@collabora.com> wrote:
> Sometimes we need to duplicate an existing drm_fb, e.g. when > pageflipping to the same buffer to kickstart the repaint loop. To handle > situations like these, and simplify resource management for dumb and > cursor buffers, refcount drm_fb. > > Differential Revision: https://phabricator.freedesktop.org/D1491 > > Signed-off-by: Daniel Stone <dani...@collabora.com> > --- > libweston/compositor-drm.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index eb735b2..a684ac9 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -140,6 +140,8 @@ enum drm_fb_type { > struct drm_fb { > enum drm_fb_type type; > > + int refcnt; > + > uint32_t fb_id, stride, handle, size; > const struct pixel_format_info *format; > int width, height; > @@ -302,6 +304,8 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int > height, > if (!fb) > return NULL; > > + fb->refcnt = 1; > + > fb->format = pixel_format_get_info(format); > if (!fb->format) { > weston_log("failed to look up format 0x%lx\n", > @@ -312,6 +316,7 @@ drm_fb_create_dumb(struct drm_backend *b, int width, int > height, > if (!fb->format->depth || !fb->format->bpp) { > weston_log("format 0x%lx is not compatible with dumb buffers\n", > (unsigned long) format); > + goto err_fb; Hi, this hunk belongs in an earlier patch. > } > > memset(&create_arg, 0, sizeof create_arg); > @@ -384,6 +389,13 @@ err_fb: > } > > static struct drm_fb * > +drm_fb_ref(struct drm_fb *fb) > +{ > + fb->refcnt++; > + return fb; > +} > + > +static struct drm_fb * > drm_fb_get_from_bo(struct gbm_bo *bo, struct drm_backend *backend, > uint32_t format, enum drm_fb_type type) > { > @@ -392,13 +404,14 @@ drm_fb_get_from_bo(struct gbm_bo *bo, struct > drm_backend *backend, > int ret; > > if (fb) > - return fb; > + return drm_fb_ref(fb); This path is now different from before, and requires adding an equivalent drm_fb_unref() call somewhere, but I don't see it. Previously unref would have released it immediately, now the first call will be a no-op. Or, if this is a bug fix, it requires an explanation in the commit message how in some cases drm_fb_unref() got called twice. Maybe this hunk belongs in a different patch instead? > > fb = zalloc(sizeof *fb); > if (fb == NULL) > return NULL; > > fb->type = type; > + fb->refcnt = 1; > fb->bo = bo; > > fb->width = gbm_bo_get_width(bo); > @@ -472,6 +485,10 @@ drm_fb_unref(struct drm_fb *fb) > if (!fb) > return; > > + assert(fb->refcnt > 0); > + if (--fb->refcnt > 0) > + return; > + > switch (fb->type) { > case BUFFER_PIXMAN_DUMB: > /* nothing: pixman buffers are destroyed manually */ It took a while to see the paths: drm_fb_unref -> gbm_bo_destroy -> drm_fb_destroy_gbm drm_fb_unref -> gbm_surface_release_buffer gbm_surface_destroy -> drm_fb_destroy_gbm drm_output_fini_pixman -> drm_fb_destroy_dumb The goal is to solidify drm_fb_unref() as the main entry point for destruction, but it's not quite there yet. Very good. If you move both of the hunks I pointed out into other patches, then this patch gets: Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk> If that's not the right thing to do, I'll review the next revision. Thanks, pq
pgpXCTX6RiQSF.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
