Re: [PATCH 6/7] drm/i915: Implement PHY lane power gating for CHV

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, May 08, 2015 at 08:19:12PM +0530, Deepak S wrote:
> 
> 
> On Friday 10 April 2015 08:51 PM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxx>
> > ---
> >   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.

> 
> 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx


-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux