intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Don't bypass LRC on CHV
@ 2015-09-08 15:05 ville.syrjala
  2015-09-08 15:05 ` [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset ville.syrjala
  2015-10-05 10:25 ` [PATCH 1/2] drm/i915: Don't bypass LRC on CHV Deepak
  0 siblings, 2 replies; 5+ messages in thread
From: ville.syrjala @ 2015-09-08 15:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The docs are unclear as usual, so it's not clear whether LRC should be
bypassed, performed normally or GRC code should be used as the LRC code.
Some old docs stated that LRC bypass ought to be used, more recent ones
no longer say that. Some docs indicated that we could use GRC as the LRC
code on CHV, but the BIOS doesn't do that, so let's not do it either.

Besides to enable LRC bypass properly, I believe we should set the bit
already before deasserting cmnreset.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c   | 5 -----
 drivers/gpu/drm/i915/intel_hdmi.c | 5 -----
 2 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 45ab25e..825d5ab 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3416,11 +3416,6 @@ static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
 		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW10(ch), val);
 	}
 
-	/* LRC Bypass */
-	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW30);
-	val |= DPIO_LRC_BYPASS;
-	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, val);
-
 	mutex_unlock(&dev_priv->sb_lock);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index feb31d8..9d7923d 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1941,11 +1941,6 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
 	val |= DPIO_PCS_SWING_CALC_TX0_TX2 | DPIO_PCS_SWING_CALC_TX1_TX3;
 	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW10(ch), val);
 
