Hi Robert, Il giorno mer 15 ago 2018 alle ore 09:37 Robert Foss < robert.f...@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-sta...@lists.freedesktop.org> > > Signed-off-by: Mauro Rossi <issor.or...@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.f...@collabora.com> > > > Rob. >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev