Skip to contentSkip to conversation

DDR/L3 Scaling support on SDM845 and SC7180 SoCs

PATCH Superseded Sibi Sankar 2 participants 10 messages May 27, 2020 – Jun 2, 2020

Subsystems (4)
Arm Qualcomm mailing list CPU frequency scaling framework Power management core Operating performance points (OPP)

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Sibi Sankar

author
#0 lore raw

v5 cover DDR/L3 Scaling support on SDM845 and SC7180 SoCs

This patch series aims to extend cpu based scaling support to L3/DDR on SDM845 and SC7180 SoCs.

Patches [1-2] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev Patches [3-5] - Update bw levels based on cpu frequency change

Based on the following series:
https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/log/?h=opp/linux-next

V5:
* Pick up R-bs from Amit
* Drop icc tag support/dt changes till the a consensus is achieved
* Use dev_pm_opp_adjust_voltage instead [Viresh]
* Drop dev_pm_opp_get_path_count [Saravana]
* Rework dev_pm_opp_set_bw

V4:
* Migrate to using Georgi's new bindings
* Misc fixups based on Matthias comments
* API fixups based on Bjorn's comments on v2
* Picked up a few R-bs from Matthias

v3:
* Migrated to using Saravana's opp-kBps bindings [1]
* Fixed some misc comments from Rajendra
* Added support for SC7180

v2:
* Incorporated Viresh's comments from:
https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
* Dropped cpufreq-map passive governor

Sibi Sankar (5):
cpufreq: blacklist SDM845 in cpufreq-dt-platdev
cpufreq: blacklist SC7180 in cpufreq-dt-platdev
OPP: Add and export helper to set bandwidth
cpufreq: qcom: Update the bandwidth levels on frequency change
cpufreq: qcom: Disable fast switch when scaling DDR/L3

 drivers/cpufreq/cpufreq-dt-platdev.c |  2 +
 drivers/cpufreq/qcom-cpufreq-hw.c    | 81 ++++++++++++++++++++++++++--
 drivers/opp/core.c                   | 31 +++++++++++
 include/linux/pm_opp.h               |  6 +++
 4 files changed, 116 insertions(+), 4 deletions(-)
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#1 lore raw

PATCH v5 1/5 cpufreq: blacklist SDM845 in cpufreq-dt-platdev

Add SDM845 to cpufreq-dt-platdev blacklist since the actual scaling is handled by the 'qcom-cpufreq-hw' driver.

Reviewed-by Amit Kucheria <[email protected]>
Reviewed-by Matthias Kaehlcke <[email protected]>
Signed-off-by Sibi Sankar <[email protected]>

---

v5:
* Picked up R-b from Amit

v4:
* Updated commit message [Matthias]
* Picked up R-b from Matthias

 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
index e8e20fef400b0..be85eb494a6b3 100644
@@ -132,6 +132,7 @@ static const struct of_device_id blacklist[] __initconst = {
 	{ .compatible = "qcom,apq8096", },
 	{ .compatible = "qcom,msm8996", },
 	{ .compatible = "qcom,qcs404", },
+	{ .compatible = "qcom,sdm845", },
 
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#2 lore raw

PATCH v5 2/5 cpufreq: blacklist SC7180 in cpufreq-dt-platdev

In reply to Sibi Sankar

Add SC7180 to cpufreq-dt-platdev blacklist since the actual scaling is handled by the 'qcom-cpufreq-hw' driver.

Reviewed-by Amit Kucheria <[email protected]>
Reviewed-by Matthias Kaehlcke <[email protected]>
Signed-off-by Sibi Sankar <[email protected]>

---

v5:
* Picked up R-b from Amit

v4:
* Updated commit message [Matthias]
* Picked up R-b from Matthias

 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 1 file changed, 1 insertion(+)
drivers/cpufreq/cpufreq-dt-platdev.c +1 -0
index be85eb494a6b3..7d01df7bfa6cd 100644
@@ -132,6 +132,7 @@ static const struct of_device_id blacklist[] __initconst = {
 	{ .compatible = "qcom,apq8096", },
 	{ .compatible = "qcom,msm8996", },
 	{ .compatible = "qcom,qcs404", },
+	{ .compatible = "qcom,sc7180", },
 	{ .compatible = "qcom,sdm845", },
 
 	{ .compatible = "st,stih407", },
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#3 lore raw

PATCH v5 3/5 OPP: Add and export helper to set bandwidth

In reply to Sibi Sankar

Add and export 'dev_pm_opp_set_bw' to set the bandwidth levels associated with an OPP.

Signed-off-by Sibi Sankar <[email protected]>

---

v5:
https://lkml.org/lkml/2020/5/27/7
* Rework the patch based on ^^

v4:
https://patchwork.kernel.org/patch/11019737/
* Pass device opp to set bw levels [Bjorn]

 drivers/opp/core.c     | 31 +++++++++++++++++++++++++++++++
 include/linux/pm_opp.h |  6 ++++++
 2 files changed, 37 insertions(+)
drivers/opp/core.c +31 -0
index df12c38045330..c87e69f26e976 100644
@@ -831,6 +831,37 @@ static int _set_required_opps(struct device *dev,
 	return ret;
 }
 
+/**
+ * dev_pm_opp_set_bw() - sets bandwidth levels corresponding to an opp
+ * @dev:	device for which we do this operation
+ * @opp:	opp based on which the bandwidth levels are to be configured
+ *
+ * This configures the bandwidth to the levels specified by the OPP. However
+ * if the OPP specified is NULL the bandwidth levels are cleared out.
+ *
+ * Return: 0 on success or a negative error value.
+ */
+int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp)
+{
+	struct opp_table *opp_table;
+	int ret;
+
+	opp_table = _find_opp_table(dev);
+	if (IS_ERR(opp_table)) {
+		dev_err(dev, "%s: device opp table doesn't exist\n", __func__);
+		return PTR_ERR(opp_table);
+	}
+
+	if (opp)
+		ret = _set_opp_bw(opp_table, opp, dev, false);
+	else
+		ret = _set_opp_bw(opp_table, opp, dev, true);
+
+	dev_pm_opp_put_opp_table(opp_table);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_bw);
+
 /**
  * dev_pm_opp_set_rate() - Configure new OPP based on frequency
  * @dev:	 device for which we do this operation
include/linux/pm_opp.h +6 -0
index d5c4a329321dd..ae68417c0ae00 100644
@@ -151,6 +151,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names
 void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
 int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
 int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
+int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp);
 int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
 int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
 void dev_pm_opp_remove_table(struct device *dev);

@@ -342,6 +343,11 @@ static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_f
 	return -ENOTSUPP;
 }
 
+static inline int dev_pm_opp_set_bw(struct device *dev, struct dev_pm_opp *opp)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask)
 {
 	return -ENOTSUPP;
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
author
#4 lore raw

PATCH v5 4/5 cpufreq: qcom: Update the bandwidth levels on frequency change

In reply to Sibi Sankar

Add support to parse optional OPP table attached to the cpu node when the OPP bandwidth values are populated. This allows for scaling of DDR/L3 bandwidth levels with frequency change.

Signed-off-by Sibi Sankar <[email protected]>

---

V5:
* Use dev_pm_opp_adjust_voltage instead [Viresh]
* Misc cleanup

v4:
* Split fast switch disable into another patch [Lukasz]

 drivers/cpufreq/qcom-cpufreq-hw.c | 77 ++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)
drivers/cpufreq/qcom-cpufreq-hw.c +75 -2
index fc92a8842e252..fbd73d106a3ae 100644
@@ -6,6 +6,7 @@
 #include <linux/bitfield.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_address.h>

@@ -31,6 +32,52 @@
 static unsigned long cpu_hw_rate, xo_rate;
 static struct platform_device *global_pdev;
 
+static int qcom_cpufreq_set_bw(struct cpufreq_policy *policy,
+			       unsigned long freq_khz)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+	struct dev_pm_opp *opp;
+	struct device *dev;
+	int ret;
+
+	dev = get_cpu_device(policy->cpu);
+	if (!dev)
+		return -ENODEV;
+
+	opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	ret = dev_pm_opp_set_bw(dev, opp);
+	dev_pm_opp_put(opp);
+	return ret;
+}
+
+static int qcom_cpufreq_update_opp(struct device *cpu_dev,
+				   unsigned long freq_khz,
+				   unsigned long volt)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+
+	if (dev_pm_opp_adjust_voltage(cpu_dev, freq_hz, volt, volt, volt))
+		return dev_pm_opp_add(cpu_dev, freq_hz, volt);
+
+	/* Enable the opp after voltage update */
+	return dev_pm_opp_enable(cpu_dev, freq_hz);
+}
+
+/* Check for optional interconnect paths on CPU0 */
+static int qcom_cpufreq_find_icc_paths(struct device *dev)
+{
+	struct device *cpu_dev;
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev)
+		return -EPROBE_DEFER;
+
+	return dev_pm_opp_of_find_icc_paths(cpu_dev, NULL);
+}
+
 static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 					unsigned int index)
 {

@@ -39,6 +86,8 @@ static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 
 	writel_relaxed(index, perf_state_reg);
 
+	qcom_cpufreq_set_bw(policy, freq);
+
 	arch_set_freq_scale(policy->related_cpus, freq,
 			    policy->cpuinfo.max_freq);
 	return 0;

@@ -88,12 +137,30 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 {
 	u32 data, src, lval, i, core_count, prev_freq = 0, freq;
 	u32 volt;
+	u64 rate;
 	struct cpufreq_frequency_table	*table;
+	struct device_node *opp_table_np, *np;
+	int ret;
 
 	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
 	if (!table)
 		return -ENOMEM;
 
+	ret = dev_pm_opp_of_add_table(cpu_dev);
+	if (!ret) {
+		/* Disable all opps and cross-validate against LUT */
+		opp_table_np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+		for_each_available_child_of_node(opp_table_np, np) {
+			ret = of_property_read_u64(np, "opp-hz", &rate);
+			if (!ret)
+				dev_pm_opp_disable(cpu_dev, rate);
+		}
+		of_node_put(opp_table_np);
+	} else if (ret != -ENODEV) {
+		dev_err(cpu_dev, "Invalid OPP table in Device tree\n");
+		return ret;
+	}
+
 	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
 		data = readl_relaxed(base + REG_FREQ_LUT +
 				      i * LUT_ROW_SIZE);

@@ -112,7 +179,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 
 		if (freq != prev_freq && core_count != LUT_TURBO_IND) {
 			table[i].frequency = freq;
-			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
+			qcom_cpufreq_update_opp(cpu_dev, freq, volt);
 			dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
 				freq, core_count);
 		} else if (core_count == LUT_TURBO_IND) {

@@ -133,7 +200,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 			if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
 				prev->frequency = prev_freq;
 				prev->flags = CPUFREQ_BOOST_FREQ;
-				dev_pm_opp_add(cpu_dev,	prev_freq * 1000, volt);
+				qcom_cpufreq_update_opp(cpu_dev, prev_freq,
+							volt);
 			}
 
 			break;

@@ -254,6 +322,7 @@ static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
 	void __iomem *base = policy->driver_data - REG_PERF_STATE;
 
 	dev_pm_opp_remove_all_dynamic(cpu_dev);
+	dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
 	kfree(policy->freq_table);
 	devm_iounmap(&global_pdev->dev, base);
 

@@ -301,6 +370,10 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
 
 	global_pdev = pdev;
 
+	ret = qcom_cpufreq_find_icc_paths(&pdev->dev);
+	if (ret)
+		return ret;
+
 	ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
 	if (ret)
 		dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
maintainer
#5 lore raw

Re: [PATCH v5 4/5] cpufreq: qcom: Update the bandwidth levels on frequency change

Quoted 52 lines · Sibi Sankar

Add support to parse optional OPP table attached to the cpu node when the OPP bandwidth values are populated. This allows for scaling of DDR/L3 bandwidth levels with frequency change.

Signed-off-by Sibi Sankar <[email protected]>

---

V5:
* Use dev_pm_opp_adjust_voltage instead [Viresh]
* Misc cleanup

v4:
* Split fast switch disable into another patch [Lukasz]

 drivers/cpufreq/qcom-cpufreq-hw.c | 77 ++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)
drivers/cpufreq/qcom-cpufreq-hw.c +20 -0
index fc92a8842e252..fbd73d106a3ae 100644
@@ -6,6 +6,7 @@
 #include <linux/bitfield.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_address.h>

@@ -31,6 +32,52 @@
 static unsigned long cpu_hw_rate, xo_rate;
 static struct platform_device *global_pdev;
 
+static int qcom_cpufreq_set_bw(struct cpufreq_policy *policy,
+			       unsigned long freq_khz)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+	struct dev_pm_opp *opp;
+	struct device *dev;
+	int ret;
+
+	dev = get_cpu_device(policy->cpu);
+	if (!dev)
+		return -ENODEV;
+
+	opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	ret = dev_pm_opp_set_bw(dev, opp);
+	dev_pm_opp_put(opp);
+	return ret;

drivers/cpufreq/qcom-cpufreq-hw.c line 63

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c +10 -0
+}
+
+static int qcom_cpufreq_update_opp(struct device *cpu_dev,
+				   unsigned long freq_khz,
+				   unsigned long volt)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+
+	if (dev_pm_opp_adjust_voltage(cpu_dev, freq_hz, volt, volt, volt))
+		return dev_pm_opp_add(cpu_dev, freq_hz, volt);

What's going on here ? Why add OPP here ?

