On Wed, Aug 10, 2016 at 9:44 AM, Michel Dänzer <mic...@daenzer.net> wrote: > On 10/08/16 03:00 PM, Nicolas Boichat wrote: >> eglMakeCurrent can also be used to change the active display. In that >> case, we need to decrement ref_count of the previous display (possibly >> destroying it), and increment it on the next display. >> >> Also, old_dsurf/old_rsurf cannot be non-NULL if old_ctx is NULL, so >> we only need to test if old_ctx is non-NULL. >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97214 >> Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display) >> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> >> Reported-by: Alexandr Zelinsky <mexahota...@w1l.ru> >> Tested-by: Alexandr Zelinsky <mexahota...@w1l.ru> >> Signed-off-by: Nicolas Boichat <drink...@chromium.org> >> --- >> src/egl/drivers/dri2/egl_dri2.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/src/egl/drivers/dri2/egl_dri2.c >> b/src/egl/drivers/dri2/egl_dri2.c >> index 3205a36..701e42a 100644 >> --- a/src/egl/drivers/dri2/egl_dri2.c >> +++ b/src/egl/drivers/dri2/egl_dri2.c >> @@ -1285,8 +1285,10 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, >> _EGLSurface *dsurf, >> >> if (!unbind) >> dri2_dpy->ref_count++; >> - if (old_dsurf || old_rsurf || old_ctx) >> - dri2_display_release(disp); >> + if (old_ctx) { >> + EGLDisplay old_disp = >> _eglGetDisplayHandle(old_ctx->Resource.Display); >> + dri2_display_release(old_disp); >> + } > > Unfortunately, this change breaks the piglit test "spec@egl > 1.4@eglterminate then unbind context", because old_ctx != NULL but > old_ctx->Resource.Display == NULL. Modifying the test to > > if (old_ctx && old_ctx->Resource.Display) { > > fixes the regression and doesn't seem to cause any other problems.
This is probably wrong as the display will leak (it definitely should be freed after calling eglTerminate + eglMakeCurrent(NULL)). I think I know where the problem is (the call to _eglReleaseDisplayResources happens too early), I'm not sure what's the best solution. I'll have a look. > Alexandr, does the patch still fix your problem with that modification? > > > Nicolas, this regression is also reproducible with > LIBGL_ALWAYS_SOFTWARE=1 . Please get used to testing your changes like > that and only send out changes for review which don't cause any regressions. Managed to build piglit, and run it using a locally built mesa. Are the commands below what you would use? export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$MESA_DIR/lib export LIBGL_DRIVERS_PATH=$MESA_DIR/lib/gallium export EGL_DRIVERS_PATH=$MESA_DIR/lib export EGL_LOG_LEVEL=debug export LIBGL_ALWAYS_SOFTWARE=1 ./piglit run -p x11_egl -t "spec@egl.*" all results/master Best, Nicolas _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
