[Mesa-dev] [PATCH v6.2] egl: Allow creation of per surface out fence

[Emil Velikov]

On 8 September 2017 at 14:47, Marathe, Yogesh <yogesh.marathe at 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-bounces at lists.freedesktop.org] On Behalf
>> Of Marathe, Yogesh
>> Sent: Friday, September 1, 2017 10:16 AM
>> To: Tomasz Figa <tfiga at chromium.org>
>> Cc: Gao, Shuo <shuo.gao at intel.com>; Liu, Zhiquan <zhiquan.liu at intel.com>;
>> daniels at collabora.com; nicolai.haehnle at amd.com; Antognolli, Rafael
>> <rafael.antognolli at intel.com>; eric at engestrom.ch; Emil Velikov
>> <emil.l.velikov at gmail.com>; Wu, Zhongmin <zhongmin.wu at intel.com>;
>> kenneth at whitecape.org; Kondapally, Kalyan <kalyan.kondapally at intel.com>;
>> fernetmenta at online.de; mesa-dev at lists.freedesktop.org;
>> tarceri at itsqueeze.com; varad.gautam at 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-bounces at lists.freedesktop.org] On
>> > Behalf Of Tomasz Figa
>> > Sent: Friday, September 1, 2017 9:53 AM
>> > To: Marathe, Yogesh <yogesh.marathe at intel.com>
>> > On Thu, Aug 31, 2017 at 2:18 AM, Marathe, Yogesh
>> > <yogesh.marathe at intel.com> wrote:
>> > >> -----Original Message-----
>> > >> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
>> > >> Behalf Of Emil Velikov
>> > >> Sent: Wednesday, August 30, 2017 9:44 PM
>> > >> To: Marathe, Yogesh <yogesh.marathe at intel.com>
>> > >> Cc: Gao, Shuo <shuo.gao at intel.com>; Liu, Zhiquan
>> > >> <zhiquan.liu at intel.com>; daniels at collabora.com;
>> > >> nicolai.haehnle at amd.com; Antognolli, Rafael
>> > >> <rafael.antognolli at intel.com>; eric at engestrom.ch; Wu, Zhongmin
>> > >> <zhongmin.wu at intel.com>; tfiga at chromium.org;
>> kenneth at whitecape.org;
>> > >> Kondapally, Kalyan <kalyan.kondapally at intel.com>;
>> > >> fernetmenta at online.de; mesa-dev at lists.freedesktop.org;
>> > >> tarceri at itsqueeze.com; varad.gautam at 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.marathe at 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 <chris at 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.

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