Re: [RFC PATCH 2/3] clk: sunxi-ng: Implement precalculated NKM rate selection

[Julian Calaby <julian.calaby@gmail.com>]

Hi Frank,

On Sat, May 27, 2023 at 11:37 PM Frank Oltmanns <frank@oltmanns.dev> wrote:
>
> Add a new precalculation method for NKM clock rate selection in the
> sunxi-ng clock driver. Introduce ccu_nkm_find_best_precalc which uses a
> precalculated table of valid NKM combinations (struct clk_nkm_table and
> struct clk_nkm_combo) to find the best rate. This approach provides
> faster rate selection by searching a table of valid combinations rather
> than calculating for all possible combinations.
>
> The table of NKM combinations needs to be initialized with meaningful
> combinations only, i.e. removing redundant combinations that result in
> the same rate.
>
> Keep the existing ccu_nkm_find_best function in place and use it as a
> fallback if no precalculated table is provided.
>
> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> ---
>  drivers/clk/sunxi-ng/ccu_nkm.c | 84 +++++++++++++++++++++++++++-------
>  drivers/clk/sunxi-ng/ccu_nkm.h | 26 +++++++++++
>  2 files changed, 94 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_nkm.c b/drivers/clk/sunxi-ng/ccu_nkm.c
> index 94d2a83992b2..9652f6df17bd 100644
> --- a/drivers/clk/sunxi-ng/ccu_nkm.c
> +++ b/drivers/clk/sunxi-ng/ccu_nkm.c
> @@ -54,6 +54,49 @@ static unsigned long ccu_nkm_find_best(unsigned long parent, unsigned long rate,
>         return best_rate;
>  }
>
> +static unsigned long ccu_nkm_find_best_precalc(unsigned long parent,
> +                                              unsigned long rate,
> +                                              struct _ccu_nkm *nkm,
> +                                              struct clk_nkm_table *table)
> +{
> +       unsigned long best_rate = 0, best_diff = ULONG_MAX;
> +       unsigned long best_n = 0, best_k = 0, best_m = 0;
> +       int start = 0, end = table->num - 1, mid;
> +
> +       while (start <= end) {
> +               unsigned long tmp_rate;
> +               unsigned long tmp_diff;
> +
> +               mid = (start + end) / 2;
> +
> +               tmp_rate = parent * table->combos[mid].n * table->combos[mid].k /
> +                          table->combos[mid].m;
> +
> +               tmp_diff = abs(rate - tmp_rate);
> +
> +               if (tmp_diff < best_diff) {
> +                       best_rate = tmp_rate;
> +                       best_diff = tmp_diff;
> +                       best_n = table->combos[mid].n;
> +                       best_k = table->combos[mid].k;
> +                       best_m = table->combos[mid].m;
> +                       if (best_diff == 0)
> +                               goto out;
> +               }

If the table was sorted by n * k / m, this could just be a process of
searching through until we either:
- find that the first rate in the table is too high
- find an exact rate
- go above the requested rate, then there's only two to compare: our
current rate and the previous one

This should massively simplify this function and would still work with
a binary search.

> +               if (rate < tmp_rate)
> +                       end = mid - 1;
> +               else
> +                       start = mid + 1;
> +       }
> +
> +out:
> +       nkm->n = best_n;
> +       nkm->k = best_k;
> +       nkm->m = best_m;
> +
> +       return best_rate;
> +}
> +
>  static void ccu_nkm_disable(struct clk_hw *hw)
>  {
>         struct ccu_nkm *nkm = hw_to_ccu_nkm(hw);
> diff --git a/drivers/clk/sunxi-ng/ccu_nkm.h b/drivers/clk/sunxi-ng/ccu_nkm.h
> index 6601defb3f38..fa5551724921 100644
> --- a/drivers/clk/sunxi-ng/ccu_nkm.h
> +++ b/drivers/clk/sunxi-ng/ccu_nkm.h
> @@ -12,6 +12,30 @@
>  #include "ccu_div.h"
>  #include "ccu_mult.h"
>
> +struct clk_nkm_combo {
> +       u8      n;
> +       u8      k;
> +       u8      m;
> +};
> +
> +/**
> + * struct clk_nkm_table - Table of all meaningful combinations for n, k, and m
> + *
> + * @num: Number of entries in the table
> + * @combos: Array of combos (of size num) that are supported by this clock.
> + *
> + * This table shall contain all meaningful combinations of n, k, and m. That
> + * means that combinations that result in the same clock rate shall only be
> + * listed once. For example, if both
> + * { .n = 1, .k = 2, .m = 2} and  { .n = 2, .k = 2, .m = 4}
> + * are valid values for n, k, and m, only one of them would be allowed because
> + * both result in a factor of 1.0.
> + */
> +struct clk_nkm_table {
> +       size_t                  num;
> +       struct clk_nkm_combo    *combos;

Should this be a "flex" array, i.e.

struct clk_nkm_combo combos[]

> +};
> +
>  /*
>   * struct ccu_nkm - Definition of an N-K-M clock
>   *

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/