Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Emil Velikov]

On 2 August 2017 at 18:29, Rafael Antognolli
 wrote:
> On Wed, Aug 02, 2017 at 02:56:34PM +0100, Emil Velikov wrote:
>> On 1 August 2017 at 16:29, Rafael Antognolli
>>  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 
>> >> > Cc: Gao, Shuo ; Liu, Zhiquan 
>> >> > ;
>> >> > Daniel Stone ; emil.l.veli...@gmail.com; Eric
>> >> > Engestrom ; Marathe, Yogesh
>> >> > ; tf...@chromium.org; Kondapally, Kalyan
>> >> > ; mesa-dev@lists.freedesktop.org; Varad
>> >> > Gautam 
>> >> > 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

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Rafael Antognolli]

On Wed, Aug 02, 2017 at 02:56:34PM +0100, Emil Velikov wrote:
> On 1 August 2017 at 16:29, Rafael Antognolli
>  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 
> >> > Cc: Gao, Shuo ; Liu, Zhiquan ;
> >> > Daniel Stone ; emil.l.veli...@gmail.com; Eric
> >> > Engestrom ; Marathe, Yogesh
> >> > ; tf...@chromium.org; Kondapally, Kalyan
> >> > ; mesa-dev@lists.freedesktop.org; Varad
> >> > Gautam 
> >> > 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.

Rafael
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Emil Velikov]

On 1 August 2017 at 16:29, Rafael Antognolli
 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 
>> > Cc: Gao, Shuo ; Liu, Zhiquan ;
>> > Daniel Stone ; emil.l.veli...@gmail.com; Eric
>> > Engestrom ; Marathe, Yogesh
>> > ; tf...@chromium.org; Kondapally, Kalyan
>> > ; mesa-dev@lists.freedesktop.org; Varad
>> > Gautam 
>> > 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

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Rafael Antognolli]

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 
> > Cc: Gao, Shuo ; Liu, Zhiquan ;
> > Daniel Stone ; emil.l.veli...@gmail.com; Eric
> > Engestrom ; Marathe, Yogesh
> > ; tf...@chromium.org; Kondapally, Kalyan
> > ; mesa-dev@lists.freedesktop.org; Varad
> > Gautam 
> > 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.

> Please let me know if 'enable_out_fence' can be changed for a better name. 
> I've already
> implemented other review other comments Rafael mentioned and will include them
> in v4.3 along with this.

I also don't have any better name in mind.

Thanks,
Rafael

> > Please see other comments below.
> > 
> > > Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
> > > Signed-off-by: Zhongmin Wu 
> > > ---
> > >  src/egl/drivers/dri2/egl_dri2.c |   51 
> > > +++
> > >  src/egl/drivers/dri2/egl_dri2.h |8 +
> > >  src/egl/drivers/dri2/platform_android.c |   12 ---
> > >  src/egl/drivers/dri2/platform_drm.c |3 +-
> > >  src/egl/drivers/dri2/platform_surfaceless.c |3 +-
> > >  src/egl/drivers/dri2/platform_wayland.c |3 +-
> > >  src/egl/drivers/dri2/platform_x11.c |3 +-
> > >  src/egl/drivers/dri2/platform_x11_dri3.c|3 +-
> > >  8 files changed, 77 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/src/egl/drivers/dri2/egl_dri2.c
> > > b/src/egl/drivers/dri2/egl_dri2.c index 020a0bc..ffd3a8a 100644
> > > --- a/src/egl/drivers/dri2/egl_dri2.c
> > > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > > @@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv,
> > _EGLDisplay *disp, _EGLContext *ctx)
> > > return EGL_TRUE;
> > >  }
> > >
> > > +EGLBoolean
> > > +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> > > +_EGLConfig *conf, const EGLint *attrib_list) {
> > > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > > +   dri2_surf->out_fence_fd = -1;
> > > +   return _eglInitSurface(surf, dpy, type, conf, attrib_list); }
> > > +
> > > +static void
> > > +dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd) {
> > > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > > +   if (dri2_surf->out_fence_fd >=0)
> > > +  close(dri2_surf->out_fence_fd);
> > > +
> > > +   dri2_surf->out_fence_fd = fence_fd; }
> > > +
> > > +void

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Marathe, Yogesh]

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 
> Cc: Gao, Shuo ; Liu, Zhiquan ;
> Daniel Stone ; emil.l.veli...@gmail.com; Eric
> Engestrom ; Marathe, Yogesh
> ; tf...@chromium.org; Kondapally, Kalyan
> ; mesa-dev@lists.freedesktop.org; Varad
> Gautam 
> 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?

Please let me know if 'enable_out_fence' can be changed for a better name. I've 
already
implemented other review other comments Rafael mentioned and will include them
in v4.3 along with this.

> Please see other comments below.
> 
> > Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
> > Signed-off-by: Zhongmin Wu 
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c |   51 
> > +++
> >  src/egl/drivers/dri2/egl_dri2.h |8 +
> >  src/egl/drivers/dri2/platform_android.c |   12 ---
> >  src/egl/drivers/dri2/platform_drm.c |3 +-
> >  src/egl/drivers/dri2/platform_surfaceless.c |3 +-
> >  src/egl/drivers/dri2/platform_wayland.c |3 +-
> >  src/egl/drivers/dri2/platform_x11.c |3 +-
> >  src/egl/drivers/dri2/platform_x11_dri3.c|3 +-
> >  8 files changed, 77 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c
> > b/src/egl/drivers/dri2/egl_dri2.c index 020a0bc..ffd3a8a 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv,
> _EGLDisplay *disp, _EGLContext *ctx)
> > return EGL_TRUE;
> >  }
> >
> > +EGLBoolean
> > +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> > +_EGLConfig *conf, const EGLint *attrib_list) {
> > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > +   dri2_surf->out_fence_fd = -1;
> > +   return _eglInitSurface(surf, dpy, type, conf, attrib_list); }
> > +
> > +static void
> > +dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd) {
> > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > +   if (dri2_surf->out_fence_fd >=0)
> > +  close(dri2_surf->out_fence_fd);
> > +
> > +   dri2_surf->out_fence_fd = fence_fd; }
> > +
> > +void
> > +dri2_surf_deinit(_EGLSurface *surf)
> > +{
> > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > +   dri2_surface_set_out_fence(surf, -1); }
> > +
> >  static EGLBoolean
> >  dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface
> > *surf)  { @@ -1318,6 +1344,22 @@ dri2_destroy_surface(_EGLDriver *drv,
> > _EGLDisplay *dpy, _EGLSurface *surf)
> > return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);  }
> >
> > +static void
> > +dri2_surf_get_fence_fd(_EGLContext 

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Marathe, Yogesh]

> -Original Message-
> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> Sent: Tuesday, July 25, 2017 11:22 PM
> To: Marathe, Yogesh 
> Cc: Wu, Zhongmin ; ML mesa-dev  d...@lists.freedesktop.org>; Kondapally, Kalyan
> ; Antognolli, Rafael
> ; Gao, Shuo ; Tomasz Figa
> ; Chris Wilson ; Eric
> Engestrom ; Rob Herring ; Daniel
> Stone ; Varad Gautam
> ; Liu, Zhiquan ; Frank
> Binns ; Brendan King ;
> Martin Peres 
> Subject: Re: [PATCH 2/2] i965: Queue the buffer with a sync fence for Android
> OS v4.2
> 
> On 25 July 2017 at 15:30, Marathe, Yogesh 
> wrote:
> > Emil,
> >
> >> -Original Message-
> >> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> >> Sent: Tuesday, July 25, 2017 7:46 PM
> >> To: Wu, Zhongmin 
> >> Cc: ML mesa-dev ; Kondapally, Kalyan
> >> ; Marathe, Yogesh
> >> ; Antognolli, Rafael
> >> ; Gao, Shuo ; Tomasz
> >> Figa ; Chris Wilson ;
> >> Eric Engestrom ; Rob Herring ;
> >> Daniel Stone ; Varad Gautam
> >> ; Liu, Zhiquan ;
> >> Frank Binns ; Brendan King
> >> ; Martin Peres
> >> 
> >> Subject: Re: [PATCH 2/2] i965: Queue the buffer with a sync fence for
> >> Android OS v4.2
> >>
> >> Hi Zhongmin,
> >>
> >> Thanks you for the update. There's a couple of important comments -
> >> dri2_make_current + droid_window_enqueue_buffer.
> >> The rest is just nitpiks.
> >>
> >> Tomasz, hats down for the immense help and guidance.
> >>
> >> On the potential performance hit (due to the extra fence), I think we
> >> could do some tests before adding extra infra.
> >> No obvious benchmarks come to mind - any suggestions?
> >>
> >
> > Sorry to jump in, flatland is the one native application on android
> > that tests this explicitly. It gives time required to render one frame
> > of particular resolution without other services running. It’s a native
> > app that comes with aosp. And we found this issue just because of that.
> >
> > App info -
> > https://android.googlesource.com/platform/frameworks/native/+/master/c
> > mds/flatland/README.txt Bug -
> > https://bugs.freedesktop.org/show_bug.cgi?id=101655
> >
> > I already tested this patch set with android and I can see scores not being 
> > that
> great.
> > May be this is the one we can use to profile this or I can continue to
> > profile based on guidance here.
> >
> I meant a completely different thing:
> 
> Don't bother with premature optimisations - see if/how much overhead of the
> patch itself adds (ideally on most platforms).
> Aka - test before/after. Which in the case of flatland is not possible, if I
> understood you correctly.

