| Message ID | 20171020221549.5781-1-rodrigo.vivi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Oct 20, 2017 at 03:15:49PM -0700, Rodrigo Vivi wrote: > Wa Display #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. > > So, we could only apply if vco is 8640000. However this > would require a synchronization with intel_csr.c. Probably > a dev_priv->wa1183 bool. > > Another equaly ugly possibility would be to save the edp_link_rate > on dev_priv. With this we could fix one corner case that > although rare I believe our code could hit that is if > desired port clock is 4.32GHz our dpll0_enable initially > sets the link rate to DPLL_CTRL1_LINK_RATE_1080 while > it should set to DPLL_CTRL1_LINK_RATE_2160. > > v2: - Avoid RMW for every step since we know exactly what > we should set. This also fix a bug on v1. > - Remove set of minimal CDCLK since this is not needed > in the WA. At least one less write to CDCLK_CTL > since the other ones cannot be grouped. > - Fix commit message. > > Cc: Arthur J Runyan <arthur.j.runyan@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 2 ++ > drivers/gpu/drm/i915/intel_cdclk.c | 37 ++++++++++++++++++++++----------- > drivers/gpu/drm/i915/intel_runtime_pm.c | 10 +++++++++ > 3 files changed, 37 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 68a58cce6ab1..816b9665e092 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -6980,6 +6980,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) > @@ -8526,6 +8527,7 @@ enum skl_power_gate { > #define BXT_CDCLK_CD2X_DIV_SEL_4 (3<<22) > #define BXT_CDCLK_CD2X_PIPE(pipe) ((pipe)<<20) > #define BXT_CDCLK_CD2X_PIPE_NONE BXT_CDCLK_CD2X_PIPE(3) > +#define DIVMUX_CD_OVERRIDE (1<<19) > #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 b2a6d62b71c0..5ecd5cb44e43 100644 > --- a/drivers/gpu/drm/i915/intel_cdclk.c > +++ b/drivers/gpu/drm/i915/intel_cdclk.c > @@ -858,18 +858,13 @@ static void skl_set_preferred_cdclk_vco(struct drm_i915_private *dev_priv, > intel_update_max_cdclk(dev_priv); > } > > -static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) > +static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco, > + u32 freq_select) > { > - 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 > @@ -894,6 +889,10 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) > I915_WRITE(DPLL_CTRL1, val); > POSTING_READ(DPLL_CTRL1); > > + /* Wa Display #1183: skl,kbl,cfl */ > + val = DIVMUX_CD_OVERRIDE; > + I915_WRITE(CDCLK_CTL, val); > + > I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE); > > if (intel_wait_for_register(dev_priv, > @@ -901,6 +900,17 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) > 5)) > DRM_ERROR("DPLL0 not locked\n"); > > + /* Wa Display #1183: skl,kbl,cfl */ > + val &= ~CDCLK_FREQ_SEL_MASK;; > + I915_WRITE(CDCLK_CTL, val); > + > + val |= freq_select; > + I915_WRITE(CDCLK_CTL, val); > + > + /* Wa Display #1183: skl,kbl,cfl */ > + val &= ~DIVMUX_CD_OVERRIDE; > + I915_WRITE(CDCLK_CTL, val); If I understood the w/a correctly we should pull all of this outa into skl_set_cdclk(). Otherwise we'd only do these gymnastics when changing the VCO between 8640 and 8100, whereas IIRC the w/a said that we should do it every time VCO=8640 is going to be used, or maybe even was used previously. > + > dev_priv->cdclk.hw.vco = vco; > > /* We'll want to keep using the current vco from now on. */ > @@ -964,15 +974,18 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, > break; > } > > + freq_select |= skl_cdclk_decimal(cdclk); > + So I'm thinking here we should first set divmux_override=1, after which we'd set freq_select=0. > if (dev_priv->cdclk.hw.vco != 0 && > dev_priv->cdclk.hw.vco != vco) > skl_dpll0_disable(dev_priv); > > - if (dev_priv->cdclk.hw.vco != vco) > - skl_dpll0_enable(dev_priv, vco); > - > - I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk)); > - POSTING_READ(CDCLK_CTL); > + if (dev_priv->cdclk.hw.vco != vco) { > + skl_dpll0_enable(dev_priv, vco, freq_select); > + } else { > + I915_WRITE(CDCLK_CTL, freq_select); > + POSTING_READ(CDCLK_CTL); > + } And here we'd first set the final freq_select here, and finally set divmux_override=0. > > /* inform PCU of the change */ > mutex_lock(&dev_priv->pcu_lock); > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 8af286c63d3b..e0bc2debdad0 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.13.5
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 68a58cce6ab1..816b9665e092 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -6980,6 +6980,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) @@ -8526,6 +8527,7 @@ enum skl_power_gate { #define BXT_CDCLK_CD2X_DIV_SEL_4 (3<<22) #define BXT_CDCLK_CD2X_PIPE(pipe) ((pipe)<<20) #define BXT_CDCLK_CD2X_PIPE_NONE BXT_CDCLK_CD2X_PIPE(3) +#define DIVMUX_CD_OVERRIDE (1<<19) #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 b2a6d62b71c0..5ecd5cb44e43 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -858,18 +858,13 @@ static void skl_set_preferred_cdclk_vco(struct drm_i915_private *dev_priv, intel_update_max_cdclk(dev_priv); } -static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) +static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco, + u32 freq_select) { - 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 @@ -894,6 +889,10 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) I915_WRITE(DPLL_CTRL1, val); POSTING_READ(DPLL_CTRL1); + /* Wa Display #1183: skl,kbl,cfl */ + val = DIVMUX_CD_OVERRIDE; + I915_WRITE(CDCLK_CTL, val); + I915_WRITE(LCPLL1_CTL, I915_READ(LCPLL1_CTL) | LCPLL_PLL_ENABLE); if (intel_wait_for_register(dev_priv, @@ -901,6 +900,17 @@ static void skl_dpll0_enable(struct drm_i915_private *dev_priv, int vco) 5)) DRM_ERROR("DPLL0 not locked\n"); + /* Wa Display #1183: skl,kbl,cfl */ + val &= ~CDCLK_FREQ_SEL_MASK;; + I915_WRITE(CDCLK_CTL, val); + + val |= freq_select; + I915_WRITE(CDCLK_CTL, val); + + /* Wa Display #1183: skl,kbl,cfl */ + val &= ~DIVMUX_CD_OVERRIDE; + I915_WRITE(CDCLK_CTL, val); + dev_priv->cdclk.hw.vco = vco; /* We'll want to keep using the current vco from now on. */ @@ -964,15 +974,18 @@ static void skl_set_cdclk(struct drm_i915_private *dev_priv, break; } + freq_select |= skl_cdclk_decimal(cdclk); + if (dev_priv->cdclk.hw.vco != 0 && dev_priv->cdclk.hw.vco != vco) skl_dpll0_disable(dev_priv); - if (dev_priv->cdclk.hw.vco != vco) - skl_dpll0_enable(dev_priv, vco); - - I915_WRITE(CDCLK_CTL, freq_select | skl_cdclk_decimal(cdclk)); - POSTING_READ(CDCLK_CTL); + if (dev_priv->cdclk.hw.vco != vco) { + skl_dpll0_enable(dev_priv, vco, freq_select); + } else { + I915_WRITE(CDCLK_CTL, freq_select); + POSTING_READ(CDCLK_CTL); + } /* inform PCU of the change */ mutex_lock(&dev_priv->pcu_lock); diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 8af286c63d3b..e0bc2debdad0 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); }
Wa Display #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. So, we could only apply if vco is 8640000. However this would require a synchronization with intel_csr.c. Probably a dev_priv->wa1183 bool. Another equaly ugly possibility would be to save the edp_link_rate on dev_priv. With this we could fix one corner case that although rare I believe our code could hit that is if desired port clock is 4.32GHz our dpll0_enable initially sets the link rate to DPLL_CTRL1_LINK_RATE_1080 while it should set to DPLL_CTRL1_LINK_RATE_2160. v2: - Avoid RMW for every step since we know exactly what we should set. This also fix a bug on v1. - Remove set of minimal CDCLK since this is not needed in the WA. At least one less write to CDCLK_CTL since the other ones cannot be grouped. - Fix commit message. Cc: Arthur J Runyan <arthur.j.runyan@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 2 ++ drivers/gpu/drm/i915/intel_cdclk.c | 37 ++++++++++++++++++++++----------- drivers/gpu/drm/i915/intel_runtime_pm.c | 10 +++++++++ 3 files changed, 37 insertions(+), 12 deletions(-)