| Message ID | 20230515171929.1.Ic8dee2cb79ce39ffc04eab2a344dde47b2f9459f@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | arm64: dts: qcom: sc7180-lite: Fix SDRAM freq for misidentified sc7180-lite boards | expand |
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>; > +};
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,
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>; +};
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(+)