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

[Pandiyan, Dhinakaran]

On Fri, 2017-12-08 at 23:37 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at 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 at intel.com> with
one comment below.


> Signed-off-by: Ville Syrjälä <ville.syrjala at 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;
>  }
>