I made a few trivial comments (mostly just asking for plane == 0 assertions more places). With that, these 5 are
Reviewed-by: Jason Ekstrand <[email protected]> On Wed, Mar 8, 2017 at 3:09 PM, Jason Ekstrand <[email protected]> wrote: > On Tue, Mar 7, 2017 at 9:31 PM, Ben Widawsky <[email protected]> wrote: > >> Unlike stride, there was no previous offset getter, so it can be right >> on the first try. >> >> v2: Return EINVAL when plane is greater than total planes to make it >> match the similar APIs. >> Avoid leak after fromPlanar (Daniel) >> Make sure when getting offsets we consider dumb images (Daniel) >> >> v3: Use Jason's recommendation for handling the non-planar case. >> >> v4: Return int64_t so we can get real errors >> >> Signed-off-by: Ben Widawsky <[email protected]> >> Reviewed-by: Eric Engestrom <[email protected]> >> Acked-by: Daniel Stone <[email protected]> >> --- >> src/gbm/backends/dri/gbm_dri.c | 33 +++++++++++++++++++++++++++++++++ >> src/gbm/gbm-symbols-check | 1 + >> src/gbm/main/gbm.c | 15 +++++++++++++++ >> src/gbm/main/gbm.h | 3 +++ >> src/gbm/main/gbmint.h | 1 + >> 5 files changed, 53 insertions(+) >> >> diff --git a/src/gbm/backends/dri/gbm_dri.c >> b/src/gbm/backends/dri/gbm_dri.c >> index 56a976fe78..b53f52f8df 100644 >> --- a/src/gbm/backends/dri/gbm_dri.c >> +++ b/src/gbm/backends/dri/gbm_dri.c >> @@ -699,6 +699,38 @@ gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane) >> return (uint32_t)stride; >> } >> >> +static int64_t >> +gbm_dri_bo_get_offset(struct gbm_bo *_bo, int plane) >> +{ >> + struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm); >> + struct gbm_dri_bo *bo = gbm_dri_bo(_bo); >> + int offset = 0; >> + >> + if (!dri->image || dri->image->base.version < 13 || >> !dri->image->fromPlanar) { >> + errno = ENOSYS; >> + return -1; >> + } >> + >> + if (plane >= get_number_planes(dri, bo->image)) { >> + errno = EINVAL; >> + return -2; >> + } >> + >> + /* Dumb images have no offset */ >> + if (!bo->image) >> > > assert(plane == 0); > > >> + return 0; >> + >> + __DRIimage *image = dri->image->fromPlanar(bo->image, plane, NULL); >> + if (image) { >> + dri->image->queryImage(image, __DRI_IMAGE_ATTRIB_OFFSET, &offset); >> + dri->image->destroyImage(image); >> + } else { >> > > assert(plane == 0); > > >> + dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_OFFSET, >> &offset); >> + } >> + >> + return (uint32_t)offset; >> +} >> + >> static void >> gbm_dri_bo_destroy(struct gbm_bo *_bo) >> { >> @@ -1194,6 +1226,7 @@ dri_device_create(int fd) >> dri->base.base.bo_get_planes = gbm_dri_bo_get_planes; >> 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_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 459006a63f..7ff78ab400 100755 >> --- a/src/gbm/gbm-symbols-check >> +++ b/src/gbm/gbm-symbols-check >> @@ -16,6 +16,7 @@ gbm_bo_get_height >> gbm_bo_get_stride >> gbm_bo_get_stride_for_plane >> gbm_bo_get_format >> +gbm_bo_get_offset >> gbm_bo_get_device >> gbm_bo_get_handle >> gbm_bo_get_fd >> diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c >> index 0a9f0bef7e..afcca63da3 100644 >> --- a/src/gbm/main/gbm.c >> +++ b/src/gbm/main/gbm.c >> @@ -194,6 +194,21 @@ gbm_bo_get_format(struct gbm_bo *bo) >> return bo->format; >> } >> >> +/** Get the offset for the data of the specified plane >> + * >> + * Extra planes, and even the first plane, may have an offset from the >> start of >> + * the buffer object. This function will provide the offset for the >> given plane >> + * to be used in various KMS APIs. >> + * >> + * \param bo The buffer object >> + * \return The offset >> > > Maybe a comment about < 0 on error? > > >> + */ >> +GBM_EXPORT int64_t >> > > I find the int64_t a bit ugly given the rest of the API, but I also can't > really see any better options. I guess we could say (uint32_t)-1 is an > invalid offset (it almost certainly is) and use that. In any case, int64_t > is just fine. > > >> +gbm_bo_get_offset(struct gbm_bo *bo, int plane) >> +{ >> + return bo->gbm->bo_get_offset(bo, plane); >> +} >> + >> /** Get the gbm device used to create the buffer object >> * >> * \param bo The buffer object >> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h >> index 1719c5312a..e3e5d34d97 100644 >> --- a/src/gbm/main/gbm.h >> +++ b/src/gbm/main/gbm.h >> @@ -309,6 +309,9 @@ gbm_bo_get_stride_for_plane(struct gbm_bo *bo, int >> plane); >> uint32_t >> gbm_bo_get_format(struct gbm_bo *bo); >> >> +int64_t >> +gbm_bo_get_offset(struct gbm_bo *bo, int plane); >> + >> struct gbm_device * >> gbm_bo_get_device(struct gbm_bo *bo); >> >> diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h >> index 26d18bab6b..a6541d91c5 100644 >> --- a/src/gbm/main/gbmint.h >> +++ b/src/gbm/main/gbmint.h >> @@ -79,6 +79,7 @@ struct gbm_device { >> int (*bo_get_planes)(struct gbm_bo *bo); >> union gbm_bo_handle (*bo_get_handle)(struct gbm_bo *bo, int plane); >> uint32_t (*bo_get_stride)(struct gbm_bo *bo, int plane); >> + int64_t (*bo_get_offset)(struct gbm_bo *bo, int plane); >> void (*bo_destroy)(struct gbm_bo *bo); >> >> struct gbm_surface *(*surface_create)(struct gbm_device *gbm, >> -- >> 2.12.0 >> >> >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
