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

[Tapani Pälli]

On 03/31/2017 10:12 AM, Tapani Pälli wrote:
>
>
> On 03/31/2017 09:06 AM, Tapani Pälli 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'?
>>
>
> aa I did not recall the problem, you mean the 'dummy pbuffer' in
> SurfaceFlinger .. yes I will check if this is related.
>

If I take away that dummy pbuffer usage (which is useless anyway), 
couple of errors disappear from the log. They are:

SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)
SurfaceFlinger: releasePendingBuffer failed: Unknown error -1 (1)

but otherwise the desktop still stays black, live wallpapers seem to 
work so there is something special about this default wallpaper. Will 
continue digging ..

// Tapani