[Mesa-dev] [PATCH] i965: Share the workaround bo between all contexts
Chad Versace
chadversary at chromium.org
Thu Jan 26 17:39:51 UTC 2017
On Thu 26 Jan 2017, Chris Wilson wrote:
> Since the workaround bo is used strictly as a write-only buffer, we need
> only allocate one per screen and use the same one from all contexts.
>
> (The caveat here is during extension initialisation, where we write into
> and read back register values from the buffer, but that is performed only
> once for the first context - and baring synchronisation issues should not
> be a problem. Safer would be to move that also to the screen.)
>
> v2: Give the workaround bo its own init function and don't piggy back
> intel_bufmgr_init() since it is not that related.
>
> v3: Drop the reference count of the workaround bo for the context since
> the context itself is owned by the screen (and so we can rely on the bo
> existing for the lifetime of the context).
I like this idea, but I have questions and comments about the details.
More questions than comments, really.
Today, with only Mesa changes, could we effectively do the same as
drm_intel_gem_bo_disable_implicit_sync(screen->workaround_bo);
by hacking Mesa to set no read/write domain when emitting relocs for the
workaround_bo? (I admit I don't fully understand the kernel's domain
tracking). If that does work, then it just would require a small hack to
brw_emit_pipe_control_write().
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Martin Peres <martin.peres at linux.intel.com>
> Cc: Chad Versace <chadversary at chromium.org>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> src/mesa/drivers/dri/i965/Makefile.am | 2 +-
> src/mesa/drivers/dri/i965/brw_pipe_control.c | 12 +++++-----
> src/mesa/drivers/dri/i965/intel_screen.c | 24 ++++++++++++++++++++
> src/mesa/drivers/dri/i965/intel_screen.h | 1 +
> src/mesa/drivers/dri/i965/libdrm_compat.h | 33 ++++++++++++++++++++++++++++
> 5 files changed, 64 insertions(+), 8 deletions(-)
> create mode 100644 src/mesa/drivers/dri/i965/libdrm_compat.h
>
> diff --git a/src/mesa/drivers/dri/i965/Makefile.am b/src/mesa/drivers/dri/i965/Makefile.am
> index 6602a17995..b208563f7d 100644
> --- a/src/mesa/drivers/dri/i965/Makefile.am
> +++ b/src/mesa/drivers/dri/i965/Makefile.am
> @@ -77,7 +77,7 @@ noinst_LTLIBRARIES = \
> libi965_compiler.la \
> $(I965_PERGEN_LIBS)
>
> -libi965_dri_la_SOURCES = $(i965_FILES)
> +libi965_dri_la_SOURCES = $(i965_FILES) libdrm_compat.h
> libi965_dri_la_LIBADD = \
> $(top_builddir)/src/intel/common/libintel_common.la \
> $(top_builddir)/src/intel/isl/libisl.la \
> diff --git a/src/mesa/drivers/dri/i965/brw_pipe_control.c b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> index b8f740640f..22c946f744 100644
> --- a/src/mesa/drivers/dri/i965/brw_pipe_control.c
> +++ b/src/mesa/drivers/dri/i965/brw_pipe_control.c
> @@ -371,20 +371,18 @@ brw_init_pipe_control(struct brw_context *brw,
> /* We can't just use brw_state_batch to get a chunk of space for
> * the gen6 workaround because it involves actually writing to
> * the buffer, and the kernel doesn't let us write to the batch.
> + *
> + * As the screen has a long lifetime than the contexts derived from
> + * it, we do not need to add our own reference count and can simply
> + * rely on the bo always existing for the duration of the context.
> */
> - brw->workaround_bo = drm_intel_bo_alloc(brw->bufmgr,
> - "pipe_control workaround",
> - 4096, 4096);
> - if (brw->workaround_bo == NULL)
> - return -ENOMEM;
> + brw->workaround_bo = brw->screen->workaround_bo;
>
> brw->pipe_controls_since_last_cs_stall = 0;
> -
> return 0;
> }
>
> void
> brw_fini_pipe_control(struct brw_context *brw)
> {
> - drm_intel_bo_unreference(brw->workaround_bo);
> }
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
> index 5f800008c1..6e788c41cc 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -107,6 +107,7 @@ DRI_CONF_END
> #include "brw_context.h"
>
> #include "i915_drm.h"
> +#include "libdrm_compat.h"
>
> /**
> * For debugging purposes, this returns a time in seconds.
> @@ -1030,6 +1031,7 @@ intelDestroyScreen(__DRIscreen * sPriv)
> {
> struct intel_screen *screen = sPriv->driverPrivate;
>
> + drm_intel_bo_unreference(screen->workaround_bo);
> dri_bufmgr_destroy(screen->bufmgr);
> driDestroyOptionInfo(&screen->optionCache);
>
> @@ -1210,6 +1212,25 @@ intel_init_bufmgr(struct intel_screen *screen)
> }
>
> static bool
> +intel_init_workaround_bo(struct intel_screen *screen)
> +{
> + /* A small scratch bo shared by all contexts, primarily used
> + * for doing PIPECONTROL serialisation writes that are discarded.
> + */
> + screen->workaround_bo =
> + drm_intel_bo_alloc(screen->bufmgr, "pipe_control w/a", 4096, 4096);
> +
> + /* We want to use this bo from any and all contexts, without undue
> + * writing ordering between them. To prevent the kernel enforcing
> + * the order due to writes from different contexts, we disable
> + * the use of (the kernel's) implicit sync on this bo.
> + */
> + drm_intel_gem_bo_disable_implicit_sync(screen->workaround_bo);
> +
> + return screen->workaround_bo != NULL;
> +}
> +
> +static bool
> intel_detect_swizzling(struct intel_screen *screen)
> {
> drm_intel_bo *buffer;
> @@ -1675,6 +1696,9 @@ __DRIconfig **intelInitScreen2(__DRIscreen *dri_screen)
> if (!intel_init_bufmgr(screen))
> return false;
>
> + if (!intel_init_workaround_bo(screen))
> + return false;
> +
> screen->deviceID = drm_intel_bufmgr_gem_get_devid(screen->bufmgr);
> if (!gen_get_device_info(screen->deviceID, &screen->devinfo))
> return false;
> diff --git a/src/mesa/drivers/dri/i965/intel_screen.h b/src/mesa/drivers/dri/i965/intel_screen.h
> index 890dd9044b..0fb83e724f 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.h
> +++ b/src/mesa/drivers/dri/i965/intel_screen.h
> @@ -74,6 +74,7 @@ struct intel_screen
> #define KERNEL_ALLOWS_COMPUTE_DISPATCH (1<<4)
>
> dri_bufmgr *bufmgr;
> + drm_intel_bo *workaround_bo;
>
> /**
> * A unique ID for shader programs.
> diff --git a/src/mesa/drivers/dri/i965/libdrm_compat.h b/src/mesa/drivers/dri/i965/libdrm_compat.h
> new file mode 100644
> index 0000000000..bef9a1286b
> --- /dev/null
> +++ b/src/mesa/drivers/dri/i965/libdrm_compat.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifndef __LIBDRM_COMPAT_H
> +#define __LIBDRM_COMPAT_H
> +
> +#include <intel_bufmgr.h>
> +
> +#ifndef HAVE_DRM_INTEL_GEM_BO_DISABLE_IMPLICIT_SYNC
> +#define drm_intel_gem_bo_disable_implicit_sync(BO) do { } while (0)
> +#endif
> +
> +#endif /* !__LIBDRM_COMPAT_H */
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list