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.run...@intel.com>
> Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@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
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