Yes, it won't be functional without patch.

> 
> About the performance numbers in question - I hope you've looked at my
> comment in 1/2.

Yes. I saw that.

> 
> -Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Emil Velikov]

On 25 July 2017 at 15:30, Marathe, Yogesh  wrote:
> Emil,
>
>> -Original Message-
>> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
>> Sent: Tuesday, July 25, 2017 7:46 PM
>> To: Wu, Zhongmin 
>> Cc: ML mesa-dev ; Kondapally, Kalyan
>> ; Marathe, Yogesh
>> ; Antognolli, Rafael
>> ; Gao, Shuo ; Tomasz Figa
>> ; Chris Wilson ; Eric
>> Engestrom ; Rob Herring ; Daniel
>> Stone ; Varad Gautam
>> ; Liu, Zhiquan ; Frank
>> Binns ; Brendan King ;
>> Martin Peres 
>> Subject: Re: [PATCH 2/2] i965: Queue the buffer with a sync fence for Android
>> OS v4.2
>>
>> Hi Zhongmin,
>>
>> Thanks you for the update. There's a couple of important comments -
>> dri2_make_current + droid_window_enqueue_buffer.
>> The rest is just nitpiks.
>>
>> Tomasz, hats down for the immense help and guidance.
>>
>> On the potential performance hit (due to the extra fence), I think we could 
>> do
>> some tests before adding extra infra.
>> No obvious benchmarks come to mind - any suggestions?
>>
>
> Sorry to jump in, flatland is the one native application on android that 
> tests this
> explicitly. It gives time required to render one frame of particular 
> resolution without
> other services running. It’s a native app that comes with aosp. And we found 
> this
> issue just because of that.
>
> App info - 
> https://android.googlesource.com/platform/frameworks/native/+/master/cmds/flatland/README.txt
> Bug - https://bugs.freedesktop.org/show_bug.cgi?id=101655
>
> I already tested this patch set with android and I can see scores not being 
> that great.
> May be this is the one we can use to profile this or I can continue to 
> profile based on
> guidance here.
>
I meant a completely different thing:

Don't bother with premature optimisations - see if/how much overhead
of the patch itself adds (ideally on most platforms).
Aka - test before/after. Which in the case of flatland is not
possible, if I understood you correctly.

About the performance numbers in question - I hope you've looked at my
comment in 1/2.

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Rafael Antognolli]

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).

Please see other comments below.

> Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
> Signed-off-by: Zhongmin Wu 
> ---
>  src/egl/drivers/dri2/egl_dri2.c |   51 
> +++
>  src/egl/drivers/dri2/egl_dri2.h |8 +
>  src/egl/drivers/dri2/platform_android.c |   12 ---
>  src/egl/drivers/dri2/platform_drm.c |3 +-
>  src/egl/drivers/dri2/platform_surfaceless.c |3 +-
>  src/egl/drivers/dri2/platform_wayland.c |3 +-
>  src/egl/drivers/dri2/platform_x11.c |3 +-
>  src/egl/drivers/dri2/platform_x11_dri3.c|3 +-
>  8 files changed, 77 insertions(+), 9 deletions(-)
> 
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 020a0bc..ffd3a8a 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay 
> *disp, _EGLContext *ctx)
> return EGL_TRUE;
>  }
>  
> +EGLBoolean
> +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> +_EGLConfig *conf, const EGLint *attrib_list)
> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   dri2_surf->out_fence_fd = -1;
> +   return _eglInitSurface(surf, dpy, type, conf, attrib_list);
> +}
> +
> +static void
> +dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd)
> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   if (dri2_surf->out_fence_fd >=0)
> +  close(dri2_surf->out_fence_fd);
> +
> +   dri2_surf->out_fence_fd = fence_fd;
> +}
> +
> +void
> +dri2_surf_deinit(_EGLSurface *surf)
> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   dri2_surface_set_out_fence(surf, -1);
> +}
> +
>  static EGLBoolean
>  dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
>  {
> @@ -1318,6 +1344,22 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay 
> *dpy, _EGLSurface *surf)
> return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);
>  }
>  
> +static void
> +dri2_surf_get_fence_fd(_EGLContext *ctx,

Maybe it's just me but every time I read this code I get confused by the name
of this function, since it says "get_fence_fd" but doesn't really return
anything. How about dri2_surf_update_fence_fd?

> +   _EGLDisplay *dpy, _EGLSurface *surf)
> +{
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   int fence_fd = -1;
> +   __DRIcontext *dri_ctx = dri2_egl_context(ctx)->dri_context;
> +   void * fence = dri2_dpy->fence->create_fence_fd(dri_ctx, -1);

Before calling any of the dri2 fence extension, shouldn't we check whether it
is available? I think you had these checks in earlier versions of this patch,
but maybe it got lost along the way.

> +   if (fence) {
> +  fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen,
> +   fence);
> +  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
> +   }
> +   dri2_surface_set_out_fence(surf, fence_fd);
> +}
> +
>  /**
>   * Called via eglMakeCurrent(), drv->API.MakeCurrent().
>   */
> @@ -1352,8 +1394,11 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, 
> _EGLSurface *dsurf,
> rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
> cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
>  
> +   int fence_fd = -1;
> if (old_ctx) {
>__DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
> +  if 

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Marathe, Yogesh]

Emil, 

> -Original Message-
> From: Emil Velikov [mailto:emil.l.veli...@gmail.com]
> Sent: Tuesday, July 25, 2017 7:46 PM
> To: Wu, Zhongmin 
> Cc: ML mesa-dev ; Kondapally, Kalyan
> ; Marathe, Yogesh
> ; Antognolli, Rafael
> ; Gao, Shuo ; Tomasz Figa
> ; Chris Wilson ; Eric
> Engestrom ; Rob Herring ; Daniel
> Stone ; Varad Gautam
> ; Liu, Zhiquan ; Frank
> Binns ; Brendan King ;
> Martin Peres 
> Subject: Re: [PATCH 2/2] i965: Queue the buffer with a sync fence for Android
> OS v4.2
> 
> Hi Zhongmin,
> 
> Thanks you for the update. There's a couple of important comments -
> dri2_make_current + droid_window_enqueue_buffer.
> The rest is just nitpiks.
> 
> Tomasz, hats down for the immense help and guidance.
> 
> On the potential performance hit (due to the extra fence), I think we could do
> some tests before adding extra infra.
> No obvious benchmarks come to mind - any suggestions?
> 

Sorry to jump in, flatland is the one native application on android that tests 
this
explicitly. It gives time required to render one frame of particular resolution 
without
other services running. It’s a native app that comes with aosp. And we found 
this
issue just because of that.

App info - 
https://android.googlesource.com/platform/frameworks/native/+/master/cmds/flatland/README.txt
Bug - https://bugs.freedesktop.org/show_bug.cgi?id=101655

I already tested this patch set with android and I can see scores not being 
that great.
May be this is the one we can use to profile this or I can continue to profile 
based on
guidance here.

