Bug 91596

Created attachment 117608 [details]
dmesg, NOTE: segfaults are due to an independent problem

Created attachment 117609 [details]
dumpsys SurfaceFlinger

Hi,

as a confirmation the described Android-x86 GUI problems are solved by reverting commit c2c2e9ab604793c6e01f85497f3f5bf645f962fa.

I am available if you need further information or to test upcoming patches to EGL_KHR_gl_colorspace code.

Mauro

What happens if you hide the extension on Android?

Hi Makek,

I have performed test by disabling EGL_KHR_gl_colorspace extension,
Chih-Wei has tried with following override 

export MESA_EXTENSION_OVERRIDE "-EGL_KHR_gl_colorspace"

With both methods (to my knowledge they should be equivalent) problem was not solved

We came to the conclusion that c2c2e9a need to be reverted to have usable android-x86 GUI.

Looking at code before commit c2c2e9a, when conf->dri_single_config was defined, conf->dri_double_config was set to NULL and viceversa,
it's just a check/possible lead cause, because I'm not sure what the code actually does. 

Also I don't see inizialization of the new dri_srgb_single_config/dri_srgb_double_config but please pardon me, if this does not help.

       memcpy(&conf->base, &base, sizeof base);
       if (double_buffer) {
-         conf->dri_double_config = dri_config;
-         conf->dri_single_config = NULL;
+         if (srgb)
+            conf->dri_srgb_double_config = dri_config;
+         else
+            conf->dri_double_config = dri_config;
       } else {
-         conf->dri_single_config = dri_config;
-         conf->dri_double_config = NULL;
+         if (srgb)
+            conf->dri_srgb_single_config = dri_config;
+         else
+            conf->dri_single_config = dri_config;
       }

To be precise:

We came to the conclusion that c2c2e9a need to be reverted on android-x86 git external/mesa upcoming project in lollipop-x86 branch

Mauro

MESA_EXTENSION_OVERRIDE won't work. You need to set KHR_gl_colorspace = false manually in the code.

Hi Ilia,

Sorry I noticed my previous sentence was not clear,
I have performed the test by disabling EGL_KHR_gl_colorspace, independently frome Chih-Wei attempt with MESA_EXTENSION_OVERRIDE, by applying the following changes that were suggested to me by Emil:

--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -586,7 +586,7 @@ dri2_setup_screen(_EGLDisplay *disp)

   if (dri2_renderer_query_integer(dri2_dpy,
                                   __DRI2_RENDERER_HAS_FRAMEBUFFER_SRGB))
-      disp->Extensions.KHR_gl_colorspace = EGL_TRUE;
+      disp->Extensions.KHR_gl_colorspace = EGL_FALSE;

   if (dri2_dpy->dri2 && dri2_dpy->dri2->base.version >= 3) {
      disp->Extensions.KHR_create_context = EGL_TRUE;

The result was still flashing/phantom elements in android GUI.
So there may be some other effect induced by c2c2e9a commit.

Are all the dri_single/dri_double srgb/non srgb correctly set?

What is strange is that all kind of chipsets are affected.

Mauro

>> Hi Ilia,

My greeting also to Ilia, but you are Marek
Sorry

Mauro

In the few moments that android-x86 was up I've noticed a very interesting commit [1]. In case you don't have a local copy, it enables a workaround #10194508, as seen here [2].

Not sure if the android-x86 version has the extra qcom 'hacks' so it might be worth experimenting on the topic - adding/reverting the qcom commit, reverting the original patch [1], all that whist toggling on/off the extension ;-)


[1] http://git.android-x86.org/?p=platform/frameworks/native.git;a=commit;h=39aa8695464e496ec4f6d22e9af54457bcfdf4ae
[2] https://github.com/omnirom/android_frameworks_native/blob/HEAD/opengl/libs/EGL/eglApi.cpp#L431

(In reply to Emil Velikov from comment #10)
> In the few moments that android-x86 was up I've noticed a very interesting
> commit [1]. In case you don't have a local copy, it enables a workaround
> #10194508, as seen here [2].
> 
> Not sure if the android-x86 version has the extra qcom 'hacks' so it might
> be worth experimenting on the topic - adding/reverting the qcom commit,
> reverting the original patch [1], all that whist toggling on/off the
> extension ;-)
> 
> 
> [1]
> http://git.android-x86.org/?p=platform/frameworks/native.git;a=commit;
> h=39aa8695464e496ec4f6d22e9af54457bcfdf4ae
> [2]
> https://github.com/omnirom/android_frameworks_native/blob/HEAD/opengl/libs/
> EGL/eglApi.cpp#L431

OK, you pointed out an old workaround we applied long time ago.
Without it, we just got black screen on i915/i965 GPU and
garbled screen on radeon.
Credit to Chia-I Wu who found the workaround.
Otherwise android-x86 has already dead.

Actually I reverted that workaround each time I tried a new Mesa version.
However, it's no help. We still need the workaround.

I just tried to revert the workaround with the extension enabled/disabled.
Nothing changed. We still need the workaround.

Mesa still doesn't support PIXEL_FORMAT_RGBA_8888?

Looking over that patch, I noticed this hunk:

       memcpy(&conf->base, &base, sizeof base);
       if (double_buffer) {
-         conf->dri_double_config = dri_config;
-         conf->dri_single_config = NULL;
+         if (srgb)
+            conf->dri_srgb_double_config = dri_config;
+         else
+            conf->dri_double_config = dri_config;
       } else {
-         conf->dri_single_config = dri_config;
-         conf->dri_double_config = NULL;
+         if (srgb)
+            conf->dri_srgb_single_config = dri_config;
+         else
+            conf->dri_single_config = dri_config;
       }

Previously the "other" configs would get cleared out whereas they aren't anymore.

(In reply to Ilia Mirkin from comment #12)
> Looking over that patch, I noticed this hunk:
> 
>        memcpy(&conf->base, &base, sizeof base);
>        if (double_buffer) {
> -         conf->dri_double_config = dri_config;
> -         conf->dri_single_config = NULL;
> +         if (srgb)
> +            conf->dri_srgb_double_config = dri_config;
> +         else
> +            conf->dri_double_config = dri_config;
>        } else {
> -         conf->dri_single_config = dri_config;
> -         conf->dri_double_config = NULL;
> +         if (srgb)
> +            conf->dri_srgb_single_config = dri_config;
> +         else
> +            conf->dri_single_config = dri_config;
>        }
> 
> Previously the "other" configs would get cleared out whereas they aren't
> anymore.

Er nevermind. Before it was malloc'd but now it's calloc'd.

(In reply to Ilia Mirkin from comment #13)
> Er nevermind. Before it was malloc'd but now it's calloc'd.

The most creepy part is that with the extension disabled, we can never get to the srgb configs, as GLColorspace will be LINEAR. Even if the user requests one, we'll error out.

(In reply to Chih-Wei Huang from comment #11)
> Mesa still doesn't support PIXEL_FORMAT_RGBA_8888?

It should, depending on how exactly you define the formats. I've looked a while back for some android documentation but I came short. Have you seen any ?

Upon closer look the mappings in drm_format_from_hal and get_pipe_format look very funny.

(In reply to Emil Velikov from comment #14)
> (In reply to Ilia Mirkin from comment #13)
> > Er nevermind. Before it was malloc'd but now it's calloc'd.
> 
> The most creepy part is that with the extension disabled, we can never get
> to the srgb configs, as GLColorspace will be LINEAR. Even if the user
> requests one, we'll error out.

Actually I stand corrected - if one disables the extension, we'll still expose/list the srgb configs. With this patch [1] + the one in comment 8 things should be back to normal. Now we need to figure out why things go crazy in the first place (hint check the format mappings in comment 15).

[1] http://patchwork.freedesktop.org/patch/60961/

(In reply to Emil Velikov from comment #15)
> (In reply to Chih-Wei Huang from comment #11)
> > Mesa still doesn't support PIXEL_FORMAT_RGBA_8888?
> 
> It should, depending on how exactly you define the formats. I've looked a
> while back for some android documentation but I came short. Have you seen
> any ?
> 
> Upon closer look the mappings in drm_format_from_hal and get_pipe_format
> look very funny.

fwiw, there is a bogus patch that needs to be reverted in drm_gralloc, see:

https://github.com/robclark/drm_gralloc/commit/34bf9d8566aaf97a0f2adb127836134078abd340

Hello Mauro,

Could you try [1] on your setup? The flashing seems to go away with freedreno with the patch.

[1] http://patchwork.freedesktop.org/patch/61151/

Hi,

tested the patch by restoring conf->base.SurfaceType = 0;

it works on NV8x, HD7750 and Intel 946GZ, problem solved.

Thanks a lot.
Mauro


diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 461735f..2e68a65 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -312,6 +312,8 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig *dri_config, int id,
          else
             conf->dri_single_config = dri_config;
       }
+
+      conf->base.SurfaceType = 0;  
       conf->base.ConfigID = config_id;
 
       _eglLinkConfig(&conf->base);

(In reply to Rob Clark from comment #17)
> (In reply to Emil Velikov from comment #15)
> > (In reply to Chih-Wei Huang from comment #11)
> > > Mesa still doesn't support PIXEL_FORMAT_RGBA_8888?
> > 
> > It should, depending on how exactly you define the formats. I've looked a

The android's definition.

> > while back for some android documentation but I came short. Have you seen
> > any ?

Please read about line 454
https://android.googlesource.com/platform/frameworks/native/+/android-5.1.1_r1/opengl/libs/EGL/eglApi.cpp

If WORKAROUND_BUG_10194508 is not enabled,
HAL_PIXEL_FORMAT_RGBA_8888 is chosen and then
we only got black screen since
SurfaceFlinger crashed due to no surface.

> > Upon closer look the mappings in drm_format_from_hal and get_pipe_format
> > look very funny.

What does it mean? What's funny?

> fwiw, there is a bogus patch that needs to be reverted in drm_gralloc, see:
> 
> https://github.com/robclark/drm_gralloc/commit/
> 34bf9d8566aaf97a0f2adb127836134078abd340

That's a try-and-error patch.
Without it we just got black screen for vmwgfx.
So what's your suggested solution?

This shouldn't be closed, because the fix hasn't been committed yet.

(In reply to Mauro Rossi from comment #19)
> Hi,
> 
> tested the patch by restoring conf->base.SurfaceType = 0;
> 
> it works on NV8x, HD7750 and Intel 946GZ, problem solved.
> 
> Thanks a lot.
> Mauro
> 
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c
> b/src/egl/drivers/dri2/egl_dri2.c
> index 461735f..2e68a65 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -312,6 +312,8 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig
> *dri_config, int id,
>           else
>              conf->dri_single_config = dri_config;
>        }
> +
> +      conf->base.SurfaceType = 0;  
>        conf->base.ConfigID = config_id;
>  
>        _eglLinkConfig(&conf->base);

(In reply to Chih-Wei Huang from comment #20)
> (In reply to Rob Clark from comment #17)
> > (In reply to Emil Velikov from comment #15)
> > > Upon closer look the mappings in drm_format_from_hal and get_pipe_format
> > > look very funny.
> 
> What does it mean? What's funny?

well, mapping both HAL_PIXEL_FORMAT_RGB_888 and HAL_PIXEL_FORMAT_BGRA_8888 to DRM_FORMAT_XRGB8888, for example..

tbh I'm not sure what "endianess" HAL_PIXEL_FORMAT_* thinks about, but if HAL_PIXEL_FORMAT_RGBA_8888 -> DRM_FORMAT_RGBA8888, then it seems odd that HAL_PIXEL_FORMAT_RGBX_8888 -> DRM_FORMAT_XBGR8888..

> > fwiw, there is a bogus patch that needs to be reverted in drm_gralloc, see:
> > 
> > https://github.com/robclark/drm_gralloc/commit/
> > 34bf9d8566aaf97a0f2adb127836134078abd340
> 
> That's a try-and-error patch.
> Without it we just got black screen for vmwgfx.
> So what's your suggested solution?

just revert the patch to the way it was before.  If you are still getting a black screen with current mesa then we should debug what is wrong in vmware gallium driver..

(In reply to Emil Velikov from comment #15)
> (In reply to Chih-Wei Huang from comment #11)
> > Mesa still doesn't support PIXEL_FORMAT_RGBA_8888?
> 
> It should, depending on how exactly you define the formats. I've looked a
> while back for some android documentation but I came short. Have you seen
> any ?


HAL_PIXEL_FORMAT_RGBA_8888 is mapped to
DRM_FORMAT_RGBA8888 in drm_galloc.
Doesn't it look good?

So whether if Mesa supports DRM_FORMAT_RGBA8888?

 
> Upon closer look the mappings in drm_format_from_hal and get_pipe_format
> look very funny.