[Mesa-dev] [PATCH 11/32] gbm: Introduce modifiers into surface/bo creation

Jason Ekstrand jason at jlekstrand.net
Tue Jan 10 01:03:48 UTC 2017


On Mon, Jan 2, 2017 at 6:37 PM, Ben Widawsky <ben at bwidawsk.net> wrote:

> The idea behind modifiers like this is that the user of GBM will have
> some mechanism to query what properties the hardware supports for its BO
> or surface. This information is directly passed in (and stored) so that
> the DRI implementation can create an image with the appropriate
> attributes.
>
> A getter() will be added later so that the user GBM will be able to
> query what modifier should be used.
>
> I've opted to store all modifiers passed in during creation and to make
> the determination happen at actual creation time for no reason other
> than it seems more flexible.
>
> v2: Make sure to check if count is non-zero in addition to testing if
> calloc fails. (Daniel)
>
> v3: Remove "usage" and "flags" from modifier creation. Requested by
> Kristian.
>
> Cc: Kristian Høgsberg <krh at bitplanet.net>
> Cc: Daniel Stone <daniel at fooishbar.org>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
> Acked-by: Daniel Stone <daniels at collabora.com>
> ---
>  src/egl/drivers/dri2/platform_drm.c | 19 +++++++++++++----
>  src/gbm/backends/dri/gbm_dri.c      | 41 ++++++++++++++++++++++++++++++
> +------
>  src/gbm/gbm-symbols-check           |  2 ++
>  src/gbm/main/gbm.c                  | 28 +++++++++++++++++++++++--
>  src/gbm/main/gbm.h                  | 12 +++++++++++
>  src/gbm/main/gbmint.h               | 16 +++++++++++++--
>  6 files changed, 104 insertions(+), 14 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/
> platform_drm.c
> index 20993147c8..86247ecaf3 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -228,10 +228,21 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
>
>     if (dri2_surf->back == NULL)
>        return -1;
> -   if (dri2_surf->back->bo == NULL)
> -      dri2_surf->back->bo = gbm_bo_create(&dri2_dpy->gbm_dri->base.base,
> -                                         surf->base.width,
> surf->base.height,
> -                                         surf->base.format,
> surf->base.flags);
> +   if (dri2_surf->back->bo == NULL) {
> +      if (surf->base.modifiers)
> +         dri2_surf->back->bo = gbm_bo_create_with_modifiers(&
> dri2_dpy->gbm_dri->base.base,
> +
> surf->base.width, surf->base.height,
> +
> surf->base.format,
> +
> surf->base.modifiers,
> +
> surf->base.count);
> +      else
> +         dri2_surf->back->bo = gbm_bo_create(&dri2_dpy->gbm_
> dri->base.base,
> +                                             surf->base.width,
> +                                             surf->base.height,
> +                                             surf->base.format,
> +                                             surf->base.flags);
> +
> +   }
>     if (dri2_surf->back->bo == NULL)
>        return -1;
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_
> dri.c
> index 20bbf27cc3..f0e67b081e 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -958,13 +958,21 @@ free_bo:
>  static struct gbm_bo *
>  gbm_dri_bo_create(struct gbm_device *gbm,
>                    uint32_t width, uint32_t height,
> -                  uint32_t format, uint32_t usage)
> +                  uint32_t format, uint32_t usage,
> +                  const uint64_t *modifiers,
> +                  const unsigned int count)
>  {
>     struct gbm_dri_device *dri = gbm_dri_device(gbm);
>     struct gbm_dri_bo *bo;
>     int dri_format;
>     unsigned dri_use = 0;
>
> +   /* Callers of this may specify a modifier, or a dri usage, but not
> both. The
> +    * newer modifier interface deprecates the older usage flags. This is
> the
> +    * equivalent of usage NAND count.
> +    */
> +   assert(~(usage & count));
>

Again, I don't think this is what you want.  You want "assert(!(usage &&
count));"


> +
>     if (usage & GBM_BO_USE_WRITE || dri->image == NULL)
>        return create_dumb(gbm, width, height, format, usage);
>
> @@ -1023,13 +1031,23 @@ gbm_dri_bo_create(struct gbm_device *gbm,
>     dri_use |= __DRI_IMAGE_USE_SHARE;
>
>     bo->image =
> -      dri->image->createImage(dri->screen,
> -                              width, height,
> -                              dri_format, dri_use,
> -                              bo);
> +      dri->image->createImageWithModifiers(dri->screen,
> +                                           width, height,
> +                                           dri_format,
> +                                           modifiers, count,
> +                                           bo);
>

Do we want to handle the case where your DRI is too old to have
createImageWithModifiers?


>     if (bo->image == NULL)
>        goto failed;
>
> +   bo->base.base.modifiers = calloc(count, sizeof(*modifiers));
>
+   if (count && !bo->base.base.modifiers) {
> +      dri->image->destroyImage(bo->image);
> +      goto failed;
> +   }
> +
> +   bo->base.base.count = count;
> +   memcpy(bo->base.base.modifiers, modifiers, count *
> sizeof(*modifiers));
> +
>     dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_HANDLE,
>                            &bo->base.base.handle.s32);
>     dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_STRIDE,
> @@ -1100,7 +1118,8 @@ gbm_dri_bo_unmap(struct gbm_bo *_bo, void *map_data)
>  static struct gbm_surface *
>  gbm_dri_surface_create(struct gbm_device *gbm,
>                         uint32_t width, uint32_t height,
> -                      uint32_t format, uint32_t flags)
> +                      uint32_t format, uint32_t flags,
> +                       const uint64_t *modifiers, const unsigned count)
>  {
>     struct gbm_dri_surface *surf;
>
> @@ -1114,6 +1133,15 @@ gbm_dri_surface_create(struct gbm_device *gbm,
>     surf->base.format = format;
>     surf->base.flags = flags;
>
> +   surf->base.modifiers = calloc(count, sizeof(*modifiers));
> +   if (count && !surf->base.modifiers) {
>
+      free(surf);
> +      return NULL;
> +   }
> +
> +   surf->base.count = count;
> +   memcpy(surf->base.modifiers, modifiers, count * sizeof(*modifiers));
> +
>     return &surf->base;
>  }
>
> @@ -1122,6 +1150,7 @@ gbm_dri_surface_destroy(struct gbm_surface *_surf)
>  {
>     struct gbm_dri_surface *surf = gbm_dri_surface(_surf);
>
> +   free(surf->base.modifiers);
>     free(surf);
>  }
>
> diff --git a/src/gbm/gbm-symbols-check b/src/gbm/gbm-symbols-check
> index 7ff78ab400..c137c6cd93 100755
> --- a/src/gbm/gbm-symbols-check
> +++ b/src/gbm/gbm-symbols-check
> @@ -8,6 +8,7 @@ gbm_device_is_format_supported
>  gbm_device_destroy
>  gbm_create_device
>  gbm_bo_create
> +gbm_bo_create_with_modifiers
>  gbm_bo_import
>  gbm_bo_map
>  gbm_bo_unmap
> @@ -27,6 +28,7 @@ gbm_bo_set_user_data
>  gbm_bo_get_user_data
>  gbm_bo_destroy
>  gbm_surface_create
> +gbm_surface_create_with_modifiers
>  gbm_surface_needs_lock_front_buffer
>  gbm_surface_lock_front_buffer
>  gbm_surface_release_buffer
> diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
> index 295f6894eb..64da03b0da 100644
> --- a/src/gbm/main/gbm.c
> +++ b/src/gbm/main/gbm.c
> @@ -369,9 +369,23 @@ gbm_bo_create(struct gbm_device *gbm,
>        return NULL;
>     }
>
> -   return gbm->bo_create(gbm, width, height, format, usage);
> +   return gbm->bo_create(gbm, width, height, format, usage, NULL, 0);
>  }
>
> +GBM_EXPORT struct gbm_bo *
> +gbm_bo_create_with_modifiers(struct gbm_device *gbm,
> +                             uint32_t width, uint32_t height,
> +                             uint32_t format,
> +                             const uint64_t *modifiers,
> +                             const unsigned int count)
> +{
> +   if (width == 0 || height == 0) {
> +      errno = EINVAL;
> +      return NULL;
> +   }
> +
> +   return gbm->bo_create(gbm, width, height, format, 0, modifiers, count);
> +}
>  /**
>   * Create a gbm buffer object from an foreign object
>   *
> @@ -477,7 +491,17 @@ gbm_surface_create(struct gbm_device *gbm,
>                     uint32_t width, uint32_t height,
>                    uint32_t format, uint32_t flags)
>  {
> -   return gbm->surface_create(gbm, width, height, format, flags);
> +   return gbm->surface_create(gbm, width, height, format, flags, NULL, 0);
> +}
> +
> +GBM_EXPORT struct gbm_surface *
> +gbm_surface_create_with_modifiers(struct gbm_device *gbm,
> +                                  uint32_t width, uint32_t height,
> +                                  uint32_t format, uint32_t flags,
> +                                  const uint64_t *modifiers, const
> unsigned int count)
> +{
> +   return gbm->surface_create(gbm, width, height, format, flags,
> +                              modifiers, count);
>  }
>
>  /**
> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
> index b089359b01..6390e60d04 100644
> --- a/src/gbm/main/gbm.h
> +++ b/src/gbm/main/gbm.h
> @@ -243,6 +243,12 @@ gbm_bo_create(struct gbm_device *gbm,
>                uint32_t width, uint32_t height,
>                uint32_t format, uint32_t flags);
>
> +struct gbm_bo *
> +gbm_bo_create_with_modifiers(struct gbm_device *gbm,
> +                             uint32_t width, uint32_t height,
> +                             uint32_t format,
> +                             const uint64_t *modifiers,
> +                             const unsigned int count);
>  #define GBM_BO_IMPORT_WL_BUFFER         0x5501
>  #define GBM_BO_IMPORT_EGL_IMAGE         0x5502
>  #define GBM_BO_IMPORT_FD                0x5503
> @@ -345,6 +351,12 @@ gbm_surface_create(struct gbm_device *gbm,
>                     uint32_t width, uint32_t height,
>                    uint32_t format, uint32_t flags);
>
> +struct gbm_surface *
> +gbm_surface_create_with_modifiers(struct gbm_device *gbm,
> +                                  uint32_t width, uint32_t height,
> +                                  uint32_t format, uint32_t flags,
> +                                  const uint64_t *modifiers,
> +                                  const unsigned int count);
>  int
>  gbm_surface_needs_lock_front_buffer(struct gbm_surface *surface);
>
> diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h
> index ac6078361a..cd437df021 100644
> --- a/src/gbm/main/gbmint.h
> +++ b/src/gbm/main/gbmint.h
> @@ -65,7 +65,9 @@ struct gbm_device {
>     struct gbm_bo *(*bo_create)(struct gbm_device *gbm,
>                                 uint32_t width, uint32_t height,
>                                 uint32_t format,
> -                               uint32_t usage);
> +                               uint32_t usage,
> +                               const uint64_t *modifiers,
> +                               const unsigned int count);
>     struct gbm_bo *(*bo_import)(struct gbm_device *gbm, uint32_t type,
>                                 void *buffer, uint32_t usage);
>     void *(*bo_map)(struct gbm_bo *bo,
> @@ -84,7 +86,9 @@ struct gbm_device {
>
>     struct gbm_surface *(*surface_create)(struct gbm_device *gbm,
>                                           uint32_t width, uint32_t height,
> -                                         uint32_t format, uint32_t flags);
> +                                         uint32_t format, uint32_t flags,
> +                                         const uint64_t *modifiers,
> +                                         const unsigned count);
>     struct gbm_bo *(*surface_lock_front_buffer)(struct gbm_surface
> *surface);
>     void (*surface_release_buffer)(struct gbm_surface *surface,
>                                    struct gbm_bo *bo);
> @@ -103,6 +107,10 @@ struct gbm_bo {
>     uint32_t height;
>     uint32_t stride;
>     uint32_t format;
> +   struct {
> +      uint64_t *modifiers;
> +      unsigned int count;
> +   };
>     union gbm_bo_handle  handle;
>     void *user_data;
>     void (*destroy_user_data)(struct gbm_bo *, void *);
> @@ -114,6 +122,10 @@ struct gbm_surface {
>     uint32_t height;
>     uint32_t format;
>     uint32_t flags;
> +   struct {
> +      uint64_t *modifiers;
> +      unsigned count;
> +   };
>  };
>
>  struct gbm_backend {
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170109/5e9d566d/attachment-0001.html>


More information about the mesa-dev mailing list