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? Thanks Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
