On Wed, May 27, 2015 at 06:28:06PM -0300, Paulo Zanoni wrote:
> > @@ -1185,90 +1278,56 @@ skl_ddi_calculate_wrpll(int clock /* in Hz */,
> > uint64_t dco_central_freq[3] = {8400000000ULL,
> > 9000000000ULL,
> > 9600000000ULL};
> > + 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 (dco = 0; dco < ARRAY_SIZE(dco_central_freq); dco++) {
> > + for (d = 0; d < ARRAY_SIZE(dividers); d++) {
>
> From what I can tell by reading the documentation, it really seems
> that the two lines above should be inverted. First we do everything
> for the even (including all DCOs), and only then for the odd. It's
> easier to realize this when you look at the part of the doc that says,
> in order: "next candidate divider \n next DCO central frequency \n
> next divider parity". Anyway, this is not exactly a problem right now
> since we never break the loop, but you change this on the next patch,
> so it's probably best to discuss this here.
You're absolutely right, those 2 lines need to be inverted.
> > + /*
> > + * gcc incorrectly analyses that these can be used without being
> > + * initialized. To be fair, it's hard to guess.
>
> Why didn't you just write the mathematical proof as a comment? I hope
> it's not too big to fit on the 80 column margin limit.
>
> Jokes aside, the only "blocker" would be the DCO first vs odd/even
> first. I'd like to hear your interpretation on this.
>
> > + */
> > + 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);
The "proof" is in tools/skl_compute_wrpll.c, in the test_multipliers()
function that unit-tests skl_wrpll_get_multipliers().
--
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
