All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
@ 2017-10-17 17:38 Rodrigo Vivi
  2017-10-17 18:03 ` ✗ Fi.CI.BAT: warning for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2017-10-17 17:38 UTC (permalink / raw)
  To: intel-gfx; +Cc: Arthur J Runyan, Rodrigo Vivi

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."

However our code is already not following the
"Skylake Seqyences to Initialize Display" line by line
so it was really hard to map this workaround there.

The biggest difference is that Spec sequence expect that
by the time we are enabling DPLL0 we already know the
eDP link rate. What it is not true for us. We handle eDP
link rate as any other DP link rate at the modeset. With
only one small difference that we check a VCO when that
is available.

WARN: It seems that DPLL0 link rate was not designed to
change on the fly and we will probably need to find a more
robuts solution caching the eDP link rate somehow.

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.

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      | 33 +++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++++++++
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5f99d4d6291b..446f4b6fade1 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)
@@ -8525,6 +8526,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..e193912b21ec 100644
--- a/drivers/gpu/drm/i915/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/intel_cdclk.c
@@ -858,7 +858,8 @@ 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;
@@ -894,6 +895,11 @@ 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 = I915_READ(CDCLK_CTL);
+	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 +907,18 @@ 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 = I915_READ(CDCLK_CTL);
+	val &= ~CDCLK_FREQ_SEL_MASK;;
+	I915_WRITE(CDCLK_CTL, val);
+
+	I915_WRITE(CDCLK_CTL, freq_select);
+	POSTING_READ(CDCLK_CTL);
+
+	val = I915_READ(CDCLK_CTL);
+	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 +982,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);
 }
 
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.BAT: warning for drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
  2017-10-17 17:38 [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl Rodrigo Vivi
@ 2017-10-17 18:03 ` Patchwork
  2017-10-17 19:25 ` [PATCH] " Ville Syrjälä
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-17 18:03 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
URL   : https://patchwork.freedesktop.org/series/32149/
State : warning

== Summary ==

Series 32149v1 drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
https://patchwork.freedesktop.org/api/1.0/series/32149/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> PASS       (fi-kbl-7500u) fdo#103285
Test gem_exec_reloc:
        Subgroup basic-cpu-active:
                fail       -> PASS       (fi-gdg-551) fdo#102582 +3
        Subgroup basic-write-gtt-active:
                fail       -> PASS       (fi-gdg-551)
Test kms_cursor_legacy:
        Subgroup basic-flip-after-cursor-legacy:
                pass       -> INCOMPLETE (fi-bxt-j4205) fdo#102035
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-edid:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup force-load-detect:
                pass       -> SKIP       (fi-ivb-3520m)
        Subgroup prune-stale-modes:
                pass       -> SKIP       (fi-ivb-3520m)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#103285 https://bugs.freedesktop.org/show_bug.cgi?id=103285
fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582
fdo#102035 https://bugs.freedesktop.org/show_bug.cgi?id=102035

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:436s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:372s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:527s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:262s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:493s
fi-bxt-j4205     total:212  pass:190  dwarn:0   dfail:0   fail:0   skip:21 
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:496s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:486s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:556s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:415s
fi-gdg-551       total:289  pass:177  dwarn:1   dfail:0   fail:2   skip:109 time:249s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:576s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:436s
fi-ivb-3520m     total:289  pass:256  dwarn:0   dfail:0   fail:0   skip:33  time:478s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:458s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:569s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:576s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:546s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:450s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:647s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:518s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:497s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:456s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:573s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:415s
fi-bdw-gvtdvm failed to connect after reboot

79104a2f00df1f402e8dad934d6a574f50a1b669 drm-tip: 2017y-10m-17d-15h-35m-59s UTC integration manifest
dc3ee3f255c1 drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6079/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
  2017-10-17 17:38 [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl Rodrigo Vivi
  2017-10-17 18:03 ` ✗ Fi.CI.BAT: warning for " Patchwork
@ 2017-10-17 19:25 ` Ville Syrjälä
  2017-10-17 23:33   ` Rodrigo Vivi
                     ` (2 more replies)
  2017-10-20 22:37 ` ✓ Fi.CI.BAT: success for drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2) Patchwork
  2017-10-21  0:09 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 3 replies; 9+ messages in thread
