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

[Kenneth Graunke]

On Wednesday, May 24, 2017 1:04:54 PM PDT 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);

Curly braces.

I think Jason and Chris concluded on IRC that we don't need the WC
domains - there are circumstances where it's needed, but we should
never hit those in i965.

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

I talked with Daniel Vetter on IRC back in March, and he mentioned that
WC mappings may be buggy on Gen4 (and maybe 4.5?).  Something about
L-shaped memory and undocumented bugs in the flushing logic.  They might
work for linear buffers, but for tiled buffers, supposedly there are
"not quite as coherent as we thought" dragons.

It looks like we're not using WC maps on tiled surfaces, so we're
probably OK...

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

>     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;
> +}
> +
>  /**
>   * Initializes the GEM buffer manager, which uses the kernel to allocate, map,
>   * and manage map buffer objections.
> @@ -1209,6 +1277,7 @@ brw_bufmgr_init(struct gen_device_info *devinfo, int fd, int batch_size)
>     }
>  
>     bufmgr->has_llc = devinfo->has_llc;
> +   bufmgr->has_mmap_wc = test_has_mmap_wc(fd);
>  
>     init_cache_buckets(bufmgr);
>  
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170530/24f2b7ea/attachment.sig>