Re: [PATCH 13/28] drm/i915: fix VLV limits and m/n/p calculations

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

On Fri, Mar 01, 2013 at 01:14:16PM -0800, Jesse Barnes wrote:
> For high res modes m n p calculation is fixed for VLV platform.
> 
> Signed-off-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com>
> Signed-off-by: Pallavi G <pallavi.g@intel.com>
> Signed-off-by: Yogesh M <yogesh.mohan.marimuthu@intel.com>
> Signed-off-by: Gajanan Bhat <gajanan.bhat@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |   31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3b189fa..fd4a0d4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -380,21 +380,21 @@ static const intel_limit_t intel_limits_vlv_dac = {
>  	.m1 = { .min = 2, .max = 3 },
>  	.m2 = { .min = 11, .max = 156 },
>  	.p = { .min = 10, .max = 30 },
> -	.p1 = { .min = 2, .max = 3 },
> +	.p1 = { .min = 1, .max = 3 },
>  	.p2 = { .dot_limit = 270000,
>  		.p2_slow = 2, .p2_fast = 20 },
>  	.find_pll = intel_vlv_find_best_pll,
>  };
>  
>  static const intel_limit_t intel_limits_vlv_hdmi = {
> -	.dot = { .min = 20000, .max = 165000 },
> -	.vco = { .min = 4000000, .max = 5994000},
> -	.n = { .min = 1, .max = 7 },
> +	.dot = { .min = 25000, .max = 180000 },
> +	.vco = { .min = 4040000, .max = 5960000 },
> +	.n = { .min = 1, .max = 5 },
>  	.m = { .min = 60, .max = 300 }, /* guess */
>  	.m1 = { .min = 2, .max = 3 },
> -	.m2 = { .min = 11, .max = 156 },
> +	.m2 = { .min = 15, .max = 149 },
>  	.p = { .min = 10, .max = 30 },
> -	.p1 = { .min = 2, .max = 3 },
> +	.p1 = { .min = 1, .max = 3 },
>  	.p2 = { .dot_limit = 270000,
>  		.p2_slow = 2, .p2_fast = 20 },
>  	.find_pll = intel_vlv_find_best_pll,
> @@ -408,7 +408,7 @@ static const intel_limit_t intel_limits_vlv_dp = {
>  	.m1 = { .min = 2, .max = 3 },
>  	.m2 = { .min = 11, .max = 156 },
>  	.p = { .min = 10, .max = 30 },
> -	.p1 = { .min = 2, .max = 3 },
> +	.p1 = { .min = 1, .max = 3 },
>  	.p2 = { .dot_limit = 270000,
>  		.p2_slow = 2, .p2_fast = 20 },
>  	.find_pll = intel_vlv_find_best_pll,
> @@ -809,10 +809,14 @@ 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)
>  {
> +#define LONG_OVERFLOW 0x7FFFFFFF
> +#define DIFF_OVERFLOW (LONG_OVERFLOW/10000)
> +
>  	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
>  	u32 m, n, fastclk;
>  	u32 updrate, minupdate, fracbits, p;
> -	unsigned long bestppm, ppm, absppm;
> +	long bestppm, ppm, absppm, ppmdiff, absppmdiff;
> +	unsigned long ulMult = 1;
>  	int dotclk, flag;
>  
>  	flag = 0;
> @@ -841,8 +845,15 @@ intel_vlv_find_best_pll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  					m = m1 * m2;
>  					vco = updrate * m;
>  					if (vco >= limit->vco.min && vco < limit->vco.max) {
> -						ppm = 1000000 * ((vco / p) - fastclk) / fastclk;
> -						absppm = (ppm > 0) ? ppm : (-ppm);
> +						ppmdiff = ((100*vco)/p) - (100*fastclk);
> +						absppmdiff = (ppmdiff > 0) ? ppmdiff : (-ppmdiff);
> +						ulMult = 1;
> +						while (absppmdiff > DIFF_OVERFLOW) {
> +							absppmdiff /= 10;
> +							ulMult *= 10;
> +						}
> +						absppm = ((absppmdiff*10000)/fastclk)*ulMult;
> +

This seems to expect long to be >32 bits. If it needs 64bit math it
should use proper sized types, and of course the division needs to
use something like div_s64().

Also it should use abs()/abs64() instead of the open-coded versions.
That would also allow getting rid of some of these temporary variables.

>  						if (absppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
>  							bestppm = 0;
>  							flag = 1;
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC