[Mesa-dev] [PATCH] i915: Add XRGB8888 format to intel_screen_make_configs

[Emil Velikov]

On 24 November 2016 at 06:22, Boyan Ding <boyan.j.ding at gmail.com> wrote:
> 2016-11-24 13:29 GMT+08:00 Derek Foreman <derekf at osg.samsung.com>:
>> On 23/11/16 07:18 PM, Boyan Ding wrote:
>>>
>>> 2016-11-24 7:01 GMT+08:00 Derek Foreman <derekf at osg.samsung.com>:
>>>>
>>>> This is a copy of commit 536003c11e4cb1172c540932ce3cce06f03bf44e
>>>> except for i915.
>>>>
>>>> Original log for the i965 commit follows:
>>>>
>>>>  Some application, such as drm backend of weston, uses XRGB8888 config as
>>>>  default. i965 doesn't provide this format, but before commit 65c8965d,
>>>>  the drm platform of EGL takes ARGB8888 as XRGB8888. Now that commit
>>>>  65c8965d makes EGL recognize format correctly so weston won't start
>>>>  because it can't find XRGB8888. Add XRGB8888 format to i965 just as
>>>>  other drivers do.
>>>>
>>>> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>>>> ---
>>>>  src/mesa/drivers/dri/i915/intel_screen.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/mesa/drivers/dri/i915/intel_screen.c
>>>> b/src/mesa/drivers/dri/i915/intel_screen.c
>>>> index 1b80df0..5c7c06a 100644
>>>> --- a/src/mesa/drivers/dri/i915/intel_screen.c
>>>> +++ b/src/mesa/drivers/dri/i915/intel_screen.c
>>>> @@ -1044,7 +1044,8 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>>>>  {
>>>>     static const mesa_format formats[] = {
>>>>        MESA_FORMAT_B5G6R5_UNORM,
>>>> -      MESA_FORMAT_B8G8R8A8_UNORM
>>>> +      MESA_FORMAT_B8G8R8A8_UNORM,
>>>> +      MESA_FORMAT_B8G8R8X8_UNORM
>>>>     };
>>>>
>>>>     /* GLX_SWAP_COPY_OML is not supported due to page flipping. */
>>>> --
>>>> 2.10.2
>>>>
>>>
>>> Hi Derek,
>>>
>>> I sent exactly the same patch one and half years ago at [1], but
>>> withdrew it because it seems no one got interested in that and I don't
>>> have the hardware to test. If you're sure it is correct, this gets my
>>>
>>> Acked-by: Boyan Ding <boyan.j.ding at gmail.com>
>>
>>
>> I'm sorry, I didn't see your patch.  It makes more sense to me that I give
>> you my RB on that patch in place of your Ack on mine.  I don't want to take
>> credit for a problem you solved over a year ago. :)
>>
>> I don't have appropriate hardware but this has been tested for me by an
>> Enlightenment user who was unable to use our GL backend because it's trying
>> to use XRGB.
>>
>> Weston ran for him but logged a warning about falling back to an ARGB
>> visual, which led me to the discovery that this had only been changed for
>> i965.
>>
>> I'll try to get him to reply with a "Tested-by" tomorrow.
>>
>>
>
> Thanks for the kind reply, but I wonder if a more thorough testing
> should be done before applying this. I remember its i965 counterpart
> (commit 28090b30d) did cause some problem (Bug 90791) although it seems
> not the fault on its own.
>
> We're facing the same problem I faced last year when posting this
> patch -- I didn't have the authority to say that it was okay, and I
> didn't have the hardware to test on as Emil once suggested[1].
>
Precisely - the patch on it's own was perfectly reasonable but it will
likely cause issues like i965 one.

Derek, skimming through the i965 history a very lengthy list of
regressions/fixes comes up[1] as the format was advertised as
supported.

Considering we don't get much testing, it would be great to avoid
breaking the world by fixing EGL/drm users (Wayland compositors,
other).

Ian, Mark Janes,
Gents, do you have the hardware/chance to test this patch ?

Thanks
Emil

[1]
28090b30dd6b5977de085f48c620574214b6b4ba i965: Add XRGB8888 format to
intel_screen_make_configs
* Adds the format since things broke as EGL/drm was fixed to correctly
honour the formats commit 65c8965d033 (egl: Take alpha bits into
account when selecting GBM formats)

8da79b8378ae87474d8c47ad955e4833edf98359 i965: Fix HW blitter pitch limits
c2d0606827412b710dcaed80268fc665de8c9c5d i915: Blit RGBX<->RGBA
drawpixels // should read i965
922c0c9fd526ce19b87bc74a3159dec7705c1de1 i965: Export format
comparison for blitting between miptrees
* Fixes serious performance issue due to the extra format.

bd38f91f8d80897ca91979962d80d4bc0acef586 i965: do_blit_drawpixels:
decode array formats
* Fixes the last commit above

2cebaac479d49cd6df4e97b466ba14bab3f30db1 i965: Don't use tiled_memcpy
to download from RGBX or BGRX surfaces
* Disable those since it causes regressions

3f10774cbab1e803f8aa3d6d24f8f6f98b8128c3 i965: Check base format to
determine whether to use tiled memcpy
* Aims to rework/fix the above.

c769efda939e06338d41e1046a5f954c690951d5 i965: Add
MESA_FORMAT_B8G8R8X8_SRGB to brw_format_for_mesa_format
43f4be5f06b7a96b96a3a7b43f5112139a1f423a i965: Add B8G8R8X8_SRGB to
the alpha format override
839793680f99b8387bee9489733d5071c10f3ace i965: Use
MESA_FORMAT_B8G8R8X8_SRGB for RGB visuals
* Aim to rework/fix the mesa <> hardware format mappings as a side
effect of the new XRGB8888 advertised

61cdb7665f7bd147533cdc5977750d990c2eafd5 Revert "i965: Use
MESA_FORMAT_B8G8R8X8_SRGB for RGB visuals"
* Revert the last one since it breaks kde/kwin (and others iirc)

35ade36c88e5aaa0b18c3cc911d9a4de3a60a80b dri/i965: fix incorrect
rgbFormat in intelCreateBuffer().
* Correctly select the correct (mesa) format based on the loader
request -> UNORM vs SRGB, XRGB vs ARGB and *RGB vs *BGR