On Thu, Aug 27, 2015 at 10:06:09AM +0530, Deepak wrote:
>
>
> On 07/09/2015 02:16 AM, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Add some checks that the state of the DPIO lanes is more or less what we
> > expect based on the overrides.
> >
> > The hardware only provides two bits per channel indicating whether all
> > or some of the lanes are powered down, so we can't do an exact check.
> >
> > Additionally, CL2 powering down before we can check it adds another
> > twist. To work around this we simply check for the 0 value of the
> > CL2 register (which is what we get when it's powered down) and
> > adjust our expectations.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 8 +++++
> > drivers/gpu/drm/i915/intel_runtime_pm.c | 54 +++++++++++++++++++++++++++++++++
> > 2 files changed, 62 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 395f556..586a0f7 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -1089,6 +1089,12 @@ enum skl_disp_power_wells {
> > #define DPIO_CHV_INT_LOCK_THRESHOLD_SEL_COARSE 1 /* 1: coarse & 0 : fine */
> > #define CHV_PLL_DW9(ch) _PIPE(ch, _CHV_PLL_DW9_CH0, _CHV_PLL_DW9_CH1)
> >
> > +#define _CHV_CMN_DW0_CH0 0x8100
> > +#define DPIO_ALLDL_POWERDOWN_SHIFT_CH0 19
> > +#define DPIO_ANYDL_POWERDOWN_SHIFT_CH0 18
> > +#define DPIO_ALLDL_POWERDOWN (1 << 1)
> > +#define DPIO_ANYDL_POWERDOWN (1 << 0)
> > +
> > #define _CHV_CMN_DW5_CH0 0x8114
> > #define CHV_BUFRIGHTENA1_DISABLE (0 << 20)
> > #define CHV_BUFRIGHTENA1_NORMAL (1 << 20)
> > @@ -1125,6 +1131,8 @@ enum skl_disp_power_wells {
> >
> > #define _CHV_CMN_DW19_CH0 0x814c
> > #define _CHV_CMN_DW6_CH1 0x8098
> > +#define DPIO_ALLDL_POWERDOWN_SHIFT_CH1 30 /* CL2 DW6 only */
> > +#define DPIO_ANYDL_POWERDOWN_SHIFT_CH1 29 /* CL2 DW6 only */
> > #define DPIO_DYNPWRDOWNEN_CH1 (1 << 28) /* CL2 DW6 only */
> > #define CHV_CMN_USEDCLKCHANNEL (1 << 13)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 002b78f..a1d9676 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1026,6 +1026,58 @@ static void chv_dpio_cmn_power_well_disable(struct drm_i915_private *dev_priv,
> > phy, dev_priv->chv_phy_control);
> > }
> >
> > +static void assert_chv_phy_powergate(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> > + enum dpio_channel ch, bool override, unsigned int mask)
> > +{
> > + enum pipe pipe = phy == DPIO_PHY0 ? PIPE_A : PIPE_C;
> Why not PIPE B? I think PIPE B can also be used to drive using PHY0?
Here we only pass 'pipe' to vlv_dpio_read() and it will just use
'DPIO_PHY(pipe)' to turn it back into an enum dpio_phy :) Since pipe A
and B are hooked up the same phy we don't need pipe B here.
I've been meaning to change vlv_dpio_read/write() to take enum dpio_phy
instead of enum pipe to get rid of this kind of sillyness, but haven't
gotten around to it yet. There may also be a few other functions that
could use the same treatment.
> > + u32 reg, val, expected, actual;
> > +
> > + if (ch == DPIO_CH0)
> > + reg = _CHV_CMN_DW0_CH0;
> > + else
> > + reg = _CHV_CMN_DW6_CH1;
> > +
> > + mutex_lock(&dev_priv->sb_lock);
> > + val = vlv_dpio_read(dev_priv, pipe, reg);
> > + mutex_unlock(&dev_priv->sb_lock);
> > +
> > + /*
> > + * This assumes !override is only used when the port is disabled.
> > + * All lanes should power down even without the override when
> > + * the port is disabled.
> > + */
> > + if (!override || mask == 0xf) {
> > + expected = DPIO_ALLDL_POWERDOWN | DPIO_ANYDL_POWERDOWN;
> > + /*
> > + * If CH1 common lane is not active anymore
> > + * (eg. for pipe B DPLL) the entire channel will
> > + * shut down, which causes the common lane registers
> > + * to read as 0. That means we can't actually check
> > + * the lane power down status bits, but as the entire
> > + * register reads as 0 it's a good indication that the
> > + * channel is indeed entirely powered down.
> > + */
> > + if (ch == DPIO_CH1 && val == 0)
> > + expected = 0;
> > + } else if (mask != 0x0) {
> > + expected = DPIO_ANYDL_POWERDOWN;
> > + } else {
> > + expected = 0;
> > + }
> > +
> > + if (ch == DPIO_CH0)
> > + actual = val >> DPIO_ANYDL_POWERDOWN_SHIFT_CH0;
> > + else
> > + actual = val >> DPIO_ANYDL_POWERDOWN_SHIFT_CH1;
> > + actual &= DPIO_ALLDL_POWERDOWN | DPIO_ANYDL_POWERDOWN;
> > +
> > + WARN(actual != expected,
> > + "Unexpected DPIO lane power down: all %d, any %d. Expected: all %d, any %d. (0x%x = 0x%08x)\n",
> > + !!(actual & DPIO_ALLDL_POWERDOWN), !!(actual & DPIO_ANYDL_POWERDOWN),
> > + !!(expected & DPIO_ALLDL_POWERDOWN), !!(expected & DPIO_ANYDL_POWERDOWN),
> > + reg, val);
> > +}
> > +
> > bool chv_phy_powergate_ch(struct drm_i915_private *dev_priv, enum dpio_phy phy,
> > enum dpio_channel ch, bool override)
> > {
> > @@ -1078,6 +1130,8 @@ void chv_phy_powergate_lanes(struct intel_encoder *encoder,
> > DRM_DEBUG_KMS("Power gating DPIO PHY%d CH%d lanes 0x%x (PHY_CONTROL=0x%08x)\n",
> > phy, ch, mask, dev_priv->chv_phy_control);
> >
> > + assert_chv_phy_powergate(dev_priv, phy, ch, override, mask);
> > +
> > mutex_unlock(&power_domains->lock);
> > }
> >
>
> _______________________________________________
> 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
