On Mon, Oct 05, 2015 at 03:57:45PM +0530, Deepak wrote: > > > On Tuesday 08 September 2015 08:35 PM, [email protected] wrote: > >From: Ville Syrjälä <[email protected]> > > > >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ä <[email protected]> > >--- > > 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<[email protected]>
Both patches applied to dinq, thanks. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
