Re: [PATCH v2] drm/i915: Make i9xx_crtc_clock_get() work for PCH DPLLs

Ville Syrjälä
2013-09-13T13:06:35Z

On Fri, Sep 13, 2013 at 04:04:03PM +0300, Jani Nikula wrote:
> On Mon, 09 Sep 2013, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Add the 120MHz refernce clock case for PCH DPLLs.
> >
> > Also determine the reference clock frequency more accurately by
> > checking for the PLLB_REF_INPUT_SPREADSPECTRUMIN refclk input
> > mode. The gen2 code already checked it, but it stil assumed a
> > fixed 66MHz refclk. Instead we need to consult the VBT for the
> > real value.
> >
> > v2: Fix refclk for SSC panel case
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 32 +++++++++++++++++++++-----------
> > 1 file changed, 21 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 754de85..4f07292 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -7324,6 +7324,22 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
> > mutex_unlock(&crtc->mutex);
> > }
> >
> > +static int i9xx_pll_refclk(struct drm_device *dev,
> > + const struct intel_crtc_config *pipe_config)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + u32 dpll = pipe_config->dpll_hw_state.dpll;
> > +
> > + if ((dpll & PLL_REF_INPUT_MASK) == PLLB_REF_INPUT_SPREADSPECTRUMIN)
>
> This seems wrong for at least gen3 and vlv. And it's a bit scary to go
> change gen2 but oh well...

Why? i8xx_update_pll(), i9xx_update_pll() and ironlake_compute_dpll()
all have the same logic for setting that bit.

For VLV I agree. But the clock readout there is totally busted anyway,
so I don't care at this point.

>
> Jani.
>
> > + return dev_priv->vbt.lvds_ssc_freq * 1000;
> > + else if (HAS_PCH_SPLIT(dev))
> > + return 120000;
> > + else if (!IS_GEN2(dev))
> > + return 96000;
> > + else
> > + return 48000;
> > +}
> > +
> > /* Returns the clock of the currently programmed mode of the given pipe. */
> > static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> > struct intel_crtc_config *pipe_config)
> > @@ -7334,6 +7350,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> > u32 dpll = pipe_config->dpll_hw_state.dpll;
> > u32 fp;
> > intel_clock_t clock;
> > + int refclk = i9xx_pll_refclk(dev, pipe_config);
> >
> > if ((dpll & DISPLAY_RATE_SELECT_FPA1) == 0)
> > fp = pipe_config->dpll_hw_state.fp0;
> > @@ -7373,9 +7390,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> > }
> >
> > if (IS_PINEVIEW(dev))
> > - pineview_clock(96000, &clock);
> > + pineview_clock(refclk, &clock);
> > else
> > - i9xx_clock(96000, &clock);
> > + i9xx_clock(refclk, &clock);
> > } else {
> > bool is_lvds = (pipe == 1) && (I915_READ(LVDS) & LVDS_PORT_EN);
> >
> > @@ -7383,13 +7400,6 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> > clock.p1 = ffs((dpll & DPLL_FPA01_P1_POST_DIV_MASK_I830_LVDS) >>
> > DPLL_FPA01_P1_POST_DIV_SHIFT);
> > clock.p2 = 14;
> > -
> > - if ((dpll & PLL_REF_INPUT_MASK) ==
> > - PLLB_REF_INPUT_SPREADSPECTRUMIN) {
> > - /* XXX: might not be 66MHz */
> > - i9xx_clock(66000, &clock);
> > - } else
> > - i9xx_clock(48000, &clock);
> > } else {
> > if (dpll & PLL_P1_DIVIDE_BY_TWO)
> > clock.p1 = 2;
> > @@ -7401,9 +7411,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
> > clock.p2 = 4;
> > else
> > clock.p2 = 2;
> > -
> > - i9xx_clock(48000, &clock);
> > }
> > +
> > + i9xx_clock(refclk, &clock);
> > }
> >
> > pipe_config->adjusted_mode.clock = clock.dot;
> > --
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center

--
Ville Syrjälä
Intel OTC