From: Frank Oltmanns <frank@oltmanns.dev>
To: Andre Przywara <andre.przywara@arm.com>,
Chen-Yu Tsai <wens@csie.org>,
Jernej Skrabec <jernej.skrabec@gmail.com>,
Maxime Ripard <maxime@cerno.tech>,
Michael Turquette <mturquette@baylibre.com>,
Roman Beranek <me@crly.cz>, Samuel Holland <samuel@sholland.org>,
Stephen Boyd <sboyd@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-sunxi@lists.linux.dev
Subject: Re: [PATCH 0/2] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate
Date: Wed, 07 Jun 2023 09:35:20 +0200 [thread overview]
Message-ID: <87legv2093.fsf@oltmanns.dev> (raw)
In-Reply-To: <20230605190745.366882-1-frank@oltmanns.dev>
On 2023-06-05 at 21:07:43 +0200, Frank Oltmanns <frank@oltmanns.dev> wrote:
> I would like to share a patchset that enables the NKM clock in pll-video0 to
> consider alternative parent rates. I have found this feature particularly useful
> to adjust the pll-video0's clock on Allwinner A64, as it allows me to achieve an
> optimal rate for driving the board's panel (in my case, the Pinephone).
>
> 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, I need
> pll-mipi to operate at a specific rate.
>
> However, I must emphasize that setting the parent's rate for NKM clocks results
> in a significant increase in the time required to find the optimal rate. For
> instance, setting DCLK on the pinephone has seen a 60-fold increase in the time
> taken, from approximately 0.5 ms to around 30 ms. These figures were obtained
> through informal measurements on my pinephone, involving kernel logging and a
> few reboots. The worst-case scenario observed was approximately 37 ms, while the
> majority of cases were just under 30 ms.
>
> The reason for this considerable increase in time is that the code now iterates
> over all combinations of NKM for pll-mipi. For each combination, it subsequently
> iterates over all combinations of NM for pll-video0.
I don't know if this increase is problematic, but if it is, there are
options to mitigate it:
a. Binary search for finding parent rate:
Precalculate an _ordered_ table of meaningful combinations of NM for
pll-video0. That means that combinations that result in the same
clock rate shall only be listed once. For example, since both
{ .n = 8, .m = 1} and { .n = 16, .m = 2 }
are valid values for n and m, only one of them would be allowed
because both result in a factor of 8.
Furthermore, the table should only contain combinations that can
result in a valid clock rate. I.e., since the parent is a fixed rate
clock at 24 MHz and the ccu_nm's min_rate is 192 MHz, the
combination
{ .n = 1, .m = 1 }
should _not_ appear in the table as the resulting rate would be 96
MHz.
Utilizing this table, we can then do a binary search in
ccu_nm_find_best instead of iterating over all combinations of NM.
b. rational_best_approximation for finding parent rate:
Using the rational best approximation from linux/rational.h for the
parent's rate also significantly reduces the time. Instead of
iterating over all combinations of NM, the algorithm uses continued
fractions to "calculate best rational approximation for a given
fraction taking into account restricted register size" (quote from
the function's description).
This reduces the ccu_nm_find_best function to the following two
lines:
rational_best_approximation(rate, parent, nm->max_n, nm->max_m, &nm->n, &nm->m);
return ccu_nm_calc_rate(parent, nm->n, nm->m);
I did a rough implementation of both and found that both approaches
reduce the time spent to set DCLK to less than 2 ms, i.e., not quite the
original 0.5 ms, but not as bad as 30 ms.
Option a. either requires addional ROM space if we generate the table
off-line or some code to generate the table during initialization (which
of course would also require time to execute, but only once).
Option b. finds the closest approximation, whereas the current
implementation of ccu_nm_find_rate finds the closest rate that is less
than the requested rate. This means that option b. requires that all of
pll-video0's children (and grand-children a.s.o.) support rates higher
than the requested rate. Currently, the sunxi-ng driver in a lot of
places only expects rates that are less than the requested rate, so all
of these would need to be changed.
So, my question: Is spending the 30 ms fine or do I need to optimize for
speed in order for this patchset to be accepted? Or is 2 ms also too
much of an increase, in which case I'm out of ideas. :-)
I'm looking forward to receiving your feedback.
Thanks,
Frank
> I greatly appreciate your feedback and suggestions for further improving this
> patchset.
>
> Thanks,
> Frank
>
> 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 | 40 +++++++++++++++++++++------
> 2 files changed, 33 insertions(+), 10 deletions(-)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-06-07 7:36 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-05 19:07 [PATCH 0/2] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate Frank Oltmanns
2023-06-05 19:07 ` [PATCH 1/2] clk: sunxi-ng: nkm: consider alternative parent rates when finding rate Frank Oltmanns
2023-06-07 6:38 ` Maxime Ripard
2023-06-07 7:39 ` Frank Oltmanns
[not found] ` <sijbhopoxuw5wodyry3smg7tm4wzoc7k6xakt4qdvxqsok32mv@u7rh4dct5ya6>
2023-06-12 16:29 ` Frank Oltmanns
2023-06-13 9:10 ` Maxime Ripard
2023-06-13 10:17 ` Frank Oltmanns
2023-06-13 15:30 ` Maxime Ripard
2023-06-15 16:04 ` Frank Oltmanns
2023-06-19 16:36 ` Maxime Ripard
2023-06-19 8:16 ` Frank Oltmanns
2023-06-19 18:05 ` Maxime Ripard
2023-06-20 18:51 ` Frank Oltmanns
2023-06-26 16:45 ` Maxime Ripard
2023-07-12 4:39 ` Frank Oltmanns
2023-07-17 14:06 ` Maxime Ripard
2023-07-23 8:59 ` Frank Oltmanns
[not found] ` <q4mwg2dhwaxofbaaiyao7nmyfh4mat4ic2avxbzpmwhq6fmzyx@dflrio4loms5>
2023-07-30 14:35 ` Frank Oltmanns
2023-06-05 19:07 ` [PATCH 2/2] clk: sunxi-ng: a64: allow pll-mipi to set parent's rate Frank Oltmanns
2023-06-07 7:35 ` Frank Oltmanns [this message]
2023-06-07 12:27 ` [PATCH 0/2] clk: sunxi-ng: Consider alternative parent rates when determining NKM clock rate Maxime Ripard
2023-06-08 9:29 ` Frank Oltmanns
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87legv2093.fsf@oltmanns.dev \
--to=frank@oltmanns.dev \
--cc=andre.przywara@arm.com \
--cc=jernej.skrabec@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=maxime@cerno.tech \
--cc=me@crly.cz \
--cc=mturquette@baylibre.com \
--cc=samuel@sholland.org \
--cc=sboyd@kernel.org \
--cc=wens@csie.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).