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

[Paulo Zanoni]

2015-06-25 14:08 GMT-03:00 Damien Lespiau <damien.lespiau at 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 at intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau at 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 at 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni