diff mbox

[4/4] drm/i915: Disable GMBUS clock gating around GMBUS transfers on gen9+

Message ID 20171208213739.16388-4-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Dec. 8, 2017, 9:37 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Gen9+ need to disable GMBUS clock gating when doing multi part
transfers. Otherwise clock gating will kick in when GMBUS is in
the WAIT state and presumably that will corrupt the transfer.
This is documented as Display WA #0868.

Apparently older hardware doesn't allow clock gating in the WAIT
state and thus are unaffected by this problem.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  4 ++++
 drivers/gpu/drm/i915/intel_i2c.c | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

Comments

Dhinakaran Pandiyan Dec. 11, 2017, 9:30 p.m. UTC | #1 
On Fri, 2017-12-08 at 23:37 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 

> Gen9+ need to disable GMBUS clock gating when doing multi part

> transfers. Otherwise clock gating will kick in when GMBUS is in

> the WAIT state and presumably that will corrupt the transfer.

> This is documented as Display WA #0868.

> 

> Apparently older hardware doesn't allow clock gating in the WAIT

> state and thus are unaffected by this problem.

> 

Checked this against bspec, lgtm.

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> with

one comment below.


> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---

>  drivers/gpu/drm/i915/i915_reg.h  |  4 ++++

>  drivers/gpu/drm/i915/intel_i2c.c | 38 ++++++++++++++++++++++++++++++++++++++

>  2 files changed, 42 insertions(+)

> 

> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h

> index 98e1781bce50..827edbfbd5df 100644

> --- a/drivers/gpu/drm/i915/i915_reg.h

> +++ b/drivers/gpu/drm/i915/i915_reg.h

> @@ -3859,6 +3859,9 @@ enum {

>  #define   PWM2_GATING_DIS		(1 << 14)

>  #define   PWM1_GATING_DIS		(1 << 13)

>  

> +#define GEN9_CLKGATE_DIS_4		_MMIO(0x4653C)

> +#define   BXT_GMBUS_GATING_DIS		(1 << 14)

> +

>  #define _CLKGATE_DIS_PSL_A		0x46520

>  #define _CLKGATE_DIS_PSL_B		0x46524

>  #define _CLKGATE_DIS_PSL_C		0x46528

> @@ -7553,6 +7556,7 @@ enum {

>  #define FDI_RX_CHICKEN(pipe)	_MMIO_PIPE(pipe, _FDI_RXA_CHICKEN, _FDI_RXB_CHICKEN)

>  

>  #define SOUTH_DSPCLK_GATE_D	_MMIO(0xc2020)

> +#define  PCH_GMBUSUNIT_CLOCK_GATE_DISABLE (1<<31)

>  #define  PCH_DPLUNIT_CLOCK_GATE_DISABLE (1<<30)

>  #define  PCH_DPLSUNIT_CLOCK_GATE_DISABLE (1<<29)

>  #define  PCH_CPUNIT_CLOCK_GATE_DISABLE (1<<14)

> diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c

> index a8c08994f505..6d3361a0ec76 100644

> --- a/drivers/gpu/drm/i915/intel_i2c.c

> +++ b/drivers/gpu/drm/i915/intel_i2c.c

> @@ -142,6 +142,32 @@ static void pnv_gmbus_clock_gating(struct drm_i915_private *dev_priv,

>  	I915_WRITE(DSPCLK_GATE_D, val);

>  }

>  

> +static void pch_gmbus_clock_gating(struct drm_i915_private *dev_priv,

> +				   bool enable)

> +{

> +	u32 val;

> +

> +	val = I915_READ(SOUTH_DSPCLK_GATE_D);

> +	if (!enable)

> +		val |= PCH_GMBUSUNIT_CLOCK_GATE_DISABLE;

> +	else

> +		val &= ~PCH_GMBUSUNIT_CLOCK_GATE_DISABLE;

> +	I915_WRITE(SOUTH_DSPCLK_GATE_D, val);

> +}

> +

> +static void bxt_gmbus_clock_gating(struct drm_i915_private *dev_priv,

> +				   bool enable)

> +{

> +	u32 val;

> +

> +	val = I915_READ(GEN9_CLKGATE_DIS_4);

> +	if (!enable)

> +		val |= BXT_GMBUS_GATING_DIS;

> +	else

> +		val &= ~BXT_GMBUS_GATING_DIS;

> +	I915_WRITE(GEN9_CLKGATE_DIS_4, val);

> +}

> +

>  static u32 get_reserved(struct intel_gmbus *bus)

>  {

>  	struct drm_i915_private *dev_priv = bus->dev_priv;

> @@ -484,6 +510,12 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)

>  	int i = 0, inc, try = 0;

>  	int ret = 0;

>  

> +	/* Display WA #0868: skl,bxt,kbl,cfl,glk,cnl */

> +	if (IS_GEN9_LP(dev_priv))

> +		bxt_gmbus_clock_gating(dev_priv, false);

> +	else if (HAS_PCH_SPLIT(dev_priv) && INTEL_GEN(dev_priv) >= 9)


Given that this is a platform specific workaround, I think it is better
to keep this [9,10]


> +		pch_gmbus_clock_gating(dev_priv, false);

> +

>  retry:

>  	I915_WRITE_FW(GMBUS0, bus->reg0);

>  

> @@ -585,6 +617,12 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)

>  	ret = -EAGAIN;

>  

>  out:

> +	/* Display WA #0868: skl,bxt,kbl,cfl,glk,cnl */

> +	if (IS_GEN9_LP(dev_priv))

> +		bxt_gmbus_clock_gating(dev_priv, true);

> +	else if (HAS_PCH_SPLIT(dev_priv) && INTEL_GEN(dev_priv) >= 9)

> +		pch_gmbus_clock_gating(dev_priv, true);

> +

>  	return ret;

>  }

