On 2 August 2017 at 18:29, Rafael Antognolli <rafael.antogno...@intel.com> wrote: > On Wed, Aug 02, 2017 at 02:56:34PM +0100, Emil Velikov wrote: >> On 1 August 2017 at 16:29, Rafael Antognolli >> <rafael.antogno...@intel.com> wrote: >> > On Mon, Jul 31, 2017 at 09:58:24AM -0700, Marathe, Yogesh wrote: >> >> Rafael, Tomasz, >> >> >> >> > -----Original Message----- >> >> > From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On Behalf >> >> > Of Rafael Antognolli >> >> > Sent: Tuesday, July 25, 2017 9:39 PM >> >> > To: Wu, Zhongmin <zhongmin...@intel.com> >> >> > Cc: Gao, Shuo <shuo....@intel.com>; Liu, Zhiquan >> >> > <zhiquan....@intel.com>; >> >> > Daniel Stone <dani...@collabora.com>; emil.l.veli...@gmail.com; Eric >> >> > Engestrom <e...@engestrom.ch>; Marathe, Yogesh >> >> > <yogesh.mara...@intel.com>; tf...@chromium.org; Kondapally, Kalyan >> >> > <kalyan.kondapa...@intel.com>; mesa-dev@lists.freedesktop.org; Varad >> >> > Gautam <varad.gau...@collabora.com> >> >> > Subject: Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync >> >> > fence >> >> > for Android OS v4.2 >> >> > >> >> > On Tue, Jul 25, 2017 at 10:07:23AM +0800, Zhongmin Wu wrote: >> >> > > Before we queued the buffer with a invalid fence (-1), it will make >> >> > > some benchmarks failed to test such as flatland. >> >> > > >> >> > > Now we get the out fence during the flushing buffer and then pass it >> >> > > to SurfaceFlinger in eglSwapbuffer function. >> >> > > >> >> > > v2: a) Also implement the fence in cancelBuffer. >> >> > > b) The last sync fence is stored in drawable object >> >> > > rather than brw context. >> >> > > c) format clear. >> >> > > >> >> > > v3: a) Save the last fence fd in DRI Context object. >> >> > > b) Return the last fence if the batch buffer is empty and >> >> > > nothing to be flushed when _intel_batchbuffer_flush_fence >> >> > > c) Add the new interface in vbtl to set the retrieve fence >> >> > > >> >> > > v3.1 a) close fd in the new vbtl interface on none Android platform >> >> > > >> >> > > v4: a) The last fence is saved in brw context. >> >> > > b) The retrieve fd is for all the platform but not just Android >> >> > > c) Add a uniform dri2 interface to initialize the surface. >> >> > > >> >> > > v4.1: a) make some changes of variable name. >> >> > > b) the patch is breaked into two patches. >> >> > > >> >> > > v4.2: a) Add a deinit interface for surface to clear the out fence >> >> > >> >> > Hi Zhongmin, >> >> > >> >> > The patch is indeed looking better. I agree with Tomasz, it would be >> >> > good to >> >> > have a way for the platform to inform whether it is interested in >> >> > fences or not. >> >> > What about some flag that you pass to dri2_surf_init? (I'm not sure >> >> > that's the >> >> > best place, though). >> >> > >> >> >> >> I would like to take it forward from here for remaining review comments, >> >> Zhongmin agrees. >> >> >> >> I added 'enable_out_fence' bool to dri2_surf_init() as a parameter and >> >> all platforms except >> >> android pass this as false. Based on 'enable_out_fence' stored with >> >> surface, >> >> 'dri2_surf_get/update_fence_fd()' has a check before it calls >> >> create_fence_fd(). Is this the >> >> right expectation here? >> > >> > Sounds good to me, although I would need to see the patch. Also please make >> > sure that the dri2 fence extension is supported, assuming you are trying to >> > enable this. >> > >> I've let glmark2 spin (egl+x11) during lunch and it showed no perf. >> difference. >> >> Admittedly may not be the best of benchmarks for this use case, even >> though it pushes 200-2000fps, while the new code is in the >> eglSwapBuffers path. >> I got the feeling that one is micro optimising then there's no need for it. >> >> Rafael, have you tried any test/benchmark on your end? > > Hi Emil, > > Are you referring to checking whether the platform requested the fence fd or > not? If so, I was thinking about making it optional mainly to avoid > creating extra fds, but I didn't think it would have an impact on performance. > > Anyway, I didn't run any benchmark on this. > Sure, fds are finite resource, but we're "wasting" one per surface ;-)
Nobody mentioned why they'd want to avoid the fence, so I've naively assumed that it was performance. Now I see your concern, thanks for the clarification. -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
