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

Submitted by Ben Widawsky on Jan. 3, 2017, 2:37 a.m.

Details

Message ID 20170103023723.25732-12-ben@bwidawsk.net
State New
Series "Renderbuffer Decompression (and GBM modifiers)"
Headers show

Commit Message

Ben Widawsky Jan. 3, 2017, 2:37 a.m.
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@bitplanet.net>
Cc: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
Acked-by: Daniel Stone <daniels@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(-)

Patch hide | download patch | download mbox

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));
+
    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);
    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 {

Comments

Ben Widawsky Jan. 3, 2017, 10:51 p.m.
On 17-01-02 18:37:02, Ben Widawsky 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@bitplanet.net>
>Cc: Daniel Stone <daniel@fooishbar.org>
>Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
>Acked-by: Daniel Stone <daniels@collabora.com>
>---

[snip]

>+
>+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)
>+{

Kristian, I missed dropping flags from gbm surface creation here. It's fixed
locally.

[snip]
Jason Ekstrand Jan. 10, 2017, 1:03 a.m.
On Mon, Jan 2, 2017 at 6:37 PM, Ben Widawsky <ben@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@bitplanet.net>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
> Acked-by: Daniel Stone <daniels@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Ben Widawsky Jan. 12, 2017, 9:06 p.m.
On 17-01-09 17:03:48, Jason Ekstrand wrote:
>On Mon, Jan 2, 2017 at 6:37 PM, Ben Widawsky <ben@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@bitplanet.net>
>> Cc: Daniel Stone <daniel@fooishbar.org>
>> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
>> Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com>
>> Acked-by: Daniel Stone <daniels@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));"
>

Yeah.

>
>> +
>>     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?
>

Yes.

Thanks.

>
>>     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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>