>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 98e1781bce50..827edbfbd5df 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3859,6 +3859,9 @@  enum {
 #define   PWM2_GATING_DIS		(1 << 14)
 #define   PWM1_GATING_DIS		(1 << 13)
 
+#define GEN9_CLKGATE_DIS_4		_MMIO(0x4653C)
+#define   BXT_GMBUS_GATING_DIS		(1 << 14)
+
 #define _CLKGATE_DIS_PSL_A		0x46520
 #define _CLKGATE_DIS_PSL_B		0x46524
 #define _CLKGATE_DIS_PSL_C		0x46528
@@ -7553,6 +7556,7 @@  enum {
 #define FDI_RX_CHICKEN(pipe)	_MMIO_PIPE(pipe, _FDI_RXA_CHICKEN, _FDI_RXB_CHICKEN)
 
 #define SOUTH_DSPCLK_GATE_D	_MMIO(0xc2020)
+#define  PCH_GMBUSUNIT_CLOCK_GATE_DISABLE (1<<31)
 #define  PCH_DPLUNIT_CLOCK_GATE_DISABLE (1<<30)
 #define  PCH_DPLSUNIT_CLOCK_GATE_DISABLE (1<<29)
 #define  PCH_CPUNIT_CLOCK_GATE_DISABLE (1<<14)
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index a8c08994f505..6d3361a0ec76 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -142,6 +142,32 @@  static void pnv_gmbus_clock_gating(struct drm_i915_private *dev_priv,
 	I915_WRITE(DSPCLK_GATE_D, val);
 }
 
+static void pch_gmbus_clock_gating(struct drm_i915_private *dev_priv,
+				   bool enable)
+{
+	u32 val;
+
+	val = I915_READ(SOUTH_DSPCLK_GATE_D);
+	if (!enable)
+		val |= PCH_GMBUSUNIT_CLOCK_GATE_DISABLE;
+	else
+		val &= ~PCH_GMBUSUNIT_CLOCK_GATE_DISABLE;
+	I915_WRITE(SOUTH_DSPCLK_GATE_D, val);
+}
+
+static void bxt_gmbus_clock_gating(struct drm_i915_private *dev_priv,
+				   bool enable)
+{
+	u32 val;
+
+	val = I915_READ(GEN9_CLKGATE_DIS_4);
+	if (!enable)
+		val |= BXT_GMBUS_GATING_DIS;
+	else
+		val &= ~BXT_GMBUS_GATING_DIS;
+	I915_WRITE(GEN9_CLKGATE_DIS_4, val);
+}
+
 static u32 get_reserved(struct intel_gmbus *bus)
 {
 	struct drm_i915_private *dev_priv = bus->dev_priv;
@@ -484,6 +510,12 @@  do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
 	int i = 0, inc, try = 0;
 	int ret = 0;
 
+	/* Display WA #0868: skl,bxt,kbl,cfl,glk,cnl */
+	if (IS_GEN9_LP(dev_priv))
+		bxt_gmbus_clock_gating(dev_priv, false);
+	else if (HAS_PCH_SPLIT(dev_priv) && INTEL_GEN(dev_priv) >= 9)
+		pch_gmbus_clock_gating(dev_priv, false);
+
 retry:
 	I915_WRITE_FW(GMBUS0, bus->reg0);
 
@@ -585,6 +617,12 @@  do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num)
 	ret = -EAGAIN;
 
 out:
+	/* Display WA #0868: skl,bxt,kbl,cfl,glk,cnl */
+	if (IS_GEN9_LP(dev_priv))
+		bxt_gmbus_clock_gating(dev_priv, true);
+	else if (HAS_PCH_SPLIT(dev_priv) && INTEL_GEN(dev_priv) >= 9)
+		pch_gmbus_clock_gating(dev_priv, true);
+
 	return ret;
 }