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 <jbar...@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@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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to