All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
@ 2017-12-04 23:22 Lucas De Marchi
  2017-12-04 23:42 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Lucas De Marchi @ 2017-12-04 23:22 UTC (permalink / raw)
  To: intel-gfx
  Cc: Arthur J Runyan, Ville Syrjälä, Rodrigo Vivi, stable,
	Lucas De Marchi

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@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: stable@vger.kernel.org
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 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

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

* ✓ Fi.CI.BAT: success for drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
  2017-12-04 23:22 [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl Lucas De Marchi
@ 2017-12-04 23:42 ` Patchwork
  2017-12-05  0:27 ` ✓ Fi.CI.IGT: " Patchwork
  2017-12-22 19:58   ` Ville Syrjälä
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-12-04 23:42 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
URL   : https://patchwork.freedesktop.org/series/34873/
State : success

== Summary ==

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

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> FAIL       (fi-skl-6700k) fdo#103191

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:443s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:387s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:516s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:281s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:505s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:491s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:477s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:178  dwarn:1   dfail:0   fail:1   skip:108 time:270s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:537s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:372s
fi-hsw-4770r     total:288  pass:224  dwarn:0   dfail:0   fail:0   skip:64  time:259s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:395s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:480s
fi-ivb-3770      total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:450s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:484s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:532s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:474s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:531s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:453s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:538s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-skl-6700k     total:288  pass:263  dwarn:0   dfail:0   fail:1   skip:24  time:517s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:497s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:447s
fi-snb-2520m     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:549s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:415s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:606s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:641s
fi-glk-dsi       total:288  pass:245  dwarn:1   dfail:4   fail:0   skip:38  time:535s

cee3f135bf4e95188222d7a5ffaeb1e4f0185fc9 drm-tip: 2017y-12m-04d-21h-06m-29s UTC integration manifest
06bdc16eade0 drm/i915: Apply Display WA #1183 on skl, kbl, and cfl

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
  2017-12-04 23:22 [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl Lucas De Marchi
  2017-12-04 23:42 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-12-05  0:27 ` Patchwork
  2017-12-22 19:58   ` Ville Syrjälä
  2 siblings, 0 replies; 8+ messages in thread
From: Patchwork @ 2017-12-05  0:27 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
URL   : https://patchwork.freedesktop.org/series/34873/
State : success

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-offscren-pri-shrfb-draw-render:
                pass       -> FAIL       (shard-snb) fdo#101623 +1
Test gem_tiled_swapping:
        Subgroup non-threaded:
                pass       -> INCOMPLETE (shard-snb) fdo#104009
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

fdo#101623 https://bugs.freedesktop.org/show_bug.cgi?id=101623
fdo#104009 https://bugs.freedesktop.org/show_bug.cgi?id=104009
fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912

shard-hsw        total:2679 pass:1536 dwarn:1   dfail:0   fail:10  skip:1132 time:9464s
shard-snb        total:2663 pass:1297 dwarn:1   dfail:0   fail:12  skip:1352 time:7915s
Blacklisted hosts:
shard-apl        total:2679 pass:1676 dwarn:1   dfail:0   fail:23  skip:978 time:13623s
shard-kbl        total:2679 pass:1795 dwarn:1   dfail:0   fail:23  skip:860 time:10813s

== Logs ==

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

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

* Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
  2017-12-04 23:22 [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl Lucas De Marchi
@ 2017-12-22 19:58   ` Ville Syrjälä
  2017-12-05  0:27 ` ✓ Fi.CI.IGT: " Patchwork
  2017-12-22 19:58   ` Ville Syrjälä
  2 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-12-22 19:58 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Arthur J Runyan, Rodrigo Vivi, stable

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@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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.

The changes here *look* correct to me, so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

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

-- 
Ville Syrjälä
Intel OTC

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

* Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
@ 2017-12-22 19:58   ` Ville Syrjälä
  0 siblings, 0 replies; 8+ messages in thread
From: Ville Syrjälä @ 2017-12-22 19:58 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx, Arthur J Runyan, Rodrigo Vivi, stable

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@intel.com>
> Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>

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.

The changes here *look* correct to me, so
Reviewed-by: Ville Syrj�l� <ville.syrjala@linux.intel.com>

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

-- 
Ville Syrj�l�
Intel OTC

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

* Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
  2017-12-22 19:58   ` Ville Syrjälä
  (?)
@ 2017-12-22 21:06   ` De Marchi, Lucas
  2017-12-22 21:52       ` Rodrigo Vivi
  -1 siblings, 1 reply; 8+ messages in thread
From: De Marchi, Lucas @ 2017-12-22 21:06 UTC (permalink / raw)
  To: ville.syrjala@linux.intel.com
  Cc: intel-gfx@lists.freedesktop.org, Runyan, Arthur J, Vivi, Rodrigo,
	stable@vger.kernel.org

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@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> 
> 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

> The changes here *look* correct to me, so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> 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
> 
> 

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

* Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
  2017-12-22 21:06   ` De Marchi, Lucas
@ 2017-12-22 21:52       ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2017-12-22 21:52 UTC (permalink / raw)
  To: De Marchi, Lucas
  Cc: intel-gfx@lists.freedesktop.org, Runyan, Arthur J,
	stable@vger.kernel.org

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@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > 
> > 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@linux.intel.com>
> > 
> > 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl
@ 2017-12-22 21:52       ` Rodrigo Vivi
  0 siblings, 0 replies; 8+ messages in thread
From: Rodrigo Vivi @ 2017-12-22 21:52 UTC (permalink / raw)
  To: De Marchi, Lucas
  Cc: ville.syrjala@linux.intel.com, intel-gfx@lists.freedesktop.org,
	Runyan, Arthur J, stable@vger.kernel.org

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@intel.com>
> > > Cc: Ville Syrj�l� <ville.syrjala@linux.intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > 
> > 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@linux.intel.com>
> > 
> > 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
> > 
> > 

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

end of thread, other threads:[~2017-12-22 21:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-04 23:22 [PATCH v4] drm/i915: Apply Display WA #1183 on skl, kbl, and cfl Lucas De Marchi
2017-12-04 23:42 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-12-05  0:27 ` ✓ Fi.CI.IGT: " Patchwork
2017-12-22 19:58 ` [PATCH v4] " Ville Syrjälä
2017-12-22 19:58   ` Ville Syrjälä
2017-12-22 21:06   ` De Marchi, Lucas
2017-12-22 21:52     ` Rodrigo Vivi
2017-12-22 21:52       ` Rodrigo Vivi

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.