From: Ville Syrjälä @ 2017-10-17 19:25 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Arthur J Runyan

On Tue, Oct 17, 2017 at 10:38:19AM -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."
> 
> However our code is already not following the
> "Skylake Seqyences to Initialize Display" line by line
> so it was really hard to map this workaround there.
> 
> The biggest difference is that Spec sequence expect that
> by the time we are enabling DPLL0 we already know the
> eDP link rate. What it is not true for us. We handle eDP
> link rate as any other DP link rate at the modeset. With
> only one small difference that we check a VCO when that
> is available.
> 
> WARN: It seems that DPLL0 link rate was not designed to
> change on the fly and we will probably need to find a more
> robuts solution caching the eDP link rate somehow.
> 
> 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.

I think the eDP link rate just refers to the VCO freq. So we should
apparently do the w/a only if 8640 MHz is used. But I'd rather do it
always if we can to keep the code simpler.

Our current sequence is pretty much this:
if necessary to disable DPLL0
	disable DPLL0
if necessary to enable DPLL0
	write CDLCK_CTL
		freq_select = 2
		decimal = whatever
	enable DPLL0
write CDCLK_CTL
	freq_select = final
	decimal = final

IIRC the first CDCLK_CTL write was put there just to make sure we start
at the minimum CDCLK when DPLL0 is first enabled. For this w/a we
should apparently just do that write even if DPLL0 is already enabled, 
and change it to select 450/432 MHz instead.

As far as the divmux override goes, I wonder if we can just do this
with two CDCLK_CTL writes, or if we really need four.

So I was thinking we'd just do this:
if necessary to disable DPLL0
	disable DPLL0
write CDLCK_CTL
	divmux_override = 1
	freq_select = 0
	decimal = whatever
if necessary to enable DPLL0
	enable DPLL0
write CDCLK_CTL
	freq_select = final
	decimal = final

Or should we even do the first CDCLK_CTL write before potentially
disabling DPLL0? Not that we would normally do that, but in theory
it can happen if the machine boots with the wrong DPLL0 settings.

And if we really need the four writes then I guess we'd maybe end
up doing something like:
if necessary to disable DPLL0
	disable DPLL0
read-modify-write CDLCK_CTL
	divmux_override = 1
write CDLCK_CTL
	divmux_override = 1
	freq_select = 0
	decimal = whatever
if necessary to enable DPLL0
	enable DPLL0
write CDCLK_CTL
	divmux_override = 1
	freq_select = final
	decimal = final
read-modify-write CDLCK_CTL
	divmux_override = 0

With the DPLL0 disable potentially moved to just before DPLL0 enable.

> 
> 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      | 33 +++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++++++++
>  3 files changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5f99d4d6291b..446f4b6fade1 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)
> @@ -8525,6 +8526,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..e193912b21ec 100644
> --- a/drivers/gpu/drm/i915/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> @@ -858,7 +858,8 @@ 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;
> @@ -894,6 +895,11 @@ 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 = I915_READ(CDCLK_CTL);
> +	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 +907,18 @@ 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 = I915_READ(CDCLK_CTL);
> +	val &= ~CDCLK_FREQ_SEL_MASK;;
> +	I915_WRITE(CDCLK_CTL, val);
> +
> +	I915_WRITE(CDCLK_CTL, freq_select);
> +	POSTING_READ(CDCLK_CTL);
> +
> +	val = I915_READ(CDCLK_CTL);
> +	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 +982,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);
>  }
>  
> -- 
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
  2017-10-17 19:25 ` [PATCH] " Ville Syrjälä
@ 2017-10-17 23:33   ` Rodrigo Vivi
  2017-10-20  0:48   ` Rodrigo Vivi
  2017-10-20 22:15   ` Rodrigo Vivi
  2 siblings, 0 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2017-10-17 23:33 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Arthur J Runyan

On Tue, Oct 17, 2017 at 07:25:46PM +0000, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 10:38:19AM -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."
> > 
> > However our code is already not following the
> > "Skylake Seqyences to Initialize Display" line by line
> > so it was really hard to map this workaround there.
> > 
> > The biggest difference is that Spec sequence expect that
> > by the time we are enabling DPLL0 we already know the
> > eDP link rate. What it is not true for us. We handle eDP
> > link rate as any other DP link rate at the modeset. With
> > only one small difference that we check a VCO when that
> > is available.
> > 
> > WARN: It seems that DPLL0 link rate was not designed to
> > change on the fly and we will probably need to find a more
> > robuts solution caching the eDP link rate somehow.
> > 
> > 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.
> 
> I think the eDP link rate just refers to the VCO freq. So we should
> apparently do the w/a only if 8640 MHz is used. But I'd rather do it
> always if we can to keep the code simpler.

hmmm... this directly map to the vco was one of the things that confused me.

> 
> Our current sequence is pretty much this:
> if necessary to disable DPLL0
> 	disable DPLL0
> if necessary to enable DPLL0
> 	write CDLCK_CTL
> 		freq_select = 2
> 		decimal = whatever
> 	enable DPLL0
> write CDCLK_CTL
> 	freq_select = final
> 	decimal = final

also this step with 2 different cdclk_ctl writes wasn't like I map the spec.

> 
> IIRC the first CDCLK_CTL write was put there just to make sure we start
> at the minimum CDCLK when DPLL0 is first enabled. For this w/a we
> should apparently just do that write even if DPLL0 is already enabled, 
> and change it to select 450/432 MHz instead.
> 
> As far as the divmux override goes, I wonder if we can just do this
> with two CDCLK_CTL writes, or if we really need four.

I hope Art can answer if this block below would be enough:

> 
> So I was thinking we'd just do this:
> if necessary to disable DPLL0
> 	disable DPLL0
> write CDLCK_CTL
> 	divmux_override = 1
> 	freq_select = 0
> 	decimal = whatever
> if necessary to enable DPLL0
> 	enable DPLL0
> write CDCLK_CTL
> 	freq_select = final
> 	decimal = final
> 
> Or should we even do the first CDCLK_CTL write before potentially
> disabling DPLL0? Not that we would normally do that, but in theory
> it can happen if the machine boots with the wrong DPLL0 settings.
> 
> And if we really need the four writes then I guess we'd maybe end
> up doing something like:
> if necessary to disable DPLL0
> 	disable DPLL0
> read-modify-write CDLCK_CTL
> 	divmux_override = 1
> write CDLCK_CTL
> 	divmux_override = 1
> 	freq_select = 0
> 	decimal = whatever
> if necessary to enable DPLL0
> 	enable DPLL0
> write CDCLK_CTL
> 	divmux_override = 1
> 	freq_select = final
> 	decimal = final
> read-modify-write CDLCK_CTL
> 	divmux_override = 0
> 
> With the DPLL0 disable potentially moved to just before DPLL0 enable.

Thanks a lot for all comments here.

> 
> > 
> > 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      | 33 +++++++++++++++++++++++++++------
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++++++++
> >  3 files changed, 39 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 5f99d4d6291b..446f4b6fade1 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)
> > @@ -8525,6 +8526,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..e193912b21ec 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -858,7 +858,8 @@ 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;
> > @@ -894,6 +895,11 @@ 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 = I915_READ(CDCLK_CTL);
> > +	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 +907,18 @@ 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 = I915_READ(CDCLK_CTL);
> > +	val &= ~CDCLK_FREQ_SEL_MASK;;
> > +	I915_WRITE(CDCLK_CTL, val);
> > +
> > +	I915_WRITE(CDCLK_CTL, freq_select);
> > +	POSTING_READ(CDCLK_CTL);
> > +
> > +	val = I915_READ(CDCLK_CTL);
> > +	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 +982,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);
> >  }
> >  
> > -- 
> > 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
  2017-10-17 19:25 ` [PATCH] " Ville Syrjälä
  2017-10-17 23:33   ` Rodrigo Vivi
@ 2017-10-20  0:48   ` Rodrigo Vivi
  2017-10-20 22:15   ` Rodrigo Vivi
  2 siblings, 0 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2017-10-20  0:48 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, Arthur J Runyan

