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

[Emil Velikov]

On 30 March 2017 at 15:30, Tomasz Figa <tfiga at chromium.org> wrote:
> On Thu, Mar 30, 2017 at 11:17 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>
>> 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 ?
>
> Ah, I actually noticed a problem with existing code, supposedly fixed
> by [1], but I'm afraid it's still wrong.
>
> Current swap_buffers calls get_back_bo(), but doesn't call
> update_buffers(), which is the function that should be called before
> to actually dequeue a buffer from Android's buffer queue. Given that,
> get_back_bo() would simply fail with !dri2_surf->buffer, because no
> buffer was dequeued.
>
Right - I was wondering why we don't hit that on EGL/GBM or EGL/Wayland.
>From a quick look - may be because EGL/Android drops the dpy mutex in
droid_window_enqueue_buffer().

> My patch removes update_buffers() and changes the buffer management so
> that there is always a buffer dequeued, starting from surface
> creation, unless there was an error somewhere.
>
Of the top of your head - is there something stopping us from using
the same method on $other platforms?

> [1] https://cgit.freedesktop.org/mesa/mesa/commit/src/egl/drivers/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb581149dbe1597
>
>>
>>
>> Not that huge of an expert on the Android specifics, so just a humble request:
>> Can we seek the code resuffle (droid_{alloc,free}_local_buffer,
Oops silly typo - s/seek/split/.

>> other?) separate from the functionality changes ?
>
> Sure. Thanks for suggestion.
>
Please give it a day or two for others to comment.

Thanks
Emil