On Wed, Aug 19, 2015 at 02:29:37PM +0300, Ville Syrjälä wrote:
> On Wed, Aug 19, 2015 at 07:47:41AM +0530, Deepak wrote:
> >
> >
> > On 07/09/2015 02:15 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > >
> > > Normmally the common lane in a PHY channel gets powered up when some
> > > of the data lanes get powered up. But when we're driving port B with
> > > pipe B we don't want to enabled any of the data lanes, and just want
> > > the DPLL in the common lane to be active.
> > >
> > > To make that happens we have to temporarily enable some data lanes
> > > after which we can access the DPLL registers in the common lane. Once
> > > the pipe is up and running we can drop the power override on the data
> > > lanes allowing them to shut down. From this point forward the common
> > > lane will in fact stay powered on until the data lanes in the other
> > > channel get powered down.
> > Patch looks fine. It does what it says. One Q, why only for port B? Port
> > C is also in same common lane right?
>
> Port B is in the first PHY channel which also houses CL1. CL1 always
> powers up whenever any lanes in either PHY channel are powered up.
> CL2 only powers up if lanes in the second channel (ie. the one with
> port C) powers up.
>
> So in this scenario (pipe B->port B) we want the DPLL from CL2, but
> ideally we only want to power up the lanes for port B. Powering up
> port B lanes will only power up CL1, but as we need CL2 instead we
> need to, temporarily, power up some lanes in port C as well.
>
> Crossing the streams the other way (pipe A->port C) is not a problem
> since CL1 powers up whenever anything else powers up. So powering up
> some port C lanes is enough on its own to make the CL1 DPLL
> operational, even though CL1 and the lanes live in separate channels.
I added this to the commit message when merging, seemed useful for the
future.
-Daniel
>
> > Reviewed-by: Deepak S <deepak.s@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 23 +++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_drv.h | 3 +++
> > > drivers/gpu/drm/i915/intel_hdmi.c | 23 +++++++++++++++++++++++
> > > drivers/gpu/drm/i915/intel_runtime_pm.c | 29 +++++++++++++++++++++++++++++
> > > 4 files changed, 78 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 6058129..8d088f3 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -2865,6 +2865,12 @@ static void chv_pre_enable_dp(struct intel_encoder *encoder)
> > > mutex_unlock(&dev_priv->sb_lock);
> > >
> > > intel_enable_dp(encoder);
> > > +
> > > + /* Second common lane will stay alive on its own now */
> > > + if (dport->release_cl2_override) {
> > > + chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, false);
> > > + dport->release_cl2_override = false;
> > > + }
> > > }
> > >
> > > static void chv_dp_pre_pll_enable(struct intel_encoder *encoder)
> > > @@ -2882,6 +2888,14 @@ static void chv_dp_pre_pll_enable(struct intel_encoder *encoder)
> > >
> > > intel_dp_prepare(encoder);
> > >
> > > + /*
> > > + * Must trick the second common lane into life.
> > > + * Otherwise we can't even access the PLL.
> > > + */
> > > + if (ch == DPIO_CH0 && pipe == PIPE_B)
> > > + dport->release_cl2_override =
> > > + !chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, true);
> > > +
> > > chv_phy_powergate_lanes(encoder, true, lane_mask);
> > >
> > > mutex_lock(&dev_priv->sb_lock);
> > > @@ -2960,6 +2974,15 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder)
> > >
> > > mutex_unlock(&dev_priv->sb_lock);
> > >
> > > + /*
> > > + * Leave the power down bit cleared for at least one
> > > + * lane so that chv_powergate_phy_ch() will power
> > > + * on something when the channel is otherwise unused.
> > > + * When the port is off and the override is removed
> > > + * the lanes power down anyway, so otherwise it doesn't
> > > + * really matter what the state of power down bits is
> > > + * after this.
> > > + */
> > > chv_phy_powergate_lanes(encoder, false, 0x0);
> > > }
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > > index f8a16dc..6133a98 100644
> > > --- a/drivers/gpu/drm/i915/intel_drv.h
> > > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > > @@ -782,6 +782,7 @@ struct intel_digital_port {
> > > struct intel_dp dp;
> > > struct intel_hdmi hdmi;
> > > enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
> > > + bool release_cl2_override;
> > > };
> > >
> > > struct intel_dp_mst_encoder {
> > > @@ -1372,6 +1373,8 @@ void intel_display_set_init_power(struct drm_i915_private *dev, bool enable);
> > >
> > > void chv_phy_powergate_lanes(struct intel_encoder *encoder,
> > > bool override, unsigned int mask);
> > > +bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> > > + enum dpio_channel ch, bool override);
> > >
> > >
> > > /* intel_pm.c */
> > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > > index b3f6c9f..4b604ee 100644
> > > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > @@ -1625,6 +1625,14 @@ static void chv_hdmi_pre_pll_enable(struct intel_encoder *encoder)
> > >
> > > intel_hdmi_prepare(encoder);
> > >
> > > + /*
> > > + * Must trick the second common lane into life.
> > > + * Otherwise we can't even access the PLL.
> > > + */
> > > + if (ch == DPIO_CH0 && pipe == PIPE_B)
> > > + dport->release_cl2_override =
> > > + !chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, true);
> > > +
> > > chv_phy_powergate_lanes(encoder, true, 0x0);
> > >
> > > mutex_lock(&dev_priv->sb_lock);
> > > @@ -1701,6 +1709,15 @@ static void chv_hdmi_post_pll_disable(struct intel_encoder *encoder)
> > >
> > > mutex_unlock(&dev_priv->sb_lock);
> > >
> > > + /*
> > > + * Leave the power down bit cleared for at least one
> > > + * lane so that chv_powergate_phy_ch() will power
> > > + * on something when the channel is otherwise unused.
> > > + * When the port is off and the override is removed
> > > + * the lanes power down anyway, so otherwise it doesn't
> > > + * really matter what the state of power down bits is
> > > + * after this.
> > > + */
> > > chv_phy_powergate_lanes(encoder, false, 0x0);
> > > }
> > >
> > > @@ -1922,6 +1939,12 @@ static void chv_hdmi_pre_enable(struct intel_encoder *encoder)
> > > g4x_enable_hdmi(encoder);
> > >
> > > vlv_wait_port_ready(dev_priv, dport, 0x0);
> > > +
> > > + /* Second common lane will stay alive on its own now */
> > > + if (dport->release_cl2_override) {
> > > + chv_phy_powergate_ch(dev_priv, DPIO_PHY0, DPIO_CH1, false);
> > > + dport->release_cl2_override = false;
> > > + }
> > > }
> > >
> > > static void intel_hdmi_destroy(struct drm_connector *connector)
> > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > index 506a8cc..551cf08 100644
> > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > > @@ -1016,6 +1016,35 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
> > > phy, dev_priv->chv_phy_control);
> > > }
> > >
> > > +bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> > > + enum dpio_channel ch, bool override)
> > > +{
> > > + struct i915_power_domains *power_domains = &dev_priv->power_domains;
> > > + bool was_override;
> > > +
> > > + mutex_lock(&power_domains->lock);
> > > +
> > > + was_override = dev_priv->chv_phy_control & PHY_CH_POWER_DOWN_OVRD_EN(phy, ch);
> > > +
> > > + if (override == was_override)
> > > + goto out;
> > > +
> > > + if (override)
> > > + dev_priv->chv_phy_control |= PHY_CH_POWER_DOWN_OVRD_EN(phy, ch);
> > > + else
> > > + dev_priv->chv_phy_control &= ~PHY_CH_POWER_DOWN_OVRD_EN(phy, ch);
> > > +
> > > + I915_WRITE(DISPLAY_PHY_CONTROL, dev_priv->chv_phy_control);
> > > +
> > > + DRM_DEBUG_KMS("Power gating DPIO PHY%d CH%d (DPIO_PHY_CONTROL=0x%08x)\n",
> > > + phy, ch, dev_priv->chv_phy_control);
> > > +
> > > +out:
> > > + mutex_unlock(&power_domains->lock);
> > > +
> > > + return was_override;
> > > +}
> > > +
> > > void chv_phy_powergate_lanes(struct intel_encoder *encoder,
> > > bool override, unsigned int mask)
> > > {
> >
> > _______________________________________________
> > 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
