[PATCH 00/14] drm/i915: VLV DPLL calc fixes and cleanups

[PATCH 00/14] drm/i915: VLV DPLL calc fixes and cleanups

[ville.syrjala]

I only wanted to do some small cleanups to vlv_find_best_dpll(),
but it seems I went slightly mad again.

After realizing that I have to cook up quite a few more patches,
I decided to also split up the functional changes from my earlier
vlv_find_best_dpll() rewrite patch. And this series is the result.

And the usual warning applies: Totally untested

Ville Syrjälä (14):
      drm/i915: Eliminate one indent leel from vlv_find_best_dpll
      drm/i915: Use DIV_ROUND_CLOSEST()
      drm/i915: Make vlv_find_best_dpll() ppm calculation safe
      drm/i915: Don't underflow bestppm
      drm/i915: Rewrite vlv_find_best_dpll()
      drm/i915: De-magic the VLV p2 divider step size
      drm/i915: Make sure we respect n.max on VLV
      drm/i915: Clarify VLV PLL p1 limits
      drm/i915: Allow p1 divider 2 on VLV
      drm/i915: Respect p2 divider minimum limit on VLV
      drm/i915: Remove the unused p and m limits for VLV
      drm/i915: Remove unused dot_limit from VLV PLL limits
      drm/i915: intel_limits_vlv_dac and intel_limits_vlv_hdmi are the same
      drm/i915: Don't lie about findind suitable PLL settings on VLV

 drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++++++++++++++----------------------------------------------------
 1 file changed, 49 insertions(+), 74 deletions(-)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/14] drm/i915: Eliminate one indent leel from vlv_find_best_dpll

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use 'continue' to get rid of one indent level in vlv_find_best_dpll()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 40 +++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 31eac15..fca56fc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -700,25 +700,27 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 					       refclk) / (2*refclk));
 					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);
-						if (absppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
-							bestppm = 0;
-							flag = 1;
-						}
-						if (absppm < bestppm - 10) {
-							bestppm = absppm;
-							flag = 1;
-						}
-						if (flag) {
-							bestn = n;
-							bestm1 = m1;
-							bestm2 = m2;
-							bestp1 = p1;
-							bestp2 = p2;
-							flag = 0;
-						}
+
+					if (vco < limit->vco.min || vco >= limit->vco.max)
+						continue;
+
+					ppm = 1000000 * ((vco / p) - fastclk) / fastclk;
+					absppm = (ppm > 0) ? ppm : (-ppm);
+					if (absppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
+						bestppm = 0;
+						flag = 1;
+					}
+					if (absppm < bestppm - 10) {
+						bestppm = absppm;
+						flag = 1;
+					}
+					if (flag) {
+						bestn = n;
+						bestm1 = m1;
+						bestm2 = m2;
+						bestp1 = p1;
+						bestp2 = p2;
+						flag = 0;
 					}
 				}
 			}
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 01/14] drm/i915: Eliminate one indent leel from vlv_find_best_dpll

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

s/leel/level in subject

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Use 'continue' to get rid of one indent level in vlv_find_best_dpll()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/14] drm/i915: Use DIV_ROUND_CLOSEST()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

vlv_find_best_dpll() has an open coded DIV_ROUND_CLOSEST(). Replace it
with the real thing.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fca56fc..4b1af94 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -696,8 +696,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 				p = p1 * p2;
 				/* based on hardware requirement, prefer bigger m1,m2 values */
 				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
-					m2 = (((2*(fastclk * p * n / m1 )) +
-					       refclk) / (2*refclk));
+					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
 					m = m1 * m2;
 					vco = updrate * m;
 
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/14] drm/i915: Use DIV_ROUND_CLOSEST()

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> vlv_find_best_dpll() has an open coded DIV_ROUND_CLOSEST(). Replace it
> with the real thing.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fca56fc..4b1af94 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -696,8 +696,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  				p = p1 * p2;
>  				/* based on hardware requirement, prefer bigger m1,m2 values */
>  				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
> -					m2 = (((2*(fastclk * p * n / m1 )) +
> -					       refclk) / (2*refclk));
> +					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
>  					m = m1 * m2;
>  					vco = updrate * m;
>  
> -- 
> 1.8.1.5

Not a problem with this patch but perhaps consideration for further
cleanups: target and refclk should be u32 and further down the line
the crtc_config.clock and xxx_get_refclk() also.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 02/14] drm/i915: Use DIV_ROUND_CLOSEST()

[Daniel Vetter]

On Thu, Sep 26, 2013 at 01:09:26PM +0300, Mika Kuoppala wrote:
> ville.syrjala@linux.intel.com writes:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > vlv_find_best_dpll() has an open coded DIV_ROUND_CLOSEST(). Replace it
> > with the real thing.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index fca56fc..4b1af94 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -696,8 +696,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
> >  				p = p1 * p2;
> >  				/* based on hardware requirement, prefer bigger m1,m2 values */
> >  				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
> > -					m2 = (((2*(fastclk * p * n / m1 )) +
> > -					       refclk) / (2*refclk));
> > +					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
> >  					m = m1 * m2;
> >  					vco = updrate * m;
> >  
> > -- 
> > 1.8.1.5
> 
> Not a problem with this patch but perhaps consideration for further
> cleanups: target and refclk should be u32 and further down the line
> the crtc_config.clock and xxx_get_refclk() also.
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Up to this all merged, thanks for patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 03/14] drm/i915: Make vlv_find_best_dpll() ppm calculation safe

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use div_u64() to make the ppm calculation in vlv_find_best_dpll() safe
against interger overflows.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4b1af94..f8d67b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -673,13 +673,11 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
 	u32 m, n, fastclk;
 	u32 updrate, minupdate, p;
-	unsigned long bestppm, ppm, absppm;
+	unsigned int bestppm = 1000000;
 	int dotclk, flag;
 
 	flag = 0;
 	dotclk = target * 1000;
-	bestppm = 1000000;
-	ppm = absppm = 0;
 	fastclk = dotclk / (2*100);
 	updrate = 0;
 	minupdate = 19200;
@@ -696,6 +694,8 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 				p = p1 * p2;
 				/* based on hardware requirement, prefer bigger m1,m2 values */
 				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
+					unsigned int ppm, diff;
+
 					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
 					m = m1 * m2;
 					vco = updrate * m;
@@ -703,14 +703,14 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 					if (vco < limit->vco.min || vco >= limit->vco.max)
 						continue;
 
-					ppm = 1000000 * ((vco / p) - fastclk) / fastclk;
-					absppm = (ppm > 0) ? ppm : (-ppm);
-					if (absppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
+					diff = abs(vco / p - fastclk);
+					ppm = div_u64(1000000ULL * diff, fastclk);
+					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
 						bestppm = 0;
 						flag = 1;
 					}
