2015-06-25 12:15 GMT-03:00 Damien Lespiau <damien.lesp...@intel.com>:
> The HW validation team came back from further testing with a slightly
> changed constraint on the deviation between the DCO frequency and the
> central frequency. Instead of +-4%, it's now +1%/-6%.
>
> Unfortunately, the previous algorithm didn't quite cope with these new
> constraints, the reason being that it wasn't thorough enough looking at
> the possible divider candidates.
>
> The new algorithm looks at all dividers, which is definitely a hammer
> approach (we could reduce further the set of dividers to good ones as a
> follow up, at the cost of a bit more complicated code). But, at least,
> we can now satisfy the +1%/+6% rule for all the "Well known" HDMI
> frequencies of my test set (373 entries).
>
> On that subject, the new code is quite extensively tested in
> intel-gpu-tools (tools/skl_compute_wrpll).
>
> v2: Fix cycling between central frequencies and dividers (Paulo)
> Properly choose the minimal deviation between postive and negative
> candidates (Paulo).
>
> On the 373 test frequencies, v2 computes better dividers than v1 (ie
> more even dividers and lower deviation on average):
>
> v1: average deviation: 206.52
> v2: average deviation: 194.47
>
> Signed-off-by: Damien Lespiau <damien.lesp...@intel.com>
Reviewed-by: Paulo Zanoni <paulo.r.zan...@intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 211
> +++++++++++++++++++++++++--------------
> 1 file changed, 137 insertions(+), 74 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> b/drivers/gpu/drm/i915/intel_ddi.c
> index 31b29e8..6e964ef 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1104,6 +1104,103 @@ hsw_ddi_pll_select(struct intel_crtc *intel_crtc,
> return true;
> }
>
> +struct skl_wrpll_context {
> + uint64_t min_deviation; /* current minimal deviation */
> + uint64_t central_freq; /* chosen central freq */
> + uint64_t dco_freq; /* chosen dco freq */
> + unsigned int p; /* chosen divider */
> +};
> +
> +static void skl_wrpll_context_init(struct skl_wrpll_context *ctx)
> +{
> + memset(ctx, 0, sizeof(*ctx));
> +
> + ctx->min_deviation = U64_MAX;
> +}
> +
> +/* DCO freq must be within +1%/-6% of the DCO central freq */
> +#define SKL_DCO_MAX_PDEVIATION 100
> +#define SKL_DCO_MAX_NDEVIATION 600
> +
> +static void skl_wrpll_try_divider(struct skl_wrpll_context *ctx,
> + uint64_t central_freq,
> + uint64_t dco_freq,
> + unsigned int divider)
> +{
> + uint64_t deviation;
> +
> + deviation = div64_u64(10000 * abs_diff(dco_freq, central_freq),
> + central_freq);
> +
> + /* positive deviation */
> + if (dco_freq >= central_freq) {
> + if (deviation < SKL_DCO_MAX_PDEVIATION &&
> + deviation < ctx->min_deviation) {
> + ctx->min_deviation = deviation;
> + ctx->central_freq = central_freq;
> + ctx->dco_freq = dco_freq;
> + ctx->p = divider;
> + }
> + /* negative deviation */
> + } else if (deviation < SKL_DCO_MAX_NDEVIATION &&
> + deviation < ctx->min_deviation) {
> + ctx->min_deviation = deviation;
> + ctx->central_freq = central_freq;
> + ctx->dco_freq = dco_freq;
> + ctx->p = divider;
> + }
> +
> +}
> +
> +static void skl_wrpll_get_multipliers(unsigned int p,
> + unsigned int *p0 /* out */,
> + unsigned int *p1 /* out */,
> + unsigned int *p2 /* out */)
> +{
> + /* even dividers */
> + if (p % 2 == 0) {
> + unsigned int half = p / 2;
> +
> + if (half == 1 || half == 2 || half == 3 || half == 5) {
> + *p0 = 2;
> + *p1 = 1;
> + *p2 = half;
> + } else if (half % 2 == 0) {
> + *p0 = 2;
> + *p1 = half / 2;
> + *p2 = 2;
> + } else if (half % 3 == 0) {
> + *p0 = 3;
> + *p1 = half / 3;
> + *p2 = 2;
> + } else if (half % 7 == 0) {
> + *p0 = 7;
> + *p1 = half / 7;
> + *p2 = 2;
> + }
> + } else if (p == 3 || p == 9) { /* 3, 5, 7, 9, 15, 21, 35 */
> + *p0 = 3;
> + *p1 = 1;
> + *p2 = p / 3;
> + } else if (p == 5 || p == 7) {
> + *p0 = p;
> + *p1 = 1;
> + *p2 = 1;
> + } else if (p == 15) {
> + *p0 = 3;
> + *p1 = 1;
> + *p2 = 5;
> + } else if (p == 21) {
> + *p0 = 7;
> + *p1 = 1;
> + *p2 = 3;
> + } else if (p == 35) {
> + *p0 = 7;
> + *p1 = 1;
> + *p2 = 5;
> + }
> +}
> +
> struct skl_wrpll_params {
> uint32_t dco_fraction;
> uint32_t dco_integer;
> @@ -1189,90 +1286,56 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
> uint64_t dco_central_freq[3] = {8400000000ULL,
> 9000000000ULL,
> 9600000000ULL};
> - uint32_t min_dco_deviation = 400;
> - uint32_t min_dco_index = 3;
> - uint32_t P0[4] = {1, 2, 3, 7};
> - uint32_t P2[4] = {1, 2, 3, 5};
> - bool found = false;
> - uint32_t candidate_p = 0;
> - uint32_t candidate_p0[3] = {0}, candidate_p1[3] = {0};
> - uint32_t candidate_p2[3] = {0};
> - uint32_t dco_central_freq_deviation[3];
> - uint32_t i, P1, k, dco_count;
> - bool retry_with_odd = false;
> -
> - /* Determine P0, P1 or P2 */
> - for (dco_count = 0; dco_count < 3; dco_count++) {
> - found = false;
> - candidate_p =
> - div64_u64(dco_central_freq[dco_count], afe_clock);
> - if (retry_with_odd == false)
> - candidate_p = (candidate_p % 2 == 0 ?
> - candidate_p : candidate_p + 1);
> -
> - for (P1 = 1; P1 < candidate_p; P1++) {
> - for (i = 0; i < 4; i++) {
> - if (!(P0[i] != 1 || P1 == 1))
> - continue;
> -
> - for (k = 0; k < 4; k++) {
> - if (P1 != 1 && P2[k] != 2)
> - continue;
> -
> - if (candidate_p == P0[i] * P1 *
> P2[k]) {
> - /* Found possible P0, P1, P2
> */
> - found = true;
> - candidate_p0[dco_count] =
> P0[i];
> - candidate_p1[dco_count] = P1;
> - candidate_p2[dco_count] =
> P2[k];
> - goto found;
> - }
> -
> - }
> - }
> - }
> -
> -found:
> - if (found) {
> - dco_central_freq_deviation[dco_count] =
> - div64_u64(10000 *
> - abs_diff(candidate_p * afe_clock,
> -
> dco_central_freq[dco_count]),
> - dco_central_freq[dco_count]);
> -
> - if (dco_central_freq_deviation[dco_count] <
> - min_dco_deviation) {
> - min_dco_deviation =
> - dco_central_freq_deviation[dco_count];
> - min_dco_index = dco_count;
> + static const int even_dividers[] = { 4, 6, 8, 10, 12, 14, 16, 18,
> 20,
> + 24, 28, 30, 32, 36, 40, 42, 44,
> + 48, 52, 54, 56, 60, 64, 66, 68,
> + 70, 72, 76, 78, 80, 84, 88, 90,
> + 92, 96, 98 };
> + static const int odd_dividers[] = { 3, 5, 7, 9, 15, 21, 35 };
> + static const struct {
> + const int *list;
> + int n_dividers;
> + } dividers[] = {
> + { even_dividers, ARRAY_SIZE(even_dividers) },
> + { odd_dividers, ARRAY_SIZE(odd_dividers) },
> + };
> + struct skl_wrpll_context ctx;
> + unsigned int dco, d, i;
> + unsigned int p0, p1, p2;
> +
> + skl_wrpll_context_init(&ctx);
> +
> + for (d = 0; d < ARRAY_SIZE(dividers); d++) {
> + for (dco = 0; dco < ARRAY_SIZE(dco_central_freq); dco++) {
> + for (i = 0; i < dividers[d].n_dividers; i++) {
> + 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 (min_dco_index > 2 && dco_count == 2) {
> - /* oh well, we tried... */
> - if (retry_with_odd)
> - break;
> -
> - retry_with_odd = true;
> - dco_count = 0;
> - }
> }
>
> - if (WARN(min_dco_index > 2,
> - "No valid parameters found for pixel clock: %dHz\n", clock))
> + if (!ctx.p) {
> + DRM_DEBUG_DRIVER("No valid divider found for %dHz\n", clock);
> return false;
> + }
>
> - skl_wrpll_params_populate(wrpll_params,
> - afe_clock,
> - dco_central_freq[min_dco_index],
> - candidate_p0[min_dco_index],
> - candidate_p1[min_dco_index],
> - candidate_p2[min_dco_index]);
> + /*
> + * gcc incorrectly analyses that these can be used without being
> + * initialized. To be fair, it's hard to guess.
> + */
> + p0 = p1 = p2 = 0;
> + skl_wrpll_get_multipliers(ctx.p, &p0, &p1, &p2);
> + skl_wrpll_params_populate(wrpll_params, afe_clock, ctx.central_freq,
> + p0, p1, p2);
>
> return true;
> }
>
> -
> static bool
> skl_ddi_pll_select(struct intel_crtc *intel_crtc,
> struct intel_crtc_state *crtc_state,
> --
> 2.1.0
>
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
