[Mesa-dev] [PATCH 1/2] egl/android: fix regression in drm_gralloc path

Emil Velikov emil.l.velikov at gmail.com
Wed Aug 15 11:16:26 UTC 2018


On 15 August 2018 at 09:13, Mauro Rossi <issor.oruam at gmail.com> wrote:
> Hi Robert,
> Il giorno mer 15 ago 2018 alle ore 09:37 Robert Foss
> <robert.foss at 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 at 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


More information about the mesa-dev mailing list