[Mesa-dev] [PATCH 10/34] gbm: Introduce modifiers into surface/bo creation
Jason Ekstrand
jason at jlekstrand.net
Tue Jan 31 19:52:38 UTC 2017
On Mon, Jan 23, 2017 at 10:21 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.
>
> v4: Take advantage of the "INVALID" modifier added by the GET_PLANE2
> series.
>
> Cc: Kristian Høgsberg <krh at bitplanet.net>
> References (v4): https://lists.freedesktop.org/archives/intel-gfx/2017-
> January/116636.html
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com> (v1)
> Acked-by: Daniel Stone <daniels at collabora.com>
> ---
> src/egl/drivers/dri2/platform_drm.c | 19 +++++++++---
> src/gbm/backends/dri/gbm_dri.c | 51
> +++++++++++++++++++++++++++-----
> src/gbm/gbm-symbols-check | 2 ++
> src/gbm/main/gbm.c | 29 ++++++++++++++++--
> src/gbm/main/gbm.h | 12 ++++++++
> src/gbm/main/gbmint.h | 16 ++++++++--
> src/mesa/drivers/dri/i965/intel_screen.c | 17 +++++++++++
> 7 files changed, 131 insertions(+), 15 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/platform_drm.c b/src/egl/drivers/dri2/
> platform_drm.c
> index e5e8c60596..cf35ce8a1f 100644
> --- a/src/egl/drivers/dri2/platform_drm.c
> +++ b/src/egl/drivers/dri2/platform_drm.c
> @@ -230,10 +230,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 43f8e15e62..a777f1a984 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -985,13 +985,20 @@ 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.
> + */
> + assert(!(usage && count));
> +
> if (usage & GBM_BO_USE_WRITE || dri->image == NULL)
> return create_dumb(gbm, width, height, format, usage);
>
> @@ -1049,14 +1056,33 @@ gbm_dri_bo_create(struct gbm_device *gbm,
> /* Gallium drivers requires shared in order to get the handle/stride */
> dri_use |= __DRI_IMAGE_USE_SHARE;
>
> - bo->image =
> - dri->image->createImage(dri->screen,
> - width, height,
> - dri_format, dri_use,
> - bo);
> + if (!dri->image || dri->image->base.version < 14 ||
> + !dri->image->createImageWithModifiers) {
> + if (modifiers)
> + fprintf(stderr, "Modifiers specified, but DRI is too old\n");
> +
> + bo->image = dri->image->createImage(dri->screen, width, height,
> + dri_format, dri_use, bo);
> + } else {
> + bo->image =
> + dri->image->createImageWithModifiers(dri->screen,
> + width, height,
> + dri_format,
> + modifiers, count,
> + bo);
>
In this case (where createImageWithModifiers is supported) we always call
createImageWithModifiers and throw away dri_use. Was that intended? It
seems to me like we really want
if (modifiers) {
createImageWithModifiers()
} else {
createImage()
}
Where the version check is just for the purpose of printing the error.
> + }
> 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,
> @@ -1127,7 +1153,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;
>
> @@ -1141,6 +1168,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;
> }
>
> @@ -1149,6 +1185,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..bfdd009bef 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,18 @@ 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,
> + const uint64_t *modifiers,
> + const unsigned int count)
> +{
> + return gbm->surface_create(gbm, width, height, format, 0,
> + modifiers, count);
> }
>
> /**
> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
> index b089359b01..39fb9d2d29 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,
> + 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 {
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index fcaf1b81fb..e3bbeefe8e 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -546,6 +546,17 @@ intel_destroy_image(__DRIimage *image)
> free(image);
> }
>
> +#ifndef DRM_FORMAT_MOD_INVALID
> +#define DRM_FORMAT_MOD_INVALID ((1ULL<<56) - 1)
> +#endif
> +static uint64_t
> +select_best_modifier(struct gen_device_info *devinfo,
> + const uint64_t *modifiers,
> + const unsigned count)
> +{
> + return DRM_FORMAT_MOD_INVALID;
> +}
> +
> static __DRIimage *
> __intel_create_image(__DRIscreen *dri_screen,
> int width, int height, int format,
> @@ -566,6 +577,12 @@ __intel_create_image(__DRIscreen *dri_screen,
> */
> assert(!(use && count));
>
> + uint64_t modifier = select_best_modifier(&screen->devinfo, modifiers,
> count);
> + assert(modifier == DRM_FORMAT_MOD_INVALID);
> +
> + /* Historically, X-tiled was the default, and so lack of modifier means
> + * X-tiled.
> + */
> tiling = I915_TILING_X;
> if (use & __DRI_IMAGE_USE_CURSOR) {
> if (width != 64 || height != 64)
> --
> 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/20170131/8869b73e/attachment-0001.html>
More information about the mesa-dev
mailing list