mirror of https://lore.kernel.org/linux-amlogic/
 help / color / mirror / Atom feed
From: Chuan Liu <chuan.liu@amlogic.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: "Christian Stefan Kövér-Draxl" <ckoever@outlook.de>,
	neil.armstrong@linaro.org, conor+dt@kernel.org,
	christian.koever-draxl@student.uibk.ac.at, rafael@kernel.org,
	sboyd@kernel.org, "Viresh Kumar" <viresh.kumar@linaro.org>,
	linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	khilman@baylibre.com, linux-amlogic@lists.infradead.org,
	krzk+dt@kernel.org, bmasney+clk@redhat.com, robh@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	"Jerome Brunet" <jbrunet@baylibre.com>
Subject: Re: [PATCH 4/4] cpufreq: add an Amlogic speed-bin driver
Date: Tue, 3 Nov 2026 17:45:40 +0800	[thread overview]
Message-ID: <48b333a3-ef88-4739-a087-3ed9d8ec60a4@amlogic.com> (raw)
In-Reply-To: <CAFBinCBkJJeRPKLFBA9KYjwXMxFjow_NOtvFoKRQdxD=rOPW3w@mail.gmail.com>

Hi Martin,
     Thank you for joining this discussion. :)

I have re-examined the capabilities provided by pm_opp and cpufreq-dt, 
and it seems that we may also be able to adapt A9 fairly well based on 
the amlogic-cpufreq.c here. So we have no objections to the driver 
implementation in amlogic-cpufreq.c here.

I was most worried about before was that cpufreq-dt might not fully 
handle the complicated clock and supply adjustment logic between 
multiple clusters, which would have required adding a separate 
cpufreq_driver. But it now looks like this can be solved by adding the 
corresponding config_clks() and config_regulators() callbacks in 
dev_pm_opp_set_config().

On 11/2/2026 6:51 AM, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi Chuan,
> 
> On Thu, Oct 29, 2026 at 6:30 AM Chuan Liu <chuan.liu@amlogic.com> wrote:
> [...]
>> Regarding the concern I mentioned above about supporting a unified
>> cpufreq driver across S4, C3, T7, ..., A9, let me explain it in more detail:
>>      - These SoCs fall into several hardware architecture categories, and
>> I'm wondering whether we could take all of them into consideration and
>> implement them in a single amlogic-cpufreq.c, for example:
> I think the main question here is: how much code is shared between
> various SoC generations.

In my view, the main difference lies in the code that handles the shared 
and coupled resources; the rest should not differ much.

> 
>>        * S4/C3: 1 * cluster
>>        * T7: 2 * clusters
>>        * S7/S7D/S6: 1 * cluster + DSU
>>        * A9: 2 * clusters + DSU (the 2 clusters share one DSU)
>>      - For different hardware architectures, the resources that the
>> cpufreq driver has to manage also differ and are coupled with each
>> other, for example:
>>        * S7/S7D/S6: the CPUs and the DSU share a single power rail, and
>> the DSU frequency scales proportionally with the CPU frequency
>> (typically dsu_clk = cpu_clk * 80%)
>>        * A9: the resources that A9 needs to manage are relatively more
>> complicated ...
>>      - Optimizing the cpufreq transition latency:
>>        * Clock latency: for most S4/C3/T7 customers, the released bl31.bin
>> only supports configuring the CPU clock through SMCs with discrete clock
>> rates. Frequent set_rate()/set_parent() calls in the kernel would result
>> in a relatively large clock latency, and would cause the sugov thread to
>> consume too much CPU time, hurting scheduling performance.
> We also support a few more SoC generations upstream:
> - Meson8/8b/8m2 (32-bit ARM Cortex-A5/A9) where we need to reparent
> the CPU clock to XTAL for stable operating during clock changes (which
> should go into a cpufreq driver that implements target_intermediate).
> There's no secure monitor or firmware involved for CPU clocks.

BTW, could we put the CPU clock handling logic in the clock driver like 
G12A does (though, of course, handling it in the cpufreq driver might 
minimize the coupling between the clock and cpufreq drivers)? That way 
we could fully reuse the existing cpufreq-dt?

> - GXBB/GXL/GXM use SCPI firmware CPU clock control so to my knowledge
> we're good with what we have (meaning: no custom cpufreq driver
> needed)
> - G12A single cluster, no DSU: needs a cpufreq driver that implements
> target_intermediate (currently it's managed on clock driver side in
> kernel space, no firmware involved)
> - G12B multi cluster, no DSU: needs a cpufreq driver that implements
> target_intermediate (currently it's managed on clock driver side in
> kernel space, no firmware involved)

The cpufreq-related resources on the SoCs mentioned above are relatively 
clean: each cluster has only one independent clock and supply of its 
own, and these SoCs also do not support voltage selection based on chip 
binning.

> - SM1 single cluster and DSU: needs a cpufreq driver that implements
> target_intermediate (currently it's managed on clock driver side in
> kernel space, no firmware involved)

Compared with the SoCs above, the difference for SM1 is that it needs to 
additionally manage a DSU clock. In this case, the clock-related 
handling can be added in dev_pm_opp_set_config() in amlogic-cpufreq.c 
(by implementing clk_names and config_clks in struct dev_pm_opp_config).

