On Fri, Sep 13, 2013 at 03:30:51PM +0300, Jani Nikula wrote: > On Fri, 06 Sep 2013, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Extract the code to calculate the dotclock from the link clock and M/N > > values into a new function from ironlake_crtc_clock_get(). > > > > The new function can be used to calculate the dotclock for both FDI and > > DP cases. > > > > Also simplify the code a bit along the way. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++--------------------- > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > 2 files changed, 19 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index b3049a6..c393c8e 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -7410,16 +7410,9 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc, > > pipe_config->adjusted_mode.clock = clock.dot; > > } > > > > -static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > - struct intel_crtc_config *pipe_config) > > +int intel_dotclock_calculate(int link_freq, > > + const struct intel_link_m_n *m_n) > > { > > - struct drm_device *dev = crtc->base.dev; > > - struct drm_i915_private *dev_priv = dev->dev_private; > > - enum transcoder cpu_transcoder = pipe_config->cpu_transcoder; > > - int link_freq; > > - u64 clock; > > - u32 link_m, link_n; > > - > > /* > > * The calculation for the data clock is: > > * pixel_clock = ((m/n)*(link_clock * nr_lanes))/bpp > > @@ -7430,6 +7423,18 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > * link_clock = (m * link_clock) / n > > */ > > > > + if (!m_n->link_n) > > + return 0; > > + > > + return div_u64((u64)m_n->link_m * link_freq, m_n->link_n); > > +} > > + > > +static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > + struct intel_crtc_config *pipe_config) > > +{ > > + struct drm_device *dev = crtc->base.dev; > > + int link_freq; > > + > > /* > > * We need to get the FDI or DP link clock here to derive > > * the M/N dividers. > > @@ -7438,21 +7443,10 @@ static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > > * For DP, it's either 1.62GHz or 2.7GHz. > > * We do our calculations in 10*MHz since we don't need much precison. > > */ > > - if (pipe_config->has_pch_encoder) > > - link_freq = intel_fdi_link_freq(dev) * 10000; > > - else > > - link_freq = pipe_config->port_clock; > > The new code loses this distinction. I don't know if it matters. Please > explain. > > > - > > - link_m = I915_READ(PIPE_LINK_M1(cpu_transcoder)); > > - link_n = I915_READ(PIPE_LINK_N1(cpu_transcoder)); > > - > > - if (!link_m || !link_n) > > - return; > > - > > - clock = ((u64)link_m * (u64)link_freq); > > - do_div(clock, link_n); > > + link_freq = intel_fdi_link_freq(dev) * 10000; > > > > - pipe_config->adjusted_mode.clock = clock; > > + pipe_config->adjusted_mode.clock = > > + intel_dotclock_calculate(link_freq, &pipe_config->fdi_m_n); > > I'm not sure if the fear in unwarranted, but can we always be sure > fdi_m_n has been set? I seem to have jumped the gun a bit with this patch. Patch 9 will make it so that we don't call ironlake_crtc_clock_get() except in the PCH encoder case (could use a rename of the function at that point maybe). Before patch 9 we still call this guy for non-PCH cases and then it might fall apart a bit. As the dp_m_n extraction patch appears before this patch in the series, I think I can just make a slightly fixed version of this patch. Stay tuned for v2... -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx