[PATCH weston v2 3/3] compositor-drm: Support linux_dmabuf output for sprite planes without gbm

[Fabien DESSENNE]

Hi,


Please find below a comment.


Fabien


On 09/30/2016 11:28 AM, Tomohito Esaki wrote:
> Multiplanar formats are supported by using drmModeAddFB2 and bypassing
> gbm. If drmModeAddFB2 isn't available, the existing gbm bo import path
> is used and multiplanar formats are unsupported.
>
> Signed-off-by: Tomohito Esaki <etom at igel.co.jp>
> ---
>   libweston/compositor-drm.c | 53 +++++++++++++++++++++++-----------------------
>   1 file changed, 26 insertions(+), 27 deletions(-)
>
> diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c
> index b15fa01..f0e6f7c 100644
> --- a/libweston/compositor-drm.c
> +++ b/libweston/compositor-drm.c
> @@ -1008,11 +1008,9 @@ page_flip_handler(int fd, unsigned int frame,
>   
>   static uint32_t
>   drm_output_check_sprite_format(struct drm_sprite *s,
> -			       struct weston_view *ev, struct gbm_bo *bo)
> +			       struct weston_view *ev, uint32_t format)
>   {
> -	uint32_t i, format;
> -
> -	format = gbm_bo_get_format(bo);
> +	uint32_t i;
>   
>   	if (format == GBM_FORMAT_ARGB8888) {
>   		pixman_region32_t r;
> @@ -1053,15 +1051,12 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>   	struct drm_sprite *s;
>   	struct linux_dmabuf_buffer *dmabuf;
>   	int found = 0;
> -	struct gbm_bo *bo;
> +	struct gbm_bo *bo = NULL;
>   	pixman_region32_t dest_rect, src_rect;
>   	pixman_box32_t *box, tbox;
>   	uint32_t format;
>   	wl_fixed_t sx1, sy1, sx2, sy2;
>   
> -	if (b->gbm == NULL)
> -		return NULL;
> -
>   	if (viewport->buffer.transform != output->base.transform)
>   		return NULL;
>   
> @@ -1101,15 +1096,9 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>   	if (!found)
>   		return NULL;
>   
> -	if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource))) {
> +	if ((dmabuf = linux_dmabuf_buffer_get(buffer_resource)) &&
> +	    b->no_addfb2 && b->gbm) {
>   #ifdef HAVE_GBM_FD_IMPORT
> -		/* XXX: TODO:
> -		 *
> -		 * Use AddFB2 directly, do not go via GBM.
> -		 * Add support for multiplanar formats.
> -		 * Both require refactoring in the DRM-backend to
> -		 * support a mix of gbm_bos and drmfbs.
> -		 */
>   		struct gbm_import_fd_data gbm_dmabuf = {
>   			.fd     = dmabuf->attributes.fd[0],
>   			.width  = dmabuf->attributes.width,
> @@ -1126,22 +1115,32 @@ drm_output_prepare_overlay_view(struct drm_output *output,
>   #else
>   		return NULL;
>   #endif
> -	} else {
> +	} else if (b->gbm) {
>   		bo = gbm_bo_import(b->gbm, GBM_BO_IMPORT_WL_BUFFER,
>   				   buffer_resource, GBM_BO_USE_SCANOUT);
>   	}
> -	if (!bo)
> -		return NULL;
>   
> -	format = drm_output_check_sprite_format(s, ev, bo);
> -	if (format == 0) {
> -		gbm_bo_destroy(bo);
> -		return NULL;
> -	}
> +	if (bo) {
> +		format = drm_output_check_sprite_format(
> +			s, ev, gbm_bo_get_format(bo));
> +		if (format == 0)
Unless I missed something, you shall call gbm_bo_destroy(bo) before 
returning.

> +			return NULL;
> +
> +		s->next = drm_fb_get_from_bo(bo, b, format);
> +		if (!s->next) {
> +			gbm_bo_destroy(bo);
> +			return NULL;
> +		}
> +	} else if (dmabuf) {
> +		format = drm_output_check_sprite_format(
> +			s, ev, dmabuf->attributes.format);
> +		if (format == 0)
> +			return NULL;
>   
> -	s->next = drm_fb_get_from_bo(bo, b, format);
> -	if (!s->next) {
> -		gbm_bo_destroy(bo);
> +		s->next = drm_fb_create_dmabuf(dmabuf, b, format);
> +		if (!s->next)
> +			return NULL;
> +	} else {
>   		return NULL;
>   	}
>