[Intel-gfx] [PATCH 11/21] drm/i915: disable GTT cache for 2M pages

[Ville Syrjälä]

On Mon, Oct 02, 2017 at 03:31:44PM +0300, Joonas Lahtinen wrote:
> On Fri, 2017-09-29 at 17:10 +0100, Matthew Auld wrote:
> > When SW enables the use of 2M/1G pages, it must disable the GTT cache.
> > 
> > v2: don't disable for Cherryview which doesn't even support 48b PPGTT!
> > 
> > v3: explicitly check that the system does support 2M/1G pages
> > 
> > Signed-off-by: Matthew Auld <matthew.auld at intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> > Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> 
> <SNIP>
> 
> > @@ -8483,10 +8483,11 @@ static void bdw_init_clock_gating(struct drm_i915_private *dev_priv)
> >  
> >  	/*
> >  	 * WaGttCachingOffByDefault:bdw
> > -	 * GTT cache may not work with big pages, so if those
> > -	 * are ever enabled GTT cache may need to be disabled.
> > +	 * The GTT cache must be disabled if the system is using 2M/1G pages.
> >  	 */
> > -	I915_WRITE(HSW_GTT_CACHE_EN, GTT_CACHE_EN_ALL);
> > +	I915_WRITE(HSW_GTT_CACHE_EN,
> > +		   HAS_PAGE_SIZES(dev_priv, I915_GTT_PAGE_SIZE_2M) ? 0 :
> > +		   GTT_CACHE_EN_ALL);
> 
> Umm, this is mixing a known W/A with decision logic.
> 
> 	bool can_use_gtt_cache = !HAS_PAGE_SIZES(dev_priv, I915_GTT_PAGE_SIZE_2M);
> 
> 	/* WaGttCachingOffByDefault:bdw */
> 	I915_WRITE(HSW_GTT_CACHE_EN, can_use_gtt_cache ? GTT_CACHE_EN_ALL : 0);
> 
> The big question is that if everyone else has GTT caching enabled by
> default, should not we actively be disabling it on other code paths?

HSW has it enabled by default. BDW B0 disabled it by default, even
though the spec seems to suggest that the big pages vs. GTT cache issue
may have been fixed properly already on BDW. Did anyone actually test
whether big pages work with the GTT cache enabled?

Bspec is a bit of a mess these days so it's kinda hard to see what the
situation is with SKL+, but it looks like the default is still 0x0. So
seems to me that someone should actually explicitly enable GTT cache on
SKL+. Enabling it did prodcuce a slight performance increase on BDW/CHV
IIRC.

-- 
Ville Syrjälä
Intel OTC