i965: Fix asynchronous mappings on !LLC platforms.

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
71651b31.

v2: Do the invalidate later, and even when asking for a brand new map.
v3: Add more comments from Chris.
Reviewed-by: Chris Wilson's avatarChris Wilson <chris@chris-wilson.co.uk>
parent 20104f19
......@@ -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"
......@@ -703,11 +704,25 @@ brw_bo_map_cpu(struct brw_context *brw, struct brw_bo *bo, unsigned flags)
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);
}
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!)
* Even if it's a brand new mapping, the kernel may have zeroed the
* buffer via CPU writes.
*
* We need to invalidate those cachelines so that we see the latest
* contents, and so long as we only read from the CPU mmap we do not
* need to write those cachelines back afterwards.
*/
gen_invalidate_range(bo->map_cpu, bo->size);
}
return bo->map_cpu;
}
......@@ -754,7 +769,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);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment