Re: [Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
On 17-03-10 11:32:35, Emil Velikov wrote: On 10 March 2017 at 01:48, Ben Widawskywrote: 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 Cc: Jason Ekstrand References (v4): https://lists.freedesktop.org/archives/intel-gfx/2017-January/116636.html Signed-off-by: Ben Widawsky Reviewed-by: Eric Engestrom (v1) Acked-by: Daniel Stone --- src/egl/drivers/dri2/platform_drm.c | 19 --- Worth splitting the patches - patch N+1 src/gbm/backends/dri/gbm_dri.c | 42 ++-- src/gbm/gbm-symbols-check| 2 ++ src/gbm/main/gbm.c | 29 -- src/gbm/main/gbm.h | 12 + src/gbm/main/gbmint.h| 12 +++-- Patch N src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ Patch N+Y --- 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)); + Similar to last patch - XOR ? I don't believe so, I still think NAND is right, but I guess I can't say indisputably that dri_use == 0 is valid. if (usage & GBM_BO_USE_WRITE || dri->image == NULL) return create_dumb(gbm, width, height, format, usage); @@ -1087,11 +1094,21 @@ 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); With the "modifiers or use" above how do we get sane value for dri_use in this fall-back ? Seems like things we'll miss/ignore the scanout/curson/linear flags. Perhaps it's worth simply bailing out ? Yes, I think you're right. Jason said basically the same thing, but the way you've presented it, it's more obvious to me that baling out is the right thing to do. So I've changed this locally. Same thing basically for gbm_dri_surface_create() --- a/src/gbm/gbm-symbols-check +++ b/src/gbm/gbm-symbols-check +gbm_bo_create_with_modifiers +gbm_surface_create_with_modifiers Thank you :-) +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; + } + Should we validate modifiers and/or count at this level as well ? Yeah, I suppose that makes sense. I got this change locally. I honestly think almost no validation should be done at this level so that the implementation can do whatever weird stuff it wants to do,
Re: [Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
On 17-03-09 18:52:52, Jason Ekstrand wrote: On Thu, Mar 9, 2017 at 5:48 PM, Ben Widawskywrote: 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 Cc: Jason Ekstrand References (v4): https://lists.freedesktop.org/ archives/intel-gfx/2017-January/116636.html Signed-off-by: Ben Widawsky Reviewed-by: Eric Engestrom (v1) Acked-by: Daniel Stone --- src/egl/drivers/dri2/platform_drm.c | 19 --- src/gbm/backends/dri/gbm_dri.c | 42 ++-- src/gbm/gbm-symbols-check| 2 ++ src/gbm/main/gbm.c | 29 -- src/gbm/main/gbm.h | 12 + src/gbm/main/gbmint.h| 12 +++-- src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ 7 files changed, 119 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(_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(_dpy->gbm_d ri->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 7106dc1229..d45ba94080 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,21 @@ 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"); This seems a bit on the sketchy side. Do we want to fail, set errno to ENOSYS, and return NULL in this case? I'm not really sure how GBM versioning works. Daniel? Kristian? I
Re: [Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
On 17-03-10 11:34:19, Jason Ekstrand wrote: On Thu, Mar 9, 2017 at 6:52 PM, Jason Ekstrandwrote: On Thu, Mar 9, 2017 at 5:48 PM, Ben Widawsky 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 Cc: Jason Ekstrand References (v4): https://lists.freedesktop.org/ archives/intel-gfx/2017-January/116636.html Signed-off-by: Ben Widawsky Reviewed-by: Eric Engestrom (v1) Acked-by: Daniel Stone --- src/egl/drivers/dri2/platform_drm.c | 19 --- src/gbm/backends/dri/gbm_dri.c | 42 ++-- src/gbm/gbm-symbols-check| 2 ++ src/gbm/main/gbm.c | 29 -- src/gbm/main/gbm.h | 12 + src/gbm/main/gbmint.h| 12 +++-- src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ 7 files changed, 119 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(_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(_dpy->gbm_d ri->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 7106dc1229..d45ba94080 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,21 @@ 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"); This seems a bit on the sketchy side. Do we want to fail, set errno to ENOSYS, and return NULL in this
Re: [Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
On Thu, Mar 9, 2017 at 6:52 PM, Jason Ekstrandwrote: > On Thu, Mar 9, 2017 at 5:48 PM, Ben Widawsky 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 >> Cc: Jason Ekstrand >> References (v4): https://lists.freedesktop.org/ >> archives/intel-gfx/2017-January/116636.html >> Signed-off-by: Ben Widawsky >> Reviewed-by: Eric Engestrom (v1) >> Acked-by: Daniel Stone >> --- >> src/egl/drivers/dri2/platform_drm.c | 19 --- >> src/gbm/backends/dri/gbm_dri.c | 42 >> ++-- >> src/gbm/gbm-symbols-check| 2 ++ >> src/gbm/main/gbm.c | 29 -- >> src/gbm/main/gbm.h | 12 + >> src/gbm/main/gbmint.h| 12 +++-- >> src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ >> 7 files changed, 119 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(_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(_dpy->gbm_d >> ri->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 7106dc1229..d45ba94080 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,21 @@ 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); >> +
Re: [Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
On 10 March 2017 at 01:48, Ben Widawskywrote: > 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 > Cc: Jason Ekstrand > References (v4): > https://lists.freedesktop.org/archives/intel-gfx/2017-January/116636.html > Signed-off-by: Ben Widawsky > Reviewed-by: Eric Engestrom (v1) > Acked-by: Daniel Stone > --- > src/egl/drivers/dri2/platform_drm.c | 19 --- Worth splitting the patches - patch N+1 > src/gbm/backends/dri/gbm_dri.c | 42 > ++-- > src/gbm/gbm-symbols-check| 2 ++ > src/gbm/main/gbm.c | 29 -- > src/gbm/main/gbm.h | 12 + > src/gbm/main/gbmint.h| 12 +++-- Patch N > src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ Patch N+Y > --- 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)); > + Similar to last patch - XOR ? > if (usage & GBM_BO_USE_WRITE || dri->image == NULL) >return create_dumb(gbm, width, height, format, usage); > > @@ -1087,11 +1094,21 @@ 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); With the "modifiers or use" above how do we get sane value for dri_use in this fall-back ? Seems like things we'll miss/ignore the scanout/curson/linear flags. Perhaps it's worth simply bailing out ? > --- a/src/gbm/gbm-symbols-check > +++ b/src/gbm/gbm-symbols-check > +gbm_bo_create_with_modifiers > +gbm_surface_create_with_modifiers Thank you :-) > +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; > + } > + Should we validate modifiers and/or count at this level as well ? > +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) > +{ Ditto ? What happens if we feed zero width/height ? Worth adding a note, even if things are
Re: [Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
On Thu, Mar 9, 2017 at 5:48 PM, Ben Widawskywrote: > 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 > Cc: Jason Ekstrand > References (v4): https://lists.freedesktop.org/ > archives/intel-gfx/2017-January/116636.html > Signed-off-by: Ben Widawsky > Reviewed-by: Eric Engestrom (v1) > Acked-by: Daniel Stone > --- > src/egl/drivers/dri2/platform_drm.c | 19 --- > src/gbm/backends/dri/gbm_dri.c | 42 > ++-- > src/gbm/gbm-symbols-check| 2 ++ > src/gbm/main/gbm.c | 29 -- > src/gbm/main/gbm.h | 12 + > src/gbm/main/gbmint.h| 12 +++-- > src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ > 7 files changed, 119 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(_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(_dpy->gbm_d > ri->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 7106dc1229..d45ba94080 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,21 @@ 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"); >
[Mesa-dev] [PATCH 2/3] gbm: Introduce modifiers into surface/bo creation
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øgsbergCc: Jason Ekstrand References (v4): https://lists.freedesktop.org/archives/intel-gfx/2017-January/116636.html Signed-off-by: Ben Widawsky Reviewed-by: Eric Engestrom (v1) Acked-by: Daniel Stone --- src/egl/drivers/dri2/platform_drm.c | 19 --- src/gbm/backends/dri/gbm_dri.c | 42 ++-- src/gbm/gbm-symbols-check| 2 ++ src/gbm/main/gbm.c | 29 -- src/gbm/main/gbm.h | 12 + src/gbm/main/gbmint.h| 12 +++-- src/mesa/drivers/dri/i965/intel_screen.c | 18 ++ 7 files changed, 119 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(_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(_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(_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 7106dc1229..d45ba94080 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,21 @@ 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,