Re: [PATCH 4/5] drm/i915/cfl: Introduce Display workarounds for Coffee Lake.

["Vivi, Rodrigo" <rodrigo.vivi@intel.com>]

On Wed, 2017-06-07 at 18:44 +0000, Pandiyan, Dhinakaran wrote:
> On Tue, 2017-06-06 at 12:20 -0700, Rodrigo Vivi wrote:
> > The whole Display engine for Coffee Lake is pretty much
> > identical to the Kabylake. For this reason let's reuse
> > all display related production workardounds here even though
> 
> Are these all the display workarounds we have or is this patch just for
> PM related ones?

All this are Display W/a but needed on pm init... Maybe they would
deserve a better and more clear place in our code like in intel_display
that gets called on the right time... not sure, but anyways for a follow
up patch.

But I just noticed not all of Display W/a are here as well. I need to
move all other intel_pm.c changes from  drm/i915/cfl: Introduce Coffee
Lake workardounds. to this patch :/ and also rename that patch to GT
workarounds.

I hope to still count with your rv-b here.


> 
> > CFL is not explicit listed at Display workarounds page at Spec.

CFL Display is identical to KBL, this is why the spec has no specific
mention about CFL. Only page that is apparently updated is the
configuration SKUs with the IDs.

> > 
> > Cc: Arthur Runyan <arthur.j.runyan@intel.com>
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index aa9d8ce..98aeba9 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3549,7 +3549,7 @@ static bool skl_needs_memory_bw_wa(struct intel_atomic_state *state)
> >  static bool
> >  intel_has_sagv(struct drm_i915_private *dev_priv)
> >  {
> > -	if (IS_KABYLAKE(dev_priv))
> > +	if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> >  		return true;
> >  
> >  	if (IS_SKYLAKE(dev_priv) &&
> > @@ -4459,8 +4459,9 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> >  		  fb->modifier == I915_FORMAT_MOD_Yf_TILED;
> >  	x_tiled = fb->modifier == I915_FORMAT_MOD_X_TILED;
> >  
> > -	/* Display WA #1141: kbl. */
> > -	if (IS_KABYLAKE(dev_priv) && dev_priv->ipc_enabled)
> > +	/* Display WA #1141: kbl,cfl */
> > +	if ((IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv)) &&
> > +	    dev_priv->ipc_enabled)
> 
> I am not sure about this, bspec does not say whether it applies to CFL
> or not. So unless we get more clarification, makes sense to go with
> this.

yep, let's consider all same as KBL. If something changes later we
follow up with updates in the code.

> 
> I have audited all occurrences of KBL special cases in this file,
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> 
> >  		latency += 4;
> >  
> >  	if (apply_memory_bw_wa && x_tiled)
> > @@ -8312,7 +8313,7 @@ static void kabylake_init_clock_gating(struct drm_i915_private *dev_priv)
> >  		I915_WRITE(GEN6_UCGCTL1, I915_READ(GEN6_UCGCTL1) |
> >  			   GEN6_GAMUNIT_CLOCK_GATE_DISABLE);
> >  
> > -	/* WaFbcNukeOnHostModify:kbl */
> > +	/* WaFbcNukeOnHostModify:kbl,cfl */
> >  	I915_WRITE(ILK_DPFC_CHICKEN, I915_READ(ILK_DPFC_CHICKEN) |
> >  		   ILK_DPFC_NUKE_ON_ANY_MODIFICATION);
> >  }
> > @@ -8780,7 +8781,7 @@ void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv)
> >  {
> >  	if (IS_SKYLAKE(dev_priv))
> >  		dev_priv->display.init_clock_gating = skylake_init_clock_gating;
> > -	else if (IS_KABYLAKE(dev_priv))
> > +	else if (IS_KABYLAKE(dev_priv) || IS_COFFEELAKE(dev_priv))
> >  		dev_priv->display.init_clock_gating = kabylake_init_clock_gating;
> >  	else if (IS_BROXTON(dev_priv))
> >  		dev_priv->display.init_clock_gating = bxt_init_clock_gating;
> 

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx