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

[Mark Janes]

I don't see any regressions in the automated test suites (piglit, deqp,
gl cts) with this patch.

-Mark

Emil Velikov <emil.l.velikov at gmail.com> writes:

> 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