On Fri, Apr 19, 2013 at 11:14:32AM +0200, Daniel Vetter wrote: > This was somehow lost in the pipe_config->dpll introduction in > > commit f47709a9502f3715cc488b788ca91cf0c142b1b1 > Author: Daniel Vetter <daniel.vetter at ffwll.ch> > Date: Thu Mar 28 10:42:02 2013 +0100 > > drm/i915: create pipe_config->dpll for clock state > > While at it, extract a few small helpers for common computations. > > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch> > --- > drivers/gpu/drm/i915/intel_display.c | 39 +++++++++++++++++++++++++++--------- > 1 file changed, 29 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index ca2433b..5437a5e 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -561,6 +561,11 @@ static void pineview_clock(int refclk, intel_clock_t *clock) > clock->dot = clock->vco / clock->p; > } > > +static uint32_t i9xx_dpll_compute_m(struct dpll *dpll) > +{ > + return 5 * (dpll->m1 + 2) + (dpll->m2 + 2); > +} There are more places where this same computation appears. There could be more unification if we either used intel_clock_t everywhere instead if struct dpll, or if we embed struct dpll inside intel_clock_t. > + > static void intel_clock(struct drm_device *dev, int refclk, intel_clock_t *clock) > { > if (IS_PINEVIEW(dev)) { > @@ -4253,6 +4258,16 @@ static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc) > crtc->config.clock_set = true; > } > > +static uint32_t pnv_dpll_compute_fp(struct dpll *dpll) > +{ > + return (1 << dpll->n) << 16 | dpll->m1 << 8 | dpll->m2; > +} > + > +static uint32_t i9xx_dpll_compute_fp(struct dpll *dpll) > +{ > + return dpll->n << 16 | dpll->m1 << 8 | dpll->m2; > +} > + > static void i9xx_update_pll_dividers(struct intel_crtc *crtc, > intel_clock_t *reduced_clock) > { > @@ -4260,15 +4275,14 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc, > struct drm_i915_private *dev_priv = dev->dev_private; > int pipe = crtc->pipe; > u32 fp, fp2 = 0; > - struct dpll *clock = &crtc->config.dpll; > > if (IS_PINEVIEW(dev)) { > - fp = (1 << clock->n) << 16 | clock->m1 << 8 | clock->m2; > + fp = pnv_dpll_compute_fp(&crtc->config.dpll); > if (reduced_clock) > fp2 = (1 << reduced_clock->n) << 16 | > reduced_clock->m1 << 8 | reduced_clock->m2; > } else { > - fp = clock->n << 16 | clock->m1 << 8 | clock->m2; > + fp = i9xx_dpll_compute_fp(&crtc->config.dpll); > if (reduced_clock) > fp2 = reduced_clock->n << 16 | reduced_clock->m1 << 8 | > reduced_clock->m2; > @@ -5604,8 +5618,13 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc) > intel_cpu_transcoder_set_m_n(intel_crtc, &m_n); > } > > +static bool ironlake_needs_fb_cb_tune(struct dpll *dpll, int factor) > +{ > + return i9xx_dpll_compute_m(dpll) < factor * dpll->n; > +} > + > static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > - intel_clock_t *clock, u32 *fp, > + u32 *fp, > intel_clock_t *reduced_clock, u32 *fp2) > { > struct drm_crtc *crtc = &intel_crtc->base; > @@ -5645,7 +5664,7 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > } else if (is_sdvo && is_tv) > factor = 20; > > - if (clock->m < factor * clock->n) > + if (ironlake_needs_fb_cb_tune(&intel_crtc->config.dpll, factor)) > *fp |= FP_CB_TUNE; > > if (fp2 && (reduced_clock->m < factor * reduced_clock->n)) > @@ -5669,11 +5688,11 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc, > dpll |= DPLL_DVO_HIGH_SPEED; > > /* compute bitmask from p1 value */ > - dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT; > + dpll |= (1 << (intel_crtc->config.dpll.p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT; > /* also FPA1 */ > - dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT; > + dpll |= (1 << (intel_crtc->config.dpll.p1 - 1)) << DPLL_FPA1_P1_POST_DIV_SHIFT; > > - switch (clock->p2) { > + switch (intel_crtc->config.dpll.p2) { > case 5: > dpll |= DPLL_DAC_SERIAL_P2_CLOCK_DIV_5; > break; > @@ -5768,12 +5787,12 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, > if (intel_crtc->config.has_pch_encoder) { > struct intel_pch_pll *pll; > > - fp = clock.n << 16 | clock.m1 << 8 | clock.m2; > + fp = i9xx_dpll_compute_fp(&intel_crtc->config.dpll); > if (has_reduced_clock) > fp2 = reduced_clock.n << 16 | reduced_clock.m1 << 8 | > reduced_clock.m2; > > - dpll = ironlake_compute_dpll(intel_crtc, &clock, > + dpll = ironlake_compute_dpll(intel_crtc, > &fp, &reduced_clock, > has_reduced_clock ? &fp2 : NULL); > > -- > 1.7.11.7 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrj?l? Intel OTC