> 
> [...]
>> The amlogic-cpufreq.c as implemented here can indeed cover the case of a
>> single cluster with no shared resources, such as S4/C3/T7. However, for
>> the upcoming SoCs where multiple clusters share resources, it may not be
>> sufficient to cover: for example, on S7/S7D/S6 the CPUs and the DSU
>> share one regulator; on A9 the two clusters share the dsu_clk, and the
>> sharing relationships among cpu_reg, cpu_sram_reg, dsu_reg and
>> dsu_sram_reg are even more complicated. For such cases, extending the
>> current amlogic-cpufreq.c would be rather difficult, so we would like to
>> make an overall evaluation.
> I think you're making a good point why it's fine to have multiple
> cpufreq drivers for Amlogic SoCs.
> My suggestion instead is to name the driver in this series
> amlogic-s4-cpufreq (or similar). From what I understand C3 support
> (maybe also T7?) can be easily added to the same driver.
> For the 32-bit SoCs I'm planning to implement amlogic-meson8-cpufreq.
> I don't know how relevant DSU is on SM1,

On SM1 (as well as other SoCs that contain a DSU), adjusting the CPU 
clock requires dynamically adjusting the DSU frequency as well. To 
balance performance and power consumption, in typical scenarios the CPU 
& DSU frequency mapping satisfies:
   - cpu_clk <= 1 GHz (the clock sources of both cpu_clk and dsu_clk 
come from fixed_pll, and sys_pll can be turned off to save power): 
dsu_clk = cpu_clk
   - 1 GHz < cpu_clk <= 1.2 GHz (the clock source of cpu_clk comes from 
sys_pll): dsu_clk = 1 GHz
   - cpu_clk > 1.2 GHz (the clock sources of cpu_clk and dsu_clk come 
from their own sys_pll respectively): dsu_clk = cpu_clk * 80%

> depending on how tricky it is
> support may either go into amlogic-meson8-cpufreq or (a new)
> amlogic-meson-g12a-cpufreq.
> And for the newer SoCs like S7/S7D/S6/A9/... I trust your instinct on
> whether they should be in one driver or multiple.

To summarize, the characteristics of the above SoCs can roughly be 
grouped as follows:
   a) SoCs up to G12B: 1 * clock + 1 * supply
   b) SM1: DSU clock handling added on top of a)
   c) S4/C3/T7: voltage selection based on chip binning added on top of a)
   d) S7/S7D/S6: have both the characteristics of b) and c)
   e) A9: has these additional features on top of d):
     - Handling the DSU clock shared by the 2 clusters
     - cluster0 and the DSU share one supply
     - The cpu_reg-sram_reg and dsu_reg-dsu_sram_reg voltage scaling 
must guarantee Step-by-Step Voltage Tracking (one can refer to 
mtk_cpufreq_voltage_tracking() in mediatek-cpufreq.c)

All in all, it looks like we can currently support all Amlogic SoCs in a 
single amlogic-cpufreq.c? Then, for the specific characteristics of 
different SoCs, we can differentiate them through the "compatible" 
property and add the corresponding drvdata for each.

I'll also try to support S7/A9 with representative differences based on 
this patch.

> 
>> We have done a lot of optimization on these issues from both the
>> hardware and software sides, and we would like the cpufreq driver we
>> submit upstream to cover these optimizations as much as possible. For
>> example, the bl31.bin for S4/C3/T7 has already been released, and
>> updating bl31.bin may cause extra trouble for customers, so we would
>> also like to balance this and see how to minimize the impact.
> I agree. I my opinion it would be best to allow the OS (in this case:
> Linux) to manage clocks so (closed source) firmware binary blob
> updates cannot cause any trouble.

Since clocks such as cpu_clk are the most fundamental guarantee for the 
system to run, in order to prevent userspace from arbitrarily modifying 
registers and causing the system to hang, we restrict these registers so 
that they can only be read/written in a secure environment (e.g. TEE).

Regarding the closed-source firmware binary issue, we are also actively 
exploring more flexible ways to avoid this kind of trouble.

> 
> 
> Best regards,
> Martin

-- 
Best regards,
Chuan


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2026-11-03  9:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-25 19:22 [PATCH 0/4] arm64: amlogic: S4 CPU frequency scaling christian.koever-draxl
2026-10-25 19:22 ` [PATCH 1/4] dt-bindings: clock: amlogic,s4-pll-clkc: add the CPU clock christian.koever-draxl
2026-10-30 11:15   ` Krzysztof Kozlowski
2026-10-25 19:22 ` [PATCH 2/4] clk: meson: s4-pll: " christian.koever-draxl
2026-10-25 19:22 ` [PATCH 3/4] firmware: meson: sm: add the CPU speed bin call christian.koever-draxl
2026-10-25 19:22 ` [PATCH 4/4] cpufreq: add an Amlogic speed-bin driver christian.koever-draxl
2026-10-26  5:30   ` Viresh Kumar
2026-10-26  8:57     ` Jerome Brunet
2026-10-26  8:59       ` Viresh Kumar
2026-10-26 10:08         ` Christain Stefan Köver-Draxl
2026-10-27  9:26         ` Chuan Liu
2026-10-27 10:22           ` Christian Hewitt
2026-10-29  5:29             ` Chuan Liu
2026-11-01 22:51               ` Martin Blumenstingl
2026-11-03  9:45                 ` Chuan Liu [this message]
2026-10-26 10:10       ` Christain Stefan Köver-Draxl
2026-10-27  9:31       ` Chuan Liu

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=48b333a3-ef88-4739-a087-3ed9d8ec60a4@amlogic.com \
    --to=chuan.liu@amlogic.com \
    --cc=bmasney+clk@redhat.com \
    --cc=christian.koever-draxl@student.uibk.ac.at \
    --cc=ckoever@outlook.de \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=khilman@baylibre.com \
    --cc=krzk+dt@kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=neil.armstrong@linaro.org \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=viresh.kumar@linaro.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

all inboxes | Powered by JetHome®