-					if (absppm < bestppm - 10) {
-						bestppm = absppm;
+					if (ppm < bestppm - 10) {
+						bestppm = ppm;
 						flag = 1;
 					}
 					if (flag) {
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/14] drm/i915: Don't underflow bestppm

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We do 'bestppm - 10' in vlv_find_best_dpll() but never check whether
that might underflow. Add such a check.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f8d67b2..f646fea 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -709,7 +709,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 						bestppm = 0;
 						flag = 1;
 					}
-					if (ppm < bestppm - 10) {
+					if (bestppm >= 10 && ppm < bestppm - 10) {
 						bestppm = ppm;
 						flag = 1;
 					}
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 05/14] drm/i915: Rewrite vlv_find_best_dpll()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rewrite vlv_find_best_dpll() to use intel_clock_t rather than
an army of local variables.

Also extract the code to calculate the derived values into
vlv_clock().

v2: Split up the earlier fixes, extract vlv_clock()

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++--------------------
 1 file changed, 31 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f646fea..c5f0794 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -438,6 +438,14 @@ static void i9xx_clock(int refclk, intel_clock_t *clock)
 	clock->dot = clock->vco / clock->p;
 }
 
+static void vlv_clock(int refclk, intel_clock_t *clock)
+{
+	clock->m = clock->m1 * clock->m2;
+	clock->p = clock->p1 * clock->p2;
+	clock->vco = refclk * clock->m / clock->n;
+	clock->dot = clock->vco / clock->p;
+}
+
 /**
  * Returns whether any output on the specified pipe is of the specified type
  */
@@ -670,66 +678,48 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
-	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
-	u32 m, n, fastclk;
-	u32 updrate, minupdate, p;
+	intel_clock_t clock;
+	u32 minupdate = 19200;
 	unsigned int bestppm = 1000000;
-	int dotclk, flag;
 
-	flag = 0;
-	dotclk = target * 1000;
-	fastclk = dotclk / (2*100);
-	updrate = 0;
-	minupdate = 19200;
-	n = p = p1 = p2 = m = m1 = m2 = vco = bestn = 0;
-	bestm1 = bestm2 = bestp1 = bestp2 = 0;
+	target *= 5; /* fast clock */
 
 	/* based on hardware requirement, prefer smaller n to precision */
-	for (n = limit->n.min; n <= ((refclk) / minupdate); n++) {
-		updrate = refclk / n;
-		for (p1 = limit->p1.max; p1 > limit->p1.min; p1--) {
-			for (p2 = limit->p2.p2_fast+1; p2 > 0; p2--) {
-				if (p2 > 10)
-					p2 = p2 - 1;
-				p = p1 * p2;
+	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
+		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
+			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
+				if (clock.p2 > 10)
+					clock.p2--;
+				clock.p = clock.p1 * clock.p2;
 				/* based on hardware requirement, prefer bigger m1,m2 values */
-				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
+				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
 					unsigned int ppm, diff;
 
-					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
-					m = m1 * m2;
-					vco = updrate * m;
+					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
+								     refclk * clock.m1);
 
-					if (vco < limit->vco.min || vco >= limit->vco.max)
+					vlv_clock(refclk, &clock);
+
+					if (clock.vco < limit->vco.min ||
+					    clock.vco >= limit->vco.max)
 						continue;
 
-					diff = abs(vco / p - fastclk);
-					ppm = div_u64(1000000ULL * diff, fastclk);
-					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
+					diff = abs(clock.dot - target);
+					ppm = div_u64(1000000ULL * diff, target);
+
+					if (ppm < 100 && clock.p > best_clock->p) {
 						bestppm = 0;
-						flag = 1;
+						*best_clock = clock;
 					}
+
 					if (bestppm >= 10 && ppm < bestppm - 10) {
 						bestppm = ppm;
-						flag = 1;
-					}
-					if (flag) {
-						bestn = n;
-						bestm1 = m1;
-						bestm2 = m2;
-						bestp1 = p1;
-						bestp2 = p2;
-						flag = 0;
+						*best_clock = clock;
 					}
 				}
 			}
 		}
 	}
-	best_clock->n = bestn;
-	best_clock->m1 = bestm1;
-	best_clock->m2 = bestm2;
-	best_clock->p1 = bestp1;
-	best_clock->p2 = bestp2;
 
 	return true;
 }
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/14] drm/i915: Rewrite vlv_find_best_dpll()

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rewrite vlv_find_best_dpll() to use intel_clock_t rather than
> an army of local variables.
>
> Also extract the code to calculate the derived values into
> vlv_clock().
>
> v2: Split up the earlier fixes, extract vlv_clock()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f646fea..c5f0794 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -438,6 +438,14 @@ static void i9xx_clock(int refclk, intel_clock_t *clock)
>  	clock->dot = clock->vco / clock->p;
>  }
>  
> +static void vlv_clock(int refclk, intel_clock_t *clock)
> +{
> +	clock->m = clock->m1 * clock->m2;
> +	clock->p = clock->p1 * clock->p2;
> +	clock->vco = refclk * clock->m / clock->n;
> +	clock->dot = clock->vco / clock->p;
> +}
> +
>  /**
>   * Returns whether any output on the specified pipe is of the specified type
>   */
> @@ -670,66 +678,48 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> -	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
> -	u32 m, n, fastclk;
> -	u32 updrate, minupdate, p;
> +	intel_clock_t clock;
> +	u32 minupdate = 19200;
>  	unsigned int bestppm = 1000000;
> -	int dotclk, flag;
>  
> -	flag = 0;
> -	dotclk = target * 1000;
> -	fastclk = dotclk / (2*100);
> -	updrate = 0;
> -	minupdate = 19200;
> -	n = p = p1 = p2 = m = m1 = m2 = vco = bestn = 0;
> -	bestm1 = bestm2 = bestp1 = bestp2 = 0;
> +	target *= 5; /* fast clock */
>  
>  	/* based on hardware requirement, prefer smaller n to precision */
> -	for (n = limit->n.min; n <= ((refclk) / minupdate); n++) {
> -		updrate = refclk / n;
> -		for (p1 = limit->p1.max; p1 > limit->p1.min; p1--) {
> -			for (p2 = limit->p2.p2_fast+1; p2 > 0; p2--) {
> -				if (p2 > 10)
> -					p2 = p2 - 1;
> -				p = p1 * p2;
> +	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
> +		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
> +			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
> +				if (clock.p2 > 10)
> +					clock.p2--;
> +				clock.p = clock.p1 * clock.p2;
>  				/* based on hardware requirement, prefer bigger m1,m2 values */

Is this comment valid as we seem to start from m1.min?

> -				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
> +				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
>  					unsigned int ppm, diff;
>  
> -					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
> -					m = m1 * m2;
> -					vco = updrate * m;
> +					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
> +								     refclk * clock.m1);
>  
> -					if (vco < limit->vco.min || vco >= limit->vco.max)
> +					vlv_clock(refclk, &clock);
> +

> +					if (clock.vco < limit->vco.min ||
> +					    clock.vco >= limit->vco.max)
>  						continue;

Can intel_PLL_is_valid() used here instead of just checking the vco?

>  
> -					diff = abs(vco / p - fastclk);
> -					ppm = div_u64(1000000ULL * diff, fastclk);
> -					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
> +					diff = abs(clock.dot - target);
> +					ppm = div_u64(1000000ULL * diff, target);
> +
> +					if (ppm < 100 && clock.p > best_clock->p) {
>  						bestppm = 0;
> -						flag = 1;
> +						*best_clock = clock;
>  					}
> +
>  					if (bestppm >= 10 && ppm < bestppm - 10) {
>  						bestppm = ppm;
> -						flag = 1;
> -					}
> -					if (flag) {
> -						bestn = n;
> -						bestm1 = m1;
> -						bestm2 = m2;
> -						bestp1 = p1;
> -						bestp2 = p2;
> -						flag = 0;
> +						*best_clock = clock;
>  					}
>  				}
>  			}
>  		}
>  	}
> -	best_clock->n = bestn;
> -	best_clock->m1 = bestm1;
> -	best_clock->m2 = bestm2;
> -	best_clock->p1 = bestp1;
> -	best_clock->p2 = bestp2;
>  
>  	return true;
>  }
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/14] drm/i915: Rewrite vlv_find_best_dpll()

