[Mesa-dev] [RFC] mesa: drop current draw/read buffer when ctx is released
Tapani Pälli
tapani.palli at intel.com
Fri Oct 28 13:08:23 UTC 2016
On 10/28/2016 03:22 PM, Rob Clark wrote:
> On Fri, Oct 28, 2016 at 1:24 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> On 10/27/2016 01:48 PM, Rob Clark wrote:
>>> On Thu, Oct 27, 2016 at 2:59 AM, Tapani Pälli <tapani.palli at intel.com>
>>> wrote:
>>>> On 10/27/2016 12:16 AM, Rob Clark wrote:
>>>>> So, not quite sure if this is the *correct* solution, but it is at least
>>>>> *a* solution to a problem with android wallpaper vs mesa that I've been
>>>>> debugging. Basically, what happens is:
>>>>
>>>> Could you tell more how to trigger this, is this with some particular
>>>> live
>>>> wallpaper and has this been working before (regression)? For me at least
>>>> these default wallpapers have been working ok.
>>> Actually, it is the default static wallpaper that is problematic. And
>>> IIRC, it has never worked, at least not with any of the gallium
>>> drivers (freedreno, virgl, vc4, etc). Live-wallpaper did work, but
>>> does not appear to exist in AOSP builds anymore.
>>>
>>> If this works with i965 on android, I'd be curious how. Or if you're
>>> android build had some mesa patches that are not upstream?
>>
>> I can confirm that default wallpaper is working on i965. Our Mesa tree
>> currently looks like this:
>>
>> https://github.com/android-ia/external-mesa
>>
>> It's now quite a bit behind though because we were dealing with some
>> non-graphics issues and are planning to rebase soon.
> I suppose it is possible that it is triggered by something different
> that mesa/st does vs dri/i965? Although I find it odd that
> dri2_make_current() would drop the reference to the EGLSurface when
> unbinding ctx, but _mesa_make_current() would not drop the reference
> to the corresponding gl_framebuffer.
>
> Maybe the classic driver is holding an extra reference to the
> EGLSurface so the _eglPutSurface() call in dri2_make_current() does
> not actually drop the last refcnt? Which would ensure when the window
> surface is created it ends up with a different address..
>
> but, I wonder if you could try w/ Rob Herring's tree.. this is what we
> are using for the upstream kernel + AOSP builds[1]:
>
> https://github.com/robherring/mesa/commits/android-m
I'll check if this tree works for me (only on Monday though ..) or what
is required on top to make it work for our build. It looks like we have
a bit of difference there but no big changes.
> I guess in theory we should both need the same patches on top of
> mesa.. although also I guess we should also just try to get to the
> point where we can both use upstream mesa tree directly.
in theory ... we've been adopting (and upstreaming) some from the ARC++
tree:
https://chromium.googlesource.com/chromiumos/third_party/mesa/+log/arc-12.1.0-pre2/
I guess we always need to come a bit behind master to be stable enough
to be 'usable' (especially because of the continuous build breaks) but
hopefully not too much.
> [1] see: https://github.com/robherring/generic_device/wiki/KConfig-based-Multi-platform-Android-Device-(and-Mesa-graphics)
>
> Btw, I guess in theory the qemu/x86 build from rob's generic_device
> stuff should also work on real hw, so I think we should be able to
> test exact same build that is failing with gallium/virgl with i965.
> But I don't have any real hw for that.
>
> BR,
> -R
>
>>> BR,
>>> -R
>>>
>>>>> EGLSurface tmpSurface = mEgl.eglCreatePbufferSurface(mEglDisplay,
>>>>> mEglConfig, attribs);
>>>>> mEgl.eglMakeCurrent(mEglDisplay, tmpSurface, tmpSurface,
>>>>> mEglContext);
>>>>>
>>>>> int[] maxSize = new int[1];
>>>>> Rect frame = surfaceHolder.getSurfaceFrame();
>>>>> glGetIntegerv(GL_MAX_TEXTURE_SIZE, maxSize, 0);
>>>>>
>>>>> mEgl.eglMakeCurrent(mEglDisplay, EGL_NO_SURFACE, EGL_NO_SURFACE,
>>>>> EGL_NO_CONTEXT);
>>>>> mEgl.eglDestroySurface(mEglDisplay, tmpSurface);
>>>>>
>>>>> ... check maxSize vs frame size and bail if needed ...
>>>>>
>>>>> mEglSurface = mEgl.eglCreateWindowSurface(mEglDisplay, mEglConfig,
>>>>> surfaceHolder, null);
>>>>> ... error checking ...
>>>>> mEgl.eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface,
>>>>> mEglContext);
>>>>>
>>>>> When the window-surface is created, it ends up with the same ptr address
>>>>> as the recently freed tmpSurface pbuffer surface. Which after many
>>>>> levels of indirection, results in st_framebuffer_validate() ending up
>>>>> with
>>>>> the same/old framebuffer object, and in the end never calling the
>>>>> DRIimageLoaderExtension::getBuffers(). Then in droid_swap_buffers(),
>>>>> the
>>>>> dri2_surf is still the old pbuffer surface (with dri2_surf->buffer being
>>>>> NULL, obviously, so when wallpaper app calls eglSwapBuffers() nothing
>>>>> gets enqueued to the compositor).
>>>>>
>>>>> Maybe instead, eglDestroySurface() should clear any references the ctx
>>>>> has to the surface. Not sure how that would work. Did I mention there
>>>>> are many levels of indirection?
>>>>> ---
>>>>> src/mesa/main/context.c | 4 ++++
>>>>> 1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
>>>>> index 5e52065..83b8cc1 100644
>>>>> --- a/src/mesa/main/context.c
>>>>> +++ b/src/mesa/main/context.c
>>>>> @@ -1646,6 +1646,10 @@ ALOGE("%s:%d, drawBuffer=%p, readBuffer=%p",
>>>>> __func__, __LINE__, drawBuffer, rea
>>>>> if (!newCtx) {
>>>>> _glapi_set_dispatch(NULL); /* none current */
>>>>> + if (curCtx) {
>>>>> + _mesa_reference_framebuffer(&curCtx->WinSysDrawBuffer, NULL);
>>>>> + _mesa_reference_framebuffer(&curCtx->WinSysReadBuffer, NULL);
>>>>> + }
>>>>> }
>>>>> else {
>>>>> _glapi_set_dispatch(newCtx->CurrentDispatch);
>>>>
>>>>
More information about the mesa-dev
mailing list