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

[Chih-Wei Huang]

2017-07-26 23:34 GMT+08:00 Rob Herring <robh at kernel.org>:
> On Tue, Jul 25, 2017 at 10:16 PM, Chih-Wei Huang
> <cwhuang at android-x86.org> wrote:
>> 2017-07-26 1:24 GMT+08:00 Rob Herring <robh at kernel.org>:
>>>
>>> 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?
>
> The hwc2 branch with virgl. There's also some kernel changes with
> fence support for virgl needed[1].

Oh, I didn't notice the hwc2 branch. I still follow your wiki
https://github.com/robherring/generic_device/wiki/Building
which points to the android-m branch. (which is HWC1)

What kernel changes? (did you forget to list the link?)

>> 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)
>
> Upstream does not yet support HWC2. The patches for support are under
> review on Gerrit.

I saw they have switched to HWC2, say
https://chromium.googlesource.com/chromiumos/drm_hwcomposer/+/ac8741504befec1d8aa2067a6eb5c2088bc84160
https://chromium.googlesource.com/chromiumos/drm_hwcomposer/+/ed2ec4b0b3dc739fc0fd3427b2b7bbbb40cc0de3

Maybe it's not complete so not work for me?
(or I missed kernel patches?)

> But I don't think this is a HWC1 vs. HWC2 issue. The EGL config code
> is the same.
>
>> About the client (SurfaceFlinger), I believe it requests
>> HAL_PIXEL_FORMAT_RGBA_8888 but finally
>> HAL_PIXEL_FORMAT_RGBX_8888 is used.
>
> Actually, by default it does not request a specific format.
>
>> 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.
>
> This first ignores the format and requests a config with all component
> sizes equal to 8. If that fails, it falls back to the requested
> format. If it falls back, you should see some messages about that.

OK. Yes.

>> 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.
>
> I looked and don't see a change either.

I finally found the AOSP version does matter
since this commit is in master but not in 7.1.2:

https://android.googlesource.com/platform/frameworks/native/+/e7f39727a484107b2d2a78eaaaacad3d7f44c24d%5E%21/#F0

By picking it to 7.1.2, now I got 8-8-8-8 in virgl.
Thanks!

>> Does Mesa version matter? May I need more patches in 17.1.5?
>
> I don't think so. I've carried the patch for some time. The main
> change was just the config ordering.
>
>> 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].
>
> Let me try that one.

With the RenderEngine's patch the app works now.
Great!


>> It complains the producer output format is RGBX_8888
>> but RGBA_8888 is expected:


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