On Mon, Mar 13, 2017 at 10:24 PM, Ben Widawsky <b...@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. > > Only in surface creation, the modifiers are stored until the BO is > actually allocated. In regular buffer allocation, the correct modifier > can (will be, in future patches be chosen at creation time. > > 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. > > v5: Don't bother with storing modifiers for gbm_bo_create because that's > a synchronous operation and we can actually select the correct modifier > at create time (done in a later patch) (Jason) > > Cc: Kristian Høgsberg <k...@bitplanet.net> > Cc: Jason Ekstrand <ja...@jlekstrand.net> > References (v4): https://lists.freedesktop.org/archives/intel-gfx/2017- > January/116636.html > Signed-off-by: Ben Widawsky <b...@bwidawsk.net> > Reviewed-by: Eric Engestrom <eric.engest...@imgtec.com> (v1) > Acked-by: Daniel Stone <dani...@collabora.com> > --- > src/gbm/backends/dri/gbm_dri.c | 62 ++++++++++++++++++++++++++++++ > ++++++------ > src/gbm/gbm-symbols-check | 2 ++ > src/gbm/main/gbm.c | 39 ++++++++++++++++++++++++-- > src/gbm/main/gbm.h | 12 ++++++++ > src/gbm/main/gbmint.h | 12 ++++++-- > 5 files changed, 115 insertions(+), 12 deletions(-) > > diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_ > dri.c > index 7106dc1229..1224ce4476 100644 > --- a/src/gbm/backends/dri/gbm_dri.c > +++ b/src/gbm/backends/dri/gbm_dri.c > @@ -1023,13 +1023,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); > > @@ -1087,11 +1094,24 @@ 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) { > This logic still isn't correct. As written, it will always call createImageWithModifiers if it's available even if no modifiers are provided. I think you want the "if (modifiers)" to be the outside bit of control-flow > + fprintf(stderr, "Modifiers specified, but DRI is too old\n"); > + errno = ENOSYS; > + goto failed; > + } > + > + 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); > + } > if (bo->image == NULL) > goto failed; > > @@ -1165,19 +1185,44 @@ 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_device *dri = gbm_dri_device(gbm); > struct gbm_dri_surface *surf; > > + if (modifiers && > + (!dri->image || dri->image->base.version < 14 || > + !dri->image->createImageWithModifiers)) { > + errno = ENOSYS; > + return NULL; > + } > + > surf = calloc(1, sizeof *surf); > - if (surf == NULL) > + if (surf == NULL) { > + errno = ENOMEM; > return NULL; > + } > > surf->base.gbm = gbm; > surf->base.width = width; > surf->base.height = height; > surf->base.format = format; > surf->base.flags = flags; > + if (!modifiers) { > + assert(!count); > + return &surf->base; > + } > + > + surf->base.modifiers = calloc(count, sizeof(*modifiers)); > + if (count && !surf->base.modifiers) { > + errno = ENOMEM; > + free(surf); > + return NULL; > + } > + > + surf->base.count = count; > + memcpy(surf->base.modifiers, modifiers, count * sizeof(*modifiers)); > > return &surf->base; > } > @@ -1187,6 +1232,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 81da1e0793..0550baddc4 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 afcca63da3..7bacd8b86a 100644 > --- a/src/gbm/main/gbm.c > +++ b/src/gbm/main/gbm.c > @@ -369,9 +369,28 @@ 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; > + } > + > + if ((count && !modifiers) || (modifiers && !count)) { > + errno = EINVAL; > + return NULL; > + } > Is modifiers == NULL ever allowed? If so, what does it mean? If the current mechanism is going to work, modifiers == NULL needs to have exactly the same meaning as flags == 0. > + > + return gbm->bo_create(gbm, width, height, format, 0, modifiers, count); > +} > /** > * Create a gbm buffer object from an foreign object > * > @@ -477,7 +496,23 @@ 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) > +{ > + if ((count && !modifiers) || (modifiers && !count)) { > + errno = EINVAL; > + return NULL; > + } > + > + 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 e3e5d34d97..5f588dab58 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 a6541d91c5..d8c9f6e5d7 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); > @@ -114,6 +118,10 @@ struct gbm_surface { > uint32_t height; > uint32_t format; > uint32_t flags; > + struct { > + uint64_t *modifiers; > + unsigned count; > + }; > }; > > struct gbm_backend { > -- > 2.12.0 > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
