On Friday 10 April 2015 08:51 PM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä > > Powergate the PHY lanes when they're not needed. For HDMI all four lanes > are needed always, but for DP we can enable only the needed lanes. And > when the port is not used all lanes can be power gated. This could > reduce power consumption a bit when only a subset of the lanes in the > PHY are required. > > A bit of extra care is needed to reconstruct the initial state of the > DPIO_PHY_CONTROL register since we can't read it. So instead we read the > actual lane status from the DPLL/PHY_STATUS registers and use that to > determine which lanes ought to be powergated initially. > > Also sprinkle a few debug prints around so that we can monitor the > DPIO_PHY_STATUS changes without having to read it and risk corrupting > it. > > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/i915/i915_reg.h | 2 + > drivers/gpu/drm/i915/intel_dp.c | 8 ++++ > drivers/gpu/drm/i915/intel_drv.h | 2 + > drivers/gpu/drm/i915/intel_hdmi.c | 5 +++ > drivers/gpu/drm/i915/intel_runtime_pm.c | 69 +++++++++++++++++++++++++++++++-- > 5 files changed, 82 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 977bad6..34c366a 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1887,10 +1887,12 @@ enum skl_disp_power_wells { > #define DPIO_PHY_STATUS (VLV_DISPLAY_BASE + 0x6240) > #define DPLL_PORTD_READY_MASK (0xf) > #define DISPLAY_PHY_CONTROL (VLV_DISPLAY_BASE + 0x60100) > +#define PHY_CH_POWER_DOWN_OVRD_EN(phy, ch) (1 << (2*(phy)+(ch)+27)) > #define PHY_LDO_DELAY_0NS 0x0 > #define PHY_LDO_DELAY_200NS 0x1 > #define PHY_LDO_DELAY_600NS 0x2 > #define PHY_LDO_SEQ_DELAY(delay, phy) ((delay) << (2*(phy)+23)) > +#define PHY_CH_POWER_DOWN_OVRD(mask, phy, ch) ((mask) << (8*(phy)+4*(ch)+11)) > #define PHY_CH_SU_PSR 0x1 > #define PHY_CH_DEEP_PSR 0x7 > #define PHY_CH_POWER_MODE(mode, phy, ch) ((mode) << (6*(phy)+3*(ch)+2)) > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index ac38fd8..0b43f99 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -2336,6 +2336,8 @@ static void chv_post_disable_dp(struct intel_encoder *encoder) > > intel_dp_link_down(intel_dp); > > + chv_powergate_phy_lanes(encoder, 0xf); > + > mutex_lock(&dev_priv->dpio_lock); > > /* Propagate soft reset to data lane reset */ > @@ -2482,6 +2484,12 @@ static void intel_enable_dp(struct intel_encoder *encoder) > if (IS_VALLEYVIEW(dev)) > vlv_init_panel_power_sequencer(intel_dp); > > + if (IS_CHERRYVIEW(dev)) { > + /* FIXME deal with lane reversal */ > + lane_mask = 0xf & ~((1 << intel_dp->lane_count) - 1); > + chv_powergate_phy_lanes(encoder, lane_mask); > + } > + > intel_dp_enable_port(intel_dp); > > edp_panel_vdd_on(intel_dp); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 3ec829a..54bcca8 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1238,6 +1238,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv); > > void intel_display_set_init_power(struct drm_i915_private *dev, bool enable); > > +void chv_powergate_phy_lanes(struct intel_encoder *encoder, unsigned int mask); > + > /* intel_pm.c */ > void intel_init_clock_gating(struct drm_device *dev); > void intel_suspend_hw(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 24b0aa1..f5842c3 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -916,6 +916,9 @@ static void intel_disable_hdmi(struct intel_encoder *encoder) > I915_WRITE(intel_hdmi->hdmi_reg, temp); > POSTING_READ(intel_hdmi->hdmi_reg); > } > + > + if (IS_CHERRYVIEW(dev)) > + chv_powergate_phy_lanes(encoder, 0xf); > } > > static int hdmi_portclock_limit(struct intel_hdmi *hdmi, bool respect_dvi_limit) > @@ -1634,6 +1637,8 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder) > intel_crtc->config->has_hdmi_sink, > adjusted_mode); > > + chv_powergate_phy_lanes(encoder, 0x0); > + > intel_enable_hdmi(encoder); > > vlv_wait_port_ready(dev_priv, dport, 0x0); > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 5cd8a51..d9e00d3 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -668,6 +668,9 @@ static void chv_dpio_cmn_power_well_enable(struct drm_i915_private *dev_priv, > > dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(phy); > I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control); > + > + DRM_DEBUG_KMS("Enabled DPIO PHY%d (DPIO_PHY_CONTROL=0x%08x)\n", > + phy, dev_priv->chv_phy_control); > } > > static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, > @@ -680,10 +683,15 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, > > if (power_well->data == PUNIT_POWER_WELL_DPIO_CMN_BC) { > phy = DPIO_PHY0; > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(0xf, phy, DPIO_CH0) | > + PHY_CH_POWER_DOWN_OVRD(0xf, phy, DPIO_CH1); > assert_pll_disabled(dev_priv, PIPE_A); > assert_pll_disabled(dev_priv, PIPE_B); > } else { > phy = DPIO_PHY1; > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(0xf, phy, DPIO_CH0); > assert_pll_disabled(dev_priv, PIPE_C); > } > > @@ -691,6 +699,25 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv, > I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control); > > vlv_set_power_well(dev_priv, power_well, false); > + > + DRM_DEBUG_KMS("Disabled DPIO PHY%d (DPIO_PHY_CONTROL=0x%08x)\n", > + phy, dev_priv->chv_phy_control); > +} > + > +void chv_powergate_phy_lanes(struct intel_encoder *encoder, unsigned int mask) > +{ > + struct drm_device *dev = encoder->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc); > + enum dpio_phy phy = DPIO_PHY(intel_crtc->pipe); > + enum dpio_channel ch = vlv_dport_to_channel(enc_to_dig_port(&encoder->base)); > + > + dev_priv->chv_phy_control &= ~PHY_CH_POWER_DOWN_OVRD(0xf, phy, ch); > + dev_priv->chv_phy_control |= PHY_CH_POWER_DOWN_OVRD(mask, phy, ch); > + I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control); > + > + DRM_DEBUG_KMS("Power gating DPIO PHY%d CH%d lanes 0x%x (DPIO_PHY_CONTROL=0x%08x)\n", > + phy, ch, mask, dev_priv->chv_phy_control); > } > Hi Ville, I think based on the spec, we needto program 0x8170 as 0xxxC3xxxx to affect the DDI0 power down and this reigster to be programted beforeDPIO_PHY_CONTROL ? Thanks Deepak > static bool chv_pipe_power_well_enabled(struct drm_i915_private *dev_priv, > @@ -1402,19 +1429,53 @@ static void chv_phy_control_init(struct drm_i915_private *dev_priv) > * DISPLAY_PHY_CONTROL can get corrupted if read. As a > * workaround never ever read DISPLAY_PHY_CONTROL, and > * instead maintain a shadow copy ourselves. Use the actual > - * power well state to reconstruct the expected initial > - * value. > + * power well state and lane status to reconstruct the > + * expected initial value. > */ > dev_priv->chv_phy_control = > + PHY_CH_POWER_DOWN_OVRD_EN(DPIO_PHY0, DPIO_CH0) | > + PHY_CH_POWER_DOWN_OVRD_EN(DPIO_PHY0, DPIO_CH1) | > + PHY_CH_POWER_DOWN_OVRD_EN(DPIO_PHY1, DPIO_CH0) | > PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY0) | > PHY_LDO_SEQ_DELAY(PHY_LDO_DELAY_600NS, DPIO_PHY1) | > PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH0) | > PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY0, DPIO_CH1) | > PHY_CH_POWER_MODE(PHY_CH_SU_PSR, DPIO_PHY1, DPIO_CH0); > - if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) > + > + if (cmn_bc->ops->is_enabled(dev_priv, cmn_bc)) { > + uint32_t status = I915_READ(DPLL(PIPE_A)); > + unsigned int mask; > + > + mask = status & DPLL_PORTB_READY_MASK; > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY0, DPIO_CH0); > + mask = (status & DPLL_PORTC_READY_MASK) >> 4; > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY0, DPIO_CH1); > + > dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY0); > - if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) > + } else { > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(0xf, DPIO_PHY0, DPIO_CH0) | > + PHY_CH_POWER_DOWN_OVRD(0xf, DPIO_PHY0, DPIO_CH1); > + } > + > + if (cmn_d->ops->is_enabled(dev_priv, cmn_d)) { > + uint32_t status = I915_READ(DPIO_PHY_STATUS); > + unsigned int mask; > + > + mask = status & DPLL_PORTD_READY_MASK; > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(mask, DPIO_PHY1, DPIO_CH0); > + > dev_priv->chv_phy_control |= PHY_COM_LANE_RESET_DEASSERT(DPIO_PHY1); > + } else { > + dev_priv->chv_phy_control |= > + PHY_CH_POWER_DOWN_OVRD(0xf, DPIO_PHY1, DPIO_CH0); > + } > + > + DRM_DEBUG_KMS("Initial DPIO_PHY_CONTROL=0x%08x\n", > + dev_priv->chv_phy_control); > } > > static void vlv_cmnlane_wa(struct drm_i915_private *dev_priv)