-	/* LRC Bypass */
-	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW30);
-	val |= DPIO_LRC_BYPASS;
-	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, val);
-
 	mutex_unlock(&dev_priv->sb_lock);
 
 	intel_hdmi->set_infoframes(&encoder->base,
-- 
2.4.6

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset
  2015-09-08 15:05 [PATCH 1/2] drm/i915: Don't bypass LRC on CHV ville.syrjala
@ 2015-09-08 15:05 ` ville.syrjala
  2015-10-05 10:27   ` Deepak
  2015-10-05 10:25 ` [PATCH 1/2] drm/i915: Don't bypass LRC on CHV Deepak
  1 sibling, 1 reply; 5+ messages in thread
From: ville.syrjala @ 2015-09-08 15:05 UTC (permalink / raw)
  To: intel-gfx; +Cc: Deepak S

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The BIOS can leave the CHV display PHY in some odd state where
some of the LDOs/lanes won't power down fully when unused. This
will trigger a host of asserts that were added in:
30142273a3e83936fd7b45aa5339311a9295ca51 drm/i915: Add CHV PHY LDO power sanity checks
6669e39f95b5530ca8cb9137703ceb5e83e5d648 drm/i915: Add some CHV DPIO lane power state asserts

To avoid that, skip the asserts until the PHY power well has been
disabled at least once. That will fully reset the PHY, and once
brought back up, the dynamic power down features will work correctly.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  3 +++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 46 ++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 503dff5..cf1880e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1956,6 +1956,9 @@ struct drm_i915_private {
 
 	bool edp_low_vswing;
 
+	/* perform PHY state sanity checks? */
+	bool chv_phy_assert[2];
+
 	/*
 	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
 	 * will be rejected. Instead look for a better place.
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 3f682a1..a0a2e6d 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -987,8 +987,29 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
 		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_D);
 	u32 phy_control = dev_priv->chv_phy_control;
 	u32 phy_status = 0;
+	u32 phy_status_mask = 0xffffffff;
 	u32 tmp;
 
+	/*
+	 * The BIOS can leave the PHY is some weird state
+	 * where it doesn't fully power down some parts.
+	 * Disable the asserts until the PHY has been fully
+	 * reset (ie. the power well has been disabled at
+	 * least once).
+	 */
+	if (!dev_priv->chv_phy_assert[DPIO_PHY0])
+		phy_status_mask &= ~(PHY_STATUS_CMN_LDO(DPIO_PHY0, DPIO_CH0) |
+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH0, 0) |
+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH0, 1) |
+				     PHY_STATUS_CMN_LDO(DPIO_PHY0, DPIO_CH1) |
+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 0) |
+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1));
+
+	if (!dev_priv->chv_phy_assert[DPIO_PHY1])
+		phy_status_mask &= ~(PHY_STATUS_CMN_LDO(DPIO_PHY1, DPIO_CH0) |
+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 0) |
+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1));
+
 	if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) {
 		phy_status |= PHY_POWERGOOD(DPIO_PHY0);
 
@@ -1049,11 +1070,13 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
 			phy_status |= PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1);
 	}
 
+	phy_status &= phy_status_mask;
+
 	/*
 	 * The PHY may be busy with some initial calibration and whatnot,
 	 * so the power state can take a while to actually change.
 	 */
-	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS)) == phy_status, 10))
+	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS) & phy_status_mask) == phy_status, 10))
 		WARN(phy_status != tmp,
 		     "Unexpected PHY_STATUS 0x%08x, expected 0x%08x (PHY_CONTROL=0x%08x)\n",
 		     tmp, phy_status, dev_priv->chv_phy_control);
@@ -1146,6 +1169,9 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
 	DRM_DEBUG_KMS("Disabled DPIO PHY%d (PHY_CONTROL=0x%08x)\n",
 		      phy, dev_priv->chv_phy_control);
 
+	/* PHY is fully reset now, so we can enable the PHY state asserts */
+	dev_priv->chv_phy_assert[phy] = true;
+
 	assert_chv_phy_status(dev_priv);
 }
 
@@ -1155,6 +1181,16 @@ static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpi
 	enum pipe pipe = phy == DPIO_PHY0 ? PIPE_A : PIPE_C;
 	u32 reg, val, expected, actual;
 
+	/*
+	 * The BIOS can leave the PHY is some weird state
+	 * where it doesn't fully power down some parts.
+	 * Disable the asserts until the PHY has been fully
+	 * reset (ie. the power well has been disabled at
+	 * least once).
+	 */
+	if (!dev_priv->chv_phy_assert[phy])
+		return;
+
 	if (ch == DPIO_CH0)
 		reg = _CHV_CMN_DW0_CH0;
 	else
@@ -1911,6 +1947,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
 			PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY0, DPIO_CH1);
 
 		dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY0);
+
+		dev_priv->chv_phy_assert[DPIO_PHY0] = false;
+	} else {
+		dev_priv->chv_phy_assert[DPIO_PHY0] = true;
 	}
 
 	if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) {
@@ -1929,6 +1969,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
 			PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY1, DPIO_CH0);
 
 		dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY1);
+
+		dev_priv->chv_phy_assert[DPIO_PHY1] = false;
+	} else {
+		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
 	}
 
 	I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);
-- 
2.4.6

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: Don't bypass LRC on CHV
  2015-09-08 15:05 [PATCH 1/2] drm/i915: Don't bypass LRC on CHV ville.syrjala
  2015-09-08 15:05 ` [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset ville.syrjala
@ 2015-10-05 10:25 ` Deepak
  1 sibling, 0 replies; 5+ messages in thread
From: Deepak @ 2015-10-05 10:25 UTC (permalink / raw)
  To: intel-gfx



On Tuesday 08 September 2015 08:35 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The docs are unclear as usual, so it's not clear whether LRC should be
> bypassed, performed normally or GRC code should be used as the LRC code.
> Some old docs stated that LRC bypass ought to be used, more recent ones
> no longer say that. Some docs indicated that we could use GRC as the LRC
> code on CHV, but the BIOS doesn't do that, so let's not do it either.
>
> Besides to enable LRC bypass properly, I believe we should set the bit
> already before deasserting cmnreset.

Can we add a Log to indicate what BIOS is configuring? I think it helps in debugging?


Change looks fine.
Reviewed-by: Deepak S<deepak.s@linux.intel.com>


> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_dp.c   | 5 -----
>   drivers/gpu/drm/i915/intel_hdmi.c | 5 -----
>   2 files changed, 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 45ab25e..825d5ab 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3416,11 +3416,6 @@ static uint32_t chv_signal_levels(struct intel_dp *intel_dp)
>   		vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW10(ch), val);
>   	}
>   
> -	/* LRC Bypass */
> -	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW30);
> -	val |= DPIO_LRC_BYPASS;
> -	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, val);
> -
>   	mutex_unlock(&dev_priv->sb_lock);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index feb31d8..9d7923d 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1941,11 +1941,6 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
>   	val |= DPIO_PCS_SWING_CALC_TX0_TX2 | DPIO_PCS_SWING_CALC_TX1_TX3;
>   	vlv_dpio_write(dev_priv, pipe, VLV_PCS23_DW10(ch), val);
>   
> -	/* LRC Bypass */
> -	val = vlv_dpio_read(dev_priv, pipe, CHV_CMN_DW30);
> -	val |= DPIO_LRC_BYPASS;
> -	vlv_dpio_write(dev_priv, pipe, CHV_CMN_DW30, val);
> -
>   	mutex_unlock(&dev_priv->sb_lock);
>   
>   	intel_hdmi->set_infoframes(&encoder->base,

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset
  2015-09-08 15:05 ` [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset ville.syrjala
@ 2015-10-05 10:27   ` Deepak
  2015-10-06  8:30     ` Daniel Vetter
  0 siblings, 1 reply; 5+ messages in thread
From: Deepak @ 2015-10-05 10:27 UTC (permalink / raw)
  To: intel-gfx



On Tuesday 08 September 2015 08:35 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The BIOS can leave the CHV display PHY in some odd state where
> some of the LDOs/lanes won't power down fully when unused. This
> will trigger a host of asserts that were added in:
> 30142273a3e83936fd7b45aa5339311a9295ca51 drm/i915: Add CHV PHY LDO power sanity checks
> 6669e39f95b5530ca8cb9137703ceb5e83e5d648 drm/i915: Add some CHV DPIO lane power state asserts
>
> To avoid that, skip the asserts until the PHY power well has been
> disabled at least once. That will fully reset the PHY, and once
> brought back up, the dynamic power down features will work correctly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h         |  3 +++
>   drivers/gpu/drm/i915/intel_runtime_pm.c | 46 ++++++++++++++++++++++++++++++++-
>   2 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 503dff5..cf1880e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1956,6 +1956,9 @@ struct drm_i915_private {
>   
>   	bool edp_low_vswing;
>   
> +	/* perform PHY state sanity checks? */
> +	bool chv_phy_assert[2];
> +
>   	/*
>   	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
>   	 * will be rejected. Instead look for a better place.
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 3f682a1..a0a2e6d 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -987,8 +987,29 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
>   		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_D);
>   	u32 phy_control = dev_priv->chv_phy_control;
>   	u32 phy_status = 0;
> +	u32 phy_status_mask = 0xffffffff;
>   	u32 tmp;
>   
> +	/*
> +	 * The BIOS can leave the PHY is some weird state
> +	 * where it doesn't fully power down some parts.
> +	 * Disable the asserts until the PHY has been fully
> +	 * reset (ie. the power well has been disabled at
> +	 * least once).
> +	 */
> +	if (!dev_priv->chv_phy_assert[DPIO_PHY0])
> +		phy_status_mask &= ~(PHY_STATUS_CMN_LDO(DPIO_PHY0, DPIO_CH0) |
> +				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH0, 0) |
> +				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH0, 1) |
> +				     PHY_STATUS_CMN_LDO(DPIO_PHY0, DPIO_CH1) |
> +				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 0) |
> +				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1));
> +
> +	if (!dev_priv->chv_phy_assert[DPIO_PHY1])
> +		phy_status_mask &= ~(PHY_STATUS_CMN_LDO(DPIO_PHY1, DPIO_CH0) |
> +				     PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 0) |
> +				     PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1));
> +
>   	if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) {
>   		phy_status |= PHY_POWERGOOD(DPIO_PHY0);
>   
> @@ -1049,11 +1070,13 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
>   			phy_status |= PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1);
>   	}
>   
> +	phy_status &= phy_status_mask;
> +
>   	/*
>   	 * The PHY may be busy with some initial calibration and whatnot,
>   	 * so the power state can take a while to actually change.
>   	 */
> -	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS)) == phy_status, 10))
> +	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS) & phy_status_mask) == phy_status, 10))
>   		WARN(phy_status != tmp,
>   		     "Unexpected PHY_STATUS 0x%08x, expected 0x%08x (PHY_CONTROL=0x%08x)\n",
>   		     tmp, phy_status, dev_priv->chv_phy_control);
> @@ -1146,6 +1169,9 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
>   	DRM_DEBUG_KMS("Disabled DPIO PHY%d (PHY_CONTROL=0x%08x)\n",
>   		      phy, dev_priv->chv_phy_control);
>   
> +	/* PHY is fully reset now, so we can enable the PHY state asserts */
> +	dev_priv->chv_phy_assert[phy] = true;
> +
>   	assert_chv_phy_status(dev_priv);
>   }
>   
> @@ -1155,6 +1181,16 @@ static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpi
>   	enum pipe pipe = phy == DPIO_PHY0 ? PIPE_A : PIPE_C;
>   	u32 reg, val, expected, actual;
>   
> +	/*
> +	 * The BIOS can leave the PHY is some weird state
> +	 * where it doesn't fully power down some parts.
> +	 * Disable the asserts until the PHY has been fully
> +	 * reset (ie. the power well has been disabled at
> +	 * least once).
> +	 */
> +	if (!dev_priv->chv_phy_assert[phy])
> +		return;
> +
>   	if (ch == DPIO_CH0)
>   		reg = _CHV_CMN_DW0_CH0;
>   	else
> @@ -1911,6 +1947,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>   			PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY0, DPIO_CH1);
>   
>   		dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY0);
> +
> +		dev_priv->chv_phy_assert[DPIO_PHY0] = false;
> +	} else {
> +		dev_priv->chv_phy_assert[DPIO_PHY0] = true;
>   	}
>   
>   	if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) {
> @@ -1929,6 +1969,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
>   			PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY1, DPIO_CH0);
>   
>   		dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY1);
> +
> +		dev_priv->chv_phy_assert[DPIO_PHY1] = false;
> +	} else {
> +		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
>   	}
>   
>   	I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);

