[Mesa-dev] [PATCH 12/16] i965: Use write-combine mappings where available

Chris Wilson chris at chris-wilson.co.uk
Thu May 25 06:47:56 UTC 2017


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.

> +
> +   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


More information about the mesa-dev mailing list