[Mesa-dev] [PATCH 14/34] gbm: Get modifiers from DRI
Jason Ekstrand
jason at jlekstrand.net
Tue Jan 31 20:38:44 UTC 2017
On Mon, Jan 23, 2017 at 10:21 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
> Replace the naive, 'save all the modifiers' with a proper query for just
> the modifier that was selected. To accomplish this, two new query tokens
> are added to the extension:
> __DRI_IMAGE_ATTRIB_MODIFIER_UPPER
> __DRI_IMAGE_ATTRIB_MODIFIER_LOWER
>
> The query extension only supported 32b queries, and modifiers are 64b,
> so we needed two of them.
>
> NOTE: The extension version is still set to 13, so none of this will
> actually be called.
>
> v2: Use stored modifiers from create instead of queryImage
>
> v3: Make sure not to query modifiers for dumb buffers (Daniel)
> Fixed comments in functions.
>
> 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/gbm/backends/dri/gbm_dri.c | 37
> ++++++++++++++++++++++++--------
> src/gbm/gbm-symbols-check | 1 +
> src/gbm/main/gbm.c | 19 ++++++++++++++++
> src/gbm/main/gbm.h | 3 +++
> src/gbm/main/gbmint.h | 5 +----
> src/mesa/drivers/dri/i965/intel_screen.c | 6 ++++++
> 6 files changed, 58 insertions(+), 13 deletions(-)
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri
> .c
> index a777f1a984..d5b458aa38 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -38,6 +38,7 @@
> #include <unistd.h>
> #include <dlfcn.h>
> #include <xf86drm.h>
> +#include <drm_fourcc.h>
>
> #include <GL/gl.h> /* dri_interface needs GL types */
> #include <GL/internal/dri_interface.h>
> @@ -732,6 +733,32 @@ gbm_dri_bo_get_offset(struct gbm_bo *_bo, int plane)
> return (uint32_t)offset;
> }
>
> +static uint64_t
> +gbm_dri_bo_get_modifier(struct gbm_bo *_bo)
> +{
> + struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
> + struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
> +
> + if (!dri->image || dri->image->base.version < 14) {
> + errno = ENOSYS;
> + return 0;
>
Do we want to return the invalid modifier in the error case? I thought 0
was "linear"
> + }
> +
> + /* Dumb buffers have no modifiers */
> + if (!bo->image)
> + return 0;
>
Same here. I'm not really sure that this is an error, but saying it's
linear might be a lie. I guess this is a static function so maybe it
doesn't matter?
> +
> + uint64_t ret = 0;
> + int mod;
> + dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_MODIFIER_UPPER,
> &mod);
> + ret = (uint64_t)mod << 32;
> +
> + dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_MODIFIER_LOWER,
> &mod);
> + ret |= mod;
> +
> + return ret;
> +}
> +
> static void
> gbm_dri_bo_destroy(struct gbm_bo *_bo)
> {
> @@ -1074,15 +1101,6 @@ gbm_dri_bo_create(struct gbm_device *gbm,
> 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));
> -
>
What's going on here? Is this in the right patch?
> dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_HANDLE,
> &bo->base.base.handle.s32);
> dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_STRIDE,
> @@ -1230,6 +1248,7 @@ dri_device_create(int fd)
> dri->base.base.bo_get_handle = gbm_dri_bo_get_handle_for_plane;
> dri->base.base.bo_get_stride = gbm_dri_bo_get_stride;
> dri->base.base.bo_get_offset = gbm_dri_bo_get_offset;
> + dri->base.base.bo_get_modifier = gbm_dri_bo_get_modifier;
> dri->base.base.bo_destroy = gbm_dri_bo_destroy;
> dri->base.base.destroy = dri_destroy;
> dri->base.base.surface_create = gbm_dri_surface_create;
> diff --git a/src/gbm/gbm-symbols-check b/src/gbm/gbm-symbols-check
> index c137c6cd93..c72fb66b03 100755
> --- a/src/gbm/gbm-symbols-check
> +++ b/src/gbm/gbm-symbols-check
> @@ -23,6 +23,7 @@ gbm_bo_get_handle
> gbm_bo_get_fd
> gbm_bo_get_plane_count
> gbm_bo_get_handle_for_plane
> +gbm_bo_get_modifier
> gbm_bo_write
> gbm_bo_set_user_data
> gbm_bo_get_user_data
> diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
> index bfdd009bef..8780b41914 100644
> --- a/src/gbm/main/gbm.c
> +++ b/src/gbm/main/gbm.c
> @@ -280,6 +280,25 @@ gbm_bo_get_handle_for_plane(struct gbm_bo *bo, int
> plane)
> return bo->gbm->bo_get_handle(bo, plane);
> }
>
> +/**
> + * Get the chosen modifier for the buffer object
> + *
> + * This function returns the modifier that was chosen for the object.
> These
> + * properties may be generic, or platform/implementation dependent.
> + *
> + * \param bo The buffer object
> + * \return Returns the selected modifier (chosen by the implementation)
> for the
> + * BO.
> + * \sa gbm_bo_create_with_modifiers() where possible modifiers are set
> + * \sa gbm_surface_create_with_modifiers() where possible modifiers are
> set
> + * \sa define DRM_FORMAT_MOD_* in drm_fourcc.h for possible modifiers
> + */
> +GBM_EXPORT uint64_t
> +gbm_bo_get_modifier(struct gbm_bo *bo)
> +{
> + return bo->gbm->bo_get_modifier(bo);
> +}
> +
> /** Write data into the buffer object
> *
> * If the buffer object was created with the GBM_BO_USE_WRITE flag,
> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
> index 39fb9d2d29..b52137ed01 100644
> --- a/src/gbm/main/gbm.h
> +++ b/src/gbm/main/gbm.h
> @@ -327,6 +327,9 @@ gbm_bo_get_handle(struct gbm_bo *bo);
> int
> gbm_bo_get_fd(struct gbm_bo *bo);
>
> +uint64_t
> +gbm_bo_get_modifier(struct gbm_bo *bo);
> +
> int
> gbm_bo_get_plane_count(struct gbm_bo *bo);
>
> diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h
> index cd437df021..c27a7a560a 100644
> --- a/src/gbm/main/gbmint.h
> +++ b/src/gbm/main/gbmint.h
> @@ -82,6 +82,7 @@ struct gbm_device {
> union gbm_bo_handle (*bo_get_handle)(struct gbm_bo *bo, int plane);
> uint32_t (*bo_get_stride)(struct gbm_bo *bo, int plane);
> uint32_t (*bo_get_offset)(struct gbm_bo *bo, int plane);
> + uint64_t (*bo_get_modifier)(struct gbm_bo *bo);
> void (*bo_destroy)(struct gbm_bo *bo);
>
> struct gbm_surface *(*surface_create)(struct gbm_device *gbm,
> @@ -107,10 +108,6 @@ struct gbm_bo {
> uint32_t height;
> uint32_t stride;
> uint32_t format;
> - struct {
> - uint64_t *modifiers;
> - unsigned int count;
> - };
>
Same here.
> union gbm_bo_handle handle;
> void *user_data;
> void (*destroy_user_data)(struct gbm_bo *, void *);
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c
> b/src/mesa/drivers/dri/i965/intel_screen.c
> index eb4f3d7e6b..91b89f0a93 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -743,6 +743,12 @@ intel_query_image(__DRIimage *image, int attrib, int
> *value)
> case __DRI_IMAGE_ATTRIB_OFFSET:
> *value = image->offset;
> return true;
> + case __DRI_IMAGE_ATTRIB_MODIFIER_LOWER:
> + *value = (image->modifier & 0xffffffff);
> + return true;
> + case __DRI_IMAGE_ATTRIB_MODIFIER_UPPER:
> + *value = ((image->modifier >> 32) & 0xffffffff);
> + return true;
>
> default:
> return false;
> --
> 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/6afdf6d6/attachment.html>
More information about the mesa-dev
mailing list