Re: [PATCH 14/15] drm/i915: Add some CHV DPIO lane power state asserts

[Deepak <deepak.s@linux.intel.com>]

On 07/09/2015 02:16 AM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add some checks that the state of the DPIO lanes is more or less what we
> expect based on the overrides.
>
> The hardware only provides two bits per channel indicating whether all
> or some of the lanes are powered down, so we can't do an exact check.
>
> Additionally, CL2 powering down before we can check it adds another
> twist. To work around this we simply check for the 0 value of the
> CL2 register (which is what we get when it's powered down) and
> adjust our expectations.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_reg.h         |  8 +++++
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 54 +++++++++++++++++++++++++++++++++
>   2 files changed, 62 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 395f556..586a0f7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1089,6 +1089,12 @@ enum skl_disp_power_wells {
>   #define  DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE	1 /* 1: coarse & 0 : fine  */
>   #define CHV_PLL_DW9(ch) _PIPE(ch, _CHV_PLL_DW9_CH0, _CHV_PLL_DW9_CH1)
>   
> +#define _CHV_CMN_DW0_CH0               0x8100
> +#define   DPIO_ALLDL_POWERDOWN_SHIFT_CH0	19
> +#define   DPIO_ANYDL_POWERDOWN_SHIFT_CH0	18
> +#define   DPIO_ALLDL_POWERDOWN			(1 << 1)
> +#define   DPIO_ANYDL_POWERDOWN			(1 << 0)
> +
>   #define _CHV_CMN_DW5_CH0               0x8114
>   #define   CHV_BUFRIGHTENA1_DISABLE	(0 << 20)
>   #define   CHV_BUFRIGHTENA1_NORMAL	(1 << 20)
> @@ -1125,6 +1131,8 @@ enum skl_disp_power_wells {
>   
>   #define _CHV_CMN_DW19_CH0		0x814c
>   #define _CHV_CMN_DW6_CH1		0x8098
> +#define   DPIO_ALLDL_POWERDOWN_SHIFT_CH1	30 /* CL2 DW6 only */
> +#define   DPIO_ANYDL_POWERDOWN_SHIFT_CH1	29 /* CL2 DW6 only */
>   #define   DPIO_DYNPWRDOWNEN_CH1		(1 << 28) /* CL2 DW6 only */
>   #define   CHV_CMN_USEDCLKCHANNEL	(1 << 13)
>   
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 002b78f..a1d9676 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1026,6 +1026,58 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
>   		      phy, dev_priv->chv_phy_control);
>   }
>   
> +static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> +				     enum dpio_channel ch, bool override, unsigned int mask)
> +{
> +	enum pipe pipe = phy == DPIO_PHY0 ? PIPE_A : PIPE_C;
Why not PIPE B? I think PIPE B can also be used to drive using PHY0?
> +	u32 reg, val, expected, actual;
> +
> +	if (ch == DPIO_CH0)
> +		reg = _CHV_CMN_DW0_CH0;
> +	else
> +		reg = _CHV_CMN_DW6_CH1;
> +
> +	mutex_lock(&dev_priv->sb_lock);
> +	val = vlv_dpio_read(dev_priv, pipe, reg);
> +	mutex_unlock(&dev_priv->sb_lock);
> +
> +	/*
> +	 * This assumes !override is only used when the port is disabled.
> +	 * All lanes should power down even without the override when
> +	 * the port is disabled.
> +	 */
> +	if (!override || mask == 0xf) {
> +		expected = DPIO_ALLDL_POWERDOWN | DPIO_ANYDL_POWERDOWN;
> +		/*
> +		 * If CH1 common lane is not active anymore
> +		 * (eg. for pipe B DPLL) the entire channel will
> +		 * shut down, which causes the common lane registers
> +		 * to read as 0. That means we can't actually check
> +		 * the lane power down status bits, but as the entire
> +		 * register reads as 0 it's a good indication that the
> +		 * channel is indeed entirely powered down.
> +		 */
> +		if (ch == DPIO_CH1 && val == 0)
> +			expected = 0;
> +	} else if (mask != 0x0) {
> +		expected = DPIO_ANYDL_POWERDOWN;
> +	} else {
> +		expected = 0;
> +	}
> +
> +	if (ch == DPIO_CH0)
> +		actual = val >> DPIO_ANYDL_POWERDOWN_SHIFT_CH0;
> +	else
> +		actual = val >> DPIO_ANYDL_POWERDOWN_SHIFT_CH1;
> +	actual &= DPIO_ALLDL_POWERDOWN | DPIO_ANYDL_POWERDOWN;
> +
> +	WARN(actual != expected,
> +	     "Unexpected DPIO lane power down: all %d, any %d. Expected: all %d, any %d. (0x%x = 0x%08x)\n",
> +	     !!(actual & DPIO_ALLDL_POWERDOWN), !!(actual & DPIO_ANYDL_POWERDOWN),
> +	     !!(expected & DPIO_ALLDL_POWERDOWN), !!(expected & DPIO_ANYDL_POWERDOWN),
> +	     reg, val);
> +}
> +
>   bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
>   			  enum dpio_channel ch, bool override)
>   {
> @@ -1078,6 +1130,8 @@ void chv_phy_powergate_lanes(struct intel_encoder *encoder,
>   	DRM_DEBUG_KMS("Power gating DPIO PHY%d CH%d lanes 0x%x (PHY_CONTROL=0x%08x)\n",
>   		      phy, ch, mask, dev_priv->chv_phy_control);
>   
> +	assert_chv_phy_powergate(dev_priv, phy, ch, override, mask);
> +
>   	mutex_unlock(&power_domains->lock);
>   }
>   

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