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

[Ben Widawsky]

On 17-01-09 15:53:06, Jason Ekstrand wrote:
>On Mon, Jan 9, 2017 at 3:50 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>> On Mon, Jan 2, 2017 at 6:36 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
>>
>>> 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 | 26 +++++++++++++++++++++++++-
>>>  src/gbm/gbm-symbols-check      |  1 +
>>>  src/gbm/main/gbm.c             | 15 ++++++++++++++-
>>>  src/gbm/main/gbm.h             |  3 +++
>>>  4 files changed, 43 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gbm/backends/dri/gbm_dri.c
>>> b/src/gbm/backends/dri/gbm_dri.c
>>> index 9b23299889..1b79682282 100644
>>> --- a/src/gbm/backends/dri/gbm_dri.c
>>> +++ b/src/gbm/backends/dri/gbm_dri.c
>>> @@ -652,7 +652,31 @@ gbm_dri_bo_get_handle_for_plane(struct gbm_bo *_bo,
>>> int plane)
>>>  static uint32_t
>>>  gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane)
>>>  {
>>> -   return _bo->stride;
>>> +   struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
>>> +   struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
>>> +   __DRIimage *image;
>>> +   int stride = 0;
>>> +
>>> +   if (!dri->image || dri->image->base.version < 11 ||
>>> !dri->image->fromPlanar) {
>>>
>>
>I'm not so sure we want this.  We just implicitly created a dependency on
>all these things for every gbm_bo_get_stride() call.  If we don't have
>these things, we probably want to check that plane == 0 and return
>_bo->stride like before.  If plane > 0 then we want ENOSYS or EINVAL but
>for plane == 0, it should always work.
>

Yes you're right. This was based on some different code earlier. Thanks.

Here is what I got from your two comments.

diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
index fc61c07166..656fef3481 100644
--- a/src/gbm/backends/dri/gbm_dri.c
+++ b/src/gbm/backends/dri/gbm_dri.c
@@ -659,6 +659,10 @@ gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane)
    __DRIimage *image;
    int stride = 0;

+   /* Preserve legacy behavior if plane is 0 */
+   if (plane == 0)
+      _bo->stride
+
    if (!dri->image || dri->image->base.version < 11 || !dri->image->fromPlanar) {
       errno = ENOSYS;
       return 0;
@@ -667,8 +671,10 @@ gbm_dri_bo_get_stride(struct gbm_bo *_bo, int plane)
    if (bo->image == NULL)
       return _bo->stride;

-   if (plane >= get_number_planes(dri, bo->image))
+   if (plane >= get_number_planes(dri, bo->image)) {
+      ERRNO = EINVAL;
       return 0;
+   }

    image = dri->image->fromPlanar(bo->image, plane, NULL);
    if (!image) {

>
>> +      errno = ENOSYS;
>>> +      return 0;
>>> +   }
>>> +
>>> +   if (bo->image == NULL)
>>> +      return _bo->stride;
>>> +
>>> +   if (plane >= get_number_planes(dri, bo->image))
>>> +      return 0;
>>>
>>
>> again, EINVAL?
>>
>>
>>> +
>>> +   image = dri->image->fromPlanar(bo->image, plane, NULL);
>>> +   if (!image) {
>>> +      /* Use the parent stride */
>>> +      image = bo->image;
>>> +   }
>>> +
>>> +   dri->image->queryImage(image, __DRI_IMAGE_ATTRIB_STRIDE, &stride);
>>> +
>>> +   return (uint32_t)stride;
>>>  }
>>>
>>>  static void
>>> diff --git a/src/gbm/gbm-symbols-check b/src/gbm/gbm-symbols-check
>>> index 1e6dd4d3ec..459006a63f 100755
>>> --- a/src/gbm/gbm-symbols-check
>>> +++ b/src/gbm/gbm-symbols-check
>>> @@ -14,6 +14,7 @@ gbm_bo_unmap
>>>  gbm_bo_get_width
>>>  gbm_bo_get_height
>>>  gbm_bo_get_stride
>>> +gbm_bo_get_stride_for_plane
>>>  gbm_bo_get_format
>>>  gbm_bo_get_device
>>>  gbm_bo_get_handle
>>> diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
>>> index 7462e90c4c..0a9f0bef7e 100644
>>> --- a/src/gbm/main/gbm.c
>>> +++ b/src/gbm/main/gbm.c
>>> @@ -165,7 +165,20 @@ gbm_bo_get_height(struct gbm_bo *bo)
>>>  GBM_EXPORT uint32_t
>>>  gbm_bo_get_stride(struct gbm_bo *bo)
>>>  {
>>> -   return bo->gbm->bo_get_stride(bo, 0);
>>> +   return gbm_bo_get_stride_for_plane(bo, 0);
>>> +}
>>> +
>>> +/** Get the stride for the given plane
>>> + *
>>> + * \param bo The buffer object
>>> + * \param plane for which you want the stride
>>> + *
>>> + * \sa gbm_bo_get_stride()
>>> + */
>>> +GBM_EXPORT uint32_t
>>> +gbm_bo_get_stride_for_plane(struct gbm_bo *bo, int plane)
>>> +{
>>> +   return bo->gbm->bo_get_stride(bo, plane);
>>>  }
>>>
>>>  /** Get the format of the buffer object
>>> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
>>> index 67548206c4..1719c5312a 100644
>>> --- a/src/gbm/main/gbm.h
>>> +++ b/src/gbm/main/gbm.h
>>> @@ -304,6 +304,9 @@ uint32_t
>>>  gbm_bo_get_stride(struct gbm_bo *bo);
>>>
>>>  uint32_t
>>> +gbm_bo_get_stride_for_plane(struct gbm_bo *bo, int plane);
>>> +
>>> +uint32_t
>>>  gbm_bo_get_format(struct gbm_bo *bo);
>>>
>>>  struct gbm_device *
>>> --
>>> 2.11.0
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>
>>