> On 25 July 2017 at 03:07, 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
> >
> > Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
> > Signed-off-by: Zhongmin Wu 
> > ---
> >  src/egl/drivers/dri2/egl_dri2.c |   51 
> > +++
> >  src/egl/drivers/dri2/egl_dri2.h |8 +
> >  src/egl/drivers/dri2/platform_android.c |   12 ---
> >  src/egl/drivers/dri2/platform_drm.c |3 +-
> >  src/egl/drivers/dri2/platform_surfaceless.c |3 +-
> >  src/egl/drivers/dri2/platform_wayland.c |3 +-
> >  src/egl/drivers/dri2/platform_x11.c |3 +-
> >  src/egl/drivers/dri2/platform_x11_dri3.c|3 +-
> >  8 files changed, 77 insertions(+), 9 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/egl_dri2.c
> > b/src/egl/drivers/dri2/egl_dri2.c index 020a0bc..ffd3a8a 100644
> > --- a/src/egl/drivers/dri2/egl_dri2.c
> > +++ b/src/egl/drivers/dri2/egl_dri2.c
> > @@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv,
> _EGLDisplay *disp, _EGLContext *ctx)
> > return EGL_TRUE;
> >  }
> >
> > +EGLBoolean
> > +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> nit: s/dri2_surf_init/dri2_init_surface/
> 
> > +_EGLConfig *conf, const EGLint *attrib_list) {
> > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > +   dri2_surf->out_fence_fd = -1;
> > +   return _eglInitSurface(surf, dpy, type, conf, attrib_list); }
> > +
> > +static void
> > +dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd)
> nit: s/dri2_surface_set_out_fence/dri2_surface_set_out_fence_fd/
> 
> > +{
> > +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> > +   if (dri2_surf->out_fence_fd >=0)
> nit: space between = and 0
> 
> > +  close(dri2_surf->out_fence_fd);
> > +
> > +   dri2_surf->out_fence_fd = fence_fd; }
> > +
> > +void
> > 

Re: [Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Emil Velikov]

Hi Zhongmin,

Thanks you for the update. There's a couple of important comments -
dri2_make_current + droid_window_enqueue_buffer.
The rest is just nitpiks.

Tomasz, hats down for the immense help and guidance.

On the potential performance hit (due to the extra fence), I think we
could do some tests before adding extra infra.
No obvious benchmarks come to mind - any suggestions?

On 25 July 2017 at 03:07, 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
>
> Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
> Signed-off-by: Zhongmin Wu 
> ---
>  src/egl/drivers/dri2/egl_dri2.c |   51 
> +++
>  src/egl/drivers/dri2/egl_dri2.h |8 +
>  src/egl/drivers/dri2/platform_android.c |   12 ---
>  src/egl/drivers/dri2/platform_drm.c |3 +-
>  src/egl/drivers/dri2/platform_surfaceless.c |3 +-
>  src/egl/drivers/dri2/platform_wayland.c |3 +-
>  src/egl/drivers/dri2/platform_x11.c |3 +-
>  src/egl/drivers/dri2/platform_x11_dri3.c|3 +-
>  8 files changed, 77 insertions(+), 9 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index 020a0bc..ffd3a8a 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay 
> *disp, _EGLContext *ctx)
> return EGL_TRUE;
>  }
>
> +EGLBoolean
> +dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
nit: s/dri2_surf_init/dri2_init_surface/

> +_EGLConfig *conf, const EGLint *attrib_list)
> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   dri2_surf->out_fence_fd = -1;
> +   return _eglInitSurface(surf, dpy, type, conf, attrib_list);
> +}
> +
> +static void
> +dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd)
nit: s/dri2_surface_set_out_fence/dri2_surface_set_out_fence_fd/

> +{
> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   if (dri2_surf->out_fence_fd >=0)
nit: space between = and 0

> +  close(dri2_surf->out_fence_fd);
> +
> +   dri2_surf->out_fence_fd = fence_fd;
> +}
> +
> +void
> +dri2_surf_deinit(_EGLSurface *surf)
> +{
nit: s/dri2_surf_deinit/dri2_fini_surface/

> +   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
> +   dri2_surface_set_out_fence(surf, -1);
> +}
> +
>  static EGLBoolean
>  dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
>  {
> @@ -1318,6 +1344,22 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay 
> *dpy, _EGLSurface *surf)
> return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);
>  }
>
> +static void
> +dri2_surf_get_fence_fd(_EGLContext *ctx,
> +   _EGLDisplay *dpy, _EGLSurface *surf)
> +{
> +   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
> +   int fence_fd = -1;
> +   __DRIcontext *dri_ctx = dri2_egl_context(ctx)->dri_context;
> +   void * fence = dri2_dpy->fence->create_fence_fd(dri_ctx, -1);
Add blank line between declarations and code.

> +   if (fence) {
> +  fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen,
> +   fence);
> +  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
> +   }
> +   dri2_surface_set_out_fence(surf, fence_fd);
> +}
> +
>  /**
>   * Called via eglMakeCurrent(), drv->API.MakeCurrent().
>   */
> @@ -1352,8 +1394,11 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, 
> _EGLSurface *dsurf,
> rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
> cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
>
> +   int fence_fd = -1;
Unused variable?

> if (old_ctx) {
>__DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
> +  if (old_dsurf)
> + dri2_surf_get_fence_fd(old_ctx, disp, old_dsurf);
Are you sure 

[Mesa-dev] [PATCH 2/2] i965: Queue the buffer with a sync fence for Android OS v4.2

[Zhongmin Wu]

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

Change-Id: Ided54d2e193cde73a6f0feb36ac1c0056e4958f2
Signed-off-by: Zhongmin Wu 
---
 src/egl/drivers/dri2/egl_dri2.c |   51 +++
 src/egl/drivers/dri2/egl_dri2.h |8 +
 src/egl/drivers/dri2/platform_android.c |   12 ---
 src/egl/drivers/dri2/platform_drm.c |3 +-
 src/egl/drivers/dri2/platform_surfaceless.c |3 +-
 src/egl/drivers/dri2/platform_wayland.c |3 +-
 src/egl/drivers/dri2/platform_x11.c |3 +-
 src/egl/drivers/dri2/platform_x11_dri3.c|3 +-
 8 files changed, 77 insertions(+), 9 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 020a0bc..ffd3a8a 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1307,6 +1307,32 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay *disp, 
_EGLContext *ctx)
return EGL_TRUE;
 }
 
+EGLBoolean
+dri2_surf_init(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
+_EGLConfig *conf, const EGLint *attrib_list)
+{
+   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+   dri2_surf->out_fence_fd = -1;
+   return _eglInitSurface(surf, dpy, type, conf, attrib_list);
+}
+
+static void
+dri2_surface_set_out_fence( _EGLSurface *surf, int fence_fd)
+{
+   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+   if (dri2_surf->out_fence_fd >=0)
+  close(dri2_surf->out_fence_fd);
+
+   dri2_surf->out_fence_fd = fence_fd;
+}
+
+void
+dri2_surf_deinit(_EGLSurface *surf)
+{
+   struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf);
+   dri2_surface_set_out_fence(surf, -1);
+}
+
 static EGLBoolean
 dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
 {
@@ -1318,6 +1344,22 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, 
_EGLSurface *surf)
return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf);
 }
 
+static void
+dri2_surf_get_fence_fd(_EGLContext *ctx,
+   _EGLDisplay *dpy, _EGLSurface *surf)
+{
+   struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
+   int fence_fd = -1;
+   __DRIcontext *dri_ctx = dri2_egl_context(ctx)->dri_context;
+   void * fence = dri2_dpy->fence->create_fence_fd(dri_ctx, -1);
+   if (fence) {
+  fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen,
+   fence);
+  dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence);
+   }
+   dri2_surface_set_out_fence(surf, fence_fd);
+}
+
 /**
  * Called via eglMakeCurrent(), drv->API.MakeCurrent().
  */
@@ -1352,8 +1394,11 @@ dri2_make_current(_EGLDriver *drv, _EGLDisplay *disp, 
_EGLSurface *dsurf,
rdraw = (rsurf) ? dri2_dpy->vtbl->get_dri_drawable(rsurf) : NULL;
cctx = (dri2_ctx) ? dri2_ctx->dri_context : NULL;
 
+   int fence_fd = -1;
if (old_ctx) {
   __DRIcontext *old_cctx = dri2_egl_context(old_ctx)->dri_context;
+  if (old_dsurf)
+ dri2_surf_get_fence_fd(old_ctx, disp, old_dsurf);
   dri2_dpy->core->unbindContext(old_cctx);
}
 
@@ -1490,6 +1535,9 @@ static EGLBoolean
 dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf)
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
+   _EGLContext *ctx = _eglGetCurrentContext();
+   if (ctx && surf)
+  dri2_surf_get_fence_fd(ctx, dpy, surf);
return dri2_dpy->vtbl->swap_buffers(drv, dpy, surf);
 }
 
@@ -1499,6 +1547,9 @@ dri2_swap_buffers_with_damage(_EGLDriver *drv, 
_EGLDisplay *dpy,
   const EGLint *rects, EGLint n_rects)
 {
struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy);
+   _EGLContext *ctx = _eglGetCurrentContext();
+   if (ctx && surf)
+  dri2_surf_get_fence_fd(ctx, dpy, surf);
return dri2_dpy->vtbl->swap_buffers_with_damage(drv, dpy, surf,
rects, n_rects);
 }
diff --git