[Mesa-dev] [RFC PATCH] egl/android: Dequeue buffers inside EGL calls
Mauro Rossi
issor.oruam at gmail.com
Thu Apr 6 18:13:31 UTC 2017
2017-04-04 0:54 GMT+02:00 Mauro Rossi <issor.oruam at gmail.com>:
> Hi Tomasz,
>
>
> 2017-04-03 10:00 GMT+02:00 Tomasz Figa <tfiga at chromium.org>:
>
>> 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/driver
>> s/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
>>
>
> With this change applied the wallpaper is still black
> Mauro
>
Is there some debug strategy or some ad hoc line of code I could apply to
help assess the problem?
Or some apitrace/other tracing tool (like Android Studio's one in
conjuction with some app) I could use to find the root cause of the issue?
Or some set of images to set as wallpaper I could try to use to assess if
the problem is due to FourCC issue or to wallpaper size?
Thanks
Mauro
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170406/1ec19360/attachment-0001.html>
More information about the mesa-dev
mailing list