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

[Ben Widawsky]

On 17-03-07 08:28:09, Jason Ekstrand wrote:
>On Mon, Mar 6, 2017 at 6:37 PM, Ben Widawsky <ben at bwidawsk.net> wrote:
>
>> v2: Make the error return be -1 instead of 0 because I think 0 is
>> actually valid.
>>
>> v3: Set errno to EINVAL when the specified plane is above the total
>> planes. (Jason Ekstrand)
>> Return the bo's handle if there is no image ie. for dumb images like
>> cursor (Daniel)
>>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> Acked-by: Daniel Stone <daniels at collabora.com>
>> ---
>>  src/gbm/backends/dri/gbm_dri.c | 35 +++++++++++++++++++++++++++++++++++
>>  src/gbm/gbm-symbols-check      |  1 +
>>  src/gbm/main/gbm.c             | 18 ++++++++++++++++++
>>  src/gbm/main/gbm.h             |  3 +++
>>  src/gbm/main/gbmint.h          |  1 +
>>  5 files changed, 58 insertions(+)
>>
>> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_
>> dri.c
>> index 0b75e411df..c3704e505b 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -624,6 +624,40 @@ gbm_dri_bo_get_planes(struct gbm_bo *_bo)
>>     return get_number_planes(dri, bo->image);
>>  }
>>
>> +static union gbm_bo_handle
>> +gbm_dri_bo_get_handle_for_plane(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);
>> +   union gbm_bo_handle ret;
>> +   ret.s32 = -1;
>> +
>> +   if (!dri->image || dri->image->base.version < 13 ||
>> !dri->image->fromPlanar) {
>> +      errno = ENOSYS;
>> +      return ret;
>> +   }
>> +
>> +   if (plane >= get_number_planes(dri, bo->image)) {
>> +      errno = EINVAL;
>> +      return ret;
>> +   }
>> +
>> +   if (!bo->image) {
>> +      ret.s32 = bo->handle;
>> +      return ret;
>> +   }
>> +
>> +   __DRIimage *image = dri->image->fromPlanar(bo->image, plane, NULL);
>> +   if (!image) {
>> +      /* Use the parent's handle */
>> +      image = bo->image;
>>
>
>Assuming get_number_of_planes does the right thing, I think this is
>correct.  Would it make sense to add an assert(plane == 0) to this error
>case and the one above?
>
>

Let me claim ignorance...

For "the one above": I thought it might be possible to have a planar dumb bo, in
which case, the assertion wouldn't always hold. However, I have other code here
which prevents that, so I think I'll modify the commit message, add some
comments, and most importantly make a patch before this which prevents dumb BOs
from being planar formats.

For the !image case: this assertion should also hold, but since fromPlanar can
be implemented however by the DRI implementation, I don't know if it's the right
thing to add assertions in that path. I'm fine with adding it, I just claim some
level of ignorance.

[snip]