Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 22, 2017 at 09:06:28PM +0000, De Marchi, Lucas wrote:
> On Fri, 2017-12-22 at 21:58 +0200, Ville Syrjälä wrote:
> > On Mon, Dec 04, 2017 at 03:22:10PM -0800, Lucas De Marchi wrote:
> > > Display WA #1183 was recently added to workaround
> > > "Failures when enabling DPLL0 with eDP link rate 2.16
> > > or 4.32 GHz and CD clock frequency 308.57 or 617.14 MHz
> > > (CDCLK_CTL CD Frequency Select 10b or 11b) used in this
> > >  enabling or in previous enabling."
> > > 
> > > This workaround was designed to minimize the impact only
> > > to save the bad case with that link rates. But HW engineers
> > > indicated that it should be safe to apply broadly, although
> > > they were expecting the DPLL0 link rate to be unchanged on
> > > runtime.
> > > 
> > > We need to cover 2 cases: when we are in fact enabling DPLL0
> > > and when we are just changing the frequency with small
> > > differences.
> > > 
> > > This is based on previous patch by Rodrigo Vivi with suggestions
> > > from Ville Syrjälä.
> > > 
> > > Cc: Arthur J Runyan <arthur.j.runyan@xxxxxxxxx>
> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > 
> > Hmm. This is v4, but it was posted after v5?
> > 
> > You didn't include a changelog in the commit message so I have no idea
> > what is the difference between these versions. They appear similar on a
> > first glance.
> 
> Sorry about the confusion. What happened is that first I was using the version
> number accounting for the initial version Rodrigo sent. However, later I ended
> up using the version from patchwork, which don't take Rodrigo's initial
> version into account, even if the subject is the same.
> 
> The diff is only the CC to stable with additional commits to cherry-pick on
> stable versions as requested by Jani.
> 
> Lucas De Marchi

Pushed to dinq. First patch to 4.17 :)

Thanks for patches, reviews, confirmations, etc!

