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

Sun Apr 2 17:48:09 UTC 2017

2017-03-31 13:05 GMT+02:00 Tapani Pälli <tapani.palli at intel.com>:

>
>
> 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/driver
>>>>>>> s/dri2/platform_android.c?id=4d4558411db166d2d66f8cec9cb5811
>>>>>>> 49dbe1597
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>> 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
>

Hi,

about the black wallpaper the only sign found in logcat is the following:

--------- beginning of main
04-02 15:09:43.148
...
04-02 15:10:41.710  1414  1414 E Layer   :
[com.android.systemui.ImageWallpaper] rejecting buffer: bufWidth=1024,
bufHeight=768, front.active.{w=1, h=1}
...
04-02 15:10:41.726  1414  1414 E Layer   :
[com.android.systemui.ImageWallpaper] rejecting buffer: bufWidth=1024,
bufHeight=768, front.active.{w=1, h=1}

Are the expected width and height correct?

In dmesg at relative dmesg timestamp around 58 seconds there is no
signal/error,
just the selinux log (set to permissive):

[   58.271833] type=1400 audit(1491145841.554:192): avc: denied { ioctl }
for pid=2141 comm="ndroid.systemui" path="/dev/dri/card0" dev="tmpfs"
ino=7199 ioctlcmd=6467 scontext=u:r:platform_app:s0:c512,c768
tcontext=u:object_r:device:s0 tclass=chr_file permissive=1
[   58.271879] type=1400 audit(1491145841.554:193): avc: denied { read
write } for pid=2141 comm="ndroid.systemui" path="/dev/dri/card0"
dev="tmpfs" ino=7199 scontext=u:r:platform_app:s0:c512,c768
tcontext=u:object_r:device:s0 tclass=chr_file permissive=1

I hope these information may help
Mauro
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170402/dbcefcdb/attachment.html>