[Mesa-dev] [PATCH 00/10] egl/android: Improve the Android EGL backend

[Tomasz Figa]

On Wed, Jul 20, 2016 at 10:43 PM, Rob Herring <robh at kernel.org> wrote:
> On Mon, Jul 18, 2016 at 11:29 PM, Tomasz Figa <tfiga at chromium.org> wrote:
>> On Tue, Jul 19, 2016 at 12:35 PM, Rob Herring <robh at kernel.org> wrote:
>>> On Fri, Jul 15, 2016 at 2:53 AM, Tomasz Figa <tfiga at chromium.org> wrote:
>>>> Hi,
>>>>
>>>> This series is a collection of various fixes and extensions we came up
>>>> with during our attempt to use Mesa for Android.
>>>>
>>>> Fixes included in this series:
>>>>  - added mandatory EGL_MAX_PBUFFER_WIDTH and _HEIGHT attributes to EGL
>>>>    configs,
>>>>  - fixed multiple issues with handling pbuffers in the backend,
>>>>  - found and fixed a DRI image leak,
>>>>  - made the implementation of DRI image loader .getBuffers callback
>>>>    conform better to the extension semantics.
>>>>
>>>> New features added by this series:
>>>>  - possibility to build the Android EGL platform without drm_gralloc
>>>>    headers,
>>>>  - support for creating EGL images from Android native buffers with
>>>>    YV12 pixel format (prime-only),
>>>>  - fallback to kms_swrast driver when no hardware driver can be loaded
>>>>    but there is still some usable DRI node present in the system.
>>>>  - more logging in case of errors to help diagnosing problems.
>>>>
>>>> Testing was done using classic i965 (gen 8) and gallium softpipe drivers
>>>> on an internal build of Android, based on gralloc backed by a DRM render
>>>> node and sharing buffers by PRIME FDs.
>>>
>>> I've tested out patches 1-6 with virgl and I don't get anything
>>> displayed. I get this message:
>>>
>>> EGL-DRI2: Front buffer is not supported for window surfaces
>>>
>>> That's as far as I investigated. I'll look into it some more tomorrow.
>>
>> Thanks a lot for testing!
>>
>> It looks like somehow your driver (or gallium) is triggering a call to
>> DRI image loader getBuffers() callback with front buffer bit set in
>> the image mask, but window surfaces on Android provide only back
>> buffers.
>
> I've debugged this a bit more, but still am not sure what's going on.
> Reverting #6 fixes things though. I don't see how a specific driver
> would cause the issue here. Here's the call stack for where the
> warning is printed:
>
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #00 pc 000000000000fc8d
>  /system/lib64/egl/libGLES_mesa.so (droid_image_get_buffers+77)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #01 pc 0000000000319554
>  /system/lib64/dri/gallium_dri.so (dri2_allocate_textures+660)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #02 pc 00000000003157c6
>  /system/lib64/dri/gallium_dri.so (dri_st_framebuffer_validate+566)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #03 pc 00000000004eadf3
>  /system/lib64/dri/gallium_dri.so (st_framebuffer_validate+115)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #04 pc 00000000004ead4e
>  /system/lib64/dri/gallium_dri.so
> (st_manager_validate_framebuffers+78)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #05 pc 00000000004c7450
>  /system/lib64/dri/gallium_dri.so (st_validate_state+352)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #06 pc 00000000004e6d5c
>  /system/lib64/dri/gallium_dri.so (st_draw_vbo+172)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #07 pc 00000000004acc31
>  /system/lib64/dri/gallium_dri.so (vbo_draw_arrays+289)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #08 pc 0000000000044dce
>  /system/lib64/libsurfaceflinger.so
> (android::GLES20RenderEngine::drawMesh(android::Mesh const&)+254)
> 07-19 21:30:37.750  2153  2153 F DEBUG   :     #09 pc 0000000000025af9
>  /system/lib64/libsurfaceflinger.so
> (android::Layer::drawWithOpenGL(android::sp<android::DisplayDevice
> const> const&, android::Region const&, bool) const+329)
>
>> My understanding of the semantics was that the callback should deny
>> such requests, so that's how I implemented it. However it isn't really
>> well documented, so potentially it should only provide buffers that
>> are available and ignore the rest without bailing out. Could someone
>> more familiar with this extension comment on this?

It looks like the semantics might be different indeed. Let me fix the
patch. Thanks a lot for testing.

I wonder how it worked on softpipe, though. Generally the requested
attachment list depends on the st_framebuffer passed to
st_framebuffer_validate(). I'm not that much familiar with Mesa state
tracker unfortunately to be able to tell what's going on exactly.

Best regards,
Tomasz