This RFC series aims to extend cpu based scaling support to L3/DDR on SDM845 and SC7180 SoCs. Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev Patches [5-7] - Hack in a way to add/remove multiple opp tables to a single device. I am yet to fix the debugfs to support multiple opp_tables per device but wanted to send what was working upstream to get an idea if multiple opp tables per device is a feature that will be useful upstream. Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845 and SC7180 SoCs. 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 Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420 Some alternate ways of hosting the opp-tables: https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 Viresh didn't really like ^^ bindings and they dont really scale well. Just including them here for completeness. Depends on the following series: [1] https://patchwork.kernel.org/cover/11277199/ [2] https://patchwork.kernel.org/cover/11055499/ [3] https://patchwork.kernel.org/cover/11326381/ Sibi Sankar (10): arm64: dts: qcom: sdm845: Add SoC compatible to MTP cpufreq: blacklist SDM845 in cpufreq-dt-platdev cpufreq: blacklist SC7180 in cpufreq-dt-platdev OPP: Add and export helper to update voltage opp: of: export _opp_of_get_opp_desc_node opp: Allow multiple opp_tables to be mapped to a single device opp: Remove multiple attached opp tables from a device cpufreq: qcom: Update the bandwidth levels on frequency change arm64: dts: qcom: sdm845: Add cpu OPP tables arm64: dts: qcom: sc7180: Add cpu OPP tables arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++ arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 ++++++++++++++++++++++++ drivers/cpufreq/cpufreq-dt-platdev.c | 2 + drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++-- drivers/opp/core.c | 111 +++++- drivers/opp/of.c | 3 +- drivers/opp/opp.h | 2 + include/linux/pm_opp.h | 10 + 9 files changed, 1083 insertions(+), 33 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add missing SoC compatible to SDM845 MTP board file. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts index 09ad37b0dd71d..54087847794aa 100644 --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts @@ -13,7 +13,7 @@ / { model = "Qualcomm Technologies, Inc. SDM845 MTP"; - compatible = "qcom,sdm845-mtp"; + compatible = "qcom,sdm845-mtp", "qcom,sdm845"; aliases { serial0 = &uart9; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add SDM845 to cpufreq-dt-platdev blacklist. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/cpufreq/cpufreq-dt-platdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index f2ae9cd455c17..5492cf3c9dc18 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -130,6 +130,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", }, -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add SC7180 to cpufreq-dt-platdev blacklist. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/cpufreq/cpufreq-dt-platdev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c index 5492cf3c9dc18..580abc777d9d8 100644 --- a/drivers/cpufreq/cpufreq-dt-platdev.c +++ b/drivers/cpufreq/cpufreq-dt-platdev.c @@ -130,6 +130,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", }, -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add and export 'dev_pm_opp_update_voltage' to find and update voltage of an opp for a given frequency. This will be useful to update the opps with voltages read back from firmware. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/opp/core.c | 55 ++++++++++++++++++++++++++++++++++++++++++ include/linux/pm_opp.h | 10 ++++++++ 2 files changed, 65 insertions(+) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index 9aa2a44a5d638..f241e83ec926a 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -2503,6 +2503,61 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq) } EXPORT_SYMBOL_GPL(dev_pm_opp_disable); +/** + * dev_pm_opp_update_voltage() - Find and update voltage + * @dev: device for which we do this operation + * @freq: OPP frequency to update voltage + * @u_volt: voltage requested for this opp + * + * Find and update voltage of a disabled opp corresponding to the given + * frequency. This is useful only for devices with single power supply. + * + * Return: 0 if modification was successful or a negative error value. + */ +int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq, + unsigned long u_volt) +{ + struct dev_pm_opp *opp = ERR_PTR(-ENODEV); + struct opp_table *opp_table; + unsigned long tol; + int ret = 0; + + /* Find the opp_table */ + opp_table = _find_opp_table(dev); + if (IS_ERR(opp_table)) { + ret = PTR_ERR(opp_table); + dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret); + return PTR_ERR(opp_table); + } + + opp = dev_pm_opp_find_freq_exact(dev, freq, false); + if (IS_ERR(opp)) { + ret = PTR_ERR(opp); + goto put_table; + } + + mutex_lock(&opp_table->lock); + + /* update only if the opp is disabled */ + if (opp->available) { + ret = -EBUSY; + goto unlock; + } + + tol = u_volt * opp_table->voltage_tolerance_v1 / 100; + opp->supplies[0].u_volt_min = u_volt - tol; + opp->supplies[0].u_volt = u_volt; + opp->supplies[0].u_volt_min = u_volt + tol; + +unlock: + mutex_unlock(&opp_table->lock); + dev_pm_opp_put(opp); +put_table: + dev_pm_opp_put_opp_table(opp_table); + return ret; +} +EXPORT_SYMBOL_GPL(dev_pm_opp_update_voltage); + /** * dev_pm_opp_register_notifier() - Register OPP notifier for the device * @dev: Device for which notifier needs to be registered diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h index 62c88898bae46..b26d492cbd635 100644 --- a/include/linux/pm_opp.h +++ b/include/linux/pm_opp.h @@ -132,6 +132,9 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq); int dev_pm_opp_disable(struct device *dev, unsigned long freq); +int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq, + unsigned long u_volt); + int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb); int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nb); @@ -311,6 +314,13 @@ static inline int dev_pm_opp_disable(struct device *dev, unsigned long freq) return 0; } +static inline int dev_pm_opp_update_voltage(struct device *dev, + unsigned long freq, + unsigned long u_volt) +{ + return 0; +} + static inline int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb) { return -ENOTSUPP; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Export _opp_of_get_opp_desc_node in preparation for attaching multiple opp_table per device. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/opp/of.c | 3 +-- drivers/opp/opp.h | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 43acf54934be7..015ee9ab09485 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -25,8 +25,7 @@ * Returns opp descriptor node for a device node, caller must * do of_node_put(). */ -static struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, - int index) +struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, int index) { /* "operating-points-v2" can be an array for power domain providers */ return of_parse_phandle(np, "operating-points-v2", index); diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h index 9e021f819acca..6f56ff6038605 100644 --- a/drivers/opp/opp.h +++ b/drivers/opp/opp.h @@ -226,12 +226,14 @@ void _put_opp_list_kref(struct opp_table *opp_table); void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index); void _of_clear_opp_table(struct opp_table *opp_table); struct opp_table *_managed_opp(struct device *dev, int index); +struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, int index); void _of_opp_free_required_opps(struct opp_table *opp_table, struct dev_pm_opp *opp); #else static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {} static inline void _of_clear_opp_table(struct opp_table *opp_table) {} static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; } +static inline struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, int index) { return NULL; } static inline void _of_opp_free_required_opps(struct opp_table *opp_table, struct dev_pm_opp *opp) {} #endif -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Introduce _find_opp_table_indexed and its unlocked variant to allow for multiple distinct opp_tables to be linked to a single device. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/opp/core.c | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index f241e83ec926a..e9d633c9e40b1 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -46,19 +46,26 @@ static struct opp_device *_find_opp_dev(const struct device *dev, return NULL; } -static struct opp_table *_find_opp_table_unlocked(struct device *dev) +static struct opp_table *_find_opp_table_indexed_unlocked(struct device *dev, + int index) { struct opp_table *opp_table; + struct device_node *np; bool found; + np = _opp_of_get_opp_desc_node(dev->of_node, index); + of_node_put(np); + list_for_each_entry(opp_table, &opp_tables, node) { mutex_lock(&opp_table->lock); found = !!_find_opp_dev(dev, opp_table); mutex_unlock(&opp_table->lock); if (found) { - _get_opp_table_kref(opp_table); + if (np && opp_table->np != np) + continue; + _get_opp_table_kref(opp_table); return opp_table; } } @@ -67,17 +74,19 @@ static struct opp_table *_find_opp_table_unlocked(struct device *dev) } /** - * _find_opp_table() - find opp_table struct using device pointer + * _find_opp_table_indexed() - find opp_table struct using device pointer * @dev: device pointer used to lookup OPP table + * @index: Index number. * - * Search OPP table for one containing matching device. + * Search OPP table for one containing matching device at the given + * index. * * Return: pointer to 'struct opp_table' if found, otherwise -ENODEV or * -EINVAL based on type of error. * * The callers must call dev_pm_opp_put_opp_table() after the table is used. */ -struct opp_table *_find_opp_table(struct device *dev) +struct opp_table *_find_opp_table_indexed(struct device *dev, int index) { struct opp_table *opp_table; @@ -87,12 +96,28 @@ struct opp_table *_find_opp_table(struct device *dev) } mutex_lock(&opp_table_lock); - opp_table = _find_opp_table_unlocked(dev); + opp_table = _find_opp_table_indexed_unlocked(dev, index); mutex_unlock(&opp_table_lock); return opp_table; } +/** + * _find_opp_table() - find opp_table struct using device pointer + * @dev: device pointer used to lookup OPP table + * + * Search OPP table for one containing matching device. + * + * Return: pointer to 'struct opp_table' if found, otherwise -ENODEV or + * -EINVAL based on type of error. + * + * The callers must call dev_pm_opp_put_opp_table() after the table is used. + */ +struct opp_table *_find_opp_table(struct device *dev) +{ + return _find_opp_table_indexed(dev, 0); +} + /** * dev_pm_opp_get_voltage() - Gets the voltage corresponding to an opp * @opp: opp for which voltage has to be returned for @@ -1238,7 +1263,7 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index) /* Hold our table modification lock here */ mutex_lock(&opp_table_lock); - opp_table = _find_opp_table_unlocked(dev); + opp_table = _find_opp_table_indexed_unlocked(dev, index); if (!IS_ERR(opp_table)) goto unlock; @@ -2612,7 +2637,7 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev) struct opp_table *opp_table; /* Check for existing table for 'dev' */ - opp_table = _find_opp_table(dev); + opp_table = _find_opp_table_indexed(dev, 0); if (IS_ERR(opp_table)) { int error = PTR_ERR(opp_table); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Introduce _dev_pm_opp_find_and_remove_table_indexed to remove all the linked opp tables from a device. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/opp/core.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/drivers/opp/core.c b/drivers/opp/core.c index e9d633c9e40b1..cc2f156db7fda 100644 --- a/drivers/opp/core.c +++ b/drivers/opp/core.c @@ -2632,12 +2632,13 @@ int dev_pm_opp_unregister_notifier(struct device *dev, } EXPORT_SYMBOL(dev_pm_opp_unregister_notifier); -void _dev_pm_opp_find_and_remove_table(struct device *dev) +static void _dev_pm_opp_find_and_remove_table_indexed(struct device *dev, + int index) { struct opp_table *opp_table; /* Check for existing table for 'dev' */ - opp_table = _find_opp_table_indexed(dev, 0); + opp_table = _find_opp_table_indexed(dev, index); if (IS_ERR(opp_table)) { int error = PTR_ERR(opp_table); @@ -2658,6 +2659,18 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev) dev_pm_opp_put_opp_table(opp_table); } +void _dev_pm_opp_find_and_remove_table(struct device *dev) +{ + int count, i; + + count = of_count_phandle_with_args(dev->of_node, + "operating-points-v2", NULL); + count = max(count, 1); + + for (i = 0; i < count; i++) + _dev_pm_opp_find_and_remove_table_indexed(dev, i); +} + /** * dev_pm_opp_remove_table() - Free all OPPs associated with the device * @dev: device pointer used to lookup OPP table. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add support to parse and update OPP tables attached to the cpu device when the required OPP bandwidth values are populated to enable scaling of DDR/L3 bandwidth levels with frequency change. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++++++++++++++++++--- 1 file changed, 225 insertions(+), 21 deletions(-) diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c index fc92a8842e252..348eb2fdaccaf 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> @@ -28,17 +29,194 @@ #define REG_VOLT_LUT 0x114 #define REG_PERF_STATE 0x920 +#define QCOM_ICC_TAG_ACTIVE_ONLY 1 + static unsigned long cpu_hw_rate, xo_rate; static struct platform_device *global_pdev; +/* opp table indices */ +enum { + QCOM_CPU_OPP_TABLE_INDEX, + QCOM_CPU_DDR_OPP_TABLE_INDEX, + QCOM_CPU_L3_OPP_TABLE_INDEX, +}; + +/* icc path indices */ +enum { + ICC_DDR_PATH, + ICC_L3_PATH, +}; + +struct qcom_cpufreq_hw_res { + void __iomem *base; + + struct device *cpu_dev; + + /* ddr/l3 icc paths */ + struct icc_path *path[2]; + + /* cpu/ddr/l3 opp tables */ + struct opp_table *opp_table[3]; +}; + +struct cpufreq_hw_icc_info { + const char *icc_path_name; + u8 table_index; + u32 tag; +}; + +static const struct cpufreq_hw_icc_info icc_info[] = { + { + .icc_path_name = "cpu-ddr", + .table_index = QCOM_CPU_DDR_OPP_TABLE_INDEX, + .tag = QCOM_ICC_TAG_ACTIVE_ONLY, + }, + { + .icc_path_name = "cpu-l3", + .table_index = QCOM_CPU_L3_OPP_TABLE_INDEX, + .tag = 0, + }, +}; + +static int qcom_cpufreq_hw_add_opp_table(struct qcom_cpufreq_hw_res *res) +{ + struct opp_table **table = res->opp_table; + struct device_node *opp_table_np, *np; + struct device *dev = res->cpu_dev; + int ret, i; + u64 rate; + + for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) { + ret = dev_pm_opp_of_add_table_indexed(dev, i); + if (ret) { + dev_dbg(dev, "Add OPP table failed index %d: %d\n", + i, ret); + goto err; + } + + table[i] = dev_pm_opp_get_opp_table_indexed(dev, i); + if (!table[i]) { + dev_dbg(dev, "Get OPP table failed index %d\n", i); + ret = -EINVAL; + goto err; + } + } + + /* Disable all cpu opps and cross-validate against LUT */ + opp_table_np = dev_pm_opp_of_get_opp_desc_node(dev); + for_each_available_child_of_node(opp_table_np, np) { + ret = of_property_read_u64(np, "opp-hz", &rate); + if (ret) + continue; + + dev_pm_opp_disable(dev, rate); + } + of_node_put(opp_table_np); + + return 0; + +err: + for (; i >= 0; i--) { + if (table[i]) { + dev_pm_opp_put_opp_table(table[i]); + table[i] = NULL; + } + } + + dev_pm_opp_remove_table(dev); + return ret; +} + +static int qcom_cpufreq_hw_icc_init(struct qcom_cpufreq_hw_res *res, u8 index) +{ + const struct cpufreq_hw_icc_info *info = &icc_info[index]; + struct icc_path **path = res->path; + struct device *dev = res->cpu_dev; + + path[index] = of_icc_get(dev, info->icc_path_name); + if (IS_ERR(path[index])) { + dev_dbg(dev, "ICC %s path get failed ret: %ld\n", + info->icc_path_name, PTR_ERR(path[index])); + return PTR_ERR(path[index]); + } + + icc_set_tag(path[index], info->tag); + + return 0; +} + +static void qcom_cpufreq_hw_icc_set(struct qcom_cpufreq_hw_res *res, + unsigned long freq) +{ + struct opp_table **table = res->opp_table; + const struct cpufreq_hw_icc_info *info; + unsigned long freq_hz = freq * 1000; + struct icc_path **path = res->path; + struct device *dev = res->cpu_dev; + struct dev_pm_opp *cpu_opp, *opp; + struct opp_table *cpu_opp_table; + unsigned long peak_bw, avg_bw; + int ret; + int i; + + cpu_opp_table = table[QCOM_CPU_OPP_TABLE_INDEX]; + if (!cpu_opp_table) + return; + + cpu_opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true); + if (IS_ERR_OR_NULL(cpu_opp)) + return; + + for (i = 0; i <= ICC_L3_PATH; i++) { + if (IS_ERR(path[i])) { + if (PTR_ERR(path[i]) != -EPROBE_DEFER) + continue; + + ret = qcom_cpufreq_hw_icc_init(res, i); + if (ret) + continue; + } + + info = &icc_info[i]; + + opp = dev_pm_opp_xlate_required_opp(cpu_opp_table, + table[info->table_index], + cpu_opp); + if (IS_ERR_OR_NULL(opp)) + continue; + + peak_bw = dev_pm_opp_get_bw(opp, &avg_bw); + dev_pm_opp_put(opp); + + icc_set_bw(res->path[i], avg_bw, peak_bw); + } + dev_pm_opp_put(cpu_opp); +} + +static int qcom_cpufreq_update_opp(struct device *dev, + unsigned long freq_khz, + unsigned long volt) +{ + unsigned long freq_hz = freq_khz * 1000; + + if (dev_pm_opp_update_voltage(dev, freq_hz, volt)) + return dev_pm_opp_add(dev, freq_hz, volt); + + /* Enable the opp after voltage update*/ + return dev_pm_opp_enable(dev, freq_hz); +} + static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, unsigned int index) { - void __iomem *perf_state_reg = policy->driver_data; + struct qcom_cpufreq_hw_res *res = policy->driver_data; + void __iomem *perf_state_reg = res->base + REG_PERF_STATE; unsigned long freq = policy->freq_table[index].frequency; writel_relaxed(index, perf_state_reg); + qcom_cpufreq_hw_icc_set(res, freq); + arch_set_freq_scale(policy->related_cpus, freq, policy->cpuinfo.max_freq); return 0; @@ -46,6 +224,7 @@ static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) { + struct qcom_cpufreq_hw_res *res; void __iomem *perf_state_reg; struct cpufreq_policy *policy; unsigned int index; @@ -54,7 +233,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) if (!policy) return 0; - perf_state_reg = policy->driver_data; + res = policy->driver_data; + perf_state_reg = res->base + REG_PERF_STATE; index = readl_relaxed(perf_state_reg); index = min(index, LUT_MAX_ENTRIES - 1); @@ -65,7 +245,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, unsigned int target_freq) { - void __iomem *perf_state_reg = policy->driver_data; + struct qcom_cpufreq_hw_res *res = policy->driver_data; + void __iomem *perf_state_reg = res->base + REG_PERF_STATE; int index; unsigned long freq; @@ -82,18 +263,24 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, return freq; } -static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, - struct cpufreq_policy *policy, - void __iomem *base) +static int qcom_cpufreq_hw_read_lut(struct cpufreq_policy *policy, + struct qcom_cpufreq_hw_res *res) { u32 data, src, lval, i, core_count, prev_freq = 0, freq; u32 volt; struct cpufreq_frequency_table *table; + struct device *cpu_dev = res->cpu_dev; + void __iomem *base = res->base; + int ret; table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL); if (!table) return -ENOMEM; + ret = qcom_cpufreq_hw_add_opp_table(res); + if (ret) + dev_dbg(cpu_dev, "Add OPP tables failed from dt\n"); + for (i = 0; i < LUT_MAX_ENTRIES; i++) { data = readl_relaxed(base + REG_FREQ_LUT + i * LUT_ROW_SIZE); @@ -112,7 +299,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 +320,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; @@ -175,11 +363,10 @@ static void qcom_get_related_cpus(int index, struct cpumask *m) static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) { struct device *dev = &global_pdev->dev; + struct qcom_cpufreq_hw_res *res; struct of_phandle_args args; struct device_node *cpu_np; struct device *cpu_dev; - struct resource *res; - void __iomem *base; int ret, index; cpu_dev = get_cpu_device(policy->cpu); @@ -201,16 +388,17 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) index = args.args[0]; - res = platform_get_resource(global_pdev, IORESOURCE_MEM, index); + res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL); if (!res) - return -ENODEV; - - base = devm_ioremap(dev, res->start, resource_size(res)); - if (!base) return -ENOMEM; + res->cpu_dev = cpu_dev; + res->base = devm_platform_ioremap_resource(global_pdev, index); + if (IS_ERR(res->base)) + return PTR_ERR(res->base); + /* HW should be in enabled state to proceed */ - if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) { + if (!(readl_relaxed(res->base + REG_ENABLE) & 0x1)) { dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index); ret = -ENODEV; goto error; @@ -223,9 +411,9 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) goto error; } - policy->driver_data = base + REG_PERF_STATE; + policy->driver_data = res; - ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy, base); + ret = qcom_cpufreq_hw_read_lut(policy, res); if (ret) { dev_err(dev, "Domain-%d failed to read LUT\n", index); goto error; @@ -240,22 +428,38 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) dev_pm_opp_of_register_em(policy->cpus); - policy->fast_switch_possible = true; + if (!res->opp_table[QCOM_CPU_OPP_TABLE_INDEX]) { + policy->fast_switch_possible = true; + } else { + qcom_cpufreq_hw_icc_init(res, ICC_DDR_PATH); + qcom_cpufreq_hw_icc_init(res, ICC_L3_PATH); + } return 0; error: - devm_iounmap(dev, base); + devm_iounmap(dev, res->base); + devm_kfree(&global_pdev->dev, res); return ret; } static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy) { + struct qcom_cpufreq_hw_res *res = policy->driver_data; struct device *cpu_dev = get_cpu_device(policy->cpu); - void __iomem *base = policy->driver_data - REG_PERF_STATE; + struct opp_table **table = res->opp_table; + void __iomem *base = res->base; + int i; + + for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) { + if (table[i]) + dev_pm_opp_put_opp_table(table[i]); + } dev_pm_opp_remove_all_dynamic(cpu_dev); kfree(policy->freq_table); + dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); devm_iounmap(&global_pdev->dev, base); + devm_kfree(&global_pdev->dev, res); return 0; } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 SoCs. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 +++++++++++++++++++++++++++ 1 file changed, 453 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index c036bab49fc03..8cb976118407b 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -199,6 +199,12 @@ qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>; next-level-cache = <&L2_0>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_0: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -221,6 +227,12 @@ qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>; next-level-cache = <&L2_100>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_100: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -240,6 +252,12 @@ qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>; next-level-cache = <&L2_200>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_200: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -259,6 +277,12 @@ qcom,freq-domain = <&cpufreq_hw 0>; #cooling-cells = <2>; next-level-cache = <&L2_300>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_300: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -278,6 +302,12 @@ qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_400>; + operating-points-v2 = <&cpu4_opp_table>, + <&cpu4_ddr_bw_opp_table>, + <&cpu4_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_400: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -297,6 +327,12 @@ qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_500>; + operating-points-v2 = <&cpu4_opp_table>, + <&cpu4_ddr_bw_opp_table>, + <&cpu4_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_500: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -316,6 +352,12 @@ qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_600>; + operating-points-v2 = <&cpu4_opp_table>, + <&cpu4_ddr_bw_opp_table>, + <&cpu4_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_600: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -335,6 +377,12 @@ qcom,freq-domain = <&cpufreq_hw 1>; #cooling-cells = <2>; next-level-cache = <&L2_700>; + operating-points-v2 = <&cpu4_opp_table>, + <&cpu4_ddr_bw_opp_table>, + <&cpu4_l3_bw_opp_table>; + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; L2_700: l2-cache { compatible = "cache"; next-level-cache = <&L3_0>; @@ -432,6 +480,231 @@ }; }; + cpu0_opp_table: cpu0_opp_table { + compatible = "operating-points-v2"; + opp-shared; + + cpu0_opp1: opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>; + }; + + cpu0_opp2: opp-403200000 { + opp-hz = /bits/ 64 <403200000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>; + }; + + cpu0_opp3: opp-480000000 { + opp-hz = /bits/ 64 <480000000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp2>; + }; + + cpu0_opp4: opp-576000000 { + opp-hz = /bits/ 64 <576000000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp2>; + }; + + cpu0_opp5: opp-652800000 { + opp-hz = /bits/ 64 <652800000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp3>; + }; + + cpu0_opp6: opp-748800000 { + opp-hz = /bits/ 64 <748800000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp4>; + }; + + cpu0_opp7: opp-825600000 { + opp-hz = /bits/ 64 <825600000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp4>; + }; + + cpu0_opp8: opp-902400000 { + opp-hz = /bits/ 64 <902400000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp5>; + }; + + cpu0_opp9: opp-979200000 { + opp-hz = /bits/ 64 <979200000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp6>; + }; + + cpu0_opp10: opp-1056000000 { + opp-hz = /bits/ 64 <1056000000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp6>; + }; + + cpu0_opp11: opp-1132800000 { + opp-hz = /bits/ 64 <1132800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp7>; + }; + + cpu0_opp12: opp-1228800000 { + opp-hz = /bits/ 64 <1228800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp8>; + }; + + cpu0_opp13: opp-1324800000 { + opp-hz = /bits/ 64 <1324800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp9>; + }; + + cpu0_opp14: opp-1420800000 { + opp-hz = /bits/ 64 <1420800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp10>; + }; + + cpu0_opp15: opp-1516800000 { + opp-hz = /bits/ 64 <1516800000>; + required-opps = <&cpu0_ddr_bw_opp5>, <&cpu0_l3_bw_opp11>; + }; + + cpu0_opp16: opp-1612800000 { + opp-hz = /bits/ 64 <1612800000>; + required-opps = <&cpu0_ddr_bw_opp7>, <&cpu0_l3_bw_opp11>; + }; + + cpu0_opp17: opp-1689600000 { + opp-hz = /bits/ 64 <1689600000>; + required-opps = <&cpu0_ddr_bw_opp7>, <&cpu0_l3_bw_opp12>; + }; + + cpu0_opp18: opp-1766400000 { + opp-hz = /bits/ 64 <1766400000>; + required-opps = <&cpu0_ddr_bw_opp7>, <&cpu0_l3_bw_opp13>; + }; + }; + + cpu4_opp_table: cpu4_opp_table { + compatible = "operating-points-v2"; + opp-shared; + + cpu4_opp1: opp-825600000 { + opp-hz = /bits/ 64 <825600000>; + required-opps = <&cpu4_ddr_bw_opp1>, <&cpu4_l3_bw_opp1>; + }; + + cpu4_opp2: opp-902400000 { + opp-hz = /bits/ 64 <902400000>; + required-opps = <&cpu4_ddr_bw_opp1>, <&cpu4_l3_bw_opp1>; + }; + + cpu4_opp3: opp-979200000 { + opp-hz = /bits/ 64 <979200000>; + required-opps = <&cpu4_ddr_bw_opp1>, <&cpu4_l3_bw_opp1>; + }; + + cpu4_opp4: opp-1056000000 { + opp-hz = /bits/ 64 <1056000000>; + required-opps = <&cpu4_ddr_bw_opp5>, <&cpu4_l3_bw_opp3>; + }; + + cpu4_opp5: opp-1209600000 { + opp-hz = /bits/ 64 <1209600000>; + required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp3>; + }; + + cpu4_opp6: opp-1286400000 { + opp-hz = /bits/ 64 <1286400000>; + required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp3>; + }; + + cpu4_opp7: opp-1363200000 { + opp-hz = /bits/ 64 <1363200000>; + required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp5>; + }; + + cpu4_opp8: opp-1459200000 { + opp-hz = /bits/ 64 <1459200000>; + required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp5>; + }; + + cpu4_opp9: opp-1536000000 { + opp-hz = /bits/ 64 <1536000000>; + required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp5>; + }; + + cpu4_opp10: opp-1612800000 { + opp-hz = /bits/ 64 <1612800000>; + required-opps = <&cpu4_ddr_bw_opp7>, <&cpu4_l3_bw_opp5>; + }; + + cpu4_opp11: opp-1689600000 { + opp-hz = /bits/ 64 <1689600000>; + required-opps = <&cpu4_ddr_bw_opp7>, <&cpu4_l3_bw_opp8>; + }; + + cpu4_opp12: opp-1766400000 { + opp-hz = /bits/ 64 <1766400000>; + required-opps = <&cpu4_ddr_bw_opp8>, <&cpu4_l3_bw_opp8>; + }; + + cpu4_opp13: opp-1843200000 { + opp-hz = /bits/ 64 <1843200000>; + required-opps = <&cpu4_ddr_bw_opp8>, <&cpu4_l3_bw_opp8>; + }; + + cpu4_opp14: opp-1920000000 { + opp-hz = /bits/ 64 <1920000000>; + required-opps = <&cpu4_ddr_bw_opp8>, <&cpu4_l3_bw_opp8>; + }; + + cpu4_opp15: opp-1996800000 { + opp-hz = /bits/ 64 <1996800000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>; + }; + + cpu4_opp16: opp-2092800000 { + opp-hz = /bits/ 64 <2092800000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>; + }; + + cpu4_opp17: opp-2169600000 { + opp-hz = /bits/ 64 <2169600000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>; + }; + + cpu4_opp18: opp-2246400000 { + opp-hz = /bits/ 64 <2246400000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>; + }; + + cpu4_opp19: opp-2323200000 { + opp-hz = /bits/ 64 <2323200000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>; + }; + + cpu4_opp20: opp-2400000000 { + opp-hz = /bits/ 64 <2400000000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>; + }; + + cpu4_opp21: opp-2476800000 { + opp-hz = /bits/ 64 <2476800000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>; + }; + + cpu4_opp22: opp-2553600000 { + opp-hz = /bits/ 64 <2553600000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>; + }; + + cpu4_opp23: opp-2649600000 { + opp-hz = /bits/ 64 <2649600000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>; + }; + + cpu4_opp24: opp-2745600000 { + opp-hz = /bits/ 64 <2745600000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp11>; + }; + + cpu4_opp25: opp-2803200000 { + opp-hz = /bits/ 64 <2803200000>; + required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp11>; + }; + }; + pmu { compatible = "arm,armv8-pmuv3"; interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>; @@ -3345,6 +3618,186 @@ #freq-domain-cells = <1>; }; + cpu0_ddr_bw_opp_table: cpu0-ddr-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu0_ddr_bw_opp1: opp-200000000 { + opp-peak-kBps =/bits/ 32 <800000>; + }; + + cpu0_ddr_bw_opp2: opp-300000000 { + opp-peak-kBps = /bits/ 32 <1200000>; + }; + + cpu0_ddr_bw_opp3: opp-451000000 { + opp-peak-kBps = /bits/ 32 <1804000>; + }; + + cpu0_ddr_bw_opp4: opp-547000000 { + opp-peak-kBps = /bits/ 32 <2188000>; + }; + + cpu0_ddr_bw_opp5: opp-681000000 { + opp-peak-kBps = /bits/ 32 <2724000>; + }; + + cpu0_ddr_bw_opp6: opp-768000000 { + opp-peak-kBps = /bits/ 32 <3072000>; + }; + + cpu0_ddr_bw_opp7: opp-1017000000 { + opp-peak-kBps = /bits/ 32 <4068000>; + }; + }; + + cpu0_l3_bw_opp_table: cpu0-l3-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu0_l3_bw_opp1: opp-300000000 { + opp-peak-kBps = /bits/ 32 <4800000>; + }; + + cpu0_l3_bw_opp2: opp-403200000 { + opp-peak-kBps = /bits/ 32 <6451200>; + }; + + cpu0_l3_bw_opp3: opp-480000000 { + opp-peak-kBps = /bits/ 32 <7680000>; + }; + + cpu0_l3_bw_opp4: opp-576000000 { + opp-peak-kBps = /bits/ 32 <9216000>; + }; + + cpu0_l3_bw_opp5: opp-652800000 { + opp-peak-kBps = /bits/ 32 <10444800>; + }; + + cpu0_l3_bw_opp6: opp-748800000 { + opp-peak-kBps = /bits/ 32 <11980800>; + }; + + cpu0_l3_bw_opp7: opp-844800000 { + opp-peak-kBps = /bits/ 32 <13516800>; + }; + + cpu0_l3_bw_opp8: opp-940800000 { + opp-peak-kBps = /bits/ 32 <15052800>; + }; + + cpu0_l3_bw_opp9: opp-1036800000 { + opp-peak-kBps = /bits/ 32 <16588800>; + }; + + cpu0_l3_bw_opp10: opp-1132800000 { + opp-peak-kBps = /bits/ 32 <18124800>; + }; + + cpu0_l3_bw_opp11: opp-1209600000 { + opp-peak-kBps = /bits/ 32 <19353600>; + }; + + cpu0_l3_bw_opp12: opp-1305600000 { + opp-peak-kBps = /bits/ 32 <20889600>; + }; + + cpu0_l3_bw_opp13: opp-1401600000 { + opp-peak-kBps = /bits/ 32 <22425600>; + }; + }; + + cpu4_ddr_bw_opp_table: cpu4-ddr-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu4_ddr_bw_opp1: opp-300000000 { + opp-peak-kBps = /bits/ 32 <1200000>; + }; + + cpu4_ddr_bw_opp2: opp-451000000 { + opp-peak-kBps = /bits/ 32 <1804000>; + }; + + cpu4_ddr_bw_opp3: opp-547000000 { + opp-peak-kBps = /bits/ 32 <2188000>; + }; + + cpu4_ddr_bw_opp4: opp-681000000 { + opp-peak-kBps = /bits/ 32 <2724000>; + }; + + cpu4_ddr_bw_opp5: opp-768000000 { + opp-peak-kBps = /bits/ 32 <3072000>; + }; + + cpu4_ddr_bw_opp6: opp-1017000000 { + opp-peak-kBps = /bits/ 32 <4068000>; + }; + + cpu4_ddr_bw_opp7: opp-1296000000 { + opp-peak-kBps = /bits/ 32 <5184000>; + }; + + cpu4_ddr_bw_opp8: opp-1555000000 { + opp-peak-kBps = /bits/ 32 <6220000>; + }; + + cpu4_ddr_bw_opp9: opp-1804000000 { + opp-peak-kBps = /bits/ 32 <7216000>; + }; + }; + + cpu4_l3_bw_opp_table: cpu4-l3-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu4_l3_bw_opp1: opp-576000000 { + opp-peak-kBps = /bits/ 32 <9216000>; + }; + + cpu4_l3_bw_opp2: opp-652800000 { + opp-peak-kBps = /bits/ 32 <10444800>; + }; + + cpu4_l3_bw_opp3: opp-748800000 { + opp-peak-kBps = /bits/ 32 <11980800>; + }; + + cpu4_l3_bw_opp4: opp-844800000 { + opp-peak-kBps = /bits/ 32 <13516800>; + }; + + cpu4_l3_bw_opp5: opp-940800000 { + opp-peak-kBps = /bits/ 32 <15052800>; + }; + + cpu4_l3_bw_opp6: opp-1036800000 { + opp-peak-kBps = /bits/ 32 <16588800>; + }; + + cpu4_l3_bw_opp7: opp-1132800000 { + opp-peak-kBps = /bits/ 32 <18124800>; + }; + + cpu4_l3_bw_opp8: opp-1209600000 { + opp-peak-kBps = /bits/ 32 <19353600>; + }; + + cpu4_l3_bw_opp9: opp-1305600000 { + opp-peak-kBps = /bits/ 32 <20889600>; + }; + + cpu4_l3_bw_opp10: opp-1401600000 { + opp-peak-kBps = /bits/ 32 <22425600>; + }; + + cpu4_l3_bw_opp11: opp-1478400000 { + opp-peak-kBps = /bits/ 32 <23654400>; + }; + }; + wifi: wifi@18800000 { compatible = "qcom,wcn3990-wifi"; status = "disabled"; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Add OPP tables required to scale DDR/L3 per freq-domain on SC7180 SoCs. Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++++++++++++++ 1 file changed, 287 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index ba53ddf17ee3a..699cafc1a727d 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -109,6 +109,12 @@ enable-method = "psci"; capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; next-level-cache = <&L2_0>; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; @@ -129,6 +135,12 @@ capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; next-level-cache = <&L2_100>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; L2_100: l2-cache { @@ -145,6 +157,12 @@ capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; next-level-cache = <&L2_200>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; L2_200: l2-cache { @@ -161,6 +179,12 @@ capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; next-level-cache = <&L2_300>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; L2_300: l2-cache { @@ -177,6 +201,12 @@ capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; next-level-cache = <&L2_400>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; L2_400: l2-cache { @@ -193,6 +223,12 @@ capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; next-level-cache = <&L2_500>; + operating-points-v2 = <&cpu0_opp_table>, + <&cpu0_ddr_bw_opp_table>, + <&cpu0_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; L2_500: l2-cache { @@ -209,6 +245,12 @@ capacity-dmips-mhz = <1740>; dynamic-power-coefficient = <405>; next-level-cache = <&L2_600>; + operating-points-v2 = <&cpu6_opp_table>, + <&cpu6_ddr_bw_opp_table>, + <&cpu6_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 1>; L2_600: l2-cache { @@ -225,6 +267,12 @@ capacity-dmips-mhz = <1740>; dynamic-power-coefficient = <405>; next-level-cache = <&L2_700>; + operating-points-v2 = <&cpu6_opp_table>, + <&cpu6_ddr_bw_opp_table>, + <&cpu6_l3_bw_opp_table>; + interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; + interconnect-names = "cpu-ddr", "cpu-l3"; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 1>; L2_700: l2-cache { @@ -270,6 +318,245 @@ }; }; + cpu0_opp_table: cpu0_opp_table { + compatible = "operating-points-v2"; + opp-shared; + + cpu0_opp1: opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>; + }; + + cpu0_opp2: opp-576000000 { + opp-hz = /bits/ 64 <576000000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>; + }; + + cpu0_opp3: opp-768000000 { + opp-hz = /bits/ 64 <768000000>; + required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>; + }; + + cpu0_opp4: opp-1017600000 { + opp-hz = /bits/ 64 <1017600000>; + required-opps = <&cpu0_ddr_bw_opp2>, <&cpu0_l3_bw_opp2>; + }; + + cpu0_opp5: opp-1248000000 { + opp-hz = /bits/ 64 <1248000000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp3>; + }; + + cpu0_opp6: opp-1324800000 { + opp-hz = /bits/ 64 <1324800000>; + required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp3>; + }; + + cpu0_opp7: opp-1516800000 { + opp-hz = /bits/ 64 <1516800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp4>; + }; + + cpu0_opp8: opp-1612800000 { + opp-hz = /bits/ 64 <1612800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp4>; + }; + + cpu0_opp9: opp-1708800000 { + opp-hz = /bits/ 64 <1708800000>; + required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp4>; + }; + + cpu0_opp10: opp-1804800000 { + opp-hz = /bits/ 64 <1804800000>; + required-opps = <&cpu0_ddr_bw_opp5>, <&cpu0_l3_bw_opp5>; + }; + }; + + cpu6_opp_table: cpu6_opp_table { + compatible = "operating-points-v2"; + opp-shared; + + cpu6_opp1: opp-300000000 { + opp-hz = /bits/ 64 <300000000>; + required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>; + }; + + cpu6_opp2: opp-652800000 { + opp-hz = /bits/ 64 <652800000>; + required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>; + }; + + cpu6_opp3: opp-825600000 { + opp-hz = /bits/ 64 <825600000>; + required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>; + }; + + cpu6_opp4: opp-979200000 { + opp-hz = /bits/ 64 <979200000>; + required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>; + }; + + cpu6_opp5: opp-1113600000 { + opp-hz = /bits/ 64 <1113600000>; + required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>; + }; + + cpu6_opp6: opp-1267200000 { + opp-hz = /bits/ 64 <1267200000>; + required-opps = <&cpu6_ddr_bw_opp2>, <&cpu6_l3_bw_opp2>; + }; + + cpu6_opp7: opp-1555200000 { + opp-hz = /bits/ 64 <1555200000>; + required-opps = <&cpu6_ddr_bw_opp2>, <&cpu6_l3_bw_opp3>; + }; + + cpu6_opp8: opp-1708800000 { + opp-hz = /bits/ 64 <1708800000>; + required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp4>; + }; + + cpu6_opp9: opp-1843200000 { + opp-hz = /bits/ 64 <1843200000>; + required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp4>; + }; + + cpu6_opp10: opp-1900800000 { + opp-hz = /bits/ 64 <1900800000>; + required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp5>; + }; + + cpu6_opp11: opp-1996800000 { + opp-hz = /bits/ 64 <1996800000>; + required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp5>; + }; + + cpu6_opp12: opp-2112000000 { + opp-hz = /bits/ 64 <2112000000>; + required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp5>; + }; + + cpu6_opp13: opp-2208000000 { + opp-hz = /bits/ 64 <2208000000>; + required-opps = <&cpu6_ddr_bw_opp4>, <&cpu6_l3_bw_opp5>; + }; + + cpu6_opp14: opp-2323200000 { + opp-hz = /bits/ 64 <2323200000>; + required-opps = <&cpu6_ddr_bw_opp4>, <&cpu6_l3_bw_opp5>; + }; + + cpu6_opp15: opp-2400000000 { + opp-hz = /bits/ 64 <2400000000>; + required-opps = <&cpu6_ddr_bw_opp5>, <&cpu6_l3_bw_opp6>; + }; + }; + + cpu0_ddr_bw_opp_table: cpu0-ddr-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu0_ddr_bw_opp1: opp-300000000 { + opp-peak-kBps =/bits/ 32 <1200000>; + }; + + cpu0_ddr_bw_opp2: opp-451000000 { + opp-peak-kBps =/bits/ 32 <1804000>; + }; + + cpu0_ddr_bw_opp3: opp-547000000 { + opp-peak-kBps =/bits/ 32 <2188000>; + }; + + cpu0_ddr_bw_opp4: opp-768000000 { + opp-peak-kBps =/bits/ 32 <3072000>; + }; + + cpu0_ddr_bw_opp5: opp-1017000000 { + opp-peak-kBps =/bits/ 32 <4068000>; + }; + }; + + cpu0_l3_bw_opp_table: cpu0-l3-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu0_l3_bw_opp1: opp-300000000 { + opp-peak-kBps = /bits/ 32 <4800000>; + }; + + cpu0_l3_bw_opp2: opp-556800000 { + opp-peak-kBps = /bits/ 32 <8908800>; + }; + + cpu0_l3_bw_opp3: opp-806400000 { + opp-peak-kBps = /bits/ 32 <12902400>; + }; + + cpu0_l3_bw_opp4: opp-940800000 { + opp-peak-kBps = /bits/ 32 <15052800>; + }; + + cpu0_l3_bw_opp5: opp-1401000000 { + opp-peak-kBps = /bits/ 32 <22425600>; + }; + }; + + cpu6_ddr_bw_opp_table: cpu6-ddr-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu6_ddr_bw_opp1: opp-547000000 { + opp-peak-kBps =/bits/ 32 <2188000>; + }; + + cpu6_ddr_bw_opp2: opp-1017000000 { + opp-peak-kBps =/bits/ 32 <4068000>; + }; + + cpu6_ddr_bw_opp3: opp-1555000000 { + opp-peak-kBps =/bits/ 32 <6220000>; + }; + + cpu6_ddr_bw_opp4: opp-1804000000 { + opp-peak-kBps =/bits/ 32 <7216000>; + }; + + cpu6_ddr_bw_opp5: opp-2133000000 { + opp-peak-kBps =/bits/ 32 <8532000>; + }; + }; + + cpu6_l3_bw_opp_table: cpu6-l3-bw-opp-table { + compatible = "operating-points-v2"; + opp-shared; + + cpu6_l3_bw_opp1: opp-556800000 { + opp-peak-kBps = /bits/ 32 <8908800>; + }; + + cpu6_l3_bw_opp2: opp-806400000 { + opp-peak-kBps = /bits/ 32 <12902400>; + }; + + cpu6_l3_bw_opp3: opp-940800000 { + opp-peak-kBps = /bits/ 32 <15052800>; + }; + + cpu6_l3_bw_opp4: opp-1209600000 { + opp-peak-kBps = /bits/ 32 <19353600>; + }; + + cpu6_l3_bw_opp5: opp-1401000000 { + opp-peak-kBps = /bits/ 32 <22425600>; + }; + + cpu6_l3_bw_opp6: opp-1459000000 { + opp-peak-kBps = /bits/ 32 <23347200>; + }; + }; + memory@80000000 { device_type = "memory"; /* We expect the bootloader to fill in the size */ -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Sibi,
On Tue, Jan 28, 2020 at 01:33:41AM +0530, Sibi Sankar wrote:
> Add missing SoC compatible to SDM845 MTP board file.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 09ad37b0dd71d..54087847794aa 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -13,7 +13,7 @@
>
> / {
> model = "Qualcomm Technologies, Inc. SDM845 MTP";
> - compatible = "qcom,sdm845-mtp";
> + compatible = "qcom,sdm845-mtp", "qcom,sdm845";
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Since this is v3 already I think you can omit the RFC tag in the next
revision of the series.
On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote: > Add SDM845 to cpufreq-dt-platdev blacklist. nit: you could mention that cpufreq is handled by the 'qcom-cpufreq-hw' driver. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/cpufreq/cpufreq-dt-platdev.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c > index f2ae9cd455c17..5492cf3c9dc18 100644 > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > @@ -130,6 +130,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", }, Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
On Tue, Jan 28, 2020 at 01:33:43AM +0530, Sibi Sankar wrote: > Add SC7180 to cpufreq-dt-platdev blacklist. nit: you could mention that cpufreq is handled by the 'qcom-cpufreq-hw' driver. > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/cpufreq/cpufreq-dt-platdev.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c > index 5492cf3c9dc18..580abc777d9d8 100644 > --- a/drivers/cpufreq/cpufreq-dt-platdev.c > +++ b/drivers/cpufreq/cpufreq-dt-platdev.c > @@ -130,6 +130,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", }, Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Hi Sibi, On Tue, Jan 28, 2020 at 01:33:44AM +0530, Sibi Sankar wrote: > Add and export 'dev_pm_opp_update_voltage' to find and update voltage > of an opp for a given frequency. This will be useful to update the opps > with voltages read back from firmware. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/opp/core.c | 55 ++++++++++++++++++++++++++++++++++++++++++ > include/linux/pm_opp.h | 10 ++++++++ > 2 files changed, 65 insertions(+) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index 9aa2a44a5d638..f241e83ec926a 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -2503,6 +2503,61 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_disable); > > +/** > + * dev_pm_opp_update_voltage() - Find and update voltage The comment should mention that this is done for an OPP. Maybe omit the 'find' part here and just say 'Update the voltage of an OPP'? > + * @dev: device for which we do this operation > + * @freq: OPP frequency to update voltage > + * @u_volt: voltage requested for this opp > + * > + * Find and update voltage of a disabled opp corresponding to the given > + * frequency. This is useful only for devices with single power supply. > + * > + * Return: 0 if modification was successful or a negative error value. > + */ > +int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq, > + unsigned long u_volt) > +{ > + struct dev_pm_opp *opp = ERR_PTR(-ENODEV); initialization is not needed > + struct opp_table *opp_table; > + unsigned long tol; > + int ret = 0; > + > + /* Find the opp_table */ Drop the comment, it's obvious from the code. > + opp_table = _find_opp_table(dev); > + if (IS_ERR(opp_table)) { > + ret = PTR_ERR(opp_table); > + dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret); > + return PTR_ERR(opp_table); return ret; > + } > + > + opp = dev_pm_opp_find_freq_exact(dev, freq, false); > + if (IS_ERR(opp)) { > + ret = PTR_ERR(opp); > + goto put_table; > + } > + > + mutex_lock(&opp_table->lock); > + > + /* update only if the opp is disabled */ > + if (opp->available) { > + ret = -EBUSY; > + goto unlock; > + } > + > + tol = u_volt * opp_table->voltage_tolerance_v1 / 100; > + opp->supplies[0].u_volt_min = u_volt - tol; > + opp->supplies[0].u_volt = u_volt; > + opp->supplies[0].u_volt_min = u_volt + tol; .u_volt_max = I suppose the assignments need to be done for all possible supplies, i.e. 0 to (opp_table->regulator_count - 1).
Hi Sibi, On Tue, Jan 28, 2020 at 01:33:49AM +0530, Sibi Sankar wrote: > Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 SoCs. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 +++++++++++++++++++++++++++ > 1 file changed, 453 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index c036bab49fc03..8cb976118407b 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -199,6 +199,12 @@ > qcom,freq-domain = <&cpufreq_hw 0>; > #cooling-cells = <2>; > next-level-cache = <&L2_0>; > + operating-points-v2 = <&cpu0_opp_table>, > + <&cpu0_ddr_bw_opp_table>, > + <&cpu0_l3_bw_opp_table>; > + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>, > + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; This apparently depends on the 'Split SDM845 interconnect nodes and consolidate RPMh support' series (https://patchwork.kernel.org/project/linux-arm-msm/list/?series=226281), which isn't mentioned in the cover letter. I also couldn't find a patch on the lists that adds the 'osm_l3' interconnect node for SDM845. The same is true for SC7180 (next patch of this series). These patches may be available in custom trees, but that isn't really helpful for upstream review. I would suggest to focus on landing the dependencies of this series, before proceding with it (or at least most of them), there are plenty and without the dependencies this series isn't going to land, it also makes it hard for testers and reviewers to get all the pieces together. In particular the last post of the series 'Add required-opps support to devfreq passive gov' (https://patchwork.kernel.org/cover/11055499/) is from July 2019 ... Thanks Matthias
On Mon, Jan 27, 2020 at 12:05 PM Sibi Sankar <sibis@codeaurora.org> wrote: > > This RFC series aims to extend cpu based scaling support to L3/DDR on > SDM845 and SC7180 SoCs. > > Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev > Patches [5-7] - Hack in a way to add/remove multiple opp tables to > a single device. I am yet to fix the debugfs to > support multiple opp_tables per device but wanted to > send what was working upstream to get an idea if multiple > opp tables per device is a feature that will be useful > upstream. > Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845 > and SC7180 SoCs. > > 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 > > Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420 > > Some alternate ways of hosting the opp-tables: > https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 > https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 > Viresh didn't really like ^^ bindings and they dont really scale well. Just > including them here for completeness. > > Depends on the following series: > [1] https://patchwork.kernel.org/cover/11277199/ > [2] https://patchwork.kernel.org/cover/11055499/ > [3] https://patchwork.kernel.org/cover/11326381/ So drive-by question, from the perspective of someone who cares about a non-cpu device that frequently enjoys a lot of bandwidth (ie. the GPU).. any thoughts on how ddr scaling would/should work for workloads where the CPU is not particularly busy? BR, -R > > Sibi Sankar (10): > arm64: dts: qcom: sdm845: Add SoC compatible to MTP > cpufreq: blacklist SDM845 in cpufreq-dt-platdev > cpufreq: blacklist SC7180 in cpufreq-dt-platdev > OPP: Add and export helper to update voltage > opp: of: export _opp_of_get_opp_desc_node > opp: Allow multiple opp_tables to be mapped to a single device > opp: Remove multiple attached opp tables from a device > cpufreq: qcom: Update the bandwidth levels on frequency change > arm64: dts: qcom: sdm845: Add cpu OPP tables > arm64: dts: qcom: sc7180: Add cpu OPP tables > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++ > arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 ++++++++++++++++++++++++ > drivers/cpufreq/cpufreq-dt-platdev.c | 2 + > drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++-- > drivers/opp/core.c | 111 +++++- > drivers/opp/of.c | 3 +- > drivers/opp/opp.h | 2 + > include/linux/pm_opp.h | 10 + > 9 files changed, 1083 insertions(+), 33 deletions(-) > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project
Hi Sibi, On 1/27/20 8:03 PM, Sibi Sankar wrote: > Add support to parse and update OPP tables attached to the cpu device > when the required OPP bandwidth values are populated to enable scaling > of DDR/L3 bandwidth levels with frequency change. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++++++++++++++++++--- > 1 file changed, 225 insertions(+), 21 deletions(-) > > diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c > index fc92a8842e252..348eb2fdaccaf 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> > @@ -28,17 +29,194 @@ > #define REG_VOLT_LUT 0x114 > #define REG_PERF_STATE 0x920 > > +#define QCOM_ICC_TAG_ACTIVE_ONLY 1 > + > static unsigned long cpu_hw_rate, xo_rate; > static struct platform_device *global_pdev; > > +/* opp table indices */ > +enum { > + QCOM_CPU_OPP_TABLE_INDEX, > + QCOM_CPU_DDR_OPP_TABLE_INDEX, > + QCOM_CPU_L3_OPP_TABLE_INDEX, > +}; > + > +/* icc path indices */ > +enum { > + ICC_DDR_PATH, > + ICC_L3_PATH, > +}; > + > +struct qcom_cpufreq_hw_res { > + void __iomem *base; > + > + struct device *cpu_dev; > + > + /* ddr/l3 icc paths */ > + struct icc_path *path[2]; > + > + /* cpu/ddr/l3 opp tables */ > + struct opp_table *opp_table[3]; > +}; > + > +struct cpufreq_hw_icc_info { > + const char *icc_path_name; > + u8 table_index; > + u32 tag; > +}; > + > +static const struct cpufreq_hw_icc_info icc_info[] = { > + { > + .icc_path_name = "cpu-ddr", > + .table_index = QCOM_CPU_DDR_OPP_TABLE_INDEX, > + .tag = QCOM_ICC_TAG_ACTIVE_ONLY, > + }, > + { > + .icc_path_name = "cpu-l3", > + .table_index = QCOM_CPU_L3_OPP_TABLE_INDEX, > + .tag = 0, > + }, > +}; > + > +static int qcom_cpufreq_hw_add_opp_table(struct qcom_cpufreq_hw_res *res) > +{ > + struct opp_table **table = res->opp_table; > + struct device_node *opp_table_np, *np; > + struct device *dev = res->cpu_dev; > + int ret, i; > + u64 rate; > + > + for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) { > + ret = dev_pm_opp_of_add_table_indexed(dev, i); > + if (ret) { > + dev_dbg(dev, "Add OPP table failed index %d: %d\n", > + i, ret); > + goto err; > + } > + > + table[i] = dev_pm_opp_get_opp_table_indexed(dev, i); > + if (!table[i]) { > + dev_dbg(dev, "Get OPP table failed index %d\n", i); > + ret = -EINVAL; > + goto err; > + } > + } > + > + /* Disable all cpu opps and cross-validate against LUT */ > + opp_table_np = dev_pm_opp_of_get_opp_desc_node(dev); > + for_each_available_child_of_node(opp_table_np, np) { > + ret = of_property_read_u64(np, "opp-hz", &rate); > + if (ret) > + continue; > + > + dev_pm_opp_disable(dev, rate); > + } > + of_node_put(opp_table_np); > + > + return 0; > + > +err: > + for (; i >= 0; i--) { > + if (table[i]) { > + dev_pm_opp_put_opp_table(table[i]); > + table[i] = NULL; > + } > + } > + > + dev_pm_opp_remove_table(dev); > + return ret; > +} > + > +static int qcom_cpufreq_hw_icc_init(struct qcom_cpufreq_hw_res *res, u8 index) > +{ > + const struct cpufreq_hw_icc_info *info = &icc_info[index]; > + struct icc_path **path = res->path; > + struct device *dev = res->cpu_dev; > + > + path[index] = of_icc_get(dev, info->icc_path_name); > + if (IS_ERR(path[index])) { > + dev_dbg(dev, "ICC %s path get failed ret: %ld\n", > + info->icc_path_name, PTR_ERR(path[index])); > + return PTR_ERR(path[index]); > + } > + > + icc_set_tag(path[index], info->tag); > + > + return 0; > +} > + > +static void qcom_cpufreq_hw_icc_set(struct qcom_cpufreq_hw_res *res, > + unsigned long freq) > +{ > + struct opp_table **table = res->opp_table; > + const struct cpufreq_hw_icc_info *info; > + unsigned long freq_hz = freq * 1000; > + struct icc_path **path = res->path; > + struct device *dev = res->cpu_dev; > + struct dev_pm_opp *cpu_opp, *opp; > + struct opp_table *cpu_opp_table; > + unsigned long peak_bw, avg_bw; > + int ret; > + int i; > + > + cpu_opp_table = table[QCOM_CPU_OPP_TABLE_INDEX]; > + if (!cpu_opp_table) > + return; > + > + cpu_opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true); > + if (IS_ERR_OR_NULL(cpu_opp)) > + return; > + > + for (i = 0; i <= ICC_L3_PATH; i++) { > + if (IS_ERR(path[i])) { > + if (PTR_ERR(path[i]) != -EPROBE_DEFER) > + continue; > + > + ret = qcom_cpufreq_hw_icc_init(res, i); > + if (ret) > + continue; > + } > + > + info = &icc_info[i]; > + > + opp = dev_pm_opp_xlate_required_opp(cpu_opp_table, > + table[info->table_index], > + cpu_opp); > + if (IS_ERR_OR_NULL(opp)) > + continue; > + > + peak_bw = dev_pm_opp_get_bw(opp, &avg_bw); > + dev_pm_opp_put(opp); > + > + icc_set_bw(res->path[i], avg_bw, peak_bw); > + } > + dev_pm_opp_put(cpu_opp); > +} > + > +static int qcom_cpufreq_update_opp(struct device *dev, > + unsigned long freq_khz, > + unsigned long volt) > +{ > + unsigned long freq_hz = freq_khz * 1000; > + > + if (dev_pm_opp_update_voltage(dev, freq_hz, volt)) > + return dev_pm_opp_add(dev, freq_hz, volt); > + > + /* Enable the opp after voltage update*/ > + return dev_pm_opp_enable(dev, freq_hz); > +} > + > static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, > unsigned int index) > { > - void __iomem *perf_state_reg = policy->driver_data; > + struct qcom_cpufreq_hw_res *res = policy->driver_data; > + void __iomem *perf_state_reg = res->base + REG_PERF_STATE; > unsigned long freq = policy->freq_table[index].frequency; > > writel_relaxed(index, perf_state_reg); > > + qcom_cpufreq_hw_icc_set(res, freq); > + > arch_set_freq_scale(policy->related_cpus, freq, > policy->cpuinfo.max_freq); > return 0; > @@ -46,6 +224,7 @@ static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy, > > static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) > { > + struct qcom_cpufreq_hw_res *res; > void __iomem *perf_state_reg; > struct cpufreq_policy *policy; > unsigned int index; > @@ -54,7 +233,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) > if (!policy) > return 0; > > - perf_state_reg = policy->driver_data; > + res = policy->driver_data; > + perf_state_reg = res->base + REG_PERF_STATE; > > index = readl_relaxed(perf_state_reg); > index = min(index, LUT_MAX_ENTRIES - 1); > @@ -65,7 +245,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) > static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, > unsigned int target_freq) > { > - void __iomem *perf_state_reg = policy->driver_data; > + struct qcom_cpufreq_hw_res *res = policy->driver_data; > + void __iomem *perf_state_reg = res->base + REG_PERF_STATE; > int index; > unsigned long freq; > > @@ -82,18 +263,24 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, > return freq; > } > > -static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, > - struct cpufreq_policy *policy, > - void __iomem *base) > +static int qcom_cpufreq_hw_read_lut(struct cpufreq_policy *policy, > + struct qcom_cpufreq_hw_res *res) > { > u32 data, src, lval, i, core_count, prev_freq = 0, freq; > u32 volt; > struct cpufreq_frequency_table *table; > + struct device *cpu_dev = res->cpu_dev; > + void __iomem *base = res->base; > + int ret; > > table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL); > if (!table) > return -ENOMEM; > > + ret = qcom_cpufreq_hw_add_opp_table(res); > + if (ret) > + dev_dbg(cpu_dev, "Add OPP tables failed from dt\n"); > + > for (i = 0; i < LUT_MAX_ENTRIES; i++) { > data = readl_relaxed(base + REG_FREQ_LUT + > i * LUT_ROW_SIZE); > @@ -112,7 +299,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 +320,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; > @@ -175,11 +363,10 @@ static void qcom_get_related_cpus(int index, struct cpumask *m) > static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > { > struct device *dev = &global_pdev->dev; > + struct qcom_cpufreq_hw_res *res; > struct of_phandle_args args; > struct device_node *cpu_np; > struct device *cpu_dev; > - struct resource *res; > - void __iomem *base; > int ret, index; > > cpu_dev = get_cpu_device(policy->cpu); > @@ -201,16 +388,17 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > index = args.args[0]; > > - res = platform_get_resource(global_pdev, IORESOURCE_MEM, index); > + res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL); > if (!res) > - return -ENODEV; > - > - base = devm_ioremap(dev, res->start, resource_size(res)); > - if (!base) > return -ENOMEM; > > + res->cpu_dev = cpu_dev; > + res->base = devm_platform_ioremap_resource(global_pdev, index); > + if (IS_ERR(res->base)) > + return PTR_ERR(res->base); > + > /* HW should be in enabled state to proceed */ > - if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) { > + if (!(readl_relaxed(res->base + REG_ENABLE) & 0x1)) { > dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index); > ret = -ENODEV; > goto error; > @@ -223,9 +411,9 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > goto error; > } > > - policy->driver_data = base + REG_PERF_STATE; > + policy->driver_data = res; > > - ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy, base); > + ret = qcom_cpufreq_hw_read_lut(policy, res); > if (ret) { > dev_err(dev, "Domain-%d failed to read LUT\n", index); > goto error; > @@ -240,22 +428,38 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) > > dev_pm_opp_of_register_em(policy->cpus); > > - policy->fast_switch_possible = true; > + if (!res->opp_table[QCOM_CPU_OPP_TABLE_INDEX]) { > + policy->fast_switch_possible = true; > + } else { > + qcom_cpufreq_hw_icc_init(res, ICC_DDR_PATH); > + qcom_cpufreq_hw_icc_init(res, ICC_L3_PATH); > + } This patch tries to disable silently the fast_switch. The fast_switch is used by SchedUtil governor. Regards, Lukasz > > return 0; > error: > - devm_iounmap(dev, base); > + devm_iounmap(dev, res->base); > + devm_kfree(&global_pdev->dev, res); > return ret; > } > > static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy) > { > + struct qcom_cpufreq_hw_res *res = policy->driver_data; > struct device *cpu_dev = get_cpu_device(policy->cpu); > - void __iomem *base = policy->driver_data - REG_PERF_STATE; > + struct opp_table **table = res->opp_table; > + void __iomem *base = res->base; > + int i; > + > + for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) { > + if (table[i]) > + dev_pm_opp_put_opp_table(table[i]); > + } > > dev_pm_opp_remove_all_dynamic(cpu_dev); > kfree(policy->freq_table); > + dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); > devm_iounmap(&global_pdev->dev, base); > + devm_kfree(&global_pdev->dev, res); > > return 0; > } >
Hi Sibi,
In my opinion this solution depends on not always true assumption that
CPUFreq notification chain will be triggered when there is a frequency
switch. Extending devfreq governor (as in one of the dependent patch
series that you have referred) by attaching to this notification
chain makes sense only when the SchedUtil and fast_switch is not in use.
The Schedutil CPUFreq governor might use the fast_switch from this
driver and the notifications will not be triggered. I have also
commented patch 08/10 which tries to disable it.
Regards,
Lukasz
On 1/27/20 8:03 PM, Sibi Sankar wrote:
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
>
> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
> a single device. I am yet to fix the debugfs to
> support multiple opp_tables per device but wanted to
> send what was working upstream to get an idea if multiple
> opp tables per device is a feature that will be useful
> upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
> and SC7180 SoCs.
>
> 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
>
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. Just
> including them here for completeness.
>
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/
>
> Sibi Sankar (10):
> arm64: dts: qcom: sdm845: Add SoC compatible to MTP
> cpufreq: blacklist SDM845 in cpufreq-dt-platdev
> cpufreq: blacklist SC7180 in cpufreq-dt-platdev
> OPP: Add and export helper to update voltage
> opp: of: export _opp_of_get_opp_desc_node
> opp: Allow multiple opp_tables to be mapped to a single device
> opp: Remove multiple attached opp tables from a device
> cpufreq: qcom: Update the bandwidth levels on frequency change
> arm64: dts: qcom: sdm845: Add cpu OPP tables
> arm64: dts: qcom: sc7180: Add cpu OPP tables
>
> arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +-
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 ++++++++++++++++++++++++
> drivers/cpufreq/cpufreq-dt-platdev.c | 2 +
> drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++--
> drivers/opp/core.c | 111 +++++-
> drivers/opp/of.c | 3 +-
> drivers/opp/opp.h | 2 +
> include/linux/pm_opp.h | 10 +
> 9 files changed, 1083 insertions(+), 33 deletions(-)
>
Hey Matthias,
Thanks for the review!
On 2020-01-29 02:10, Matthias Kaehlcke wrote:
> Hi Sibi,
>
> On Tue, Jan 28, 2020 at 01:33:41AM +0530, Sibi Sankar wrote:
>> Add missing SoC compatible to SDM845 MTP board file.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> index 09ad37b0dd71d..54087847794aa 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -13,7 +13,7 @@
>>
>> / {
>> model = "Qualcomm Technologies, Inc. SDM845 MTP";
>> - compatible = "qcom,sdm845-mtp";
>> + compatible = "qcom,sdm845-mtp", "qcom,sdm845";
>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
>
> Since this is v3 already I think you can omit the RFC tag in the next
> revision of the series.
Yeah would like to drop the RFC
but every version seems to have
a feature which we havn't reached
a consensus on.
v1: CPUfreq passive governor
v2: New bandwidth bindings
v3: Multiple opp tables per device
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
Thanks for the review! On 2020-01-29 02:14, Matthias Kaehlcke wrote: > On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote: >> Add SDM845 to cpufreq-dt-platdev blacklist. > > nit: you could mention that cpufreq is handled by the > 'qcom-cpufreq-hw' driver. will add it in the next re-spin. > >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> drivers/cpufreq/cpufreq-dt-platdev.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c >> b/drivers/cpufreq/cpufreq-dt-platdev.c >> index f2ae9cd455c17..5492cf3c9dc18 100644 >> --- a/drivers/cpufreq/cpufreq-dt-platdev.c >> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c >> @@ -130,6 +130,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", }, > > Reviewed-by: Matthias Kaehlcke <mka@chromium.org> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On 2020-01-29 03:03, Matthias Kaehlcke wrote: > Hi Sibi, > > On Tue, Jan 28, 2020 at 01:33:44AM +0530, Sibi Sankar wrote: >> Add and export 'dev_pm_opp_update_voltage' to find and update voltage >> of an opp for a given frequency. This will be useful to update the >> opps >> with voltages read back from firmware. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> drivers/opp/core.c | 55 >> ++++++++++++++++++++++++++++++++++++++++++ >> include/linux/pm_opp.h | 10 ++++++++ >> 2 files changed, 65 insertions(+) >> >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c >> index 9aa2a44a5d638..f241e83ec926a 100644 >> --- a/drivers/opp/core.c >> +++ b/drivers/opp/core.c >> @@ -2503,6 +2503,61 @@ int dev_pm_opp_disable(struct device *dev, >> unsigned long freq) >> } >> EXPORT_SYMBOL_GPL(dev_pm_opp_disable); >> >> +/** >> + * dev_pm_opp_update_voltage() - Find and update voltage > > The comment should mention that this is done for an OPP. > > Maybe omit the 'find' part here and just say 'Update the voltage of > an OPP'? sure makes sense > >> + * @dev: device for which we do this operation >> + * @freq: OPP frequency to update voltage >> + * @u_volt: voltage requested for this opp >> + * >> + * Find and update voltage of a disabled opp corresponding to the >> given >> + * frequency. This is useful only for devices with single power >> supply. >> + * >> + * Return: 0 if modification was successful or a negative error >> value. >> + */ >> +int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq, >> + unsigned long u_volt) >> +{ >> + struct dev_pm_opp *opp = ERR_PTR(-ENODEV); > > initialization is not needed > >> + struct opp_table *opp_table; >> + unsigned long tol; >> + int ret = 0; >> + >> + /* Find the opp_table */ > > Drop the comment, it's obvious from the code. > >> + opp_table = _find_opp_table(dev); >> + if (IS_ERR(opp_table)) { >> + ret = PTR_ERR(opp_table); >> + dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret); >> + return PTR_ERR(opp_table); > > return ret; missed that :( > >> + } >> + >> + opp = dev_pm_opp_find_freq_exact(dev, freq, false); >> + if (IS_ERR(opp)) { >> + ret = PTR_ERR(opp); >> + goto put_table; >> + } >> + >> + mutex_lock(&opp_table->lock); >> + >> + /* update only if the opp is disabled */ >> + if (opp->available) { >> + ret = -EBUSY; >> + goto unlock; >> + } >> + >> + tol = u_volt * opp_table->voltage_tolerance_v1 / 100; >> + opp->supplies[0].u_volt_min = u_volt - tol; >> + opp->supplies[0].u_volt = u_volt; >> + opp->supplies[0].u_volt_min = u_volt + tol; > > .u_volt_max = > > I suppose the assignments need to be done for all possible supplies, > i.e. 0 to (opp_table->regulator_count - 1). a single value for all possible supplies seems wrong. Anyway will wait to see what Viresh thinks about it. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Hey Matthias, Thanks for the review! On 2020-01-29 06:54, Matthias Kaehlcke wrote: > Hi Sibi, > > On Tue, Jan 28, 2020 at 01:33:49AM +0530, Sibi Sankar wrote: >> Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 >> SoCs. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 >> +++++++++++++++++++++++++++ >> 1 file changed, 453 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi >> b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> index c036bab49fc03..8cb976118407b 100644 >> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi >> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi >> @@ -199,6 +199,12 @@ >> qcom,freq-domain = <&cpufreq_hw 0>; >> #cooling-cells = <2>; >> next-level-cache = <&L2_0>; >> + operating-points-v2 = <&cpu0_opp_table>, >> + <&cpu0_ddr_bw_opp_table>, >> + <&cpu0_l3_bw_opp_table>; >> + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc >> SLAVE_EBI1>, >> + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; > > This apparently depends on the 'Split SDM845 interconnect nodes and > consolidate RPMh support' series > (https://patchwork.kernel.org/project/linux-arm-msm/list/?series=226281), > which isn't mentioned in the cover letter. > > I also couldn't find a patch on the lists that adds the 'osm_l3' > interconnect node for SDM845. The same is true for SC7180 (next > patch of this series). These patches may be available in custom trees, > but that isn't really helpful for upstream review. yeah I missed adding the interconnect refactor dependency and the nodes. > > I would suggest to focus on landing the dependencies of this series, > before proceding with it (or at least most of them), there are plenty > and without the dependencies this series isn't going to land, it also > makes it hard for testers and reviewers to get all the pieces yes I understand but wanted the series out asap because since there are a few points where we still havn't reached a consensus on. > together. In particular the last post of the series 'Add > required-opps support to devfreq passive gov' > (https://patchwork.kernel.org/cover/11055499/) is from July 2019 ... https://lore.kernel.org/lkml/CAGTfZH37ALwUHd8SpRRrBzZ6x1-++YtzS60_yRQvN-TN6rOzaA@mail.gmail.com/ The pending patch for lazy linking was posted a while back. Now that it has a tested-by, majority of the series should go in since the devfreq maintainers wanted the series pulled in. > > Thanks > > Matthias -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Hey Rob, On 2020-01-29 08:24, Rob Clark wrote: > On Mon, Jan 27, 2020 at 12:05 PM Sibi Sankar <sibis@codeaurora.org> > wrote: >> >> This RFC series aims to extend cpu based scaling support to L3/DDR on >> SDM845 and SC7180 SoCs. >> >> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev >> Patches [5-7] - Hack in a way to add/remove multiple opp tables to >> a single device. I am yet to fix the debugfs to >> support multiple opp_tables per device but wanted to >> send what was working upstream to get an idea if >> multiple >> opp tables per device is a feature that will be useful >> upstream. >> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845 >> and SC7180 SoCs. >> >> 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 >> >> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420 >> >> Some alternate ways of hosting the opp-tables: >> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 >> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 >> Viresh didn't really like ^^ bindings and they dont really scale well. >> Just >> including them here for completeness. >> >> Depends on the following series: >> [1] https://patchwork.kernel.org/cover/11277199/ >> [2] https://patchwork.kernel.org/cover/11055499/ >> [3] https://patchwork.kernel.org/cover/11326381/ > > So drive-by question, from the perspective of someone who cares about > a non-cpu device that frequently enjoys a lot of bandwidth (ie. the > GPU).. any thoughts on how ddr scaling would/should work for workloads > where the CPU is not particularly busy? I guess Jordan would have more info about ^^. From what I can see from the CAF trees there is a similar device freq -> DDR Freq mapping that you can achieve with required-opps bindings and a custom governor ;) > > BR, > -R > >> >> Sibi Sankar (10): >> arm64: dts: qcom: sdm845: Add SoC compatible to MTP >> cpufreq: blacklist SDM845 in cpufreq-dt-platdev >> cpufreq: blacklist SC7180 in cpufreq-dt-platdev >> OPP: Add and export helper to update voltage >> opp: of: export _opp_of_get_opp_desc_node >> opp: Allow multiple opp_tables to be mapped to a single device >> opp: Remove multiple attached opp tables from a device >> cpufreq: qcom: Update the bandwidth levels on frequency change >> arm64: dts: qcom: sdm845: Add cpu OPP tables >> arm64: dts: qcom: sc7180: Add cpu OPP tables >> >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++ >> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 >> ++++++++++++++++++++++++ >> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + >> drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++-- >> drivers/opp/core.c | 111 +++++- >> drivers/opp/of.c | 3 +- >> drivers/opp/opp.h | 2 + >> include/linux/pm_opp.h | 10 + >> 9 files changed, 1083 insertions(+), 33 deletions(-) >> >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Hey Lukasz, Thanks for taking time to review the series! On 2020-01-29 15:05, Lukasz Luba wrote: > Hi Sibi, > > On 1/27/20 8:03 PM, Sibi Sankar wrote: >> Add support to parse and update OPP tables attached to the cpu device >> when the required OPP bandwidth values are populated to enable scaling >> of DDR/L3 bandwidth levels with frequency change. >> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> drivers/cpufreq/qcom-cpufreq-hw.c | 246 >> +++++++++++++++++++++++++++--- >> 1 file changed, 225 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c >> b/drivers/cpufreq/qcom-cpufreq-hw.c >> index fc92a8842e252..348eb2fdaccaf 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> >> @@ -28,17 +29,194 @@ >> #define REG_VOLT_LUT 0x114 >> #define REG_PERF_STATE 0x920 >> +#define QCOM_ICC_TAG_ACTIVE_ONLY 1 >> + >> static unsigned long cpu_hw_rate, xo_rate; >> static struct platform_device *global_pdev; >> +/* opp table indices */ >> +enum { >> + QCOM_CPU_OPP_TABLE_INDEX, >> + QCOM_CPU_DDR_OPP_TABLE_INDEX, >> + QCOM_CPU_L3_OPP_TABLE_INDEX, >> +}; >> + >> +/* icc path indices */ >> +enum { >> + ICC_DDR_PATH, >> + ICC_L3_PATH, >> +}; >> + >> +struct qcom_cpufreq_hw_res { >> + void __iomem *base; >> + >> + struct device *cpu_dev; >> + >> + /* ddr/l3 icc paths */ >> + struct icc_path *path[2]; >> + >> + /* cpu/ddr/l3 opp tables */ >> + struct opp_table *opp_table[3]; >> +}; >> + >> +struct cpufreq_hw_icc_info { >> + const char *icc_path_name; >> + u8 table_index; >> + u32 tag; >> +}; >> + >> +static const struct cpufreq_hw_icc_info icc_info[] = { >> + { >> + .icc_path_name = "cpu-ddr", >> + .table_index = QCOM_CPU_DDR_OPP_TABLE_INDEX, >> + .tag = QCOM_ICC_TAG_ACTIVE_ONLY, >> + }, >> + { >> + .icc_path_name = "cpu-l3", >> + .table_index = QCOM_CPU_L3_OPP_TABLE_INDEX, >> + .tag = 0, >> + }, >> +}; >> + >> +static int qcom_cpufreq_hw_add_opp_table(struct qcom_cpufreq_hw_res >> *res) >> +{ >> + struct opp_table **table = res->opp_table; >> + struct device_node *opp_table_np, *np; >> + struct device *dev = res->cpu_dev; >> + int ret, i; >> + u64 rate; >> + >> + for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) { >> + ret = dev_pm_opp_of_add_table_indexed(dev, i); >> + if (ret) { >> + dev_dbg(dev, "Add OPP table failed index %d: %d\n", >> + i, ret); >> + goto err; >> + } >> + >> + table[i] = dev_pm_opp_get_opp_table_indexed(dev, i); >> + if (!table[i]) { >> + dev_dbg(dev, "Get OPP table failed index %d\n", i); >> + ret = -EINVAL; >> + goto err; >> + } >> + } >> + >> + /* Disable all cpu opps and cross-validate against LUT */ >> + opp_table_np = dev_pm_opp_of_get_opp_desc_node(dev); >> + for_each_available_child_of_node(opp_table_np, np) { >> + ret = of_property_read_u64(np, "opp-hz", &rate); >> + if (ret) >> + continue; >> + >> + dev_pm_opp_disable(dev, rate); >> + } >> + of_node_put(opp_table_np); >> + >> + return 0; >> + >> +err: >> + for (; i >= 0; i--) { >> + if (table[i]) { >> + dev_pm_opp_put_opp_table(table[i]); >> + table[i] = NULL; >> + } >> + } >> + >> + dev_pm_opp_remove_table(dev); >> + return ret; >> +} >> + >> +static int qcom_cpufreq_hw_icc_init(struct qcom_cpufreq_hw_res *res, >> u8 index) >> +{ >> + const struct cpufreq_hw_icc_info *info = &icc_info[index]; >> + struct icc_path **path = res->path; >> + struct device *dev = res->cpu_dev; >> + >> + path[index] = of_icc_get(dev, info->icc_path_name); >> + if (IS_ERR(path[index])) { >> + dev_dbg(dev, "ICC %s path get failed ret: %ld\n", >> + info->icc_path_name, PTR_ERR(path[index])); >> + return PTR_ERR(path[index]); >> + } >> + >> + icc_set_tag(path[index], info->tag); >> + >> + return 0; >> +} >> + >> +static void qcom_cpufreq_hw_icc_set(struct qcom_cpufreq_hw_res *res, >> + unsigned long freq) >> +{ >> + struct opp_table **table = res->opp_table; >> + const struct cpufreq_hw_icc_info *info; >> + unsigned long freq_hz = freq * 1000; >> + struct icc_path **path = res->path; >> + struct device *dev = res->cpu_dev; >> + struct dev_pm_opp *cpu_opp, *opp; >> + struct opp_table *cpu_opp_table; >> + unsigned long peak_bw, avg_bw; >> + int ret; >> + int i; >> + >> + cpu_opp_table = table[QCOM_CPU_OPP_TABLE_INDEX]; >> + if (!cpu_opp_table) >> + return; >> + >> + cpu_opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true); >> + if (IS_ERR_OR_NULL(cpu_opp)) >> + return; >> + >> + for (i = 0; i <= ICC_L3_PATH; i++) { >> + if (IS_ERR(path[i])) { >> + if (PTR_ERR(path[i]) != -EPROBE_DEFER) >> + continue; >> + >> + ret = qcom_cpufreq_hw_icc_init(res, i); >> + if (ret) >> + continue; >> + } >> + >> + info = &icc_info[i]; >> + >> + opp = dev_pm_opp_xlate_required_opp(cpu_opp_table, >> + table[info->table_index], >> + cpu_opp); >> + if (IS_ERR_OR_NULL(opp)) >> + continue; >> + >> + peak_bw = dev_pm_opp_get_bw(opp, &avg_bw); >> + dev_pm_opp_put(opp); >> + >> + icc_set_bw(res->path[i], avg_bw, peak_bw); >> + } >> + dev_pm_opp_put(cpu_opp); >> +} >> + >> +static int qcom_cpufreq_update_opp(struct device *dev, >> + unsigned long freq_khz, >> + unsigned long volt) >> +{ >> + unsigned long freq_hz = freq_khz * 1000; >> + >> + if (dev_pm_opp_update_voltage(dev, freq_hz, volt)) >> + return dev_pm_opp_add(dev, freq_hz, volt); >> + >> + /* Enable the opp after voltage update*/ >> + return dev_pm_opp_enable(dev, freq_hz); >> +} >> + >> static int qcom_cpufreq_hw_target_index(struct cpufreq_policy >> *policy, >> unsigned int index) >> { >> - void __iomem *perf_state_reg = policy->driver_data; >> + struct qcom_cpufreq_hw_res *res = policy->driver_data; >> + void __iomem *perf_state_reg = res->base + REG_PERF_STATE; >> unsigned long freq = policy->freq_table[index].frequency; >> writel_relaxed(index, perf_state_reg); >> + qcom_cpufreq_hw_icc_set(res, freq); >> + >> arch_set_freq_scale(policy->related_cpus, freq, >> policy->cpuinfo.max_freq); >> return 0; >> @@ -46,6 +224,7 @@ static int qcom_cpufreq_hw_target_index(struct >> cpufreq_policy *policy, >> static unsigned int qcom_cpufreq_hw_get(unsigned int cpu) >> { >> + struct qcom_cpufreq_hw_res *res; >> void __iomem *perf_state_reg; >> struct cpufreq_policy *policy; >> unsigned int index; >> @@ -54,7 +233,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned >> int cpu) >> if (!policy) >> return 0; >> - perf_state_reg = policy->driver_data; >> + res = policy->driver_data; >> + perf_state_reg = res->base + REG_PERF_STATE; >> index = readl_relaxed(perf_state_reg); >> index = min(index, LUT_MAX_ENTRIES - 1); >> @@ -65,7 +245,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned >> int cpu) >> static unsigned int qcom_cpufreq_hw_fast_switch(struct >> cpufreq_policy *policy, >> unsigned int target_freq) >> { >> - void __iomem *perf_state_reg = policy->driver_data; >> + struct qcom_cpufreq_hw_res *res = policy->driver_data; >> + void __iomem *perf_state_reg = res->base + REG_PERF_STATE; >> int index; >> unsigned long freq; >> @@ -82,18 +263,24 @@ static unsigned int >> qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy, >> return freq; >> } >> -static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev, >> - struct cpufreq_policy *policy, >> - void __iomem *base) >> +static int qcom_cpufreq_hw_read_lut(struct cpufreq_policy *policy, >> + struct qcom_cpufreq_hw_res *res) >> { >> u32 data, src, lval, i, core_count, prev_freq = 0, freq; >> u32 volt; >> struct cpufreq_frequency_table *table; >> + struct device *cpu_dev = res->cpu_dev; >> + void __iomem *base = res->base; >> + int ret; >> table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL); >> if (!table) >> return -ENOMEM; >> + ret = qcom_cpufreq_hw_add_opp_table(res); >> + if (ret) >> + dev_dbg(cpu_dev, "Add OPP tables failed from dt\n"); >> + >> for (i = 0; i < LUT_MAX_ENTRIES; i++) { >> data = readl_relaxed(base + REG_FREQ_LUT + >> i * LUT_ROW_SIZE); >> @@ -112,7 +299,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 +320,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; >> @@ -175,11 +363,10 @@ static void qcom_get_related_cpus(int index, >> struct cpumask *m) >> static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy) >> { >> struct device *dev = &global_pdev->dev; >> + struct qcom_cpufreq_hw_res *res; >> struct of_phandle_args args; >> struct device_node *cpu_np; >> struct device *cpu_dev; >> - struct resource *res; >> - void __iomem *base; >> int ret, index; >> cpu_dev = get_cpu_device(policy->cpu); >> @@ -201,16 +388,17 @@ static int qcom_cpufreq_hw_cpu_init(struct >> cpufreq_policy *policy) >> index = args.args[0]; >> - res = platform_get_resource(global_pdev, IORESOURCE_MEM, index); >> + res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL); >> if (!res) >> - return -ENODEV; >> - >> - base = devm_ioremap(dev, res->start, resource_size(res)); >> - if (!base) >> return -ENOMEM; >> + res->cpu_dev = cpu_dev; >> + res->base = devm_platform_ioremap_resource(global_pdev, index); >> + if (IS_ERR(res->base)) >> + return PTR_ERR(res->base); >> + >> /* HW should be in enabled state to proceed */ >> - if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) { >> + if (!(readl_relaxed(res->base + REG_ENABLE) & 0x1)) { >> dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index); >> ret = -ENODEV; >> goto error; >> @@ -223,9 +411,9 @@ static int qcom_cpufreq_hw_cpu_init(struct >> cpufreq_policy *policy) >> goto error; >> } >> - policy->driver_data = base + REG_PERF_STATE; >> + policy->driver_data = res; >> - ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy, base); >> + ret = qcom_cpufreq_hw_read_lut(policy, res); >> if (ret) { >> dev_err(dev, "Domain-%d failed to read LUT\n", index); >> goto error; >> @@ -240,22 +428,38 @@ static int qcom_cpufreq_hw_cpu_init(struct >> cpufreq_policy *policy) >> dev_pm_opp_of_register_em(policy->cpus); >> - policy->fast_switch_possible = true; >> + if (!res->opp_table[QCOM_CPU_OPP_TABLE_INDEX]) { >> + policy->fast_switch_possible = true; >> + } else { >> + qcom_cpufreq_hw_icc_init(res, ICC_DDR_PATH); >> + qcom_cpufreq_hw_icc_init(res, ICC_L3_PATH); >> + } > > This patch tries to disable silently the fast_switch. The fast_switch > is used by SchedUtil governor. I agree that the series would benefit from splitting this patch into more chunks. SchedUtil will still be used as the governor but will operate in the slow path. The main reason is that the icc bandwidth votes through rpmh on SDM845/SC7180 cant be done in the fast path. > > Regards, > Lukasz > >> return 0; >> error: >> - devm_iounmap(dev, base); >> + devm_iounmap(dev, res->base); >> + devm_kfree(&global_pdev->dev, res); >> return ret; >> } >> static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy) >> { >> + struct qcom_cpufreq_hw_res *res = policy->driver_data; >> struct device *cpu_dev = get_cpu_device(policy->cpu); >> - void __iomem *base = policy->driver_data - REG_PERF_STATE; >> + struct opp_table **table = res->opp_table; >> + void __iomem *base = res->base; >> + int i; >> + >> + for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) { >> + if (table[i]) >> + dev_pm_opp_put_opp_table(table[i]); >> + } >> dev_pm_opp_remove_all_dynamic(cpu_dev); >> kfree(policy->freq_table); >> + dev_pm_opp_of_cpumask_remove_table(policy->related_cpus); >> devm_iounmap(&global_pdev->dev, base); >> + devm_kfree(&global_pdev->dev, res); >> return 0; >> } >> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Hey Lukasz, Thanks for taking time to review the series! On 2020-01-29 15:16, Lukasz Luba wrote: > Hi Sibi, > > In my opinion this solution depends on not always true assumption that > CPUFreq notification chain will be triggered when there is a frequency This series does not create any devfreq devices nor use the cpufreq notification chain. It just relies on the opening up of required-opps from being exclusive to gen-pd using patches 1,2,3 from patch series[1]. With the fast path disabled and schedutil enabled, this series will not miss any cpufreq changes. [1] https://patchwork.kernel.org/cover/11055499/ > switch. Extending devfreq governor (as in one of the dependent patch > series that you have referred) by attaching to this notification > chain makes sense only when the SchedUtil and fast_switch is not in > use. fast_switch and cpu notifier chains are mutually exclusive but schedutil will still operate in the slow path IIRC. > The Schedutil CPUFreq governor might use the fast_switch from this > driver and the notifications will not be triggered. I have also > commented patch 08/10 which tries to disable it. > > Regards, > Lukasz > > On 1/27/20 8:03 PM, Sibi Sankar wrote: >> This RFC series aims to extend cpu based scaling support to L3/DDR on >> SDM845 and SC7180 SoCs. >> >> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev >> Patches [5-7] - Hack in a way to add/remove multiple opp tables to >> a single device. I am yet to fix the debugfs to >> support multiple opp_tables per device but wanted to >> send what was working upstream to get an idea if multiple >> opp tables per device is a feature that will be useful >> upstream. >> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845 >> and SC7180 SoCs. >> >> 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 >> >> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420 >> >> Some alternate ways of hosting the opp-tables: >> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 >> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 >> Viresh didn't really like ^^ bindings and they dont really scale well. >> Just >> including them here for completeness. >> >> Depends on the following series: >> [1] https://patchwork.kernel.org/cover/11277199/ >> [2] https://patchwork.kernel.org/cover/11055499/ >> [3] https://patchwork.kernel.org/cover/11326381/ >> >> Sibi Sankar (10): >> arm64: dts: qcom: sdm845: Add SoC compatible to MTP >> cpufreq: blacklist SDM845 in cpufreq-dt-platdev >> cpufreq: blacklist SC7180 in cpufreq-dt-platdev >> OPP: Add and export helper to update voltage >> opp: of: export _opp_of_get_opp_desc_node >> opp: Allow multiple opp_tables to be mapped to a single device >> opp: Remove multiple attached opp tables from a device >> cpufreq: qcom: Update the bandwidth levels on frequency change >> arm64: dts: qcom: sdm845: Add cpu OPP tables >> arm64: dts: qcom: sc7180: Add cpu OPP tables >> >> arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++ >> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- >> arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 >> ++++++++++++++++++++++++ >> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + >> drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++-- >> drivers/opp/core.c | 111 +++++- >> drivers/opp/of.c | 3 +- >> drivers/opp/opp.h | 2 + >> include/linux/pm_opp.h | 10 + >> 9 files changed, 1083 insertions(+), 33 deletions(-) >> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On 1/29/20 2:37 PM, Sibi Sankar wrote: > Hey Lukasz, > Thanks for taking time to review > the series! > > On 2020-01-29 15:16, Lukasz Luba wrote: >> Hi Sibi, >> >> In my opinion this solution depends on not always true assumption that >> CPUFreq notification chain will be triggered when there is a frequency > > This series does not create any > devfreq devices nor use the cpufreq > notification chain. It just relies > on the opening up of required-opps > from being exclusive to gen-pd using > patches 1,2,3 from patch series[1]. > With the fast path disabled and > schedutil enabled, this series will > not miss any cpufreq changes. Thank you Sibi for clarifying this. I spotted in patch 08/10 that the fast_switch is removed and the normal path is in use, which also triggers the notification chain in CPUFreq. Then maybe other machinery which you have put in your 'depends on' list accidentally works thanks to this. > > [1] https://patchwork.kernel.org/cover/11055499/ > >> switch. Extending devfreq governor (as in one of the dependent patch >> series that you have referred) by attaching to this notification >> chain makes sense only when the SchedUtil and fast_switch is not in use. > > fast_switch and cpu notifier chains > are mutually exclusive but schedutil > will still operate in the slow path > IIRC. True, SchedUtil would work in slow_path. The driver and SoC support 'fast_switch', your solution when is properly initialized, disables it. I would suggest to put this information in the commit message. I don't know the side effects on the performance, though. The other side effect would be: the CPUFreq notifications will be triggered. The fast_switch is now the preferred way, any new ideas should consider also this path. > >> The Schedutil CPUFreq governor might use the fast_switch from this >> driver and the notifications will not be triggered. I have also >> commented patch 08/10 which tries to disable it. >> >> Regards, >> Lukasz >> >> On 1/27/20 8:03 PM, Sibi Sankar wrote: >>> This RFC series aims to extend cpu based scaling support to L3/DDR on >>> SDM845 and SC7180 SoCs. >>> >>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev >>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to >>> a single device. I am yet to fix the debugfs to >>> support multiple opp_tables per device but wanted to >>> send what was working upstream to get an idea if multiple >>> opp tables per device is a feature that will be useful >>> upstream. >>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845 >>> and SC7180 SoCs. >>> >>> 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 >>> >>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420 >>> >>> Some alternate ways of hosting the opp-tables: >>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 >>> >>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 >>> >>> Viresh didn't really like ^^ bindings and they dont really scale >>> well. Just >>> including them here for completeness. >>> >>> Depends on the following series: >>> [1] https://patchwork.kernel.org/cover/11277199/ >>> [2] https://patchwork.kernel.org/cover/11055499/ >>> [3] https://patchwork.kernel.org/cover/11326381/ >>> >>> Sibi Sankar (10): >>> arm64: dts: qcom: sdm845: Add SoC compatible to MTP >>> cpufreq: blacklist SDM845 in cpufreq-dt-platdev >>> cpufreq: blacklist SC7180 in cpufreq-dt-platdev >>> OPP: Add and export helper to update voltage >>> opp: of: export _opp_of_get_opp_desc_node >>> opp: Allow multiple opp_tables to be mapped to a single device >>> opp: Remove multiple attached opp tables from a device >>> cpufreq: qcom: Update the bandwidth levels on frequency change >>> arm64: dts: qcom: sdm845: Add cpu OPP tables >>> arm64: dts: qcom: sc7180: Add cpu OPP tables >>> >>> arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++ >>> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- >>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 ++++++++++++++++++++++++ >>> drivers/cpufreq/cpufreq-dt-platdev.c | 2 + >>> drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++-- >>> drivers/opp/core.c | 111 +++++- >>> drivers/opp/of.c | 3 +- >>> drivers/opp/opp.h | 2 + >>> include/linux/pm_opp.h | 10 + >>> 9 files changed, 1083 insertions(+), 33 deletions(-) >>> >
Hi Sibi, On Wed, Jan 29, 2020 at 07:35:21PM +0530, Sibi Sankar wrote: > Hey Matthias, > > Thanks for the review! > > On 2020-01-29 06:54, Matthias Kaehlcke wrote: > > Hi Sibi, > > > > On Tue, Jan 28, 2020 at 01:33:49AM +0530, Sibi Sankar wrote: > > > Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 > > > SoCs. > > > > > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > > --- > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 > > > +++++++++++++++++++++++++++ > > > 1 file changed, 453 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > index c036bab49fc03..8cb976118407b 100644 > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > @@ -199,6 +199,12 @@ > > > qcom,freq-domain = <&cpufreq_hw 0>; > > > #cooling-cells = <2>; > > > next-level-cache = <&L2_0>; > > > + operating-points-v2 = <&cpu0_opp_table>, > > > + <&cpu0_ddr_bw_opp_table>, > > > + <&cpu0_l3_bw_opp_table>; > > > + interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc > > > SLAVE_EBI1>, > > > + <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; > > > > This apparently depends on the 'Split SDM845 interconnect nodes and > > consolidate RPMh support' series > > (https://patchwork.kernel.org/project/linux-arm-msm/list/?series=226281), > > which isn't mentioned in the cover letter. > > > > I also couldn't find a patch on the lists that adds the 'osm_l3' > > interconnect node for SDM845. The same is true for SC7180 (next > > patch of this series). These patches may be available in custom trees, > > but that isn't really helpful for upstream review. > > yeah I missed adding the interconnect > refactor dependency and the nodes. > > > > > I would suggest to focus on landing the dependencies of this series, > > before proceding with it (or at least most of them), there are plenty > > and without the dependencies this series isn't going to land, it also > > makes it hard for testers and reviewers to get all the pieces > > yes I understand but wanted the series > out asap because since there are a few > points where we still havn't reached > a consensus on. Ok, I just wanted to make sure we are not burning the limited maintainer/reviewer bandwidth on code with hard dependencies on things that aren't moving forward. > > together. In particular the last post of the series 'Add > > required-opps support to devfreq passive gov' > > (https://patchwork.kernel.org/cover/11055499/) is from July 2019 ... > > https://lore.kernel.org/lkml/CAGTfZH37ALwUHd8SpRRrBzZ6x1-++YtzS60_yRQvN-TN6rOzaA@mail.gmail.com/ > > The pending patch for lazy linking > was posted a while back. Now that > it has a tested-by, majority of the > series should go in since the devfreq > maintainers wanted the series pulled > in. Thanks for the clarification. For reference the post is https://patchwork.kernel.org/patch/11048277/#23020727 It sems the series will require at least another re-spin: "So once that's (lazy linking) added, I should be able to drop a few patches in this series, do some minor updates and then this will be good to go." https://patchwork.kernel.org/cover/11055499/#23001445 So it looks like we are waiting for the lazy linking patch to land in the PM/Linus' tree and then a re-spin of the 'Add required-opps support to devfreq passive gov' series.
On Tue, Jan 28, 2020 at 12:44:23PM -0800, Matthias Kaehlcke wrote:
> On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote:
> > Add SDM845 to cpufreq-dt-platdev blacklist.
>
> nit: you could mention that cpufreq is handled by the
> 'qcom-cpufreq-hw' driver.
>
IIUC, these platforms get the OPP table from the firmware and there shouldn't
be OPP entries in the DT. If not, why not fix that to avoid more confusion.
Can we make cpu0_node_has_opp_v2_prop return false in short.
--
Regards,
Sudeep
Hey Sudeep, Thanks for the review! On 2020-01-30 17:10, Sudeep Holla wrote: > On Tue, Jan 28, 2020 at 12:44:23PM -0800, Matthias Kaehlcke wrote: >> On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote: >> > Add SDM845 to cpufreq-dt-platdev blacklist. >> >> nit: you could mention that cpufreq is handled by the >> 'qcom-cpufreq-hw' driver. >> > > IIUC, these platforms get the OPP table from the firmware and there > shouldn't > be OPP entries in the DT. If not, why not fix that to avoid more > confusion. > Can we make cpu0_node_has_opp_v2_prop return false in short. The entire point of the series is to define a way of identifying required opp-kBps values for L3/DDR at different cpu frequencies. The required-opps is chosen as the way of showcasing the dependencies. Hence the need to for a superset cpu opp-table consisting of all possible cpu freq values supported by the family of SoCs and enabling only the opps that are supported by the board from the values read back from firmware. > > -- > Regards, > Sudeep -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On 2020-01-28 01:33, Sibi Sankar wrote: > This RFC series aims to extend cpu based scaling support to L3/DDR on > SDM845 and SC7180 SoCs. > Hey Viresh/Saravana, Ping! Can you take a stab at reviewing the series, it has been on the list for a while now. > Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev > Patches [5-7] - Hack in a way to add/remove multiple opp tables to > a single device. I am yet to fix the debugfs to > support multiple opp_tables per device but wanted to > send what was working upstream to get an idea if multiple > opp tables per device is a feature that will be useful > upstream. > Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845 > and SC7180 SoCs. > > 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 > > Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420 > > Some alternate ways of hosting the opp-tables: > https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 > https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 > Viresh didn't really like ^^ bindings and they dont really scale well. > Just > including them here for completeness. > > Depends on the following series: > [1] https://patchwork.kernel.org/cover/11277199/ > [2] https://patchwork.kernel.org/cover/11055499/ > [3] https://patchwork.kernel.org/cover/11326381/ > > Sibi Sankar (10): > arm64: dts: qcom: sdm845: Add SoC compatible to MTP > cpufreq: blacklist SDM845 in cpufreq-dt-platdev > cpufreq: blacklist SC7180 in cpufreq-dt-platdev > OPP: Add and export helper to update voltage > opp: of: export _opp_of_get_opp_desc_node > opp: Allow multiple opp_tables to be mapped to a single device > opp: Remove multiple attached opp tables from a device > cpufreq: qcom: Update the bandwidth levels on frequency change > arm64: dts: qcom: sdm845: Add cpu OPP tables > arm64: dts: qcom: sc7180: Add cpu OPP tables > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++ > arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 ++++++++++++++++++++++++ > drivers/cpufreq/cpufreq-dt-platdev.c | 2 + > drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++-- > drivers/opp/core.c | 111 +++++- > drivers/opp/of.c | 3 +- > drivers/opp/opp.h | 2 + > include/linux/pm_opp.h | 10 + > 9 files changed, 1083 insertions(+), 33 deletions(-) -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On 18-03-20, 02:13, Sibi Sankar wrote:
> On 2020-01-28 01:33, Sibi Sankar wrote:
> > This RFC series aims to extend cpu based scaling support to L3/DDR on
> > SDM845 and SC7180 SoCs.
> >
>
> Hey Viresh/Saravana,
>
> Ping! Can you take a stab at reviewing
> the series, it has been on the list for
> a while now.
I believe this depends on Saravana's series on which I have raised
some doubts few weeks back ? I am still waiting for them to get
clarified by him.
--
viresh
On 3/18/2020 9:12 AM, Viresh Kumar wrote:
> On 18-03-20, 02:13, Sibi Sankar wrote:
>> On 2020-01-28 01:33, Sibi Sankar wrote:
>>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>>> SDM845 and SC7180 SoCs.
>>>
>>
>> Hey Viresh/Saravana,
>>
>> Ping! Can you take a stab at reviewing
>> the series, it has been on the list for
>> a while now.
>
> I believe this depends on Saravana's series on which I have raised
> some doubts few weeks back ? I am still waiting for them to get
> clarified by him.
Could you please post a link to the discussion that you are referring to here?
I looked at a few links posted in the cover letter as dependencies and it seems
like the discussions are pending for *months* and not weeks but I might have looked
at the wrong ones.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On 2020-03-19 15:12, Rajendra Nayak wrote: > On 3/18/2020 9:12 AM, Viresh Kumar wrote: >> On 18-03-20, 02:13, Sibi Sankar wrote: >>> On 2020-01-28 01:33, Sibi Sankar wrote: >>>> This RFC series aims to extend cpu based scaling support to L3/DDR >>>> on >>>> SDM845 and SC7180 SoCs. >>>> >>> >>> Hey Viresh/Saravana, >>> >>> Ping! Can you take a stab at reviewing >>> the series, it has been on the list for >>> a while now. >> >> I believe this depends on Saravana's series on which I have raised >> some doubts few weeks back ? I am still waiting for them to get >> clarified by him. Viresh, Saravana's example does show a device with multiple opp tables but doesn't need multiple opp table support to land though (since it works fine with the current implementation). I am more interested in understanding your/ Stephen's/Saravana's stance on adding multiple opp-table support. Personally I feel its inevitable, since multiple qc drivers using interconnect opp-tables, routinely need vote on multiple paths in a non-trivial manner. > > Could you please post a link to the discussion that you are referring > to here? > I looked at a few links posted in the cover letter as dependencies and > it seems > like the discussions are pending for *months* and not weeks but I > might have looked > at the wrong ones. https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/ Rajendra, Viresh is referring to ^^ one -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On 19-03-20, 15:41, Sibi Sankar wrote: > Viresh, > Saravana's example does show a device > with multiple opp tables but doesn't > need multiple opp table support to > land though (since it works fine with > the current implementation). I am more > interested in understanding your/ > Stephen's/Saravana's stance on adding > multiple opp-table support. Personally > I feel its inevitable, since multiple > qc drivers using interconnect opp-tables, > routinely need vote on multiple paths in > a non-trivial manner. The OPP core doesn't support multiple OPP tables for a device and I don't understand how it will. And so I have been waiting for a reply. > > > > Could you please post a link to the discussion that you are referring to > > here? > > I looked at a few links posted in the cover letter as dependencies and > > it seems > > like the discussions are pending for *months* and not weeks but I > > might have looked > > at the wrong ones. > > https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/ > > Rajendra, > Viresh is referring to ^^ one Right, thanks. -- viresh
On 3/19/2020 3:54 PM, Viresh Kumar wrote: > On 19-03-20, 15:41, Sibi Sankar wrote: >> Viresh, >> Saravana's example does show a device >> with multiple opp tables but doesn't >> need multiple opp table support to >> land though (since it works fine with >> the current implementation). I am more >> interested in understanding your/ >> Stephen's/Saravana's stance on adding >> multiple opp-table support. Personally >> I feel its inevitable, since multiple >> qc drivers using interconnect opp-tables, >> routinely need vote on multiple paths in >> a non-trivial manner. > > The OPP core doesn't support multiple OPP tables for a device and I > don't understand how it will. And so I have been waiting for a reply. I thought this series indeed is proposing to add that support in OPP core? a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device" > >>> >>> Could you please post a link to the discussion that you are referring to >>> here? >>> I looked at a few links posted in the cover letter as dependencies and >>> it seems >>> like the discussions are pending for *months* and not weeks but I >>> might have looked >>> at the wrong ones. >> >> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/ >> >> Rajendra, >> Viresh is referring to ^^ one > > Right, thanks. These discussions are stalled for over 2 months now waiting on a response from Saravana. Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response? -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
On 2020-03-19 15:54, Viresh Kumar wrote: > On 19-03-20, 15:41, Sibi Sankar wrote: >> Viresh, >> Saravana's example does show a device >> with multiple opp tables but doesn't >> need multiple opp table support to >> land though (since it works fine with >> the current implementation). I am more >> interested in understanding your/ >> Stephen's/Saravana's stance on adding >> multiple opp-table support. Personally >> I feel its inevitable, since multiple >> qc drivers using interconnect opp-tables, >> routinely need vote on multiple paths in >> a non-trivial manner. > > The OPP core doesn't support multiple OPP tables for a device and I > don't understand how it will. And so I have been waiting for a reply. This series tries to add minimal support for multiple opp-tables per device and can be tested on db845c/sdm845mtp/sc7180. Debugfs still needs to be fixed though and fixing it did feel non-trivial (I can get to it if we reach a consensus). Perhaps we can fork out icc oppp-tables from regular opp-tables and allow only multiple instances of icc opp-tables per device(though Rob didn't like the bindings associated with it) that way all the current users wont be affected but from what I've tested the series shouldn't cause any regression. > >> > >> > Could you please post a link to the discussion that you are referring to >> > here? >> > I looked at a few links posted in the cover letter as dependencies and >> > it seems >> > like the discussions are pending for *months* and not weeks but I >> > might have looked >> > at the wrong ones. >> >> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/ >> >> Rajendra, >> Viresh is referring to ^^ one > > Right, thanks. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
On 19-03-20, 16:23, Rajendra Nayak wrote:
>
>
> On 3/19/2020 3:54 PM, Viresh Kumar wrote:
> I thought this series indeed is proposing to add that support in OPP core?
> a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"
>
> These discussions are stalled for over 2 months now waiting on a response from Saravana.
> Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?
I agree and I am equally worried about it. So lets clear the air a bit
first. Can someone answer following :
- This series depends on the series from Saravana ? Right, so that
needs to get merged/accepted first ?
- If yes, then what is the way forward as Saravana isn't responding
right now ..
--
viresh
On 3/19/2020 4:38 PM, Viresh Kumar wrote:
> On 19-03-20, 16:23, Rajendra Nayak wrote:
>>
>>
>> On 3/19/2020 3:54 PM, Viresh Kumar wrote:
>> I thought this series indeed is proposing to add that support in OPP core?
>> a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"
>>
>> These discussions are stalled for over 2 months now waiting on a response from Saravana.
>> Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?
>
> I agree and I am equally worried about it. So lets clear the air a bit
> first. Can someone answer following :
>
> - This series depends on the series from Saravana ? Right, so that
> needs to get merged/accepted first ?
>
> - If yes, then what is the way forward as Saravana isn't responding
> right now ..
sure, I understand there is a dependency, however refusing to review the approach
(to add multiple OPPS tables per device) that this series is taking because of an outstanding
question which, if I read it right is "We can not add multiple OPP tables for a single device right now"
seems odd.
Its fine if you are not happy with the approach taken here and you can propose something else,
but it looks inevitable that we would need something like this to be supported (multiple OPP tables per device)
and hence the request to review the patches.
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
On 19-03-20, 17:03, Rajendra Nayak wrote:
> sure, I understand there is a dependency, however refusing to review the approach
> (to add multiple OPPS tables per device) that this series is taking because of an outstanding
> question which, if I read it right is "We can not add multiple OPP tables for a single device right now"
> seems odd.
>
> Its fine if you are not happy with the approach taken here and you can propose something else,
> but it looks inevitable that we would need something like this to be supported (multiple OPP tables per device)
> and hence the request to review the patches.
I understand the frustration you have right now, but honestly most of
the delay is not from my part. I normally try to be very quick in
giving reviews or even accepting material for next cycle. Still, I am
part of the chain which has blocked this series and I am really sorry
for that.
Coming back to reviews. I never liked the idea of keeping separate OPP
tables just for the relation that a peripheral share with the
interconnect, i.e. the b/w tables getting discussed right now.
And I am not at all sure if such tables should be added as device
tables anyway, it was present in cover letter of Saravana for sometime
but not in the real patches and so I missed it until the time Sibi
asked me something on IRC.
To be clear, I don't think we will allow multiple active OPP tables
for a device for now, unless we have a very good reason to do that. It
just doesn't make sense to me to have one table for keeping
frequency/voltage thing, and another set of tables for b/w
requirements from interconnect.
So, the changes proposed by this patchset even doesn't matter as I am
not agreeing on the binding itself. And we need a binding change for
that, which Saravana never sent, i.e. to have multiple active tables
for a device.
Yes, we need a solution for this and a quick one. I am fine to discuss
this all on a hangout session (anyway, everyone is working from home
now anyways) anytime next week with the interested parties. I think
/me and Vincent can join it.
--
viresh