[Intel-gfx] [PATCH 6/7] drm/i915: Implement PHY lane power gating for CHV
Ville Syrjälä
ville.syrjala at linux.intel.com
Mon May 11 04:43:17 PDT 2015
On Sat, May 09, 2015 at 11:05:27AM +0530, Deepak S wrote:
>
>
> On Friday 08 May 2015 09:35 PM, Ville Syrjälä wrote:
> > On Fri, May 08, 2015 at 08:19:12PM +0530, Deepak S wrote:
> >>
> >> On Friday 10 April 2015 08:51 PM, ville.syrjala at linux.intel.com wrote:
> >>> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >>>
> >>> 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ä <ville.syrjala at linux.intel.com>
> >>> ---
> >>> 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 ?
> > Doh. Nice catch. Looks like the dynamic power down bits default to off.
> > I also found the same bit in CL2 DW6, which isn't mentioned by the spec.
> > I'll cook up a patch to flip them and run a few tests to see if stuff
> > still works.
>
> Ok Thanks
> One more Q. I think we need to re-program phy lanes power down override /enable values after power gating/ungating PHY0/1?
I've taken a stab at this, and sadly its making things quite a bit more
complicated. Whenever the lane is power gated we also lose the relevant
registers, so if we power gate the unused lanes before programming all
the PCS/TX registers, we need to change the code so that it doesn't
try to program the registers in the power gated lanes.
Also I'm not entirely sure how LRC etc. works when some/all lanes power
gated. There are some notes that sort of make me think the hw internally
powers up everything regardless of the input signals to guarantee that
things are properly calibrated, but if we lose the registers does that
mean we lose the calibrationresults as well?
And on the other hand there are a bunch of other notes that make me think
they assumed we would only powergate the unused lanes after link training
has finished, which again suggests that we should train the link with max
lanes initially and drop the unused lanes afterwards. But then it's not
clear what happens if we need to increase the number of lanes afterwards
without doing a full PHY reinit.
I also had the machine hard hang if I enabled the CL1 PSR power down bit
in the single channel PHY, whereas setting it in the dual channel PHY is
apparently OK.
So looks like this topic needs a bit more study.
>
> >
> >> 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)
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
Ville Syrjälä
Intel OTC
More information about the Intel-gfx
mailing list