Change looks fine.
Reviewed-by: Deepak S<deepak.s@linux.intel.com>

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset
  2015-10-05 10:27   ` Deepak
@ 2015-10-06  8:30     ` Daniel Vetter
  0 siblings, 0 replies; 5+ messages in thread
From: Daniel Vetter @ 2015-10-06  8:30 UTC (permalink / raw)
  To: Deepak; +Cc: intel-gfx

On Mon, Oct 05, 2015 at 03:57:45PM +0530, Deepak wrote:
> 
> 
> On Tuesday 08 September 2015 08:35 PM, ville.syrjala@linux.intel.com wrote:
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >The BIOS can leave the CHV display PHY in some odd state where
> >some of the LDOs/lanes won't power down fully when unused. This
> >will trigger a host of asserts that were added in:
> >30142273a3e83936fd7b45aa5339311a9295ca51 drm/i915: Add CHV PHY LDO power sanity checks
> >6669e39f95b5530ca8cb9137703ceb5e83e5d648 drm/i915: Add some CHV DPIO lane power state asserts
> >
> >To avoid that, skip the asserts until the PHY power well has been
> >disabled at least once. That will fully reset the PHY, and once
> >brought back up, the dynamic power down features will work correctly.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h         |  3 +++
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 46 ++++++++++++++++++++++++++++++++-
> >  2 files changed, 48 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index 503dff5..cf1880e 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -1956,6 +1956,9 @@ struct drm_i915_private {
> >  	bool edp_low_vswing;
> >+	/* perform PHY state sanity checks? */
> >+	bool chv_phy_assert[2];
> >+
> >  	/*
> >  	 * NOTE: This is the dri1/ums dungeon, don't add stuff here. Your patch
> >  	 * will be rejected. Instead look for a better place.
> >diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >index 3f682a1..a0a2e6d 100644
> >--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> >+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> >@@ -987,8 +987,29 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
> >  		lookup_power_well(dev_priv, PUNIT_POWER_WELL_DPIO_CMN_D);
> >  	u32 phy_control = dev_priv->chv_phy_control;
> >  	u32 phy_status = 0;
> >+	u32 phy_status_mask = 0xffffffff;
> >  	u32 tmp;
> >+	/*
> >+	 * The BIOS can leave the PHY is some weird state
> >+	 * where it doesn't fully power down some parts.
> >+	 * Disable the asserts until the PHY has been fully
> >+	 * reset (ie. the power well has been disabled at
> >+	 * least once).
> >+	 */
> >+	if (!dev_priv->chv_phy_assert[DPIO_PHY0])
> >+		phy_status_mask &= ~(PHY_STATUS_CMN_LDO(DPIO_PHY0, DPIO_CH0) |
> >+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH0, 0) |
> >+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH0, 1) |
> >+				     PHY_STATUS_CMN_LDO(DPIO_PHY0, DPIO_CH1) |
> >+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 0) |
> >+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY0, DPIO_CH1, 1));
> >+
> >+	if (!dev_priv->chv_phy_assert[DPIO_PHY1])
> >+		phy_status_mask &= ~(PHY_STATUS_CMN_LDO(DPIO_PHY1, DPIO_CH0) |
> >+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 0) |
> >+				     PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1));
> >+
> >  	if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) {
> >  		phy_status |= PHY_POWERGOOD(DPIO_PHY0);
> >@@ -1049,11 +1070,13 @@ static void assert_chv_phy_status(struct drm_i915_private *dev_priv)
> >  			phy_status |= PHY_STATUS_SPLINE_LDO(DPIO_PHY1, DPIO_CH0, 1);
> >  	}
> >+	phy_status &= phy_status_mask;
> >+
> >  	/*
> >  	 * The PHY may be busy with some initial calibration and whatnot,
> >  	 * so the power state can take a while to actually change.
> >  	 */
> >-	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS)) == phy_status, 10))
> >+	if (wait_for((tmp = I915_READ(DISPLAY_PHY_STATUS) & phy_status_mask) == phy_status, 10))
> >  		WARN(phy_status != tmp,
> >  		     "Unexpected PHY_STATUS 0x%08x, expected 0x%08x (PHY_CONTROL=0x%08x)\n",
> >  		     tmp, phy_status, dev_priv->chv_phy_control);
> >@@ -1146,6 +1169,9 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
> >  	DRM_DEBUG_KMS("Disabled DPIO PHY%d (PHY_CONTROL=0x%08x)\n",
> >  		      phy, dev_priv->chv_phy_control);
> >+	/* PHY is fully reset now, so we can enable the PHY state asserts */
> >+	dev_priv->chv_phy_assert[phy] = true;
> >+
> >  	assert_chv_phy_status(dev_priv);
> >  }
> >@@ -1155,6 +1181,16 @@ static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpi
> >  	enum pipe pipe = phy == DPIO_PHY0 ? PIPE_A : PIPE_C;
> >  	u32 reg, val, expected, actual;
> >+	/*
> >+	 * The BIOS can leave the PHY is some weird state
> >+	 * where it doesn't fully power down some parts.
> >+	 * Disable the asserts until the PHY has been fully
> >+	 * reset (ie. the power well has been disabled at
> >+	 * least once).
> >+	 */
> >+	if (!dev_priv->chv_phy_assert[phy])
> >+		return;
> >+
> >  	if (ch == DPIO_CH0)
> >  		reg = _CHV_CMN_DW0_CH0;
> >  	else
> >@@ -1911,6 +1947,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
> >  			PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY0, DPIO_CH1);
> >  		dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY0);
> >+
> >+		dev_priv->chv_phy_assert[DPIO_PHY0] = false;
> >+	} else {
> >+		dev_priv->chv_phy_assert[DPIO_PHY0] = true;
> >  	}
> >  	if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) {
> >@@ -1929,6 +1969,10 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv)
> >  			PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY1, DPIO_CH0);
> >  		dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY1);
> >+
> >+		dev_priv->chv_phy_assert[DPIO_PHY1] = false;
> >+	} else {
> >+		dev_priv->chv_phy_assert[DPIO_PHY1] = true;
> >  	}
> >  	I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);
> 
> Change looks fine.
> Reviewed-by: Deepak S<deepak.s@linux.intel.com>

Both patches applied to dinq, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-10-06  8:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-08 15:05 [PATCH 1/2] drm/i915: Don't bypass LRC on CHV ville.syrjala
2015-09-08 15:05 ` [PATCH 2/2] drm/i915: Skip CHV PHY asserts until PHY has been fully reset ville.syrjala
2015-10-05 10:27   ` Deepak
2015-10-06  8:30     ` Daniel Vetter
2015-10-05 10:25 ` [PATCH 1/2] drm/i915: Don't bypass LRC on CHV Deepak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).