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

[Chih-Wei Huang]

2017-08-01 0:18 GMT+08:00 Rob Herring :
> On Fri, Jul 28, 2017 at 6:06 AM, Chih-Wei Huang  
> wrote:
>> Hi Rob,
>> I'm testing this patch on an AMD radeon chip (PALM) now.
>> I use our legacy drm_gralloc since the radeon driver
>> doesn't support atomic api required by drm_hwcomposer.
>>
>> On SurfaceFlinger started, the screen just messed up.
>
> In what way? Swapped red and blue?

No. Just garbage pictures.
(I should take a picture but I'm unable to do it now)

>> If reverting this patch, it becomes normal as before.



-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

On Fri, Jul 28, 2017 at 6:06 AM, Chih-Wei Huang  wrote:
> Hi Rob,
> I'm testing this patch on an AMD radeon chip (PALM) now.
> I use our legacy drm_gralloc since the radeon driver
> doesn't support atomic api required by drm_hwcomposer.
>
> On SurfaceFlinger started, the screen just messed up.

In what way? Swapped red and blue?

> If reverting this patch, it becomes normal as before.
>
> Does the radeon driver need more patches to
> support RGBA_ / RGBX_?
> Any idea?
>
>
> --
> Chih-Wei
> Android-x86 project
> http://www.android-x86.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

On Sun, Jul 30, 2017 at 10:48 AM, Chih-Wei Huang
 wrote:
> Hi Rob,
> Sorry to bother you again.
> This patch also breaks the srwast/llvmpipe on Android
> since the red and blue are just swapped.

My guess is because in this case you use the framebuffer (/dev/fb0)
and the format is fixed. The same problem exists with KMS and dumb
buffers as there is no way to either communicate what is the format or
set the format. It should be a simple switch in the kernel to change
the format, but I'm not sure what the proper fix is as there's
resistance to any extending of dumb buffers.

I've been trying to get llvmpipe to work in my build. I think I'm
close using kms_swrast, but the only thing I can get on screen is the
color I memset allocated buffers too.

> I guess nouveau has the same issue but
> I'm waiting Mauro's confirmation.

I'd guess it doesn't. However, it could just not support those
formats. I had to add them on freedreno for example.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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  wrote:
> 2017-07-26 23:34 GMT+08:00 Rob Herring :
>> On Tue, Jul 25, 2017 at 10:16 PM, Chih-Wei Huang
>>  wrote:
>>> 2017-07-26 1:24 GMT+08:00 Rob Herring :

 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/+/ed2ec4b0b3dc739fc0fd3427b2b740cc0de3

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_ but finally
>>> HAL_PIXEL_FORMAT_RGBX_ 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_
>>>(if HWC2 is used or if HWC1 api ver >= 1.1)
>>> 2. RenderEngine::create() calls RenderEngine::chooseEglConfig()
>>>with format=HAL_PIXEL_FORMAT_RGBA_
>>> 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_
>>>is used. (if alpha > 0, HAL_PIXEL_FORMAT_RGBA_ 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/+/e7f39727a484107b2d2a78ecad3d7f44c24d%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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

On Sun, Jul 30, 2017 at 5:49 PM, Chih-Wei Huang 
wrote:

> 2017-07-28 22:59 GMT+08:00 Marek Olšák :
> > Hi,
> >
> > I've sent a request to revert this commit in 17.2. I'll keep it in
> > master, but I'll add a fix not to expose the new formats for GLX.
>
> I thought it doesn't break GLX according to
> the commit message (investigation by Chad Versace).
> Isn't it correct?
>

The commit message is wrong. The GLX visuals are exposed, which means any
app can use them.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Chih-Wei Huang]

Hi Rob,
Sorry to bother you again.
This patch also breaks the srwast/llvmpipe on Android
since the red and blue are just swapped.
I guess nouveau has the same issue but
I'm waiting Mauro's confirmation.

Any comment?

-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Chih-Wei Huang]

2017-07-28 22:59 GMT+08:00 Marek Olšák :
> Hi,
>
> I've sent a request to revert this commit in 17.2. I'll keep it in
> master, but I'll add a fix not to expose the new formats for GLX.

I thought it doesn't break GLX according to
the commit message (investigation by Chad Versace).
Isn't it correct?

The issues I reported are all for Android.
Sorry if I didn't make it clear enough.

We still hope to use this patch to support
RGBA_ on Android since it gives
better app compatibility.
I guess some drivers (radeon/amdgpu/nouveau/swrast?)
need to be fixed to support RGBA_ better.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Marek Olšák]

Hi,

I've sent a request to revert this commit in 17.2. I'll keep it in
master, but I'll add a fix not to expose the new formats for GLX.

Marek

On Tue, Jul 11, 2017 at 10:34 PM, Rob Herring  wrote:
> From: Marek Olšák 
>
> 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 
> Cc: Chad Versace 
> Cc: Mauro Rossi 
> Reviewed-by: Marek Olšák 
> Signed-off-by: Rob Herring 
> ---
> v2:
> - Incorporated dri_fill_st_visual RGBA/X handling from Marek
> - Handle RGBA/X in dri2_drawable_get_buffers for completeness
>
>  src/gallium/state_trackers/dri/dri2.c   |  8 
>  src/gallium/state_trackers/dri/dri_screen.c | 69 
> -
>  2 files changed, 65 insertions(+), 12 deletions(-)
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c 
> b/src/gallium/state_trackers/dri/dri2.c
> index 60ec38d8e445..e20b2c075361 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -186,6 +186,9 @@ static enum pipe_format dri2_format_to_pipe_format (int 
> format)
> case __DRI_IMAGE_FORMAT_ARGB:
>pf = PIPE_FORMAT_BGRA_UNORM;
>break;
> +   case __DRI_IMAGE_FORMAT_XBGR:
> +  pf = PIPE_FORMAT_RGBX_UNORM;
> +  break;
> case __DRI_IMAGE_FORMAT_ABGR:
>pf = PIPE_FORMAT_RGBA_UNORM;
>break;
> @@ -356,9 +359,11 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
> */
>switch(format) {
>case PIPE_FORMAT_BGRA_UNORM:
> +  case PIPE_FORMAT_RGBA_UNORM:
>  depth = 32;
>  break;
>case PIPE_FORMAT_BGRX_UNORM:
> +  case PIPE_FORMAT_RGBX_UNORM:
>  depth = 24;
>  break;
>case PIPE_FORMAT_B5G6R5_UNORM:
> @@ -434,6 +439,9 @@ dri_image_drawable_get_buffers(struct dri_drawable 
> *drawable,
>case PIPE_FORMAT_BGRA_UNORM:
>   image_format = __DRI_IMAGE_FORMAT_ARGB;
>   break;
> +  case PIPE_FORMAT_RGBX_UNORM:
> + image_format = __DRI_IMAGE_FORMAT_XBGR;
> + break;
>case PIPE_FORMAT_RGBA_UNORM:
>   image_format = __DRI_IMAGE_FORMAT_ABGR;
>   break;
> diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
> b/src/gallium/state_trackers/dri/dri_screen.c
> index 6b58830e0b42..a0d9b34d667b 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -132,6 +132,27 @@ dri_fill_in_modes(struct dri_screen *screen)
>MESA_FORMAT_B8G8R8A8_SRGB,
>MESA_FORMAT_B8G8R8X8_SRGB,
>MESA_FORMAT_B5G6R5_UNORM,
> +
> +  /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
> +   * Likewise for RGBX and BGRX.  Otherwise, the GLX client and the GLX
> +   * server may disagree on which format the GLXFBConfig represents,
> +   * resulting in swapped color channels.
> +   *
> +   * The problem, as of 2017-05-30:
> +   * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the 
> channel
> +   * order and chooses the first __DRIconfig with the expected channel
> +   * sizes. Specifically, GLX compares the GLXFBConfig's and 
> __DRIconfig's
> +   * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
> +   *
> +   * EGL does not suffer from this problem. It correctly compares the
> +   * channel masks when matching EGLConfig to __DRIconfig.
> +   */
> +
> +  /* Required by Android, for HAL_PIXEL_FORMAT_RGBA_. */
> +  MESA_FORMAT_R8G8B8A8_UNORM,
> +
> +  /* Required by Android, for HAL_PIXEL_FORMAT_RGBX_. */
> +  MESA_FORMAT_R8G8B8X8_UNORM,
> };
> static const enum pipe_format pipe_formats[] = {
>PIPE_FORMAT_BGRA_UNORM,
> @@ -139,6 +160,8 @@ dri_fill_in_modes(struct dri_screen *screen)
>PIPE_FORMAT_BGRA_SRGB,
>PIPE_FORMAT_BGRX_SRGB,
>PIPE_FORMAT_B5G6R5_UNORM,
> +  PIPE_FORMAT_RGBA_UNORM,
> +  PIPE_FORMAT_RGBX_UNORM,
> };
> mesa_format format;
> __DRIconfig **configs = NULL;
> @@ -275,19 +298,41 @@ dri_fill_st_visual(struct st_visual *stvis, struct 
> dri_screen *screen,
> if (!mode)
>return;
>
> -   if (mode->redBits == 8) {
> -  if (mode->alphaBits == 8)
> - if (mode->sRGBCapable)
> -stvis->color_format = PIPE_FORMAT_BGRA_SRGB;
> - else
> -stvis->color_format = PIPE_FORMAT_BGRA_UNORM;
> -  else

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

[Chih-Wei Huang]

Hi Rob,
I'm testing this patch on an AMD radeon chip (PALM) now.
I use our legacy drm_gralloc since the radeon driver
doesn't support atomic api required by drm_hwcomposer.

On SurfaceFlinger started, the screen just messed up.
If reverting this patch, it becomes normal as before.

Does the radeon driver need more patches to
support RGBA_ / RGBX_?
Any idea?


-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [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 :
> On Tue, Jul 25, 2017 at 10:16 PM, Chih-Wei Huang
>  wrote:
>> 2017-07-26 1:24 GMT+08:00 Rob Herring :
>>>
>>> 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/+/ed2ec4b0b3dc739fc0fd3427b2b740cc0de3

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_ but finally
>> HAL_PIXEL_FORMAT_RGBX_ 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_
>>(if HWC2 is used or if HWC1 api ver >= 1.1)
>> 2. RenderEngine::create() calls RenderEngine::chooseEglConfig()
>>with format=HAL_PIXEL_FORMAT_RGBA_
>> 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_
>>is used. (if alpha > 0, HAL_PIXEL_FORMAT_RGBA_ 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/+/e7f39727a484107b2d2a78ecad3d7f44c24d%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_ 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_
>> but RGBA_ is expected:


-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

On Tue, Jul 25, 2017 at 10:16 PM, Chih-Wei Huang
 wrote:
> 2017-07-26 1:24 GMT+08:00 Rob Herring :
>> On Tue, Jul 25, 2017 at 10:15 AM, Emil Velikov  
>> wrote:
>>> On 25 July 2017 at 03:46, Chih-Wei Huang  wrote:
 On Tue 11 Jul 2017, Rob Herring wrote:
>> From: Marek Olšák 
>>
>> 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 
>> Cc: Chad Versace 
>> Cc: Mauro Rossi 
>> Reviewed-by: Marek Olšák 
>> Signed-off-by: Rob Herring 
>> ---

 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?

The hwc2 branch with virgl. There's also some kernel changes with
fence support for virgl needed[1].

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

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_ but finally
> HAL_PIXEL_FORMAT_RGBX_ 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_
>(if HWC2 is used or if HWC1 api ver >= 1.1)
> 2. RenderEngine::create() calls RenderEngine::chooseEglConfig()
>with format=HAL_PIXEL_FORMAT_RGBA_
> 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.

> 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_
>is used. (if alpha > 0, HAL_PIXEL_FORMAT_RGBA_ 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.

> 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_ still can't work.
> An example is the screen capture apps like Screenshot touch[3].

Let me try that one.

> It complains the producer output format is RGBX_
> but RGBA_ 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
> 

Re: [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 :
> On Tue, Jul 25, 2017 at 10:15 AM, Emil Velikov  
> wrote:
>> On 25 July 2017 at 03:46, Chih-Wei Huang  wrote:
>>> On Tue 11 Jul 2017, Rob Herring wrote:
> From: Marek Olšák 
>
> 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 
> Cc: Chad Versace 
> Cc: Mauro Rossi 
> Reviewed-by: Marek Olšák 
> Signed-off-by: Rob Herring 
> ---
>>>
>>> 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_ but finally
HAL_PIXEL_FORMAT_RGBX_ is used.

The logic I saw is:
1. In SurfaceFlinger::init(), it calls RenderEngine::create() with
hwcFormat=HAL_PIXEL_FORMAT_RGBA_
   (if HWC2 is used or if HWC1 api ver >= 1.1)
2. RenderEngine::create() calls RenderEngine::chooseEglConfig()
   with format=HAL_PIXEL_FORMAT_RGBA_
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_
   is used. (if alpha > 0, HAL_PIXEL_FORMAT_RGBA_ 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_ still can't work.
An example is the screen capture apps like Screenshot touch[3].
It complains the producer output format is RGBX_
but RGBA_ 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

On Tue, Jul 25, 2017 at 10:15 AM, Emil Velikov  wrote:
> On 25 July 2017 at 03:46, Chih-Wei Huang  wrote:
>> On Tue 11 Jul 2017, Rob Herring wrote:
 From: Marek Olšák 

 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 
 Cc: Chad Versace 
 Cc: Mauro Rossi 
 Reviewed-by: Marek Olšák 
 Signed-off-by: Rob Herring 
 ---
 v2:
 - Incorporated dri_fill_st_visual RGBA/X handling from Marek
 - Handle RGBA/X in dri2_drawable_get_buffers for completeness
>>
>> 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.

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Emil Velikov]

On 25 July 2017 at 03:46, Chih-Wei Huang  wrote:
> On Tue 11 Jul 2017, Rob Herring wrote:
>>> From: Marek Olšák 
>>>
>>> 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 
>>> Cc: Chad Versace 
>>> Cc: Mauro Rossi 
>>> Reviewed-by: Marek Olšák 
>>> Signed-off-by: Rob Herring 
>>> ---
>>> v2:
>>> - Incorporated dri_fill_st_visual RGBA/X handling from Marek
>>> - Handle RGBA/X in dri2_drawable_get_buffers for completeness
>
> 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?

If there's some problems, I'd check that first.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

On Mon, Jul 24, 2017 at 9:46 PM, Chih-Wei Huang  wrote:
> On Tue 11 Jul 2017, Rob Herring wrote:
>>> From: Marek Olšák 
>>>
>>> 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 
>>> Cc: Chad Versace 
>>> Cc: Mauro Rossi 
>>> Reviewed-by: Marek Olšák 
>>> Signed-off-by: Rob Herring 
>>> ---
>>> v2:
>>> - Incorporated dri_fill_st_visual RGBA/X handling from Marek
>>> - Handle RGBA/X in dri2_drawable_get_buffers for completeness
>
> 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()
>
> 07-25 02:19:13.188  1125  1125 I SurfaceFlinger: EGL information: format=0x1
> 07-25 02:19:13.188  1125  1125 I SurfaceFlinger: vendor: Android
> 07-25 02:19:13.188  1125  1125 I SurfaceFlinger: version   : 1.4
> Android META-EGL
> 07-25 02:19:13.188  1125  1125 I SurfaceFlinger: extensions:
> EGL_KHR_get_all_proc_addresses EGL_ANDROID_presentation_time
> EGL_KHR_swap_buffers_with_damage EGL_ANDROID_create_native_cli
> ent_buffer EGL_ANDROID_front_buffer_auto_refresh EGL_KHR_image_base
> EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_3D_image
> EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_ima
> ge EGL_KHR_reusable_sync EGL_KHR_fence_sync EGL_KHR_create_context
> EGL_KHR_config_attribs EGL_KHR_surfaceless_context
> EGL_ANDROID_image_native_buffer EGL_KHR_wait_sync EGL_ANDROID_reco
> rdable EGL_EXT_buffer_age
> 07-25 02:19:13.188  1125  1125 I SurfaceFlinger: Client API: OpenGL_ES
> 07-25 02:19:13.188  1125  1125 I SurfaceFlinger: EGLSurface: 8-8-8-8,
> config=0xa5485880
>
>  (r-b-g-a)
> 07-25 02:19:13.211  1125  1125 I SurfaceFlinger: OpenGL ES
> informations: format=0x1
> 07-25 02:19:13.211  1125  1125 I SurfaceFlinger: vendor: Red Hat
> 07-25 02:19:13.211  1125  1125 I SurfaceFlinger: renderer  : Gallium
> 0.4 on virgl
> 07-25 02:19:13.211  1125  1125 I SurfaceFlinger: version   : OpenGL ES
> 3.0 Mesa 17.1.5 (git-317b5bd)
>
> After applying the patch, however, alpha becomes 0
>
> 07-25 02:34:46.522  1125  1125 I SurfaceFlinger: EGL information: format=0x1
> 07-25 02:34:46.522  1125  1125 I SurfaceFlinger: vendor: Android
> 07-25 02:34:46.522  1125  1125 I SurfaceFlinger: version   : 1.4
> Android META-EGL
> 07-25 02:34:46.522  1125  1125 I SurfaceFlinger: extensions:
> EGL_KHR_get_all_proc_addresses EGL_ANDROID_presentation_time
> EGL_KHR_swap_buffers_with_damage
> EGL_ANDROID_create_native_client_buffer
> EGL_ANDROID_front_buffer_auto_refresh EGL_KHR_image_base
> EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_3D_image
> EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_image
> EGL_KHR_reusable_sync EGL_KHR_fence_sync EGL_KHR_create_context
> EGL_KHR_config_attribs EGL_KHR_surfaceless_context
> EGL_ANDROID_image_native_buffer EGL_KHR_wait_sync
> EGL_ANDROID_recordable EGL_EXT_buffer_age
> 07-25 02:34:46.522  1125  1125 I SurfaceFlinger: Client API: OpenGL_ES
> 07-25 02:34:46.522  1125  1125 I SurfaceFlinger: EGLSurface: 8-8-8-0,
> config=0xabc24d80
>
> 
> 07-25 02:34:46.574  1125  1125 I SurfaceFlinger: OpenGL ES
> informations: format=0x1
> 07-25 02:34:46.574  1125  1125 I SurfaceFlinger: vendor: Red Hat
> 07-25 02:34:46.574  1125  1125 I SurfaceFlinger: renderer  : Gallium
> 0.4 on virgl
> 07-25 02:34:46.574  1125  1125 I SurfaceFlinger: version   : OpenGL ES
> 3.0 Mesa 17.1.5 (git-317b5bd)
>
>
> Therefore, eglCreateWindowSurface() finally chose
> HAL_PIXEL_FORMAT_RGBX_ instead of
> HAL_PIXEL_FORMAT_RGBA_.
> Is that expected?

Yes. I believe the client requested a config without alpha, so RGBX
satisfies that.

Do you observe any problems because of the change?

Rob
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Chih-Wei Huang]

On Tue 11 Jul 2017, Rob Herring wrote:
>> From: Marek Olšák 
>>
>> 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 
>> Cc: Chad Versace 
>> Cc: Mauro Rossi 
>> Reviewed-by: Marek Olšák 
>> Signed-off-by: Rob Herring 
>> ---
>> v2:
>> - Incorporated dri_fill_st_visual RGBA/X handling from Marek
>> - Handle RGBA/X in dri2_drawable_get_buffers for completeness

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

07-25 02:19:13.188  1125  1125 I SurfaceFlinger: EGL information: format=0x1
07-25 02:19:13.188  1125  1125 I SurfaceFlinger: vendor: Android
07-25 02:19:13.188  1125  1125 I SurfaceFlinger: version   : 1.4
Android META-EGL
07-25 02:19:13.188  1125  1125 I SurfaceFlinger: extensions:
EGL_KHR_get_all_proc_addresses EGL_ANDROID_presentation_time
EGL_KHR_swap_buffers_with_damage EGL_ANDROID_create_native_cli
ent_buffer EGL_ANDROID_front_buffer_auto_refresh EGL_KHR_image_base
EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_3D_image
EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_ima
ge EGL_KHR_reusable_sync EGL_KHR_fence_sync EGL_KHR_create_context
EGL_KHR_config_attribs EGL_KHR_surfaceless_context
EGL_ANDROID_image_native_buffer EGL_KHR_wait_sync EGL_ANDROID_reco
rdable EGL_EXT_buffer_age
07-25 02:19:13.188  1125  1125 I SurfaceFlinger: Client API: OpenGL_ES
07-25 02:19:13.188  1125  1125 I SurfaceFlinger: EGLSurface: 8-8-8-8,
config=0xa5485880

 (r-b-g-a)
07-25 02:19:13.211  1125  1125 I SurfaceFlinger: OpenGL ES
informations: format=0x1
07-25 02:19:13.211  1125  1125 I SurfaceFlinger: vendor: Red Hat
07-25 02:19:13.211  1125  1125 I SurfaceFlinger: renderer  : Gallium
0.4 on virgl
07-25 02:19:13.211  1125  1125 I SurfaceFlinger: version   : OpenGL ES
3.0 Mesa 17.1.5 (git-317b5bd)

After applying the patch, however, alpha becomes 0

07-25 02:34:46.522  1125  1125 I SurfaceFlinger: EGL information: format=0x1
07-25 02:34:46.522  1125  1125 I SurfaceFlinger: vendor: Android
07-25 02:34:46.522  1125  1125 I SurfaceFlinger: version   : 1.4
Android META-EGL
07-25 02:34:46.522  1125  1125 I SurfaceFlinger: extensions:
EGL_KHR_get_all_proc_addresses EGL_ANDROID_presentation_time
EGL_KHR_swap_buffers_with_damage
EGL_ANDROID_create_native_client_buffer
EGL_ANDROID_front_buffer_auto_refresh EGL_KHR_image_base
EGL_KHR_gl_texture_2D_image EGL_KHR_gl_texture_3D_image
EGL_KHR_gl_texture_cubemap_image EGL_KHR_gl_renderbuffer_image
EGL_KHR_reusable_sync EGL_KHR_fence_sync EGL_KHR_create_context
EGL_KHR_config_attribs EGL_KHR_surfaceless_context
EGL_ANDROID_image_native_buffer EGL_KHR_wait_sync
EGL_ANDROID_recordable EGL_EXT_buffer_age
07-25 02:34:46.522  1125  1125 I SurfaceFlinger: Client API: OpenGL_ES
07-25 02:34:46.522  1125  1125 I SurfaceFlinger: EGLSurface: 8-8-8-0,
config=0xabc24d80


07-25 02:34:46.574  1125  1125 I SurfaceFlinger: OpenGL ES
informations: format=0x1
07-25 02:34:46.574  1125  1125 I SurfaceFlinger: vendor: Red Hat
07-25 02:34:46.574  1125  1125 I SurfaceFlinger: renderer  : Gallium
0.4 on virgl
07-25 02:34:46.574  1125  1125 I SurfaceFlinger: version   : OpenGL ES
3.0 Mesa 17.1.5 (git-317b5bd)


Therefore, eglCreateWindowSurface() finally chose
HAL_PIXEL_FORMAT_RGBX_ instead of
HAL_PIXEL_FORMAT_RGBA_.
Is that expected?
I thought it should be HAL_PIXEL_FORMAT_RGBA_.


-- 
Chih-Wei
Android-x86 project
http://www.android-x86.org
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Chad Versace]

On Tue 11 Jul 2017, Rob Herring wrote:
> From: Marek Olšák 
> 
> 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 
> Cc: Chad Versace 
> Cc: Mauro Rossi 
> Reviewed-by: Marek Olšák 
> Signed-off-by: Rob Herring 
> ---
> v2:
> - Incorporated dri_fill_st_visual RGBA/X handling from Marek
> - Handle RGBA/X in dri2_drawable_get_buffers for completeness

Reviewed-by: Chad Versace 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

From: Marek Olšák 

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 
Cc: Chad Versace 
Cc: Mauro Rossi 
Reviewed-by: Marek Olšák 
Signed-off-by: Rob Herring 
---
v2:
- Incorporated dri_fill_st_visual RGBA/X handling from Marek
- Handle RGBA/X in dri2_drawable_get_buffers for completeness

 src/gallium/state_trackers/dri/dri2.c   |  8 
 src/gallium/state_trackers/dri/dri_screen.c | 69 -
 2 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/src/gallium/state_trackers/dri/dri2.c 
b/src/gallium/state_trackers/dri/dri2.c
index 60ec38d8e445..e20b2c075361 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -186,6 +186,9 @@ static enum pipe_format dri2_format_to_pipe_format (int 
format)
case __DRI_IMAGE_FORMAT_ARGB:
   pf = PIPE_FORMAT_BGRA_UNORM;
   break;
+   case __DRI_IMAGE_FORMAT_XBGR:
+  pf = PIPE_FORMAT_RGBX_UNORM;
+  break;
case __DRI_IMAGE_FORMAT_ABGR:
   pf = PIPE_FORMAT_RGBA_UNORM;
   break;
@@ -356,9 +359,11 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
*/
   switch(format) {
   case PIPE_FORMAT_BGRA_UNORM:
+  case PIPE_FORMAT_RGBA_UNORM:
 depth = 32;
 break;
   case PIPE_FORMAT_BGRX_UNORM:
+  case PIPE_FORMAT_RGBX_UNORM:
 depth = 24;
 break;
   case PIPE_FORMAT_B5G6R5_UNORM:
@@ -434,6 +439,9 @@ dri_image_drawable_get_buffers(struct dri_drawable 
*drawable,
   case PIPE_FORMAT_BGRA_UNORM:
  image_format = __DRI_IMAGE_FORMAT_ARGB;
  break;
+  case PIPE_FORMAT_RGBX_UNORM:
+ image_format = __DRI_IMAGE_FORMAT_XBGR;
+ break;
   case PIPE_FORMAT_RGBA_UNORM:
  image_format = __DRI_IMAGE_FORMAT_ABGR;
  break;
diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
b/src/gallium/state_trackers/dri/dri_screen.c
index 6b58830e0b42..a0d9b34d667b 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -132,6 +132,27 @@ dri_fill_in_modes(struct dri_screen *screen)
   MESA_FORMAT_B8G8R8A8_SRGB,
   MESA_FORMAT_B8G8R8X8_SRGB,
   MESA_FORMAT_B5G6R5_UNORM,
+
+  /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
+   * Likewise for RGBX and BGRX.  Otherwise, the GLX client and the GLX
+   * server may disagree on which format the GLXFBConfig represents,
+   * resulting in swapped color channels.
+   *
+   * The problem, as of 2017-05-30:
+   * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
+   * order and chooses the first __DRIconfig with the expected channel
+   * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
+   * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
+   *
+   * EGL does not suffer from this problem. It correctly compares the
+   * channel masks when matching EGLConfig to __DRIconfig.
+   */
+
+  /* Required by Android, for HAL_PIXEL_FORMAT_RGBA_. */
+  MESA_FORMAT_R8G8B8A8_UNORM,
+
+  /* Required by Android, for HAL_PIXEL_FORMAT_RGBX_. */
+  MESA_FORMAT_R8G8B8X8_UNORM,
};
static const enum pipe_format pipe_formats[] = {
   PIPE_FORMAT_BGRA_UNORM,
@@ -139,6 +160,8 @@ dri_fill_in_modes(struct dri_screen *screen)
   PIPE_FORMAT_BGRA_SRGB,
   PIPE_FORMAT_BGRX_SRGB,
   PIPE_FORMAT_B5G6R5_UNORM,
+  PIPE_FORMAT_RGBA_UNORM,
+  PIPE_FORMAT_RGBX_UNORM,
};
mesa_format format;
__DRIconfig **configs = NULL;
@@ -275,19 +298,41 @@ dri_fill_st_visual(struct st_visual *stvis, struct 
dri_screen *screen,
if (!mode)
   return;
 
-   if (mode->redBits == 8) {
-  if (mode->alphaBits == 8)
- if (mode->sRGBCapable)
-stvis->color_format = PIPE_FORMAT_BGRA_SRGB;
- else
-stvis->color_format = PIPE_FORMAT_BGRA_UNORM;
-  else
- if (mode->sRGBCapable)
-stvis->color_format = PIPE_FORMAT_BGRX_SRGB;
- else
-stvis->color_format = PIPE_FORMAT_BGRX_UNORM;
-   } else {
+   /* Deduce the color format. */
+   switch (mode->redMask) {
+   case 0x00FF:
+  if (mode->alphaMask) {
+ assert(mode->alphaMask == 0xFF00);
+ stvis->color_format = mode->sRGBCapable ?
+  

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

[Eric Anholt]

Rob Herring  writes:

> Add support for 32-bit RGBX/RGBA formats which are preferred for Android.
>
> Signed-off-by: Rob Herring 
> ---
> v2:
> - Rebase to current master after introduction of new 
>   dri2_format_to_pipe_format function.

Still gets my r-b.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Rob Herring]

Add support for 32-bit RGBX/RGBA formats which are preferred for Android.

Signed-off-by: Rob Herring 
---
v2:
- Rebase to current master after introduction of new 
  dri2_format_to_pipe_format function.

 src/gallium/state_trackers/dri/dri2.c   | 6 ++
 src/gallium/state_trackers/dri/dri_screen.c | 4 
 2 files changed, 10 insertions(+)

diff --git a/src/gallium/state_trackers/dri/dri2.c 
b/src/gallium/state_trackers/dri/dri2.c
index 468d056..3f9ef1a 100644
--- a/src/gallium/state_trackers/dri/dri2.c
+++ b/src/gallium/state_trackers/dri/dri2.c
@@ -115,6 +115,9 @@ static enum pipe_format dri2_format_to_pipe_format (int 
format)
case __DRI_IMAGE_FORMAT_ARGB:
   pf = PIPE_FORMAT_BGRA_UNORM;
   break;
+   case __DRI_IMAGE_FORMAT_XBGR:
+  pf = PIPE_FORMAT_RGBX_UNORM;
+  break;
case __DRI_IMAGE_FORMAT_ABGR:
   pf = PIPE_FORMAT_RGBA_UNORM;
   break;
@@ -292,6 +295,9 @@ dri_image_drawable_get_buffers(struct dri_drawable 
*drawable,
   case PIPE_FORMAT_BGRA_UNORM:
  image_format = __DRI_IMAGE_FORMAT_ARGB;
  break;
+  case PIPE_FORMAT_RGBX_UNORM:
+ image_format = __DRI_IMAGE_FORMAT_XBGR;
+ break;
   case PIPE_FORMAT_RGBA_UNORM:
  image_format = __DRI_IMAGE_FORMAT_ABGR;
  break;
diff --git a/src/gallium/state_trackers/dri/dri_screen.c 
b/src/gallium/state_trackers/dri/dri_screen.c
index 2ac55c8..2856ec0 100644
--- a/src/gallium/state_trackers/dri/dri_screen.c
+++ b/src/gallium/state_trackers/dri/dri_screen.c
@@ -104,6 +104,8 @@ static const __DRIconfig **
 dri_fill_in_modes(struct dri_screen *screen)
 {
static const mesa_format mesa_formats[] = {
+  MESA_FORMAT_R8G8B8A8_UNORM,
+  MESA_FORMAT_R8G8B8X8_UNORM,
   MESA_FORMAT_B8G8R8A8_UNORM,
   MESA_FORMAT_B8G8R8X8_UNORM,
   MESA_FORMAT_B8G8R8A8_SRGB,
@@ -111,6 +113,8 @@ dri_fill_in_modes(struct dri_screen *screen)
   MESA_FORMAT_B5G6R5_UNORM,
};
static const enum pipe_format pipe_formats[] = {
+  PIPE_FORMAT_RGBA_UNORM,
+  PIPE_FORMAT_RGBX_UNORM,
   PIPE_FORMAT_BGRA_UNORM,
   PIPE_FORMAT_BGRX_UNORM,
   PIPE_FORMAT_BGRA_SRGB,
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev