[1/2] egl/android: fix regression in drm_gralloc path

Submitted by Mauro Rossi on Aug. 14, 2018, 8:27 p.m.

Details

Message ID 20180814202712.19955-1-issor.oruam@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mauro Rossi Aug. 14, 2018, 8:27 p.m.
This patch fixes a regression in mesa 18.2 and mesa-dev branches 
for HAVE_DRM_GRALLOC code path which is causing black screen on Android
and prevents boot due to SIGSEGV MAPERR crash related to unproper handling
of drm_gralloc drm FD in new droid_open_device() path.

The problem due to c7bb82136b ("egl/android: Add DRM node probing and filtering")

...  3173  3307 D GRALLOC-DRM: drmOpen radeon: 71
...  3173  3307 I GRALLOC-RADEON: detected chipset 0x6841 family 0x31 (vram size 238MiB, gart size 1021MiB)
...  3173  3307 I GRALLOC-DRM: create radeon for driver radeon
...  3173  3307 W EGL-MAIN: Could not get native buffer FD
--------- beginning of crash
...  3173  3307 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault addr 0x18 in tid 3307 (RenderThread), pid 3173 (ndroid.systemui)
...     0     0 D         : [drm:radeon_crtc_page_flip_target [radeon]] flip-ioctl() cur_rbo = 000000003512328a, new_rbo = 000000
...  3420  3420 I crash_dump64: performing dump of process 3173 (target tid = 3307)
...  3420  3420 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
...  3420  3420 F DEBUG   : Build fingerprint: 'Android-x86/android_x86_64/x86_64:8.1.0/OPM6.171019.030.E1/uten07210645:userdebug/test-keys'
...  3420  3420 F DEBUG   : Revision: '0'
...  3420  3420 F DEBUG   : ABI: 'x86_64'
...  3420  3420 F DEBUG   : pid: 3173, tid: 3307, name: RenderThread  >>> com.android.systemui <<<
...  3420  3420 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x18
...  3420  3420 F DEBUG   : Cause: null pointer dereference
...  3420  3420 F DEBUG   :     rax 0000000000000000  rbx 00007c6ac3a3eee0  rcx 0000000000000000  rdx 0000000000000038
...  3420  3420 F DEBUG   :     rsi 0000000000000000  rdi 00007c6ab5bfeaa0
...  3420  3420 F DEBUG   :     r8  00007c6ab04c16e4  r9  00007c6b4ee6a220  r10 0000000000000000  r11 0000000000000200
...  3420  3420 F DEBUG   :     r12 0000000000000000  r13 0000000000000000  r14 0000000000000001  r15 00007c6ab04c1600
...  3420  3420 F DEBUG   :     cs  0000000000000033  ss  000000000000002b
...  3420  3420 F DEBUG   :     rip 00007c6ab0cee444  rbp 00007c6ac3ae8400  rsp 00007c6ab5bfea80  eflags 0000000000010246
...  3420  3420 F DEBUG   :
...  3420  3420 F DEBUG   : backtrace:
...  3420  3420 F DEBUG   :     #00 pc 000000000056b444  /system/vendor/lib64/dri/gallium_dri.so (st_update_framebuffer_state+660)
...  3420  3420 F DEBUG   :     #01 pc 0000000000569d61  /system/vendor/lib64/dri/gallium_dri.so (st_validate_state+561)
...  3420  3420 F DEBUG   :     #02 pc 0000000000572374  /system/vendor/lib64/dri/gallium_dri.so (st_Clear+116)
...  3420  3420 F DEBUG   :     #03 pc 000000000004a9c0  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #04 pc 0000000000085cab  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #05 pc 0000000000085f31  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #06 pc 000000000006e8c1  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #07 pc 000000000006e3d9  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #08 pc 000000000006c4e3  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #09 pc 00000000000704c8  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #10 pc 0000000000077fb9  /android/system/lib64/libhwui.so
...  3420  3420 F DEBUG   :     #11 pc 00000000000117fa  /android/system/lib64/libutils.so
...  3420  3420 F DEBUG   :     #12 pc 00000000000ba193  /android/system/lib64/libandroid_runtime.so
...  3420  3420 F DEBUG   :     #13 pc 0000000000079f0b  /android/system/lib64/libc.so
...  3420  3420 F DEBUG   :     #14 pc 0000000000028c5d  /android/system/lib64/libc.so
...  3420  3420 F DEBUG   :     #15 pc 0000000000027555  /android/system/lib64/libc.so

To avoid the crash the former existing working droid_open_device() is restored,
renamed droid_open_device_drm_gralloc() and kept within HAVE_DRM_GRALLOC braces.

NOTE: Definition of enum{} for GRALLOC_MODULE_PERFORM_GET_DRM_FD
is not necessary and it is actually causing a redefinition building error,
because in HAVE_DRM_GRALLOC path gralloc_drm.h is already exported
by libgralloc_drm which is currently still a dependency.

Tested with mesa-dev and mesa 18.2 branch and oreo-x86 bootanimation
and Androdi GUI booting is fixed with i965, nouveau, radeon.

The changes are compatible with gbm_gralloc, I've tested build with hwc too.

Fixes: c7bb82136b ("egl/android: Add DRM node probing and filtering")
Cc: "18.2" <mesa-stable@lists.freedesktop.org>
Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
---
 src/egl/drivers/dri2/platform_android.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
index cc16fd8118..834bbd258e 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -1134,6 +1134,25 @@  droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy)
    return (config_count != 0);
 }
 
+#ifdef HAVE_DRM_GRALLOC
+static int
+droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy)
+{
+   int fd = -1, err = -EINVAL;
+
+   if (dri2_dpy->gralloc->perform)
+         err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
+                                          GRALLOC_MODULE_PERFORM_GET_DRM_FD,
+                                          &fd);
+   if (err || fd < 0) {
+      _eglLog(_EGL_WARNING, "fail to get drm fd");
+      fd = -1;
+   }
+
+   return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
+}
+#endif /* HAVE_DRM_GRALLOC */
+
 static const struct dri2_egl_display_vtbl droid_display_vtbl = {
    .authenticate = NULL,
    .create_window_surface = droid_create_window_surface,
@@ -1384,7 +1403,11 @@  dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp)
 
    disp->DriverData = (void *) dri2_dpy;
 
+   #ifdef HAVE_DRM_GRALLOC
+   dri2_dpy->fd = droid_open_device_drm_gralloc(dri2_dpy);
+   #else
    dri2_dpy->fd = droid_open_device(disp);
+   #endif
    if (dri2_dpy->fd < 0) {
       err = "DRI2: failed to open device";
       goto cleanup;

Comments

Hi all,
sorry for the typo in the subject the patch is only one, it should have
been titled

[PATCH] egl/android: fix regression in drm_gralloc path

Please review and I am available to test if there are working alternatives
to avoid the breakage of drm_gralloc users.
Mauro

Il giorno mar 14 ago 2018 alle ore 22:27 Mauro Rossi <issor.oruam@gmail.com>
ha scritto:

> This patch fixes a regression in mesa 18.2 and mesa-dev branches
> for HAVE_DRM_GRALLOC code path which is causing black screen on Android
> and prevents boot due to SIGSEGV MAPERR crash related to unproper handling
> of drm_gralloc drm FD in new droid_open_device() path.
>
> The problem due to c7bb82136b ("egl/android: Add DRM node probing and
> filtering")
>
> ...  3173  3307 D GRALLOC-DRM: drmOpen radeon: 71
> ...  3173  3307 I GRALLOC-RADEON: detected chipset 0x6841 family 0x31
> (vram size 238MiB, gart size 1021MiB)
> ...  3173  3307 I GRALLOC-DRM: create radeon for driver radeon
> ...  3173  3307 W EGL-MAIN: Could not get native buffer FD
> --------- beginning of crash
> ...  3173  3307 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault addr
> 0x18 in tid 3307 (RenderThread), pid 3173 (ndroid.systemui)
> ...     0     0 D         : [drm:radeon_crtc_page_flip_target [radeon]]
> flip-ioctl() cur_rbo = 000000003512328a, new_rbo = 000000
> ...  3420  3420 I crash_dump64: performing dump of process 3173 (target
> tid = 3307)
> ...  3420  3420 F DEBUG   : *** *** *** *** *** *** *** *** *** *** ***
> *** *** *** *** ***
> ...  3420  3420 F DEBUG   : Build fingerprint:
> 'Android-x86/android_x86_64/x86_64:8.1.0/OPM6.171019.030.E1/uten07210645:userdebug/test-keys'
> ...  3420  3420 F DEBUG   : Revision: '0'
> ...  3420  3420 F DEBUG   : ABI: 'x86_64'
> ...  3420  3420 F DEBUG   : pid: 3173, tid: 3307, name: RenderThread  >>>
> com.android.systemui <<<
> ...  3420  3420 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR),
> fault addr 0x18
> ...  3420  3420 F DEBUG   : Cause: null pointer dereference
> ...  3420  3420 F DEBUG   :     rax 0000000000000000  rbx
> 00007c6ac3a3eee0  rcx 0000000000000000  rdx 0000000000000038
> ...  3420  3420 F DEBUG   :     rsi 0000000000000000  rdi 00007c6ab5bfeaa0
> ...  3420  3420 F DEBUG   :     r8  00007c6ab04c16e4  r9
> 00007c6b4ee6a220  r10 0000000000000000  r11 0000000000000200
> ...  3420  3420 F DEBUG   :     r12 0000000000000000  r13
> 0000000000000000  r14 0000000000000001  r15 00007c6ab04c1600
> ...  3420  3420 F DEBUG   :     cs  0000000000000033  ss  000000000000002b
> ...  3420  3420 F DEBUG   :     rip 00007c6ab0cee444  rbp
> 00007c6ac3ae8400  rsp 00007c6ab5bfea80  eflags 0000000000010246
> ...  3420  3420 F DEBUG   :
> ...  3420  3420 F DEBUG   : backtrace:
> ...  3420  3420 F DEBUG   :     #00 pc 000000000056b444
> /system/vendor/lib64/dri/gallium_dri.so (st_update_framebuffer_state+660)
> ...  3420  3420 F DEBUG   :     #01 pc 0000000000569d61
> /system/vendor/lib64/dri/gallium_dri.so (st_validate_state+561)
> ...  3420  3420 F DEBUG   :     #02 pc 0000000000572374
> /system/vendor/lib64/dri/gallium_dri.so (st_Clear+116)
> ...  3420  3420 F DEBUG   :     #03 pc 000000000004a9c0
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #04 pc 0000000000085cab
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #05 pc 0000000000085f31
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #06 pc 000000000006e8c1
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #07 pc 000000000006e3d9
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #08 pc 000000000006c4e3
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #09 pc 00000000000704c8
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #10 pc 0000000000077fb9
> /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #11 pc 00000000000117fa
> /android/system/lib64/libutils.so
> ...  3420  3420 F DEBUG   :     #12 pc 00000000000ba193
> /android/system/lib64/libandroid_runtime.so
> ...  3420  3420 F DEBUG   :     #13 pc 0000000000079f0b
> /android/system/lib64/libc.so
> ...  3420  3420 F DEBUG   :     #14 pc 0000000000028c5d
> /android/system/lib64/libc.so
> ...  3420  3420 F DEBUG   :     #15 pc 0000000000027555
> /android/system/lib64/libc.so
>
> To avoid the crash the former existing working droid_open_device() is
> restored,
> renamed droid_open_device_drm_gralloc() and kept within HAVE_DRM_GRALLOC
> braces.
>
> NOTE: Definition of enum{} for GRALLOC_MODULE_PERFORM_GET_DRM_FD
> is not necessary and it is actually causing a redefinition building error,
> because in HAVE_DRM_GRALLOC path gralloc_drm.h is already exported
> by libgralloc_drm which is currently still a dependency.
>
> Tested with mesa-dev and mesa 18.2 branch and oreo-x86 bootanimation
> and Androdi GUI booting is fixed with i965, nouveau, radeon.
>
> The changes are compatible with gbm_gralloc, I've tested build with hwc
> too.
>
> Fixes: c7bb82136b ("egl/android: Add DRM node probing and filtering")
> Cc: "18.2" <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> ---
>  src/egl/drivers/dri2/platform_android.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> index cc16fd8118..834bbd258e 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -1134,6 +1134,25 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
> _EGLDisplay *dpy)
>     return (config_count != 0);
>  }
>
> +#ifdef HAVE_DRM_GRALLOC
> +static int
> +droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy)
> +{
> +   int fd = -1, err = -EINVAL;
> +
> +   if (dri2_dpy->gralloc->perform)
> +         err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
> +
> GRALLOC_MODULE_PERFORM_GET_DRM_FD,
> +                                          &fd);
> +   if (err || fd < 0) {
> +      _eglLog(_EGL_WARNING, "fail to get drm fd");
> +      fd = -1;
> +   }
> +
> +   return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> +}
> +#endif /* HAVE_DRM_GRALLOC */
> +
>  static const struct dri2_egl_display_vtbl droid_display_vtbl = {
>     .authenticate = NULL,
>     .create_window_surface = droid_create_window_surface,
> @@ -1384,7 +1403,11 @@ dri2_initialize_android(_EGLDriver *drv,
> _EGLDisplay *disp)
>
>     disp->DriverData = (void *) dri2_dpy;
>
> +   #ifdef HAVE_DRM_GRALLOC
> +   dri2_dpy->fd = droid_open_device_drm_gralloc(dri2_dpy);
> +   #else
>     dri2_dpy->fd = droid_open_device(disp);
> +   #endif
>     if (dri2_dpy->fd < 0) {
>        err = "DRI2: failed to open device";
>        goto cleanup;
> --
> 2.17.1
>
>
Hey Mauro,

Thanks for catching this.

On 14/08/2018 22.27, Mauro Rossi wrote:
> This patch fixes a regression in mesa 18.2 and mesa-dev branches
> for HAVE_DRM_GRALLOC code path which is causing black screen on Android
> and prevents boot due to SIGSEGV MAPERR crash related to unproper handling
> of drm_gralloc drm FD in new droid_open_device() path.
> 
> The problem due to c7bb82136b ("egl/android: Add DRM node probing and filtering")
> 
> ...  3173  3307 D GRALLOC-DRM: drmOpen radeon: 71
> ...  3173  3307 I GRALLOC-RADEON: detected chipset 0x6841 family 0x31 (vram size 238MiB, gart size 1021MiB)
> ...  3173  3307 I GRALLOC-DRM: create radeon for driver radeon
> ...  3173  3307 W EGL-MAIN: Could not get native buffer FD
> --------- beginning of crash
> ...  3173  3307 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault addr 0x18 in tid 3307 (RenderThread), pid 3173 (ndroid.systemui)
> ...     0     0 D         : [drm:radeon_crtc_page_flip_target [radeon]] flip-ioctl() cur_rbo = 000000003512328a, new_rbo = 000000
> ...  3420  3420 I crash_dump64: performing dump of process 3173 (target tid = 3307)
> ...  3420  3420 F DEBUG   : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
> ...  3420  3420 F DEBUG   : Build fingerprint: 'Android-x86/android_x86_64/x86_64:8.1.0/OPM6.171019.030.E1/uten07210645:userdebug/test-keys'
> ...  3420  3420 F DEBUG   : Revision: '0'
> ...  3420  3420 F DEBUG   : ABI: 'x86_64'
> ...  3420  3420 F DEBUG   : pid: 3173, tid: 3307, name: RenderThread  >>> com.android.systemui <<<
> ...  3420  3420 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x18
> ...  3420  3420 F DEBUG   : Cause: null pointer dereference
> ...  3420  3420 F DEBUG   :     rax 0000000000000000  rbx 00007c6ac3a3eee0  rcx 0000000000000000  rdx 0000000000000038
> ...  3420  3420 F DEBUG   :     rsi 0000000000000000  rdi 00007c6ab5bfeaa0
> ...  3420  3420 F DEBUG   :     r8  00007c6ab04c16e4  r9  00007c6b4ee6a220  r10 0000000000000000  r11 0000000000000200
> ...  3420  3420 F DEBUG   :     r12 0000000000000000  r13 0000000000000000  r14 0000000000000001  r15 00007c6ab04c1600
> ...  3420  3420 F DEBUG   :     cs  0000000000000033  ss  000000000000002b
> ...  3420  3420 F DEBUG   :     rip 00007c6ab0cee444  rbp 00007c6ac3ae8400  rsp 00007c6ab5bfea80  eflags 0000000000010246
> ...  3420  3420 F DEBUG   :
> ...  3420  3420 F DEBUG   : backtrace:
> ...  3420  3420 F DEBUG   :     #00 pc 000000000056b444  /system/vendor/lib64/dri/gallium_dri.so (st_update_framebuffer_state+660)
> ...  3420  3420 F DEBUG   :     #01 pc 0000000000569d61  /system/vendor/lib64/dri/gallium_dri.so (st_validate_state+561)
> ...  3420  3420 F DEBUG   :     #02 pc 0000000000572374  /system/vendor/lib64/dri/gallium_dri.so (st_Clear+116)
> ...  3420  3420 F DEBUG   :     #03 pc 000000000004a9c0  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #04 pc 0000000000085cab  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #05 pc 0000000000085f31  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #06 pc 000000000006e8c1  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #07 pc 000000000006e3d9  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #08 pc 000000000006c4e3  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #09 pc 00000000000704c8  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #10 pc 0000000000077fb9  /android/system/lib64/libhwui.so
> ...  3420  3420 F DEBUG   :     #11 pc 00000000000117fa  /android/system/lib64/libutils.so
> ...  3420  3420 F DEBUG   :     #12 pc 00000000000ba193  /android/system/lib64/libandroid_runtime.so
> ...  3420  3420 F DEBUG   :     #13 pc 0000000000079f0b  /android/system/lib64/libc.so
> ...  3420  3420 F DEBUG   :     #14 pc 0000000000028c5d  /android/system/lib64/libc.so
> ...  3420  3420 F DEBUG   :     #15 pc 0000000000027555  /android/system/lib64/libc.so
> 
> To avoid the crash the former existing working droid_open_device() is restored,
> renamed droid_open_device_drm_gralloc() and kept within HAVE_DRM_GRALLOC braces.
> 
> NOTE: Definition of enum{} for GRALLOC_MODULE_PERFORM_GET_DRM_FD
> is not necessary and it is actually causing a redefinition building error,
> because in HAVE_DRM_GRALLOC path gralloc_drm.h is already exported
> by libgralloc_drm which is currently still a dependency.
> 
> Tested with mesa-dev and mesa 18.2 branch and oreo-x86 bootanimation
> and Androdi GUI booting is fixed with i965, nouveau, radeon.
> 
> The changes are compatible with gbm_gralloc, I've tested build with hwc too.

I would maybe consider shortening the commit message a little bit, or at least 
remove
the crash-logs.

> 
> Fixes: c7bb82136b ("egl/android: Add DRM node probing and filtering")
> Cc: "18.2" <mesa-stable@lists.freedesktop.org>
> Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> ---
>   src/egl/drivers/dri2/platform_android.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
> 
> diff --git a/src/egl/drivers/dri2/platform_android.c b/src/egl/drivers/dri2/platform_android.c
> index cc16fd8118..834bbd258e 100644
> --- a/src/egl/drivers/dri2/platform_android.c
> +++ b/src/egl/drivers/dri2/platform_android.c
> @@ -1134,6 +1134,25 @@ droid_add_configs_for_visuals(_EGLDriver *drv, _EGLDisplay *dpy)
>      return (config_count != 0);
>   }
>   
> +#ifdef HAVE_DRM_GRALLOC
> +static int
> +droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy)
> +{
> +   int fd = -1, err = -EINVAL;
> +
> +   if (dri2_dpy->gralloc->perform)
> +         err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
> +                                          GRALLOC_MODULE_PERFORM_GET_DRM_FD,
> +                                          &fd);
> +   if (err || fd < 0) {
> +      _eglLog(_EGL_WARNING, "fail to get drm fd");
> +      fd = -1;
> +   }
> +
> +   return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> +}
> +#endif /* HAVE_DRM_GRALLOC */
> +
>   static const struct dri2_egl_display_vtbl droid_display_vtbl = {
>      .authenticate = NULL,
>      .create_window_surface = droid_create_window_surface,
> @@ -1384,7 +1403,11 @@ dri2_initialize_android(_EGLDriver *drv, _EGLDisplay *disp)
>   
>      disp->DriverData = (void *) dri2_dpy;
>   
> +   #ifdef HAVE_DRM_GRALLOC
> +   dri2_dpy->fd = droid_open_device_drm_gralloc(dri2_dpy);
> +   #else
>      dri2_dpy->fd = droid_open_device(disp);
> +   #endif
>      if (dri2_dpy->fd < 0) {
>         err = "DRI2: failed to open device";
>         goto cleanup;
> 

This does seem fairly un-intrusive if the GRALLOC_MODULE_PERFORM_GET_DRM_FD 
define is already being provided by drm_gralloc.

If we are going to support the drm_gralloc usecase and this patch is needed
to do so, I'm all for it.

With the above suggestion fixed:
Reviewed-by: Robert Foss <robert.foss@collabora.com>


Rob.
Hi Robert,
Il giorno mer 15 ago 2018 alle ore 09:37 Robert Foss <
robert.foss@collabora.com> ha scritto:

> Hey Mauro,
>
> Thanks for catching this.
>
> On 14/08/2018 22.27, Mauro Rossi wrote:
> > This patch fixes a regression in mesa 18.2 and mesa-dev branches
> > for HAVE_DRM_GRALLOC code path which is causing black screen on Android
> > and prevents boot due to SIGSEGV MAPERR crash related to unproper
> handling
> > of drm_gralloc drm FD in new droid_open_device() path.
> >
> > The problem due to c7bb82136b ("egl/android: Add DRM node probing and
> filtering")
> >
> > ...  3173  3307 D GRALLOC-DRM: drmOpen radeon: 71
> > ...  3173  3307 I GRALLOC-RADEON: detected chipset 0x6841 family 0x31
> (vram size 238MiB, gart size 1021MiB)
> > ...  3173  3307 I GRALLOC-DRM: create radeon for driver radeon
> > ...  3173  3307 W EGL-MAIN: Could not get native buffer FD
> > --------- beginning of crash
> > ...  3173  3307 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault
> addr 0x18 in tid 3307 (RenderThread), pid 3173 (ndroid.systemui)
> > ...     0     0 D         : [drm:radeon_crtc_page_flip_target [radeon]]
> flip-ioctl() cur_rbo = 000000003512328a, new_rbo = 000000
> > ...  3420  3420 I crash_dump64: performing dump of process 3173 (target
> tid = 3307)
> > ...  3420  3420 F DEBUG   : *** *** *** *** *** *** *** *** *** *** ***
> *** *** *** *** ***
> > ...  3420  3420 F DEBUG   : Build fingerprint:
> 'Android-x86/android_x86_64/x86_64:8.1.0/OPM6.171019.030.E1/uten07210645:userdebug/test-keys'
> > ...  3420  3420 F DEBUG   : Revision: '0'
> > ...  3420  3420 F DEBUG   : ABI: 'x86_64'
> > ...  3420  3420 F DEBUG   : pid: 3173, tid: 3307, name: RenderThread
> >>> com.android.systemui <<<
> > ...  3420  3420 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR),
> fault addr 0x18
> > ...  3420  3420 F DEBUG   : Cause: null pointer dereference
> > ...  3420  3420 F DEBUG   :     rax 0000000000000000  rbx
> 00007c6ac3a3eee0  rcx 0000000000000000  rdx 0000000000000038
> > ...  3420  3420 F DEBUG   :     rsi 0000000000000000  rdi
> 00007c6ab5bfeaa0
> > ...  3420  3420 F DEBUG   :     r8  00007c6ab04c16e4  r9
> 00007c6b4ee6a220  r10 0000000000000000  r11 0000000000000200
> > ...  3420  3420 F DEBUG   :     r12 0000000000000000  r13
> 0000000000000000  r14 0000000000000001  r15 00007c6ab04c1600
> > ...  3420  3420 F DEBUG   :     cs  0000000000000033  ss
> 000000000000002b
> > ...  3420  3420 F DEBUG   :     rip 00007c6ab0cee444  rbp
> 00007c6ac3ae8400  rsp 00007c6ab5bfea80  eflags 0000000000010246
> > ...  3420  3420 F DEBUG   :
> > ...  3420  3420 F DEBUG   : backtrace:
> > ...  3420  3420 F DEBUG   :     #00 pc 000000000056b444
> /system/vendor/lib64/dri/gallium_dri.so (st_update_framebuffer_state+660)
> > ...  3420  3420 F DEBUG   :     #01 pc 0000000000569d61
> /system/vendor/lib64/dri/gallium_dri.so (st_validate_state+561)
> > ...  3420  3420 F DEBUG   :     #02 pc 0000000000572374
> /system/vendor/lib64/dri/gallium_dri.so (st_Clear+116)
> > ...  3420  3420 F DEBUG   :     #03 pc 000000000004a9c0
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #04 pc 0000000000085cab
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #05 pc 0000000000085f31
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #06 pc 000000000006e8c1
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #07 pc 000000000006e3d9
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #08 pc 000000000006c4e3
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #09 pc 00000000000704c8
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #10 pc 0000000000077fb9
> /android/system/lib64/libhwui.so
> > ...  3420  3420 F DEBUG   :     #11 pc 00000000000117fa
> /android/system/lib64/libutils.so
> > ...  3420  3420 F DEBUG   :     #12 pc 00000000000ba193
> /android/system/lib64/libandroid_runtime.so
> > ...  3420  3420 F DEBUG   :     #13 pc 0000000000079f0b
> /android/system/lib64/libc.so
> > ...  3420  3420 F DEBUG   :     #14 pc 0000000000028c5d
> /android/system/lib64/libc.so
> > ...  3420  3420 F DEBUG   :     #15 pc 0000000000027555
> /android/system/lib64/libc.so
> >
> > To avoid the crash the former existing working droid_open_device() is
> restored,
> > renamed droid_open_device_drm_gralloc() and kept within HAVE_DRM_GRALLOC
> braces.
> >
> > NOTE: Definition of enum{} for GRALLOC_MODULE_PERFORM_GET_DRM_FD
> > is not necessary and it is actually causing a redefinition building
> error,
> > because in HAVE_DRM_GRALLOC path gralloc_drm.h is already exported
> > by libgralloc_drm which is currently still a dependency.
> >
> > Tested with mesa-dev and mesa 18.2 branch and oreo-x86 bootanimation
> > and Androdi GUI booting is fixed with i965, nouveau, radeon.
> >
> > The changes are compatible with gbm_gralloc, I've tested build with hwc
> too.
>
> I would maybe consider shortening the commit message a little bit, or at
> least
> remove
> the crash-logs.
>

Of course, I will reduce the final commit message.

The segfault log was provided as a mean to understand what went wrong,
in case you/other developers will pursue the goal of having
one droid_open_device() procedure
applicable to all gralloc implementation

and I will be available to test there is no regression on android-x86
drm_gralloc.
Kind regards

Mauro


>
> >
> > Fixes: c7bb82136b ("egl/android: Add DRM node probing and filtering")
> > Cc: "18.2" <mesa-stable@lists.freedesktop.org>
> > Signed-off-by: Mauro Rossi <issor.oruam@gmail.com>
> > ---
> >   src/egl/drivers/dri2/platform_android.c | 23 +++++++++++++++++++++++
> >   1 file changed, 23 insertions(+)
> >
> > diff --git a/src/egl/drivers/dri2/platform_android.c
> b/src/egl/drivers/dri2/platform_android.c
> > index cc16fd8118..834bbd258e 100644
> > --- a/src/egl/drivers/dri2/platform_android.c
> > +++ b/src/egl/drivers/dri2/platform_android.c
> > @@ -1134,6 +1134,25 @@ droid_add_configs_for_visuals(_EGLDriver *drv,
> _EGLDisplay *dpy)
> >      return (config_count != 0);
> >   }
> >
> > +#ifdef HAVE_DRM_GRALLOC
> > +static int
> > +droid_open_device_drm_gralloc(struct dri2_egl_display *dri2_dpy)
> > +{
> > +   int fd = -1, err = -EINVAL;
> > +
> > +   if (dri2_dpy->gralloc->perform)
> > +         err = dri2_dpy->gralloc->perform(dri2_dpy->gralloc,
> > +
> GRALLOC_MODULE_PERFORM_GET_DRM_FD,
> > +                                          &fd);
> > +   if (err || fd < 0) {
> > +      _eglLog(_EGL_WARNING, "fail to get drm fd");
> > +      fd = -1;
> > +   }
> > +
> > +   return (fd >= 0) ? fcntl(fd, F_DUPFD_CLOEXEC, 3) : -1;
> > +}
> > +#endif /* HAVE_DRM_GRALLOC */
> > +
> >   static const struct dri2_egl_display_vtbl droid_display_vtbl = {
> >      .authenticate = NULL,
> >      .create_window_surface = droid_create_window_surface,
> > @@ -1384,7 +1403,11 @@ dri2_initialize_android(_EGLDriver *drv,
> _EGLDisplay *disp)
> >
> >      disp->DriverData = (void *) dri2_dpy;
> >
> > +   #ifdef HAVE_DRM_GRALLOC
> > +   dri2_dpy->fd = droid_open_device_drm_gralloc(dri2_dpy);
> > +   #else
> >      dri2_dpy->fd = droid_open_device(disp);
> > +   #endif
> >      if (dri2_dpy->fd < 0) {
> >         err = "DRI2: failed to open device";
> >         goto cleanup;
> >
>
> This does seem fairly un-intrusive if the
> GRALLOC_MODULE_PERFORM_GET_DRM_FD
> define is already being provided by drm_gralloc.
>
> If we are going to support the drm_gralloc usecase and this patch is needed
> to do so, I'm all for it.
>
> With the above suggestion fixed:
> Reviewed-by: Robert Foss <robert.foss@collabora.com>
>
>
> Rob.
>
On 15 August 2018 at 09:13, Mauro Rossi <issor.oruam@gmail.com> wrote:
> Hi Robert,
> Il giorno mer 15 ago 2018 alle ore 09:37 Robert Foss
> <robert.foss@collabora.com> ha scritto:
>>
>> Hey Mauro,
>>
>> Thanks for catching this.
>>
>> On 14/08/2018 22.27, Mauro Rossi wrote:
>> > This patch fixes a regression in mesa 18.2 and mesa-dev branches
>> > for HAVE_DRM_GRALLOC code path which is causing black screen on Android
>> > and prevents boot due to SIGSEGV MAPERR crash related to unproper
>> > handling
>> > of drm_gralloc drm FD in new droid_open_device() path.
>> >
>> > The problem due to c7bb82136b ("egl/android: Add DRM node probing and
>> > filtering")
>> >
>> > ...  3173  3307 D GRALLOC-DRM: drmOpen radeon: 71
>> > ...  3173  3307 I GRALLOC-RADEON: detected chipset 0x6841 family 0x31
>> > (vram size 238MiB, gart size 1021MiB)
>> > ...  3173  3307 I GRALLOC-DRM: create radeon for driver radeon
>> > ...  3173  3307 W EGL-MAIN: Could not get native buffer FD
>> > --------- beginning of crash
>> > ...  3173  3307 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault
>> > addr 0x18 in tid 3307 (RenderThread), pid 3173 (ndroid.systemui)
>> > ...     0     0 D         : [drm:radeon_crtc_page_flip_target [radeon]]
>> > flip-ioctl() cur_rbo = 000000003512328a, new_rbo = 000000
>> > ...  3420  3420 I crash_dump64: performing dump of process 3173 (target
>> > tid = 3307)
>> > ...  3420  3420 F DEBUG   : *** *** *** *** *** *** *** *** *** *** ***
>> > *** *** *** *** ***
>> > ...  3420  3420 F DEBUG   : Build fingerprint:
>> > 'Android-x86/android_x86_64/x86_64:8.1.0/OPM6.171019.030.E1/uten07210645:userdebug/test-keys'
>> > ...  3420  3420 F DEBUG   : Revision: '0'
>> > ...  3420  3420 F DEBUG   : ABI: 'x86_64'
>> > ...  3420  3420 F DEBUG   : pid: 3173, tid: 3307, name: RenderThread
>> > >>> com.android.systemui <<<
>> > ...  3420  3420 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR),
>> > fault addr 0x18
>> > ...  3420  3420 F DEBUG   : Cause: null pointer dereference
>> > ...  3420  3420 F DEBUG   :     rax 0000000000000000  rbx
>> > 00007c6ac3a3eee0  rcx 0000000000000000  rdx 0000000000000038
>> > ...  3420  3420 F DEBUG   :     rsi 0000000000000000  rdi
>> > 00007c6ab5bfeaa0
>> > ...  3420  3420 F DEBUG   :     r8  00007c6ab04c16e4  r9
>> > 00007c6b4ee6a220  r10 0000000000000000  r11 0000000000000200
>> > ...  3420  3420 F DEBUG   :     r12 0000000000000000  r13
>> > 0000000000000000  r14 0000000000000001  r15 00007c6ab04c1600
>> > ...  3420  3420 F DEBUG   :     cs  0000000000000033  ss
>> > 000000000000002b
>> > ...  3420  3420 F DEBUG   :     rip 00007c6ab0cee444  rbp
>> > 00007c6ac3ae8400  rsp 00007c6ab5bfea80  eflags 0000000000010246
>> > ...  3420  3420 F DEBUG   :
>> > ...  3420  3420 F DEBUG   : backtrace:
>> > ...  3420  3420 F DEBUG   :     #00 pc 000000000056b444
>> > /system/vendor/lib64/dri/gallium_dri.so (st_update_framebuffer_state+660)
>> > ...  3420  3420 F DEBUG   :     #01 pc 0000000000569d61
>> > /system/vendor/lib64/dri/gallium_dri.so (st_validate_state+561)
>> > ...  3420  3420 F DEBUG   :     #02 pc 0000000000572374
>> > /system/vendor/lib64/dri/gallium_dri.so (st_Clear+116)
>> > ...  3420  3420 F DEBUG   :     #03 pc 000000000004a9c0
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #04 pc 0000000000085cab
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #05 pc 0000000000085f31
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #06 pc 000000000006e8c1
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #07 pc 000000000006e3d9
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #08 pc 000000000006c4e3
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #09 pc 00000000000704c8
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #10 pc 0000000000077fb9
>> > /android/system/lib64/libhwui.so
>> > ...  3420  3420 F DEBUG   :     #11 pc 00000000000117fa
>> > /android/system/lib64/libutils.so
>> > ...  3420  3420 F DEBUG   :     #12 pc 00000000000ba193
>> > /android/system/lib64/libandroid_runtime.so
>> > ...  3420  3420 F DEBUG   :     #13 pc 0000000000079f0b
>> > /android/system/lib64/libc.so
>> > ...  3420  3420 F DEBUG   :     #14 pc 0000000000028c5d
>> > /android/system/lib64/libc.so
>> > ...  3420  3420 F DEBUG   :     #15 pc 0000000000027555
>> > /android/system/lib64/libc.so
>> >
>> > To avoid the crash the former existing working droid_open_device() is
>> > restored,
>> > renamed droid_open_device_drm_gralloc() and kept within HAVE_DRM_GRALLOC
>> > braces.
>> >
>> > NOTE: Definition of enum{} for GRALLOC_MODULE_PERFORM_GET_DRM_FD
>> > is not necessary and it is actually causing a redefinition building
>> > error,
>> > because in HAVE_DRM_GRALLOC path gralloc_drm.h is already exported
>> > by libgralloc_drm which is currently still a dependency.
>> >
>> > Tested with mesa-dev and mesa 18.2 branch and oreo-x86 bootanimation
>> > and Androdi GUI booting is fixed with i965, nouveau, radeon.
>> >
>> > The changes are compatible with gbm_gralloc, I've tested build with hwc
>> > too.
>>
>> I would maybe consider shortening the commit message a little bit, or at
>> least
>> remove
>> the crash-logs.
>
>
> Of course, I will reduce the final commit message.
>
> The segfault log was provided as a mean to understand what went wrong,
> in case you/other developers will pursue the goal of having one
> droid_open_device() procedure
> applicable to all gralloc implementation
>
> and I will be available to test there is no regression on android-x86
> drm_gralloc.

Good call on having the crash log in the original submission. Extra
information can trivially be removed ;-)
With that and the small nitpick below the patch is:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

Personally I won't bother re-sending the patch. Simply fixup locally and commit.

>> >
>> > +   #ifdef HAVE_DRM_GRALLOC
Please move this (and the else/endif) at the start of the line. We
don't indent pre-processor directives in Mesa (modulo odd case that
slipped).

Thanks
Emil
Hi Emil,

Il Mer 15 Ago 2018, 13:16 Emil Velikov <emil.l.velikov@gmail.com> ha
scritto:

> On 15 August 2018 at 09:13, Mauro Rossi <issor.oruam@gmail.com> wrote:
> > Hi Robert,
> > Il giorno mer 15 ago 2018 alle ore 09:37 Robert Foss
> > <robert.foss@collabora.com> ha scritto:
> >>
> >> Hey Mauro,
> >>
> >> Thanks for catching this.
> >>
> >> On 14/08/2018 22.27, Mauro Rossi wrote:
> >> > This patch fixes a regression in mesa 18.2 and mesa-dev branches
> >> > for HAVE_DRM_GRALLOC code path which is causing black screen on
> Android
> >> > and prevents boot due to SIGSEGV MAPERR crash related to unproper
> >> > handling
> >> > of drm_gralloc drm FD in new droid_open_device() path.
> >> >
> >> > The problem due to c7bb82136b ("egl/android: Add DRM node probing and
> >> > filtering")
> >> >
> >> > ...  3173  3307 D GRALLOC-DRM: drmOpen radeon: 71
> >> > ...  3173  3307 I GRALLOC-RADEON: detected chipset 0x6841 family 0x31
> >> > (vram size 238MiB, gart size 1021MiB)
> >> > ...  3173  3307 I GRALLOC-DRM: create radeon for driver radeon
> >> > ...  3173  3307 W EGL-MAIN: Could not get native buffer FD
> >> > --------- beginning of crash
> >> > ...  3173  3307 F libc    : Fatal signal 11 (SIGSEGV), code 1, fault
> >> > addr 0x18 in tid 3307 (RenderThread), pid 3173 (ndroid.systemui)
> >> > ...     0     0 D         : [drm:radeon_crtc_page_flip_target
> [radeon]]
> >> > flip-ioctl() cur_rbo = 000000003512328a, new_rbo = 000000
> >> > ...  3420  3420 I crash_dump64: performing dump of process 3173
> (target
> >> > tid = 3307)
> >> > ...  3420  3420 F DEBUG   : *** *** *** *** *** *** *** *** *** ***
> ***
> >> > *** *** *** *** ***
> >> > ...  3420  3420 F DEBUG   : Build fingerprint:
> >> >
> 'Android-x86/android_x86_64/x86_64:8.1.0/OPM6.171019.030.E1/uten07210645:userdebug/test-keys'
> >> > ...  3420  3420 F DEBUG   : Revision: '0'
> >> > ...  3420  3420 F DEBUG   : ABI: 'x86_64'
> >> > ...  3420  3420 F DEBUG   : pid: 3173, tid: 3307, name: RenderThread
> >> > >>> com.android.systemui <<<
> >> > ...  3420  3420 F DEBUG   : signal 11 (SIGSEGV), code 1 (SEGV_MAPERR),
> >> > fault addr 0x18
> >> > ...  3420  3420 F DEBUG   : Cause: null pointer dereference
> >> > ...  3420  3420 F DEBUG   :     rax 0000000000000000  rbx
> >> > 00007c6ac3a3eee0  rcx 0000000000000000  rdx 0000000000000038
> >> > ...  3420  3420 F DEBUG   :     rsi 0000000000000000  rdi
> >> > 00007c6ab5bfeaa0
> >> > ...  3420  3420 F DEBUG   :     r8  00007c6ab04c16e4  r9
> >> > 00007c6b4ee6a220  r10 0000000000000000  r11 0000000000000200
> >> > ...  3420  3420 F DEBUG   :     r12 0000000000000000  r13
> >> > 0000000000000000  r14 0000000000000001  r15 00007c6ab04c1600
> >> > ...  3420  3420 F DEBUG   :     cs  0000000000000033  ss
> >> > 000000000000002b
> >> > ...  3420  3420 F DEBUG   :     rip 00007c6ab0cee444  rbp
> >> > 00007c6ac3ae8400  rsp 00007c6ab5bfea80  eflags 0000000000010246
> >> > ...  3420  3420 F DEBUG   :
> >> > ...  3420  3420 F DEBUG   : backtrace:
> >> > ...  3420  3420 F DEBUG   :     #00 pc 000000000056b444
> >> > /system/vendor/lib64/dri/gallium_dri.so
> (st_update_framebuffer_state+660)
> >> > ...  3420  3420 F DEBUG   :     #01 pc 0000000000569d61
> >> > /system/vendor/lib64/dri/gallium_dri.so (st_validate_state+561)
> >> > ...  3420  3420 F DEBUG   :     #02 pc 0000000000572374
> >> > /system/vendor/lib64/dri/gallium_dri.so (st_Clear+116)
> >> > ...  3420  3420 F DEBUG   :     #03 pc 000000000004a9c0
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #04 pc 0000000000085cab
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #05 pc 0000000000085f31
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #06 pc 000000000006e8c1
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #07 pc 000000000006e3d9
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #08 pc 000000000006c4e3
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #09 pc 00000000000704c8
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #10 pc 0000000000077fb9
> >> > /android/system/lib64/libhwui.so
> >> > ...  3420  3420 F DEBUG   :     #11 pc 00000000000117fa
> >> > /android/system/lib64/libutils.so
> >> > ...  3420  3420 F DEBUG   :     #12 pc 00000000000ba193
> >> > /android/system/lib64/libandroid_runtime.so
> >> > ...  3420  3420 F DEBUG   :     #13 pc 0000000000079f0b
> >> > /android/system/lib64/libc.so
> >> > ...  3420  3420 F DEBUG   :     #14 pc 0000000000028c5d
> >> > /android/system/lib64/libc.so
> >> > ...  3420  3420 F DEBUG   :     #15 pc 0000000000027555
> >> > /android/system/lib64/libc.so
> >> >
> >> > To avoid the crash the former existing working droid_open_device() is
> >> > restored,
> >> > renamed droid_open_device_drm_gralloc() and kept within
> HAVE_DRM_GRALLOC
> >> > braces.
> >> >
> >> > NOTE: Definition of enum{} for GRALLOC_MODULE_PERFORM_GET_DRM_FD
> >> > is not necessary and it is actually causing a redefinition building
> >> > error,
> >> > because in HAVE_DRM_GRALLOC path gralloc_drm.h is already exported
> >> > by libgralloc_drm which is currently still a dependency.
> >> >
> >> > Tested with mesa-dev and mesa 18.2 branch and oreo-x86 bootanimation
> >> > and Androdi GUI booting is fixed with i965, nouveau, radeon.
> >> >
> >> > The changes are compatible with gbm_gralloc, I've tested build with
> hwc
> >> > too.
> >>
> >> I would maybe consider shortening the commit message a little bit, or at
> >> least
> >> remove
> >> the crash-logs.
> >
> >
> > Of course, I will reduce the final commit message.
> >
> > The segfault log was provided as a mean to understand what went wrong,
> > in case you/other developers will pursue the goal of having one
> > droid_open_device() procedure
> > applicable to all gralloc implementation
> >
> > and I will be available to test there is no regression on android-x86
> > drm_gralloc.
>
> Good call on having the crash log in the original submission. Extra
> information can trivially be removed ;-)
> With that and the small nitpick below the patch is:
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
>
> Personally I won't bother re-sending the patch. Simply fixup locally and
> commit.
>
> >> >
> >> > +   #ifdef HAVE_DRM_GRALLOC
> Please move this (and the else/endif) at the start of the line. We
> don't indent pre-processor directives in Mesa (modulo odd case that
> slipped).
>

Thanks, I had the doubt about those indentations.

I will send an additional patch to align HAVE_DRM_GRALLOC directives.

Mauro


> Thanks
> Emil
>