> 
> > The changes here *look* correct to me, so
> > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > 
> > But I'm not going to push this right now because I'm too confused about
> > the version of the patch. I'll be out for a few weeks so if the
> > confusion is lifted during that time and you want this in please poke
> > at someone else to push it.
> > 
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h         |  2 ++
> > >  drivers/gpu/drm/i915/intel_cdclk.c      | 35 ++++++++++++++++++++++++--
> > > -------
> > >  drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++++++++
> > >  3 files changed, 38 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index 09bf043c1c2e..73335e709ed6 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -7021,6 +7021,7 @@ enum {
> > >  #define  RESET_PCH_HANDSHAKE_ENABLE	(1<<4)
> > >  
> > >  #define GEN8_CHICKEN_DCPR_1		_MMIO(0x46430)
> > > +#define   SKL_SELECT_ALTERNATE_DC_EXIT	(1<<30)
> > >  #define   MASK_WAKEMEM			(1<<13)
> > >  
> > >  #define SKL_DFSM			_MMIO(0x51000)
> > > @@ -8575,6 +8576,7 @@ enum skl_power_gate {
> > >  #define  BXT_CDCLK_CD2X_DIV_SEL_2	(2<<22)
> > >  #define  BXT_CDCLK_CD2X_DIV_SEL_4	(3<<22)
> > >  #define  BXT_CDCLK_CD2X_PIPE(pipe)	((pipe)<<20)
> > > +#define  CDCLK_DIVMUX_CD_OVERRIDE	(1<<19)
> > >  #define  BXT_CDCLK_CD2X_PIPE_NONE	BXT_CDCLK_CD2X_PIPE(3)
> > >  #define  BXT_CDCLK_SSA_PRECHARGE_ENABLE	(1<<16)
> > >  #define  CDCLK_FREQ_DECIMAL_MASK	(0x7ff)
> > > diff --git a/drivers/gpu/drm/i915/intel_cdclk.c
> > > b/drivers/gpu/drm/i915/intel_cdclk.c
> > > index 9c5ceb98d48f..d77e2bec1e29 100644
> > > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > > @@ -931,16 +931,10 @@ static void skl_set_preferred_cdclk_vco(struct
> > > drm_i915_private *dev_priv,
> > >  
> > >  static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco)
> > >  {
> > > -	int min_cdclk = skl_calc_cdclk(0, vco);
> > >  	u32 val;
> > >  
> > >  	WARN_ON(vco != 8100000 && vco != 8640000);
> > >  
> > > -	/* select the minimum CDCLK before enabling DPLL 0 */
> > > -	val = CDCLK_FREQ_337_308 | skl_cdclk_decimal(min_cdclk);
> > > -	I915_WRITE(CDCLK_CTL, val);
> > > -	POSTING_READ(CDCLK_CTL);
> > > -
> > >  	/*
> > >  	 * We always enable DPLL0 with the lowest link rate possible, but
> > > still
> > >  	 * taking into account the VCO required to operate the eDP panel
> > > at the
> > > @@ -994,7 +988,7 @@ static void skl_set_cdclk(struct drm_i915_private
> > > *dev_priv,
> > >  {
> > >  	int cdclk = cdclk_state->cdclk;
> > >  	int vco = cdclk_state->vco;
> > > -	u32 freq_select;
> > > +	u32 freq_select, cdclk_ctl;
> > >  	int ret;
> > >  
> > >  	mutex_lock(&dev_priv->pcu_lock);
> > > @@ -1009,7 +1003,7 @@ static void skl_set_cdclk(struct drm_i915_private
> > > *dev_priv,
> > >  		return;
> > >  	}
> > >  
> > > -	/* set CDCLK_CTL */
> > > +	/* Choose frequency for this cdclk */
> > >  	switch (cdclk) {
> > >  	default:
> > >  		WARN_ON(cdclk != dev_priv->cdclk.hw.ref);
> > > @@ -1036,10 +1030,33 @@ static void skl_set_cdclk(struct drm_i915_private
> > > *dev_priv,
> > >  	    dev_priv->cdclk.hw.vco != vco)
> > >  		skl_dpll0_disable(dev_priv);
> > >  
> > > +	cdclk_ctl = I915_READ(CDCLK_CTL);
> > > +
> > > +	if (dev_priv->cdclk.hw.vco != vco) {
> > > +		/* Wa Display #1183: skl,kbl,cfl */
> > > +		cdclk_ctl &= ~(CDCLK_FREQ_SEL_MASK |
> > > CDCLK_FREQ_DECIMAL_MASK);
> > > +		cdclk_ctl |= freq_select | skl_cdclk_decimal(cdclk);
> > > +		I915_WRITE(CDCLK_CTL, cdclk_ctl);
> > > +	}
> > > +
> > > +	/* Wa Display #1183: skl,kbl,cfl */
> > > +	cdclk_ctl |= CDCLK_DIVMUX_CD_OVERRIDE;
> > > +	I915_WRITE(CDCLK_CTL, cdclk_ctl);
> > > +	POSTING_READ(CDCLK_CTL);
> > > +
> > >  	if (dev_priv->cdclk.hw.vco != vco)
> > >  		skl_dpll0_enable(dev_priv, vco);
> > >  
> > > -	I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk));
> > > +	/* Wa Display #1183: skl,kbl,cfl */
> > > +	cdclk_ctl &= ~(CDCLK_FREQ_SEL_MASK | CDCLK_FREQ_DECIMAL_MASK);
> > > +	I915_WRITE(CDCLK_CTL, cdclk_ctl);
> > > +
> > > +	cdclk_ctl |= freq_select | skl_cdclk_decimal(cdclk);
> > > +	I915_WRITE(CDCLK_CTL, cdclk_ctl);
> > > +
> > > +	/* Wa Display #1183: skl,kbl,cfl */
> > > +	cdclk_ctl &= ~CDCLK_DIVMUX_CD_OVERRIDE;
> > > +	I915_WRITE(CDCLK_CTL, cdclk_ctl);
> > >  	POSTING_READ(CDCLK_CTL);
> > >  
> > >  	/* inform PCU of the change */
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 8315499452dc..35796fa8e6b4 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -598,6 +598,11 @@ void gen9_enable_dc5(struct drm_i915_private
> > > *dev_priv)
> > >  
> > >  	DRM_DEBUG_KMS("Enabling DC5\n");
> > >  
> > > +	/* Wa Display #1183: skl,kbl,cfl */
> > > +	if (IS_GEN9_BC(dev_priv))
> > > +		I915_WRITE(GEN8_CHICKEN_DCPR_1,
> > > I915_READ(GEN8_CHICKEN_DCPR_1) |
> > > +			   SKL_SELECT_ALTERNATE_DC_EXIT);
> > > +
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_EN_UPTO_DC5);
> > >  }
> > >  
> > > @@ -625,6 +630,11 @@ void skl_disable_dc6(struct drm_i915_private
> > > *dev_priv)
> > >  {
> > >  	DRM_DEBUG_KMS("Disabling DC6\n");
> > >  
> > > +	/* Wa Display #1183: skl,kbl,cfl */
> > > +	if (IS_GEN9_BC(dev_priv))
> > > +		I915_WRITE(GEN8_CHICKEN_DCPR_1,
> > > I915_READ(GEN8_CHICKEN_DCPR_1) |
> > > +			   SKL_SELECT_ALTERNATE_DC_EXIT);
> > > +
> > >  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> > >  }
> > >  
> > > -- 
> > > 2.14.3
> > 
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux