On Thu, 28 Mar 2013 10:42:02 +0100
Daniel Vetter <daniel.vet...@ffwll.ch> wrote:

> Clock computations and handling are highly encoder specific, both in
> the optimal clock selection and also in which clocks to use and when
> sharing of clocks is possible.
> 
> So the best place to do this is somewhere in the encoders, with a
> generic fallback for those encoders without special needs. To facility
> this, add a pipe_config->clocks_set boolean.
> 
> This patch here is only prep work, it simply sets the computed clock
> values in pipe_config->dpll, and uses that data in the hw clock
> setting functions.
> 
> Haswell code isn't touched, simply because Haswell clocks work much
> different and need their own infrastructure (with probably a
> Haswell-specific config->ddi_clock substruct).
> 
> Signed-off-by: Daniel Vetter <daniel.vet...@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 155 
> +++++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  12 +++
>  2 files changed, 95 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index c9e873e..5319133 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4118,37 +4118,38 @@ static int i9xx_get_refclk(struct drm_crtc *crtc, int 
> num_connectors)
>       return refclk;
>  }
>  
> -static void i9xx_adjust_sdvo_tv_clock(struct drm_display_mode *adjusted_mode,
> -                                   intel_clock_t *clock)
> +static void i9xx_adjust_sdvo_tv_clock(struct intel_crtc *crtc)
>  {
> +     unsigned dotclock = crtc->config.adjusted_mode.clock;
> +     struct dpll *clock = &crtc->config.dpll;
> +
>       /* SDVO TV has fixed PLL values depend on its clock range,
>          this mirrors vbios setting. */
> -     if (adjusted_mode->clock >= 100000
> -         && adjusted_mode->clock < 140500) {
> +     if (dotclock >= 100000 && dotclock < 140500) {
>               clock->p1 = 2;
>               clock->p2 = 10;
>               clock->n = 3;
>               clock->m1 = 16;
>               clock->m2 = 8;
> -     } else if (adjusted_mode->clock >= 140500
> -                && adjusted_mode->clock <= 200000) {
> +     } else if (dotclock >= 140500 && dotclock <= 200000) {
>               clock->p1 = 1;
>               clock->p2 = 10;
>               clock->n = 6;
>               clock->m1 = 12;
>               clock->m2 = 8;
>       }
> +
> +     crtc->config.clock_set = true;
>  }
>  
> -static void i9xx_update_pll_dividers(struct drm_crtc *crtc,
> -                                  intel_clock_t *clock,
> +static void i9xx_update_pll_dividers(struct intel_crtc *crtc,
>                                    intel_clock_t *reduced_clock)
>  {
> -     struct drm_device *dev = crtc->dev;
> +     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 = 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;
> @@ -4164,11 +4165,11 @@ static void i9xx_update_pll_dividers(struct drm_crtc 
> *crtc,
>  
>       I915_WRITE(FP0(pipe), fp);
>  
> -     intel_crtc->lowfreq_avail = false;
> -     if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +     crtc->lowfreq_avail = false;
> +     if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>           reduced_clock && i915_powersave) {
>               I915_WRITE(FP1(pipe), fp2);
> -             intel_crtc->lowfreq_avail = true;
> +             crtc->lowfreq_avail = true;
>       } else {
>               I915_WRITE(FP1(pipe), fp);
>       }
> @@ -4182,14 +4183,11 @@ static void intel_dp_set_m_n(struct intel_crtc *crtc)
>               intel_cpu_transcoder_set_m_n(crtc, &crtc->config.dp_m_n);
>  }
>  
> -static void vlv_update_pll(struct drm_crtc *crtc,
> -                        intel_clock_t *clock, intel_clock_t *reduced_clock,
> -                        int num_connectors)
> +static void vlv_update_pll(struct intel_crtc *crtc)
>  {
> -     struct drm_device *dev = crtc->dev;
> +     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 = crtc->pipe;
>       u32 dpll, mdiv, pdiv;
>       u32 bestn, bestm1, bestm2, bestp1, bestp2;
>       bool is_sdvo;
> @@ -4197,8 +4195,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>       mutex_lock(&dev_priv->dpio_lock);
>  
> -     is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> -             intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> +     is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> +             intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>  
>       dpll = DPLL_VGA_MODE_DIS;
>       dpll |= DPLL_EXT_BUFFER_ENABLE_VLV;
> @@ -4208,11 +4206,11 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>       I915_WRITE(DPLL(pipe), dpll);
>       POSTING_READ(DPLL(pipe));
>  
> -     bestn = clock->n;
> -     bestm1 = clock->m1;
> -     bestm2 = clock->m2;
> -     bestp1 = clock->p1;
> -     bestp2 = clock->p2;
> +     bestn = crtc->config.dpll.n;
> +     bestm1 = crtc->config.dpll.m1;
> +     bestm2 = crtc->config.dpll.m2;
> +     bestp1 = crtc->config.dpll.p1;
> +     bestp2 = crtc->config.dpll.p2;
>  
>       /*
>        * In Valleyview PLL and program lane counter registers are exposed
> @@ -4244,8 +4242,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>  
>       intel_dpio_write(dev_priv, DPIO_FASTCLK_DISABLE, 0x620);
>  
> -     if (intel_crtc->config.has_dp_encoder)
> -             intel_dp_set_m_n(intel_crtc);
> +     if (crtc->config.has_dp_encoder)
> +             intel_dp_set_m_n(crtc);
>  
>       I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4256,8 +4254,8 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>       temp = 0;
>       if (is_sdvo) {
>               temp = 0;
> -             if (intel_crtc->config.pixel_multiplier > 1) {
> -                     temp = (intel_crtc->config.pixel_multiplier - 1)
> +             if (crtc->config.pixel_multiplier > 1) {
> +                     temp = (crtc->config.pixel_multiplier - 1)
>                               << DPLL_MD_UDI_MULTIPLIER_SHIFT;
>               }
>       }
> @@ -4265,16 +4263,15 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>       POSTING_READ(DPLL_MD(pipe));
>  
>       /* Now program lane control registers */
> -     if(intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)
> -                     || intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> -     {
> +     if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT)
> +        || intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI)) {
>               temp = 0x1000C4;
>               if(pipe == 1)
>                       temp |= (1 << 21);
>               intel_dpio_write(dev_priv, DPIO_DATA_CHANNEL1, temp);
>       }
> -     if(intel_pipe_has_type(crtc,INTEL_OUTPUT_EDP))
> -     {
> +
> +     if(intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_EDP)) {
>               temp = 0x1000C4;
>               if(pipe == 1)
>                       temp |= (1 << 21);
> @@ -4284,39 +4281,39 @@ static void vlv_update_pll(struct drm_crtc *crtc,
>       mutex_unlock(&dev_priv->dpio_lock);
>  }
>  
> -static void i9xx_update_pll(struct drm_crtc *crtc,
> -                         intel_clock_t *clock, intel_clock_t *reduced_clock,
> +static void i9xx_update_pll(struct intel_crtc *crtc,
> +                         intel_clock_t *reduced_clock,
>                           int num_connectors)
>  {
> -     struct drm_device *dev = crtc->dev;
> +     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);
>       struct intel_encoder *encoder;
> -     int pipe = intel_crtc->pipe;
> +     int pipe = crtc->pipe;
>       u32 dpll;
>       bool is_sdvo;
> +     struct dpll *clock = &crtc->config.dpll;
>  
> -     i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> +     i9xx_update_pll_dividers(crtc, reduced_clock);
>  
> -     is_sdvo = intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO) ||
> -             intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI);
> +     is_sdvo = intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_SDVO) ||
> +             intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_HDMI);
>  
>       dpll = DPLL_VGA_MODE_DIS;
>  
> -     if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
> +     if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS))
>               dpll |= DPLLB_MODE_LVDS;
>       else
>               dpll |= DPLLB_MODE_DAC_SERIAL;
>  
>       if (is_sdvo) {
> -             if ((intel_crtc->config.pixel_multiplier > 1) &&
> +             if ((crtc->config.pixel_multiplier > 1) &&
>                   (IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev))) {
> -                     dpll |= (intel_crtc->config.pixel_multiplier - 1)
> +                     dpll |= (crtc->config.pixel_multiplier - 1)
>                               << SDVO_MULTIPLIER_SHIFT_HIRES;
>               }
>               dpll |= DPLL_DVO_HIGH_SPEED;
>       }
> -     if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT))
> +     if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_DISPLAYPORT))
>               dpll |= DPLL_DVO_HIGH_SPEED;
>  
>       /* compute bitmask from p1 value */
> @@ -4344,13 +4341,13 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>       if (INTEL_INFO(dev)->gen >= 4)
>               dpll |= (6 << PLL_LOAD_PULSE_PHASE_SHIFT);
>  
> -     if (is_sdvo && intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> +     if (is_sdvo && intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>               dpll |= PLL_REF_INPUT_TVCLKINBC;
> -     else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_TVOUT))
> +     else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_TVOUT))
>               /* XXX: just matching BIOS for now */
>               /*      dpll |= PLL_REF_INPUT_TVCLKINBC; */
>               dpll |= 3;
> -     else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +     else if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>                intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>               dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>       else
> @@ -4361,12 +4358,12 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>       POSTING_READ(DPLL(pipe));
>       udelay(150);
>  
> -     for_each_encoder_on_crtc(dev, crtc, encoder)
> +     for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>               if (encoder->pre_pll_enable)
>                       encoder->pre_pll_enable(encoder);
>  
> -     if (intel_crtc->config.has_dp_encoder)
> -             intel_dp_set_m_n(intel_crtc);
> +     if (crtc->config.has_dp_encoder)
> +             intel_dp_set_m_n(crtc);
>  
>       I915_WRITE(DPLL(pipe), dpll);
>  
> @@ -4378,8 +4375,8 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>               u32 temp = 0;
>               if (is_sdvo) {
>                       temp = 0;
> -                     if (intel_crtc->config.pixel_multiplier > 1) {
> -                             temp = (intel_crtc->config.pixel_multiplier - 1)
> +                     if (crtc->config.pixel_multiplier > 1) {
> +                             temp = (crtc->config.pixel_multiplier - 1)
>                                       << DPLL_MD_UDI_MULTIPLIER_SHIFT;
>                       }
>               }
> @@ -4394,23 +4391,23 @@ static void i9xx_update_pll(struct drm_crtc *crtc,
>       }
>  }
>  
> -static void i8xx_update_pll(struct drm_crtc *crtc,
> +static void i8xx_update_pll(struct intel_crtc *crtc,
>                           struct drm_display_mode *adjusted_mode,
> -                         intel_clock_t *clock, intel_clock_t *reduced_clock,
> +                         intel_clock_t *reduced_clock,
>                           int num_connectors)
>  {
> -     struct drm_device *dev = crtc->dev;
> +     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);
>       struct intel_encoder *encoder;
> -     int pipe = intel_crtc->pipe;
> +     int pipe = crtc->pipe;
>       u32 dpll;
> +     struct dpll *clock = &crtc->config.dpll;
>  
> -     i9xx_update_pll_dividers(crtc, clock, reduced_clock);
> +     i9xx_update_pll_dividers(crtc, reduced_clock);
>  
>       dpll = DPLL_VGA_MODE_DIS;
>  
> -     if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS)) {
> +     if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS)) {
>               dpll |= (1 << (clock->p1 - 1)) << DPLL_FPA01_P1_POST_DIV_SHIFT;
>       } else {
>               if (clock->p1 == 2)
> @@ -4421,7 +4418,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>                       dpll |= PLL_P2_DIVIDE_BY_4;
>       }
>  
> -     if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) &&
> +     if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) &&
>                intel_panel_use_ssc(dev_priv) && num_connectors < 2)
>               dpll |= PLLB_REF_INPUT_SPREADSPECTRUMIN;
>       else
> @@ -4432,7 +4429,7 @@ static void i8xx_update_pll(struct drm_crtc *crtc,
>       POSTING_READ(DPLL(pipe));
>       udelay(150);
>  
> -     for_each_encoder_on_crtc(dev, crtc, encoder)
> +     for_each_encoder_on_crtc(dev, &crtc->base, encoder)
>               if (encoder->pre_pll_enable)
>                       encoder->pre_pll_enable(encoder);
>  
> @@ -4579,20 +4576,26 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>                                                   &clock,
>                                                   &reduced_clock);
>       }
> +     /* Compat-code for transition, will disappear. */
> +     if (!intel_crtc->config.clock_set) {
> +             intel_crtc->config.dpll.n = clock.n;
> +             intel_crtc->config.dpll.m1 = clock.m1;
> +             intel_crtc->config.dpll.m2 = clock.m2;
> +             intel_crtc->config.dpll.p1 = clock.p1;
> +             intel_crtc->config.dpll.p2 = clock.p2;
> +     }
>  
>       if (is_sdvo && is_tv)
> -             i9xx_adjust_sdvo_tv_clock(adjusted_mode, &clock);
> +             i9xx_adjust_sdvo_tv_clock(intel_crtc);
>  
>       if (IS_GEN2(dev))
> -             i8xx_update_pll(crtc, adjusted_mode, &clock,
> +             i8xx_update_pll(intel_crtc, adjusted_mode,
>                               has_reduced_clock ? &reduced_clock : NULL,
>                               num_connectors);
>       else if (IS_VALLEYVIEW(dev))
> -             vlv_update_pll(crtc, &clock,
> -                             has_reduced_clock ? &reduced_clock : NULL,
> -                             num_connectors);
> +             vlv_update_pll(intel_crtc);
>       else
> -             i9xx_update_pll(crtc, &clock,
> +             i9xx_update_pll(intel_crtc,
>                               has_reduced_clock ? &reduced_clock : NULL,
>                               num_connectors);
>  
> @@ -5221,7 +5224,7 @@ static bool ironlake_compute_clocks(struct drm_crtc 
> *crtc,
>       }
>  
>       if (is_sdvo && is_tv)
> -             i9xx_adjust_sdvo_tv_clock(adjusted_mode, clock);
> +             i9xx_adjust_sdvo_tv_clock(to_intel_crtc(crtc));
>  
>       return true;
>  }
> @@ -5525,6 +5528,14 @@ static int ironlake_crtc_mode_set(struct drm_crtc 
> *crtc,
>               DRM_ERROR("Couldn't find PLL settings for mode!\n");
>               return -EINVAL;
>       }
> +     /* Compat-code for transition, will disappear. */
> +     if (!intel_crtc->config.clock_set) {
> +             intel_crtc->config.dpll.n = clock.n;
> +             intel_crtc->config.dpll.m1 = clock.m1;
> +             intel_crtc->config.dpll.m2 = clock.m2;
> +             intel_crtc->config.dpll.p1 = clock.p1;
> +             intel_crtc->config.dpll.p2 = clock.p2;
> +     }
>  
>       /* Ensure that the cursor is valid for the new mode before changing... 
> */
>       intel_crtc_update_cursor(crtc, true);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 2a86a12..d27885a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -196,6 +196,18 @@ struct intel_crtc_config {
>        * accordingly. */
>       bool has_dp_encoder;
>       bool dither;
> +
> +     /* Controls for the clock computation, to override various stages. */
> +     bool clock_set;
> +
> +     /* Settings for the intel dpll used on pretty much everything but
> +      * haswell. */
> +     struct dpll {
> +             unsigned n;
> +             unsigned m1, m2;
> +             unsigned p1, p2;
> +     } dpll;
> +
>       int pipe_bpp;
>       struct intel_link_m_n dp_m_n;
>       /**

This one's hard to review since you mixed in a drm_crtc->intel_crtc
function arg change.

I'd rather have that split out, but it looks ok.

Reviewed-by: Jesse Barnes <jbar...@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to