On ti, 2016-05-24 at 13:22 +0300, Ville Syrjälä wrote:
> On Tue, May 24, 2016 at 12:27:50PM +0300, Imre Deak wrote:
> > If the CDCLK PLL isn't locked we can just assume that it's off resulting
> > in fully re-initializing both CDCLK PLL and CDCLK dividers. This way the
> > CDCLK PLL sanitization added in the following patch can be done on BXT
> > the same way as it's done on SKL.
> >
> > Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++------------
> > 1 file changed, 11 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c1e666b..b8e5995 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -5461,14 +5461,14 @@ skl_dpll0_update(struct drm_i915_private *dev_priv)
> > u32 val;
> >
> > dev_priv->cdclk_pll.ref = 24000;
> > + dev_priv->cdclk_pll.vco = 0;
> >
> > val = I915_READ(LCPLL1_CTL);
> > - if ((val & LCPLL_PLL_ENABLE) == 0) {
> > - dev_priv->cdclk_pll.vco = 0;
> > + if ((val & LCPLL_PLL_ENABLE) == 0)
> > return;
> > - }
> >
> > - WARN_ON((val & LCPLL_PLL_LOCK) == 0);
> > + if (WARN_ON((val & LCPLL_PLL_LOCK) == 0))
> > + return;
> >
> > val = I915_READ(DPLL_CTRL1);
> >
> > @@ -5690,9 +5690,10 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > if ((I915_READ(SWF_ILK(0x18)) & 0x00FFFFFF) == 0)
> > goto sanitize;
> >
> > + intel_update_cdclk(dev_priv->dev);
> > /* Is PLL enabled and locked ? */
> > - if ((I915_READ(LCPLL1_CTL) & (LCPLL_PLL_ENABLE | LCPLL_PLL_LOCK)) !=
> > - (LCPLL_PLL_ENABLE | LCPLL_PLL_LOCK))
> > + if (!dev_priv->cdclk_pll.vco ||
>
> == 0 please. I find that more pleasing to the eye when we end up mixing
> with == anyway on the next line.
Ok.
> Actually is there any extra benefit from the cdclk_freq check? As
> in would vco==0 be sufficient on its own?
The other check is for the case of an invalid CDCLK divider setting.
Don't we care about that?
> > + dev_priv->cdclk_freq == dev_priv->cdclk_pll.ref)
> > goto sanitize;
> >
> > if ((I915_READ(DPLL_CTRL1) & (DPLL_CTRL1_HDMI_MODE(SKL_DPLL0) |
>
> Maybe toss out this DPLL_CTRL1 check that I added as well then, and have
> skl_dpll0_update() set the vco to 0 when it's crap. If we ever actually
> hit this in the real world, we'll get the warn, and then we perhaps get
> to rethink this stuff, but for now simpler seems better.
Ok, makes sense.
> > @@ -5701,8 +5702,6 @@ static void skl_sanitize_cdclk(struct drm_i915_private *dev_priv)
> > DPLL_CTRL1_OVERRIDE(SKL_DPLL0))
> > goto sanitize;
> >
> > - intel_update_cdclk(dev_priv->dev);
> > -
> > /* DPLL okay; verify the cdclock
> > *
> > * Noticed in some instances that the freq selection is correct but
> > @@ -6608,14 +6607,14 @@ static void bxt_de_pll_update(struct drm_i915_private *dev_priv)
> > u32 val;
> >
> > dev_priv->cdclk_pll.ref = 19200;
> > + dev_priv->cdclk_pll.vco = 0;
> >
> > val = I915_READ(BXT_DE_PLL_ENABLE);
> > - if ((val & BXT_DE_PLL_PLL_ENABLE) == 0) {
> > - dev_priv->cdclk_pll.vco = 0;
> > + if ((val & BXT_DE_PLL_PLL_ENABLE) == 0)
> > return;
> > - }
> >
> > - WARN_ON((val & BXT_DE_PLL_LOCK) == 0);
> > + if (WARN_ON((val & BXT_DE_PLL_LOCK) == 0))
> > + return;
> >
> > val = I915_READ(BXT_DE_PLL_CTL);
> > dev_priv->cdclk_pll.vco = (val & BXT_DE_PLL_RATIO_MASK) *
> > --
> > 2.5.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
