Re: [PATCH v2 1/2] clk: sunxi-ng: nkm: consider alternative parent rates when finding rate

[Maxime Ripard]

On Sun, Jun 25, 2023 at 12:45:45PM +0200, Frank Oltmanns wrote:
> Hi Maxime,
> 
> On 2023-06-12 at 14:31:21 +0200, Maxime Ripard <maxime@xxxxxxxxxx> wrote:
> > [[PGP Signed Part:Undecided]]
> > On Mon, Jun 12, 2023 at 10:51:52AM +0200, Frank Oltmanns wrote:
> >> > @@ -28,12 +68,17 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
> >> >  			for (_m = nkm->min_m; _m <= nkm->max_m; _m++) {
> >>
> >> According to the manual M/N has to be <= 3. Therefore we need a
> >> different maximum value for the _m-for-loop:
> >>
> >>         unsigned long max_m = min(3 * _n, nkm->max_m);
> >>         for (_m = nkm->min_m; _m <= max_m; _m++) {
> >>
> >> I suggest that I add an optional member max_mn_ratio to the structs
> >> ccu_nkm and _ccu_nkm. Optional meaning: Ignore if 0.
> >
> > Which workload is affected by this restriction?
> >
> 
> Firstly, the restriction increases the minimum rate that pll-mipi of the
> A64 SoC can use. The rate off pll-mipi is
>         pll-video0 * K * N / M
> 
> The Allwinner's user manual ([1], p.94) states that the maximum ratio of
> M/N (note how numerator and denominator changed) is 3. So, looking back
> to the original formula, the N / M part can be at most 1/3. That
> effectively limits the minimum rate that pll-mipi can provide to
>         min(pll-video0) * 2 * 1 / 3
> 
> The minimum rate of pll-video0 is 192 MHz, i.e., the minimum rate for
> pll-mipi becomes 128 MHz. Without the restriction, the minimum rate
> currently is 24 MHz. It is my (albeit limited) understanding, that is no
> real limitation because no panel would request such low rates. I should
> also mention that Allwinner states in the user manual ([1], p. 94) that
> the rate must be in the 500 MHz - 1.4 GHz range.
> 
> Secondly, it decreases the number of options for M for all N <= 6.
> Therefore it reduces the number of meaningful NKM combinations from 275
> (without the restriction) to 238. (With meaningful combinations, I mean
> the combinations that result in a different rate for pll-mipi, e.g.,
> K=2, M=1, N=2 is the same as K=4, M=1, N=1). The consequence is that the
> precision of pll-mipi is slightly reduced. Note, however, that this loss
> of precision is more than offset by the option that pll-mipi can now
> "freely" choose its parent rate.
> 
> In conclusion, I don't see any real world limitation that this
> restriction introduces.

If we want to go that way, I'd rather use a function that validates
whether or not the current set of parameter is valid.

That way, we can express pretty much any constraints without
special-casing the main structure too much.

Maxime

Attachment: signature.asc
Description: PGP signature