Re: [PATCH v3 14/16] drm/i915/bxt: Add HW state verification for DDI PHY and CDCLK

[David Weinehall <david.weinehall@linux.intel.com>]

On Mon, Apr 04, 2016 at 05:27:10PM +0300, Imre Deak wrote:
> I caught a few errors in our current PHY/CDCLK programming by sanity
> checking the actual programmed state, so I thought it would be also
> useful for the future. In addition to verifying the state after
> programming it also verify it after exiting DC5, to make sure DMC
> restored/kept intact everything related.
> 
> v2:
> - Inlining __phy_reg_verify_state() doesn't make sense and also
>   incorrect, so don't do it (PW/CI gcc)
> v3:
> - Rebase on latest -nightly
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: David Weinehall <david.weinehall@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |   1 +
>  drivers/gpu/drm/i915/intel_ddi.c        | 124 +++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c    |   5 ++
>  drivers/gpu/drm/i915/intel_drv.h        |   2 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c |   8 +++
>  5 files changed, 138 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dd18772..6f4a721 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1908,6 +1908,7 @@ struct drm_i915_private {
>  	 * crappiness (can't read out DPLL_MD for pipes B & C).
>  	 */
>  	u32 chv_dpll_md[I915_MAX_PIPES];
> +	u32 bxt_phy_grc;
>  
>  	u32 suspend_count;
>  	bool suspended_to_idle;
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index d944bff..fd20119 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1753,6 +1753,13 @@ static bool broxton_phy_is_enabled(struct drm_i915_private *dev_priv,
>  	return true;
>  }
>  
> +static u32 broxton_get_grc(struct drm_i915_private *dev_priv, enum dpio_phy phy)
> +{
> +	u32 val = I915_READ(BXT_PORT_REF_DW6(phy));
> +
> +	return (val & GRC_CODE_MASK) >> GRC_CODE_SHIFT;
> +}
> +
>  static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  			     enum dpio_phy phy)
>  {
> @@ -1762,6 +1769,9 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  	if (broxton_phy_is_enabled(dev_priv, phy)) {
>  		DRM_DEBUG_DRIVER("DDI PHY %d already enabled, "
>  				 "won't reprogram it\n", phy);
> +		/* Still read out the GRC value for state verification */
> +		if (phy == DPIO_PHY1)
> +			dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv, phy);
>  
>  		return;
>  	}
> @@ -1857,8 +1867,8 @@ static void broxton_phy_init(struct drm_i915_private *dev_priv,
>  			     10))
>  			DRM_ERROR("timeout waiting for PHY1 GRC\n");
>  
> -		val = I915_READ(BXT_PORT_REF_DW6(DPIO_PHY1));
> -		val = (val & GRC_CODE_MASK) >> GRC_CODE_SHIFT;
> +		val = dev_priv->bxt_phy_grc = broxton_get_grc(dev_priv,
> +							      DPIO_PHY1);
>  		grc_code = val << GRC_CODE_FAST_SHIFT |
>  			   val << GRC_CODE_SLOW_SHIFT |
>  			   val;
> @@ -1901,6 +1911,116 @@ void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv)
>  	broxton_phy_uninit(dev_priv, DPIO_PHY0);
>  }
>  
> +static bool __printf(6, 7)
> +__phy_reg_verify_state(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> +		       i915_reg_t reg, u32 mask, u32 expected,
> +		       const char *reg_fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +	u32 val;
> +
> +	val = I915_READ(reg);
> +	if ((val & mask) == expected)
> +		return true;
> +
> +	va_start(args, reg_fmt);
> +	vaf.fmt = reg_fmt;
> +	vaf.va = &args;
> +
> +	DRM_DEBUG_DRIVER("DDI PHY %d reg %pV [%08x] state mismatch: "
> +			 "current %08x, expected %08x (mask %08x)\n",
> +			 phy, &vaf, reg.reg, val, (val & ~mask) | expected,
> +			 mask);
> +
> +	va_end(args);
> +
> +	return false;
> +}
> +
> +static bool broxton_phy_verify_state(struct drm_i915_private *dev_priv,
> +				     enum dpio_phy phy)
> +{
> +	enum port port;
> +	u32 ports;
> +	uint32_t mask;
> +	bool ok;
> +
> +#define _CHK(reg, mask, exp, fmt, ...)					\
> +	__phy_reg_verify_state(dev_priv, phy, reg, mask, exp, fmt,	\
> +			       ## __VA_ARGS__)
> +
> +	/* We expect the PHY to be always enabled */
> +	if (!broxton_phy_is_enabled(dev_priv, phy))
> +		return false;
> +
> +	ok = true;
> +
> +	if (phy == DPIO_PHY0)
> +		ports = BIT(PORT_B) | BIT(PORT_C);
> +	else
> +		ports = BIT(PORT_A);
> +
> +	for_each_port_masked(port, ports) {
> +		int lane;
> +
> +		for (lane = 0; lane < 4; lane++)
> +			ok &= _CHK(BXT_PORT_TX_DW14_LN(port, lane),
> +				    LATENCY_OPTIM,
> +				    lane != 1 ? LATENCY_OPTIM : 0,
> +				    "BXT_PORT_TX_DW14_LN(%d, %d)", port, lane);
> +	}
> +
> +	/* PLL Rcomp code offset */
> +	ok &= _CHK(BXT_PORT_CL1CM_DW9(phy),
> +		    IREF0RC_OFFSET_MASK, 0xe4 << IREF0RC_OFFSET_SHIFT,
> +		    "BXT_PORT_CL1CM_DW9(%d)", phy);
> +	ok &= _CHK(BXT_PORT_CL1CM_DW10(phy),
> +		    IREF1RC_OFFSET_MASK, 0xe4 << IREF1RC_OFFSET_SHIFT,
> +		    "BXT_PORT_CL1CM_DW10(%d)", phy);
> +
> +	/* Power gating */
> +	mask = OCL1_POWER_DOWN_EN | DW28_OLDO_DYN_PWR_DOWN_EN | SUS_CLK_CONFIG;
> +	ok &= _CHK(BXT_PORT_CL1CM_DW28(phy), mask, mask,
> +		    "BXT_PORT_CL1CM_DW28(%d)", phy);
> +
> +	if (phy == DPIO_PHY0)
> +		ok &= _CHK(BXT_PORT_CL2CM_DW6_BC,
> +			   DW6_OLDO_DYN_PWR_DOWN_EN, DW6_OLDO_DYN_PWR_DOWN_EN,
> +			   "BXT_PORT_CL2CM_DW6_BC");
> +
> +	/*
> +	 * TODO: Verify BXT_PORT_CL1CM_DW30 bit OCL2_LDOFUSE_PWR_DIS,
> +	 * at least on stepping A this bit is read-only and fixed at 0.
> +	 */
> +
> +	if (phy == DPIO_PHY0) {
> +		u32 grc_code = dev_priv->bxt_phy_grc;
> +
> +		grc_code = grc_code << GRC_CODE_FAST_SHIFT |
> +		           grc_code << GRC_CODE_SLOW_SHIFT |
> +			   grc_code;
> +		mask = GRC_CODE_FAST_MASK | GRC_CODE_SLOW_MASK |
> +		       GRC_CODE_NOM_MASK;
> +		ok &= _CHK(BXT_PORT_REF_DW6(DPIO_PHY0), mask, grc_code,
> +			    "BXT_PORT_REF_DW6(%d)", DPIO_PHY0);
> +
> +		mask = GRC_DIS | GRC_RDY_OVRD;
> +		ok &= _CHK(BXT_PORT_REF_DW8(DPIO_PHY0), mask, mask,
> +			    "BXT_PORT_REF_DW8(%d)", DPIO_PHY0);
> +	}
> +
> +	return ok;
> +#undef _CHK
> +}
> +
> +void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv)
> +{
> +	if (!broxton_phy_verify_state(dev_priv, DPIO_PHY0) ||
> +	    !broxton_phy_verify_state(dev_priv, DPIO_PHY1))
> +		i915_report_error(dev_priv, "DDI PHY state mismatch\n");
> +}
> +
>  void intel_ddi_prepare_link_retrain(struct intel_dp *intel_dp)
>  {
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8999fe8..842ca5d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5460,6 +5460,11 @@ static bool broxton_cdclk_is_enabled(struct drm_i915_private *dev_priv)
>  	return true;
>  }
>  
> +bool broxton_cdclk_verify_state(struct drm_i915_private *dev_priv)
> +{
> +	return broxton_cdclk_is_enabled(dev_priv);
> +}
> +
>  void broxton_init_cdclk(struct drm_i915_private *dev_priv)
>  {
>  	/* check if cd clock is enabled */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4c2083d..4708c49 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1224,8 +1224,10 @@ void hsw_enable_pc8(struct drm_i915_private *dev_priv);
>  void hsw_disable_pc8(struct drm_i915_private *dev_priv);
>  void broxton_init_cdclk(struct drm_i915_private *dev_priv);
>  void broxton_uninit_cdclk(struct drm_i915_private *dev_priv);
> +bool broxton_cdclk_verify_state(struct drm_i915_private *dev_priv);
>  void broxton_ddi_phy_init(struct drm_i915_private *dev_priv);
>  void broxton_ddi_phy_uninit(struct drm_i915_private *dev_priv);
> +void broxton_ddi_phy_verify_state(struct drm_i915_private *dev_priv);
>  void bxt_enable_dc9(struct drm_i915_private *dev_priv);
>  void bxt_disable_dc9(struct drm_i915_private *dev_priv);
>  void skl_init_cdclk(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index e60b02e..59b30c9 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -797,6 +797,11 @@ static void gen9_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>  					  struct i915_power_well *power_well)
>  {
>  	gen9_set_dc_state(dev_priv, DC_STATE_DISABLE);
> +
> +	if (IS_BROXTON(dev_priv)) {
> +		broxton_cdclk_verify_state(dev_priv);
> +		broxton_ddi_phy_verify_state(dev_priv);
> +	}
>  }
>  
>  static void gen9_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> @@ -2184,6 +2189,9 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv,
>  	broxton_init_cdclk(dev_priv);
>  	broxton_ddi_phy_init(dev_priv);
>  
> +	broxton_cdclk_verify_state(dev_priv);
> +	broxton_ddi_phy_verify_state(dev_priv);
> +
>  	if (resume && dev_priv->csr.dmc_payload)
>  		intel_csr_load_program(dev_priv);
>  }
> -- 
> 2.5.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx