On Tue, May 14, 2013 at 05:08:27PM -0700, Jesse Barnes wrote: > We need this for comparing modes between configuration changes. > > v2: try harder to calulate non-simple pixel clocks (Daniel) > call get_clock after getting the encoder config, needed for pixel multiply > (Jesse) > v3: drop get_clock now that the pixel_multiply has been moved into > get_pipe_config > v4: re-add get_clock; we need to get the pixel multiplier in the > encoder, so need to calculate the clock value after the encoder's > get_config is called > v5: drop hsw clock_get, still needs to be written > > Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org> Two things: - the pixel_multiplier should be a separate patch, and I think we should add pipe_config check support for it. Maybe if we pick a default value of 1 (both for the get_config hw state readout and the compute config stuff in the modeset code) that should also simplify a bit since only SDVO uses it atm. We still need to implement pixel multiplier stuff for native hdmi ports ... - For the clock readout code I think we should be able to have pipe config compare support (with adjusted_mode->clock), with a bit of fuzz at least. Not on current dinq, but with my cleanup to give adjusted_mode->clock saner semantics: http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/21750 - There's no reason imo for the new ->get_clock vfunc, it splits exactly the same as the existing ->get_pipe_config. So could simply be included there. With those three adjustements I think this is ready to go. For the follow-up patches though I think we should guard them with a fastboot module option. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_crt.c | 1 + > drivers/gpu/drm/i915/intel_display.c | 78 +++++++++++++++++++++++++++++++--- > drivers/gpu/drm/i915/intel_dp.c | 8 ++++ > drivers/gpu/drm/i915/intel_drv.h | 3 ++ > drivers/gpu/drm/i915/intel_dvo.c | 1 + > drivers/gpu/drm/i915/intel_hdmi.c | 1 + > drivers/gpu/drm/i915/intel_lvds.c | 1 + > drivers/gpu/drm/i915/intel_sdvo.c | 24 +++++++++++ > 9 files changed, 111 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 14817de..2e284bb 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -323,6 +323,7 @@ struct drm_i915_display_funcs { > * fills out the pipe-config with the hw state. */ > bool (*get_pipe_config)(struct intel_crtc *, > struct intel_crtc_config *); > + void (*get_clock)(struct intel_crtc *, struct intel_crtc_config *); > int (*crtc_mode_set)(struct drm_crtc *crtc, > int x, int y, > struct drm_framebuffer *old_fb); > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > index 789c4ef..bd91e72 100644 > --- a/drivers/gpu/drm/i915/intel_crt.c > +++ b/drivers/gpu/drm/i915/intel_crt.c > @@ -101,6 +101,7 @@ static void intel_crt_get_config(struct intel_encoder *encoder, > flags |= DRM_MODE_FLAG_NVSYNC; > > pipe_config->adjusted_mode.flags |= flags; > + pipe_config->pixel_multiplier = 1; > } > > static void intel_disable_crt(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 163b97e..356404c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -45,6 +45,11 @@ bool intel_pipe_has_type(struct drm_crtc *crtc, int type); > static void intel_increase_pllclock(struct drm_crtc *crtc); > static void intel_crtc_update_cursor(struct drm_crtc *crtc, bool on); > > +static void i9xx_crtc_clock_get(struct intel_crtc *crtc, > + struct intel_crtc_config *pipe_config); > +static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > + struct intel_crtc_config *pipe_config); > + > typedef struct { > int min, max; > } intel_range_t; > @@ -6906,11 +6911,12 @@ void intel_release_load_detect_pipe(struct drm_connector *connector, > } > > /* Returns the clock of the currently programmed mode of the given pipe. */ > -static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc) > +static void i9xx_crtc_clock_get(struct intel_crtc *crtc, > + struct intel_crtc_config *pipe_config) > { > + struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int pipe = intel_crtc->pipe; > + int pipe = pipe_config->cpu_transcoder; > u32 dpll = I915_READ(DPLL(pipe)); > u32 fp; > intel_clock_t clock; > @@ -6949,7 +6955,8 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc) > default: > DRM_DEBUG_KMS("Unknown DPLL mode %08x in programmed " > "mode\n", (int)(dpll & DPLL_MODE_MASK)); > - return 0; > + pipe_config->adjusted_mode.clock = 0; > + return; > } > > /* XXX: Handle the 100Mhz refclk */ > @@ -6988,8 +6995,54 @@ static int intel_crtc_clock_get(struct drm_device *dev, struct drm_crtc *crtc) > * i830PllIsValid() because it relies on the xf86_config connector > * configuration being accurate, which it isn't necessarily. > */ > + pipe_config->adjusted_mode.clock = clock.dot; > +} > + > +static void ironlake_crtc_clock_get(struct intel_crtc *crtc, > + struct intel_crtc_config *pipe_config) > +{ > + 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, repeat; > + u64 clock; > + u32 link_m, link_n; > > - return clock.dot; > + repeat = pipe_config->pixel_multiplier; > + > + /* > + * The calculation for the data clock is: > + * pixel_clock = ((m/n)*(link_clock * nr_lanes * repeat))/bpp > + * But we want to avoid losing precison if possible, so: > + * pixel_clock = ((m * link_clock * nr_lanes * repeat)/(n*bpp)) > + * > + * and the link clock is simpler: > + * link_clock = (m * link_clock * repeat) / n > + */ > + > + /* > + * We need to get the FDI or DP link clock here to derive > + * the M/N dividers. > + * > + * For FDI, we read it from the BIOS or use a fixed 2.7GHz. > + * 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->cpu_edp_link_rate; > + > + 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 * (u64)repeat); > + do_div(clock, link_n); > + > + pipe_config->adjusted_mode.clock = clock; > } > > /** Returns the currently programmed mode of the given pipe. */ > @@ -7000,6 +7053,7 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > enum transcoder cpu_transcoder = intel_crtc->config.cpu_transcoder; > struct drm_display_mode *mode; > + struct intel_crtc_config pipe_config; > int htot = I915_READ(HTOTAL(cpu_transcoder)); > int hsync = I915_READ(HSYNC(cpu_transcoder)); > int vtot = I915_READ(VTOTAL(cpu_transcoder)); > @@ -7009,7 +7063,10 @@ struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, > if (!mode) > return NULL; > > - mode->clock = intel_crtc_clock_get(dev, crtc); > + pipe_config.cpu_transcoder = intel_crtc->pipe; > + i9xx_crtc_clock_get(intel_crtc, &pipe_config); > + > + mode->clock = pipe_config.adjusted_mode.clock; > mode->hdisplay = (htot & 0xffff) + 1; > mode->htotal = ((htot & 0xffff0000) >> 16) + 1; > mode->hsync_start = (hsync & 0xffff) + 1; > @@ -9011,6 +9068,7 @@ static void intel_init_display(struct drm_device *dev) > dev_priv->display.update_plane = ironlake_update_plane; > } else if (HAS_PCH_SPLIT(dev)) { > dev_priv->display.get_pipe_config = ironlake_get_pipe_config; > + dev_priv->display.get_clock = ironlake_crtc_clock_get; > dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set; > dev_priv->display.crtc_enable = ironlake_crtc_enable; > dev_priv->display.crtc_disable = ironlake_crtc_disable; > @@ -9018,6 +9076,7 @@ static void intel_init_display(struct drm_device *dev) > dev_priv->display.update_plane = ironlake_update_plane; > } else if (IS_VALLEYVIEW(dev)) { > dev_priv->display.get_pipe_config = i9xx_get_pipe_config; > + dev_priv->display.get_clock = i9xx_crtc_clock_get; > dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set; > dev_priv->display.crtc_enable = valleyview_crtc_enable; > dev_priv->display.crtc_disable = i9xx_crtc_disable; > @@ -9025,6 +9084,7 @@ static void intel_init_display(struct drm_device *dev) > dev_priv->display.update_plane = i9xx_update_plane; > } else { > dev_priv->display.get_pipe_config = i9xx_get_pipe_config; > + dev_priv->display.get_clock = i9xx_crtc_clock_get; > dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set; > dev_priv->display.crtc_enable = i9xx_crtc_enable; > dev_priv->display.crtc_disable = i9xx_crtc_disable; > @@ -9593,8 +9653,12 @@ setup_pipes: > if (encoder->get_hw_state(encoder, &pipe)) { > crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > encoder->base.crtc = &crtc->base; > - if (encoder->get_config) > + if (encoder->get_config && > + dev_priv->display.get_clock) { > encoder->get_config(encoder, &crtc->config); > + dev_priv->display.get_clock(crtc, > + &crtc->config); > + } > } else { > encoder->base.crtc = NULL; > } > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index a423256..c32429d 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1376,6 +1376,14 @@ static void intel_dp_get_config(struct intel_encoder *encoder, > flags |= DRM_MODE_FLAG_NVSYNC; > > pipe_config->adjusted_mode.flags |= flags; > + pipe_config->pixel_multiplier = 1; > + > + if (is_cpu_edp(intel_dp)) { > + if ((I915_READ(DP_A) & DP_PLL_FREQ_MASK) == DP_PLL_FREQ_160MHZ) > + pipe_config->cpu_edp_link_rate = 162000; > + else > + pipe_config->cpu_edp_link_rate = 270000; > + } > } > > static void intel_disable_dp(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 4802483..a7c9293 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -268,6 +268,9 @@ struct intel_crtc_config { > /* FDI configuration, only valid if has_pch_encoder is set. */ > int fdi_lanes; > struct intel_link_m_n fdi_m_n; > + > + /* CPU eDP config */ > + int cpu_edp_link_rate; > }; > > struct intel_crtc { > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c > index 91e9905..d020a9c 100644 > --- a/drivers/gpu/drm/i915/intel_dvo.c > +++ b/drivers/gpu/drm/i915/intel_dvo.c > @@ -147,6 +147,7 @@ static void intel_dvo_get_config(struct intel_encoder *encoder, > flags |= DRM_MODE_FLAG_NVSYNC; > > pipe_config->adjusted_mode.flags |= flags; > + pipe_config->pixel_multiplier = 1; > } > > static void intel_disable_dvo(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c > index 9091655..7b55a8a 100644 > --- a/drivers/gpu/drm/i915/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/intel_hdmi.c > @@ -678,6 +678,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder, > flags |= DRM_MODE_FLAG_NVSYNC; > > pipe_config->adjusted_mode.flags |= flags; > + pipe_config->pixel_multiplier = 1; > } > > static void intel_enable_hdmi(struct intel_encoder *encoder) > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 6554860..f0cbb02 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -109,6 +109,7 @@ static void intel_lvds_get_config(struct intel_encoder *encoder, > flags |= DRM_MODE_FLAG_PVSYNC; > > pipe_config->adjusted_mode.flags |= flags; > + pipe_config->pixel_multiplier = 1; > } > > /* The LVDS pin pair needs to be on before the DPLLs are enabled. > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c > index 416d23c..ee7f0ab 100644 > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -780,6 +780,11 @@ static bool intel_sdvo_set_clock_rate_mult(struct intel_sdvo *intel_sdvo, u8 val > return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_CLOCK_RATE_MULT, &val, 1); > } > > +static bool intel_sdvo_get_clock_rate_mult(struct intel_sdvo *intel_sdvo, u8 *val) > +{ > + return intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_CLOCK_RATE_MULT, &val, 1); > +} > + > static void intel_sdvo_get_dtd_from_mode(struct intel_sdvo_dtd *dtd, > const struct drm_display_mode *mode) > { > @@ -1315,6 +1320,7 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, > struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base); > struct intel_sdvo_dtd dtd; > u32 flags = 0; > + u8 val; > bool ret; > > ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd); > @@ -1334,6 +1340,24 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder, > flags |= DRM_MODE_FLAG_NVSYNC; > > pipe_config->adjusted_mode.flags |= flags; > + > + ret = intel_sdvo_get_clock_rate_mult(intel_sdvo, &val); > + if (!ret) { > + DRM_DEBUG_DRIVER("failed to retrieve clock rate multiplier\n"); > + return; > + } > + > + switch (val) { > + case SDVO_CLOCK_RATE_MULT_1X: > + pipe_config->pixel_multiplier = 1; > + break; > + case SDVO_CLOCK_RATE_MULT_2X: > + pipe_config->pixel_multiplier = 2; > + break; > + case SDVO_CLOCK_RATE_MULT_4X: > + pipe_config->pixel_multiplier = 4; > + break; > + } > } > > static void intel_disable_sdvo(struct intel_encoder *encoder) > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch