[Mesa-dev] [PATCH 0/6] i965: Add RGBX, RGBA configs, even on gen9

[Rob Herring]

On Tue, Jun 13, 2017 at 1:55 PM, Chad Versace <chadversary at chromium.org> wrote:
> On Fri 09 Jun 2017, Tapani Pälli wrote:
>>
>>
>> On 06/08/2017 09:27 PM, Chad Versace wrote:
>> > On Thu 08 Jun 2017, Tomasz Figa wrote:
>> > > On Thu, Jun 8, 2017 at 4:08 PM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> > > >
>> > > > On 06/08/2017 09:36 AM, Tapani Pälli wrote:
>> > > > >
>> > > > >
>> > > > >
>> > > > > On 06/08/2017 06:05 AM, Tomasz Figa wrote:
>> > > > > >
>> > > > > > On Wed, Jun 7, 2017 at 5:36 AM, Chad Versace <chadversary at chromium.org>
>> > > > > > wrote:
>> > > > > > >
>> > > > > > > More patches to break your formats... again ;)
>> > > > > > >
>> > > > > > > The Android framework requires support for EGLConfigs with
>> > > > > > > HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888. This prevents
>> > > > > > > Chrome OS from updating its Android drivers, because earlier this year
>> > > > > > > Intel disabled all rgbx formats for gen >=9 in brw_surface_formats.c.
>> > > > > > > This patch series safely (hopefully?) fixes that problem.
>> > > > > > >
>> > > > > > > If you want the meat, read patches 2 and 6.
>> > > > > > >
>> > > > > > > Chad Versace (6):
>> > > > > > >     mesa: Add _mesa_format_fallback_rgba_to_rgbx()
>> > > > > > >     i965: Add a RGBX->RGBA fallback for glEGLImageTextureTarget2D()
>> > > > > > >     i965: Rename some vague format members of brw_context
>> > > > > > >     i965/dri: Add intel_screen param to intel_create_winsys_renderbuffer
>> > > > > > >     i965: Move brw_context format arrays to intel_screen
>> > > > > > >     i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
>> > > > > >
>> > > > > >
>> > > > > > Thanks a lot Chad!
>> > > > > >
>> > > > > > Just to make sure, did you have a chance to test it with X11 apps,
>> > > > > > that were reported to have incorrect colors last time we tried
>> > > > > > enabling these formats? I.e.
>> > > > > > https://bugs.freedesktop.org/show_bug.cgi?id=95071
>> >
>> > tfiga, I only tested with glxgears. To be safe, I'll also test the KDE
>> > apps in that bug report and reply back with the results.
>> >
>> > > > > I just tested this set on Android and I'm getting wrong colors with this,
>> > > > > red and blue swapped.
>> >
>> > Uh oh.
>> >
>> > I did something dumb. When testing this on ARC++, I chose
>> > a BLACK-AND-WHITE game! I'll retest with a color game.
>>
>> It was pretty hard to keep coffee in mouth when reading this comment.
>>
>> > https://play.google.com/store/apps/details?id=com.ZephirothGames.AsteroidStormFREE&hl=en
>> >
>> > > > I can 'fix' this by reordering the configs in intel_screen_make_configs so
>> > > > that the new configs (RGBA, RGBX) required by Android are before the old
>> > > > ones (BGRA, BGRX).
>> > >
>> > > I think that would have exactly the opposite effect on the X11 apps I
>> > > mentioned before. In my testing, they were sensitive to the order of
>> > > configs, due to component bit masks not being considered in selection,
>> > > only bit depths.
>> >
>> > Yes.
>> >
>> > > However, for Android, I thought EGL already used bit masks, so
>> > > possibly there is still an unrelated bug somewhere.
>> >
>> > Android certainly inspects the channel masks, using dri2_add_config().
>> > So I'm also curious why Tapani sees swapped channels. The
>> > platform_android.c is patched with format hacks, and I assume the same
>> > is true for android-ia. Maybe the hacks are intefering somehow. I'll
>> > investigate.
>>
>> We shouldn't have related 'hacks', but here are some changes that might be
>> interesting or somehow related:
>>
>> add EGL_ALPHA_SIZE attrib:
>>
>> https://github.com/android-ia/frameworks-native/commit/8237c9f8eb36d4f9ead40c8cb4a68ae9518d3c9f
>>
>> sorting display configs:
>>
>> https://github.com/android-ia/frameworks-native/pull/2/commits/bb29af0777e747effacd239565f52aad96c45558
>>
>> visuals being added:
>>
>> https://github.com/android-ia/external-mesa/commit/ceac31ff7e53ec5034bc379d37ce365c000e9e4a
>
> I confirmed that this series causes no regressions with Gnome and KDE
> apps. For KDE, I used Amarok as the test app. To prove to myself
> I *really* was testing this series, I tested the series with the new
> formats placed at the end of the array, and then I tested again with the new
> formats placed at the start of the array. As expected, Amarok's colors
> were correct in the first test, and were wrong in the second test (blue
> and red were swapped.
>
> I still haven't succeeded in fully testing these patches against
> Android, for a combination of technical and non-technical reasons. I'm
> travelling this week, so I will probably be able to resume testing
> Android apps on Thursday.

While not i965, I tested the similar code and change in gallium. With
the RGBA/RGBX formats at the end of the list I get:

EGL-MAIN: Native format mismatch: 0x1 != 0x5

That's coming from SurfaceFlinger.

Rob