On Mon, Jan 23, 2017 at 10:19 PM, Ben Widawsky <b...@bwidawsk.net> 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)
>
> 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 | 36 ++++++++++++++++++++++++++++++++++++
> 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, 56 insertions(+)
>
> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_
> dri.c
> index 395f78e851..43f8e15e62 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -697,6 +697,41 @@ gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane)
> return (uint32_t)stride;
> }
>
> +static uint32_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 0;
> + }
> +
> + if (plane >= get_number_planes(dri, bo->image)) {
> + errno = EINVAL;
> + return 0;
> + }
> +
> + /* Dumb images have no offset */
> + if (!bo->image)
> + return 0;
> +
> + __DRIimage *image = dri->image->fromPlanar(bo->image, plane, NULL);
> + if (!image) {
> + /* Use the parent offset */
> + image = bo->image;
> + }
> +
> + dri->image->queryImage(image, __DRI_IMAGE_ATTRIB_OFFSET, &offset);
> +
> + if (image != bo->image)
> + dri->image->destroyImage(image);
>
It's a bit unclear to me how this if is related to the one above. Is it
possible for fromPlanar to return bo->image or does fromPlanar always
create a new image? Would this be more clear:
__DRIimage *plane_image = dri->image->fromPlanar(bo->image, plane, NULL);
if (plane_image) {
dri->image->queryImage(plane_image, __DRI_IMAGE_ATTRIB_OFFSET, &offset);
dri->image->destroyImage(plane_image);
} else {
dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_OFFSET, &offset);
}
It appears to me as if that's what you're trying to accomplish but I don't
really understand the ownership semantics of fromPlanar.
This question also applies to the previous patch.
> +
> + return (uint32_t)offset;
> +}
> +
> static void
> gbm_dri_bo_destroy(struct gbm_bo *_bo)
> {
> @@ -1157,6 +1192,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..295f6894eb 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
> + */
> +GBM_EXPORT uint32_t
> +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..b089359b01 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);
>
> +uint32_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..ac6078361a 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);
> + uint32_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.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
