[Mesa-dev] [PATCH 07/34] gbm: Export a per plane getter for offset

[Jason Ekstrand]

On Mon, Jan 23, 2017 at 10:19 PM, Ben Widawsky <ben at 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 <ben at bwidawsk.net>
> Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>
> Acked-by: Daniel Stone <daniels at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170131/8a40f4c6/attachment.html>