On Tue, Oct 17, 2017 at 07:25:46PM +0000, Ville Syrjälä wrote:
> On Tue, Oct 17, 2017 at 10:38:19AM -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."
> > 
> > However our code is already not following the
> > "Skylake Seqyences to Initialize Display" line by line
> > so it was really hard to map this workaround there.
> > 
> > The biggest difference is that Spec sequence expect that
> > by the time we are enabling DPLL0 we already know the
> > eDP link rate. What it is not true for us. We handle eDP
> > link rate as any other DP link rate at the modeset. With
> > only one small difference that we check a VCO when that
> > is available.
> > 
> > WARN: It seems that DPLL0 link rate was not designed to
> > change on the fly and we will probably need to find a more
> > robuts solution caching the eDP link rate somehow.
> > 
> > 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.
> 
> I think the eDP link rate just refers to the VCO freq. So we should
> apparently do the w/a only if 8640 MHz is used. But I'd rather do it
> always if we can to keep the code simpler.
> 
> Our current sequence is pretty much this:
> if necessary to disable DPLL0
> 	disable DPLL0
> if necessary to enable DPLL0
> 	write CDLCK_CTL
> 		freq_select = 2
> 		decimal = whatever
> 	enable DPLL0
> write CDCLK_CTL
> 	freq_select = final
> 	decimal = final
> 
> IIRC the first CDCLK_CTL write was put there just to make sure we start
> at the minimum CDCLK when DPLL0 is first enabled. For this w/a we
> should apparently just do that write even if DPLL0 is already enabled, 
> and change it to select 450/432 MHz instead.
> 
> As far as the divmux override goes, I wonder if we can just do this
> with two CDCLK_CTL writes, or if we really need four.
> 
> So I was thinking we'd just do this:
> if necessary to disable DPLL0
> 	disable DPLL0
> write CDLCK_CTL
> 	divmux_override = 1
> 	freq_select = 0
> 	decimal = whatever
> if necessary to enable DPLL0
> 	enable DPLL0
> write CDCLK_CTL
> 	freq_select = final
> 	decimal = final

ok, so the code I posted do:
if necessary to disable DPLL0
	disable DPLL0
if necessary to enable DPLL0
   	write CDLCK_CTL
		freq_select = 337_308 (min)
		decimal = decimal(min_cdclk)
   	rm-write CDLCK_CTL
		divmux_override = 1
       	rm-write CDLCK_CTL
		freq_select = 0
	enable DPLL0
	write CDCLK_CTL
		freq_select = final
		decimal = final
	rm-write CDLCK_CTL
		divmux_override = 0

5 writes actually to cdclk_ctl.

and ouch! I just saw we are doing a non rmw on the middle... a v2 is needed anyways.

Art confirmed that we need to interleave the writes...

Maybe I thought that we could avoid the first one that is the
minimun CDCLK before enabling DPLL, it seems there is no impact
on changing that before enabling it.

Also I saw that what I can avoid are the many mmio reads since
we are constructing the right "val" (just carefully to rename it
to avoid mistaking with val usage on ctrl1.

Thoughts?

> 
> Or should we even do the first CDCLK_CTL write before potentially
> disabling DPLL0? Not that we would normally do that, but in theory
> it can happen if the machine boots with the wrong DPLL0 settings.
> 
> And if we really need the four writes then I guess we'd maybe end
> up doing something like:
> if necessary to disable DPLL0
> 	disable DPLL0
> read-modify-write CDLCK_CTL
> 	divmux_override = 1
> write CDLCK_CTL
> 	divmux_override = 1
> 	freq_select = 0
> 	decimal = whatever
> if necessary to enable DPLL0
> 	enable DPLL0
> write CDCLK_CTL
> 	divmux_override = 1
> 	freq_select = final
> 	decimal = final
> read-modify-write CDLCK_CTL
> 	divmux_override = 0
> 
> With the DPLL0 disable potentially moved to just before DPLL0 enable.
> 
> > 
> > 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      | 33 +++++++++++++++++++++++++++------
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 10 ++++++++++
> >  3 files changed, 39 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 5f99d4d6291b..446f4b6fade1 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)
> > @@ -8525,6 +8526,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..e193912b21ec 100644
> > --- a/drivers/gpu/drm/i915/intel_cdclk.c
> > +++ b/drivers/gpu/drm/i915/intel_cdclk.c
> > @@ -858,7 +858,8 @@ 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;
> > @@ -894,6 +895,11 @@ 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 = I915_READ(CDCLK_CTL);
> > +	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 +907,18 @@ 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 = I915_READ(CDCLK_CTL);
> > +	val &= ~CDCLK_FREQ_SEL_MASK;;
> > +	I915_WRITE(CDCLK_CTL, val);
> > +
> > +	I915_WRITE(CDCLK_CTL, freq_select);
> > +	POSTING_READ(CDCLK_CTL);
> > +
> > +	val = I915_READ(CDCLK_CTL);
> > +	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 +982,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);
> >  }
> >  
> > -- 
> > 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
  2017-10-17 19:25 ` [PATCH] " Ville Syrjälä
  2017-10-17 23:33   ` Rodrigo Vivi
  2017-10-20  0:48   ` Rodrigo Vivi
@ 2017-10-20 22:15   ` Rodrigo Vivi
  2017-10-23 13:30     ` Ville Syrjälä
  2 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2017-10-20 22:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Arthur J Runyan, Rodrigo Vivi

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);
+
 	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);
 }
 
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2)
  2017-10-17 17:38 [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl Rodrigo Vivi
  2017-10-17 18:03 ` ✗ Fi.CI.BAT: warning for " Patchwork
  2017-10-17 19:25 ` [PATCH] " Ville Syrjälä
@ 2017-10-20 22:37 ` Patchwork
  2017-10-21  0:09 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-20 22:37 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2)
URL   : https://patchwork.freedesktop.org/series/32149/
State : success

== Summary ==

Series 32149v2 drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
https://patchwork.freedesktop.org/api/1.0/series/32149/revisions/2/mbox/

Test chamelium:
        Subgroup dp-hpd-fast:
                incomplete -> SKIP       (fi-bdw-gvtdvm) fdo#102332
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> INCOMPLETE (fi-skl-6700hq) fdo#102035
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#101705

fdo#102332 https://bugs.freedesktop.org/show_bug.cgi?id=102332
fdo#102035 https://bugs.freedesktop.org/show_bug.cgi?id=102035
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:444s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:371s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:522s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:263s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:497s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:495s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:491s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:477s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:556s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:417s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:256s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:580s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:457s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:431s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:432s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:459s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:481s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:569s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:475s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:581s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:535s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-skl-6700hq    total:219  pass:198  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:514s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:457s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:559s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:418s

0760516f31272c1c7cbdb6e415b4f367d3681d8e drm-tip: 2017y-10m-20d-18h-40m-36s UTC integration manifest
379b7d48c37f drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6132/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2)
  2017-10-17 17:38 [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2017-10-20 22:37 ` ✓ Fi.CI.BAT: success for drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2) Patchwork
@ 2017-10-21  0:09 ` Patchwork
  3 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2017-10-21  0:09 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2)
URL   : https://patchwork.freedesktop.org/series/32149/
State : success

== Summary ==

Test kms_flip:
        Subgroup modeset-vs-vblank-race-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#103060
Test kms_busy:
        Subgroup extended-modeset-hang-newfb-with-reset-render-B:
                dmesg-warn -> PASS       (shard-hsw) fdo#103038

fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#103038 https://bugs.freedesktop.org/show_bug.cgi?id=103038

shard-hsw        total:2540 pass:1429 dwarn:1   dfail:0   fail:9   skip:1101 time:9226s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6132/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl.
  2017-10-20 22:15   ` Rodrigo Vivi
@ 2017-10-23 13:30     ` Ville Syrjälä
  0 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2017-10-23 13:30 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx, Arthur J Runyan

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-10-23 13:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-17 17:38 [PATCH] drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl Rodrigo Vivi
2017-10-17 18:03 ` ✗ Fi.CI.BAT: warning for " Patchwork
2017-10-17 19:25 ` [PATCH] " Ville Syrjälä
2017-10-17 23:33   ` Rodrigo Vivi
2017-10-20  0:48   ` Rodrigo Vivi
2017-10-20 22:15   ` Rodrigo Vivi
2017-10-23 13:30     ` Ville Syrjälä
2017-10-20 22:37 ` ✓ Fi.CI.BAT: success for drm/i915: Apply Wa Display #1183 on skl, kbl, and cfl. (rev2) Patchwork
2017-10-21  0:09 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.