[PATCH] arm64: dts: qcom: sc7180-lite: Fix SDRAM freq for misidentified sc7180-lite boards

[PATCH] arm64: dts: qcom: sc7180-lite: Fix SDRAM freq for misidentified sc7180-lite boards

[Douglas Anderson]

In general, the three SKUs of sc7180 (lite, normal, and pro) are
handled dynamically.

The cpufreq table in sc7180.dtsi includes the superset of all CPU
frequencies. The "qcom-cpufreq-hw" driver in Linux shows that we can
dynamically detect which frequencies are actually available on the
currently running CPU and then we can just enable those ones.

The GPU is similarly dynamic. The nvmem has a fuse in it (see
"gpu_speed_bin" in sc7180.dtsi) that the GPU driver can use to figure
out which frequencies to enable.

There is one part, however, that is not so dynamic. The way SDRAM
frequency works in sc7180 is that it's tied to cpufreq. At the busiest
cpufreq operating points we'll pick the top supported SDRAM frequency.
They ramp down together.

For the "pro" SKU of sc7180, we only enable one extra cpufreq step.
That extra cpufreq step runs SDRAM at the same speed as the step
below. Thus, for normal and pro things are OK. There is no sc7180-pro
device tree snippet.

For the "lite" SKU if sc7180, however, things aren't so easy. The
"lite" SKU drops 3 cpufreq entries but can still run SDRAM at max
frequency. That messed things up with the whole scheme. This is why we
added the "sc7180-lite" fragment in commit 8fd01e01fd6f ("arm64: dts:
qcom: sc7180-lite: Tweak DDR/L3 scaling on SC7180-lite").

When the lite scheme came about, it was agreed that the WiFi SKUs of
lazor would _always_ be "lite" and would, in fact, be the only "lite"
devices. Unfortunately, this decision changed and folks didn't realize
that it would be a problem. Specifically, some later lazor WiFi-only
devices were built with "pro" CPUs.

Building WiFi-only lazor with "pro" CPUs isn't the end of the world.
The SDRAM will ramp up a little sooner than it otherwise would, but
aside from a small power hit things work OK. One problem, though, is
that the SDRAM scaling becomes a bit quirky. Specifically, with the
current tables we'll max out SDRAM frequency at 2.1GHz but then
_lower_ it at 2.2GHz / 2.3GHz only to raise it back to max for 2.4GHz
and 2.55GHz.

Let's at least fix this so that the SDRAM frequency doesn't go down in
that quirky way. On true "lite" SKUs this change will be a no-op
because the operating points we're touching are disabled. This change
is only useful when a board that thinks it has a "lite" CPU actually
has a "normal" or "pro" one stuffed.

Fixes: 8fd01e01fd6f ("arm64: dts: qcom: sc7180-lite: Tweak DDR/L3 scaling on SC7180-lite")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm64/boot/dts/qcom/sc7180-lite.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi b/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi
index d8ed1d7b4ec7..4b306a59d9be 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi
@@ -16,3 +16,11 @@ &cpu6_opp11 {
 &cpu6_opp12 {
 	opp-peak-kBps = <8532000 23347200>;
 };
+
+&cpu6_opp13 {
+	opp-peak-kBps = <8532000 23347200>;
+};
+
+&cpu6_opp14 {
+	opp-peak-kBps = <8532000 23347200>;
+};
-- 
2.40.1.606.ga4b1b128d6-goog

Re: [PATCH] arm64: dts: qcom: sc7180-lite: Fix SDRAM freq for misidentified sc7180-lite boards

[Konrad Dybcio]

On 16.05.2023 02:19, Douglas Anderson wrote:
> In general, the three SKUs of sc7180 (lite, normal, and pro) are
> handled dynamically.
> 
> The cpufreq table in sc7180.dtsi includes the superset of all CPU
> frequencies. The "qcom-cpufreq-hw" driver in Linux shows that we can
> dynamically detect which frequencies are actually available on the
> currently running CPU and then we can just enable those ones.
> 
> The GPU is similarly dynamic. The nvmem has a fuse in it (see
> "gpu_speed_bin" in sc7180.dtsi) that the GPU driver can use to figure
> out which frequencies to enable.
> 
> There is one part, however, that is not so dynamic. The way SDRAM
> frequency works in sc7180 is that it's tied to cpufreq. At the busiest
> cpufreq operating points we'll pick the top supported SDRAM frequency.
> They ramp down together.
> 
> For the "pro" SKU of sc7180, we only enable one extra cpufreq step.
> That extra cpufreq step runs SDRAM at the same speed as the step
> below. Thus, for normal and pro things are OK. There is no sc7180-pro
> device tree snippet.
> 
> For the "lite" SKU if sc7180, however, things aren't so easy. The
> "lite" SKU drops 3 cpufreq entries but can still run SDRAM at max
> frequency. That messed things up with the whole scheme. This is why we
> added the "sc7180-lite" fragment in commit 8fd01e01fd6f ("arm64: dts:
> qcom: sc7180-lite: Tweak DDR/L3 scaling on SC7180-lite").
> 
> When the lite scheme came about, it was agreed that the WiFi SKUs of
> lazor would _always_ be "lite" and would, in fact, be the only "lite"
> devices. Unfortunately, this decision changed and folks didn't realize
> that it would be a problem. Specifically, some later lazor WiFi-only
> devices were built with "pro" CPUs.
> 
> Building WiFi-only lazor with "pro" CPUs isn't the end of the world.
> The SDRAM will ramp up a little sooner than it otherwise would, but
> aside from a small power hit things work OK. One problem, though, is
> that the SDRAM scaling becomes a bit quirky. Specifically, with the
> current tables we'll max out SDRAM frequency at 2.1GHz but then
> _lower_ it at 2.2GHz / 2.3GHz only to raise it back to max for 2.4GHz
> and 2.55GHz.
> 
> Let's at least fix this so that the SDRAM frequency doesn't go down in
> that quirky way. On true "lite" SKUs this change will be a no-op
> because the operating points we're touching are disabled. This change
> is only useful when a board that thinks it has a "lite" CPU actually
> has a "normal" or "pro" one stuffed.
> 
> Fixes: 8fd01e01fd6f ("arm64: dts: qcom: sc7180-lite: Tweak DDR/L3 scaling on SC7180-lite")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
I'd love it if you wrote a book detailing all the crazy stories like
this one, given it was probably not a one-off :P

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: <stable@vger.kernel.org>

Konrad
> 
>  arch/arm64/boot/dts/qcom/sc7180-lite.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi b/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi
> index d8ed1d7b4ec7..4b306a59d9be 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-lite.dtsi
> @@ -16,3 +16,11 @@ &cpu6_opp11 {
>  &cpu6_opp12 {
>  	opp-peak-kBps = <8532000 23347200>;
>  };
> +
> +&cpu6_opp13 {
> +	opp-peak-kBps = <8532000 23347200>;
> +};
> +
> +&cpu6_opp14 {
> +	opp-peak-kBps = <8532000 23347200>;
> +};

Re: [PATCH] arm64: dts: qcom: sc7180-lite: Fix SDRAM freq for misidentified sc7180-lite boards

[Bjorn Andersson]

On Mon, 15 May 2023 17:19:29 -0700, Douglas Anderson wrote:
> In general, the three SKUs of sc7180 (lite, normal, and pro) are
> handled dynamically.
> 
> The cpufreq table in sc7180.dtsi includes the superset of all CPU
> frequencies. The "qcom-cpufreq-hw" driver in Linux shows that we can
> dynamically detect which frequencies are actually available on the
> currently running CPU and then we can just enable those ones.
> 
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: sc7180-lite: Fix SDRAM freq for misidentified sc7180-lite boards
      commit: 3a735530c159b75e1402c08abe1ba4eb99a1f7a3

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>