[Ville Syrjälä]

On Thu, Sep 26, 2013 at 06:30:55PM +0300, Mika Kuoppala wrote:
> ville.syrjala@linux.intel.com writes:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Rewrite vlv_find_best_dpll() to use intel_clock_t rather than
> > an army of local variables.
> >
> > Also extract the code to calculate the derived values into
> > vlv_clock().
> >
> > v2: Split up the earlier fixes, extract vlv_clock()
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++--------------------
> >  1 file changed, 31 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f646fea..c5f0794 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -438,6 +438,14 @@ static void i9xx_clock(int refclk, intel_clock_t *clock)
> >  	clock->dot = clock->vco / clock->p;
> >  }
> >  
> > +static void vlv_clock(int refclk, intel_clock_t *clock)
> > +{
> > +	clock->m = clock->m1 * clock->m2;
> > +	clock->p = clock->p1 * clock->p2;
> > +	clock->vco = refclk * clock->m / clock->n;
> > +	clock->dot = clock->vco / clock->p;
> > +}
> > +
> >  /**
> >   * Returns whether any output on the specified pipe is of the specified type
> >   */
> > @@ -670,66 +678,48 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > -	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
> > -	u32 m, n, fastclk;
> > -	u32 updrate, minupdate, p;
> > +	intel_clock_t clock;
> > +	u32 minupdate = 19200;
> >  	unsigned int bestppm = 1000000;
> > -	int dotclk, flag;
> >  
> > -	flag = 0;
> > -	dotclk = target * 1000;
> > -	fastclk = dotclk / (2*100);
> > -	updrate = 0;
> > -	minupdate = 19200;
> > -	n = p = p1 = p2 = m = m1 = m2 = vco = bestn = 0;
> > -	bestm1 = bestm2 = bestp1 = bestp2 = 0;
> > +	target *= 5; /* fast clock */
> >  
> >  	/* based on hardware requirement, prefer smaller n to precision */
> > -	for (n = limit->n.min; n <= ((refclk) / minupdate); n++) {
> > -		updrate = refclk / n;
> > -		for (p1 = limit->p1.max; p1 > limit->p1.min; p1--) {
> > -			for (p2 = limit->p2.p2_fast+1; p2 > 0; p2--) {
> > -				if (p2 > 10)
> > -					p2 = p2 - 1;
> > -				p = p1 * p2;
> > +	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
> > +		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
> > +			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
> > +				if (clock.p2 > 10)
> > +					clock.p2--;
> > +				clock.p = clock.p1 * clock.p2;
> >  				/* based on hardware requirement, prefer bigger m1,m2 values */
> 
> Is this comment valid as we seem to start from m1.min?

We anyway try to find the closest m2 based on m1,n,p1 and p2, and since
we start w/ large p dividers, m1*m2 will come out as something big to
compensate. Though starting with small n does mean m2 doesn't come out
as large as it could be, but I guess having a small n is considered
more important than having a large m.

The bestppm comparison we do guarantees that we prefer an earlier result
unless the new ppm is at least 10 better, and since we start with small
n and large p, it should do what we want.

Then there's ppm<100 comparison which is a bit different. It means we
favor anything that is considered good enough (ppm < 100) as long as
the p divider increases, and hence the VCO frequency increases. That
would seem to be in line with the other stated goals of big m and small n.

> 
> > -				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
> > +				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
> >  					unsigned int ppm, diff;
> >  
> > -					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
> > -					m = m1 * m2;
> > -					vco = updrate * m;
> > +					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
> > +								     refclk * clock.m1);
> >  
> > -					if (vco < limit->vco.min || vco >= limit->vco.max)
> > +					vlv_clock(refclk, &clock);
> > +
> 
> > +					if (clock.vco < limit->vco.min ||
> > +					    clock.vco >= limit->vco.max)
> >  						continue;
> 
> Can intel_PLL_is_valid() used here instead of just checking the vco?

We'd need to modify intel_PLL_is_valid() a bit to skip the m1<=m2 check,
and we'd also need to skip the 'm' and 'p' divider check, or populate
the m and p min/max with something that makes sense.

It would do the clock.dot min/max check that we're currently missing
from this function, and I guess it would allow easier debugging since it
has the INTELPllInvalid() macro for that purpose. So it would seem to be
a good idea to use it.

> 
> >  
> > -					diff = abs(vco / p - fastclk);
> > -					ppm = div_u64(1000000ULL * diff, fastclk);
> > -					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
> > +					diff = abs(clock.dot - target);
> > +					ppm = div_u64(1000000ULL * diff, target);
> > +
> > +					if (ppm < 100 && clock.p > best_clock->p) {
> >  						bestppm = 0;
> > -						flag = 1;
> > +						*best_clock = clock;
> >  					}
> > +
> >  					if (bestppm >= 10 && ppm < bestppm - 10) {
> >  						bestppm = ppm;
> > -						flag = 1;
> > -					}
> > -					if (flag) {
> > -						bestn = n;
> > -						bestm1 = m1;
> > -						bestm2 = m2;
> > -						bestp1 = p1;
> > -						bestp2 = p2;
> > -						flag = 0;
> > +						*best_clock = clock;
> >  					}
> >  				}
> >  			}
> >  		}
> >  	}
> > -	best_clock->n = bestn;
> > -	best_clock->m1 = bestm1;
> > -	best_clock->m2 = bestm2;
> > -	best_clock->p1 = bestp1;
> > -	best_clock->p2 = bestp2;
> >  
> >  	return true;
> >  }
> > -- 
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH 15/14] drm/i915: Use intel_PLL_is_valid() in vlv_find_best_dpll()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Everyone else uses intel_PLL_is_valid(), so make VLV use it as well.

