[PATCH v2 0/2] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate

[PATCH v2 0/2] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate

[Frank Oltmanns]

This is V2 of a patchset that enables NKM clocks to consider alternative parent
rates and utilize this new feature to adjust the pll-video0 clock on Allwinner
A64.

This allows to achieve an optimal rate for driving the board's panel.

To provide some context, the clock structure involved in this process is as follows:
    clock                       clock type
    --------------------------------------
    pll-video0                  ccu_nm
       pll-mipi                 ccu_nkm
          tcon0                 ccu_mux
             tcon-data-clock    sun4i_dclk

The divider between tcon0 and tcon-data-clock is fixed at 4. Therefore, in order
to achieve a rate that closely matches the desired rate of the panel, pll-mipi
needs to operate at a specific rate.

Changes in V2:
 - Move optimal parent rate calculation to dedicated function
 - Choose a parent rate that does not to overshoot requested rate
 - Add comments to ccu_nkm_find_best
 - Make sure that best_parent_rate stays at original parent rate in the unlikely
   case that all combinations overshoot.

Link to V1:
https://lore.kernel.org/lkml/20230605190745.366882-1-frank@oltmanns.dev/

Frank Oltmanns (2):
  clk: sunxi-ng: nkm: consider alternative parent rates when finding
    rate
  clk: sunxi-ng: a64: allow pll-mipi to set parent's rate

 drivers/clk/sunxi-ng/ccu-sun50i-a64.c |  3 +-
 drivers/clk/sunxi-ng/ccu_nkm.c        | 66 +++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 9 deletions(-)

-- 
2.41.0

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

[Frank Oltmanns]

In case the CLK_SET_RATE_PARENT flag is set, consider using a different
parent rate when determining a new rate.

To find the best match for the requested rate, perform the following
steps for each NKM combination:
 - calculate the optimal parent rate,
 - find the best parent rate that the parent clock actually supports
 - use that parent rate to calculate the effective rate.

In case the clk does not support setting the parent rate, use the same
algorithm as before.

Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
---
 drivers/clk/sunxi-ng/ccu_nkm.c | 66 +++++++++++++++++++++++++++++-----
 1 file changed, 58 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c
index a0978a50edae..c49d5879fe73 100644
--- a/drivers/clk/sunxi-ng/ccu_nkm.c
+++ b/drivers/clk/sunxi-ng/ccu_nkm.c
@@ -6,6 +6,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/io.h>
+#include <linux/math.h>
 
 #include "ccu_gate.h"
 #include "ccu_nkm.h"
@@ -16,10 +17,49 @@ struct _ccu_nkm {
 	unsigned long	m, min_m, max_m;
 };
 
-static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
-				       struct _ccu_nkm *nkm)
+static unsigned long optimal_parent_rate(unsigned long rate, unsigned long n,
+					 unsigned long k, unsigned long m)
 {
-	unsigned long best_rate = 0;
+	unsigned long _rate, parent;
+
+	// We must first try to find the desired parent rate that is rounded up, because there are
+	// cases where truncating makes us miss the requested rate.
+	// E.g. rate=449035712, n=11, k=3, m=16
+	// When truncating, we'd get parent=217714284 and calculating the rate from that would give
+	// us 449035710. When rounding up, we get parent=217714285 which would give us the requested
+	// rate of 449035712.
+	parent = DIV_ROUND_UP(rate * m, n * k);
+
+	// But there are other cases, where rounding up the parent gives us a too high rate.
+	// Therefore, we need to check for this case and, if necessary, truncate the parent rate
+	// instead of rounding up.
+	_rate = parent * n * k / m;
+	if (_rate > rate)
+		parent = rate * m / (n * k);
+	return parent;
+}
+
+/**
+ * ccu_nkm_find_best - Find the best nkm combination for a given rate
+ *
+ * @parent: rate of parent clock. This is used either as an input or out parameter:
+ *           - In cases where the parent clock can be set, this parameter will be updated to contain
+ *             the optimal rate for the parent to achieve the best rate for the nkm clock.
+ *           - In cases where the parent clock can not be set, this parameter must contain the
+ *             current rate of the parent, which is used to determine the best combination of n, k,
+ *             and m.
+ * @rate: requested rate.
+ * @nkm: Input/output parameter that contains the clocks constraints on the n, k, m combinations and
+ *       is updated in this function to contain the resulting best n, k, m combination.
+ * @parent_hw: parent clock. If set, this function assumes that the parent clock can be updated to a
+ *             rate that would be best to in order to get as close as possible to @rate. This
+ *             parameter must be set to NULL if this function shall not try to find the optimal
+ *             parent rate for the requested rate.
+ */
+static unsigned long ccu_nkm_find_best(unsigned long *parent, unsigned long rate,
+				       struct _ccu_nkm *nkm, struct clk_hw *parent_hw)
+{
+	unsigned long best_rate = 0, best_parent_rate = *parent, tmp_parent = *parent;
 	unsigned long best_n = 0, best_k = 0, best_m = 0;
 	unsigned long _n, _k, _m;
 
@@ -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++) {
 				unsigned long tmp_rate;
 
-				tmp_rate = parent * _n * _k / _m;
-
+				if (parent_hw) {
+					tmp_parent = optimal_parent_rate(rate, _n, _k, _m);
+					tmp_parent = clk_hw_round_rate(parent_hw, tmp_parent);
+				}
+				tmp_rate = tmp_parent * _n * _k / _m;
 				if (tmp_rate > rate)
 					continue;
+
 				if ((rate - tmp_rate) < (rate - best_rate)) {
 					best_rate = tmp_rate;
+					best_parent_rate = tmp_parent;
 					best_n = _n;
 					best_k = _k;
 					best_m = _m;
@@ -46,6 +91,8 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
 	nkm->k = best_k;
 	nkm->m = best_m;
 
+	*parent = best_parent_rate;
+
 	return best_rate;
 }
 
@@ -106,7 +153,7 @@ static unsigned long ccu_nkm_recalc_rate(struct clk_hw *hw,
 }
 
 static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
-					struct clk_hw *hw,
+					struct clk_hw *parent_hw,
 					unsigned long *parent_rate,
 					unsigned long rate,
 					void *data)
@@ -124,7 +171,10 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
 	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
 		rate *= nkm->fixed_post_div;
 
-	rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm);
+	if (!clk_hw_can_set_rate_parent(&nkm->common.hw))
+		rate = ccu_nkm_find_best(parent_rate, rate, &_nkm, NULL);
+	else
+		rate = ccu_nkm_find_best(parent_rate, rate, &_nkm, parent_hw);
 
 	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
 		rate /= nkm->fixed_post_div;
@@ -159,7 +209,7 @@ static int ccu_nkm_set_rate(struct clk_hw *hw, unsigned long rate,
 	_nkm.min_m = 1;
 	_nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
 
-	ccu_nkm_find_best(parent_rate, rate, &_nkm);
+	ccu_nkm_find_best(&parent_rate, rate, &_nkm, NULL);
 
 	spin_lock_irqsave(nkm->common.lock, flags);
 
-- 
2.41.0

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

[Frank Oltmanns]

Hi all,

I just found in the Allwinner A64 User Manual V1.0, that there are
additional constraints on the combinations for pll-mipi, that are not
(and never were) enforced by by ccu_nkm.

On 2023-06-11 at 11:01:42 +0200, Frank Oltmanns <frank@oltmanns.dev> wrote:
> In case the CLK_SET_RATE_PARENT flag is set, consider using a different
> parent rate when determining a new rate.
>
> To find the best match for the requested rate, perform the following
> steps for each NKM combination:
>  - calculate the optimal parent rate,
>  - find the best parent rate that the parent clock actually supports
>  - use that parent rate to calculate the effective rate.
>
> In case the clk does not support setting the parent rate, use the same
> algorithm as before.
>
> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> ---
>  drivers/clk/sunxi-ng/ccu_nkm.c | 66 +++++++++++++++++++++++++++++-----
>  1 file changed, 58 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c
> index a0978a50edae..c49d5879fe73 100644
> --- a/drivers/clk/sunxi-ng/ccu_nkm.c
> +++ b/drivers/clk/sunxi-ng/ccu_nkm.c
> @@ -6,6 +6,7 @@
>
>  #include <linux/clk-provider.h>
>  #include <linux/io.h>
> +#include <linux/math.h>
>
>  #include "ccu_gate.h"
>  #include "ccu_nkm.h"
> @@ -16,10 +17,49 @@ struct _ccu_nkm {
>  	unsigned long	m, min_m, max_m;
>  };
>
> -static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
> -				       struct _ccu_nkm *nkm)
> +static unsigned long optimal_parent_rate(unsigned long rate, unsigned long n,
> +					 unsigned long k, unsigned long m)
>  {
> -	unsigned long best_rate = 0;
> +	unsigned long _rate, parent;
> +
> +	// We must first try to find the desired parent rate that is rounded up, because there are
> +	// cases where truncating makes us miss the requested rate.
> +	// E.g. rate=449035712, n=11, k=3, m=16
> +	// When truncating, we'd get parent=217714284 and calculating the rate from that would give
> +	// us 449035710. When rounding up, we get parent=217714285 which would give us the requested
> +	// rate of 449035712.
> +	parent = DIV_ROUND_UP(rate * m, n * k);
> +
> +	// But there are other cases, where rounding up the parent gives us a too high rate.
> +	// Therefore, we need to check for this case and, if necessary, truncate the parent rate
> +	// instead of rounding up.
> +	_rate = parent * n * k / m;
> +	if (_rate > rate)
> +		parent = rate * m / (n * k);
> +	return parent;
> +}
> +
> +/**
> + * ccu_nkm_find_best - Find the best nkm combination for a given rate
> + *
> + * @parent: rate of parent clock. This is used either as an input or out parameter:
> + *           - In cases where the parent clock can be set, this parameter will be updated to contain
> + *             the optimal rate for the parent to achieve the best rate for the nkm clock.
> + *           - In cases where the parent clock can not be set, this parameter must contain the
> + *             current rate of the parent, which is used to determine the best combination of n, k,
> + *             and m.
> + * @rate: requested rate.
> + * @nkm: Input/output parameter that contains the clocks constraints on the n, k, m combinations and
> + *       is updated in this function to contain the resulting best n, k, m combination.
> + * @parent_hw: parent clock. If set, this function assumes that the parent clock can be updated to a
> + *             rate that would be best to in order to get as close as possible to @rate. This
> + *             parameter must be set to NULL if this function shall not try to find the optimal
> + *             parent rate for the requested rate.
> + */
> +static unsigned long ccu_nkm_find_best(unsigned long *parent, unsigned long rate,
> +				       struct _ccu_nkm *nkm, struct clk_hw *parent_hw)
> +{
> +	unsigned long best_rate = 0, best_parent_rate = *parent, tmp_parent = *parent;
>  	unsigned long best_n = 0, best_k = 0, best_m = 0;
>  	unsigned long _n, _k, _m;
>
> @@ -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.

>  				unsigned long tmp_rate;
>
> -				tmp_rate = parent * _n * _k / _m;
> -
> +				if (parent_hw) {
> +					tmp_parent = optimal_parent_rate(rate, _n, _k, _m);
> +					tmp_parent = clk_hw_round_rate(parent_hw, tmp_parent);
> +				}

Another constraint is PLL-VIDEO0 rate / M >= 24 MHz. Therefore we also
need:
        if (tmp_parent < 24000000 * _m)
                continue;

So, we need another optional member min_m_times_parent or, for
shortness, maybe min_m_parent. I could use help finding a good name for
this.

I guess there needs to be a V3 of this patchset. :)

Regards,
  Frank

> +				tmp_rate = tmp_parent * _n * _k / _m;
>  				if (tmp_rate > rate)
>  					continue;
> +
>  				if ((rate - tmp_rate) < (rate - best_rate)) {
>  					best_rate = tmp_rate;
> +					best_parent_rate = tmp_parent;
>  					best_n = _n;
>  					best_k = _k;
>  					best_m = _m;
> @@ -46,6 +91,8 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
>  	nkm->k = best_k;
>  	nkm->m = best_m;
>
> +	*parent = best_parent_rate;
> +
>  	return best_rate;
>  }
>
> @@ -106,7 +153,7 @@ static unsigned long ccu_nkm_recalc_rate(struct clk_hw *hw,
>  }
>
>  static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
> -					struct clk_hw *hw,
> +					struct clk_hw *parent_hw,
>  					unsigned long *parent_rate,
>  					unsigned long rate,
>  					void *data)
> @@ -124,7 +171,10 @@ static unsigned long ccu_nkm_round_rate(struct ccu_mux_internal *mux,
>  	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
>  		rate *= nkm->fixed_post_div;
>
> -	rate = ccu_nkm_find_best(*parent_rate, rate, &_nkm);
> +	if (!clk_hw_can_set_rate_parent(&nkm->common.hw))
> +		rate = ccu_nkm_find_best(parent_rate, rate, &_nkm, NULL);
> +	else
> +		rate = ccu_nkm_find_best(parent_rate, rate, &_nkm, parent_hw);
>
>  	if (nkm->common.features & CCU_FEATURE_FIXED_POSTDIV)
>  		rate /= nkm->fixed_post_div;
> @@ -159,7 +209,7 @@ static int ccu_nkm_set_rate(struct clk_hw *hw, unsigned long rate,
>  	_nkm.min_m = 1;
>  	_nkm.max_m = nkm->m.max ?: 1 << nkm->m.width;
>
> -	ccu_nkm_find_best(parent_rate, rate, &_nkm);
> +	ccu_nkm_find_best(&parent_rate, rate, &_nkm, NULL);
>
>  	spin_lock_irqsave(nkm->common.lock, flags);

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1329 bytes --]

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?

> >  				unsigned long tmp_rate;
> >
> > -				tmp_rate = parent * _n * _k / _m;
> > -
> > +				if (parent_hw) {
> > +					tmp_parent = optimal_parent_rate(rate, _n, _k, _m);
> > +					tmp_parent = clk_hw_round_rate(parent_hw, tmp_parent);
> > +				}
> 
> Another constraint is PLL-VIDEO0 rate / M >= 24 MHz. Therefore we also need:
>         if (tmp_parent < 24000000 * _m)
>                 continue;
>
> So, we need another optional member min_m_times_parent or, for
> shortness, maybe min_m_parent. I could use help finding a good name for
> this.

Again, if it's not causing any harm I'd rather keep the code as simple
and maintainable as possible.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Frank Oltmanns]

Hi Maxime,

On 2023-06-12 at 14:31:21 +0200, Maxime Ripard <maxime@cerno.tech> 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.

>> >  				unsigned long tmp_rate;
>> >
>> > -				tmp_rate = parent * _n * _k / _m;
>> > -
>> > +				if (parent_hw) {
>> > +					tmp_parent = optimal_parent_rate(rate, _n, _k, _m);
>> > +					tmp_parent = clk_hw_round_rate(parent_hw, tmp_parent);
>> > +				}
>>
>> Another constraint is PLL-VIDEO0 rate / M >= 24 MHz. Therefore we also need:
>>         if (tmp_parent < 24000000 * _m)
>>                 continue;
>>
>> So, we need another optional member min_m_times_parent or, for
>> shortness, maybe min_m_parent. I could use help finding a good name for
>> this.
>
> Again, if it's not causing any harm I'd rather keep the code as simple
> and maintainable as possible.

Unfortunately, in Allwinner's User Manual, I could not find any
consequences of driving the SoC outside its specifications. Maybe
someone with more experience in that area could weigh in here.

I want to highlight, though, that currently (i.e., without this patch
series), the rate of pll-video0 is not set by the kernel. So in most
installations, it probably runs at 294 (u-boot) or 297 MHz (default
rate). That allows for M to be smaller than or equal to 12. But with
this patch series, we set pll-video0 to any rate that pll-mipi requests.
Where previously, people could have hand-crafted rates that were within
the SoC's specification - based on a well-known pll-video0 rate - this
no longer applies.

Therefore, I think we should be careful not to request rates from the
parent that drives pll-mipi outside its specification. What do you think
about implementing the min_m_parent functionality in a separate patch?
That would probably facilitate the assessment of the patch from a
simplicity and maintenance perspective. If we agree on a way forward, I
can still squash it with other patches in a later version of this patch
series. But if you are not convinced, I can simply drop it at any point
- including now ;).

In short, I don't know if violating the restriction causes any harm in a
real-world application. However, Allwinner may have reasons for listing
such limitations, even if they don't tell us the details. Those are my
two cents.

Thanks for your feedback,
  Frank

[1] https://github.com/OLIMEX/OLINUXINO/blob/027087da32d69651a58a4e6193bedadef9c036ec/DOCUMENTS/A64-PDFs/Allwinner%20A64%20User%20Manual%20v1.0.pdf

>
> Maxime
>
> [[End of PGP Signed Part]]

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2699 bytes --]

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@cerno.tech> 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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

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

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 747 bytes --]

Hi,

On Sun, Jun 11, 2023 at 11:01:42AM +0200, Frank Oltmanns wrote:
> In case the CLK_SET_RATE_PARENT flag is set, consider using a different
> parent rate when determining a new rate.
> 
> To find the best match for the requested rate, perform the following
> steps for each NKM combination:
>  - calculate the optimal parent rate,
>  - find the best parent rate that the parent clock actually supports
>  - use that parent rate to calculate the effective rate.
> 
> In case the clk does not support setting the parent rate, use the same
> algorithm as before.
> 
> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>

Most of the comments I had on v1 still apply there, so let's figure them
out before sending a v3.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 2/2] clk: sunxi-ng: a64: allow pll-mipi to set parent's rate

[Frank Oltmanns]

The nkm clock now supports setting the parent's rate. Utilize this
option to find the optimal rate for pll-mipi.

Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
---
 drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
index eb36f8f77d55..125ae097d96c 100644
--- a/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-a64.c
@@ -179,7 +179,8 @@ static struct ccu_nkm pll_mipi_clk = {
 	.common		= {
 		.reg		= 0x040,
 		.hw.init	= CLK_HW_INIT("pll-mipi", "pll-video0",
-					      &ccu_nkm_ops, CLK_SET_RATE_UNGATE),
+					      &ccu_nkm_ops,
+					      CLK_SET_RATE_UNGATE | CLK_SET_RATE_PARENT),
 	},
 };
 
-- 
2.41.0