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

[Tapani Pälli]

On 03/30/2017 05:57 PM, Emil Velikov wrote:
> 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.

I'm trying to debug why this causes our homescreen (wallpaper) to be 
black. Otherwise I haven't seen any issues with these changes.

Thanks;

// Tapani