Bug 101571

Vulkan is accessing the incoherent buffers without alerting the kernel ever to the change of cache domain. Vulkan tries to take over cache management, but left the kernel thinking it still had to manage it as well..

diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c
index 401580cdf9..24355a23e4 100644
--- a/src/intel/vulkan/anv_gem.c
+++ b/src/intel/vulkan/anv_gem.c
@@ -50,6 +50,7 @@ anv_gem_create(struct anv_device *device, uint64_t size)
       return 0;
    }
 
+   anv_gem_set_domain(device, gem_create.handle, I915_GEM_DOMAIN_GTT, 0);
    return gem_create.handle;
 }
 

aligns the cache domain in the kernel with the usage bxt and makes crucible happy. It is not CPU relocs. Will take some time to figure out what disagreement is causing the confusion, e.g. one of the changes is that the kernel does its clflushing asynchronously and given that vulkan doesn't want/need the kernel to clflush at all might lead to some data conflict. But that's a long shot.

It's the use of ASYNC which tells the kernel to skip its implict sync of the CPU cache, which vulkan is relying upon by accident to flush the CPU cache of render targets.

(In reply to Chris Wilson from comment #3)
> It's the use of ASYNC which tells the kernel to skip its implict sync of the
> CPU cache, which vulkan is relying upon by accident to flush the CPU cache
> of render targets.

That makes some sense.  It could also explain the hang issue that someone reported with the Sascha multithreading demo.

That said, what do you mean by "flushing the CPU cache of render targets"?  Why does the CPU cache need flushing?

Any new bo returned to userspace is zeroed. That data may entirely reside in the CPU cache, and may remain there across the GPU write into main memory. The clflush performed before the read to invalidate the cache may in fact just cause the writeback of the cache to main memory, overwriting the results.

In that case, by telling the kernel we want it in the GTT domain, it will do the clflush and then always treat it as uncached.  (In particular, any relocations will be either done through a GTT map or clflushed.)  That seems like a perfectly reasonable thing for anv to do.  I'm not entirely sure where we want to put the SET_DOMAIN call but gem_create is probably ok but maybe anv_bo_init_new would be better.

Created attachment 132507 [details]
Conversation with Chris

After a lengthy IRC conversation with Chris, we determined that this is actually a kernel bug with a fairly simple fix.

Jason mentioned that is relate to bug 100932, so listing all GEN8+ SOCs as affected and raising importance (issue related to GPU hangs in many tests and rendering errors in all Vulkan programs cannot be medium/normal).

Can we get a kernel owner for this bug?

Chris has a patch:

https://patchwork.freedesktop.org/patch/166536/

Tested Chris' patch with drm-tip and Mesa git. It fixes rendering on all SoC platforms: BXT, BSW, BYT.

I didn't see any GPU hangs anymore either (see bug 100932), but those may have been fixed already yesterday by something else.

(On BYT, Vulkan tests can still randomly get device lost errors, but at least they  render now correctly.)

commit 686348c782d3247e3bf39fce98d4ac44e147a746
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Jul 21 15:50:37 2017 +0100

    drm/i915: Force CPU synchronisation even if userspace requests ASYNC
    
    The goal here was to minimise doing any thing or any check inside the
    kernel that was not strictly required. For a userspace that assumes
    complete control over the cache domains, the kernel is usually using
    outdated information and may trigger clflushes where none were
    required.
    
    However, swapping is a situation where userspace has no knowledge of the
    domain transfer, and will leave the object in the CPU cache. The kernel
    must flush this out to the backing storage prior to use with the GPU. As
    we use an asynchronous task tracked by an implicit fence for this, we
    also need to cancel the ASYNC flag on the object so that the object will
    wait for the clflush to complete before being executed. This also absolves
    userspace of the responsibility imposed by commit 77ae9957897d ("drm/i915:
    Enable userspace to opt-out of implicit fencing") that its needed to ensure
    that the object was out of the CPU cache prior to use on the GPU.
    
    Fixes: 77ae9957897d ("drm/i915: Enable userspace to opt-out of implicit fencing")
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101571
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
    Cc: Jason Ekstrand <jason@jlekstrand.net>
    Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
    Link: https://patchwork.freedesktop.org/patch/msgid/20170721145037.25105-5-chris@chris-wilson.co.uk
    Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>