[Mesa-dev] [PATCH] egl: dri2: Use present extension. (Was: Re: [RFC] egl: Add DRI3 support to the EGL backend.)

[Joonas Lahtinen]

On ke, 2014-11-05 at 15:19 +0000, Emil Velikov wrote:
> Hi Joonas,
> 
> Does getting rid of the viewport hack give you any noticeable
> performance improvement?

Yes, it significantly reduces the CPU load when multiple glViewport
calls are made per frame (4x4 grid or so).

> Is there any interest in converting the
> egl_dri2 backend to dri3, rather than just copying over the present bits ?
> 

This could be one thing to do. But in the meanwhile, I would commit this
present extension patch so that the affected use cases get the
improvements.

Regards, Joonas

> On 05/11/14 11:14, Joonas Lahtinen wrote:
> > Hi,
> > 
> > Modified not refer to DRI3, just uses the present extension to get rid
> > of the excess buffer invalidations.
> > 
> > Regards, Joonas
> > 
> > From 257e2a8c750f9dcf868cce9da8632df3cae0fcec Mon Sep 17 00:00:00 2001
> > From: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Date: Wed, 5 Nov 2014 12:25:32 +0200
> > Subject: [PATCH] egl: dri2: Use present extension.
> > 
> > Present extension is used to avoid excess buffer invalidations, because
> > the XCB interface doesn't supply that information.
> > 
> > Signed-off-by: Daniel van der Wath <danielx.j.van.der.wath at intel.com>
> > Signed-off-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > ---
> >  configure.ac                            |    5 +-
> >  src/egl/drivers/dri2/egl_dri2.c         |    2 +-
> >  src/egl/drivers/dri2/egl_dri2.h         |   24 ++-
> >  src/egl/drivers/dri2/platform_x11.c     |  247 ++++++++++++++++++++++++++++---
> >  src/mesa/drivers/dri/i965/brw_context.c |    9 +-
> >  5 files changed, 262 insertions(+), 25 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index fc7d372..75d90c0 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -952,7 +952,8 @@ xyesno)
> >              fi
> >  
> >              if test x"$enable_dri" = xyes; then
> > -               dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED"
> > +               PKG_CHECK_MODULES([PRESENTPROTO], [presentproto >= $PRESENTPROTO_REQUIRED])
> > +               dri_modules="$dri_modules xcb-dri2 >= $XCBDRI2_REQUIRED xcb-present"
> Afaics you are not changing anything on the dri modules (or glx/dri2) to
> require the above changes. Perhaps you need to push the presentproto
> check in the x11 case below ?
> 
> >              fi
> >  
> >              if test x"$enable_dri3" = xyes; then
> > @@ -1564,7 +1565,7 @@ for plat in $egl_platforms; do
> >  		;;
> >  
> >  	x11)
> > -		PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 >= $XCBDRI2_REQUIRED xcb-xfixes])
> > +		PKG_CHECK_MODULES([XCB_DRI2], [x11-xcb xcb xcb-dri2 >= $XCBDRI2_REQUIRED xcb-xfixes xcb-present])
> >  		;;
> >  
> >  	drm)
> [snip]
> > diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c
> > index f8c4b70..a1445b2 100644
> > --- a/src/egl/drivers/dri2/platform_x11.c
> > +++ b/src/egl/drivers/dri2/platform_x11.c
> > @@ -188,6 +188,205 @@ get_xcb_screen(xcb_screen_iterator_t iter, int screen)
> >      return NULL;
> >  }
> >  
> > +/*
> > + * Called by the XCB_PRESENT_COMPLETE_NOTIFY case.
> > + */
> > +static void
> > +dri2_update_num_back(struct dri2_egl_surface *priv)
> > +{
> > +   priv->num_back = 1;
> > +   if (priv->flipping)
> > +      priv->num_back++;
> > +   if (priv->base.SwapInterval == 0)
> > +      priv->num_back++;
> > +}
> > +
> This seems to be out of sync with dri3_glx. Don't you need something
> similar to commit f7a355556ef5fe23056299a77414f9ad8b5e5a1d ?
> 
> [snip]
> > +/**
> > + *
> > + * Process any present events that have been received from the X server
> > + *
> > + * From glx, we additionally invalidate the drawable here if there has a been a special event.
> > + */
> > +static void
> > +dri2_flush_present_events(struct dri2_egl_display *dri2_dpy, struct dri2_egl_surface *priv)
> > +{
> > +   xcb_connection_t     *c = dri2_dpy->conn;
> > +
> > +   /* Check to see if any configuration changes have occurred
> > +    * since we were last invoked
> > +    */
> > +   if (priv->special_event) {
> > +      xcb_generic_event_t    *ev;
> > +
> > +      while ((ev = xcb_poll_for_special_event(c, priv->special_event)) != NULL) {
> > +         xcb_present_generic_event_t *ge = (void *) ev;
> > +         dri2_handle_present_event(priv, ge);
> > +         _eglLog(_EGL_INFO, "DRI: Invalidating buffer 0x%x\n", priv->dri_drawable);
> > +         (*dri2_dpy->flush->invalidate)(priv->dri_drawable);
> Hmm why does one need to invalidate at this stage - I take that it's
> related to lack of fence objects ?
> 
> [snip]
> > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> > index e1a994a..dbadd10 100644
> > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > @@ -148,6 +148,9 @@ intel_viewport(struct gl_context *ctx)
> >     __DRIcontext *driContext = brw->driContext;
> >  
> >     if (_mesa_is_winsys_fbo(ctx->DrawBuffer)) {
> > +      if (unlikely(INTEL_DEBUG & DEBUG_DRI))
> > +         fprintf(stderr, "invalidating drawables\n");
> > +
> >        dri2InvalidateDrawable(driContext->driDrawablePriv);
> >        dri2InvalidateDrawable(driContext->driReadablePriv);
> >     }
> > @@ -252,11 +255,9 @@ brw_init_driver_functions(struct brw_context *brw,
> >     _mesa_init_driver_functions(functions);
> >  
> >     /* GLX uses DRI2 invalidate events to handle window resizing.
> > -    * Unfortunately, EGL does not - libEGL is written in XCB (not Xlib),
> > -    * which doesn't provide a mechanism for snooping the event queues.
> > +    * EGL uses present invalidate events to do the same.
> >      *
> > -    * So EGL still relies on viewport hacks to handle window resizing.
> > -    * This should go away with DRI3000.
> > +    * Others have to rely on viewport hacks to handle window resizing.
> >      */
> Bikeshed: maybe move the i965 changes in a separate patch ?
> 
> 
> Cheers,
> Emil
> 
> >     if (!brw->driContext->driScreenPriv->dri2.useInvalidate)
> >        functions->Viewport = intel_viewport;
> > 
>