[Intel-gfx] [PATCH] drm/i915: Return IDI Hash Mask setup for gen9+

[Ville Syrjälä]

On Thu, Sep 14, 2017 at 12:18:41PM -0700, Rodrigo Vivi wrote:
> Spec teels:
> "It is required for GFX Driver to set [19:16] to 1 when
> eDRAM configuration is enabled."
> 
> This basically reverts commit 666fbcf5c21d ("drm/i915: Don't
> program eLLC IDI hash mask for gen9+")
> 
> This requirement hasn't changed since HSW up to CNL.
> But that commit was created before EDRAM was properly
> organized with commit 3accaf7e734d ("drm/i915: Store
> and use edram capabilities")
> 
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Matthew Auld <matthew.auld at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index f445587c1a4b..25b59dbb29b0 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4714,7 +4714,7 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv)
>  	/* Double layer security blanket, see i915_gem_init() */
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>  
> -	if (HAS_EDRAM(dev_priv) && INTEL_GEN(dev_priv) < 9)
> +	if (HAS_EDRAM(dev_priv))
>  		I915_WRITE(HSW_IDICR, I915_READ(HSW_IDICR) | IDIHASHMSK(0xf));

Hmm. For me the spec seems to be saying
"Note: It is required for GFX Driver to set [19:16] to 1 when eDRAM
 configuration is enabled.
 For Skylake, S/W is not needed to program this register as eDRAM is a
 memory side cache."

But for CNL I only see the first sentence. Does that mean they moved
the eLLC back to the ring side of the system agent, or just someone
forgot to update this for CNL? The fact that eLLC moved to the memory
side of the system agent on SKL isn't really mentioned anywhere else
in the spec either AFAICS. Seems like a rather big elephant to overlook.
Might be nice to get some clarification into the spec for this stuff.

Oh and I wonder what we should be doing with bits [21:20]?

>  
>  	if (IS_HASWELL(dev_priv))
> -- 
> 2.13.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC