[Mesa-dev] [PATCH 7/7] i965: Fix asynchronous mappings on !LLC platforms.

[Kenneth Graunke]

When using a read-only CPU mapping, we may encounter stale buffer
contents.  For example, the Piglit primitive-restart test offers the
following scenario:

   1. Read data via a CPU map.
   2. Destroy that buffer.
   3. Create a new buffer - obtaining the same one via the BO cache.
   4. Call BufferSubData, which does a GTT map with MAP_WRITE | MAP_ASYNC.
      (We avoid set_domain for async mappings, so no flushing occurs.)
   5. Read data via a CPU map.
      (Without explicit clflushing, this will contain data from step 1!)

Otherwise, everything ought to work, keeping in mind that we never use
CPU maps for writing - just read-only CPU maps.

This restores the performance gains after Matt's revert in commit
71651b3139c501f50e6547c21a1cdb816b0a9dde.
---
 src/mesa/drivers/dri/i965/brw_bufmgr.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 7756e2b5f6c..46696be3577 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -56,6 +56,7 @@
 #ifndef ETIME
 #define ETIME ETIMEDOUT
 #endif
+#include "common/gen_clflush.h"
 #include "common/gen_debug.h"
 #include "common/gen_device_info.h"
 #include "libdrm_macros.h"
@@ -698,12 +699,22 @@ brw_bo_map_cpu(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
          VG(VALGRIND_FREELIKE_BLOCK(map, 0));
          drm_munmap(map, bo->size);
       }
+   } else if (!bo->cache_coherent) {
+      /* If we're reusing an existing CPU mapping, the CPU caches may
+       * contain stale data from the last time we read from that mapping.
+       * (With the BO cache, it might even be data from a previous buffer!)
+       *
+       * We need to invalidate those cachelines so that we see the latest
+       * contents.
+       */
+      gen_invalidate_range(bo->map_cpu, bo->size);
    }
+
    DBG("brw_bo_map_cpu: %d (%s) -> %p, ", bo->gem_handle, bo->name,
        bo->map_cpu);
    print_flags(flags);
 
-   if (!(flags & MAP_ASYNC) || !bufmgr->has_llc) {
+   if (!(flags & MAP_ASYNC)) {
       set_domain(brw, "CPU mapping", bo, I915_GEM_DOMAIN_CPU,
                  flags & MAP_WRITE ? I915_GEM_DOMAIN_CPU : 0);
    }
@@ -754,7 +765,7 @@ brw_bo_map_gtt(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
    DBG("bo_map_gtt: %d (%s) -> %p, ", bo->gem_handle, bo->name, bo->map_gtt);
    print_flags(flags);
 
-   if (!(flags & MAP_ASYNC) || !bufmgr->has_llc) {
+   if (!(flags & MAP_ASYNC)) {
       set_domain(brw, "GTT mapping", bo,
                  I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
    }
-- 
2.13.2