We don't have any special p and m limits on VLV, so skip those tests,
and we also need to skip the m1<=m2 test line PNV.

Reorganize the function a bit to move the n check alongside the rest of
the test for the non-derived dividers, and check the derived values
afterwards.

Note that this changes vlv_find_best_dpll() in two ways:
- The .vco comparison is now >max instead of >=max, and since we round
  down when calculating that stuff, we may now allow frequencies slightly
  above the max as we do on other platforms. The previous method
  disallowed exactly max and anything above it.
- We now check the .dot frequency against the data rate limits, which we
  didn't do before.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 35 ++++++++++++++++++++++++-----------
 1 file changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 6429cbb..11d3ddf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -311,7 +311,13 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
 };
 
 static const intel_limit_t intel_limits_vlv = {
-	.dot = { .min = 25000, .max = 270000 },
+	 /*
+	  * These are the data rate limits (measured in fast clocks)
+	  * since those are the strictest limits we have. The fast
+	  * clock and actual rate limits are more relaxed, so checking
+	  * them would make no difference.
+	  */
+	.dot = { .min = 25000 * 5, .max = 270000 * 5 },
 	.vco = { .min = 4000000, .max = 6000000 },
 	.n = { .min = 1, .max = 7 },
 	.m1 = { .min = 2, .max = 3 },
@@ -452,20 +458,26 @@ static bool intel_PLL_is_valid(struct drm_device *dev,
 			       const intel_limit_t *limit,
 			       const intel_clock_t *clock)
 {
+	if (clock->n   < limit->n.min   || limit->n.max   < clock->n)
+		INTELPllInvalid("n out of range\n");
 	if (clock->p1  < limit->p1.min  || limit->p1.max  < clock->p1)
 		INTELPllInvalid("p1 out of range\n");
-	if (clock->p   < limit->p.min   || limit->p.max   < clock->p)
-		INTELPllInvalid("p out of range\n");
 	if (clock->m2  < limit->m2.min  || limit->m2.max  < clock->m2)
 		INTELPllInvalid("m2 out of range\n");
 	if (clock->m1  < limit->m1.min  || limit->m1.max  < clock->m1)
 		INTELPllInvalid("m1 out of range\n");
-	if (clock->m1 <= clock->m2 && !IS_PINEVIEW(dev))
-		INTELPllInvalid("m1 <= m2\n");
-	if (clock->m   < limit->m.min   || limit->m.max   < clock->m)
-		INTELPllInvalid("m out of range\n");
-	if (clock->n   < limit->n.min   || limit->n.max   < clock->n)
-		INTELPllInvalid("n out of range\n");
+
+	if (!IS_PINEVIEW(dev) && !IS_VALLEYVIEW(dev))
+		if (clock->m1 <= clock->m2)
+			INTELPllInvalid("m1 <= m2\n");
+
+	if (!IS_VALLEYVIEW(dev)) {
+		if (clock->p < limit->p.min || limit->p.max < clock->p)
+			INTELPllInvalid("p out of range\n");
+		if (clock->m < limit->m.min || limit->m.max < clock->m)
+			INTELPllInvalid("m out of range\n");
+	}
+
 	if (clock->vco < limit->vco.min || limit->vco.max < clock->vco)
 		INTELPllInvalid("vco out of range\n");
 	/* XXX: We may need to be checking "Dot clock" depending on the multiplier,
@@ -659,6 +671,7 @@ vlv_find_best_dpll(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;
 	unsigned int bestppm = 1000000;
 	/* min update 19.2 MHz */
@@ -684,8 +697,8 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 
 					vlv_clock(refclk, &clock);
 
-					if (clock.vco < limit->vco.min ||
-					    clock.vco >= limit->vco.max)
+					if (!intel_PLL_is_valid(dev, limit,
+								&clock))
 						continue;
 
 					diff = abs(clock.dot - target);
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/14] drm/i915: Rewrite vlv_find_best_dpll()

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Rewrite vlv_find_best_dpll() to use intel_clock_t rather than
> an army of local variables.
>
> Also extract the code to calculate the derived values into
> vlv_clock().
>
> v2: Split up the earlier fixes, extract vlv_clock()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++--------------------
>  1 file changed, 31 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f646fea..c5f0794 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -438,6 +438,14 @@ static void i9xx_clock(int refclk, intel_clock_t *clock)
>  	clock->dot = clock->vco / clock->p;
>  }
>  
> +static void vlv_clock(int refclk, intel_clock_t *clock)
> +{
> +	clock->m = clock->m1 * clock->m2;
> +	clock->p = clock->p1 * clock->p2;
> +	clock->vco = refclk * clock->m / clock->n;
> +	clock->dot = clock->vco / clock->p;
> +}
> +
>  /**
>   * Returns whether any output on the specified pipe is of the specified type
>   */
> @@ -670,66 +678,48 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
>  		   int target, int refclk, intel_clock_t *match_clock,
>  		   intel_clock_t *best_clock)
>  {
> -	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
> -	u32 m, n, fastclk;
> -	u32 updrate, minupdate, p;
> +	intel_clock_t clock;
> +	u32 minupdate = 19200;
>  	unsigned int bestppm = 1000000;
> -	int dotclk, flag;
>  
> -	flag = 0;
> -	dotclk = target * 1000;
> -	fastclk = dotclk / (2*100);
> -	updrate = 0;
> -	minupdate = 19200;
> -	n = p = p1 = p2 = m = m1 = m2 = vco = bestn = 0;
> -	bestm1 = bestm2 = bestp1 = bestp2 = 0;
> +	target *= 5; /* fast clock */
>  
>  	/* based on hardware requirement, prefer smaller n to precision */
> -	for (n = limit->n.min; n <= ((refclk) / minupdate); n++) {
> -		updrate = refclk / n;
> -		for (p1 = limit->p1.max; p1 > limit->p1.min; p1--) {
> -			for (p2 = limit->p2.p2_fast+1; p2 > 0; p2--) {
> -				if (p2 > 10)
> -					p2 = p2 - 1;
> -				p = p1 * p2;
> +	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
> +		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
> +			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
> +				if (clock.p2 > 10)
> +					clock.p2--;
> +				clock.p = clock.p1 * clock.p2;
>  				/* based on hardware requirement, prefer bigger m1,m2 values */
> -				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
> +				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
>  					unsigned int ppm, diff;
>  
> -					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
> -					m = m1 * m2;
> -					vco = updrate * m;
> +					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
> +								     refclk * clock.m1);
>  
> -					if (vco < limit->vco.min || vco >= limit->vco.max)
> +					vlv_clock(refclk, &clock);
> +
> +					if (clock.vco < limit->vco.min ||
> +					    clock.vco >= limit->vco.max)
>  						continue;
>  
> -					diff = abs(vco / p - fastclk);
> -					ppm = div_u64(1000000ULL * diff, fastclk);
> -					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
> +					diff = abs(clock.dot - target);
> +					ppm = div_u64(1000000ULL * diff, target);
> +
> +					if (ppm < 100 && clock.p > best_clock->p) {

best_clock needs to be initialized or you end up comparing against
bytes from stack on first hitting here.

>  						bestppm = 0;
> -						flag = 1;
> +						*best_clock = clock;
>  					}
> +
>  					if (bestppm >= 10 && ppm < bestppm - 10) {
>  						bestppm = ppm;
> -						flag = 1;
> -					}
> -					if (flag) {
> -						bestn = n;
> -						bestm1 = m1;
> -						bestm2 = m2;
> -						bestp1 = p1;
> -						bestp2 = p2;
> -						flag = 0;
> +						*best_clock = clock;
>  					}
>  				}
>  			}
>  		}
>  	}
> -	best_clock->n = bestn;
> -	best_clock->m1 = bestm1;
> -	best_clock->m2 = bestm2;
> -	best_clock->p1 = bestp1;
> -	best_clock->p2 = bestp2;
>  
>  	return true;
>  }
> -- 
> 1.8.1.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/14] drm/i915: Rewrite vlv_find_best_dpll()

[Ville Syrjälä]

On Fri, Sep 27, 2013 at 02:55:32PM +0300, Mika Kuoppala wrote:
> ville.syrjala@linux.intel.com writes:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Rewrite vlv_find_best_dpll() to use intel_clock_t rather than
> > an army of local variables.
> >
> > Also extract the code to calculate the derived values into
> > vlv_clock().
> >
> > v2: Split up the earlier fixes, extract vlv_clock()
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++--------------------
> >  1 file changed, 31 insertions(+), 41 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f646fea..c5f0794 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -438,6 +438,14 @@ static void i9xx_clock(int refclk, intel_clock_t *clock)
> >  	clock->dot = clock->vco / clock->p;
> >  }
> >  
> > +static void vlv_clock(int refclk, intel_clock_t *clock)
> > +{
> > +	clock->m = clock->m1 * clock->m2;
> > +	clock->p = clock->p1 * clock->p2;
> > +	clock->vco = refclk * clock->m / clock->n;
> > +	clock->dot = clock->vco / clock->p;
> > +}
> > +
> >  /**
> >   * Returns whether any output on the specified pipe is of the specified type
> >   */
> > @@ -670,66 +678,48 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
> >  		   int target, int refclk, intel_clock_t *match_clock,
> >  		   intel_clock_t *best_clock)
> >  {
> > -	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
> > -	u32 m, n, fastclk;
> > -	u32 updrate, minupdate, p;
> > +	intel_clock_t clock;
> > +	u32 minupdate = 19200;
> >  	unsigned int bestppm = 1000000;
> > -	int dotclk, flag;
> >  
> > -	flag = 0;
> > -	dotclk = target * 1000;
> > -	fastclk = dotclk / (2*100);
> > -	updrate = 0;
> > -	minupdate = 19200;
> > -	n = p = p1 = p2 = m = m1 = m2 = vco = bestn = 0;
> > -	bestm1 = bestm2 = bestp1 = bestp2 = 0;
> > +	target *= 5; /* fast clock */
> >  
> >  	/* based on hardware requirement, prefer smaller n to precision */
> > -	for (n = limit->n.min; n <= ((refclk) / minupdate); n++) {
> > -		updrate = refclk / n;
> > -		for (p1 = limit->p1.max; p1 > limit->p1.min; p1--) {
> > -			for (p2 = limit->p2.p2_fast+1; p2 > 0; p2--) {
> > -				if (p2 > 10)
> > -					p2 = p2 - 1;
> > -				p = p1 * p2;
> > +	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
> > +		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
> > +			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
> > +				if (clock.p2 > 10)
> > +					clock.p2--;
> > +				clock.p = clock.p1 * clock.p2;
> >  				/* based on hardware requirement, prefer bigger m1,m2 values */
> > -				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
> > +				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
> >  					unsigned int ppm, diff;
> >  
> > -					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
> > -					m = m1 * m2;
> > -					vco = updrate * m;
> > +					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
> > +								     refclk * clock.m1);
> >  
> > -					if (vco < limit->vco.min || vco >= limit->vco.max)
> > +					vlv_clock(refclk, &clock);
> > +
> > +					if (clock.vco < limit->vco.min ||
> > +					    clock.vco >= limit->vco.max)
> >  						continue;
> >  
> > -					diff = abs(vco / p - fastclk);
> > -					ppm = div_u64(1000000ULL * diff, fastclk);
> > -					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
> > +					diff = abs(clock.dot - target);
> > +					ppm = div_u64(1000000ULL * diff, target);
> > +
> > +					if (ppm < 100 && clock.p > best_clock->p) {
> 
> best_clock needs to be initialized or you end up comparing against
> bytes from stack on first hitting here.

Right. v3 coming up.

> 
> >  						bestppm = 0;
> > -						flag = 1;
> > +						*best_clock = clock;
> >  					}
> > +
> >  					if (bestppm >= 10 && ppm < bestppm - 10) {
> >  						bestppm = ppm;
> > -						flag = 1;
> > -					}
> > -					if (flag) {
> > -						bestn = n;
> > -						bestm1 = m1;
> > -						bestm2 = m2;
> > -						bestp1 = p1;
> > -						bestp2 = p2;
> > -						flag = 0;
> > +						*best_clock = clock;
> >  					}
> >  				}
> >  			}
> >  		}
> >  	}
> > -	best_clock->n = bestn;
> > -	best_clock->m1 = bestm1;
> > -	best_clock->m2 = bestm2;
> > -	best_clock->p1 = bestp1;
> > -	best_clock->p2 = bestp2;
> >  
> >  	return true;
> >  }
> > -- 
> > 1.8.1.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

[PATCH v3 05/14] drm/i915: Rewrite vlv_find_best_dpll()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rewrite vlv_find_best_dpll() to use intel_clock_t rather than
an army of local variables.

Also extract the code to calculate the derived values into
vlv_clock().

v2: Split up the earlier fixes, extract vlv_clock()
v3: Initialize best_clock

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 74 ++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f646fea..9faf3bb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -438,6 +438,14 @@ static void i9xx_clock(int refclk, intel_clock_t *clock)
 	clock->dot = clock->vco / clock->p;
 }
 
+static void vlv_clock(int refclk, intel_clock_t *clock)
+{
+	clock->m = clock->m1 * clock->m2;
+	clock->p = clock->p1 * clock->p2;
+	clock->vco = refclk * clock->m / clock->n;
+	clock->dot = clock->vco / clock->p;
+}
+
 /**
  * Returns whether any output on the specified pipe is of the specified type
  */
