[Mesa-dev] [PATCH 03/27] gbm: Export a plane getter function

[Ben Widawsky]

On 16-12-02 17:57:40, Eric Engestrom wrote:
>On Thursday, 2016-12-01 14:09:44 -0800, Ben Widawsky wrote:
>> From: Ben Widawsky <ben at bwidawsk.net>
>>
>> This will be used by clients that need to know the number of planes
>> allocated for them on behalf of the GL or other API. The best current
>> example of this is when an extra "plane" is allocated to store
>> compression data for the primary plane.
>>
>> Cc: Daniel Stone <daniels at collabora.com>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>> ---
>>  src/gbm/backends/dri/gbm_dri.c | 25 +++++++++++++++++++++++++
>>  src/gbm/gbm-symbols-check      |  1 +
>>  src/gbm/main/gbm.c             | 10 ++++++++++
>>  src/gbm/main/gbm.h             |  3 +++
>>  src/gbm/main/gbmint.h          |  1 +
>>  5 files changed, 40 insertions(+)
>>
>> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
>> index 45cb42a..c61d56b 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -598,6 +598,30 @@ gbm_dri_bo_get_fd(struct gbm_bo *_bo)
>>     return fd;
>>  }
>>
>> +static int
>> +get_number_planes(struct gbm_dri_device *dri, __DRIimage *image)
>> +{
>> +   int num_planes = 0;
>> +   dri->image->queryImage(image, __DRI_IMAGE_ATTRIB_NUM_PLANES, &num_planes);
>> +
>> +   if (num_planes <= 0)
>> +      num_planes = 1;
>
>When __DRI_IMAGE_ATTRIB_NUM_PLANES is invalid, why hide this and
>return 1 anyway?
>

Crud. My response to this apparently never made it out. The reasoning was that I
don't quite understand the DRIimage stuff works and it seemed like a nice
fallback for cases where the query (__DRI_IMAGE_ATTRIB_NUM_PLANES) doesn't
exist. Since prior to that, everything had 1 plane. We could go with what you're
suggesting and just return an error when the query fails, but again, I think
it's always safe to assume 1 anyway, when the query fails.

I don't really care much either way, and since I can't test this on non-intel
platforms, I'm willing to change this if that's what people want.

The one real concern I have is for drivers which expect to get back a 0 for some
odd format type.


So, what's the consensus?


>> +
>> +   return num_planes;
>> +}
>> +
>> +static int
>> +gbm_dri_bo_get_planes(struct gbm_bo *_bo)
>> +{
>> +   struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
>> +   struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
>> +
>> +   if (bo->image == NULL)
>> +      return -1;
>> +
>> +   return get_number_planes(dri, bo->image);
>> +}
>> +
>>  static void
>>  gbm_dri_bo_destroy(struct gbm_bo *_bo)
>>  {
>> @@ -1055,6 +1079,7 @@ dri_device_create(int fd)
>>     dri->base.base.is_format_supported = gbm_dri_is_format_supported;
>>     dri->base.base.bo_write = gbm_dri_bo_write;
>>     dri->base.base.bo_get_fd = gbm_dri_bo_get_fd;
>> +   dri->base.base.bo_get_planes = gbm_dri_bo_get_planes;
>>     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 5a333ff..8c4da1b 100755
>> --- a/src/gbm/gbm-symbols-check
>> +++ b/src/gbm/gbm-symbols-check
>> @@ -18,6 +18,7 @@ gbm_bo_get_format
>>  gbm_bo_get_device
>>  gbm_bo_get_handle
>>  gbm_bo_get_fd
>> +gbm_bo_get_plane_count
>>  gbm_bo_write
>>  gbm_bo_set_user_data
>>  gbm_bo_get_user_data
>> diff --git a/src/gbm/main/gbm.c b/src/gbm/main/gbm.c
>> index 00113fa..b5e0316 100644
>> --- a/src/gbm/main/gbm.c
>> +++ b/src/gbm/main/gbm.c
>> @@ -223,6 +223,16 @@ gbm_bo_get_fd(struct gbm_bo *bo)
>>     return bo->gbm->bo_get_fd(bo);
>>  }
>>
>> +/** Get the number of planes for the given bo.
>> + *
>> + * \param bo The buffer object
>> + * \return The number of planes
>> + */
>> +GBM_EXPORT int
>> +gbm_bo_get_plane_count(struct gbm_bo *bo)
>> +{
>> +   return bo->gbm->bo_get_planes(bo);
>> +}
>>
>>  /** Write data into the buffer object
>>   *
>> diff --git a/src/gbm/main/gbm.h b/src/gbm/main/gbm.h
>> index efb329e..b4873ab 100644
>> --- a/src/gbm/main/gbm.h
>> +++ b/src/gbm/main/gbm.h
>> @@ -316,6 +316,9 @@ int
>>  gbm_bo_get_fd(struct gbm_bo *bo);
>>
>>  int
>> +gbm_bo_get_plane_count(struct gbm_bo *bo);
>> +
>> +int
>>  gbm_bo_write(struct gbm_bo *bo, const void *buf, size_t count);
>>
>>  void
>> diff --git a/src/gbm/main/gbmint.h b/src/gbm/main/gbmint.h
>> index cfef5ee..c6a6701 100644
>> --- a/src/gbm/main/gbmint.h
>> +++ b/src/gbm/main/gbmint.h
>> @@ -76,6 +76,7 @@ struct gbm_device {
>>     void (*bo_unmap)(struct gbm_bo *bo, void *map_data);
>>     int (*bo_write)(struct gbm_bo *bo, const void *buf, size_t data);
>>     int (*bo_get_fd)(struct gbm_bo *bo);
>> +   int (*bo_get_planes)(struct gbm_bo *bo);
>>     void (*bo_destroy)(struct gbm_bo *bo);
>>
>>     struct gbm_surface *(*surface_create)(struct gbm_device *gbm,
>> --
>> 2.10.2
>>