On Tue, Jul 30, 2013 at 06:36:15PM +0100, Chris Wilson wrote:
> On Tue, Jul 30, 2013 at 08:19:28PM +0300, Ville Syrjälä wrote:
> > On Tue, Jul 30, 2013 at 05:58:36PM +0100, Chris Wilson wrote:
> > > Haswell GT3e has the unique feature of supporting Write-Through cacheing
> > > of objects within the eLLC. The purpose of this is to enable the display
> > > plane to remain coherent whilst objects lie resident in the eLLC - so
> > > that we in theory get the best of both worlds, perfect display and fast
> > > access.
> >
> > The description here talks about eLLC only, but you set the PTE for
> > WT in LLC/eLLC both.
>
> s/eLLC/LLC/
>
> For some reason, I keep telling myself that it is a magic property of
> the eLLC otherwise why wouldn't they do it for all LLC!
>
> > > - ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE);
> > > + ret = i915_gem_object_set_cache_level(obj,
> > > + HAS_WT(obj->base.dev) ?
> > > I915_CACHE_WT : I915_CACHE_NONE);
> >
> > Don't we need to tweak the write domain like we do for UC to make sure
> > already dirty lines get flushed from caches?
>
> You need to explicitly do the flush, which gets ugly.
Ah right, because i915_gem_clflush_object() checks the cache_level.
> I choose to ignore
> the problem as unlike the LLC -> UC transition, I haven't spotted any
> dirt. However, it doesn't look too bad if we replace it like so:
Hmm, and what about CPU access in general? CPU doesn't know about the
WT policy, so I'm assuming we'd need to flush after all CPU writes.
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ac1b9cd..0e089e2 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3362,27 +3362,32 @@ int i915_gem_object_set_cache_level(struct
> drm_i915_gem_object *obj,
> i915_gem_obj_ggtt_set_color(obj, cache_level);
> }
>
> - if (cache_level == I915_CACHE_NONE) {
> - u32 old_read_domains, old_write_domain;
> -
> + if (cache_level == I915_CACHE_NONE ||
> + cache_level == I915_CACHE_WT) {
> /* If we're coming from LLC cached, then we haven't
> * actually been tracking whether the data is in the
> * CPU cache or not, since we only allow one bit set
> * in obj->write_domain and have been skipping the clflushes.
> - * Just set it to the CPU cache for now.
> + * Do them now.
> */
> - WARN_ON(obj->base.write_domain & ~I915_GEM_DOMAIN_CPU);
> - WARN_ON(obj->base.read_domains & ~I915_GEM_DOMAIN_CPU);
> + if (obj->base.read_domains & I915_GEM_DOMAIN_CPU) {
> + u32 old_read_domains, old_write_domain;
>
> - old_read_domains = obj->base.read_domains;
> - old_write_domain = obj->base.write_domain;
> + BUG_ON(obj->pages == NULL);
>
> - obj->base.read_domains = I915_GEM_DOMAIN_CPU;
> - obj->base.write_domain = I915_GEM_DOMAIN_CPU;
> + trace_i915_gem_object_clflush(obj);
> + drm_clflush_sg(obj->pages);
>
> - trace_i915_gem_object_change_domain(obj,
> - old_read_domains,
> - old_write_domain);
> + old_read_domains = obj->base.read_domains;
> + old_write_domain = obj->base.write_domain;
> +
> + obj->base.read_domains &= ~I915_GEM_DOMAIN_CPU;
> + obj->base.write_domain &= ~I915_GEM_DOMAIN_CPU;
> +
> + trace_i915_gem_object_change_domain(obj,
> + old_read_domains,
> + old_write_domain);
> + }
> }
>
> obj->cache_level = cache_level;
>
> --
> Chris Wilson, Intel Open Source Technology Centre
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