Quoted 7 lines
drivers/cpufreq/qcom-cpufreq-hw.c +7 -0
+
+	/* Enable the opp after voltage update */
+	return dev_pm_opp_enable(cpu_dev, freq_hz);
+}
+
+/* Check for optional interconnect paths on CPU0 */
+static int qcom_cpufreq_find_icc_paths(struct device *dev)

drivers/cpufreq/qcom-cpufreq-hw.c line 80

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c +10 -0
+{
+	struct device *cpu_dev;
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev)
+		return -EPROBE_DEFER;
+
+	return dev_pm_opp_of_find_icc_paths(cpu_dev, NULL);
+}
+

open code this into the probe routine.

Quoted 21 lines
drivers/cpufreq/qcom-cpufreq-hw.c +5 -0
 static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 					unsigned int index)
 {

@@ -39,6 +86,8 @@ static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 
 	writel_relaxed(index, perf_state_reg);
 
+	qcom_cpufreq_set_bw(policy, freq);
+
 	arch_set_freq_scale(policy->related_cpus, freq,
 			    policy->cpuinfo.max_freq);
 	return 0;

@@ -88,12 +137,30 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 {
 	u32 data, src, lval, i, core_count, prev_freq = 0, freq;
 	u32 volt;
+	u64 rate;
 	struct cpufreq_frequency_table	*table;
+	struct device_node *opp_table_np, *np;
+	int ret;

drivers/cpufreq/qcom-cpufreq-hw.c static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, · line 154

Quoted 10 lines
drivers/cpufreq/qcom-cpufreq-hw.c +6 -0
 	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
 	if (!table)
 		return -ENOMEM;
 
+	ret = dev_pm_opp_of_add_table(cpu_dev);
+	if (!ret) {
+		/* Disable all opps and cross-validate against LUT */
+		opp_table_np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+		for_each_available_child_of_node(opp_table_np, np) {
+			ret = of_property_read_u64(np, "opp-hz", &rate);

No way, please use dev_pm_opp_find_freq_*() here instead to grab OPPs one by one.

drivers/cpufreq/qcom-cpufreq-hw.c static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, · line 162

Quoted 8 lines
drivers/cpufreq/qcom-cpufreq-hw.c +8 -0
+			if (!ret)
+				dev_pm_opp_disable(cpu_dev, rate);
+		}
+		of_node_put(opp_table_np);
+	} else if (ret != -ENODEV) {
+		dev_err(cpu_dev, "Invalid OPP table in Device tree\n");
+		return ret;
+	}

Rather put this in the if (ret) block and so the else part doesn't need extra indentation.

Quoted 23 lines
drivers/cpufreq/qcom-cpufreq-hw.c +4 -2
+
 	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
 		data = readl_relaxed(base + REG_FREQ_LUT +
 				      i * LUT_ROW_SIZE);

@@ -112,7 +179,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 
 		if (freq != prev_freq && core_count != LUT_TURBO_IND) {
 			table[i].frequency = freq;
-			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
+			qcom_cpufreq_update_opp(cpu_dev, freq, volt);
 			dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
 				freq, core_count);
 		} else if (core_count == LUT_TURBO_IND) {

@@ -133,7 +200,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 			if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
 				prev->frequency = prev_freq;
 				prev->flags = CPUFREQ_BOOST_FREQ;
-				dev_pm_opp_add(cpu_dev,	prev_freq * 1000, volt);
+				qcom_cpufreq_update_opp(cpu_dev, prev_freq,
+							volt);
 			}
 
 			break;
Signature · 1 line
viresh
author
#6 lore raw

Re: [PATCH v5 4/5] cpufreq: qcom: Update the bandwidth levels on frequency change

Hey Viresh,
Thanks for taking time to review the series :)

Quoted 58 lines · Viresh Kumar
Quoted 57 lines · Sibi Sankar

Add support to parse optional OPP table attached to the cpu node when the OPP bandwidth values are populated. This allows for scaling of DDR/L3 bandwidth levels with frequency change.

Signed-off-by Sibi Sankar <[email protected]>

---

V5:
* Use dev_pm_opp_adjust_voltage instead [Viresh]
* Misc cleanup

v4:
* Split fast switch disable into another patch [Lukasz]

 drivers/cpufreq/qcom-cpufreq-hw.c | 77 
++++++++++++++++++++++++++++++-
 1 file changed, 75 insertions(+), 2 deletions(-)
drivers/cpufreq/qcom-cpufreq-hw.c +24 -0
index fc92a8842e252..fbd73d106a3ae 100644
@@ -6,6 +6,7 @@
 #include <linux/bitfield.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_address.h>

@@ -31,6 +32,52 @@
 static unsigned long cpu_hw_rate, xo_rate;
 static struct platform_device *global_pdev;
 
+static int qcom_cpufreq_set_bw(struct cpufreq_policy *policy,
+			       unsigned long freq_khz)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+	struct dev_pm_opp *opp;
+	struct device *dev;
+	int ret;
+
+	dev = get_cpu_device(policy->cpu);
+	if (!dev)
+		return -ENODEV;
+
+	opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
+	if (IS_ERR(opp))
+		return PTR_ERR(opp);
+
+	ret = dev_pm_opp_set_bw(dev, opp);
+	dev_pm_opp_put(opp);
+	return ret;
+}
+
+static int qcom_cpufreq_update_opp(struct device *cpu_dev,
+				   unsigned long freq_khz,
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-hw.c line 63

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-hw.c +6 -0
+				   unsigned long volt)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+
+	if (dev_pm_opp_adjust_voltage(cpu_dev, freq_hz, volt, volt, volt))
+		return dev_pm_opp_add(cpu_dev, freq_hz, volt);

What's going on here ? Why add OPP here ?

We update the voltage if opp were initially added as part of dev_pm_opp_of_add_table. However
if the cpu node does not have an opp table associated with it, we
do a opp_add_v1 instead.

Quoted 12 lines
Quoted 12 lines
drivers/cpufreq/qcom-cpufreq-hw.c +11 -0
+
+	/* Enable the opp after voltage update */
+	return dev_pm_opp_enable(cpu_dev, freq_hz);
+}
+
+/* Check for optional interconnect paths on CPU0 */
+static int qcom_cpufreq_find_icc_paths(struct device *dev)
+{
+	struct device *cpu_dev;
+
+	cpu_dev = get_cpu_device(0);
Quoted 8 lines

drivers/cpufreq/qcom-cpufreq-hw.c line 80

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-hw.c +6 -0
+	if (!cpu_dev)
+		return -EPROBE_DEFER;
+
+	return dev_pm_opp_of_find_icc_paths(cpu_dev, NULL);
+}
+

open code this into the probe routine.

sure

Quoted 29 lines
Quoted 29 lines
drivers/cpufreq/qcom-cpufreq-hw.c +0 -0
 static int qcom_cpufreq_hw_target_index(struct cpufreq_policy 
 *policy,
 					unsigned int index)

{

drivers/cpufreq/qcom-cpufreq-hw.c +2 -0
@@ -39,6 +86,8 @@ static int qcom_cpufreq_hw_target_index(struct 
 cpufreq_policy *policy,
 
 	writel_relaxed(index, perf_state_reg);
 
+	qcom_cpufreq_set_bw(policy, freq);
+
 	arch_set_freq_scale(policy->related_cpus, freq,
 			    policy->cpuinfo.max_freq);
drivers/cpufreq/qcom-cpufreq-hw.c +3 -0
 	return 0;

@@ -88,12 +137,30 @@ static int qcom_cpufreq_hw_read_lut(struct device 
 *cpu_dev,
 {
 	u32 data, src, lval, i, core_count, prev_freq = 0, freq;
 	u32 volt;
+	u64 rate;
 	struct cpufreq_frequency_table	*table;
+	struct device_node *opp_table_np, *np;
+	int ret;
 
 	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
 	if (!table)
 		return -ENOMEM;
Quoted 9 lines

drivers/cpufreq/qcom-cpufreq-hw.c static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, · line 154

Quoted 6 lines
drivers/cpufreq/qcom-cpufreq-hw.c +6 -0
+	ret = dev_pm_opp_of_add_table(cpu_dev);
+	if (!ret) {
+		/* Disable all opps and cross-validate against LUT */
+		opp_table_np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+		for_each_available_child_of_node(opp_table_np, np) {
+			ret = of_property_read_u64(np, "opp-hz", &rate);

No way, please use dev_pm_opp_find_freq_*() here instead to grab OPPs one by one.

sure I'll use a dev_pm_opp_find_freq_ceil loop to do the same :P

Quoted 12 lines

drivers/cpufreq/qcom-cpufreq-hw.c static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, · line 162

Quoted 9 lines
drivers/cpufreq/qcom-cpufreq-hw.c +8 -0
+			if (!ret)
+				dev_pm_opp_disable(cpu_dev, rate);
+		}
+		of_node_put(opp_table_np);
+	} else if (ret != -ENODEV) {
+		dev_err(cpu_dev, "Invalid OPP table in Device tree\n");
+		return ret;
+	}

Rather put this in the if (ret) block and so the else part doesn't need extra indentation.

https://patchwork.kernel.org/patch/11573905/

I'll need to enable fast_switch when the device does not have a opp-table associated with it or throw a error when an improper table is specified. If a table
with bw values is specified, we disable fast switch and enable scaling.

Quoted 26 lines
Quoted 26 lines
drivers/cpufreq/qcom-cpufreq-hw.c +1 -0
+
 	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
 		data = readl_relaxed(base + REG_FREQ_LUT +
drivers/cpufreq/qcom-cpufreq-hw.c +1 -1
 				      i * LUT_ROW_SIZE);

@@ -112,7 +179,7 @@ static int qcom_cpufreq_hw_read_lut(struct device 
 *cpu_dev,
 
 		if (freq != prev_freq && core_count != LUT_TURBO_IND) {
 			table[i].frequency = freq;
-			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
+			qcom_cpufreq_update_opp(cpu_dev, freq, volt);
 			dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
 				freq, core_count);
drivers/cpufreq/qcom-cpufreq-hw.c +2 -1
 		} else if (core_count == LUT_TURBO_IND) {

@@ -133,7 +200,8 @@ static int qcom_cpufreq_hw_read_lut(struct device 
 *cpu_dev,
 			if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
 				prev->frequency = prev_freq;
 				prev->flags = CPUFREQ_BOOST_FREQ;
-				dev_pm_opp_add(cpu_dev,	prev_freq * 1000, volt);
+				qcom_cpufreq_update_opp(cpu_dev, prev_freq,
+							volt);
 			}

break;

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
maintainer
#7 lore raw

Re: [PATCH v5 4/5] cpufreq: qcom: Update the bandwidth levels on frequency change

Quoted 11 lines · Sibi Sankar
Quoted 10 lines

drivers/cpufreq/qcom-cpufreq-hw.c line 63

Quoted 8 lines
drivers/cpufreq/qcom-cpufreq-hw.c +8 -0
+static int qcom_cpufreq_update_opp(struct device *cpu_dev,
+				   unsigned long freq_khz,
+				   unsigned long volt)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+
+	if (dev_pm_opp_adjust_voltage(cpu_dev, freq_hz, volt, volt, volt))
+		return dev_pm_opp_add(cpu_dev, freq_hz, volt);

What's going on here ? Why add OPP here ?

Quoted 6 lines

We update the voltage if opp were initially added as part of dev_pm_opp_of_add_table. However if the cpu node does not have an opp table associated with it, we do a opp_add_v1 instead.

Instead of depending on the failure of dev_pm_opp_adjust_voltage(), pass a flag to qcom_cpufreq_update_opp() which will decide if we want to adjust voltage or add an opp.

Signature · 1 line
viresh
author
#8 lore raw

Re: [PATCH v5 4/5] cpufreq: qcom: Update the bandwidth levels on frequency change

Quoted 19 lines · Viresh Kumar
Quoted 11 lines · Sibi Sankar
Quoted 10 lines

drivers/cpufreq/qcom-cpufreq-hw.c line 63

Quoted 8 lines
drivers/cpufreq/qcom-cpufreq-hw.c +8 -0
+static int qcom_cpufreq_update_opp(struct device *cpu_dev,
+				   unsigned long freq_khz,
+				   unsigned long volt)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+
+	if (dev_pm_opp_adjust_voltage(cpu_dev, freq_hz, volt, volt, volt))
+		return dev_pm_opp_add(cpu_dev, freq_hz, volt);

What's going on here ? Why add OPP here ?

Quoted 6 lines

We update the voltage if opp were initially added as part of dev_pm_opp_of_add_table. However if the cpu node does not have an opp table associated with it, we do a opp_add_v1 instead.

Quoted 3 lines

Instead of depending on the failure of dev_pm_opp_adjust_voltage(), pass a flag to qcom_cpufreq_update_opp() which will decide if we want to adjust voltage or add an opp.

Sure will add it in the next
re-spin.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
author
#9 lore raw

PATCH v5 5/5 cpufreq: qcom: Disable fast switch when scaling DDR/L3

In reply to Sibi Sankar

Disable fast switch when the opp-tables required for scaling DDR/L3 are populated.

Signed-off-by Sibi Sankar <[email protected]>

---

V5:
* Drop dev_pm_opp_get_path_count [Saravana]

 drivers/cpufreq/qcom-cpufreq-hw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/cpufreq/qcom-cpufreq-hw.c +2 -2
index fbd73d106a3ae..ff6ad7ac38411 100644
@@ -159,6 +159,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 	} else if (ret != -ENODEV) {
 		dev_err(cpu_dev, "Invalid OPP table in Device tree\n");
 		return ret;
+	} else {
+		policy->fast_switch_possible = true;
 	}
 
 	for (i = 0; i < LUT_MAX_ENTRIES; i++) {

@@ -308,8 +310,6 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 
 	dev_pm_opp_of_register_em(policy->cpus);
 
-	policy->fast_switch_possible = true;
-
 	return 0;
 error:
 	devm_iounmap(dev, base);
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project