[Mesa-dev] [PATCH 11/32] gbm: Introduce modifiers into surface/bo creation
Ben Widawsky
ben at bwidawsk.net
Thu Jan 12 21:06:07 UTC 2017
On 17-01-09 17:03:48, Jason Ekstrand wrote:
>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));"
>
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 at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
More information about the mesa-dev
mailing list