Hi Joonas,
Does getting rid of the viewport hack give you any noticeable
performance improvement ? Is there any interest in converting the
egl_dri2 backend to dri3, rather than just copying over the present bits ?
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