@@ -670,66 +678,50 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 		   int target, int refclk, intel_clock_t *match_clock,
 		   intel_clock_t *best_clock)
 {
-	u32 p1, p2, m1, m2, vco, bestn, bestm1, bestm2, bestp1, bestp2;
-	u32 m, n, fastclk;
-	u32 updrate, minupdate, p;
+	intel_clock_t clock;
+	u32 minupdate = 19200;
 	unsigned int bestppm = 1000000;
-	int dotclk, flag;
 
-	flag = 0;
-	dotclk = target * 1000;
-	fastclk = dotclk / (2*100);
-	updrate = 0;
-	minupdate = 19200;
-	n = p = p1 = p2 = m = m1 = m2 = vco = bestn = 0;
-	bestm1 = bestm2 = bestp1 = bestp2 = 0;
+	target *= 5; /* fast clock */
+
+	memset(best_clock, 0, sizeof(*best_clock));
 
 	/* based on hardware requirement, prefer smaller n to precision */
-	for (n = limit->n.min; n <= ((refclk) / minupdate); n++) {
-		updrate = refclk / n;
-		for (p1 = limit->p1.max; p1 > limit->p1.min; p1--) {
-			for (p2 = limit->p2.p2_fast+1; p2 > 0; p2--) {
-				if (p2 > 10)
-					p2 = p2 - 1;
-				p = p1 * p2;
+	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
+		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
+			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
+				if (clock.p2 > 10)
+					clock.p2--;
+				clock.p = clock.p1 * clock.p2;
 				/* based on hardware requirement, prefer bigger m1,m2 values */
-				for (m1 = limit->m1.min; m1 <= limit->m1.max; m1++) {
+				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
 					unsigned int ppm, diff;
 
-					m2 = DIV_ROUND_CLOSEST(fastclk * p * n, refclk * m1);
-					m = m1 * m2;
-					vco = updrate * m;
+					clock.m2 = DIV_ROUND_CLOSEST(target * clock.p * clock.n,
+								     refclk * clock.m1);
+
+					vlv_clock(refclk, &clock);
 
-					if (vco < limit->vco.min || vco >= limit->vco.max)
+					if (clock.vco < limit->vco.min ||
+					    clock.vco >= limit->vco.max)
 						continue;
 
-					diff = abs(vco / p - fastclk);
-					ppm = div_u64(1000000ULL * diff, fastclk);
-					if (ppm < 100 && ((p1 * p2) > (bestp1 * bestp2))) {
+					diff = abs(clock.dot - target);
+					ppm = div_u64(1000000ULL * diff, target);
+
+					if (ppm < 100 && clock.p > best_clock->p) {
 						bestppm = 0;
-						flag = 1;
+						*best_clock = clock;
 					}
+
 					if (bestppm >= 10 && ppm < bestppm - 10) {
 						bestppm = ppm;
-						flag = 1;
-					}
-					if (flag) {
-						bestn = n;
-						bestm1 = m1;
-						bestm2 = m2;
-						bestp1 = p1;
-						bestp2 = p2;
-						flag = 0;
+						*best_clock = clock;
 					}
 				}
 			}
 		}
 	}
-	best_clock->n = bestn;
-	best_clock->m1 = bestm1;
-	best_clock->m2 = bestm2;
-	best_clock->p1 = bestp1;
-	best_clock->p2 = bestp2;
 
 	return true;
 }
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/14] drm/i915: De-magic the VLV p2 divider step size

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The p2 divider on VLV needs to be even when it's > 10. The current code
to make that happen is rather weird. Just make the step size adjustement
in the for loop decrement step.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c5f0794..330fefc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -687,9 +687,8 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	/* based on hardware requirement, prefer smaller n to precision */
 	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
 		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
-			for (clock.p2 = limit->p2.p2_fast+1; clock.p2 > 0; clock.p2--) {
-				if (clock.p2 > 10)
-					clock.p2--;
+			for (clock.p2 = limit->p2.p2_fast; clock.p2 > 0;
+			     clock.p2 -= clock.p2 > 10 ? 2 : 1) {
 				clock.p = clock.p1 * clock.p2;
 				/* based on hardware requirement, prefer bigger m1,m2 values */
 				for (clock.m1 = limit->m1.min; clock.m1 <= limit->m1.max; clock.m1++) {
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/14] drm/i915: Make sure we respect n.max on VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We limit the maximum n divider value in order to make sure the PLL's
reference inout is at least 19.2 MHz. I assume that is done to satisfy
some hardware requirement.

However we never check whether that calculated limit is below the
maximum supoorted N divider value (7). In practice that is always true
since we only support 100 MHz reference clock, but making the code
safe against higher reference clocks seems like a reasoanble thing to
do.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 330fefc..333e53b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -679,13 +679,14 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 		   intel_clock_t *best_clock)
 {
 	intel_clock_t clock;
-	u32 minupdate = 19200;
 	unsigned int bestppm = 1000000;
+	/* min update 19.2 MHz */
+	int max_n = min(limit->n.max, refclk / 19200);
 
 	target *= 5; /* fast clock */
 
 	/* based on hardware requirement, prefer smaller n to precision */
-	for (clock.n = limit->n.min; clock.n <= ((refclk) / minupdate); clock.n++) {
+	for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
 		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
 			for (clock.p2 = limit->p2.p2_fast; clock.p2 > 0;
 			     clock.p2 -= clock.p2 > 10 ? 2 : 1) {
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/14] drm/i915: Clarify VLV PLL p1 limits

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

For some reason there's a sort of off by one issue with the p1 divider.
The actual p1 limits according to
VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm is 2-3, so we should
just say that instead of saying 1-3 and avoiding the 1 via the choice of
comparison operator.

I don't know why we're using different p1 limits for intel_limits_vlv_dac
and intel_limits_vlv_hdmi, but let's preserve that for now.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 333e53b..d838bd7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -318,7 +318,7 @@ 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 = 1, .max = 3 },
+	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 },
 };
@@ -331,7 +331,7 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 	.m1 = { .min = 2, .max = 3 },
 	.m2 = { .min = 11, .max = 156 },
 	.p = { .min = 10, .max = 30 },
-	.p1 = { .min = 2, .max = 3 },
+	.p1 = { .min = 3, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 },
 };
@@ -687,7 +687,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 
 	/* based on hardware requirement, prefer smaller n to precision */
 	for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
-		for (clock.p1 = limit->p1.max; clock.p1 > limit->p1.min; clock.p1--) {
+		for (clock.p1 = limit->p1.max; clock.p1 >= limit->p1.min; clock.p1--) {
 			for (clock.p2 = limit->p2.p2_fast; clock.p2 > 0;
 			     clock.p2 -= clock.p2 > 10 ? 2 : 1) {
 				clock.p = clock.p1 * clock.p2;
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/14] drm/i915: Allow p1 divider 2 on VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

According to VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm p1
can be 2-3 always.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d838bd7..4815ddc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -331,7 +331,7 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 	.m1 = { .min = 2, .max = 3 },
 	.m2 = { .min = 11, .max = 156 },
 	.p = { .min = 10, .max = 30 },
-	.p1 = { .min = 3, .max = 3 },
+	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 },
 };
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/14] drm/i915: Respect p2 divider minimum limit on VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm tells us that the
minimum p2 divider is 2. Use that limit on the code.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4815ddc..e00924d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -320,7 +320,7 @@ static const intel_limit_t intel_limits_vlv_dac = {
 	.p = { .min = 10, .max = 30 },
 	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
-		.p2_slow = 2, .p2_fast = 20 },
+		.p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
 };
 
 static const intel_limit_t intel_limits_vlv_hdmi = {
@@ -333,7 +333,7 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 	.p = { .min = 10, .max = 30 },
 	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
-		.p2_slow = 2, .p2_fast = 20 },
+		.p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
 };
 
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
@@ -688,7 +688,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	/* based on hardware requirement, prefer smaller n to precision */
 	for (clock.n = limit->n.min; clock.n <= max_n; clock.n++) {
 		for (clock.p1 = limit->p1.max; clock.p1 >= limit->p1.min; clock.p1--) {
-			for (clock.p2 = limit->p2.p2_fast; clock.p2 > 0;
+			for (clock.p2 = limit->p2.p2_fast; clock.p2 >= limit->p2.p2_slow;
 			     clock.p2 -= clock.p2 > 10 ? 2 : 1) {
 				clock.p = clock.p1 * clock.p2;
 				/* based on hardware requirement, prefer bigger m1,m2 values */
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/14] drm/i915: Remove the unused p and m limits for VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We never check the p and m limits (which according to comments are
based on someone's guesswork), so just remove them.

VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm has no p and m
limits listed.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e00924d..2e00964 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -314,10 +314,8 @@ static const intel_limit_t intel_limits_vlv_dac = {
 	.dot = { .min = 25000, .max = 270000 },
 	.vco = { .min = 4000000, .max = 6000000 },
 	.n = { .min = 1, .max = 7 },
-	.m = { .min = 22, .max = 450 }, /* guess */
 	.m1 = { .min = 2, .max = 3 },
 	.m2 = { .min = 11, .max = 156 },
-	.p = { .min = 10, .max = 30 },
 	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
@@ -327,10 +325,8 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 	.dot = { .min = 25000, .max = 270000 },
 	.vco = { .min = 4000000, .max = 6000000 },
 	.n = { .min = 1, .max = 7 },
-	.m = { .min = 60, .max = 300 }, /* guess */
 	.m1 = { .min = 2, .max = 3 },
 	.m2 = { .min = 11, .max = 156 },
-	.p = { .min = 10, .max = 30 },
 	.p1 = { .min = 2, .max = 3 },
 	.p2 = { .dot_limit = 270000,
 		.p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/14] drm/i915: Remove unused dot_limit from VLV PLL limits

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We don't use .dot_limit for anything on VLV, so don't populate it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2e00964..202e054 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -317,8 +317,7 @@ static const intel_limit_t intel_limits_vlv_dac = {
 	.m1 = { .min = 2, .max = 3 },
 	.m2 = { .min = 11, .max = 156 },
 	.p1 = { .min = 2, .max = 3 },
-	.p2 = { .dot_limit = 270000,
-		.p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
+	.p2 = { .p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
 };
 
 static const intel_limit_t intel_limits_vlv_hdmi = {
@@ -328,8 +327,7 @@ static const intel_limit_t intel_limits_vlv_hdmi = {
 	.m1 = { .min = 2, .max = 3 },
 	.m2 = { .min = 11, .max = 156 },
 	.p1 = { .min = 2, .max = 3 },
-	.p2 = { .dot_limit = 270000,
-		.p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
+	.p2 = { .p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
 };
 
 static const intel_limit_t *intel_ironlake_limit(struct drm_crtc *crtc,
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 13/14] drm/i915: intel_limits_vlv_dac and intel_limits_vlv_hdmi are the same

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

After aligning the p1 divider limits, and removing the unused p and m
limits, intel_limits_vlv_dac and intel_limits_vlv_hdmi are identical.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 202e054..b57ced9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -310,17 +310,7 @@ static const intel_limit_t intel_limits_ironlake_dual_lvds_100m = {
 		.p2_slow = 7, .p2_fast = 7 },
 };
 
-static const intel_limit_t intel_limits_vlv_dac = {
-	.dot = { .min = 25000, .max = 270000 },
-	.vco = { .min = 4000000, .max = 6000000 },
-	.n = { .min = 1, .max = 7 },
-	.m1 = { .min = 2, .max = 3 },
-	.m2 = { .min = 11, .max = 156 },
-	.p1 = { .min = 2, .max = 3 },
-	.p2 = { .p2_slow = 2, .p2_fast = 20 }, /* slow=min, fast=max */
-};
-
-static const intel_limit_t intel_limits_vlv_hdmi = {
+static const intel_limit_t intel_limits_vlv = {
 	.dot = { .min = 25000, .max = 270000 },
 	.vco = { .min = 4000000, .max = 6000000 },
 	.n = { .min = 1, .max = 7 },
@@ -390,10 +380,7 @@ static const intel_limit_t *intel_limit(struct drm_crtc *crtc, int refclk)
 		else
 			limit = &intel_limits_pineview_sdvo;
 	} else if (IS_VALLEYVIEW(dev)) {
-		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG))
-			limit = &intel_limits_vlv_dac;
-		else
-			limit = &intel_limits_vlv_hdmi;
+		limit = &intel_limits_vlv;
 	} else if (!IS_GEN2(dev)) {
 		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS))
 			limit = &intel_limits_i9xx_lvds;
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 14/14] drm/i915: Don't lie about findind suitable PLL settings on VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If vlv_find_best_dpll() couldn't find suitable PLL settings,
just say so instead of lying to caller.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b57ced9..c919582 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -663,6 +663,7 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 	unsigned int bestppm = 1000000;
 	/* min update 19.2 MHz */
 	int max_n = min(limit->n.max, refclk / 19200);
+	bool found = false;
 
 	target *= 5; /* fast clock */
 
@@ -691,18 +692,20 @@ vlv_find_best_dpll(const intel_limit_t *limit, struct drm_crtc *crtc,
 					if (ppm < 100 && clock.p > best_clock->p) {
 						bestppm = 0;
 						*best_clock = clock;
+						found = true;
 					}
 
 					if (bestppm >= 10 && ppm < bestppm - 10) {
 						bestppm = ppm;
 						*best_clock = clock;
+						found = true;
 					}
 				}
 			}
 		}
 	}
 
-	return true;
+	return found;
 }
 
 bool intel_crtc_active(struct drm_crtc *crtc)
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/14] drm/i915: VLV DPLL calc fixes and cleanups

[Daniel Vetter]

On Tue, Sep 24, 2013 at 09:26:17PM +0300, ville.syrjala@linux.intel.com wrote:
> I only wanted to do some small cleanups to vlv_find_best_dpll(),
> but it seems I went slightly mad again.
> 
> After realizing that I have to cook up quite a few more patches,
> I decided to also split up the functional changes from my earlier
> vlv_find_best_dpll() rewrite patch. And this series is the result.
> 
> And the usual warning applies: Totally untested

Can you pls also take a look at the DP regression due to Chon Lee's
patches that Jesse reported when you go about testing this stuff?
-Daniel

> 
> Ville Syrjälä (14):
>       drm/i915: Eliminate one indent leel from vlv_find_best_dpll
>       drm/i915: Use DIV_ROUND_CLOSEST()
>       drm/i915: Make vlv_find_best_dpll() ppm calculation safe
>       drm/i915: Don't underflow bestppm
>       drm/i915: Rewrite vlv_find_best_dpll()
>       drm/i915: De-magic the VLV p2 divider step size
>       drm/i915: Make sure we respect n.max on VLV
>       drm/i915: Clarify VLV PLL p1 limits
>       drm/i915: Allow p1 divider 2 on VLV
>       drm/i915: Respect p2 divider minimum limit on VLV
>       drm/i915: Remove the unused p and m limits for VLV
>       drm/i915: Remove unused dot_limit from VLV PLL limits
>       drm/i915: intel_limits_vlv_dac and intel_limits_vlv_hdmi are the same
>       drm/i915: Don't lie about findind suitable PLL settings on VLV
> 
>  drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++++++++++++++----------------------------------------------------
>  1 file changed, 49 insertions(+), 74 deletions(-)
> 
> _______________________________________________
> 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

[PATCH] drm/i915: Fix 1.62 DP DPLL settings for VLV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the recommended PLL dividers from
VLV2_DPLL_mphy_hsdpll_frequency_table_ww6_rev1p1.xlsm. The previous
values were really bogus. The 2.7 values look good however.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5e1de35..a5e4e61 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -59,7 +59,7 @@ static const struct dp_link_dpll pch_dpll[] = {
 
 static const struct dp_link_dpll vlv_dpll[] = {
 	{ DP_LINK_BW_1_62,
-		{ .p1 = 3, .p2 = 2, .n = 5, .m1 = 5, .m2 = 3 } },
+		{ .p1 = 3, .p2 = 2, .n = 5, .m1 = 3, .m2 = 81 } },
 	{ DP_LINK_BW_2_7,
 		{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } }
 };
-- 
1.8.1.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/14] drm/i915: VLV DPLL calc fixes and cleanups

