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

[Tomasz Figa]

Hi Mauro,

On Mon, Apr 3, 2017 at 1:38 AM, Mauro Rossi <issor.oruam at gmail.com> wrote:
>
>
> 2017-03-30 16:17 GMT+02:00 Emil Velikov <emil.l.velikov at gmail.com>:
>>
>> 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 ?
>
>
> After applying last version of Tomasz patch,
> I could not boot nougat-x86, the same way as per Tapani get_back_bo()
> throwing and EGL_BAD_ALLOC
> which is a show stopper for surfaceflinger

Hmm, must be something I missed in the code, because with my patch
applied, there should be no condition that could make get_back_bo()
fail, unless previous swap_buffers() failed in droid_dequeue_buffer()
or there is something wrong with the first buffer being dequeued in
create_surface(). Would you be able to check where exactly
get_back_bo() fails with your setup?

>
> So I reverted [1] and now I can boot and I also see the black wallpaper like
> Tapani.
> dumpsys SurfaceFlinger output shows a buffer allocated,
> but for some reason both HWC and GLES composition (used in nougat-x86) show
> black wallpaper.
>
> ----
> h/w composer state:
>   h/w composer not present and enabled
> Allocated buffers:
> ...
> 0x7b60f301e380: 29440.00 KiB | 2880 (2944) x 2560 |        5 | 0x00000900 |
> com.android.systemui.ImageWallpaper
> ...
> Total allocated (estimate): 54728.50 KB
> ----
>
>>
>>
>> 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,
>> other?) separate from the functionality changes ?
>>
>> -Emil
>
>
> I'd also kindly request to confirm the test environment used to verify
> Tomasz patch v2,
> which in my understanding has been the following, common between ChomiumOS
> and Android-IA:
>
> minigbm based gralloc

On ChromeOS it's still a drm_gralloc-based gralloc, but heavily
rewritten to use DMA-buf FDs and satisfy some other requirements for
production.

> dma FDs for buffers
> kernel based explicit fences with FDs

There is no support for explicit fences in Mesa at the moment, at
least in Android EGL backend. It always waits synchronously on the
acquire fence and returns -1 as release fence FD.

> HWC2 for compositing
> (?) Render nodes - but I don't know if/when they are used

We don't use control nodes from the Android running in ChromeOS at
all. We use the most suitable render node (i.e. i965 on Intel
platforms) for allocating buffers and rendering.

>
> In android-x86 (nougat-x86) situation is the following:
>
> drm_gralloc based gralloc
> buffer handles
> Not 100% sure about sync/fences, but I don't recall about using explicit
> fences with FDs
> GLES for compositing
> we don't use render nodes
>
> Pardon me if this seems a long checklist or if it's not 100% accurate,
> but I would assume that this patch should just work Out-Of-The-Box with
> android-x86 (nougat-x86)
> even if most of CrOS/Android-IA optimizations are not (yet) used there.
>
> Is this assumption correct?

Yeah, I believe so. Although I might have missed something within the
BO name/handle code path, as it can be quite tricky.

>
> In this moment the only way to boot nougat-x86 is to revert [1]
> but besides this and black wallpaper, which both require investigation,
> I've not seen any particular regression.

Thanks for testing!

Best regards,
Tomasz