diff mbox series

[v4,06/12] cpufreq: qcom: Update the bandwidth levels on frequency change

Message ID 20200504202243.5476-7-sibis@codeaurora.org (mailing list archive)
State Not Applicable, archived
Headers show
Series DDR/L3 Scaling support on SDM845 and SC7180 SoCs | expand

Commit Message

Sibi Sankar May 4, 2020, 8:22 p.m. UTC
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 <sibis@codeaurora.org>
---

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

 drivers/cpufreq/qcom-cpufreq-hw.c | 85 ++++++++++++++++++++++++++++++-
 1 file changed, 83 insertions(+), 2 deletions(-)

Comments

Viresh Kumar May 5, 2020, 4:50 a.m. UTC | #1
On 05-05-20, 01:52, Sibi Sankar wrote:
> 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 <sibis@codeaurora.org>

What about using opp_set_rate instead ?
Sibi Sankar May 5, 2020, 7:19 a.m. UTC | #2
On 2020-05-05 10:20, Viresh Kumar wrote:
> On 05-05-20, 01:52, Sibi Sankar wrote:
>> 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 <sibis@codeaurora.org>
> 
> What about using opp_set_rate instead ?

I can't use opp_set_rate since
the cpu dev does not have a
clock associated with it and the
scaling is done through writing
on perf state register.
Sibi Sankar May 26, 2020, 5:48 p.m. UTC | #3
On 2020-05-05 12:49, Sibi Sankar wrote:
> On 2020-05-05 10:20, Viresh Kumar wrote:
>> On 05-05-20, 01:52, Sibi Sankar wrote:
>>> 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 <sibis@codeaurora.org>
>> 
>> What about using opp_set_rate instead ?
> 
> I can't use opp_set_rate since
> the cpu dev does not have a
> clock associated with it and the
> scaling is done through writing
> on perf state register.

Viresh,

https://patchwork.kernel.org/cover/11548479/
GPU driver uses Georgi's series
for scaling and will need a way to
remove the icc votes in the suspend
path, (this looks like a pattern
that might be used by other clients
as well) I could probably update
opp_set_bw to support removing bw
when NULL opp is specified. Similarly
opp_set_rate will need to support
set bw to 0 when set_rate is passed
0 as target freq for the same use case.
Viresh Kumar May 27, 2020, 3:53 a.m. UTC | #4
On 26-05-20, 23:18, Sibi Sankar wrote:
> https://patchwork.kernel.org/cover/11548479/
> GPU driver uses Georgi's series
> for scaling and will need a way to
> remove the icc votes in the suspend
> path, (this looks like a pattern
> that might be used by other clients
> as well) I could probably update
> opp_set_bw to support removing bw
> when NULL opp is specified. Similarly
> opp_set_rate will need to support
> set bw to 0 when set_rate is passed
> 0 as target freq for the same use case.

Sure, please send a patch for that.
Viresh Kumar May 27, 2020, 4:05 a.m. UTC | #5
On 27-05-20, 09:23, Viresh Kumar wrote:
> On 26-05-20, 23:18, Sibi Sankar wrote:
> > https://patchwork.kernel.org/cover/11548479/
> > GPU driver uses Georgi's series
> > for scaling and will need a way to
> > remove the icc votes in the suspend
> > path, (this looks like a pattern
> > that might be used by other clients
> > as well) I could probably update
> > opp_set_bw to support removing bw
> > when NULL opp is specified. Similarly
> > opp_set_rate will need to support
> > set bw to 0 when set_rate is passed
> > 0 as target freq for the same use case.
> 
> Sure, please send a patch for that.

On a second thought, here is the patch. Please test it.

-------------------------8<-------------------------

Subject: [PATCH] opp: Remove bandwidth votes when target_freq is zero

We already drop several votes when target_freq is set to zero, drop
bandwidth votes as well.

Reported-by: Sibi Sankar <sibis@codeaurora.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c | 47 +++++++++++++++++++++++++++++++++++-----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 56d3022c1ca2..0c259d5ed232 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -725,6 +725,34 @@ static int _generic_set_opp_regulator(struct opp_table *opp_table,
 	return ret;
 }
 
+static int _set_opp_bw(const struct opp_table *opp_table,
+		       struct dev_pm_opp *opp, bool remove)
+{
+	u32 avg, peak;
+	int i, ret;
+
+	if (!opp_table->paths)
+		return 0;
+
+	for (i = 0; i < opp_table->path_count; i++) {
+		if (remove) {
+			avg = 0;
+			peak = 0;
+		} else {
+			avg = opp->bandwidth[i].avg;
+			peak = opp->bandwidth[i].peak;
+		}
+		ret = icc_set_bw(opp_table->paths[i], avg, peak);
+		if (ret) {
+			dev_err(dev, "Failed to %s bandwidth[%d]: %d\n",
+				remove ? "remove" : "set", i, ret);
+			retrun ret;
+		}
+	}
+
+	return 0;
+}
+
 static int _set_opp_custom(const struct opp_table *opp_table,
 			   struct device *dev, unsigned long old_freq,
 			   unsigned long freq,
@@ -837,12 +865,17 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 		if (!_get_opp_count(opp_table))
 			return 0;
 
-		if (!opp_table->required_opp_tables && !opp_table->regulators) {
+		if (!opp_table->required_opp_tables && !opp_table->regulators &&
+		    !opp_table->paths) {
 			dev_err(dev, "target frequency can't be 0\n");
 			ret = -EINVAL;
 			goto put_opp_table;
 		}
 
+		ret = _set_opp_bw(opp_table, opp, true);
+		if (ret)
+			return ret;
+
 		if (opp_table->regulator_enabled) {
 			regulator_disable(opp_table->regulators[0]);
 			opp_table->regulator_enabled = false;
@@ -932,16 +965,8 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq)
 			dev_err(dev, "Failed to set required opps: %d\n", ret);
 	}
 
-	if (!ret && opp_table->paths) {
-		for (i = 0; i < opp_table->path_count; i++) {
-			ret = icc_set_bw(opp_table->paths[i],
-					 opp->bandwidth[i].avg,
-					 opp->bandwidth[i].peak);
-			if (ret)
-				dev_err(dev, "Failed to set bandwidth[%d]: %d\n",
-					i, ret);
-		}
-	}
+	if (!ret)
+		ret = _set_opp_bw(opp_table, opp, false);
 
 put_opp:
 	dev_pm_opp_put(opp);
diff mbox series

Patch

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index fc92a8842e252..4fb489b69bc61 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -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,63 @@ 
 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_update_voltage(cpu_dev, freq_hz, 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_verify_icc_paths(struct device *dev)
+{
+	struct device *cpu_dev;
+	struct icc_path *path;
+	int ret;
+
+	cpu_dev = get_cpu_device(0);
+	if (!cpu_dev)
+		return -EPROBE_DEFER;
+
+	path = of_icc_get(cpu_dev, NULL);
+	ret = PTR_ERR_OR_ZERO(path);
+	if (ret) {
+		if (ret != -EPROBE_DEFER)
+			dev_err(cpu_dev, "Failed to get paths ddr/l3 scaling off\n");
+		return ret;
+	}
+
+	icc_put(path);
+	return ret;
+}
+
 static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 					unsigned int index)
 {
@@ -39,6 +97,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 +148,27 @@  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);
+	}
+
 	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
 		data = readl_relaxed(base + REG_FREQ_LUT +
 				      i * LUT_ROW_SIZE);
@@ -112,7 +187,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 +208,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 +330,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 +378,10 @@  static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
 
 	global_pdev = pdev;
 
+	ret = qcom_cpufreq_verify_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");