On Mon, Jan 23, 2017 at 10:21 PM, Ben Widawsky <b...@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 <dan...@fooishbar.org> > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com> > Acked-by: Daniel Stone <dani...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev