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

[Tomasz Figa]

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.

Best regards,
Tomasz