| 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 |
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 ?
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.
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.
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 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 --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");
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(-)