Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230527132747.83196-1-frank@oltmanns.dev> <20230527132747.83196-3-frank@oltmanns.dev>
In-Reply-To: <20230527132747.83196-3-frank@oltmanns.dev>
From:   Julian Calaby <julian.calaby@gmail.com>
Date:   Sun, 28 May 2023 09:19:36 +1000
Message-ID: <CAGRGNgWP6McbfORNQrrdvktEOVMgS-KCXuhC5GRYz-+SgsFx1w@mail.gmail.com>
Subject: Re: [RFC PATCH 2/3] clk: sunxi-ng: Implement precalculated NKM rate selection
To:     Frank Oltmanns <frank@oltmanns.dev>
Cc:     linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev,
        Andre Przywara <andre.przywara@arm.com>,
        Chen-Yu Tsai <wens@csie.org>, Icenowy Zheng <icenowy@aosc.io>,
        Jernej Skrabec <jernej.skrabec@gmail.com>,
        Maxime Ripard <mripard@kernel.org>,
        Michael Turquette <mturquette@baylibre.com>,
        Rob Herring <robh@kernel.org>,
        Samuel Holland <samuel@sholland.org>,
        Stephen Boyd <sboyd@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Frank,

On Sat, May 27, 2023 at 11:37=E2=80=AFPM 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_nk=
m.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 p=
arent, 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 *tabl=
e)
> +{
> +       unsigned long best_rate =3D 0, best_diff =3D ULONG_MAX;
> +       unsigned long best_n =3D 0, best_k =3D 0, best_m =3D 0;
> +       int start =3D 0, end =3D table->num - 1, mid;
> +
> +       while (start <=3D end) {
> +               unsigned long tmp_rate;
> +               unsigned long tmp_diff;
> +
> +               mid =3D (start + end) / 2;
> +
> +               tmp_rate =3D parent * table->combos[mid].n * table->combo=
s[mid].k /
> +                          table->combos[mid].m;
> +
> +               tmp_diff =3D abs(rate - tmp_rate);
> +
> +               if (tmp_diff < best_diff) {
> +                       best_rate =3D tmp_rate;
> +                       best_diff =3D tmp_diff;
> +                       best_n =3D table->combos[mid].n;
> +                       best_k =3D table->combos[mid].k;
> +                       best_m =3D table->combos[mid].m;
> +                       if (best_diff =3D=3D 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 =3D mid - 1;
> +               else
> +                       start =3D mid + 1;
> +       }
> +
> +out:
> +       nkm->n =3D best_n;
> +       nkm->k =3D best_k;
> +       nkm->m =3D best_m;
> +
> +       return best_rate;
> +}
> +
>  static void ccu_nkm_disable(struct clk_hw *hw)
>  {
>         struct ccu_nkm *nkm =3D hw_to_ccu_nkm(hw);
> diff --git a/drivers/clk/sunxi-ng/ccu_nkm.h b/drivers/clk/sunxi-ng/ccu_nk=
m.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 clo=
ck.
> + *
> + * 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 =3D 1, .k =3D 2, .m =3D 2} and  { .n =3D 2, .k =3D 2, .m =3D 4}
> + * are valid values for n, k, and m, only one of them would be allowed b=
ecause
> + * 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,

--=20
Julian Calaby

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