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

[Pekka Paalanen]

On Fri, 27 May 2016 18:52:28 +0900
Tomohito Esaki <etom at igel.co.jp> 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>
> ---
>  src/compositor-drm.c | 47 +++++++++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 28 deletions(-)

Hi,

I'm not quite sure the testing for the AddFB2 working is totally
correct, as the previous patch is adding a new call to AddFB2 without
dealing with no_addfb2, but I do not think that is significant. I'm
fine leaving it the way you have it.

> diff --git a/src/compositor-drm.c b/src/compositor-drm.c
> index cd2fb71..d6a4a5a 100644
> --- a/src/compositor-drm.c
> +++ b/src/compositor-drm.c
> @@ -960,11 +960,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;
> @@ -1005,15 +1003,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;
>  
> @@ -1053,15 +1048,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,
> @@ -1078,22 +1067,24 @@ 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;
> -	}
>  
> -	s->next = drm_fb_get_from_bo(bo, b, format);
> -	if (!s->next) {
> -		gbm_bo_destroy(bo);
> +	if (bo) {
> +		if (((format = drm_output_check_sprite_format(
> +			      s, ev, gbm_bo_get_format(bo))) == 0) ||
> +		    (!(s->next = drm_fb_get_from_bo(bo, b, format)))) {
> +			gbm_bo_destroy(bo);
> +			return NULL;
> +		}
> +	} else if (dmabuf) {
> +		if (((format = drm_output_check_sprite_format(
> +			      s, ev, dmabuf->attributes.format)) == 0) ||
> +		    (!(s->next = drm_fb_create_dmabuf(dmabuf, b, format))))
> +			return NULL;

The same comments here as for the previous patch for splitting
assignments into separate statements for readability.

> +	} else {
>  		return NULL;
>  	}
>  

Otherwise this patch looks good. Would you like to fix and re-post both?

Some ideas for further development if you are interested:

- Check that dmabuf_attributes::flags does not contain anything we
  don't support in the direct scanout paths.

- Handle dmabuf_attributes::modifier[] when possible, or reject if
  non-zero.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160615/e4585467/attachment-0001.sig>