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

[Rob Herring]

On Fri, Jul 28, 2017 at 1:25 AM, Chih-Wei Huang <cwhuang at android-x86.org> wrote:
> 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?)

Yes, this branch:

https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/log/?h=virgl-fence

Gustavo has an updated branch, but it crashes for me and I need to debug it.

>
>>> 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

Those are just the beginning.

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

Many of the ones here:

https://chromium-review.googlesource.com/q/drm_hwcomposer

The necessary commits are on my hwc2 branch.

>
>> 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.

Great. I would think that commit should only matter if you have a
video overlay, but maybe not.

Rob