On Fri, Oct 28, 2016 at 1:24 AM, Tapani Pälli <tapani.pa...@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.pa...@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 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. [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); >>> >>> >>> > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev