On Thu, Aug 18, 2016 at 04:59:35PM +0300, Mika Kuoppala wrote: > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes: > > > If userspace is asynchronously streaming into the batch or other > > execobjects, we may not flush those writes along with a change in cache > > domain (as there is no change). Therefore those writes may end up in > > internal chipset buffers and not visible to the GPU upon execution. We > > must issue a flush command or otherwise we encounter incoherency in the > > batchbuffers and the GPU executing invalid commands (i.e. hanging) quite > > regularly. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90841 > > Fixes: 1816f9236303 ("drm/i915: Support creation of unbound wc user...") > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Akash Goel <akash.goel@xxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> > > Tested-by: Matti Hämäläinen <ccr@xxxxxxxx> > > Cc: stable@xxxxxxxxxxxxxxx > > --- > > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 +++---------- > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > index 699315304748..bce587abc601 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > > @@ -1015,8 +1015,6 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req, > > { > > const unsigned int other_rings = eb_other_engines(req); > > struct i915_vma *vma; > > - uint32_t flush_domains = 0; > > - bool flush_chipset = false; > > int ret; > > > > list_for_each_entry(vma, vmas, exec_list) { > > @@ -1029,16 +1027,11 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req, > > } > > > > if (obj->base.write_domain & I915_GEM_DOMAIN_CPU) > > - flush_chipset |= i915_gem_clflush_object(obj, false); > > - > > - flush_domains |= obj->base.write_domain; > > + i915_gem_clflush_object(obj, false); > > } > > > > - if (flush_chipset) > > - i915_gem_chipset_flush(req->engine->i915); > > - > > - if (flush_domains & I915_GEM_DOMAIN_GTT) > > - wmb(); > > Was a bit worried about this vanishing. > > But after chatting with Chris and also founding this: > https://lwn.net/Articles/174655/ > [in I386 AND X86_64 SPECIFIC NOTES] > > I am convinced that the uncached write to force the chipset_flush > will be strong barrier enough. > > Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> Fwiw, the v2 that was meant to be here puts the wmb() back into i915_gem_chipset_flush() so that we always have at least an sfence. It should be redundant, but it shouldn't be noticeable either. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
