On Thu, May 25, 2017 at 07:47:56AM +0100, Chris Wilson wrote:
> On Wed, May 24, 2017 at 01:04:54PM -0700, Matt Turner wrote:
> > Write-combine mappings give much better performance on writes than
> > uncached access through the GTT.
> > ---
> > src/mesa/drivers/dri/i965/brw_bufmgr.c | 69
> > ++++++++++++++++++++++++++++++++++
> > 1 file changed, 69 insertions(+)
> >
> > diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > index aec07e1..bbcda04 100644
> > --- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > +++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
> > @@ -110,6 +110,7 @@ struct brw_bufmgr {
> > struct hash_table *handle_table;
> >
> > bool has_llc:1;
> > + bool has_mmap_wc:1;
> > bool bo_reuse:1;
> > };
> >
> > @@ -701,6 +702,48 @@ brw_bo_map_cpu(struct brw_context *brw, struct brw_bo
> > *bo, unsigned flags)
> > }
> >
> > static void *
> > +brw_bo_map_wc(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
> > +{
> > + struct brw_bufmgr *bufmgr = bo->bufmgr;
> > +
> > + pthread_mutex_lock(&bufmgr->lock);
> > +
> > + if (!bo->map_wc) {
> > + struct drm_i915_gem_mmap mmap_arg;
> > +
> > + DBG("brw_bo_map_wc: %d (%s), map_count=%d\n",
> > + bo->gem_handle, bo->name, bo->map_count);
> > +
> > + memclear(mmap_arg);
> > + mmap_arg.handle = bo->gem_handle;
> > + mmap_arg.size = bo->size;
> > + mmap_arg.flags = I915_MMAP_WC;
> > + int ret = drmIoctl(bufmgr->fd, DRM_IOCTL_I915_GEM_MMAP, &mmap_arg);
> > + if (ret != 0) {
> > + ret = -errno;
> > + DBG("%s:%d: Error mapping buffer %d (%s): %s .\n",
> > + __FILE__, __LINE__, bo->gem_handle, bo->name,
> > strerror(errno));
> > + pthread_mutex_unlock(&bufmgr->lock);
> > + return NULL;
> > + }
> > + bo->map_count++;
> > + VG(VALGRIND_MALLOCLIKE_BLOCK(mmap_arg.addr_ptr, mmap_arg.size, 0,
> > 1));
> > + bo->map_wc = (void *) (uintptr_t) mmap_arg.addr_ptr;
> > + }
> > + DBG("brw_bo_map_wc: %d (%s) -> %p\n", bo->gem_handle, bo->name,
> > bo->map_wc);
> > +
> > + if (!(flags & MAP_ASYNC))
> > + set_domain(brw, "WC mapping", bo, I915_GEM_DOMAIN_GTT,
> > + flags & MAP_WRITE ? I915_GEM_DOMAIN_GTT : 0);
>
> I had to make WC a separate cache domain, which is required if you want
> to mix GTT access and WC access (on !llc). The kernel may use GTT
> access for pread, pwrite and relocations. To be safe use
> I915_GEM_DOMAIN_WC (0x80)
>
> Note that write_domain must always be the read_domain and using more
> than read_domain is meaningless (at present, and for the api since it is
> meant to declare the next access). You could simplify set_domain() just
> to take flags (for MAP_WRITE) and the domain.
Might be good to quite the kernel commit in full in the commit message for
this:
commit e22d8e3c69a9f432b40baaaf3f894a128fdc2222
Author: Chris Wilson <ch...@chris-wilson.co.uk>
Date: Wed Apr 12 12:01:11 2017 +0100
drm/i915: Treat WC a separate cache domain
When discussing a new WC mmap, we based the interface upon the
assumption that GTT was fully coherent. How naive! Commits 3b5724d702ef
("drm/i915: Wait for writes through the GTT to land before reading
back") and ed4596ea992d ("drm/i915/guc: WA to address the Ringbuffer
coherency issue") demonstrate that writes through the GTT are indeed
delayed and may be overtaken by direct WC access. To be safe, if
userspace is mixing WC mmaps with other potential GTT access (pwrite,
GTT mmaps) it should use set_domain(WC).
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96563
Testcase: igt/gem_pwrite/small-gtt*
Testcase: igt/drv_selftest/coherency
Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Link:
http://patchwork.freedesktop.org/patch/msgid/20170412110111.26626-2-ch...@chris-wilson.co.uk
Note you need to check for recent enough kernel before using this :-/
-Daniel
>
> > +
> > + bo_mark_mmaps_incoherent(bo);
> > + VG(VALGRIND_MAKE_MEM_DEFINED(bo->map_wc, bo->size));
> > + pthread_mutex_unlock(&bufmgr->lock);
> > +
> > + return bo->map_wc;
> > +}
> > +
> > +static void *
> > brw_bo_map_gtt(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
> > {
> > struct brw_bufmgr *bufmgr = bo->bufmgr;
> > @@ -771,10 +814,14 @@ can_map_cpu(struct brw_bo *bo, unsigned flags)
> > void *
> > brw_bo_map(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
> > {
> > + struct brw_bufmgr *bufmgr = bo->bufmgr;
> > +
> > if (bo->tiling_mode != I915_TILING_NONE && !(flags & MAP_RAW))
> > return brw_bo_map_gtt(brw, bo, flags);
> > else if (can_map_cpu(bo, flags))
> > return brw_bo_map_cpu(brw, bo, flags);
> > + else if (bufmgr->has_mmap_wc)
> > + return brw_bo_map_wc(brw, bo, flags);
> > else
> > return brw_bo_map_gtt(brw, bo, flags);
> > }
> > @@ -1177,6 +1224,27 @@ brw_reg_read(struct brw_bufmgr *bufmgr, uint32_t
> > offset, uint64_t *result)
> > return ret;
> > }
> >
> > +static int
> > +gem_param(int fd, int name)
> > +{
> > + drm_i915_getparam_t gp;
> > + int v = -1; /* No param uses (yet) the sign bit, reserve it for errors
> > */
> > +
> > + memset(&gp, 0, sizeof(gp));
> > + gp.param = name;
> > + gp.value = &v;
> > + if (drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
> > + return -1;
> > +
> > + return v;
> > +}
> > +
> > +static bool
> > +test_has_mmap_wc(int fd)
> > +{
> > + return gem_param(fd, I915_PARAM_MMAP_VERSION) > 0;
>
> Of course I had to make this more complicated :(
>
> static bool
> test_has_mmap_wc_ioctl(int fd)
> {
> return gem_param(fd, I915_PARAM_MMAP_VERSION) > 0;
> }
>
> static bool
> test_has_wc_domain(int fd)
> {
> return gem_param(fd, I915_PARAM_MMAP_GTT_VERSION) >= 2;
> }
>
> static bool
> test_has_mmap_wc(int fd)
> {
> return test_has_mmap_wc_ioctl(fd) && test_has_wc_domain(fd);
> }
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
