Hi Laurent
Thank you for your feedback
> > + * NOTES
> > + * N = (n + 1), M = (m + 1), P = 2
> > + * 2000 < fvco < 4096Mhz
>
> Are you sure that the fvco constraint is really 2kHz, and not 2GHz ? 2kHz -
> 4GHz would be a surprisingly large range.
It is 2kHz. This is came from HW team, and indicated
on HW design sheet (?)
> > + * Basically M=1
>
> Why is this ?
This is came from HW team, too.
They are assuming M=1, basically.
But yes confusable, let's remove it from comment.
m is started from 0 (= M=1), no need to explain.
> > + for (m = 0; m < 4; m++) {
> > + for (n = 119; n > 38; n--) {
> > + unsigned long long fvco = input * 2 * (n + 1) / (m + 1);
>
> This code runs for Gen3 only, so unsigned long would be enough. The rest of
> the function already relies on native support for 64-bit calculation. If you
> wanted to run this on a 32-bit CPU, you would likely need to do_div() for the
> division, and convert input to u64 to avoid integer overflows, otherwise the
> calculation will be performed on 32-bit before a final conversion to 64-bit.
(snip)
> > + if ((fvco < 2000) ||
> > + (fvco > 4096000000ll))
>
> No need for the inner parentheses, and you can write both conditions on a
> single line. Furthemore 4096 MHz will fit in a 32-bit number, so there's no
> need for the ll.
Yes, but compiled by 32bit too, right ?
Without this "ll", 32bit compiler say
warning: this decimal constant is unsigned only in ISO C90
# anyway, I will add this assumption (= used only by 64bit CPU)
# on comment to avoid future confusion
> I think you can then drop the output >= 4000000000 check inside the inner
> fdpll loop, as the output frequency can't be higher than 4GHz if the VCO
> frequency isn't.
I think code has
if (output >= 400000000)
This is 400MHz, not 4GHz
> > for (fdpll = 1; fdpll < 32; fdpll++) {
> > unsigned long output;
>
> The output frequency on the line below can be calculated with
>
> output = fvco / 2 / (fdpll + 1)
>
> to avoid the multiplication by (n + 1) and division by (m + 1).
It is nice idea to avoid extra calculation.
I will use this idea, and add extrate comment to avoid confusion
Best regards
---
Kuninori Morimoto
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