[Mika Kuoppala]

ville.syrjala@linux.intel.com writes:

> I only wanted to do some small cleanups to vlv_find_best_dpll(),
> but it seems I went slightly mad again.
>
> After realizing that I have to cook up quite a few more patches,
> I decided to also split up the functional changes from my earlier
> vlv_find_best_dpll() rewrite patch. And this series is the result.
>
> And the usual warning applies: Totally untested
>
> Ville Syrjälä (14):
>       drm/i915: Eliminate one indent leel from vlv_find_best_dpll
>       drm/i915: Use DIV_ROUND_CLOSEST()
>       drm/i915: Make vlv_find_best_dpll() ppm calculation safe
>       drm/i915: Don't underflow bestppm
>       drm/i915: Rewrite vlv_find_best_dpll()
>       drm/i915: De-magic the VLV p2 divider step size
>       drm/i915: Make sure we respect n.max on VLV
>       drm/i915: Clarify VLV PLL p1 limits
>       drm/i915: Allow p1 divider 2 on VLV
>       drm/i915: Respect p2 divider minimum limit on VLV
>       drm/i915: Remove the unused p and m limits for VLV
>       drm/i915: Remove unused dot_limit from VLV PLL limits
>       drm/i915: intel_limits_vlv_dac and intel_limits_vlv_hdmi are the same
>       drm/i915: Don't lie about findind suitable PLL settings on VLV
>
>  drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++++++++++++++----------------------------------------------------
>  1 file changed, 49 insertions(+), 74 deletions(-)
>

I have read these through and tested the algorithm
against the values in the freq table excel sheet.

The pre-patchset algorithm sometimes skipped large but 
valid p1 values due to underflow of bestppm - 10.
Patch 04/15 fixes this issue.

After Ville's patches the find_best_dpll() 
seems to match exactly against what the sheet gives.
Atleast on selected set of rates.

And the whole thing is much more readable now, so:

Patches: 3-4, 5 v3, 6-14 and 15/14

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>









_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 00/14] drm/i915: VLV DPLL calc fixes and cleanups

[Daniel Vetter]

On Mon, Sep 30, 2013 at 06:06:09PM +0300, Mika Kuoppala wrote:
> ville.syrjala@linux.intel.com writes:
> 
> > I only wanted to do some small cleanups to vlv_find_best_dpll(),
> > but it seems I went slightly mad again.
> >
> > After realizing that I have to cook up quite a few more patches,
> > I decided to also split up the functional changes from my earlier
> > vlv_find_best_dpll() rewrite patch. And this series is the result.
> >
> > And the usual warning applies: Totally untested
> >
> > Ville Syrjälä (14):
> >       drm/i915: Eliminate one indent leel from vlv_find_best_dpll
> >       drm/i915: Use DIV_ROUND_CLOSEST()
> >       drm/i915: Make vlv_find_best_dpll() ppm calculation safe
> >       drm/i915: Don't underflow bestppm
> >       drm/i915: Rewrite vlv_find_best_dpll()
> >       drm/i915: De-magic the VLV p2 divider step size
> >       drm/i915: Make sure we respect n.max on VLV
> >       drm/i915: Clarify VLV PLL p1 limits
> >       drm/i915: Allow p1 divider 2 on VLV
> >       drm/i915: Respect p2 divider minimum limit on VLV
> >       drm/i915: Remove the unused p and m limits for VLV
> >       drm/i915: Remove unused dot_limit from VLV PLL limits
> >       drm/i915: intel_limits_vlv_dac and intel_limits_vlv_hdmi are the same
> >       drm/i915: Don't lie about findind suitable PLL settings on VLV
> >
> >  drivers/gpu/drm/i915/intel_display.c | 123 +++++++++++++++++++++++++++++++++++----------------------------------------------------
> >  1 file changed, 49 insertions(+), 74 deletions(-)
> >
> 
> I have read these through and tested the algorithm
> against the values in the freq table excel sheet.
> 
> The pre-patchset algorithm sometimes skipped large but 
> valid p1 values due to underflow of bestppm - 10.
> Patch 04/15 fixes this issue.
> 
> After Ville's patches the find_best_dpll() 
> seems to match exactly against what the sheet gives.
> Atleast on selected set of rates.
> 
> And the whole thing is much more readable now, so:
> 
> Patches: 3-4, 5 v3, 6-14 and 15/14
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

I've merged them all - we're early in the -testing cycle so fixing fallout
(if there is any) shouldn't be too ugly. Thanks for the patches&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch