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

[Rob Clark]

On Fri, Mar 31, 2017 at 2:06 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>
>
> On 03/31/2017 08:24 AM, Rob Clark wrote:
>>
>> On Fri, Mar 31, 2017 at 12:22 AM, Tapani Pälli <tapani.palli at intel.com>
>> wrote:
>>>
>>>
>>>
>>> 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.
>>>
>>
>> wallpaper seems to be a special sorta hell..  I wonder if there is
>> somehow some sort of interaction with what I fixed / worked-around in
>> a5e733c6b52e93de3000647d075f5ca2f55fcb71 ??
>>
>> Maybe at least try commenting out the temp-pbuffer thing to get max
>> texture size, and see if that "fixes" things
>
>
> Can you give more details, I still live in la la land and don't know about
> 'temp-pbuffer thing'?


Sorry, this is something in the wallpaper app java code.. I forget
exactly where it lives now, but there was some code matching what I
pasted in the commit msg which was creating and then destroying a
temporary pbuffer..

BR,
-R