[PATCH weston 21/68] compositor-drm: Use drm_fb for cursor buffers

[Pekka Paalanen]

On Fri,  9 Dec 2016 19:57:36 +0000
Daniel Stone <daniels at 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 <daniels at 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.paalanen at collabora.co.uk>


Thanks,
pq

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20170222/71686507/attachment.sig>