Hi all,
On 6/17/24 8:28 PM, Alexey Charkov wrote:
RK3588j is the 'industrial' variant of RK3588, and it uses a different
set of OPPs both in terms of allowed frequencies and in terms of
applicable voltages at each frequency setpoint.
Add the OPPs that apply to RK3588j (and apparently RK3588m too) to
enable dynamic CPU frequency scaling.
OPP values are derived from Rockchip downstream sources [1] by taking
only those OPPs which have the highest frequency for a given voltage
level and dropping the rest (if they are included, the kernel complains
at boot time about them being inefficient)
[1] https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
Funny stuff actually. Rockchip have their own downstream cpufreq driver
which autodetects at runtime the SoC variant and filter out OPPs based
on that info. And this patch is based on those values and filters.
However, they also decided that maybe this isn't the best way to do it
and they decided to have an rk3588j.dtsi where they remove some of those
OPPs instead of just updating the mask/filter in their base dtsi
(rk3588s.dtsi downstream). See
https://github.com/rockchip-linux/kernel/blob/604cec4004abe5a96c734f2fab7b74809d2d742f/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
So...
Signed-off-by: Alexey Charkov <alchark@xxxxxxxxx>
---
arch/arm64/boot/dts/rockchip/rk3588j.dtsi | 108 ++++++++++++++++++++++++++++++
1 file changed, 108 insertions(+)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
index 0bbeee399a63..b7e69553857b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588j.dtsi
@@ -5,3 +5,111 @@
*/
#include "rk3588-extra.dtsi"
+
+/ {
+ cluster0_opp_table: opp-table-cluster0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <750000 750000 950000>;
+ clock-latency-ns = <40000>;
+ opp-suspend;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <887500 887500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1704000000 {
+ opp-hz = /bits/ 64 <1704000000>;
+ opp-microvolt = <937500 937500 950000>;
+ clock-latency-ns = <40000>;
+ };
None of those are valid according to Rockchip, we should have
opp-1296000000 {
opp-hz = /bits/ 64 <1296000000>;
opp-microvolt = <750000 750000 950000>;
clock-latency-ns = <40000>;
opp-suspend;
};
instead?
and...
+ };
+
+ cluster1_opp_table: opp-table-cluster1 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <750000 750000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <787500 787500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <875000 875000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <950000 950000 950000>;
+ clock-latency-ns = <40000>;
+ };
opp-1800000000 and opp-2016000000 should be removed.
and...
+ };
+
+ cluster2_opp_table: opp-table-cluster2 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-1416000000 {
+ opp-hz = /bits/ 64 <1416000000>;
+ opp-microvolt = <750000 750000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1608000000 {
+ opp-hz = /bits/ 64 <1608000000>;
+ opp-microvolt = <787500 787500 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-1800000000 {
+ opp-hz = /bits/ 64 <1800000000>;
+ opp-microvolt = <875000 875000 950000>;
+ clock-latency-ns = <40000>;
+ };
+ opp-2016000000 {
+ opp-hz = /bits/ 64 <2016000000>;
+ opp-microvolt = <950000 950000 950000>;
+ clock-latency-ns = <40000>;
+ };
opp-1800000000 and opp-2016000000 should be removed as well.
Did I misunderstand what Rockchip did here? Adding Kever in Cc at least
for awareness on Rockchip side :)
I guess another option could be to mark those above as
turbo-mode;
though no clue what this would entail. From a glance at cpufreq, it
seems that for Rockchip since we use the default cpufreq-dt, it would
mark those as unusable, so essentially a no-op, so better remove them
entirely?
Cheers,
Quentin
