Re: [PATCH] drm/i915: Return IDI Hash Mask setup for gen9+

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx