[Mesa-dev] [PATCH v2] st/dri: add 32-bit RGBX/RGBA formats

[Chih-Wei Huang]

2017-07-26 1:24 GMT+08:00 Rob Herring <robh at kernel.org>:
> On Tue, Jul 25, 2017 at 10:15 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 25 July 2017 at 03:46, Chih-Wei Huang <cwhuang at android-x86.org> wrote:
>>> On Tue 11 Jul 2017, Rob Herring wrote:
>>>>> From: Marek Olšák <marek.olsak at amd.com>
>>>>>
>>>>> Add support for 32-bit RGBX/RGBA formats which are required for Android.
>>>>>
>>>>> The original patch (commit ccdcf91104a5) was reverted (commit
>>>>> c0c6ca40a25e) in mesa as it broke GLX resulting in swapped colors. Based
>>>>> on further investigation by Chad Versace, moving the RGBX/RGBA configs
>>>>> to the end is enough to prevent breaking GLX.
>>>>>
>>>>> The handling of RGBA/RGBX in dri_fill_st_visual is a fix from Marek
>>>>> Olšák.
>>>>>
>>>>> Cc: Eric Anholt <eric at anholt.net>
>>>>> Cc: Chad Versace <chadversary at chromium.org>
>>>>> Cc: Mauro Rossi <issor.oruam at gmail.com>
>>>>> Reviewed-by: Marek Olšák <marek.olsak at amd.com>
>>>>> Signed-off-by: Rob Herring <robh at kernel.org>
>>>>> ---
>>>
>>> Hi Rob,
>>> I'm testing this patch with your gbm_gralloc and mesa 17.1.5.
>>> Before applying this patch, the SurfaceFlinger sees
>>> the alpha=8 in RenderEngine::chooseEglConfig()
>>>
>> May want to check for patches in the Android EGL (meta) library.
>> I think, in does/did have a handful of workarounds.
>>
>> Is the Android-x86 one in sync with the one RobH uses?
>
> I double checked and I get 8-8-8-8. I'm have HWC2 enabled and
> SurfaceFlinger is unpatched master branch.

Hmm, strange.
Which hwcomposer branch did you use and
what GPU did you test?

As I explained to you (in another private email),
I'm testing QEMU x86 virgl with your branches:

http://github.com/robherring/gbm_gralloc  (master branch)
http://github.com/robherring/drm_hwcomposer (android-m branch)

If I understand your code correctly, it supports
only HWC1, no HWC2 yet.
I accidentally set HWC2=true in the first time testing
but it didn't work. Setting HWC2=false works.
(on the other hand, the chromium upstream[1] does
switch to HWC2 now, but I can't make it work yet)

About the client (SurfaceFlinger), I believe it requests
HAL_PIXEL_FORMAT_RGBA_8888 but finally
HAL_PIXEL_FORMAT_RGBX_8888 is used.

The logic I saw is:
1. In SurfaceFlinger::init(), it calls RenderEngine::create() with
    hwcFormat=HAL_PIXEL_FORMAT_RGBA_8888
   (if HWC2 is used or if HWC1 api ver >= 1.1)
2. RenderEngine::create() calls RenderEngine::chooseEglConfig()
   with format=HAL_PIXEL_FORMAT_RGBA_8888
3. chooseEglConfig() tries to find a config and prints
   the info as we see in the logcat. With this patch,
   the selected config is 8-8-8-0.
4. RenderEngine::create() saves the config to mEGLConfig
   by engine->setEGLHandles()
5. Later, new DisplayDevice::DisplayDevice() is called with
   with the selected config (mRenderEngine->getEGLConfig()).
   Then it calls eglCreateWindowSurface() with the config.
6. eglCreateWindowSurface() checks[2] the alpha attrib
   of this config to decide which format to be used.
   With this patch, alpha=0 so HAL_PIXEL_FORMAT_RGBX_8888
   is used. (if alpha > 0, HAL_PIXEL_FORMAT_RGBA_8888 is used)

The differences between you and me maybe:
* AOSP: master vs 7.1.2
* Mesa: master(?) vs 17.1.5
* GPU: ?? vs virgl

I think AOSP doesn't matter. I don't see explicit change
of the above logic in the master branch.
Does Mesa version matter? May I need more patches in 17.1.5?
About GPU, did you test virgl and still see 8-8-8-8?

About the problem of the change, it's not explicitly.
But some apps require RGBA_8888 still can't work.
An example is the screen capture apps like Screenshot touch[3].
It complains the producer output format is RGBX_8888
but RGBA_8888 is expected:

07-26 10:18:28.236  2602  2669 E ImageReader_JNI: Producer output
buffer format: 0x2, ImageReader configured format: 0x1

I expect the patch fix it but it's not.

[1]: https://chromium.googlesource.com/chromiumos/drm_hwcomposer
[2]: https://android.googlesource.com/platform/frameworks/native/+/master/opengl/libs/EGL/eglApi.cpp#484
[3]: https://play.google.com/store/apps/details?id=com.mdiwebma.screenshot


-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org