diff mbox

drm/i915/skl: Skip remaining dividers when deviation is 0

Message ID 1435252100-8234-1-git-send-email-damien.lespiau@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lespiau, Damien June 25, 2015, 5:08 p.m. UTC
We can't improve a 0 deviation, so when we find such a divider, skip the
remaining ones they won't be better.

This short-circuit the search for 34 of the 373 test frequencies in the
corresponding i-g-t test (tools/skl_compute_wrpll)

Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
---
This patch has to be applied after patch 12 and 13 for the recent SKL DPLL
series.

 drivers/gpu/drm/i915/intel_ddi.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

Comments

Paulo Zanoni June 26, 2015, 5:18 p.m. UTC | #1
2015-06-25 14:08 GMT-03:00 Damien Lespiau <damien.lespiau@intel.com>:
> We can't improve a 0 deviation, so when we find such a divider, skip the
> remaining ones they won't be better.
>
> This short-circuit the search for 34 of the 373 test frequencies in the
> corresponding i-g-t test (tools/skl_compute_wrpll)
>
> Suggested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> This patch has to be applied after patch 12 and 13 for the recent SKL DPLL
> series.
>
>  drivers/gpu/drm/i915/intel_ddi.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f6b3ccc..45116d8 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1122,7 +1122,11 @@ static void skl_wrpll_context_init(struct skl_wrpll_context *ctx)
>  #define SKL_DCO_MAX_PDEVIATION 100
>  #define SKL_DCO_MAX_NDEVIATION 600
>
> -static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
> +/*
> + * Returns true if we're sure to have found the definitive divider (ie
> + * deviation == 0).
> + */
> +static bool skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
>                                   uint64_t central_freq,
>                                   uint64_t dco_freq,
>                                   unsigned int divider)
> @@ -1141,6 +1145,10 @@ static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
>                         ctx->dco_freq = dco_freq;
>                         ctx->p = divider;
>                 }
> +
> +               /* we can't improve a 0 deviation */
> +               if (deviation == 0)
> +                       return true;

Took me a while to understand why this was exactly here :)


>         /* negative deviation */
>         } else if (deviation < SKL_DCO_MAX_NDEVIATION &&
>                    deviation < ctx->min_deviation) {
> @@ -1150,6 +1158,7 @@ static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
>                 ctx->p = divider;
>         }
>
> +       return false;
>  }
>
>  static void skl_wrpll_get_multipliers(unsigned int p,
> @@ -1311,13 +1320,15 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
>                                 unsigned int p = dividers[d].list[i];
>                                 uint64_t dco_freq = p * afe_clock;
>
> -                               skl_wrpll_try_divider(&ctx,
> -                                                     dco_central_freq[dco],
> -                                                     dco_freq,
> -                                                     p);
> +                               if (skl_wrpll_try_divider(&ctx,
> +                                                         dco_central_freq[dco],
> +                                                         dco_freq,
> +                                                         p))
> +                                       goto skip_remaining_dividers;

Bikeshed: instead of touching skl_wrpll_try_divider(), you could just:

if (ctx.min_deviation == 0) goto skip_remaining_dividers;

That would keep the logic of the optimization restricted to this
function. IMHO, much simpler.

With or without changes: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>                         }
>                 }
>
> +skip_remaining_dividers:
>                 /*
>                  * If a solution is found with an even divider, prefer
>                  * this one.
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lespiau, Damien June 26, 2015, 5:23 p.m. UTC | #2
On Fri, Jun 26, 2015 at 02:18:39PM -0300, Paulo Zanoni wrote:
> > @@ -1311,13 +1320,15 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
> >                                 unsigned int p = dividers[d].list[i];
> >                                 uint64_t dco_freq = p * afe_clock;
> >
> > -                               skl_wrpll_try_divider(&ctx,
> > -                                                     dco_central_freq[dco],
> > -                                                     dco_freq,
> > -                                                     p);
> > +                               if (skl_wrpll_try_divider(&ctx,
> > +                                                         dco_central_freq[dco],
> > +                                                         dco_freq,
> > +                                                         p))
> > +                                       goto skip_remaining_dividers;
> 
> Bikeshed: instead of touching skl_wrpll_try_divider(), you could just:
> 
> if (ctx.min_deviation == 0) goto skip_remaining_dividers;
> 
> That would keep the logic of the optimization restricted to this
> function. IMHO, much simpler.
> 
> With or without changes: Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I like that, v2 it is!
Shuang He June 29, 2015, 4:57 a.m. UTC | #3
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6633
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                                  302/302              302/302
SNB                                  312/316              312/316
IVB                                  343/343              343/343
BYT                                  287/287              287/287
HSW                                  380/380              380/380
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f6b3ccc..45116d8 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1122,7 +1122,11 @@  static void skl_wrpll_context_init(struct skl_wrpll_context *ctx)
 #define SKL_DCO_MAX_PDEVIATION	100
 #define SKL_DCO_MAX_NDEVIATION	600
 
-static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
+/*
+ * Returns true if we're sure to have found the definitive divider (ie
+ * deviation == 0).
+ */
+static bool skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
 				  uint64_t central_freq,
 				  uint64_t dco_freq,
 				  unsigned int divider)
@@ -1141,6 +1145,10 @@  static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
 			ctx->dco_freq = dco_freq;
 			ctx->p = divider;
 		}
+
+		/* we can't improve a 0 deviation */
+		if (deviation == 0)
+			return true;
 	/* negative deviation */
 	} else if (deviation < SKL_DCO_MAX_NDEVIATION &&
 		   deviation < ctx->min_deviation) {
@@ -1150,6 +1158,7 @@  static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
 		ctx->p = divider;
 	}
 
+	return false;
 }
 
 static void skl_wrpll_get_multipliers(unsigned int p,
@@ -1311,13 +1320,15 @@  skl_ddi_calculate_wrpll(int clock /* in Hz */,
 				unsigned int p = dividers[d].list[i];
 				uint64_t dco_freq = p * afe_clock;
 
-				skl_wrpll_try_divider(&ctx,
-						      dco_central_freq[dco],
-						      dco_freq,
-						      p);
+				if (skl_wrpll_try_divider(&ctx,
+							  dco_central_freq[dco],
+							  dco_freq,
+							  p))
+					goto skip_remaining_dividers;
 			}
 		}
 
+skip_remaining_dividers:
 		/*
 		 * If a solution is found with an even divider, prefer
 		 * this one.