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

Wed Mar 8 23:10:33 UTC 2017

I made a few trivial comments (mostly just asking for plane == 0 assertions
more places).  With that, these 5 are

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

On Wed, Mar 8, 2017 at 3:09 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:

> On Tue, Mar 7, 2017 at 9:31 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)
>>
>> 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 <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 | 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170308/917fbf21/attachment.html>