<div dir="auto"><div>Hi Emil, <div class="gmail_quote"><div dir="ltr">Il Mer 15 Ago 2018, 13:16 Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>> ha scritto: </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 15 August 2018 at 09:13, Mauro Rossi <<a href="mailto:issor.oruam@gmail.com" target="_blank" rel="noreferrer">issor.oruam@gmail.com</a>> wrote: > Hi Robert, > Il giorno mer 15 ago 2018 alle ore 09:37 Robert Foss > <<a href="mailto:robert.foss@collabora.com" target="_blank" rel="noreferrer">robert.foss@collabora.com</a>> 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 <<a href="mailto:emil.velikov@collabora.com" target="_blank" rel="noreferrer">emil.velikov@collabora.com</a>> 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). </blockquote></div></div><div dir="auto"> </div><div dir="auto">Thanks, I had the doubt about those indentations.</div><div dir="auto"> </div><div dir="auto">I will send an additional patch to align HAVE_DRM_GRALLOC directives.</div><div dir="auto"> </div><div dir="auto">Mauro</div><div dir="auto"> </div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks Emil </blockquote></div></div></div>