On Tue, 21 Jan 2014 13:36:56 +0200 Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > +static int intel_ddi_calc_wrpll_link(u32 wrpll) > > +{ > > + int n, p, r; > > + > > + r = wrpll & WRPLL_DIVIDER_REF_MASK; > > + p = (wrpll & WRPLL_DIVIDER_POST_MASK) >> WRPLL_DIVIDER_POST_SHIFT; > > + n = (wrpll & WRPLL_DIVIDER_FB_MASK) >> WRPLL_DIVIDER_FB_SHIFT; > > + > > + return (LC_FREQ * n) / (p * r); > > This is assuming the WRPLL will use the LCPLL as reference. Ideally we > should read out the ref clock settings too. I don't think I see that in this config, but I've added code to look for that. Not sure if I got the ref freq right either; I think it's 135MHz in the PCH case... > > + case PORT_CLK_SEL_SPLL: > > + link_clock = 135000; > > SPLL could also output 810 MHz. And even 2700 MHz. Fixed. > > + break; > > + default: > > + WARN(1, "bad port clock sel\n"); > > + return; > > + } > > Could do the port_clock = link_clock * 2; here, and then pass port clock > to intel_dotclock_calculate() and avoid having to multiply crtc_clock by > 2 afterwards. Yeah that tidies things up nicely. Fixed. > As a side note, I must say it's a bit annoying that the DDI PLL code is > different to the rest of our PLL code. Hz vs. kHz etc. Makes it a bit > harder to figure out what it's doing. Yeah that could be converted, mixing them up definitely isn't ideal. > > + > > + if (pipe_config->has_pch_encoder) > > + pipe_config->adjusted_mode.crtc_clock = > > + intel_dotclock_calculate(link_clock, > > + &pipe_config->fdi_m_n); > > + else > > else if (has_dp_encoder) > > > + pipe_config->adjusted_mode.crtc_clock = > > + intel_dotclock_calculate(link_clock, > > + &pipe_config->dp_m_n); > > else > .crtc_clock = link_clock; // or port_clock if you take my suggestion above Fixed. > > + intel_cpu_transcoder_get_m_n(crtc, pipe_config->cpu_transcoder, > > + &pipe_config->dp_m_n); > > Not needed. We already do intel_dp_get_m_n() in intel_ddi_get_config(). Ah right, just above. Fixed. Thanks a lot for the review. -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx