On Fri, 27 May 2016 18:52:28 +0900
Tomohito Esaki <[email protected]> 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 <[email protected]>
> ---
>  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

Attachment: pgpB88DjT_W6d.pgp
Description: OpenPGP digital signature

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to