[Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls

Mon Apr 3 05:42:12 UTC 2017

On 04/02/2017 08:12 PM, Tomasz Figa wrote:
> Sorry for replying to myself, just got enlightened...
>
> On Mon, Apr 3, 2017 at 2:07 AM, Tomasz Figa <tfiga at chromium.org> wrote:
>> Hi Mauro,
>>
>> On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>>>
>>>
>>> 2017-03-30 16:17 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
>>>>
>>>> On 30 March 2017 at 11:55, Tomasz Figa <tfiga at chromium.org> wrote:
>>>>> Android buffer queues can be abandoned, which results in failing to
>>>>> dequeue next buffer. Currently this would fail somewhere deep within
>>>>> the DRI stack calling loader's getBuffers*(), without any error
>>>>> reporting to the client app. However Android framework code relies on
>>>>> proper signaling of this event, so we move buffer dequeue to
>>>>> createWindowSurface() and swapBuffers() call, which can generate proper
>>>>> EGL errors. To keep the performance benefits of delayed buffer handling,
>>>>> if any, fence wait and DRI image creation is kept delayed until
>>>>> getBuffers*() is called by the DRI driver.
>>>>>
>>>> Thank you Tomasz.
>>>>
>>>> I'm fairly confident that this should resolve the crash [in
>>>> swap_buffers] that Mauro was seeing.
>>>> Mauro can you give it a test ?
>>>
>>>
>>> After applying last version of Tomasz patch,
>>> I could not boot nougat-x86, the same way as per Tapani get_back_bo()
>>> throwing and EGL_BAD_ALLOC
>>> which is a show stopper for surfaceflinger
>>
>> Hmm, must be something I missed in the code, because with my patch
>> applied, there should be no condition that could make get_back_bo()
>> fail, unless previous swap_buffers() failed in droid_dequeue_buffer()
>> or there is something wrong with the first buffer being dequeued in
>> create_surface(). Would you be able to check where exactly
>> get_back_bo() fails with your setup?
>
> Ah, wait, I just realized that get_back_bo() is valid only when the
> image loader is used, which is only on DMA-buf FD-based systems. No
> wonder that it fails on android-x86.
>
> Tapani, would you be able to give a bit more details on the crash
> being observed without that call? AFAICT, without my patch, even with
> the call, the code is still not fully correct, because on the first
> call to swap_buffers() without any rendering the dri2_surf->buffer
> would be NULL and get_back_bo() would simply fail (but not crash
> indeed). With my patch, it won't fail, because there is always a
> buffer dequeued, so it should be the closest to correct behavior.

Reason why that crash started to happen (observed only in one app so 
far!) were changes done in 'EGL/Android: Add EGL_EXT_buffer_age 
extension'. That patch adds 'age' for buffers and modifies 
droid_swap_buffers to set back buffer age to value 1. My original patch 
was to set the age only if there is a back buffer but then discussed 
with Emil and decided to unify approach with other backends and call 
get_back_bo.

You are right that in the crash case dri2_surf->buffer is NULL as well.

for long time flow looks like this:

01-01 00:00:54.087  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c88788 dri2_surf->back 0xca35ede8
01-01 00:00:54.104  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8e84e08 dri2_surf->back 0xca35edd8
01-01 00:00:54.120  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c885a8 dri2_surf->back 0xca35ede0

then all of sudden there is a frame where buffer and back are 0:

01-01 00:00:55.386  2771  2791 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c885a8 dri2_surf->back 0xca35ede0
01-01 00:00:55.390  2771  2831 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0xc8c8b2a8 dri2_surf->back 0xca360098
01-01 00:00:55.391  2771  2831 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0x0 dri2_surf->back 0x0
01-01 00:00:55.395  1943  2088 D EGL-DRI2: droid_swap_buffers: 
dri2_surf->buffer 0x7c449f5ed610 dri2_surf->back 0x7c44b2cfc010

then we crash:

01-01 00:00:56.101  2836  2836 F DEBUG   : backtrace:
01-01 00:00:56.101  2836  2836 F DEBUG   :     #00 pc 00014046 
/system/lib/egl/libGLES_mesa.so (droid_swap_buffers+166)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #01 pc 00011832 
/system/lib/egl/libGLES_mesa.so (dri2_swap_buffers+50)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #02 pc 000058c2 
/system/lib/egl/libGLES_mesa.so (eglSwapBuffers+386)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #03 pc 00011329 
/system/lib/libEGL.so (eglSwapBuffersWithDamageKHR+553)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #04 pc 000118e7 
/system/lib/libEGL.so (eglSwapBuffers+55)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #05 pc 000754dc 
/system/lib/libandroid_runtime.so
01-01 00:00:56.101  2836  2836 F DEBUG   :     #06 pc 0249217c 
/system/framework/x86/boot-framework.oat (offset 0x1584000) 
(com.google.android.gles_jni.EGLImpl.eglSwapBuffers+168)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #07 pc 01cc8dec 
/system/framework/x86/boot-framework.oat (offset 0x1584000) 
(android.opengl.GLSurfaceView$EglHelper.swap+72)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #08 pc 01cc9ebe 
/system/framework/x86/boot-framework.oat (offset 0x1584000) 
(android.opengl.GLSurfaceView$GLThread.guardedRun+3882)
01-01 00:00:56.101  2836  2836 F DEBUG   :     #09 pc 01ccb794 
/system/framework/x86/boot-framework.oat (offset 0x1584000) 
(android.opengl.GLSurfaceView$GLThread.run+192)

// Tapani