Hi Nicolas, On 4 August 2016 at 02:51, Nicolas Boichat <drink...@chromium.org> wrote: > On Thu, Aug 4, 2016 at 9:38 AM, Michel Dänzer <mic...@daenzer.net> wrote: >> On 04.08.2016 09:53, Nicolas Boichat wrote: >>> On Thu, Aug 4, 2016 at 12:22 AM, Martin Peres >>> <martin.pe...@linux.intel.com> wrote: >>>> On 03/08/16 16:54, Nicolas Boichat wrote: >>>>> >>>>> In the case where dri2_initialize is called with a TestOnly display, >>>>> the display is not actually initialized, so dri2_egl_display always >>>>> fails, and we cannot do any reference counting. >>>>> >>>>> Fixes piglit spec@egl_khr_create_context@verify gl flavor (reproducible >>>>> with LIBGL_ALWAYS_SOFTWARE=1) and spec@egl_khr_fence_sync@conformance. >>>>> >>>>> Fixes: 9ee683f877 (egl/dri2: Add reference count for dri2_egl_display) >>>>> Cc: "12.0" <mesa-sta...@lists.freedesktop.org> >>>>> Reported-by: Michel Dänzer <mic...@daenzer.net> >>>>> Signed-off-by: Nicolas Boichat <drink...@chromium.org> >>>>> --- >>>>> >>>>> Compile-tested only, please give it a spin, thanks! >>>> >>>> Still crashes, same backtrace before and after the patch: >>> >>> Actually, I was thinking about this bug: >>> https://bugs.freedesktop.org/show_bug.cgi?id=97136, which should be >>> spec@egl_khr_create_context@verify gl flavor? Did you try that test? >> >> Your patch fixes this test for me. >> >> Tested-by: Michel Dänzer <michel.daen...@amd.com> >> >> Please remove the reference to the egl_khr_fence_sync test from the >> commit log. > > Thanks. > > Emil: Can you fixup the commit message before applying? > Sure I can do that. Yet this change fixes another unexpected/undocumented bug - currently calling eglGetProcAddress will fail, when libEGL is built without the said platform. Yet the API/documentation is clear - entry points are independent of display (which is platform specific) or context.
Did I miss something, does the above make sense ? >>> Not easy for me to reproduce, but... Looking that the test source code: >>> https://cgit.freedesktop.org/piglit/tree/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c >>> >>> Do you know why we end up in the error path in init_display? >>> >>> My guess is that >>> eglInitialize->dri2_initialize->dri2_initialize_wayland fails after >>> setting disp->DriverData, so the display refcount is == 0, but the >>> display is not null, leading to the crash in egl_terminate. >>> >>> I just spotted this patch for x11: >>> https://patchwork.freedesktop.org/patch/101934/ >>> >>> platform_wayland needs to be modified in a similar way. >> >> Indeed, that fixes the egl_khr_fence_sync test for me FWIW. > > I'll send a series to fix that very soon, on all platforms. > >>> For the record, Emil spotted this issue when I submitted the offending >>> patch, and I haven't followed up ,-( >> >> For future patches, please make sure there are no piglit regressions, at >> least for the tests which run with swrast via LIBGL_ALWAYS_SOFTWARE=1. > > Is there some simple instructions to do that? Running piglit should be straight forward - please check the README (and tell is it somethings is odd). > Or maybe some continuous > integration farm somewhere we can submit the patch to? (I see > appveyor.yml, but that's for Windows?) Appveyor/Travis is for build testing piglit only. Most people have their CI setups, but they're not public. Would be great if we can have such a beast (I think we can use jenkins.fd.o), but I think if mostly boils down to having access to a ton of hardware ;-) The Intel guys have their setup files available on github [1], if you're interested. Regards, Emil [1] https://github.com/janesma/mesa_jenkins _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
