Re: [PATCH 2/2] drm/i915: Move Valleyview DP DPLL divisor calc to intel_dp_set_clock v2

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Sep 04, 2013 at 01:30:38AM +0800, Chon Ming Lee wrote:
> For DP pll settings, there is only two golden configs.  Instead of
> running through the algorithm to determine it, hardcode the value and get it
> determine in intel_dp_set_clock.
> 
> v2: Rework on the intel_limit compiler warning. (Jani)
> 
> Signed-off-by: Chon Ming Lee <chon.ming.lee@intel.com>

lgtm, both patches merged. Note that checkpatch.pl was a bit unhappy about
the structure initalization layout, so I've fixed this up while applying.

I'm not too insisting on checkpatch clean code (especially the 80 char
limit for debug output often results in ugly code so is better ignored in
those cases). So please run it before submitting patches and fix up
reported issues (if it doesn't result in ugly code ofc).

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c |   20 +++-----------------
>  drivers/gpu/drm/i915/intel_dp.c      |   11 ++++++++++-
>  2 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f526ea9..1a567d2 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -339,19 +339,6 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
>  		.p2_slow = 2, .p2_fast = 20 },
>  };
>  
> -static const intel_limit_t intel_limits_vlv_dp = {
> -	.dot = { .min = 25000, .max = 270000 },
> -	.vco = { .min = 4000000, .max = 6000000 },
> -	.n = { .min = 1, .max = 7 },
> -	.m = { .min = 22, .max = 450 },
> -	.m1 = { .min = 2, .max = 3 },
> -	.m2 = { .min = 11, .max = 156 },
> -	.p = { .min = 10, .max = 30 },
> -	.p1 = { .min = 1, .max = 3 },
> -	.p2 = { .dot_limit = 270000,
> -		.p2_slow = 2, .p2_fast = 20 },
> -};
> -
>  static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
>  						int refclk)
>  {
> @@ -414,10 +401,8 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
>  	} else if (IS_VALLEYVIEW(dev)) {
>  		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
>  			limit = &intel_limits_vlv_dac;
> -		else if (intel_pipe_has_type(crtc, INTEL_OUTPUT_HDMI))
> -			limit = &intel_limits_vlv_hdmi;
>  		else
> -			limit = &intel_limits_vlv_dp;
> +			limit = &intel_limits_vlv_hdmi;
>  	} else if (!IS_GEN2(dev)) {
>  		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
>  			limit = &intel_limits_i9xx_lvds;
> @@ -4890,7 +4875,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  
>  	refclk = i9xx_get_refclk(crtc, num_connectors);
>  
> -	if (!is_dsi) {
> +	if (!is_dsi && !intel_crtc->config.clock_set) {
>  		/*
>  		 * Returns a set of divisors for the desired target clock with
>  		 * the given refclk, or FALSE.  The returned values represent
> @@ -4917,6 +4902,7 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
>  		 * by using the FP0/FP1. In such case we will disable the LVDS
>  		 * downclock feature.
>  		*/
> +		limit = intel_limit(crtc, refclk);
>  		has_reduced_clock =
>  			dev_priv->display.find_dpll(limit, crtc,
>  						    dev_priv->lvds_downclock,
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index fd09058..76b4372 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -59,6 +59,14 @@ static const struct dp_link_dpll pch_dpll[] =
>  		{ .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 }}
>  };
>  
> +static const struct dp_link_dpll vlv_dpll[] =
> +{
> +	{ DP_LINK_BW_1_62,
> +		{ .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 }},
> +	{ DP_LINK_BW_2_7,
> +		{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 }}
> +};
> +
>  /**
>   * is_edp - is the given port attached to an eDP panel (either CPU or PCH)
>   * @intel_dp: DP struct
> @@ -682,7 +690,8 @@ intel_dp_set_clock(struct intel_encoder *encoder,
>  		divisor = pch_dpll;
>  		count = ARRAY_SIZE(pch_dpll);
>  	} else if (IS_VALLEYVIEW(dev)) {
> -		/* FIXME: Need to figure out optimized DP clocks for vlv. */
> +		divisor = vlv_dpll;
> +		count = ARRAY_SIZE(vlv_dpll);
>  	}
>  
>  	if (divisor && count) {
> -- 
> 1.7.7.6
> 
> _______________________________________________
> 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