Re: [Intel-gfx] [PATCH] drm/i915/skl: Skip remaining dividers when deviation is 0

2015-06-28 Thread shuang . he 
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: 
shuang...@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  Testdrm-intel-nightly  Series 
Applied
Note: You need to pay more attention to line start with '*'
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/skl: Skip remaining dividers when deviation is 0

2015-06-26 Thread Damien Lespiau 
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.zan...@intel.com

I like that, v2 it is!

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

Re: [Intel-gfx] [PATCH] drm/i915/skl: Skip remaining dividers when deviation is 0

2015-06-26 Thread Paulo Zanoni 
2015-06-25 14:08 GMT-03:00 Damien Lespiau damien.lesp...@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.zan...@intel.com
 Signed-off-by: Damien Lespiau damien.lesp...@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.zan...@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



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

[Intel-gfx] [PATCH] drm/i915/skl: Skip remaining dividers when deviation is 0

2015-06-25 Thread Damien Lespiau 
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.zan...@intel.com
Signed-off-by: Damien Lespiau damien.lesp...@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;
/* 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.
-- 
2.1.0

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