[Mesa-dev] [PATCH v2 06/11] gbm: Check harder for supported formats

[Emil Velikov]

On 13 July 2017 at 17:07, Jason Ekstrand <jason at jlekstrand.net> wrote:
> On July 13, 2017 4:12:53 AM Daniel Stone <daniels at collabora.com> wrote:
>
>> Luckily no-one really used the is_format_supported() call, because it
>> only supported three formats.
>>
>> Also, since buffers with alpha can be displayed on planes, stop banning
>> them from use.
>>
>> Signed-off-by: Daniel Stone <daniels at collabora.com>
>> ---
>>  src/gbm/backends/dri/gbm_dri.c | 39
>> +++++++++++++++++++++++++--------------
>>  1 file changed, 25 insertions(+), 14 deletions(-)
>>
>> v2: Fix compilation error accidentally squashed into previous patch,
>>     remove GBM_BO_FORMAT_* handling as it's now dealt with in the
>>     helper.
>>
>> diff --git a/src/gbm/backends/dri/gbm_dri.c
>> b/src/gbm/backends/dri/gbm_dri.c
>> index 99df265c6b..94d48988de 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -591,29 +591,40 @@ gbm_dri_to_gbm_format(int dri_format)
>>     return 0;
>>  }
>>
>> -
>>  static int
>>  gbm_dri_is_format_supported(struct gbm_device *gbm,
>>                              uint32_t format,
>>                              uint32_t usage)
>>  {
>> -   switch (format) {
>> -   case GBM_BO_FORMAT_XRGB8888:
>> -   case GBM_FORMAT_XBGR8888:
>> -   case GBM_FORMAT_XRGB8888:
>> -      break;
>> -   case GBM_BO_FORMAT_ARGB8888:
>> -   case GBM_FORMAT_ARGB8888:
>> -      if (usage & GBM_BO_USE_SCANOUT)
>> -         return 0;
>> -      break;
>> -   default:
>> +   struct gbm_dri_device *dri = gbm_dri_device(gbm);
>> +   int count;
>> +
>> +   if ((usage & GBM_BO_USE_CURSOR) && (usage & GBM_BO_USE_RENDERING))
>>        return 0;
>> +
>> +   if (gbm_format_to_dri_format(format) == 0)
>> +      return 0;
>> +
>> +   /* If there is no query, fall back to the small table which was
>> originally
>> +    * here. */
>> +   if (dri->image->base.version <= 15 ||
>> !dri->image->queryDmaBufModifiers) {
>> +      switch (format) {
>> +      case GBM_FORMAT_XRGB8888:
>> +      case GBM_FORMAT_ARGB8888:
>> +      case GBM_FORMAT_XBGR8888:
>
>
> What happened to the ARGB formats?
>
I think you meant the "BO_FORMAT" formats. And yes, those seems to
have disappeared and we'd want them back.

>> +         return 1;
>> +      default:
>> +         return 0;
>> +      }
>>     }
>>
>> -   if (usage & GBM_BO_USE_CURSOR &&
>> -       usage & GBM_BO_USE_RENDERING)
>> +   /* Check if the driver returns any modifiers for this format; since
>> linear
>> +    * is counted as a modifier, we will have at least one modifier for
>> any
>> +    * supported format. */
>> +   if (!dri->image->queryDmaBufModifiers(dri->screen, format, 0, NULL,
>> NULL,
>> +                                         &count) || count == 0) {
>
>
> The "|| count == 0" here really threw me off.  It could be dropped in favor
> of "return count != 0;".  .it sure if that's really better though.
>
We'll need to initialise count since it remains unchanged, when the
function call fails.

Suggestion seems simpler/easier to read.

Thanks
Emil