[Mesa-dev] [PATCH kmscube] formats: use weston's egl config matching logic, move format defines up

[Ilia Mirkin]

On Thu, Feb 8, 2018 at 6:28 AM, Daniel Stone <daniel at fooishbar.org> wrote:
> Hi Ilia,
>
> On 8 February 2018 at 04:17, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> The GBM surface format has to match the DRM mode. Both are used in a
>> couple of places, so move the defines to a common place so that they can
>> be adjusted easily.
>
> Thanks for doing this Ilia! Hopefully people stop copying broken code
> now. :) Especially as eglChooseConfig will happily ignore a visual ID
> specification and choose something completely incorrect.
>
>> @@ -78,7 +78,7 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
>>         }
>>
>>         ret = drmModeAddFB2WithModifiers(drm_fd, width, height,
>> -                       DRM_FORMAT_XRGB8888, handles, strides, offsets,
>> +                       DRM_FORMAT, handles, strides, offsets,
>>                         modifiers, &fb->fb_id, flags);
>>  #endif
>>         if (ret) {
>> @@ -88,7 +88,7 @@ struct drm_fb * drm_fb_get_from_bo(struct gbm_bo *bo)
>>                 memcpy(handles, (uint32_t [4]){gbm_bo_get_handle(bo).u32,0,0,0}, 16);
>>                 memcpy(strides, (uint32_t [4]){gbm_bo_get_stride(bo),0,0,0}, 16);
>>                 memset(offsets, 0, 16);
>> -               ret = drmModeAddFB2(drm_fd, width, height, DRM_FORMAT_XRGB8888,
>> +               ret = drmModeAddFB2(drm_fd, width, height, DRM_FORMAT,
>
> Nitpick: use gbm_bo_get_format(bo) for both of these. With that:

That'd just come back with a GBM_FORMAT_* no? I need a DRM_FORMAT_*
here. And it might not 1:1 correspond to the GBM format, e.g. I might
want an alpha-having GBM format and a non-alpha DRM format.

But, the point is well taken and I think this applies to where I run
egl_choose_config -- that should take the gbm_bo_get_format() rather
than using GBM_FORMAT a second time. (And ideally gbm->surface has
that in it -- didn't check the API yet.)

> Reviewed-by: Daniel Stone <daniels at collabora.com>
>
> Cheers,
> Daniel