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.lahti...@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.w...@intel.com>
> > Signed-off-by: Joonas Lahtinen <joonas.lahti...@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;
> >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
