On Fri, Sep 08, 2017 at 08:32:05AM -0700, Marathe, Yogesh wrote: > > -----Original Message----- > > From: Emil Velikov [mailto:emil.l.veli...@gmail.com] > > Sent: Friday, September 8, 2017 8:28 PM > > To: Marathe, Yogesh <yogesh.mara...@intel.com> > > Cc: Tomasz Figa <tf...@chromium.org>; Antognolli, Rafael > > <rafael.antogno...@intel.com>; Janes, Mark A <mark.a.ja...@intel.com>; > > mesa-dev@lists.freedesktop.org; Gao, Shuo <shuo....@intel.com>; Liu, > > Zhiquan <zhiquan....@intel.com>; dani...@collabora.com; > > nicolai.haeh...@amd.com; e...@engestrom.ch; Wu, Zhongmin > > <zhongmin...@intel.com>; kenn...@whitecape.org; Kondapally, Kalyan > > <kalyan.kondapa...@intel.com>; fernetme...@online.de; > > tarc...@itsqueeze.com; varad.gau...@collabora.com > > Subject: Re: [Mesa-dev] [PATCH v6.2] egl: Allow creation of per surface out > > fence > > > > On 8 September 2017 at 14:47, Marathe, Yogesh <yogesh.mara...@intel.com> > > wrote: > > > Hello Folks, > > > > > > Sorry for late reply, I took quite some time to CTS up, comments below. > > > > > >> -----Original Message----- > > >> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > > >> Behalf Of Marathe, Yogesh > > >> Sent: Friday, September 1, 2017 10:16 AM > > >> To: Tomasz Figa <tf...@chromium.org> > > >> Cc: Gao, Shuo <shuo....@intel.com>; Liu, Zhiquan > > >> <zhiquan....@intel.com>; dani...@collabora.com; > > >> nicolai.haeh...@amd.com; Antognolli, Rafael > > >> <rafael.antogno...@intel.com>; e...@engestrom.ch; Emil Velikov > > >> <emil.l.veli...@gmail.com>; Wu, Zhongmin <zhongmin...@intel.com>; > > >> kenn...@whitecape.org; Kondapally, Kalyan > > >> <kalyan.kondapa...@intel.com>; fernetme...@online.de; > > >> mesa-dev@lists.freedesktop.org; tarc...@itsqueeze.com; > > >> varad.gau...@collabora.com > > >> Subject: Re: [Mesa-dev] [PATCH v6.2] egl: Allow creation of per > > >> surface out fence > > >> > > >> Tomasz, > > >> > > >> > -----Original Message----- > > >> > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On > > >> > Behalf Of Tomasz Figa > > >> > Sent: Friday, September 1, 2017 9:53 AM > > >> > To: Marathe, Yogesh <yogesh.mara...@intel.com> On Thu, Aug 31, 2017 > > >> > at 2:18 AM, Marathe, Yogesh <yogesh.mara...@intel.com> wrote: > > >> > >> -----Original Message----- > > >> > >> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] > > >> > >> On Behalf Of Emil Velikov > > >> > >> Sent: Wednesday, August 30, 2017 9:44 PM > > >> > >> To: Marathe, Yogesh <yogesh.mara...@intel.com> > > >> > >> Cc: Gao, Shuo <shuo....@intel.com>; Liu, Zhiquan > > >> > >> <zhiquan....@intel.com>; dani...@collabora.com; > > >> > >> nicolai.haeh...@amd.com; Antognolli, Rafael > > >> > >> <rafael.antogno...@intel.com>; e...@engestrom.ch; Wu, Zhongmin > > >> > >> <zhongmin...@intel.com>; tf...@chromium.org; > > >> kenn...@whitecape.org; > > >> > >> Kondapally, Kalyan <kalyan.kondapa...@intel.com>; > > >> > >> fernetme...@online.de; mesa-dev@lists.freedesktop.org; > > >> > >> tarc...@itsqueeze.com; varad.gau...@collabora.com > > >> > >> Subject: Re: [Mesa-dev] [PATCH v6.2] egl: Allow creation of per > > >> > >> surface out fence > > >> > >> > > >> > >> On 30 August 2017 at 15:39, Marathe, Yogesh > > >> > >> <yogesh.mara...@intel.com> > > >> > >> wrote: > > >> > >> > > >> > >> > > > >> > >> > Thank you, Tomasz and all involved for the help and guidance. > > >> > >> > > > >> > >> Our excitement was short lived - see commit > > >> > >> 8c9df0daf20206fafb7df77b1edcbc41b8e91372. > > >> > >> > > >> > >> Seems the patch was not run through the Intel CI, though I'm > > >> > >> should not have assumed that you're aware of if. > > >> > >> Please get in touch with Mark Janes (Cc'd here, janesma on IRC). > > >> > >> He can set you up and/or run a branch for you. > > >> > >> > > >> > > > > >> > > No problem. I will contact Mark. > > >> > > > > >> > > Primarily looks like platform / kernel version issue. > > >> > > intel_get_boolean() for I915_PARAM_HAS_EXEC_FENCE is false, but I > > >> > > see following in i915_drv.c:915_getparam in kernel, no clue why > > >> > > that would come false in UMD. > > >> > > > > >> > > ... > > >> > > case I915_PARAM_HAS_EXEC_FENCE: > > >> > > /* For the time being all of these are always true; > > >> > > * if some supported hardware does not have one of > > >> > > these > > >> > > * features this value needs to be provided from > > >> > > * INTEL_INFO(), a feature macro, or similar. > > >> > > */ > > >> > > value = 1; > > >> > > break; > > >> > > ... > > >> > > > >> > Which kernel are you looking at? Remember that not everyone uses > > >> > current upstream master. There is a number of upstream stable > > >> > releases and downstream forks. Grepping for > > >> > I915_PARAM_HAS_EXEC_FENCE on http://elixir.free-electrons.com, shows > > that it was only added in Linux 4.12. > > >> > > > >> > > >> I'm on 4.9.x but I see my kernel tree has following patch, so this > > >> looks like it is done for android (cherry picked / backport). That’s why > > >> it > > worked for me always! > > >> > > >> commit f0683754f03fa308a2657cb1dadbf235c9607188 > > >> Author: Chris Wilson <ch...@chris-wilson.co.uk> > > >> Date: Fri Jan 27 09:40:08 2017 +0000 > > >> > > >> drm/i915: Support explicit fencing for execbuf > > >> > > >> Nonetheless, as you mentioned, I've synced up with Mark and we've > > >> created a separate branch where CTS / intel mesa CI can run. Let me try > > >> to fix > > this. > > >> > > >> Caveat: To have flatland running on android there was another issue > > >> in kernel which needed a fix. Details - > > >> https://bugs.freedesktop.org/show_bug.cgi?id=101656 > > > > > > I was able to run CTS (https://github.com/KhronosGroup/VK-GL-CTS) on > > > this patch for x11_egl. I see exact same results before and after patch on > > Ubuntu 16.04 setup. > > > Mark had also mentioned this works fine on 4.12 onwards (essentially with > > drm/i915: > > > Support explicit fencing for execbuf patch in kernel). > > > > > > Regarding the primary reason why this was reverted > > > > > > mesa/drivers/dri/i965/brw_sync.c:491: brw_dri_create_fence_fd: > > > Assertion `brw->screen->has_exec_fence' failed. > > > > > > This assertion evident on older kernels. Although I'm bit surprised > > > here after looking at the code. Older kernels where this explicit > > > fencing is not supported must've returned 'false' for has_exec_fence in > > intelInitScreen2(). > > > > > > screen->has_exec_fence = > > > intel_get_boolean(screen, I915_PARAM_HAS_EXEC_FENCE); > > > > > > It appears that’s coming 'true' and due to that we set > > > enable_out_fence in dri2_init_surface() (based on get_capabilities()) > > > which > > causes create_fence_fd call on non-supported kernels. > > > Isn't this strange? Can someone please comment? > > > > > In all fairness there was a few wtf moments as Mark mentioned the issue. As > > on > > a quick look "it cannot happen" :-\ > > > > One way is to add some printfs "debugging" across the board and check with > > Mark if he can run (only?) the affected test on the CI. > > > > Number of tests failing on CI due to this are huge, any 'one' can be picked > up. I do have > my CI branch setup now but I don’t think I can use it for debugging (not > advised). I'll sync > up with Mark again. Just wanted a confirmation, I'm not missing something > obvious. Thanks.
Hi Yogesh, I replied to you already when you messaged in private, the error is not related to the kernel returning true for that, it's related to a memory corruption caused by wrong use of the dri2_surf_init inside platform_x11_dri3.c. Quoting myself: "More specifically, it looks like this test fails every time: glcts -n dEQP-EGL.functional.query_context.get_current_context.rgb888_window I see several valgrind warnings inside platform_x11_dri3.c. I believe you are probably accessing the dri2_surf before it was allocated, or after it was freed..." When I tested this back then, the "out_fence_enable" (or whatever was called) in dri2 was false, but after a couple runs it would become a bogus number, which also points to memory corruption. I suggest ignoring the kernel and focusing on valgrind debugging. Rafael > > Personally I would be quite generous with my printfs - from the ioctl call > > via the > > has_exec_fence handling to the __DRI_FENCE_CAP_NATIVE_FD check. > > > > -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
