On Tue, Oct 09, 2012 at 07:24:40PM +0100, Chris Wilson wrote:
> We need to treat the GPU core as a distinct processor and so apply the
> same SMP memory barriers. In this case, in addition to flushing the
> chipset cache, which is a no-op on LLC platforms, apply a write barrier
> beforehand. And then when we invalidate the CPU cache, make sure the
> memory is coherent (again this was a no-op on LLC platforms).
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
I think this one here deserves some love still:
- the fancy new pwrite/pread code does some crazy coherency tricks behind
the domain tracking code. This patch misses those.
- like Jesse said: comments.
- I'd still wish we'd have some i-g-t tests for this stuff ...
And now my crazy new theory: We've already had some bug reports that
suggested that we're not fully coherent around unbind/rebind and papered
over it with:
commit c501ae7f332cdaf42e31af30b72b4b66cbbb1604
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date: Wed Dec 14 13:57:23 2011 +0100
drm/i915: Only clear the GPU domains upon a successful finish
And now we have the cpu_reloc regression from Dave Airlie which could be
explained with similar rebinding penalties (if we're creative). I hope
somewhat that we could explain these with the lack of proper memory
barriers ... So if you can gather tested-by's with the above duct-tape
reverted and these patches applied, I'd be almost as happy as with some
i-g-t tests for these patches.
Cheers, Daniel
> ---
> drivers/char/agp/intel-gtt.c | 1 +
> drivers/gpu/drm/i915/i915_gem.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 8b0f6d19..1223128 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -1706,6 +1706,7 @@ EXPORT_SYMBOL(intel_gtt_get);
>
> void intel_gtt_chipset_flush(void)
> {
> + wmb();
> if (intel_private.driver->chipset_flush)
> intel_private.driver->chipset_flush();
> }
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ed8d21a..b1ebb88 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3528,6 +3528,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write)
> /* Flush the CPU cache if it's still invalid. */
> if ((obj->base.read_domains & I915_GEM_DOMAIN_CPU) == 0) {
> i915_gem_clflush_object(obj);
> + mb(); /* in case the clflush above is optimised away */
>
> obj->base.read_domains |= I915_GEM_DOMAIN_CPU;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
