[Mesa-dev] [PATCH 07/34] gbm: Export a per plane getter for offset
Ben Widawsky
ben at bwidawsk.net
Sat Feb 4 06:29:55 UTC 2017
On 17-01-31 11:40:04, Jason Ekstrand wrote:
>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.
>
Yeah, that looks better to me too. Thanks.
>This question also applies to the previous patch.
>
Oops. I realized I didn't cleanup the new DRIimage in 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
>>
--
Ben Widawsky, Intel Open Source Technology Center
More information about the mesa-dev
mailing list