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@xxxxxxxxx> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > --- > 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 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx