[Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls
Tomasz Figa
tfiga at chromium.org
Mon Apr 3 08:00:15 UTC 2017
Hi Mauro,
On Mon, Apr 3, 2017 at 2:48 AM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>
>
> 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/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
>
>
>
> 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
I found that the code was missing acquire fence wait in case of the
DRI loader (droid_get_buffers_parse_attachments()). Would you be able
to add the following chunk and check if the wallpaper is still broken?
diff --git a/src/egl/drivers/dri2/platform_android.c
b/src/egl/drivers/dri2/platform_android.c
index 35aba3a7f0..dc29cc24b2 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -917,6 +917,10 @@ droid_get_buffers_parse_attachments(struct
dri2_egl_surface *dri2_surf,
if (!dri2_surf->buffer)
continue;
+ /* Android might have given us an acquire fence to wait for. If so,
+ * we need to wait for it and close the descriptor after that. */
+ wait_and_close_acquire_fence(dri2_surf);
+
buf->attachment = attachments[i];
buf->name = get_native_buffer_name(dri2_surf->buffer);
buf->cpp = get_format_bpp(dri2_surf->buffer->format);
Best regards,
Tomasz
More information about the mesa-dev
mailing list