Re: [PATCH 3/7] drm/i915: move dp clock computations to encoder->compute_config

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 19, 2013 at 11:14:33AM +0200, Daniel Vetter wrote:
> With the exception of hsw, which has dedicated DP clocks which run at
> the fixed frequency already, and vlv, which doesn't have optmized
> pre-defined dp clock parameters (yet).

So it looks like were still going through the full compute clocks thing,
which will possibly produce something different, and then we overwrite it
with the fixed clocks afterwards. I'm assuming that's just a transitional
issue and will get fixed later.

The only real concern I had was that the fixed clocks don't include the
derived m factor for example, but it looks like we shouldn't need that
except for the LVDS reduced clock thing.

So yeah, I think it should be OK.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> 
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 97 +-----------------------------------
>  drivers/gpu/drm/i915/intel_dp.c      | 45 +++++++++++++++++
>  2 files changed, 46 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5437a5e..d91bad8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -114,15 +114,6 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
>  			intel_clock_t *best_clock);
>  
>  static bool
> -intel_find_pll_g4x_dp(const intel_limit_t *, struct drm_crtc *crtc,
> -		      int target, int refclk, intel_clock_t *match_clock,
> -		      intel_clock_t *best_clock);
> -static bool
> -intel_find_pll_ironlake_dp(const intel_limit_t *, struct drm_crtc *crtc,
> -			   int target, int refclk, intel_clock_t *match_clock,
> -			   intel_clock_t *best_clock);
> -
> -static bool
>  intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  			int target, int refclk, intel_clock_t *match_clock,
>  			intel_clock_t *best_clock);
> @@ -254,20 +245,6 @@ static const intel_limit_t intel_limits_g4x_dual_channel_lvds = {
>  	.find_pll = intel_g4x_find_best_PLL,
>  };
>  
> -static const intel_limit_t intel_limits_g4x_display_port = {
> -	.dot = { .min = 161670, .max = 227000 },
> -	.vco = { .min = 1750000, .max = 3500000},
> -	.n = { .min = 1, .max = 2 },
> -	.m = { .min = 97, .max = 108 },
> -	.m1 = { .min = 0x10, .max = 0x12 },
> -	.m2 = { .min = 0x05, .max = 0x06 },
> -	.p = { .min = 10, .max = 20 },
> -	.p1 = { .min = 1, .max = 2},
> -	.p2 = { .dot_limit = 0,
> -		.p2_slow = 10, .p2_fast = 10 },
> -	.find_pll = intel_find_pll_g4x_dp,
> -};
> -
>  static const intel_limit_t intel_limits_pineview_sdvo = {
>  	.dot = { .min = 20000, .max = 400000},
>  	.vco = { .min = 1700000, .max = 3500000 },
> @@ -374,20 +351,6 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
>  	.find_pll = intel_g4x_find_best_PLL,
>  };
>  
> -static const intel_limit_t intel_limits_ironlake_display_port = {
> -	.dot = { .min = 25000, .max = 350000 },
> -	.vco = { .min = 1760000, .max = 3510000},
> -	.n = { .min = 1, .max = 2 },
> -	.m = { .min = 81, .max = 90 },
> -	.m1 = { .min = 12, .max = 22 },
> -	.m2 = { .min = 5, .max = 9 },
> -	.p = { .min = 10, .max = 20 },
> -	.p1 = { .min = 1, .max = 2},
> -	.p2 = { .dot_limit = 0,
> -		.p2_slow = 10, .p2_fast = 10 },
> -	.find_pll = intel_find_pll_ironlake_dp,
> -};
> -
>  static const intel_limit_t intel_limits_vlv_dac = {
>  	.dot = { .min = 25000, .max = 270000 },
>  	.vco = { .min = 4000000, .max = 6000000 },
> @@ -485,10 +448,7 @@ static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
>  			else
>  				limit = &intel_limits_ironlake_single_lvds;
>  		}
> -	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT) ||
> -		   intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))
> -		limit = &intel_limits_ironlake_display_port;
> -	else
> +	} else
>  		limit = &intel_limits_ironlake_dac;
>  
>  	return limit;
> @@ -509,8 +469,6 @@ static const intel_limit_t *intel_g4x_limit(struct drm_crtc *crtc)
>  		limit = &intel_limits_g4x_hdmi;
>  	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_SDVO)) {
>  		limit = &intel_limits_g4x_sdvo;
> -	} else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DISPLAYPORT)) {
> -		limit = &intel_limits_g4x_display_port;
>  	} else /* The option is for other outputs */
>  		limit = &intel_limits_i9xx_sdvo;
>  
> @@ -758,59 +716,6 @@ intel_g4x_find_best_PLL(const intel_limit_t *limit, struct drm_crtc *crtc,
>  }
>  
>  static bool
> -intel_find_pll_ironlake_dp(const intel_limit_t *limit, struct drm_crtc *crtc,
> -			   int target, int refclk, intel_clock_t *match_clock,
> -			   intel_clock_t *best_clock)
> -{
> -	struct drm_device *dev = crtc->dev;
> -	intel_clock_t clock;
> -
> -	if (target < 200000) {
> -		clock.n = 1;
> -		clock.p1 = 2;
> -		clock.p2 = 10;
> -		clock.m1 = 12;
> -		clock.m2 = 9;
> -	} else {
> -		clock.n = 2;
> -		clock.p1 = 1;
> -		clock.p2 = 10;
> -		clock.m1 = 14;
> -		clock.m2 = 8;
> -	}
> -	intel_clock(dev, refclk, &clock);
> -	memcpy(best_clock, &clock, sizeof(intel_clock_t));
> -	return true;
> -}
> -
> -/* DisplayPort has only two frequencies, 162MHz and 270MHz */
> -static bool
> -intel_find_pll_g4x_dp(const intel_limit_t *limit, struct drm_crtc *crtc,
> -		      int target, int refclk, intel_clock_t *match_clock,
> -		      intel_clock_t *best_clock)
> -{
> -	intel_clock_t clock;
> -	if (target < 200000) {
> -		clock.p1 = 2;
> -		clock.p2 = 10;
> -		clock.n = 2;
> -		clock.m1 = 23;
> -		clock.m2 = 8;
> -	} else {
> -		clock.p1 = 1;
> -		clock.p2 = 10;
> -		clock.n = 1;
> -		clock.m1 = 14;
> -		clock.m2 = 2;
> -	}
> -	clock.m = 5 * (clock.m1 + 2) + (clock.m2 + 2);
> -	clock.p = (clock.p1 * clock.p2);
> -	clock.dot = 96000 * clock.m / (clock.n + 2) / clock.p;
> -	clock.vco = 0;
> -	memcpy(best_clock, &clock, sizeof(intel_clock_t));
> -	return true;
> -}
> -static bool
>  intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  			int target, int refclk, intel_clock_t *match_clock,
>  			intel_clock_t *best_clock)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b55f3ec..b1a3a64 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -660,6 +660,49 @@ intel_dp_i2c_init(struct intel_dp *intel_dp,
>  	return ret;
>  }
>  
> +static void
> +intel_dp_set_clock(struct intel_encoder *encoder,
> +		   struct intel_crtc_config *pipe_config, int link_bw)
> +{
> +	struct drm_device *dev = encoder->base.dev;
> +
> +	if (IS_G4X(dev)) {
> +		if (link_bw == DP_LINK_BW_1_62) {
> +			pipe_config->dpll.p1 = 2;
> +			pipe_config->dpll.p2 = 10;
> +			pipe_config->dpll.n = 2;
> +			pipe_config->dpll.m1 = 23;
> +			pipe_config->dpll.m2 = 8;
> +		} else {
> +			pipe_config->dpll.p1 = 1;
> +			pipe_config->dpll.p2 = 10;
> +			pipe_config->dpll.n = 1;
> +			pipe_config->dpll.m1 = 14;
> +			pipe_config->dpll.m2 = 2;
> +		}
> +		pipe_config->clock_set = true;
> +	} else if (IS_HASWELL(dev)) {
> +		/* Haswell has special-purpose DP DDI clocks. */
> +	} else if (HAS_PCH_SPLIT(dev)) {
> +		if (link_bw == DP_LINK_BW_1_62) {
> +			pipe_config->dpll.n = 1;
> +			pipe_config->dpll.p1 = 2;
> +			pipe_config->dpll.p2 = 10;
> +			pipe_config->dpll.m1 = 12;
> +			pipe_config->dpll.m2 = 9;
> +		} else {
> +			pipe_config->dpll.n = 2;
> +			pipe_config->dpll.p1 = 1;
> +			pipe_config->dpll.p2 = 10;
> +			pipe_config->dpll.m1 = 14;
> +			pipe_config->dpll.m2 = 8;
> +		}
> +		pipe_config->clock_set = true;
> +	} else if (IS_VALLEYVIEW(dev)) {
> +		/* FIXME: Need to figure out optimized DP clocks for vlv. */
> +	}
> +}
> +
>  bool
>  intel_dp_compute_config(struct intel_encoder *encoder,
>  			struct intel_crtc_config *pipe_config)
> @@ -765,6 +808,8 @@ found:
>  	}
>  	pipe_config->pipe_bpp = bpp;
>  
> +	intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
> +
>  	return true;
>  }
>  
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC