[Mesa-dev] [PATCH 1/2] i965/bufmgr: Explicitly wait instead of using I915_GEM_SET_DOMAIN.

[Kenneth Graunke]

With the advent of asynchronous maps, domain tracking doesn't make a
whole lot of sense.  Buffers can be in use on both the CPU and GPU at
the same time.  In order to avoid blocking, we stopped using set_domain
for asynchronous mappings, which means that the kernel's tracking has
lies.  We can't properly track it in userspace either, as the kernel
can change domains on us spontaneously (for example, when un-swapping).

I915_GEM_SET_DOMAIN combines two aspects: cache flushing, and waiting
for a buffer to be idle.  In order to stop using it, we'll need to do
clflushing ourselves, and use I915_GEM_WAIT to wait on buffers.

For cache-coherent buffers (most on LLC systems), we don't need to do
any clflushing - the CPU and GPU views are coherent.  For non-coherent
buffers (most on non-LLC systems), we currently only use the CPU for
read-only maps, and we explicitly clflush when necessary.
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

Here's a strawman patch for Chris to shoot down. :)  I'm probably
missing something obvious, but it seems like our existing
gen_invalidate_range when CPU mapping non-coherent buffers ought
to be sufficient.  If we were doing CPU write mapping, we'd
definitely need more tracking, but we don't, so...

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 46da53d3530..e1b98755e39 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -657,22 +657,13 @@ brw_bo_unreference(struct brw_bo *bo)
 }
 
 static void
-set_domain(struct brw_context *brw, const char *action,
-           struct brw_bo *bo, uint32_t read_domains, uint32_t write_domain)
+bo_wait_with_stall_warning(struct brw_context *brw,
+                           struct brw_bo *bo,
+                           const char *action)
 {
-   struct drm_i915_gem_set_domain sd = {
-      .handle = bo->gem_handle,
-      .read_domains = read_domains,
-      .write_domain = write_domain,
-   };
-
    double elapsed = unlikely(brw && brw->perf_debug) ? -get_time() : 0.0;
 
-   if (drmIoctl(bo->bufmgr->fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &sd) != 0) {
-      DBG("%s:%d: Error setting memory domains %d (%08x %08x): %s.\n",
-          __FILE__, __LINE__, bo->gem_handle, read_domains, write_domain,
-          strerror(errno));
-   }
+   brw_bo_wait_rendering(bo);
 
    if (unlikely(brw && brw->perf_debug)) {
       elapsed += get_time();
@@ -742,8 +733,7 @@ brw_bo_map_cpu(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
    print_flags(flags);
 
    if (!(flags & MAP_ASYNC)) {
-      set_domain(brw, "CPU mapping", bo, I915_GEM_DOMAIN_CPU,
-                 flags & MAP_WRITE ? I915_GEM_DOMAIN_CPU : 0);
+      bo_wait_with_stall_warning(brw, bo, "CPU mapping");
    }
 
    if (!bo->cache_coherent) {
@@ -813,8 +803,7 @@ brw_bo_map_gtt(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
    print_flags(flags);
 
    if (!(flags & MAP_ASYNC)) {
-      set_domain(brw, "GTT mapping", bo,
-                 I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
+      bo_wait_with_stall_warning(brw, bo, "GTT mapping");
    }
 
    return bo->map_gtt;
-- 
2.13.3