From 95d5a7214b86e64dc97db3d35f6d97533f89fb32 Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Fri, 1 Apr 2022 15:06:03 +0100 Subject: iio: chemical: scd30: Export dev_pm_ops instead of suspend() and resume() Whilst here move to the new infrastructure using pm_sleep_ptr() and EXPORT_DEV_PM_OPS() so as to let the compiler remove the unused code if CONFIG_SLEEP is not defined. Signed-off-by: Jonathan Cameron Cc: Tomasz Duszynski Signed-off-by: Rafael J. Wysocki --- drivers/iio/chemical/scd30.h | 5 +---- drivers/iio/chemical/scd30_core.c | 8 ++++---- drivers/iio/chemical/scd30_i2c.c | 2 +- drivers/iio/chemical/scd30_serial.c | 2 +- 4 files changed, 7 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/chemical/scd30.h b/drivers/iio/chemical/scd30.h index f60127bfe0f4..1ac9f3f79271 100644 --- a/drivers/iio/chemical/scd30.h +++ b/drivers/iio/chemical/scd30.h @@ -68,10 +68,7 @@ struct scd30_state { scd30_command_t command; }; -int scd30_suspend(struct device *dev); -int scd30_resume(struct device *dev); - -static __maybe_unused SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume); +extern const struct dev_pm_ops scd30_pm_ops; int scd30_probe(struct device *dev, int irq, const char *name, void *priv, scd30_command_t command); diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c index 9fe6bbe9ee04..6c6c11c2772a 100644 --- a/drivers/iio/chemical/scd30_core.c +++ b/drivers/iio/chemical/scd30_core.c @@ -517,7 +517,7 @@ static const struct iio_chan_spec scd30_channels[] = { IIO_CHAN_SOFT_TIMESTAMP(3), }; -int __maybe_unused scd30_suspend(struct device *dev) +static int scd30_suspend(struct device *dev) { struct iio_dev *indio_dev = dev_get_drvdata(dev); struct scd30_state *state = iio_priv(indio_dev); @@ -529,9 +529,8 @@ int __maybe_unused scd30_suspend(struct device *dev) return regulator_disable(state->vdd); } -EXPORT_SYMBOL(scd30_suspend); -int __maybe_unused scd30_resume(struct device *dev) +static int scd30_resume(struct device *dev) { struct iio_dev *indio_dev = dev_get_drvdata(dev); struct scd30_state *state = iio_priv(indio_dev); @@ -543,7 +542,8 @@ int __maybe_unused scd30_resume(struct device *dev) return scd30_command_write(state, CMD_START_MEAS, state->pressure_comp); } -EXPORT_SYMBOL(scd30_resume); + +EXPORT_SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume); static void scd30_stop_meas(void *data) { diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c index 875892a070ee..7c332e4e8e46 100644 --- a/drivers/iio/chemical/scd30_i2c.c +++ b/drivers/iio/chemical/scd30_i2c.c @@ -128,7 +128,7 @@ static struct i2c_driver scd30_i2c_driver = { .driver = { .name = KBUILD_MODNAME, .of_match_table = scd30_i2c_of_match, - .pm = &scd30_pm_ops, + .pm = pm_sleep_ptr(&scd30_pm_ops), }, .probe_new = scd30_i2c_probe, }; diff --git a/drivers/iio/chemical/scd30_serial.c b/drivers/iio/chemical/scd30_serial.c index 568b34486c44..8664f3ce6b33 100644 --- a/drivers/iio/chemical/scd30_serial.c +++ b/drivers/iio/chemical/scd30_serial.c @@ -252,7 +252,7 @@ static struct serdev_device_driver scd30_serdev_driver = { .driver = { .name = KBUILD_MODNAME, .of_match_table = scd30_serdev_of_match, - .pm = &scd30_pm_ops, + .pm = pm_sleep_ptr(&scd30_pm_ops), }, .probe = scd30_serdev_probe, }; -- cgit v1.2.3 From bd8284e968ecfc7777703cd76eabbbbf9f3ac9ff Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Fri, 1 Apr 2022 15:06:05 +0100 Subject: iio: chemical: scd30: Move symbol exports into IIO_SCD30 namespace Avoid unnecessary pollution of the global symbol namespace by moving library functions in to a specific namespace and import that into the drivers that make use of the functions. For more info: https://lwn.net/Articles/760045/ Signed-off-by: Jonathan Cameron Cc: Tomasz Duszynski Signed-off-by: Rafael J. Wysocki --- drivers/iio/chemical/scd30_core.c | 4 ++-- drivers/iio/chemical/scd30_i2c.c | 1 + drivers/iio/chemical/scd30_serial.c | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c index 6c6c11c2772a..682fca39d14d 100644 --- a/drivers/iio/chemical/scd30_core.c +++ b/drivers/iio/chemical/scd30_core.c @@ -543,7 +543,7 @@ static int scd30_resume(struct device *dev) return scd30_command_write(state, CMD_START_MEAS, state->pressure_comp); } -EXPORT_SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume); +EXPORT_NS_SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend, scd30_resume, IIO_SCD30); static void scd30_stop_meas(void *data) { @@ -759,7 +759,7 @@ int scd30_probe(struct device *dev, int irq, const char *name, void *priv, return devm_iio_device_register(dev, indio_dev); } -EXPORT_SYMBOL(scd30_probe); +EXPORT_SYMBOL_NS(scd30_probe, IIO_SCD30); MODULE_AUTHOR("Tomasz Duszynski "); MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor core driver"); diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c index 7c332e4e8e46..bae479a4721f 100644 --- a/drivers/iio/chemical/scd30_i2c.c +++ b/drivers/iio/chemical/scd30_i2c.c @@ -137,3 +137,4 @@ module_i2c_driver(scd30_i2c_driver); MODULE_AUTHOR("Tomasz Duszynski "); MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor i2c driver"); MODULE_LICENSE("GPL v2"); +MODULE_IMPORT_NS(IIO_SCD30); diff --git a/drivers/iio/chemical/scd30_serial.c b/drivers/iio/chemical/scd30_serial.c index 8664f3ce6b33..3c519103d30b 100644 --- a/drivers/iio/chemical/scd30_serial.c +++ b/drivers/iio/chemical/scd30_serial.c @@ -261,3 +261,4 @@ module_serdev_device_driver(scd30_serdev_driver); MODULE_AUTHOR("Tomasz Duszynski "); MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor serial driver"); MODULE_LICENSE("GPL v2"); +MODULE_IMPORT_NS(IIO_SCD30); -- cgit v1.2.3 From a85ee6401a47ae3fc64ba506cacb3e7873823c65 Mon Sep 17 00:00:00 2001 From: Kevin Hao Date: Sun, 23 Jan 2022 20:45:08 +0800 Subject: cpufreq: governor: Use kobject release() method to free dbs_data The struct dbs_data embeds a struct gov_attr_set and the struct gov_attr_set embeds a kobject. Since every kobject must have a release() method and we can't use kfree() to free it directly, so introduce cpufreq_dbs_data_release() to release the dbs_data via the kobject::release() method. This fixes the calltrace like below: ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x34 WARNING: CPU: 12 PID: 810 at lib/debugobjects.c:505 debug_print_object+0xb8/0x100 Modules linked in: CPU: 12 PID: 810 Comm: sh Not tainted 5.16.0-next-20220120-yocto-standard+ #536 Hardware name: Marvell OcteonTX CN96XX board (DT) pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : debug_print_object+0xb8/0x100 lr : debug_print_object+0xb8/0x100 sp : ffff80001dfcf9a0 x29: ffff80001dfcf9a0 x28: 0000000000000001 x27: ffff0001464f0000 x26: 0000000000000000 x25: ffff8000090e3f00 x24: ffff80000af60210 x23: ffff8000094dfb78 x22: ffff8000090e3f00 x21: ffff0001080b7118 x20: ffff80000aeb2430 x19: ffff800009e8f5e0 x18: 0000000000000000 x17: 0000000000000002 x16: 00004d62e58be040 x15: 013590470523aff8 x14: ffff8000090e1828 x13: 0000000001359047 x12: 00000000f5257d14 x11: 0000000000040591 x10: 0000000066c1ffea x9 : ffff8000080d15e0 x8 : ffff80000a1765a8 x7 : 0000000000000000 x6 : 0000000000000001 x5 : ffff800009e8c000 x4 : ffff800009e8c760 x3 : 0000000000000000 x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff0001474ed040 Call trace: debug_print_object+0xb8/0x100 __debug_check_no_obj_freed+0x1d0/0x25c debug_check_no_obj_freed+0x24/0xa0 kfree+0x11c/0x440 cpufreq_dbs_governor_exit+0xa8/0xac cpufreq_exit_governor+0x44/0x90 cpufreq_set_policy+0x29c/0x570 store_scaling_governor+0x110/0x154 store+0xb0/0xe0 sysfs_kf_write+0x58/0x84 kernfs_fop_write_iter+0x12c/0x1c0 new_sync_write+0xf0/0x18c vfs_write+0x1cc/0x220 ksys_write+0x74/0x100 __arm64_sys_write+0x28/0x3c invoke_syscall.constprop.0+0x58/0xf0 do_el0_svc+0x70/0x170 el0_svc+0x54/0x190 el0t_64_sync_handler+0xa4/0x130 el0t_64_sync+0x1a0/0x1a4 irq event stamp: 189006 hardirqs last enabled at (189005): [] finish_task_switch.isra.0+0xe0/0x2c0 hardirqs last disabled at (189006): [] el1_dbg+0x24/0xa0 softirqs last enabled at (188966): [] __do_softirq+0x4b0/0x6a0 softirqs last disabled at (188957): [] __irq_exit_rcu+0x108/0x1a4 [ rjw: Because can be freed by the gov_attr_set_put() in cpufreq_dbs_governor_exit() now, it is also necessary to put the invocation of the governor ->exit() callback into the new cpufreq_dbs_data_release() function. ] Fixes: c4435630361d ("cpufreq: governor: New sysfs show/store callbacks for governor tunables") Signed-off-by: Kevin Hao Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq_governor.c | 20 +++++++++++++------- drivers/cpufreq/cpufreq_governor.h | 1 + 2 files changed, 14 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c index 0d42cf8b88d8..85da677c43d6 100644 --- a/drivers/cpufreq/cpufreq_governor.c +++ b/drivers/cpufreq/cpufreq_governor.c @@ -388,6 +388,15 @@ static void free_policy_dbs_info(struct policy_dbs_info *policy_dbs, gov->free(policy_dbs); } +static void cpufreq_dbs_data_release(struct kobject *kobj) +{ + struct dbs_data *dbs_data = to_dbs_data(to_gov_attr_set(kobj)); + struct dbs_governor *gov = dbs_data->gov; + + gov->exit(dbs_data); + kfree(dbs_data); +} + int cpufreq_dbs_governor_init(struct cpufreq_policy *policy) { struct dbs_governor *gov = dbs_governor_of(policy); @@ -425,6 +434,7 @@ int cpufreq_dbs_governor_init(struct cpufreq_policy *policy) goto free_policy_dbs_info; } + dbs_data->gov = gov; gov_attr_set_init(&dbs_data->attr_set, &policy_dbs->list); ret = gov->init(dbs_data); @@ -447,6 +457,7 @@ int cpufreq_dbs_governor_init(struct cpufreq_policy *policy) policy->governor_data = policy_dbs; gov->kobj_type.sysfs_ops = &governor_sysfs_ops; + gov->kobj_type.release = cpufreq_dbs_data_release; ret = kobject_init_and_add(&dbs_data->attr_set.kobj, &gov->kobj_type, get_governor_parent_kobj(policy), "%s", gov->gov.name); @@ -488,13 +499,8 @@ void cpufreq_dbs_governor_exit(struct cpufreq_policy *policy) policy->governor_data = NULL; - if (!count) { - if (!have_governor_per_policy()) - gov->gdbs_data = NULL; - - gov->exit(dbs_data); - kfree(dbs_data); - } + if (!count && !have_governor_per_policy()) + gov->gdbs_data = NULL; free_policy_dbs_info(policy_dbs, gov); diff --git a/drivers/cpufreq/cpufreq_governor.h b/drivers/cpufreq/cpufreq_governor.h index a5a0bc3cc23e..168c23fd7fca 100644 --- a/drivers/cpufreq/cpufreq_governor.h +++ b/drivers/cpufreq/cpufreq_governor.h @@ -37,6 +37,7 @@ enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE}; /* Governor demand based switching data (per-policy or global). */ struct dbs_data { struct gov_attr_set attr_set; + struct dbs_governor *gov; void *tuners; unsigned int ignore_nice_load; unsigned int sampling_rate; -- cgit v1.2.3 From 75a3a99a5a9886af13be44e640cb415ebda80db2 Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Mon, 21 Mar 2022 09:57:25 +0000 Subject: PM: EM: Change the order of arguments in the .active_power() callback The .active_power() callback passes the device pointer when it's called. Aligned with a convetion present in other subsystems and pass the 'dev' as a first argument. It looks more cleaner. Adjust all affected drivers which implement that API callback. Suggested-by: Ionela Voinescu Signed-off-by: Lukasz Luba Reviewed-by: Ionela Voinescu Signed-off-by: Rafael J. Wysocki --- Documentation/power/energy-model.rst | 4 ++-- drivers/cpufreq/mediatek-cpufreq-hw.c | 4 ++-- drivers/cpufreq/scmi-cpufreq.c | 4 ++-- drivers/opp/of.c | 6 +++--- include/linux/energy_model.h | 6 +++--- kernel/power/energy_model.c | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) (limited to 'drivers') diff --git a/Documentation/power/energy-model.rst b/Documentation/power/energy-model.rst index 49549aab41b4..fd29ed2506c0 100644 --- a/Documentation/power/energy-model.rst +++ b/Documentation/power/energy-model.rst @@ -181,8 +181,8 @@ EM framework:: -> drivers/cpufreq/foo_cpufreq.c - 01 static int est_power(unsigned long *mW, unsigned long *KHz, - 02 struct device *dev) + 01 static int est_power(struct device *dev, unsigned long *mW, + 02 unsigned long *KHz) 03 { 04 long freq, power; 05 diff --git a/drivers/cpufreq/mediatek-cpufreq-hw.c b/drivers/cpufreq/mediatek-cpufreq-hw.c index 0a94c56ddad2..813cccbfe934 100644 --- a/drivers/cpufreq/mediatek-cpufreq-hw.c +++ b/drivers/cpufreq/mediatek-cpufreq-hw.c @@ -51,8 +51,8 @@ static const u16 cpufreq_mtk_offsets[REG_ARRAY_SIZE] = { }; static int __maybe_unused -mtk_cpufreq_get_cpu_power(unsigned long *mW, - unsigned long *KHz, struct device *cpu_dev) +mtk_cpufreq_get_cpu_power(struct device *cpu_dev, unsigned long *mW, + unsigned long *KHz) { struct mtk_cpufreq_data *data; struct cpufreq_policy *policy; diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c index 919fa6e3f462..6d2a4cf46db7 100644 --- a/drivers/cpufreq/scmi-cpufreq.c +++ b/drivers/cpufreq/scmi-cpufreq.c @@ -96,8 +96,8 @@ scmi_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask) } static int __maybe_unused -scmi_get_cpu_power(unsigned long *power, unsigned long *KHz, - struct device *cpu_dev) +scmi_get_cpu_power(struct device *cpu_dev, unsigned long *power, + unsigned long *KHz) { unsigned long Hz; int ret, domain; diff --git a/drivers/opp/of.c b/drivers/opp/of.c index 440ab5a03df9..485ea980bde7 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -1448,7 +1448,7 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_get_of_node); * Returns 0 on success or a proper -EINVAL value in case of error. */ static int __maybe_unused -_get_dt_power(unsigned long *mW, unsigned long *kHz, struct device *dev) +_get_dt_power(struct device *dev, unsigned long *mW, unsigned long *kHz) { struct dev_pm_opp *opp; unsigned long opp_freq, opp_power; @@ -1482,8 +1482,8 @@ _get_dt_power(unsigned long *mW, unsigned long *kHz, struct device *dev) * Returns -EINVAL if the power calculation failed because of missing * parameters, 0 otherwise. */ -static int __maybe_unused _get_power(unsigned long *mW, unsigned long *kHz, - struct device *dev) +static int __maybe_unused _get_power(struct device *dev, unsigned long *mW, + unsigned long *kHz) { struct dev_pm_opp *opp; struct device_node *np; diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h index 92e82a322859..8419bffb4398 100644 --- a/include/linux/energy_model.h +++ b/include/linux/energy_model.h @@ -101,11 +101,11 @@ struct em_data_callback { /** * active_power() - Provide power at the next performance state of * a device + * @dev : Device for which we do this operation (can be a CPU) * @power : Active power at the performance state * (modified) * @freq : Frequency at the performance state in kHz * (modified) - * @dev : Device for which we do this operation (can be a CPU) * * active_power() must find the lowest performance state of 'dev' above * 'freq' and update 'power' and 'freq' to the matching active power @@ -117,8 +117,8 @@ struct em_data_callback { * * Return 0 on success. */ - int (*active_power)(unsigned long *power, unsigned long *freq, - struct device *dev); + int (*active_power)(struct device *dev, unsigned long *power, + unsigned long *freq); /** * get_cost() - Provide the cost at the given performance state of diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c index 95a3b33001f6..babefc72085d 100644 --- a/kernel/power/energy_model.c +++ b/kernel/power/energy_model.c @@ -140,7 +140,7 @@ static int em_create_perf_table(struct device *dev, struct em_perf_domain *pd, * lowest performance state of 'dev' above 'freq' and updates * 'power' and 'freq' accordingly. */ - ret = cb->active_power(&power, &freq, dev); + ret = cb->active_power(dev, &power, &freq); if (ret) { dev_err(dev, "EM: invalid perf. state: %d\n", ret); -- cgit v1.2.3 From 9926bbec3ae6c10d4e6fc297afde6a7ef7ad640b Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Mon, 21 Mar 2022 09:57:28 +0000 Subject: thermal: cooling: Check Energy Model type in cpufreq_cooling and devfreq_cooling The Energy Model can now be artificial, which means the power values are mathematically generated to leverage EAS while not expected to be on an uniform scale with other devices providing power information. If this EM type is in use, the thermal governor IPA should not be allowed to operate, since the relation between cooling devices is not properly defined. Thus, it might be possible that big GPU has lower power values than a Little CPU. To mitigate a misbehaviour of the thermal control algorithm, simply do not register the cooling device as IPA's power actor. Signed-off-by: Lukasz Luba Acked-by: Viresh Kumar Reviewed-by: Ionela Voinescu Signed-off-by: Rafael J. Wysocki --- drivers/thermal/cpufreq_cooling.c | 2 +- drivers/thermal/devfreq_cooling.c | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/thermal/cpufreq_cooling.c b/drivers/thermal/cpufreq_cooling.c index 0bfb8eebd126..b8151d95a806 100644 --- a/drivers/thermal/cpufreq_cooling.c +++ b/drivers/thermal/cpufreq_cooling.c @@ -328,7 +328,7 @@ static inline bool em_is_sane(struct cpufreq_cooling_device *cpufreq_cdev, struct cpufreq_policy *policy; unsigned int nr_levels; - if (!em) + if (!em || em_is_artificial(em)) return false; policy = cpufreq_cdev->policy; diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c index 4310cb342a9f..b04dcbbf721a 100644 --- a/drivers/thermal/devfreq_cooling.c +++ b/drivers/thermal/devfreq_cooling.c @@ -358,6 +358,7 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df, struct thermal_cooling_device *cdev; struct device *dev = df->dev.parent; struct devfreq_cooling_device *dfc; + struct em_perf_domain *em; char *name; int err, num_opps; @@ -367,8 +368,9 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df, dfc->devfreq = df; - dfc->em_pd = em_pd_get(dev); - if (dfc->em_pd) { + em = em_pd_get(dev); + if (em && !em_is_artificial(em)) { + dfc->em_pd = em; devfreq_cooling_ops.get_requested_power = devfreq_cooling_get_requested_power; devfreq_cooling_ops.state2power = devfreq_cooling_state2power; @@ -379,7 +381,7 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df, num_opps = em_pd_nr_perf_states(dfc->em_pd); } else { /* Backward compatibility for drivers which do not use IPA */ - dev_dbg(dev, "missing EM for cooling device\n"); + dev_dbg(dev, "missing proper EM for cooling device\n"); num_opps = dev_pm_opp_get_opp_count(dev); -- cgit v1.2.3 From 985a67709a66c456414182ed179544786e00321e Mon Sep 17 00:00:00 2001 From: Lukasz Luba Date: Mon, 21 Mar 2022 09:57:29 +0000 Subject: powercap: DTPM: Check for Energy Model type The Energy Model power values might be artificial. In such case it's safe to bail out during the registration, since the PowerCap framework supports only micro-Watts. Signed-off-by: Lukasz Luba Reviewed-by: Ionela Voinescu Signed-off-by: Rafael J. Wysocki --- drivers/powercap/dtpm_cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/powercap/dtpm_cpu.c b/drivers/powercap/dtpm_cpu.c index bca2f912d349..f5eced0842b3 100644 --- a/drivers/powercap/dtpm_cpu.c +++ b/drivers/powercap/dtpm_cpu.c @@ -211,7 +211,7 @@ static int __dtpm_cpu_setup(int cpu, struct dtpm *parent) return 0; pd = em_cpu_get(cpu); - if (!pd) + if (!pd || em_is_artificial(pd)) return -EINVAL; dtpm_cpu = kzalloc(sizeof(*dtpm_cpu), GFP_KERNEL); -- cgit v1.2.3 From eefa861810a896e55f9d9246ac824f544f0700b3 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 1 Apr 2022 10:49:50 +0200 Subject: PM: domains: Extend dev_pm_domain_detach() doc Mention all domain attach menthods which dev_pm_domain_detach() reverses. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Rafael J. Wysocki --- drivers/base/power/common.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index bbddb267c2e6..72115917e0bd 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -172,10 +172,10 @@ EXPORT_SYMBOL_GPL(dev_pm_domain_attach_by_name); * @dev: Device to detach. * @power_off: Used to indicate whether we should power off the device. * - * This functions will reverse the actions from dev_pm_domain_attach() and - * dev_pm_domain_attach_by_id(), thus it detaches @dev from its PM domain. - * Typically it should be invoked during the remove phase, either from - * subsystem level code or from drivers. + * This functions will reverse the actions from dev_pm_domain_attach(), + * dev_pm_domain_attach_by_id() and dev_pm_domain_attach_by_name(), thus it + * detaches @dev from its PM domain. Typically it should be invoked during the + * remove phase, either from subsystem level code or from drivers. * * Callers must ensure proper synchronization of this function with power * management callbacks. -- cgit v1.2.3 From 95996a675757a0f3f75babb8cd4f2ebcd0bda478 Mon Sep 17 00:00:00 2001 From: Christophe Leroy Date: Fri, 1 Apr 2022 19:24:28 +0200 Subject: cpufreq: Prepare cleanup of powerpc's asm/prom.h powerpc's asm/prom.h brings some headers that it doesn't need itself. In order to clean it up, first add missing headers in users of asm/prom.h Signed-off-by: Christophe Leroy Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/pasemi-cpufreq.c | 1 - drivers/cpufreq/pmac32-cpufreq.c | 2 +- drivers/cpufreq/pmac64-cpufreq.c | 2 +- drivers/cpufreq/ppc_cbe_cpufreq.c | 1 - drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 2 +- 5 files changed, 3 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/pasemi-cpufreq.c b/drivers/cpufreq/pasemi-cpufreq.c index 815645170c4d..039a66bbe1be 100644 --- a/drivers/cpufreq/pasemi-cpufreq.c +++ b/drivers/cpufreq/pasemi-cpufreq.c @@ -18,7 +18,6 @@ #include #include -#include #include #include diff --git a/drivers/cpufreq/pmac32-cpufreq.c b/drivers/cpufreq/pmac32-cpufreq.c index 4f20c6a9108d..20f64a8b0a35 100644 --- a/drivers/cpufreq/pmac32-cpufreq.c +++ b/drivers/cpufreq/pmac32-cpufreq.c @@ -24,7 +24,7 @@ #include #include #include -#include + #include #include #include diff --git a/drivers/cpufreq/pmac64-cpufreq.c b/drivers/cpufreq/pmac64-cpufreq.c index d7542a106e6b..ba9c31d98bd6 100644 --- a/drivers/cpufreq/pmac64-cpufreq.c +++ b/drivers/cpufreq/pmac64-cpufreq.c @@ -22,7 +22,7 @@ #include #include #include -#include + #include #include #include diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c index c58abb4cca3a..e3313ce63b38 100644 --- a/drivers/cpufreq/ppc_cbe_cpufreq.c +++ b/drivers/cpufreq/ppc_cbe_cpufreq.c @@ -12,7 +12,6 @@ #include #include -#include #include #include "ppc_cbe_cpufreq.h" diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c index 037fe23bc6ed..4fba3637b115 100644 --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c @@ -13,9 +13,9 @@ #include #include #include +#include #include -#include #include #include -- cgit v1.2.3 From 82586a721595fde2c1bc0dac7deed2fe9d42c106 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 13 Apr 2022 17:30:32 +0200 Subject: PM: runtime: Avoid device usage count underflows A PM-runtime device usage count underflow is potentially critical, because it may cause a device to be suspended when it is expected to be operational. It is also a programming problem that would be good to catch and warn about. For this reason, (1) make rpm_check_suspend_allowed() return an error when the device usage count is negative to prevent devices from being suspended in that case, (2) introduce rpm_drop_usage_count() that will detect device usage count underflows, warn about them and fix them up, and (3) use it to drop the usage count in a few places instead of atomic_dec_and_test(). Signed-off-by: Rafael J. Wysocki Reviewed-by: Ulf Hansson --- drivers/base/power/runtime.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index d4059e6ffeae..23bea81160e5 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -263,7 +263,7 @@ static int rpm_check_suspend_allowed(struct device *dev) retval = -EINVAL; else if (dev->power.disable_depth > 0) retval = -EACCES; - else if (atomic_read(&dev->power.usage_count) > 0) + else if (atomic_read(&dev->power.usage_count)) retval = -EAGAIN; else if (!dev->power.ignore_children && atomic_read(&dev->power.child_count)) @@ -1039,13 +1039,33 @@ int pm_schedule_suspend(struct device *dev, unsigned int delay) } EXPORT_SYMBOL_GPL(pm_schedule_suspend); +static int rpm_drop_usage_count(struct device *dev) +{ + int ret; + + ret = atomic_sub_return(1, &dev->power.usage_count); + if (ret >= 0) + return ret; + + /* + * Because rpm_resume() does not check the usage counter, it will resume + * the device even if the usage counter is 0 or negative, so it is + * sufficient to increment the usage counter here to reverse the change + * made above. + */ + atomic_inc(&dev->power.usage_count); + dev_warn(dev, "Runtime PM usage count underflow!\n"); + return -EINVAL; +} + /** * __pm_runtime_idle - Entry point for runtime idle operations. * @dev: Device to send idle notification for. * @rpmflags: Flag bits. * * If the RPM_GET_PUT flag is set, decrement the device's usage count and - * return immediately if it is larger than zero. Then carry out an idle + * return immediately if it is larger than zero (if it becomes negative, log a + * warning, increment it, and return an error). Then carry out an idle * notification, either synchronous or asynchronous. * * This routine may be called in atomic context if the RPM_ASYNC flag is set, @@ -1057,7 +1077,10 @@ int __pm_runtime_idle(struct device *dev, int rpmflags) int retval; if (rpmflags & RPM_GET_PUT) { - if (!atomic_dec_and_test(&dev->power.usage_count)) { + retval = rpm_drop_usage_count(dev); + if (retval < 0) { + return retval; + } else if (retval > 0) { trace_rpm_usage_rcuidle(dev, rpmflags); return 0; } @@ -1079,7 +1102,8 @@ EXPORT_SYMBOL_GPL(__pm_runtime_idle); * @rpmflags: Flag bits. * * If the RPM_GET_PUT flag is set, decrement the device's usage count and - * return immediately if it is larger than zero. Then carry out a suspend, + * return immediately if it is larger than zero (if it becomes negative, log a + * warning, increment it, and return an error). Then carry out a suspend, * either synchronous or asynchronous. * * This routine may be called in atomic context if the RPM_ASYNC flag is set, @@ -1091,7 +1115,10 @@ int __pm_runtime_suspend(struct device *dev, int rpmflags) int retval; if (rpmflags & RPM_GET_PUT) { - if (!atomic_dec_and_test(&dev->power.usage_count)) { + retval = rpm_drop_usage_count(dev); + if (retval < 0) { + return retval; + } else if (retval > 0) { trace_rpm_usage_rcuidle(dev, rpmflags); return 0; } @@ -1527,14 +1554,17 @@ EXPORT_SYMBOL_GPL(pm_runtime_forbid); */ void pm_runtime_allow(struct device *dev) { + int ret; + spin_lock_irq(&dev->power.lock); if (dev->power.runtime_auto) goto out; dev->power.runtime_auto = true; - if (atomic_dec_and_test(&dev->power.usage_count)) + ret = rpm_drop_usage_count(dev); + if (ret == 0) rpm_idle(dev, RPM_AUTO | RPM_ASYNC); - else + else if (ret > 0) trace_rpm_usage_rcuidle(dev, RPM_AUTO | RPM_ASYNC); out: -- cgit v1.2.3 From addca285120b0edf2fef795f7809c83774cf74b7 Mon Sep 17 00:00:00 2001 From: Chen Yu Date: Fri, 8 Apr 2022 07:42:58 +0800 Subject: cpufreq: intel_pstate: Handle no_turbo in frequency invariance Problem statement: Once the user has disabled turbo frequency by # echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo the cfs_rq's util_avg becomes quite small when compared with CPU capacity. Step to reproduce: # echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo # ./x86_cpuload --count 1 --start 3 --timeout 100 --busy 99 would launch 1 thread and bind it to CPU3, lasting for 100 seconds, with a CPU utilization of 99%. [1] top result: %Cpu3 : 98.4 us, 0.0 sy, 0.0 ni, 1.6 id, 0.0 wa, 0.0 hi, 0.0 si, 0.0 st check util_avg: cat /sys/kernel/debug/sched/debug | grep "cfs_rq\[3\]" -A 20 | grep util_avg .util_avg : 611 So the util_avg/cpu capacity is 611/1024, which is much smaller than 98.4% shown in the top result. This might impact some logic in the scheduler. For example, group_is_overloaded() would compare the group_capacity and group_util in the sched group, to check if this sched group is overloaded or not. With this gap, even when there is a nearly 100% workload, the sched group will not be regarded as overloaded. Besides group_is_overloaded(), there are also other victims. There is a ongoing work that aims to optimize the task wakeup in a LLC domain. The main idea is to stop searching idle CPUs if the sched domain is overloaded[2]. This proposal also relies on the util_avg/CPU capacity to decide whether the LLC domain is overloaded. Analysis: CPU frequency invariance has caused this difference. In summary, the util_sum of cfs rq would decay quite fast when the CPU is in idle, when the CPU frequency invariance is enabled. The detail is as followed: As depicted in update_rq_clock_pelt(), when the frequency invariance is enabled, there would be two clock variables on each rq, clock_task and clock_pelt: The clock_pelt scales the time to reflect the effective amount of computation done during the running delta time but then syncs back to clock_task when rq is idle. absolute time | 1| 2| 3| 4| 5| 6| 7| 8| 9|10|11|12|13|14|15|16 @ max frequency ------******---------------******--------------- @ half frequency ------************---------************--------- clock pelt | 1| 2| 3| 4| 7| 8| 9| 10| 11|14|15|16 The fast decay of util_sum during idle is due to: 1. rq->clock_pelt is always behind rq->clock_task 2. rq->last_update is updated to rq->clock_pelt' after invoking ___update_load_sum() 3. Then the CPU becomes idle, the rq->clock_pelt' would be suddenly increased a lot to rq->clock_task 4. Enters ___update_load_sum() again, the idle period is calculated by rq->clock_task - rq->last_update, AKA, rq->clock_task - rq->clock_pelt'. The lower the CPU frequency is, the larger the delta = rq->clock_task - rq->clock_pelt' will be. Since the idle period will be used to decay the util_sum only, the util_sum drops significantly during idle period. Proposal: This symptom is not only caused by disabling turbo frequency, but it would also appear if the user limits the max frequency at runtime. Because, if the frequency is always lower than the max frequency, CPU frequency invariance would decay the util_sum quite fast during idle. As some end users would disable turbo after boot up, this patch aims to present this symptom and deals with turbo scenarios for now. It might be ideal if CPU frequency invariance is aware of the max CPU frequency (user specified) at runtime in the future. Link: https://github.com/yu-chen-surf/x86_cpuload.git #1 Link: https://lore.kernel.org/lkml/20220310005228.11737-1-yu.c.chen@intel.com/ #2 Signed-off-by: Chen Yu Acked-by: Peter Zijlstra (Intel) Reviewed-by: Giovanni Gherdovich Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/intel_pstate.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 846bb3a78788..2216b24b6f84 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -1322,6 +1322,7 @@ static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b, mutex_unlock(&intel_pstate_limits_lock); intel_pstate_update_policies(); + arch_set_max_freq_ratio(global.no_turbo); mutex_unlock(&intel_pstate_driver_lock); -- cgit v1.2.3 From 5f50c52f13f12d63699feaae54ee2b1e4aee5a86 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:08:52 -0800 Subject: PM / devfreq: rk3399_dmc: Drop undocumented ondemand DT props These properties are: * undocumented * directly representing software properties, not hardware properties * unused (no in-tree users, yet; this IP block has so far only been used in downstream kernels) Let's just stick the values that downstream users have been using directly in the driver and call it a day. Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 293857ebfd75..e982862f6ac2 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -430,10 +430,8 @@ no_pmu: goto err_edev; } - of_property_read_u32(np, "upthreshold", - &data->ondemand_data.upthreshold); - of_property_read_u32(np, "downdifferential", - &data->ondemand_data.downdifferential); + data->ondemand_data.upthreshold = 25; + data->ondemand_data.downdifferential = 15; data->rate = clk_get_rate(data->dmc_clk); -- cgit v1.2.3 From b82acf8215c40fcb7d29e34aab17c51df58c4f40 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:08:53 -0800 Subject: PM / devfreq: rk3399_dmc: Drop excess timing properties All of these properties are initialized by ARM Trusted Firmware, and have been since the early days of this chip. It's redundant (and possibly wrong) to do this here now. What's more, there seems to be some confusion about the units and some of the definitions of this timing struct: the DT docs say MHz for many of these, but downstream users were in Hz (and therefore, the ATF interface was Hz). Also, the in-driver usage for some of these (e.g., for comparing to target frequency) were in Hz too. So doubly wrong. We can avoid thinking about who got the right units by dropping the unnecessary code and properties. They are marked deprecated in the binding schema. Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 144 +++++++++---------------------------------- 1 file changed, 29 insertions(+), 115 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index e982862f6ac2..8f447217303f 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -23,38 +23,6 @@ #include #include -struct dram_timing { - unsigned int ddr3_speed_bin; - unsigned int pd_idle; - unsigned int sr_idle; - unsigned int sr_mc_gate_idle; - unsigned int srpd_lite_idle; - unsigned int standby_idle; - unsigned int auto_pd_dis_freq; - unsigned int dram_dll_dis_freq; - unsigned int phy_dll_dis_freq; - unsigned int ddr3_odt_dis_freq; - unsigned int ddr3_drv; - unsigned int ddr3_odt; - unsigned int phy_ddr3_ca_drv; - unsigned int phy_ddr3_dq_drv; - unsigned int phy_ddr3_odt; - unsigned int lpddr3_odt_dis_freq; - unsigned int lpddr3_drv; - unsigned int lpddr3_odt; - unsigned int phy_lpddr3_ca_drv; - unsigned int phy_lpddr3_dq_drv; - unsigned int phy_lpddr3_odt; - unsigned int lpddr4_odt_dis_freq; - unsigned int lpddr4_drv; - unsigned int lpddr4_dq_odt; - unsigned int lpddr4_ca_odt; - unsigned int phy_lpddr4_ca_drv; - unsigned int phy_lpddr4_ck_cs_drv; - unsigned int phy_lpddr4_dq_drv; - unsigned int phy_lpddr4_odt; -}; - struct rk3399_dmcfreq { struct device *dev; struct devfreq *devfreq; @@ -62,13 +30,21 @@ struct rk3399_dmcfreq { struct clk *dmc_clk; struct devfreq_event_dev *edev; struct mutex lock; - struct dram_timing timing; struct regulator *vdd_center; struct regmap *regmap_pmu; unsigned long rate, target_rate; unsigned long volt, target_volt; unsigned int odt_dis_freq; int odt_pd_arg0, odt_pd_arg1; + + unsigned int pd_idle; + unsigned int sr_idle; + unsigned int sr_mc_gate_idle; + unsigned int srpd_lite_idle; + unsigned int standby_idle; + unsigned int ddr3_odt_dis_freq; + unsigned int lpddr3_odt_dis_freq; + unsigned int lpddr4_odt_dis_freq; }; static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, @@ -238,69 +214,27 @@ static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev) static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend, rk3399_dmcfreq_resume); -static int of_get_ddr_timings(struct dram_timing *timing, - struct device_node *np) +static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data, + struct device_node *np) { int ret = 0; - ret = of_property_read_u32(np, "rockchip,ddr3_speed_bin", - &timing->ddr3_speed_bin); ret |= of_property_read_u32(np, "rockchip,pd_idle", - &timing->pd_idle); + &data->pd_idle); ret |= of_property_read_u32(np, "rockchip,sr_idle", - &timing->sr_idle); + &data->sr_idle); ret |= of_property_read_u32(np, "rockchip,sr_mc_gate_idle", - &timing->sr_mc_gate_idle); + &data->sr_mc_gate_idle); ret |= of_property_read_u32(np, "rockchip,srpd_lite_idle", - &timing->srpd_lite_idle); + &data->srpd_lite_idle); ret |= of_property_read_u32(np, "rockchip,standby_idle", - &timing->standby_idle); - ret |= of_property_read_u32(np, "rockchip,auto_pd_dis_freq", - &timing->auto_pd_dis_freq); - ret |= of_property_read_u32(np, "rockchip,dram_dll_dis_freq", - &timing->dram_dll_dis_freq); - ret |= of_property_read_u32(np, "rockchip,phy_dll_dis_freq", - &timing->phy_dll_dis_freq); + &data->standby_idle); ret |= of_property_read_u32(np, "rockchip,ddr3_odt_dis_freq", - &timing->ddr3_odt_dis_freq); - ret |= of_property_read_u32(np, "rockchip,ddr3_drv", - &timing->ddr3_drv); - ret |= of_property_read_u32(np, "rockchip,ddr3_odt", - &timing->ddr3_odt); - ret |= of_property_read_u32(np, "rockchip,phy_ddr3_ca_drv", - &timing->phy_ddr3_ca_drv); - ret |= of_property_read_u32(np, "rockchip,phy_ddr3_dq_drv", - &timing->phy_ddr3_dq_drv); - ret |= of_property_read_u32(np, "rockchip,phy_ddr3_odt", - &timing->phy_ddr3_odt); + &data->ddr3_odt_dis_freq); ret |= of_property_read_u32(np, "rockchip,lpddr3_odt_dis_freq", - &timing->lpddr3_odt_dis_freq); - ret |= of_property_read_u32(np, "rockchip,lpddr3_drv", - &timing->lpddr3_drv); - ret |= of_property_read_u32(np, "rockchip,lpddr3_odt", - &timing->lpddr3_odt); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr3_ca_drv", - &timing->phy_lpddr3_ca_drv); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr3_dq_drv", - &timing->phy_lpddr3_dq_drv); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr3_odt", - &timing->phy_lpddr3_odt); + &data->lpddr3_odt_dis_freq); ret |= of_property_read_u32(np, "rockchip,lpddr4_odt_dis_freq", - &timing->lpddr4_odt_dis_freq); - ret |= of_property_read_u32(np, "rockchip,lpddr4_drv", - &timing->lpddr4_drv); - ret |= of_property_read_u32(np, "rockchip,lpddr4_dq_odt", - &timing->lpddr4_dq_odt); - ret |= of_property_read_u32(np, "rockchip,lpddr4_ca_odt", - &timing->lpddr4_ca_odt); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_ca_drv", - &timing->phy_lpddr4_ca_drv); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_ck_cs_drv", - &timing->phy_lpddr4_ck_cs_drv); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_dq_drv", - &timing->phy_lpddr4_dq_drv); - ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_odt", - &timing->phy_lpddr4_odt); + &data->lpddr4_odt_dis_freq); return ret; } @@ -311,8 +245,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = pdev->dev.of_node, *node; struct rk3399_dmcfreq *data; - int ret, index, size; - uint32_t *timing; + int ret; struct dev_pm_opp *opp; u32 ddr_type; u32 val; @@ -343,26 +276,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) return ret; } - /* - * Get dram timing and pass it to arm trust firmware, - * the dram driver in arm trust firmware will get these - * timing and to do dram initial. - */ - if (!of_get_ddr_timings(&data->timing, np)) { - timing = &data->timing.ddr3_speed_bin; - size = sizeof(struct dram_timing) / 4; - for (index = 0; index < size; index++) { - arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, *timing++, index, - ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM, - 0, 0, 0, 0, &res); - if (res.a0) { - dev_err(dev, "Failed to set dram param: %ld\n", - res.a0); - ret = -EINVAL; - goto err_edev; - } - } - } + rk3399_dmcfreq_of_props(data, np); node = of_parse_phandle(np, "rockchip,pmu", 0); if (!node) @@ -381,13 +295,13 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev) switch (ddr_type) { case RK3399_PMUGRF_DDRTYPE_DDR3: - data->odt_dis_freq = data->timing.ddr3_odt_dis_freq; + data->odt_dis_freq = data->ddr3_odt_dis_freq; break; case RK3399_PMUGRF_DDRTYPE_LPDDR3: - data->odt_dis_freq = data->timing.lpddr3_odt_dis_freq; + data->odt_dis_freq = data->lpddr3_odt_dis_freq; break; case RK3399_PMUGRF_DDRTYPE_LPDDR4: - data->odt_dis_freq = data->timing.lpddr4_odt_dis_freq; + data->odt_dis_freq = data->lpddr4_odt_dis_freq; break; default: ret = -EINVAL; @@ -414,11 +328,11 @@ no_pmu: * arg2: * bit[0] : odt enable */ - data->odt_pd_arg0 = (data->timing.sr_idle & 0xff) | - ((data->timing.sr_mc_gate_idle & 0xff) << 8) | - ((data->timing.standby_idle & 0xffff) << 16); - data->odt_pd_arg1 = (data->timing.pd_idle & 0xfff) | - ((data->timing.srpd_lite_idle & 0xfff) << 16); + data->odt_pd_arg0 = (data->sr_idle & 0xff) | + ((data->sr_mc_gate_idle & 0xff) << 8) | + ((data->standby_idle & 0xffff) << 16); + data->odt_pd_arg1 = (data->pd_idle & 0xfff) | + ((data->srpd_lite_idle & 0xfff) << 16); /* * We add a devfreq driver to our parent since it has a device tree node -- cgit v1.2.3 From a5ca18540dab8f9477883ad9365d33a88abca958 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:08:54 -0800 Subject: PM / devfreq: rk3399_dmc: Use bitfield macro definitions for ODT_PD We're going to add new usages, and it's cleaner to work with macros instead of comments and magic numbers. Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 43 ++++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 19 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 8f447217303f..c4efbc15cbb1 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -23,6 +24,15 @@ #include #include +#define RK3399_SET_ODT_PD_0_SR_IDLE GENMASK(7, 0) +#define RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE GENMASK(15, 8) +#define RK3399_SET_ODT_PD_0_STANDBY_IDLE GENMASK(31, 16) + +#define RK3399_SET_ODT_PD_1_PD_IDLE GENMASK(11, 0) +#define RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE GENMASK(27, 16) + +#define RK3399_SET_ODT_PD_2_ODT_ENABLE BIT(0) + struct rk3399_dmcfreq { struct device *dev; struct devfreq *devfreq; @@ -55,7 +65,6 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, unsigned long old_clk_rate = dmcfreq->rate; unsigned long target_volt, target_rate; struct arm_smccc_res res; - bool odt_enable = false; int err; opp = devfreq_recommended_opp(dev, freq, flags); @@ -72,8 +81,10 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, mutex_lock(&dmcfreq->lock); if (dmcfreq->regmap_pmu) { + unsigned int odt_pd_arg2 = 0; + if (target_rate >= dmcfreq->odt_dis_freq) - odt_enable = true; + odt_pd_arg2 |= RK3399_SET_ODT_PD_2_ODT_ENABLE; /* * This makes a SMC call to the TF-A to set the DDR PD @@ -83,7 +94,7 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0, dmcfreq->odt_pd_arg1, ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD, - odt_enable, 0, 0, 0, &res); + odt_pd_arg2, 0, 0, 0, &res); } /* @@ -316,23 +327,17 @@ no_pmu: /* * In TF-A there is a platform SIP call to set the PD (power-down) * timings and to enable or disable the ODT (on-die termination). - * This call needs three arguments as follows: - * - * arg0: - * bit[0-7] : sr_idle - * bit[8-15] : sr_mc_gate_idle - * bit[16-31] : standby idle - * arg1: - * bit[0-11] : pd_idle - * bit[16-27] : srpd_lite_idle - * arg2: - * bit[0] : odt enable */ - data->odt_pd_arg0 = (data->sr_idle & 0xff) | - ((data->sr_mc_gate_idle & 0xff) << 8) | - ((data->standby_idle & 0xffff) << 16); - data->odt_pd_arg1 = (data->pd_idle & 0xfff) | - ((data->srpd_lite_idle & 0xfff) << 16); + data->odt_pd_arg0 = + FIELD_PREP(RK3399_SET_ODT_PD_0_SR_IDLE, data->sr_idle) | + FIELD_PREP(RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE, + data->sr_mc_gate_idle) | + FIELD_PREP(RK3399_SET_ODT_PD_0_STANDBY_IDLE, + data->standby_idle); + data->odt_pd_arg1 = + FIELD_PREP(RK3399_SET_ODT_PD_1_PD_IDLE, data->pd_idle) | + FIELD_PREP(RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE, + data->srpd_lite_idle); /* * We add a devfreq driver to our parent since it has a device tree node -- cgit v1.2.3 From e442172191782d2488c5543b5a43f22f912ed9b5 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:08:55 -0800 Subject: PM / devfreq: rk3399_dmc: Support new disable-freq properties Implement the newly-defined properties to allow disabling certain power-saving-at-idle features at higher frequencies. This is a rewritten version of work by Lin Huang . Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 51 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index c4efbc15cbb1..fc740c1f6747 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -55,6 +55,12 @@ struct rk3399_dmcfreq { unsigned int ddr3_odt_dis_freq; unsigned int lpddr3_odt_dis_freq; unsigned int lpddr4_odt_dis_freq; + + unsigned int pd_idle_dis_freq; + unsigned int sr_idle_dis_freq; + unsigned int sr_mc_gate_idle_dis_freq; + unsigned int srpd_lite_idle_dis_freq; + unsigned int standby_idle_dis_freq; }; static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, @@ -81,8 +87,25 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, mutex_lock(&dmcfreq->lock); if (dmcfreq->regmap_pmu) { + unsigned int odt_pd_arg0 = dmcfreq->odt_pd_arg0; + unsigned int odt_pd_arg1 = dmcfreq->odt_pd_arg1; unsigned int odt_pd_arg2 = 0; + if (target_rate >= dmcfreq->sr_idle_dis_freq) + odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_SR_IDLE; + + if (target_rate >= dmcfreq->sr_mc_gate_idle_dis_freq) + odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE; + + if (target_rate >= dmcfreq->standby_idle_dis_freq) + odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_STANDBY_IDLE; + + if (target_rate >= dmcfreq->pd_idle_dis_freq) + odt_pd_arg1 &= ~RK3399_SET_ODT_PD_1_PD_IDLE; + + if (target_rate >= dmcfreq->srpd_lite_idle_dis_freq) + odt_pd_arg1 &= ~RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE; + if (target_rate >= dmcfreq->odt_dis_freq) odt_pd_arg2 |= RK3399_SET_ODT_PD_2_ODT_ENABLE; @@ -91,10 +114,9 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, * (power-down) timings and to enable or disable the * ODT (on-die termination) resistors. */ - arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0, - dmcfreq->odt_pd_arg1, - ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD, - odt_pd_arg2, 0, 0, 0, &res); + arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, odt_pd_arg0, odt_pd_arg1, + ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD, odt_pd_arg2, + 0, 0, 0, &res); } /* @@ -230,6 +252,16 @@ static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data, { int ret = 0; + /* + * These are all optional, and serve as minimum bounds. Give them large + * (i.e., never "disabled") values if the DT doesn't specify one. + */ + data->pd_idle_dis_freq = + data->sr_idle_dis_freq = + data->sr_mc_gate_idle_dis_freq = + data->srpd_lite_idle_dis_freq = + data->standby_idle_dis_freq = UINT_MAX; + ret |= of_property_read_u32(np, "rockchip,pd_idle", &data->pd_idle); ret |= of_property_read_u32(np, "rockchip,sr_idle", @@ -247,6 +279,17 @@ static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data, ret |= of_property_read_u32(np, "rockchip,lpddr4_odt_dis_freq", &data->lpddr4_odt_dis_freq); + ret |= of_property_read_u32(np, "rockchip,pd-idle-dis-freq-hz", + &data->pd_idle_dis_freq); + ret |= of_property_read_u32(np, "rockchip,sr-idle-dis-freq-hz", + &data->sr_idle_dis_freq); + ret |= of_property_read_u32(np, "rockchip,sr-mc-gate-idle-dis-freq-hz", + &data->sr_mc_gate_idle_dis_freq); + ret |= of_property_read_u32(np, "rockchip,srpd-lite-idle-dis-freq-hz", + &data->srpd_lite_idle_dis_freq); + ret |= of_property_read_u32(np, "rockchip,standby-idle-dis-freq-hz", + &data->standby_idle_dis_freq); + return ret; } -- cgit v1.2.3 From fd5b8479ef7e6a6cf1cf83ed62349c11b3864f12 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:08:56 -0800 Subject: PM / devfreq: rk3399_dmc: Support new *-ns properties We want to keep the idle time fixed, so compute based on the current DDR frequency. The old properties were deprecated and never used, so we can safely drop them from the driver. This is a rewritten version of work by Lin Huang . Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 85 ++++++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 35 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index fc740c1f6747..f778564cab49 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -24,6 +24,8 @@ #include #include +#define NS_TO_CYCLE(NS, MHz) (((NS) * (MHz)) / NSEC_PER_USEC) + #define RK3399_SET_ODT_PD_0_SR_IDLE GENMASK(7, 0) #define RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE GENMASK(15, 8) #define RK3399_SET_ODT_PD_0_STANDBY_IDLE GENMASK(31, 16) @@ -45,13 +47,12 @@ struct rk3399_dmcfreq { unsigned long rate, target_rate; unsigned long volt, target_volt; unsigned int odt_dis_freq; - int odt_pd_arg0, odt_pd_arg1; - unsigned int pd_idle; - unsigned int sr_idle; - unsigned int sr_mc_gate_idle; - unsigned int srpd_lite_idle; - unsigned int standby_idle; + unsigned int pd_idle_ns; + unsigned int sr_idle_ns; + unsigned int sr_mc_gate_idle_ns; + unsigned int srpd_lite_idle_ns; + unsigned int standby_idle_ns; unsigned int ddr3_odt_dis_freq; unsigned int lpddr3_odt_dis_freq; unsigned int lpddr4_odt_dis_freq; @@ -70,9 +71,14 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, struct dev_pm_opp *opp; unsigned long old_clk_rate = dmcfreq->rate; unsigned long target_volt, target_rate; + unsigned int ddrcon_mhz; struct arm_smccc_res res; int err; + u32 odt_pd_arg0 = 0; + u32 odt_pd_arg1 = 0; + u32 odt_pd_arg2 = 0; + opp = devfreq_recommended_opp(dev, freq, flags); if (IS_ERR(opp)) return PTR_ERR(opp); @@ -86,11 +92,35 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, mutex_lock(&dmcfreq->lock); - if (dmcfreq->regmap_pmu) { - unsigned int odt_pd_arg0 = dmcfreq->odt_pd_arg0; - unsigned int odt_pd_arg1 = dmcfreq->odt_pd_arg1; - unsigned int odt_pd_arg2 = 0; + /* + * Some idle parameters may be based on the DDR controller clock, which + * is half of the DDR frequency. + * pd_idle and standby_idle are based on the controller clock cycle. + * sr_idle_cycle, sr_mc_gate_idle_cycle, and srpd_lite_idle_cycle + * are based on the 1024 controller clock cycle + */ + ddrcon_mhz = target_rate / USEC_PER_SEC / 2; + + u32p_replace_bits(&odt_pd_arg1, + NS_TO_CYCLE(dmcfreq->pd_idle_ns, ddrcon_mhz), + RK3399_SET_ODT_PD_1_PD_IDLE); + u32p_replace_bits(&odt_pd_arg0, + NS_TO_CYCLE(dmcfreq->standby_idle_ns, ddrcon_mhz), + RK3399_SET_ODT_PD_0_STANDBY_IDLE); + u32p_replace_bits(&odt_pd_arg0, + DIV_ROUND_UP(NS_TO_CYCLE(dmcfreq->sr_idle_ns, + ddrcon_mhz), 1024), + RK3399_SET_ODT_PD_0_SR_IDLE); + u32p_replace_bits(&odt_pd_arg0, + DIV_ROUND_UP(NS_TO_CYCLE(dmcfreq->sr_mc_gate_idle_ns, + ddrcon_mhz), 1024), + RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE); + u32p_replace_bits(&odt_pd_arg1, + DIV_ROUND_UP(NS_TO_CYCLE(dmcfreq->srpd_lite_idle_ns, + ddrcon_mhz), 1024), + RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE); + if (dmcfreq->regmap_pmu) { if (target_rate >= dmcfreq->sr_idle_dis_freq) odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_SR_IDLE; @@ -262,16 +292,16 @@ static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data, data->srpd_lite_idle_dis_freq = data->standby_idle_dis_freq = UINT_MAX; - ret |= of_property_read_u32(np, "rockchip,pd_idle", - &data->pd_idle); - ret |= of_property_read_u32(np, "rockchip,sr_idle", - &data->sr_idle); - ret |= of_property_read_u32(np, "rockchip,sr_mc_gate_idle", - &data->sr_mc_gate_idle); - ret |= of_property_read_u32(np, "rockchip,srpd_lite_idle", - &data->srpd_lite_idle); - ret |= of_property_read_u32(np, "rockchip,standby_idle", - &data->standby_idle); + ret |= of_property_read_u32(np, "rockchip,pd-idle-ns", + &data->pd_idle_ns); + ret |= of_property_read_u32(np, "rockchip,sr-idle-ns", + &data->sr_idle_ns); + ret |= of_property_read_u32(np, "rockchip,sr-mc-gate-idle-ns", + &data->sr_mc_gate_idle_ns); + ret |= of_property_read_u32(np, "rockchip,srpd-lite-idle-ns", + &data->srpd_lite_idle_ns); + ret |= of_property_read_u32(np, "rockchip,standby-idle-ns", + &data->standby_idle_ns); ret |= of_property_read_u32(np, "rockchip,ddr3_odt_dis_freq", &data->ddr3_odt_dis_freq); ret |= of_property_read_u32(np, "rockchip,lpddr3_odt_dis_freq", @@ -367,21 +397,6 @@ no_pmu: ROCKCHIP_SIP_CONFIG_DRAM_INIT, 0, 0, 0, 0, &res); - /* - * In TF-A there is a platform SIP call to set the PD (power-down) - * timings and to enable or disable the ODT (on-die termination). - */ - data->odt_pd_arg0 = - FIELD_PREP(RK3399_SET_ODT_PD_0_SR_IDLE, data->sr_idle) | - FIELD_PREP(RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE, - data->sr_mc_gate_idle) | - FIELD_PREP(RK3399_SET_ODT_PD_0_STANDBY_IDLE, - data->standby_idle); - data->odt_pd_arg1 = - FIELD_PREP(RK3399_SET_ODT_PD_1_PD_IDLE, data->pd_idle) | - FIELD_PREP(RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE, - data->srpd_lite_idle); - /* * We add a devfreq driver to our parent since it has a device tree node * with operating points. -- cgit v1.2.3 From 2fccf9e6050e0e3b8b4cd275d41daf7f7fa22804 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:08:59 -0800 Subject: PM / devfreq: rk3399_dmc: Disable edev on remove() Otherwise we hit an unablanced enable-count when unbinding the DFI device: [ 1279.659119] ------------[ cut here ]------------ [ 1279.659179] WARNING: CPU: 2 PID: 5638 at drivers/devfreq/devfreq-event.c:360 devfreq_event_remove_edev+0x84/0x8c ... [ 1279.659352] Hardware name: Google Kevin (DT) [ 1279.659363] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO BTYPE=--) [ 1279.659371] pc : devfreq_event_remove_edev+0x84/0x8c [ 1279.659380] lr : devm_devfreq_event_release+0x1c/0x28 ... [ 1279.659571] Call trace: [ 1279.659582] devfreq_event_remove_edev+0x84/0x8c [ 1279.659590] devm_devfreq_event_release+0x1c/0x28 [ 1279.659602] release_nodes+0x1cc/0x244 [ 1279.659611] devres_release_all+0x44/0x60 [ 1279.659621] device_release_driver_internal+0x11c/0x1ac [ 1279.659629] device_driver_detach+0x20/0x2c [ 1279.659641] unbind_store+0x7c/0xb0 [ 1279.659650] drv_attr_store+0x2c/0x40 [ 1279.659663] sysfs_kf_write+0x44/0x58 [ 1279.659672] kernfs_fop_write_iter+0xf4/0x190 [ 1279.659684] vfs_write+0x2b0/0x2e4 [ 1279.659693] ksys_write+0x80/0xec [ 1279.659701] __arm64_sys_write+0x24/0x30 [ 1279.659714] el0_svc_common+0xf0/0x1d8 [ 1279.659724] do_el0_svc_compat+0x28/0x3c [ 1279.659738] el0_svc_compat+0x10/0x1c [ 1279.659746] el0_sync_compat_handler+0xa8/0xcc [ 1279.659758] el0_sync_compat+0x188/0x1c0 [ 1279.659768] ---[ end trace cec200e5094155b4 ]--- Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index f778564cab49..fca9fcbd4249 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -452,6 +452,8 @@ static int rk3399_dmcfreq_remove(struct platform_device *pdev) { struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(&pdev->dev); + devfreq_event_disable_edev(dmcfreq->edev); + /* * Before remove the opp table we need to unregister the opp notifier. */ -- cgit v1.2.3 From cb178a9585946d5f2691d784640038edd4111cd1 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:09:00 -0800 Subject: PM / devfreq: rk3399_dmc: Use devm_pm_opp_of_add_table() This simplifies error-cleanup and remove(). Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index fca9fcbd4249..9615658d04ae 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -401,7 +401,7 @@ no_pmu: * We add a devfreq driver to our parent since it has a device tree node * with operating points. */ - if (dev_pm_opp_of_add_table(dev)) { + if (devm_pm_opp_of_add_table(dev)) { dev_err(dev, "Invalid operating-points in device tree.\n"); ret = -EINVAL; goto err_edev; @@ -415,7 +415,7 @@ no_pmu: opp = devfreq_recommended_opp(dev, &data->rate, 0); if (IS_ERR(opp)) { ret = PTR_ERR(opp); - goto err_free_opp; + goto err_edev; } data->rate = dev_pm_opp_get_freq(opp); @@ -430,7 +430,7 @@ no_pmu: &data->ondemand_data); if (IS_ERR(data->devfreq)) { ret = PTR_ERR(data->devfreq); - goto err_free_opp; + goto err_edev; } devm_devfreq_register_opp_notifier(dev, data->devfreq); @@ -440,8 +440,6 @@ no_pmu: return 0; -err_free_opp: - dev_pm_opp_of_remove_table(&pdev->dev); err_edev: devfreq_event_disable_edev(data->edev); @@ -454,12 +452,6 @@ static int rk3399_dmcfreq_remove(struct platform_device *pdev) devfreq_event_disable_edev(dmcfreq->edev); - /* - * Before remove the opp table we need to unregister the opp notifier. - */ - devm_devfreq_unregister_opp_notifier(dmcfreq->dev, dmcfreq->devfreq); - dev_pm_opp_of_remove_table(dmcfreq->dev); - return 0; } -- cgit v1.2.3 From 5d521a307526871f5613d24a8e71babf1869c486 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 8 Mar 2022 11:09:01 -0800 Subject: PM / devfreq: rk3399_dmc: Avoid static (reused) profile This static struct can get reused if the device gets removed/reprobed, and that causes use-after-free in its ->freq_table. Let's just move the struct to our dynamic allocation. Signed-off-by: Brian Norris Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index 9615658d04ae..e494d1497d60 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -38,6 +38,7 @@ struct rk3399_dmcfreq { struct device *dev; struct devfreq *devfreq; + struct devfreq_dev_profile profile; struct devfreq_simple_ondemand_data ondemand_data; struct clk *dmc_clk; struct devfreq_event_dev *edev; @@ -228,13 +229,6 @@ static int rk3399_dmcfreq_get_cur_freq(struct device *dev, unsigned long *freq) return 0; } -static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = { - .polling_ms = 200, - .target = rk3399_dmcfreq_target, - .get_dev_status = rk3399_dmcfreq_get_dev_status, - .get_cur_freq = rk3399_dmcfreq_get_cur_freq, -}; - static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev) { struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev); @@ -422,10 +416,16 @@ no_pmu: data->volt = dev_pm_opp_get_voltage(opp); dev_pm_opp_put(opp); - rk3399_devfreq_dmc_profile.initial_freq = data->rate; + data->profile = (struct devfreq_dev_profile) { + .polling_ms = 200, + .target = rk3399_dmcfreq_target, + .get_dev_status = rk3399_dmcfreq_get_dev_status, + .get_cur_freq = rk3399_dmcfreq_get_cur_freq, + .initial_freq = data->rate, + }; data->devfreq = devm_devfreq_add_device(dev, - &rk3399_devfreq_dmc_profile, + &data->profile, DEVFREQ_GOV_SIMPLE_ONDEMAND, &data->ondemand_data); if (IS_ERR(data->devfreq)) { -- cgit v1.2.3 From 13966517d41efb0a549544022db7d522503bc718 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Fri, 1 Apr 2022 16:11:23 +0200 Subject: PM: runtime: Allow to call __pm_runtime_set_status() from atomic context The only two users of __pm_runtime_set_status() are pm_runtime_set_active() and pm_runtime_set_suspended(). These are widely used and should be called from non-atomic context to work as expected. However, it would be convenient to allow them be called from atomic context too, as shown from a subsequent change, so let's add support for this. Signed-off-by: Ulf Hansson Tested-by: Maulik Shah Signed-off-by: Rafael J. Wysocki --- drivers/base/power/runtime.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index d4059e6ffeae..91d757914686 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1210,12 +1210,13 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) { struct device *parent = dev->parent; bool notify_parent = false; + unsigned long flags; int error = 0; if (status != RPM_ACTIVE && status != RPM_SUSPENDED) return -EINVAL; - spin_lock_irq(&dev->power.lock); + spin_lock_irqsave(&dev->power.lock, flags); /* * Prevent PM-runtime from being enabled for the device or return an @@ -1226,7 +1227,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) else error = -EAGAIN; - spin_unlock_irq(&dev->power.lock); + spin_unlock_irqrestore(&dev->power.lock, flags); if (error) return error; @@ -1247,7 +1248,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) device_links_read_unlock(idx); } - spin_lock_irq(&dev->power.lock); + spin_lock_irqsave(&dev->power.lock, flags); if (dev->power.runtime_status == status || !parent) goto out_set; @@ -1288,7 +1289,7 @@ int __pm_runtime_set_status(struct device *dev, unsigned int status) dev->power.runtime_error = 0; out: - spin_unlock_irq(&dev->power.lock); + spin_unlock_irqrestore(&dev->power.lock, flags); if (notify_parent) pm_request_idle(parent); -- cgit v1.2.3 From 171b66e2e2e9d80b93c8cff799e6175074b22297 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Fri, 1 Apr 2022 16:11:24 +0200 Subject: cpuidle: PSCI: Improve support for suspend-to-RAM for PSCI OSI mode When PSCI OSI mode is supported the syscore flag is set for the CPU devices that becomes attached to their PM domains (genpds). In the suspend-to-idle case, we call dev_pm_genpd_suspend|resume() to allow genpd to properly manage the power-off/on operations (pick an idlestate and manage the on/off notifications). For suspend-to-ram, dev_pm_genpd_suspend|resume() is currently not being called, which causes a problem that the genpd on/off notifiers do not get sent as expected. This prevents the platform-specific operations from being executed, typically needed just before/after the boot CPU is being turned off/on. To deal with this problem, let's register a syscore ops for cpuidle-psci when PSCI OSI mode is being used and call dev_pm_genpd_suspend|resume() from them. In this way, genpd regains control of the PM domain topology and then sends the on/off notifications when it's appropriate. Reported-by: Maulik Shah Suggested-by: Maulik Shah Signed-off-by: Ulf Hansson Tested-by: Maulik Shah Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-psci.c | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) (limited to 'drivers') diff --git a/drivers/cpuidle/cpuidle-psci.c b/drivers/cpuidle/cpuidle-psci.c index b51b5df08450..540105ca0781 100644 --- a/drivers/cpuidle/cpuidle-psci.c +++ b/drivers/cpuidle/cpuidle-psci.c @@ -23,6 +23,7 @@ #include #include #include +#include #include @@ -131,6 +132,49 @@ static int psci_idle_cpuhp_down(unsigned int cpu) return 0; } +static void psci_idle_syscore_switch(bool suspend) +{ + bool cleared = false; + struct device *dev; + int cpu; + + for_each_possible_cpu(cpu) { + dev = per_cpu_ptr(&psci_cpuidle_data, cpu)->dev; + + if (dev && suspend) { + dev_pm_genpd_suspend(dev); + } else if (dev) { + dev_pm_genpd_resume(dev); + + /* Account for userspace having offlined a CPU. */ + if (pm_runtime_status_suspended(dev)) + pm_runtime_set_active(dev); + + /* Clear domain state to re-start fresh. */ + if (!cleared) { + psci_set_domain_state(0); + cleared = true; + } + } + } +} + +static int psci_idle_syscore_suspend(void) +{ + psci_idle_syscore_switch(true); + return 0; +} + +static void psci_idle_syscore_resume(void) +{ + psci_idle_syscore_switch(false); +} + +static struct syscore_ops psci_idle_syscore_ops = { + .suspend = psci_idle_syscore_suspend, + .resume = psci_idle_syscore_resume, +}; + static void psci_idle_init_cpuhp(void) { int err; @@ -138,6 +182,8 @@ static void psci_idle_init_cpuhp(void) if (!psci_cpuidle_use_cpuhp) return; + register_syscore_ops(&psci_idle_syscore_ops); + err = cpuhp_setup_state_nocalls(CPUHP_AP_CPU_PM_STARTING, "cpuidle/psci:online", psci_idle_cpuhp_up, -- cgit v1.2.3 From bd40cbb0e3b37a4d2a2d9e2ac40122cdf619b1f3 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Tue, 19 Apr 2022 19:29:16 +0200 Subject: PM: domains: Move genpd's time-accounting to ktime_get_mono_fast_ns() To move towards a more consistent behaviour between genpd and the runtime PM core, let's start by converting genpd's time-accounting from ktime_get() into ktime_get_mono_fast_ns(). Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 77 +++++++++++++++++++++++++-------------------- include/linux/pm_domain.h | 6 ++-- 2 files changed, 46 insertions(+), 37 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 1ee878d126fd..18cd7965e5d6 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -225,24 +225,23 @@ static void genpd_debug_remove(struct generic_pm_domain *genpd) static void genpd_update_accounting(struct generic_pm_domain *genpd) { - ktime_t delta, now; + u64 delta, now; - now = ktime_get(); - delta = ktime_sub(now, genpd->accounting_time); + now = ktime_get_mono_fast_ns(); + if (now <= genpd->accounting_time) + return; + + delta = now - genpd->accounting_time; /* * If genpd->status is active, it means we are just * out of off and so update the idle time and vice * versa. */ - if (genpd->status == GENPD_STATE_ON) { - int state_idx = genpd->state_idx; - - genpd->states[state_idx].idle_time = - ktime_add(genpd->states[state_idx].idle_time, delta); - } else { - genpd->on_time = ktime_add(genpd->on_time, delta); - } + if (genpd->status == GENPD_STATE_ON) + genpd->states[genpd->state_idx].idle_time += delta; + else + genpd->on_time += delta; genpd->accounting_time = now; } @@ -1999,7 +1998,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd, genpd->max_off_time_changed = true; genpd->provider = NULL; genpd->has_provider = false; - genpd->accounting_time = ktime_get(); + genpd->accounting_time = ktime_get_mono_fast_ns(); genpd->domain.ops.runtime_suspend = genpd_runtime_suspend; genpd->domain.ops.runtime_resume = genpd_runtime_resume; genpd->domain.ops.prepare = genpd_prepare; @@ -3163,6 +3162,7 @@ static int sub_domains_show(struct seq_file *s, void *data) static int idle_states_show(struct seq_file *s, void *data) { struct generic_pm_domain *genpd = s->private; + u64 now, delta, idle_time = 0; unsigned int i; int ret = 0; @@ -3173,17 +3173,19 @@ static int idle_states_show(struct seq_file *s, void *data) seq_puts(s, "State Time Spent(ms) Usage Rejected\n"); for (i = 0; i < genpd->state_count; i++) { - ktime_t delta = 0; - s64 msecs; + idle_time += genpd->states[i].idle_time; - if ((genpd->status == GENPD_STATE_OFF) && - (genpd->state_idx == i)) - delta = ktime_sub(ktime_get(), genpd->accounting_time); + if (genpd->status == GENPD_STATE_OFF && genpd->state_idx == i) { + now = ktime_get_mono_fast_ns(); + if (now > genpd->accounting_time) { + delta = now - genpd->accounting_time; + idle_time += delta; + } + } - msecs = ktime_to_ms( - ktime_add(genpd->states[i].idle_time, delta)); - seq_printf(s, "S%-13i %-14lld %-14llu %llu\n", i, msecs, - genpd->states[i].usage, genpd->states[i].rejected); + do_div(idle_time, NSEC_PER_MSEC); + seq_printf(s, "S%-13i %-14llu %-14llu %llu\n", i, idle_time, + genpd->states[i].usage, genpd->states[i].rejected); } genpd_unlock(genpd); @@ -3193,18 +3195,22 @@ static int idle_states_show(struct seq_file *s, void *data) static int active_time_show(struct seq_file *s, void *data) { struct generic_pm_domain *genpd = s->private; - ktime_t delta = 0; + u64 now, on_time, delta = 0; int ret = 0; ret = genpd_lock_interruptible(genpd); if (ret) return -ERESTARTSYS; - if (genpd->status == GENPD_STATE_ON) - delta = ktime_sub(ktime_get(), genpd->accounting_time); + if (genpd->status == GENPD_STATE_ON) { + now = ktime_get_mono_fast_ns(); + if (now > genpd->accounting_time) + delta = now - genpd->accounting_time; + } - seq_printf(s, "%lld ms\n", ktime_to_ms( - ktime_add(genpd->on_time, delta))); + on_time = genpd->on_time + delta; + do_div(on_time, NSEC_PER_MSEC); + seq_printf(s, "%llu ms\n", on_time); genpd_unlock(genpd); return ret; @@ -3213,7 +3219,7 @@ static int active_time_show(struct seq_file *s, void *data) static int total_idle_time_show(struct seq_file *s, void *data) { struct generic_pm_domain *genpd = s->private; - ktime_t delta = 0, total = 0; + u64 now, delta, total = 0; unsigned int i; int ret = 0; @@ -3222,16 +3228,19 @@ static int total_idle_time_show(struct seq_file *s, void *data) return -ERESTARTSYS; for (i = 0; i < genpd->state_count; i++) { + total += genpd->states[i].idle_time; - if ((genpd->status == GENPD_STATE_OFF) && - (genpd->state_idx == i)) - delta = ktime_sub(ktime_get(), genpd->accounting_time); - - total = ktime_add(total, genpd->states[i].idle_time); + if (genpd->status == GENPD_STATE_OFF && genpd->state_idx == i) { + now = ktime_get_mono_fast_ns(); + if (now > genpd->accounting_time) { + delta = now - genpd->accounting_time; + total += delta; + } + } } - total = ktime_add(total, delta); - seq_printf(s, "%lld ms\n", ktime_to_ms(total)); + do_div(total, NSEC_PER_MSEC); + seq_printf(s, "%llu ms\n", total); genpd_unlock(genpd); return ret; diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 67017c9390c8..043d48e4420a 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -98,7 +98,7 @@ struct genpd_power_state { u64 usage; u64 rejected; struct fwnode_handle *fwnode; - ktime_t idle_time; + u64 idle_time; void *data; }; @@ -149,8 +149,8 @@ struct generic_pm_domain { unsigned int state_count); unsigned int state_count; /* number of states */ unsigned int state_idx; /* state that genpd will go to when off */ - ktime_t on_time; - ktime_t accounting_time; + u64 on_time; + u64 accounting_time; const struct genpd_lock_ops *lock_ops; union { struct mutex mlock; -- cgit v1.2.3 From ae0dc7ed1a7c713ee9ba563a328d3b4d59223d7c Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Thu, 21 Apr 2022 23:07:36 +0800 Subject: powercap: intel_rapl: add support for RaptorLake Add intel_rapl support for the RaptorLake platform. Signed-off-by: Zhang Rui Signed-off-by: Rafael J. Wysocki --- drivers/powercap/intel_rapl_common.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c index 07611a00b78f..83da499dd6ec 100644 --- a/drivers/powercap/intel_rapl_common.c +++ b/drivers/powercap/intel_rapl_common.c @@ -1107,6 +1107,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(ROCKETLAKE, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &rapl_defaults_core), + X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &rapl_defaults_spr_server), X86_MATCH_INTEL_FAM6_MODEL(LAKEFIELD, &rapl_defaults_core), -- cgit v1.2.3 From f346e96267cd76175d6c201b40f770c0116a8a04 Mon Sep 17 00:00:00 2001 From: Schspa Shi Date: Thu, 21 Apr 2022 03:15:41 +0800 Subject: cpufreq: Fix possible race in cpufreq online error path When cpufreq online fails, the policy->cpus mask is not cleared and policy->rwsem is released too early, so the driver can be invoked via the cpuinfo_cur_freq sysfs attribute while its ->offline() or ->exit() callbacks are being run. Take policy->clk as an example: static int cpufreq_online(unsigned int cpu) { ... // policy->cpus != 0 at this time down_write(&policy->rwsem); ret = cpufreq_add_dev_interface(policy); up_write(&policy->rwsem); return 0; out_destroy_policy: for_each_cpu(j, policy->real_cpus) remove_cpu_dev_symlink(policy, get_cpu_device(j)); up_write(&policy->rwsem); ... out_exit_policy: if (cpufreq_driver->exit) cpufreq_driver->exit(policy); clk_put(policy->clk); // policy->clk is a wild pointer ... ^ | Another process access __cpufreq_get cpufreq_verify_current_freq cpufreq_generic_get // acces wild pointer of policy->clk; | | out_offline_policy: | cpufreq_policy_free(policy); | // deleted here, and will wait for no body reference cpufreq_policy_put_kobj(policy); } Address this by modifying cpufreq_online() to release policy->rwsem in the error path after the driver callbacks have run and to clear policy->cpus before releasing the semaphore. Fixes: 7106e02baed4 ("cpufreq: release policy->rwsem on error") Signed-off-by: Schspa Shi [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 80f535cc8a75..0d58b0f8f3af 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1533,8 +1533,6 @@ out_destroy_policy: for_each_cpu(j, policy->real_cpus) remove_cpu_dev_symlink(policy, get_cpu_device(j)); - up_write(&policy->rwsem); - out_offline_policy: if (cpufreq_driver->offline) cpufreq_driver->offline(policy); @@ -1543,6 +1541,9 @@ out_exit_policy: if (cpufreq_driver->exit) cpufreq_driver->exit(policy); + cpumask_clear(policy->cpus); + up_write(&policy->rwsem); + out_free_policy: cpufreq_policy_free(policy); return ret; -- cgit v1.2.3 From d1cf8bbfed1edc5108220342ab39e4544d55fbc3 Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Fri, 15 Apr 2022 17:39:51 +0800 Subject: intel_idle: Add AlderLake support Similar to SPR, the C1 and C1E states on ADL are mutually exclusive. Only one of them can be enabled at a time. But contrast to SPR, which usually has a strong latency requirement as a Xeon processor, C1E is preferred on ADL for better energy efficiency. Add custom C-state tables for ADL with both C1 and C1E, and 1. Enable the "C1E promotion" bit in MSR_IA32_POWER_CTL and mark C1 with the CPUIDLE_FLAG_UNUSABLE flag, so C1 is not available by default. 2. Add support for the "preferred_cstates" module parameter, so that users can choose to use C1 instead of C1E by booting with "intel_idle.preferred_cstates=2". Separate custom C-state tables are introduced for the ADL mobile and desktop processors, because of the exit latency differences between these two variants, especially with respect to PC10. Signed-off-by: Zhang Rui [ rjw: Changelog edits, code rearrangement ] Signed-off-by: Rafael J. Wysocki --- drivers/idle/intel_idle.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) (limited to 'drivers') diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c index 47551ab73ca8..b9bb94bd0f67 100644 --- a/drivers/idle/intel_idle.c +++ b/drivers/idle/intel_idle.c @@ -764,6 +764,106 @@ static struct cpuidle_state icx_cstates[] __initdata = { .enter = NULL } }; +/* + * On AlderLake C1 has to be disabled if C1E is enabled, and vice versa. + * C1E is enabled only if "C1E promotion" bit is set in MSR_IA32_POWER_CTL. + * But in this case there is effectively no C1, because C1 requests are + * promoted to C1E. If the "C1E promotion" bit is cleared, then both C1 + * and C1E requests end up with C1, so there is effectively no C1E. + * + * By default we enable C1E and disable C1 by marking it with + * 'CPUIDLE_FLAG_UNUSABLE'. + */ +static struct cpuidle_state adl_cstates[] __initdata = { + { + .name = "C1", + .desc = "MWAIT 0x00", + .flags = MWAIT2flg(0x00) | CPUIDLE_FLAG_UNUSABLE, + .exit_latency = 1, + .target_residency = 1, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C1E", + .desc = "MWAIT 0x01", + .flags = MWAIT2flg(0x01) | CPUIDLE_FLAG_ALWAYS_ENABLE, + .exit_latency = 2, + .target_residency = 4, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C6", + .desc = "MWAIT 0x20", + .flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED, + .exit_latency = 220, + .target_residency = 600, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C8", + .desc = "MWAIT 0x40", + .flags = MWAIT2flg(0x40) | CPUIDLE_FLAG_TLB_FLUSHED, + .exit_latency = 280, + .target_residency = 800, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C10", + .desc = "MWAIT 0x60", + .flags = MWAIT2flg(0x60) | CPUIDLE_FLAG_TLB_FLUSHED, + .exit_latency = 680, + .target_residency = 2000, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .enter = NULL } +}; + +static struct cpuidle_state adl_l_cstates[] __initdata = { + { + .name = "C1", + .desc = "MWAIT 0x00", + .flags = MWAIT2flg(0x00) | CPUIDLE_FLAG_UNUSABLE, + .exit_latency = 1, + .target_residency = 1, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C1E", + .desc = "MWAIT 0x01", + .flags = MWAIT2flg(0x01) | CPUIDLE_FLAG_ALWAYS_ENABLE, + .exit_latency = 2, + .target_residency = 4, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C6", + .desc = "MWAIT 0x20", + .flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED, + .exit_latency = 170, + .target_residency = 500, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C8", + .desc = "MWAIT 0x40", + .flags = MWAIT2flg(0x40) | CPUIDLE_FLAG_TLB_FLUSHED, + .exit_latency = 200, + .target_residency = 600, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .name = "C10", + .desc = "MWAIT 0x60", + .flags = MWAIT2flg(0x60) | CPUIDLE_FLAG_TLB_FLUSHED, + .exit_latency = 230, + .target_residency = 700, + .enter = &intel_idle, + .enter_s2idle = intel_idle_s2idle, }, + { + .enter = NULL } +}; + /* * On Sapphire Rapids Xeon C1 has to be disabled if C1E is enabled, and vice * versa. On SPR C1E is enabled only if "C1E promotion" bit is set in @@ -1147,6 +1247,14 @@ static const struct idle_cpu idle_cpu_icx __initconst = { .use_acpi = true, }; +static const struct idle_cpu idle_cpu_adl __initconst = { + .state_table = adl_cstates, +}; + +static const struct idle_cpu idle_cpu_adl_l __initconst = { + .state_table = adl_l_cstates, +}; + static const struct idle_cpu idle_cpu_spr __initconst = { .state_table = spr_cstates, .disable_promotion_to_c1e = true, @@ -1215,6 +1323,8 @@ static const struct x86_cpu_id intel_idle_ids[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, &idle_cpu_skx), X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_X, &idle_cpu_icx), X86_MATCH_INTEL_FAM6_MODEL(ICELAKE_D, &idle_cpu_icx), + X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &idle_cpu_adl), + X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &idle_cpu_adl_l), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &idle_cpu_spr), X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNL, &idle_cpu_knl), X86_MATCH_INTEL_FAM6_MODEL(XEON_PHI_KNM, &idle_cpu_knl), @@ -1573,6 +1683,25 @@ static void __init skx_idle_state_table_update(void) } } +/** + * adl_idle_state_table_update - Adjust AlderLake idle states table. + */ +static void __init adl_idle_state_table_update(void) +{ + /* Check if user prefers C1 over C1E. */ + if (preferred_states_mask & BIT(1) && !(preferred_states_mask & BIT(2))) { + cpuidle_state_table[0].flags &= ~CPUIDLE_FLAG_UNUSABLE; + cpuidle_state_table[1].flags |= CPUIDLE_FLAG_UNUSABLE; + + /* Disable C1E by clearing the "C1E promotion" bit. */ + c1e_promotion = C1E_PROMOTION_DISABLE; + return; + } + + /* Make sure C1E is enabled by default */ + c1e_promotion = C1E_PROMOTION_ENABLE; +} + /** * spr_idle_state_table_update - Adjust Sapphire Rapids idle states table. */ @@ -1642,6 +1771,10 @@ static void __init intel_idle_init_cstates_icpu(struct cpuidle_driver *drv) case INTEL_FAM6_SAPPHIRERAPIDS_X: spr_idle_state_table_update(); break; + case INTEL_FAM6_ALDERLAKE: + case INTEL_FAM6_ALDERLAKE_L: + adl_idle_state_table_update(); + break; } for (cstate = 0; cstate < CPUIDLE_STATE_MAX; ++cstate) { -- cgit v1.2.3 From f55ae08c89873e140c7cac2a7fa161d31a0d60cf Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 4 May 2022 13:51:35 +0530 Subject: cpufreq: Avoid unnecessary frequency updates due to mismatch For some platforms, the frequency returned by hardware may be slightly different from what is provided in the frequency table. For example, hardware may return 499 MHz instead of 500 MHz. In such cases it is better to avoid getting into unnecessary frequency updates, as we may end up switching policy->cur between the two and sending unnecessary pre/post update notifications, etc. This patch has chosen allows the hardware frequency and table frequency to deviate by 1 MHz for now, we may want to increase it a bit later on if someone still complains. Reported-by: Rex-BC Chen Signed-off-by: Viresh Kumar Tested-by: Jia-wei Chang Reviewed-by: Matthias Brugger Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 0d58b0f8f3af..233e8af48848 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -28,6 +28,7 @@ #include #include #include +#include #include static LIST_HEAD(cpufreq_policy_list); @@ -1708,6 +1709,16 @@ static unsigned int cpufreq_verify_current_freq(struct cpufreq_policy *policy, b return new_freq; if (policy->cur != new_freq) { + /* + * For some platforms, the frequency returned by hardware may be + * slightly different from what is provided in the frequency + * table, for example hardware may return 499 MHz instead of 500 + * MHz. In such cases it is better to avoid getting into + * unnecessary frequency updates. + */ + if (abs(policy->cur - new_freq) < HZ_PER_MHZ) + return policy->cur; + cpufreq_out_of_sync(policy, new_freq); if (update) schedule_work(&policy->update); -- cgit v1.2.3 From d3c3db41df7e1bdefc9c68073070b62ce3b260bd Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Mon, 25 Apr 2022 14:38:07 +0200 Subject: cpufreq: CPPC: Add per_cpu efficiency_class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In ACPI, describing power efficiency of CPUs can be done through the following arm specific field: ACPI 6.4, s5.2.12.14 'GIC CPU Interface (GICC) Structure', 'Processor Power Efficiency Class field': Describes the relative power efficiency of the associated pro- cessor. Lower efficiency class numbers are more efficient than higher ones (e.g. efficiency class 0 should be treated as more efficient than efficiency class 1). However, absolute values of this number have no meaning: 2 isn’t necessarily half as efficient as 1. The efficiency_class field is stored in the GicC structure of the ACPI MADT table and it's currently supported in Linux for arm64 only. Thus, this new functionality is introduced for arm64 only. To allow the cppc_cpufreq driver to know and preprocess the efficiency_class values of all the CPUs, add a per_cpu efficiency_class variable to store them. At least 2 different efficiency classes must be present, otherwise there is no use in creating an Energy Model. The efficiency_class values are squeezed in [0:#efficiency_class-1] while conserving the order. For instance, efficiency classes of: [111, 212, 250] will be mapped to: [0 (was 111), 1 (was 212), 2 (was 250)]. Each policy being independently registered in the driver, populating the per_cpu efficiency_class is done only once at the driver initialization. This prevents from having each policy re-searching the efficiency_class values of other CPUs. The EM will be registered in a following patch. The patch also exports acpi_cpu_get_madt_gicc() to fetch the GicC structure of the ACPI MADT table for each CPU. Acked-by: Catalin Marinas Signed-off-by: Pierre Gondois Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- arch/arm64/kernel/smp.c | 1 + drivers/cpufreq/cppc_cpufreq.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) (limited to 'drivers') diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index 3b46041f2b97..62ed361a4376 100644 --- a/arch/arm64/kernel/smp.c +++ b/arch/arm64/kernel/smp.c @@ -512,6 +512,7 @@ struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu) { return &cpu_madt_gicc[cpu]; } +EXPORT_SYMBOL_GPL(acpi_cpu_get_madt_gicc); /* * acpi_map_gic_cpu_interface - parse processor MADT entry diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 82d370ae6a4a..3cd05651707d 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -420,12 +420,53 @@ static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu) return cppc_get_transition_latency(cpu) / NSEC_PER_USEC; } +static DEFINE_PER_CPU(unsigned int, efficiency_class); + +static int populate_efficiency_class(void) +{ + struct acpi_madt_generic_interrupt *gicc; + DECLARE_BITMAP(used_classes, 256) = {}; + int class, cpu, index; + + for_each_possible_cpu(cpu) { + gicc = acpi_cpu_get_madt_gicc(cpu); + class = gicc->efficiency_class; + bitmap_set(used_classes, class, 1); + } + + if (bitmap_weight(used_classes, 256) <= 1) { + pr_debug("Efficiency classes are all equal (=%d). " + "No EM registered", class); + return -EINVAL; + } + + /* + * Squeeze efficiency class values on [0:#efficiency_class-1]. + * Values are per spec in [0:255]. + */ + index = 0; + for_each_set_bit(class, used_classes, 256) { + for_each_possible_cpu(cpu) { + gicc = acpi_cpu_get_madt_gicc(cpu); + if (gicc->efficiency_class == class) + per_cpu(efficiency_class, cpu) = index; + } + index++; + } + + return 0; +} + #else static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu) { return cppc_get_transition_latency(cpu) / NSEC_PER_USEC; } +static int populate_efficiency_class(void) +{ + return 0; +} #endif @@ -742,6 +783,7 @@ static int __init cppc_cpufreq_init(void) cppc_check_hisi_workaround(); cppc_freq_invariance_init(); + populate_efficiency_class(); ret = cpufreq_register_driver(&cppc_cpufreq_driver); if (ret) -- cgit v1.2.3 From 740fcdc2c20ecf855b36b919d7fa1b872b5a7eae Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Mon, 25 Apr 2022 14:38:08 +0200 Subject: cpufreq: CPPC: Register EM based on efficiency class information Performance states and energy consumption values are not advertised in ACPI. In the GicC structure of the MADT table, the "Processor Power Efficiency Class field" (called efficiency class from now) allows to describe the relative energy efficiency of CPUs. To leverage the EM and EAS, the CPPC driver creates a set of artificial performance states and registers them in the Energy Model (EM), such as: - Every 20 capacity unit, a performance state is created. - The energy cost of each performance state gradually increases. No power value is generated as only the cost is used in the EM. During task placement, a task can raise the frequency of its whole pd. This can make EAS place a task on a pd with CPUs that are individually less energy efficient. As cost values are artificial, and to place tasks on CPUs with the lower efficiency class, a gap in cost values is generated for adjacent efficiency classes. E.g.: - efficiency class = 0, capacity is in [0-1024], so cost values are in [0: 51] (one performance state every 20 capacity unit) - efficiency class = 1, capacity is in [0-1024], cost values are in [1*gap+0: 1*gap+51]. The value of the cost gap is chosen to absorb a the energy of 4 CPUs at their maximum capacity. This means that between: 1- a pd of 4 CPUs, each of them being used at almost their full capacity. Their efficiency class is N. 2- a CPU using almost none of its capacity. Its efficiency class is N+1 EAS will choose the first option. This patch also populates the (struct cpufreq_driver).register_em callback if the valid efficiency_class ACPI values are provided. Signed-off-by: Pierre Gondois Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cppc_cpufreq.c | 144 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 144 insertions(+) (limited to 'drivers') diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 3cd05651707d..3eaa23d1aaf5 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -421,6 +421,134 @@ static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu) } static DEFINE_PER_CPU(unsigned int, efficiency_class); +static void cppc_cpufreq_register_em(struct cpufreq_policy *policy); + +/* Create an artificial performance state every CPPC_EM_CAP_STEP capacity unit. */ +#define CPPC_EM_CAP_STEP (20) +/* Increase the cost value by CPPC_EM_COST_STEP every performance state. */ +#define CPPC_EM_COST_STEP (1) +/* Add a cost gap correspnding to the energy of 4 CPUs. */ +#define CPPC_EM_COST_GAP (4 * SCHED_CAPACITY_SCALE * CPPC_EM_COST_STEP \ + / CPPC_EM_CAP_STEP) + +static unsigned int get_perf_level_count(struct cpufreq_policy *policy) +{ + struct cppc_perf_caps *perf_caps; + unsigned int min_cap, max_cap; + struct cppc_cpudata *cpu_data; + int cpu = policy->cpu; + + cpu_data = policy->driver_data; + perf_caps = &cpu_data->perf_caps; + max_cap = arch_scale_cpu_capacity(cpu); + min_cap = div_u64(max_cap * perf_caps->lowest_perf, perf_caps->highest_perf); + if ((min_cap == 0) || (max_cap < min_cap)) + return 0; + return 1 + max_cap / CPPC_EM_CAP_STEP - min_cap / CPPC_EM_CAP_STEP; +} + +/* + * The cost is defined as: + * cost = power * max_frequency / frequency + */ +static inline unsigned long compute_cost(int cpu, int step) +{ + return CPPC_EM_COST_GAP * per_cpu(efficiency_class, cpu) + + step * CPPC_EM_COST_STEP; +} + +static int cppc_get_cpu_power(struct device *cpu_dev, + unsigned long *power, unsigned long *KHz) +{ + unsigned long perf_step, perf_prev, perf, perf_check; + unsigned int min_step, max_step, step, step_check; + unsigned long prev_freq = *KHz; + unsigned int min_cap, max_cap; + struct cpufreq_policy *policy; + + struct cppc_perf_caps *perf_caps; + struct cppc_cpudata *cpu_data; + + policy = cpufreq_cpu_get_raw(cpu_dev->id); + cpu_data = policy->driver_data; + perf_caps = &cpu_data->perf_caps; + max_cap = arch_scale_cpu_capacity(cpu_dev->id); + min_cap = div_u64(max_cap * perf_caps->lowest_perf, + perf_caps->highest_perf); + + perf_step = CPPC_EM_CAP_STEP * perf_caps->highest_perf / max_cap; + min_step = min_cap / CPPC_EM_CAP_STEP; + max_step = max_cap / CPPC_EM_CAP_STEP; + + perf_prev = cppc_cpufreq_khz_to_perf(cpu_data, *KHz); + step = perf_prev / perf_step; + + if (step > max_step) + return -EINVAL; + + if (min_step == max_step) { + step = max_step; + perf = perf_caps->highest_perf; + } else if (step < min_step) { + step = min_step; + perf = perf_caps->lowest_perf; + } else { + step++; + if (step == max_step) + perf = perf_caps->highest_perf; + else + perf = step * perf_step; + } + + *KHz = cppc_cpufreq_perf_to_khz(cpu_data, perf); + perf_check = cppc_cpufreq_khz_to_perf(cpu_data, *KHz); + step_check = perf_check / perf_step; + + /* + * To avoid bad integer approximation, check that new frequency value + * increased and that the new frequency will be converted to the + * desired step value. + */ + while ((*KHz == prev_freq) || (step_check != step)) { + perf++; + *KHz = cppc_cpufreq_perf_to_khz(cpu_data, perf); + perf_check = cppc_cpufreq_khz_to_perf(cpu_data, *KHz); + step_check = perf_check / perf_step; + } + + /* + * With an artificial EM, only the cost value is used. Still the power + * is populated such as 0 < power < EM_MAX_POWER. This allows to add + * more sense to the artificial performance states. + */ + *power = compute_cost(cpu_dev->id, step); + + return 0; +} + +static int cppc_get_cpu_cost(struct device *cpu_dev, unsigned long KHz, + unsigned long *cost) +{ + unsigned long perf_step, perf_prev; + struct cppc_perf_caps *perf_caps; + struct cpufreq_policy *policy; + struct cppc_cpudata *cpu_data; + unsigned int max_cap; + int step; + + policy = cpufreq_cpu_get_raw(cpu_dev->id); + cpu_data = policy->driver_data; + perf_caps = &cpu_data->perf_caps; + max_cap = arch_scale_cpu_capacity(cpu_dev->id); + + perf_prev = cppc_cpufreq_khz_to_perf(cpu_data, KHz); + perf_step = CPPC_EM_CAP_STEP * perf_caps->highest_perf / max_cap; + step = perf_prev / perf_step; + + *cost = compute_cost(cpu_dev->id, step); + + return 0; +} static int populate_efficiency_class(void) { @@ -453,10 +581,23 @@ static int populate_efficiency_class(void) } index++; } + cppc_cpufreq_driver.register_em = cppc_cpufreq_register_em; return 0; } +static void cppc_cpufreq_register_em(struct cpufreq_policy *policy) +{ + struct cppc_cpudata *cpu_data; + struct em_data_callback em_cb = + EM_ADV_DATA_CB(cppc_get_cpu_power, cppc_get_cpu_cost); + + cpu_data = policy->driver_data; + em_dev_register_perf_domain(get_cpu_device(policy->cpu), + get_perf_level_count(policy), &em_cb, + cpu_data->shared_cpu_map, 0); +} + #else static unsigned int cppc_cpufreq_get_transition_delay_us(unsigned int cpu) @@ -467,6 +608,9 @@ static int populate_efficiency_class(void) { return 0; } +static void cppc_cpufreq_register_em(struct cpufreq_policy *policy) +{ +} #endif -- cgit v1.2.3 From defec178df76e0caadd4e8ef68f3d655a2088198 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 5 Apr 2022 18:48:41 -0700 Subject: soc: rockchip: power-domain: Manage resource conflicts with firmware On RK3399 platforms, power domains are managed mostly by the kernel (drivers/soc/rockchip/pm_domains.c), but there are a few exceptions where ARM Trusted Firmware has to be involved: (1) system suspend/resume (2) DRAM DVFS (a.k.a., "ddrfreq") Exception (1) does not cause much conflict, since the kernel has quiesced itself by the time we make the relevant PSCI call. Exception (2) can cause conflict, because of two actions: (a) ARM Trusted Firmware needs to read/modify/write the PMU_BUS_IDLE_REQ register to idle the memory controller domain; the kernel driver also has to touch this register for other domains. (b) ARM Trusted Firmware needs to manage the clocks associated with these domains. To elaborate on (b): idling a power domain has always required ungating an array of clocks; see this old explanation from Rockchip: https://lore.kernel.org/linux-arm-kernel/54503C19.9060607@rock-chips.com/ Historically, ARM Trusted Firmware has avoided this issue by using a special PMU_CRU_GATEDIS_CON0 register -- this register ungates all the necessary clocks -- when idling the memory controller. Unfortunately, we've found that this register is not 100% sufficient; it does not turn the relevant PLLs on [0]. So it's possible to trigger issues with something like the following: 1. enable a power domain (e.g., RK3399_PD_VDU) -- kernel will temporarily enable relevant clocks/PLLs, then turn them back off 2. a PLL (e.g., PLL_NPLL) is part of the clock tree for RK3399_PD_VDU's clocks but otherwise unused; NPLL is disabled 3. perform a ddrfreq transition (rk3399_dmcfreq_target() -> ... drivers/clk/rockchip/clk-ddr.c / ROCKCHIP_SIP_DRAM_FREQ) 4. ARM Trusted Firmware unagates VDU clocks (via PMU_CRU_GATEDIS_CON0) 5. ARM Trusted firmware idles the memory controller domain 6. Step 5 waits on the VDU domain/clocks, but NPLL is still off i.e., we hang the system. So for (b), we need to at a minimum manage the relevant PLLs on behalf of firmware. It's easier to simply manage the whole clock tree, in a similar way we do in rockchip_pd_power(). For (a), we need to provide mutual exclusion betwen rockchip_pd_power() and firmware. To resolve that, we simply grab the PMU mutex and release it when ddrfreq is done. The Chromium OS kernel has been carrying versions of part of this hack for a while, based on some new custom notifiers [1]. I've rewritten as a simple function call between the drivers, which is OK because: * the PMU driver isn't enabled, and we don't have this problem at all (the firmware should have left us in an OK state, and there are no runtime conflicts); or * the PMU driver is present, and is a single instance. And the power-domain driver cannot be removed, so there's no lifetime management to worry about. For completeness, there's a 'dmc_pmu_mutex' to guard (likely theoretical?) probe()-time races. It's OK for the memory controller driver to start running before the PMU, because the PMU will avoid any critical actions during the block() sequence. [0] The RK3399 TRM for PMU_CRU_GATEDIS_CON0 only talks about ungating clocks. Based on experimentation, we've found that it does not power up the necessary PLLs. [1] CHROMIUM: soc: rockchip: power-domain: Add notifier to dmc driver https://chromium-review.googlesource.com/q/I242dbd706d352f74ff706f5cbf42ebb92f9bcc60 Notably, the Chromium solution only handled conflict (a), not (b). In practice, item (b) wasn't a problem in many cases because we never managed to fully power off PLLs. Now that the (upstream) video decoder driver performs runtime clock management, we often power off NPLL. Signed-off-by: Brian Norris Tested-by: Peter Geis Reviewed-by: Heiko Stuebner Signed-off-by: Chanwoo Choi --- drivers/soc/rockchip/pm_domains.c | 118 ++++++++++++++++++++++++++++++++++++++ include/soc/rockchip/pm_domains.h | 25 ++++++++ 2 files changed, 143 insertions(+) create mode 100644 include/soc/rockchip/pm_domains.h (limited to 'drivers') diff --git a/drivers/soc/rockchip/pm_domains.c b/drivers/soc/rockchip/pm_domains.c index 0868b7d406fb..b1cf7d29dafd 100644 --- a/drivers/soc/rockchip/pm_domains.c +++ b/drivers/soc/rockchip/pm_domains.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -16,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -139,6 +141,109 @@ struct rockchip_pmu { #define DOMAIN_RK3568(name, pwr, req, wakeup) \ DOMAIN_M(name, pwr, pwr, req, req, req, wakeup) +/* + * Dynamic Memory Controller may need to coordinate with us -- see + * rockchip_pmu_block(). + * + * dmc_pmu_mutex protects registration-time races, so DMC driver doesn't try to + * block() while we're initializing the PMU. + */ +static DEFINE_MUTEX(dmc_pmu_mutex); +static struct rockchip_pmu *dmc_pmu; + +/* + * Block PMU transitions and make sure they don't interfere with ARM Trusted + * Firmware operations. There are two conflicts, noted in the comments below. + * + * Caller must unblock PMU transitions via rockchip_pmu_unblock(). + */ +int rockchip_pmu_block(void) +{ + struct rockchip_pmu *pmu; + struct generic_pm_domain *genpd; + struct rockchip_pm_domain *pd; + int i, ret; + + mutex_lock(&dmc_pmu_mutex); + + /* No PMU (yet)? Then we just block rockchip_pmu_probe(). */ + if (!dmc_pmu) + return 0; + pmu = dmc_pmu; + + /* + * mutex blocks all idle transitions: we can't touch the + * PMU_BUS_IDLE_REQ (our ".idle_offset") register while ARM Trusted + * Firmware might be using it. + */ + mutex_lock(&pmu->mutex); + + /* + * Power domain clocks: Per Rockchip, we *must* keep certain clocks + * enabled for the duration of power-domain transitions. Most + * transitions are handled by this driver, but some cases (in + * particular, DRAM DVFS / memory-controller idle) must be handled by + * firmware. Firmware can handle most clock management via a special + * "ungate" register (PMU_CRU_GATEDIS_CON0), but unfortunately, this + * doesn't handle PLLs. We can assist this transition by doing the + * clock management on behalf of firmware. + */ + for (i = 0; i < pmu->genpd_data.num_domains; i++) { + genpd = pmu->genpd_data.domains[i]; + if (genpd) { + pd = to_rockchip_pd(genpd); + ret = clk_bulk_enable(pd->num_clks, pd->clks); + if (ret < 0) { + dev_err(pmu->dev, + "failed to enable clks for domain '%s': %d\n", + genpd->name, ret); + goto err; + } + } + } + + return 0; + +err: + for (i = i - 1; i >= 0; i--) { + genpd = pmu->genpd_data.domains[i]; + if (genpd) { + pd = to_rockchip_pd(genpd); + clk_bulk_disable(pd->num_clks, pd->clks); + } + } + mutex_unlock(&pmu->mutex); + mutex_unlock(&dmc_pmu_mutex); + + return ret; +} +EXPORT_SYMBOL_GPL(rockchip_pmu_block); + +/* Unblock PMU transitions. */ +void rockchip_pmu_unblock(void) +{ + struct rockchip_pmu *pmu; + struct generic_pm_domain *genpd; + struct rockchip_pm_domain *pd; + int i; + + if (dmc_pmu) { + pmu = dmc_pmu; + for (i = 0; i < pmu->genpd_data.num_domains; i++) { + genpd = pmu->genpd_data.domains[i]; + if (genpd) { + pd = to_rockchip_pd(genpd); + clk_bulk_disable(pd->num_clks, pd->clks); + } + } + + mutex_unlock(&pmu->mutex); + } + + mutex_unlock(&dmc_pmu_mutex); +} +EXPORT_SYMBOL_GPL(rockchip_pmu_unblock); + static bool rockchip_pmu_domain_is_idle(struct rockchip_pm_domain *pd) { struct rockchip_pmu *pmu = pd->pmu; @@ -690,6 +795,12 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev) error = -ENODEV; + /* + * Prevent any rockchip_pmu_block() from racing with the remainder of + * setup (clocks, register initialization). + */ + mutex_lock(&dmc_pmu_mutex); + for_each_available_child_of_node(np, node) { error = rockchip_pm_add_one_domain(pmu, node); if (error) { @@ -719,10 +830,17 @@ static int rockchip_pm_domain_probe(struct platform_device *pdev) goto err_out; } + /* We only expect one PMU. */ + if (!WARN_ON_ONCE(dmc_pmu)) + dmc_pmu = pmu; + + mutex_unlock(&dmc_pmu_mutex); + return 0; err_out: rockchip_pm_domain_cleanup(pmu); + mutex_unlock(&dmc_pmu_mutex); return error; } diff --git a/include/soc/rockchip/pm_domains.h b/include/soc/rockchip/pm_domains.h new file mode 100644 index 000000000000..7dbd941fc937 --- /dev/null +++ b/include/soc/rockchip/pm_domains.h @@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright 2022, The Chromium OS Authors. All rights reserved. + */ + +#ifndef __SOC_ROCKCHIP_PM_DOMAINS_H__ +#define __SOC_ROCKCHIP_PM_DOMAINS_H__ + +#ifdef CONFIG_ROCKCHIP_PM_DOMAINS + +int rockchip_pmu_block(void); +void rockchip_pmu_unblock(void); + +#else /* CONFIG_ROCKCHIP_PM_DOMAINS */ + +static inline int rockchip_pmu_block(void) +{ + return 0; +} + +static inline void rockchip_pmu_unblock(void) { } + +#endif /* CONFIG_ROCKCHIP_PM_DOMAINS */ + +#endif /* __SOC_ROCKCHIP_PM_DOMAINS_H__ */ -- cgit v1.2.3 From 2e691421a2c9e0462175fe98171afa632861b199 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Tue, 5 Apr 2022 18:48:42 -0700 Subject: PM / devfreq: rk3399_dmc: Block PMU during transitions See the previous patch ("soc: rockchip: power-domain: Manage resource conflicts with firmware") for a thorough explanation of the conflicts. While ARM Trusted Firmware may be modifying memory controller and power-domain states, we need to block the kernel's power-domain driver. If the power-domain driver is disabled, there is no resource conflict and this becomes a no-op. Signed-off-by: Brian Norris Reviewed-by: Douglas Anderson Acked-by: Chanwoo Choi Signed-off-by: Chanwoo Choi --- drivers/devfreq/rk3399_dmc.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'drivers') diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c index e494d1497d60..daff40702615 100644 --- a/drivers/devfreq/rk3399_dmc.c +++ b/drivers/devfreq/rk3399_dmc.c @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -93,6 +94,16 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, mutex_lock(&dmcfreq->lock); + /* + * Ensure power-domain transitions don't interfere with ARM Trusted + * Firmware power-domain idling. + */ + err = rockchip_pmu_block(); + if (err) { + dev_err(dev, "Failed to block PMU: %d\n", err); + goto out_unlock; + } + /* * Some idle parameters may be based on the DDR controller clock, which * is half of the DDR frequency. @@ -198,6 +209,8 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq, dmcfreq->volt = target_volt; out: + rockchip_pmu_unblock(); +out_unlock: mutex_unlock(&dmcfreq->lock); return err; } -- cgit v1.2.3 From 85f0e42bd65d01b351d561efb38e584d4c596553 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Mon, 9 May 2022 09:27:37 +0530 Subject: Revert "cpufreq: Fix possible race in cpufreq online error path" This reverts commit f346e96267cd76175d6c201b40f770c0116a8a04. The commit tried to fix a possible real bug but it made it even worse. The fix was simply buggy as now an error out to out_offline_policy or out_exit_policy will try to release a semaphore which was never taken in the first place. This works fine only if we failed late, i.e. via out_destroy_policy. Fixes: f346e96267cd ("cpufreq: Fix possible race in cpufreq online error path") Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 233e8af48848..fbaa8e6c7d23 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1534,6 +1534,8 @@ out_destroy_policy: for_each_cpu(j, policy->real_cpus) remove_cpu_dev_symlink(policy, get_cpu_device(j)); + up_write(&policy->rwsem); + out_offline_policy: if (cpufreq_driver->offline) cpufreq_driver->offline(policy); @@ -1542,9 +1544,6 @@ out_exit_policy: if (cpufreq_driver->exit) cpufreq_driver->exit(policy); - cpumask_clear(policy->cpus); - up_write(&policy->rwsem); - out_free_policy: cpufreq_policy_free(policy); return ret; -- cgit v1.2.3 From bbd67f1b5a9449b090560ca5288aef0cfe623275 Mon Sep 17 00:00:00 2001 From: Srinivas Pandruvada Date: Mon, 2 May 2022 13:50:52 -0700 Subject: cpufreq: intel_pstate: Support Sapphire Rapids OOB mode Prevent intel_pstate to load when OOB (Out Of Band) P-states mode is enabled in Sapphire Rapids. The OOB identifying bits are same as the prior generation CPUs like Ice Lake servers. So, also add Sapphire Rapids to intel_pstate_cpu_oob_ids list. Signed-off-by: Srinivas Pandruvada Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/intel_pstate.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c index 2216b24b6f84..57cdb3679885 100644 --- a/drivers/cpufreq/intel_pstate.c +++ b/drivers/cpufreq/intel_pstate.c @@ -2425,6 +2425,7 @@ static const struct x86_cpu_id intel_pstate_cpu_oob_ids[] __initconst = { X86_MATCH(BROADWELL_X, core_funcs), X86_MATCH(SKYLAKE_X, core_funcs), X86_MATCH(ICELAKE_X, core_funcs), + X86_MATCH(SAPPHIRERAPIDS_X, core_funcs), {} }; -- cgit v1.2.3 From 5c84c1b84bd80a735b2c7804e7be86d043fca595 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 11 May 2022 14:36:24 +0530 Subject: cpufreq: Clear real_cpus mask from remove_cpu_dev_symlink() add_cpu_dev_symlink() is responsible for setting the CPUs in the real_cpus mask, the reverse of which should be done from remove_cpu_dev_symlink() to make it look clean and avoid any breakage later on. Move the call to clear the mask to remove_cpu_dev_symlink(). Signed-off-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index fbaa8e6c7d23..c8bf6c68597c 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1020,11 +1020,12 @@ static void add_cpu_dev_symlink(struct cpufreq_policy *policy, unsigned int cpu, dev_err(dev, "cpufreq symlink creation failed\n"); } -static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, +static void remove_cpu_dev_symlink(struct cpufreq_policy *policy, int cpu, struct device *dev) { dev_dbg(dev, "%s: Removing symlink\n", __func__); sysfs_remove_link(&dev->kobj, "cpufreq"); + cpumask_clear_cpu(cpu, policy->real_cpus); } static int cpufreq_add_dev_interface(struct cpufreq_policy *policy) @@ -1532,7 +1533,7 @@ static int cpufreq_online(unsigned int cpu) out_destroy_policy: for_each_cpu(j, policy->real_cpus) - remove_cpu_dev_symlink(policy, get_cpu_device(j)); + remove_cpu_dev_symlink(policy, j, get_cpu_device(j)); up_write(&policy->rwsem); @@ -1657,8 +1658,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) if (cpu_online(cpu)) cpufreq_offline(cpu); - cpumask_clear_cpu(cpu, policy->real_cpus); - remove_cpu_dev_symlink(policy, dev); + remove_cpu_dev_symlink(policy, cpu, dev); if (cpumask_empty(policy->real_cpus)) { /* We did light-weight exit earlier, do full tear down now */ -- cgit v1.2.3 From 515755906921fa9393d6c9de18fac4343882a88d Mon Sep 17 00:00:00 2001 From: Sumeet Pawnikar Date: Fri, 6 May 2022 19:23:52 +0530 Subject: powercap: RAPL: Add Power Limit4 support for RaptorLake Add RaptorLake to the list of processor models for which Power Limit4 is supported by the Intel RAPL driver. Signed-off-by: Sumeet Pawnikar [ rjw: Changelog rewrite ] Signed-off-by: Rafael J. Wysocki --- drivers/powercap/intel_rapl_msr.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/powercap/intel_rapl_msr.c b/drivers/powercap/intel_rapl_msr.c index 1be45f36ab6c..9d23984d8931 100644 --- a/drivers/powercap/intel_rapl_msr.c +++ b/drivers/powercap/intel_rapl_msr.c @@ -140,6 +140,7 @@ static const struct x86_cpu_id pl4_support_ids[] = { { X86_VENDOR_INTEL, 6, INTEL_FAM6_TIGERLAKE_L, X86_FEATURE_ANY }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ALDERLAKE, X86_FEATURE_ANY }, { X86_VENDOR_INTEL, 6, INTEL_FAM6_ALDERLAKE_L, X86_FEATURE_ANY }, + { X86_VENDOR_INTEL, 6, INTEL_FAM6_RAPTORLAKE, X86_FEATURE_ANY }, {} }; -- cgit v1.2.3 From e1e962c5b9edbc628a335bcdbd010331a12d3e5b Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 11 May 2022 17:48:41 +0200 Subject: cpufreq: Reorganize checks in cpufreq_offline() Notice that cpufreq_offline() only needs to check policy_is_inactive() once and rearrange the code in there to make that happen. No expected functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index c8bf6c68597c..79cedc2bd7fc 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1591,24 +1591,18 @@ static int cpufreq_offline(unsigned int cpu) } down_write(&policy->rwsem); + if (has_target()) cpufreq_stop_governor(policy); cpumask_clear_cpu(cpu, policy->cpus); - if (policy_is_inactive(policy)) { - if (has_target()) - strncpy(policy->last_governor, policy->governor->name, - CPUFREQ_NAME_LEN); - else - policy->last_policy = policy->policy; - } else if (cpu == policy->cpu) { - /* Nominate new CPU */ - policy->cpu = cpumask_any(policy->cpus); - } - - /* Start governor again for active policy */ if (!policy_is_inactive(policy)) { + /* Nominate a new CPU if necessary. */ + if (cpu == policy->cpu) + policy->cpu = cpumask_any(policy->cpus); + + /* Start the governor again for the active policy. */ if (has_target()) { ret = cpufreq_start_governor(policy); if (ret) @@ -1618,6 +1612,12 @@ static int cpufreq_offline(unsigned int cpu) goto unlock; } + if (has_target()) + strncpy(policy->last_governor, policy->governor->name, + CPUFREQ_NAME_LEN); + else + policy->last_policy = policy->policy; + if (cpufreq_thermal_control_enabled(cpufreq_driver)) { cpufreq_cooling_unregister(policy->cdev); policy->cdev = NULL; -- cgit v1.2.3 From fddd8f86dff4a24742a7f0322ccbb34c6c1c9850 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 11 May 2022 17:50:09 +0200 Subject: cpufreq: Split cpufreq_offline() Split the "core" part running under the policy rwsem out of cpufreq_offline() to allow the locking in cpufreq_remove_dev() to be rearranged more easily. As a side-effect this eliminates the unlock label that's not needed any more. No expected functional impact. Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 79cedc2bd7fc..9664a86766c9 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1577,21 +1577,10 @@ static int cpufreq_add_dev(struct device *dev, struct subsys_interface *sif) return 0; } -static int cpufreq_offline(unsigned int cpu) +static void __cpufreq_offline(unsigned int cpu, struct cpufreq_policy *policy) { - struct cpufreq_policy *policy; int ret; - pr_debug("%s: unregistering CPU %u\n", __func__, cpu); - - policy = cpufreq_cpu_get_raw(cpu); - if (!policy) { - pr_debug("%s: No cpu_data found\n", __func__); - return 0; - } - - down_write(&policy->rwsem); - if (has_target()) cpufreq_stop_governor(policy); @@ -1609,7 +1598,7 @@ static int cpufreq_offline(unsigned int cpu) pr_err("%s: Failed to start governor\n", __func__); } - goto unlock; + return; } if (has_target()) @@ -1636,8 +1625,24 @@ static int cpufreq_offline(unsigned int cpu) cpufreq_driver->exit(policy); policy->freq_table = NULL; } +} + +static int cpufreq_offline(unsigned int cpu) +{ + struct cpufreq_policy *policy; + + pr_debug("%s: unregistering CPU %u\n", __func__, cpu); + + policy = cpufreq_cpu_get_raw(cpu); + if (!policy) { + pr_debug("%s: No cpu_data found\n", __func__); + return 0; + } + + down_write(&policy->rwsem); + + __cpufreq_offline(cpu, policy); -unlock: up_write(&policy->rwsem); return 0; } -- cgit v1.2.3 From f339f3541701d824a0256ad4bf14c26ceb6d79c3 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 11 May 2022 17:51:39 +0200 Subject: cpufreq: Rearrange locking in cpufreq_remove_dev() Currently, cpufreq_remove_dev() invokes the ->exit() driver callback without holding the policy rwsem which is inconsistent with what happens if ->exit() is invoked directly from cpufreq_offline(). It also manipulates the real_cpus mask and removes the CPU device symlink without holding the policy rwsem, but cpufreq_offline() holds the rwsem around the modifications thereof. For consistency, modify cpufreq_remove_dev() to hold the policy rwsem until the ->exit() callback has been called (or it has been determined that it is not necessary to call it). Signed-off-by: Rafael J. Wysocki Acked-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 9664a86766c9..1f6667ce43bd 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1660,18 +1660,25 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) if (!policy) return; + down_write(&policy->rwsem); + if (cpu_online(cpu)) - cpufreq_offline(cpu); + __cpufreq_offline(cpu, policy); remove_cpu_dev_symlink(policy, cpu, dev); - if (cpumask_empty(policy->real_cpus)) { - /* We did light-weight exit earlier, do full tear down now */ - if (cpufreq_driver->offline) - cpufreq_driver->exit(policy); - - cpufreq_policy_free(policy); + if (!cpumask_empty(policy->real_cpus)) { + up_write(&policy->rwsem); + return; } + + /* We did light-weight exit earlier, do full tear down now */ + if (cpufreq_driver->offline) + cpufreq_driver->exit(policy); + + up_write(&policy->rwsem); + + cpufreq_policy_free(policy); } /** -- cgit v1.2.3 From 713472e53e6e53c985e283782b0fd76b8ecfd47e Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Mon, 1 Mar 2021 02:07:29 +0900 Subject: PM / devfreq: Export devfreq_get_freq_range symbol within devfreq In order to get frequency range within devfreq governors, export devfreq_get_freq_range symbol within devfreq. Reviewed-by: Matthias Kaehlcke Tested-by: Chen-Yu Tsai Tested-by: Johnson Wang Signed-off-by: Chanwoo Choi --- drivers/devfreq/devfreq.c | 20 ++++++++++++-------- drivers/devfreq/governor.h | 2 ++ 2 files changed, 14 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c index a525a609dfc6..01474daf4548 100644 --- a/drivers/devfreq/devfreq.c +++ b/drivers/devfreq/devfreq.c @@ -112,16 +112,16 @@ static unsigned long find_available_max_freq(struct devfreq *devfreq) } /** - * get_freq_range() - Get the current freq range + * devfreq_get_freq_range() - Get the current freq range * @devfreq: the devfreq instance * @min_freq: the min frequency * @max_freq: the max frequency * * This takes into consideration all constraints. */ -static void get_freq_range(struct devfreq *devfreq, - unsigned long *min_freq, - unsigned long *max_freq) +void devfreq_get_freq_range(struct devfreq *devfreq, + unsigned long *min_freq, + unsigned long *max_freq) { unsigned long *freq_table = devfreq->profile->freq_table; s32 qos_min_freq, qos_max_freq; @@ -158,6 +158,7 @@ static void get_freq_range(struct devfreq *devfreq, if (*min_freq > *max_freq) *min_freq = *max_freq; } +EXPORT_SYMBOL(devfreq_get_freq_range); /** * devfreq_get_freq_level() - Lookup freq_table for the frequency @@ -418,7 +419,7 @@ int devfreq_update_target(struct devfreq *devfreq, unsigned long freq) err = devfreq->governor->get_target_freq(devfreq, &freq); if (err) return err; - get_freq_range(devfreq, &min_freq, &max_freq); + devfreq_get_freq_range(devfreq, &min_freq, &max_freq); if (freq < min_freq) { freq = min_freq; @@ -785,6 +786,7 @@ struct devfreq *devfreq_add_device(struct device *dev, { struct devfreq *devfreq; struct devfreq_governor *governor; + unsigned long min_freq, max_freq; int err = 0; if (!dev || !profile || !governor_name) { @@ -849,6 +851,8 @@ struct devfreq *devfreq_add_device(struct device *dev, goto err_dev; } + devfreq_get_freq_range(devfreq, &min_freq, &max_freq); + devfreq->suspend_freq = dev_pm_opp_get_suspend_opp_freq(dev); devfreq->opp_table = dev_pm_opp_get_opp_table(dev); if (IS_ERR(devfreq->opp_table)) @@ -1587,7 +1591,7 @@ static ssize_t min_freq_show(struct device *dev, struct device_attribute *attr, unsigned long min_freq, max_freq; mutex_lock(&df->lock); - get_freq_range(df, &min_freq, &max_freq); + devfreq_get_freq_range(df, &min_freq, &max_freq); mutex_unlock(&df->lock); return sprintf(buf, "%lu\n", min_freq); @@ -1641,7 +1645,7 @@ static ssize_t max_freq_show(struct device *dev, struct device_attribute *attr, unsigned long min_freq, max_freq; mutex_lock(&df->lock); - get_freq_range(df, &min_freq, &max_freq); + devfreq_get_freq_range(df, &min_freq, &max_freq); mutex_unlock(&df->lock); return sprintf(buf, "%lu\n", max_freq); @@ -1955,7 +1959,7 @@ static int devfreq_summary_show(struct seq_file *s, void *data) mutex_lock(&devfreq->lock); cur_freq = devfreq->previous_freq; - get_freq_range(devfreq, &min_freq, &max_freq); + devfreq_get_freq_range(devfreq, &min_freq, &max_freq); timer = devfreq->profile->timer; if (IS_SUPPORTED_ATTR(devfreq->governor->attrs, POLLING_INTERVAL)) diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h index 002a7d67e39d..b0dbfee8bbf2 100644 --- a/drivers/devfreq/governor.h +++ b/drivers/devfreq/governor.h @@ -89,6 +89,8 @@ int devm_devfreq_add_governor(struct device *dev, int devfreq_update_status(struct devfreq *devfreq, unsigned long freq); int devfreq_update_target(struct devfreq *devfreq, unsigned long freq); +void devfreq_get_freq_range(struct devfreq *devfreq, unsigned long *min_freq, + unsigned long *max_freq); static inline int devfreq_update_stats(struct devfreq *df) { -- cgit v1.2.3 From a03dacb0316f74400846aaf144d6c73f4217ca08 Mon Sep 17 00:00:00 2001 From: Saravana Kannan Date: Tue, 2 Mar 2021 15:58:21 +0900 Subject: PM / devfreq: Add cpu based scaling support to passive governor Many CPU architectures have caches that can scale independent of the CPUs. Frequency scaling of the caches is necessary to make sure that the cache is not a performance bottleneck that leads to poor performance and power. The same idea applies for RAM/DDR. To achieve this, this patch adds support for cpu based scaling to the passive governor. This is accomplished by taking the current frequency of each CPU frequency domain and then adjust the frequency of the cache (or any devfreq device) based on the frequency of the CPUs. It listens to CPU frequency transition notifiers to keep itself up to date on the current CPU frequency. To decide the frequency of the device, the governor does one of the following: * Derives the optimal devfreq device opp from required-opps property of the parent cpu opp_table. * Scales the device frequency in proportion to the CPU frequency. So, if the CPUs are running at their max frequency, the device runs at its max frequency. If the CPUs are running at their min frequency, the device runs at its min frequency. It is interpolated for frequencies in between. Tested-by: Chen-Yu Tsai Tested-by: Johnson Wang Signed-off-by: Saravana Kannan [Sibi: Integrated cpu-freqmap governor into passive_governor] Signed-off-by: Sibi Sankar [Chanwoo: Fix conflict with latest code and cleanup code] Signed-off-by: Chanwoo Choi --- drivers/devfreq/governor.h | 22 +++ drivers/devfreq/governor_passive.c | 298 +++++++++++++++++++++++++++++++++++-- include/linux/devfreq.h | 17 ++- 3 files changed, 323 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h index b0dbfee8bbf2..335c4a491254 100644 --- a/drivers/devfreq/governor.h +++ b/drivers/devfreq/governor.h @@ -47,6 +47,28 @@ #define DEVFREQ_GOV_ATTR_POLLING_INTERVAL BIT(0) #define DEVFREQ_GOV_ATTR_TIMER BIT(1) +/** + * struct devfreq_cpu_data - Hold the per-cpu data + * @dev: reference to cpu device. + * @first_cpu: the cpumask of the first cpu of a policy. + * @opp_table: reference to cpu opp table. + * @cur_freq: the current frequency of the cpu. + * @min_freq: the min frequency of the cpu. + * @max_freq: the max frequency of the cpu. + * + * This structure stores the required cpu_data of a cpu. + * This is auto-populated by the governor. + */ +struct devfreq_cpu_data { + struct device *dev; + unsigned int first_cpu; + + struct opp_table *opp_table; + unsigned int cur_freq; + unsigned int min_freq; + unsigned int max_freq; +}; + /** * struct devfreq_governor - Devfreq policy governor * @node: list node - contains registered devfreq governors diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c index fc09324a03e0..7f30088b500b 100644 --- a/drivers/devfreq/governor_passive.c +++ b/drivers/devfreq/governor_passive.c @@ -8,11 +8,85 @@ */ #include +#include +#include +#include +#include #include #include #include "governor.h" -static int devfreq_passive_get_target_freq(struct devfreq *devfreq, +#define HZ_PER_KHZ 1000 + +static unsigned long get_target_freq_by_required_opp(struct device *p_dev, + struct opp_table *p_opp_table, + struct opp_table *opp_table, + unsigned long *freq) +{ + struct dev_pm_opp *opp = NULL, *p_opp = NULL; + unsigned long target_freq; + + if (!p_dev || !p_opp_table || !opp_table || !freq) + return 0; + + p_opp = devfreq_recommended_opp(p_dev, freq, 0); + if (IS_ERR(p_opp)) + return 0; + + opp = dev_pm_opp_xlate_required_opp(p_opp_table, opp_table, p_opp); + dev_pm_opp_put(p_opp); + + if (IS_ERR(opp)) + return 0; + + target_freq = dev_pm_opp_get_freq(opp); + dev_pm_opp_put(opp); + + return target_freq; +} + +static int get_target_freq_with_cpufreq(struct devfreq *devfreq, + unsigned long *target_freq) +{ + struct devfreq_passive_data *p_data = + (struct devfreq_passive_data *)devfreq->data; + struct devfreq_cpu_data *parent_cpu_data; + unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent; + unsigned long dev_min, dev_max; + unsigned long freq = 0; + + for_each_online_cpu(cpu) { + parent_cpu_data = p_data->parent_cpu_data[cpu]; + if (!parent_cpu_data || parent_cpu_data->first_cpu != cpu) + continue; + + /* Get target freq via required opps */ + cpu_cur = parent_cpu_data->cur_freq * HZ_PER_KHZ; + freq = get_target_freq_by_required_opp(parent_cpu_data->dev, + parent_cpu_data->opp_table, + devfreq->opp_table, &cpu_cur); + if (freq) { + *target_freq = max(freq, *target_freq); + continue; + } + + /* Use interpolation if required opps is not available */ + devfreq_get_freq_range(devfreq, &dev_min, &dev_max); + + cpu_min = parent_cpu_data->min_freq; + cpu_max = parent_cpu_data->max_freq; + cpu_cur = parent_cpu_data->cur_freq; + + cpu_percent = ((cpu_cur - cpu_min) * 100) / (cpu_max - cpu_min); + freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100); + + *target_freq = max(freq, *target_freq); + } + + return 0; +} + +static int get_target_freq_with_devfreq(struct devfreq *devfreq, unsigned long *freq) { struct devfreq_passive_data *p_data @@ -99,6 +173,181 @@ no_required_opp: return 0; } +static int devfreq_passive_get_target_freq(struct devfreq *devfreq, + unsigned long *freq) +{ + struct devfreq_passive_data *p_data = + (struct devfreq_passive_data *)devfreq->data; + int ret; + + if (!p_data) + return -EINVAL; + + /* + * If the devfreq device with passive governor has the specific method + * to determine the next frequency, should use the get_target_freq() + * of struct devfreq_passive_data. + */ + if (p_data->get_target_freq) + return p_data->get_target_freq(devfreq, freq); + + switch (p_data->parent_type) { + case DEVFREQ_PARENT_DEV: + ret = get_target_freq_with_devfreq(devfreq, freq); + break; + case CPUFREQ_PARENT_DEV: + ret = get_target_freq_with_cpufreq(devfreq, freq); + break; + default: + ret = -EINVAL; + dev_err(&devfreq->dev, "Invalid parent type\n"); + break; + } + + return ret; +} + +static int cpufreq_passive_notifier_call(struct notifier_block *nb, + unsigned long event, void *ptr) +{ + struct devfreq_passive_data *p_data = + container_of(nb, struct devfreq_passive_data, nb); + struct devfreq *devfreq = (struct devfreq *)p_data->this; + struct devfreq_cpu_data *parent_cpu_data; + struct cpufreq_freqs *freqs = ptr; + unsigned int cur_freq; + int ret; + + if (event != CPUFREQ_POSTCHANGE || !freqs || + !p_data->parent_cpu_data[freqs->policy->cpu]) + return 0; + + parent_cpu_data = p_data->parent_cpu_data[freqs->policy->cpu]; + if (parent_cpu_data->cur_freq == freqs->new) + return 0; + + cur_freq = parent_cpu_data->cur_freq; + parent_cpu_data->cur_freq = freqs->new; + + mutex_lock(&devfreq->lock); + ret = devfreq_update_target(devfreq, freqs->new); + mutex_unlock(&devfreq->lock); + if (ret) { + parent_cpu_data->cur_freq = cur_freq; + dev_err(&devfreq->dev, "failed to update the frequency.\n"); + return ret; + } + + return 0; +} + +static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq) +{ + struct devfreq_passive_data *p_data + = (struct devfreq_passive_data *)devfreq->data; + struct devfreq_cpu_data *parent_cpu_data; + int cpu, ret; + + if (p_data->nb.notifier_call) { + ret = cpufreq_unregister_notifier(&p_data->nb, + CPUFREQ_TRANSITION_NOTIFIER); + if (ret < 0) + return ret; + } + + for_each_possible_cpu(cpu) { + parent_cpu_data = p_data->parent_cpu_data[cpu]; + if (!parent_cpu_data) + continue; + + if (parent_cpu_data->opp_table) + dev_pm_opp_put_opp_table(parent_cpu_data->opp_table); + kfree(parent_cpu_data); + } + + return 0; +} + +static int cpufreq_passive_register_notifier(struct devfreq *devfreq) +{ + struct devfreq_passive_data *p_data + = (struct devfreq_passive_data *)devfreq->data; + struct device *dev = devfreq->dev.parent; + struct opp_table *opp_table = NULL; + struct devfreq_cpu_data *parent_cpu_data; + struct cpufreq_policy *policy; + struct device *cpu_dev; + unsigned int cpu; + int ret; + + p_data->nb.notifier_call = cpufreq_passive_notifier_call; + ret = cpufreq_register_notifier(&p_data->nb, CPUFREQ_TRANSITION_NOTIFIER); + if (ret) { + dev_err(dev, "failed to register cpufreq notifier\n"); + p_data->nb.notifier_call = NULL; + goto err; + } + + for_each_possible_cpu(cpu) { + if (p_data->parent_cpu_data[cpu]) + continue; + + policy = cpufreq_cpu_get(cpu); + if (!policy) { + ret = -EPROBE_DEFER; + goto err; + } + + parent_cpu_data = kzalloc(sizeof(*parent_cpu_data), + GFP_KERNEL); + if (!parent_cpu_data) { + ret = -ENOMEM; + goto err_put_policy; + } + + cpu_dev = get_cpu_device(cpu); + if (!cpu_dev) { + dev_err(dev, "failed to get cpu device\n"); + ret = -ENODEV; + goto err_free_cpu_data; + } + + opp_table = dev_pm_opp_get_opp_table(cpu_dev); + if (IS_ERR(opp_table)) { + dev_err(dev, "failed to get opp_table of cpu%d\n", cpu); + ret = PTR_ERR(opp_table); + goto err_free_cpu_data; + } + + parent_cpu_data->dev = cpu_dev; + parent_cpu_data->opp_table = opp_table; + parent_cpu_data->first_cpu = cpumask_first(policy->related_cpus); + parent_cpu_data->cur_freq = policy->cur; + parent_cpu_data->min_freq = policy->cpuinfo.min_freq; + parent_cpu_data->max_freq = policy->cpuinfo.max_freq; + + p_data->parent_cpu_data[cpu] = parent_cpu_data; + cpufreq_cpu_put(policy); + } + + mutex_lock(&devfreq->lock); + ret = devfreq_update_target(devfreq, 0L); + mutex_unlock(&devfreq->lock); + if (ret) + dev_err(dev, "failed to update the frequency\n"); + + return ret; + +err_free_cpu_data: + kfree(parent_cpu_data); +err_put_policy: + cpufreq_cpu_put(policy); +err: + WARN_ON(cpufreq_passive_unregister_notifier(devfreq)); + + return ret; +} + static int devfreq_passive_notifier_call(struct notifier_block *nb, unsigned long event, void *ptr) { @@ -131,30 +380,55 @@ static int devfreq_passive_notifier_call(struct notifier_block *nb, return NOTIFY_DONE; } -static int devfreq_passive_event_handler(struct devfreq *devfreq, - unsigned int event, void *data) +static int devfreq_passive_unregister_notifier(struct devfreq *devfreq) +{ + struct devfreq_passive_data *p_data + = (struct devfreq_passive_data *)devfreq->data; + struct devfreq *parent = (struct devfreq *)p_data->parent; + struct notifier_block *nb = &p_data->nb; + + return devfreq_unregister_notifier(parent, nb, DEVFREQ_TRANSITION_NOTIFIER); +} + +static int devfreq_passive_register_notifier(struct devfreq *devfreq) { struct devfreq_passive_data *p_data = (struct devfreq_passive_data *)devfreq->data; struct devfreq *parent = (struct devfreq *)p_data->parent; struct notifier_block *nb = &p_data->nb; - int ret = 0; if (!parent) return -EPROBE_DEFER; + nb->notifier_call = devfreq_passive_notifier_call; + return devfreq_register_notifier(parent, nb, DEVFREQ_TRANSITION_NOTIFIER); +} + +static int devfreq_passive_event_handler(struct devfreq *devfreq, + unsigned int event, void *data) +{ + struct devfreq_passive_data *p_data + = (struct devfreq_passive_data *)devfreq->data; + int ret = -EINVAL; + + if (!p_data) + return -EINVAL; + + if (!p_data->this) + p_data->this = devfreq; + switch (event) { case DEVFREQ_GOV_START: - if (!p_data->this) - p_data->this = devfreq; - - nb->notifier_call = devfreq_passive_notifier_call; - ret = devfreq_register_notifier(parent, nb, - DEVFREQ_TRANSITION_NOTIFIER); + if (p_data->parent_type == DEVFREQ_PARENT_DEV) + ret = devfreq_passive_register_notifier(devfreq); + else if (p_data->parent_type == CPUFREQ_PARENT_DEV) + ret = cpufreq_passive_register_notifier(devfreq); break; case DEVFREQ_GOV_STOP: - WARN_ON(devfreq_unregister_notifier(parent, nb, - DEVFREQ_TRANSITION_NOTIFIER)); + if (p_data->parent_type == DEVFREQ_PARENT_DEV) + WARN_ON(devfreq_passive_unregister_notifier(devfreq)); + else if (p_data->parent_type == CPUFREQ_PARENT_DEV) + WARN_ON(cpufreq_passive_unregister_notifier(devfreq)); break; default: break; diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h index 142474b4af96..b1e4a6f796ce 100644 --- a/include/linux/devfreq.h +++ b/include/linux/devfreq.h @@ -38,6 +38,7 @@ enum devfreq_timer { struct devfreq; struct devfreq_governor; +struct devfreq_cpu_data; struct thermal_cooling_device; /** @@ -288,6 +289,11 @@ struct devfreq_simple_ondemand_data { #endif #if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE) +enum devfreq_parent_dev_type { + DEVFREQ_PARENT_DEV, + CPUFREQ_PARENT_DEV, +}; + /** * struct devfreq_passive_data - ``void *data`` fed to struct devfreq * and devfreq_add_device @@ -299,8 +305,11 @@ struct devfreq_simple_ondemand_data { * using governors except for passive governor. * If the devfreq device has the specific method to decide * the next frequency, should use this callback. - * @this: the devfreq instance of own device. - * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER list + * @parent_type: the parent type of the device. + * @this: the devfreq instance of own device. + * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER or + * CPUFREQ_TRANSITION_NOTIFIER list. + * @parent_cpu_data: the state min/max/current frequency of all online cpu's. * * The devfreq_passive_data have to set the devfreq instance of parent * device with governors except for the passive governor. But, don't need to @@ -314,9 +323,13 @@ struct devfreq_passive_data { /* Optional callback to decide the next frequency of passvice device */ int (*get_target_freq)(struct devfreq *this, unsigned long *freq); + /* Should set the type of parent device */ + enum devfreq_parent_dev_type parent_type; + /* For passive governor's internal use. Don't need to set them */ struct devfreq *this; struct notifier_block nb; + struct devfreq_cpu_data *parent_cpu_data[NR_CPUS]; }; #endif -- cgit v1.2.3 From 05723e71234b60a1a47313ea1a889797ec648f1c Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Tue, 2 Mar 2021 17:22:50 +0900 Subject: PM / devfreq: passive: Reduce duplicate code when passive_devfreq case In order to keep the consistent coding style between passive_devfreq and passive_cpufreq, use common code for handling required opp property. Also remove the unneed conditional statement and unify the comment of both passive_devfreq and passive_cpufreq when getting the target frequency. Tested-by: Chen-Yu Tsai Tested-by: Johnson Wang Signed-off-by: Chanwoo Choi --- drivers/devfreq/governor_passive.c | 66 +++++--------------------------------- 1 file changed, 8 insertions(+), 58 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c index 7f30088b500b..ffcce613a48c 100644 --- a/drivers/devfreq/governor_passive.c +++ b/drivers/devfreq/governor_passive.c @@ -93,65 +93,16 @@ static int get_target_freq_with_devfreq(struct devfreq *devfreq, = (struct devfreq_passive_data *)devfreq->data; struct devfreq *parent_devfreq = (struct devfreq *)p_data->parent; unsigned long child_freq = ULONG_MAX; - struct dev_pm_opp *opp, *p_opp; int i, count; - /* - * If the devfreq device with passive governor has the specific method - * to determine the next frequency, should use the get_target_freq() - * of struct devfreq_passive_data. - */ - if (p_data->get_target_freq) - return p_data->get_target_freq(devfreq, freq); + /* Get target freq via required opps */ + child_freq = get_target_freq_by_required_opp(parent_devfreq->dev.parent, + parent_devfreq->opp_table, + devfreq->opp_table, freq); + if (child_freq) + goto out; - /* - * If the parent and passive devfreq device uses the OPP table, - * get the next frequency by using the OPP table. - */ - - /* - * - parent devfreq device uses the governors except for passive. - * - passive devfreq device uses the passive governor. - * - * Each devfreq has the OPP table. After deciding the new frequency - * from the governor of parent devfreq device, the passive governor - * need to get the index of new frequency on OPP table of parent - * device. And then the index is used for getting the suitable - * new frequency for passive devfreq device. - */ - if (!devfreq->profile || !devfreq->profile->freq_table - || devfreq->profile->max_state <= 0) - return -EINVAL; - - /* - * The passive governor have to get the correct frequency from OPP - * list of parent device. Because in this case, *freq is temporary - * value which is decided by ondemand governor. - */ - if (devfreq->opp_table && parent_devfreq->opp_table) { - p_opp = devfreq_recommended_opp(parent_devfreq->dev.parent, - freq, 0); - if (IS_ERR(p_opp)) - return PTR_ERR(p_opp); - - opp = dev_pm_opp_xlate_required_opp(parent_devfreq->opp_table, - devfreq->opp_table, p_opp); - dev_pm_opp_put(p_opp); - - if (IS_ERR(opp)) - goto no_required_opp; - - *freq = dev_pm_opp_get_freq(opp); - dev_pm_opp_put(opp); - - return 0; - } - -no_required_opp: - /* - * Get the OPP table's index of decided frequency by governor - * of parent device. - */ + /* Use interpolation if required opps is not available */ for (i = 0; i < parent_devfreq->profile->max_state; i++) if (parent_devfreq->profile->freq_table[i] == *freq) break; @@ -159,7 +110,6 @@ no_required_opp: if (i == parent_devfreq->profile->max_state) return -EINVAL; - /* Get the suitable frequency by using index of parent device. */ if (i < devfreq->profile->max_state) { child_freq = devfreq->profile->freq_table[i]; } else { @@ -167,7 +117,7 @@ no_required_opp: child_freq = devfreq->profile->freq_table[count - 1]; } - /* Return the suitable frequency for passive device. */ +out: *freq = child_freq; return 0; -- cgit v1.2.3 From 26984d9d581e5049bd75091d2e789b9cc3ea12e0 Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Wed, 27 Apr 2022 03:49:19 +0900 Subject: PM / devfreq: passive: Keep cpufreq_policy for possible cpus The passive governor requires the cpu data to get the next target frequency of devfreq device if depending on cpu. In order to reduce the unnecessary memory data, keep cpufreq_policy data for possible cpus instead of NR_CPU. Tested-by: Chen-Yu Tsai Tested-by: Johnson Wang Signed-off-by: Chanwoo Choi --- drivers/devfreq/governor.h | 3 ++ drivers/devfreq/governor_passive.c | 75 ++++++++++++++++++++++++++++++-------- include/linux/devfreq.h | 4 +- 3 files changed, 64 insertions(+), 18 deletions(-) (limited to 'drivers') diff --git a/drivers/devfreq/governor.h b/drivers/devfreq/governor.h index 335c4a491254..0adfebc0467a 100644 --- a/drivers/devfreq/governor.h +++ b/drivers/devfreq/governor.h @@ -49,6 +49,7 @@ /** * struct devfreq_cpu_data - Hold the per-cpu data + * @node: list node * @dev: reference to cpu device. * @first_cpu: the cpumask of the first cpu of a policy. * @opp_table: reference to cpu opp table. @@ -60,6 +61,8 @@ * This is auto-populated by the governor. */ struct devfreq_cpu_data { + struct list_head node; + struct device *dev; unsigned int first_cpu; diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c index ffcce613a48c..7306e943a234 100644 --- a/drivers/devfreq/governor_passive.c +++ b/drivers/devfreq/governor_passive.c @@ -1,4 +1,4 @@ -// SPDX-License-Identifier: GPL-2.0-only + // SPDX-License-Identifier: GPL-2.0-only /* * linux/drivers/devfreq/governor_passive.c * @@ -18,6 +18,22 @@ #define HZ_PER_KHZ 1000 +static struct devfreq_cpu_data * +get_parent_cpu_data(struct devfreq_passive_data *p_data, + struct cpufreq_policy *policy) +{ + struct devfreq_cpu_data *parent_cpu_data; + + if (!p_data || !policy) + return NULL; + + list_for_each_entry(parent_cpu_data, &p_data->cpu_data_list, node) + if (parent_cpu_data->first_cpu == cpumask_first(policy->related_cpus)) + return parent_cpu_data; + + return NULL; +} + static unsigned long get_target_freq_by_required_opp(struct device *p_dev, struct opp_table *p_opp_table, struct opp_table *opp_table, @@ -51,14 +67,24 @@ static int get_target_freq_with_cpufreq(struct devfreq *devfreq, struct devfreq_passive_data *p_data = (struct devfreq_passive_data *)devfreq->data; struct devfreq_cpu_data *parent_cpu_data; + struct cpufreq_policy *policy; unsigned long cpu, cpu_cur, cpu_min, cpu_max, cpu_percent; unsigned long dev_min, dev_max; unsigned long freq = 0; + int ret = 0; for_each_online_cpu(cpu) { - parent_cpu_data = p_data->parent_cpu_data[cpu]; - if (!parent_cpu_data || parent_cpu_data->first_cpu != cpu) + policy = cpufreq_cpu_get(cpu); + if (!policy) { + ret = -EINVAL; + continue; + } + + parent_cpu_data = get_parent_cpu_data(p_data, policy); + if (!parent_cpu_data) { + cpufreq_cpu_put(policy); continue; + } /* Get target freq via required opps */ cpu_cur = parent_cpu_data->cur_freq * HZ_PER_KHZ; @@ -67,6 +93,7 @@ static int get_target_freq_with_cpufreq(struct devfreq *devfreq, devfreq->opp_table, &cpu_cur); if (freq) { *target_freq = max(freq, *target_freq); + cpufreq_cpu_put(policy); continue; } @@ -81,9 +108,10 @@ static int get_target_freq_with_cpufreq(struct devfreq *devfreq, freq = dev_min + mult_frac(dev_max - dev_min, cpu_percent, 100); *target_freq = max(freq, *target_freq); + cpufreq_cpu_put(policy); } - return 0; + return ret; } static int get_target_freq_with_devfreq(struct devfreq *devfreq, @@ -168,12 +196,11 @@ static int cpufreq_passive_notifier_call(struct notifier_block *nb, unsigned int cur_freq; int ret; - if (event != CPUFREQ_POSTCHANGE || !freqs || - !p_data->parent_cpu_data[freqs->policy->cpu]) + if (event != CPUFREQ_POSTCHANGE || !freqs) return 0; - parent_cpu_data = p_data->parent_cpu_data[freqs->policy->cpu]; - if (parent_cpu_data->cur_freq == freqs->new) + parent_cpu_data = get_parent_cpu_data(p_data, freqs->policy); + if (!parent_cpu_data || parent_cpu_data->cur_freq == freqs->new) return 0; cur_freq = parent_cpu_data->cur_freq; @@ -196,7 +223,7 @@ static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq) struct devfreq_passive_data *p_data = (struct devfreq_passive_data *)devfreq->data; struct devfreq_cpu_data *parent_cpu_data; - int cpu, ret; + int cpu, ret = 0; if (p_data->nb.notifier_call) { ret = cpufreq_unregister_notifier(&p_data->nb, @@ -206,16 +233,26 @@ static int cpufreq_passive_unregister_notifier(struct devfreq *devfreq) } for_each_possible_cpu(cpu) { - parent_cpu_data = p_data->parent_cpu_data[cpu]; - if (!parent_cpu_data) + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); + if (!policy) { + ret = -EINVAL; continue; + } + parent_cpu_data = get_parent_cpu_data(p_data, policy); + if (!parent_cpu_data) { + cpufreq_cpu_put(policy); + continue; + } + + list_del(&parent_cpu_data->node); if (parent_cpu_data->opp_table) dev_pm_opp_put_opp_table(parent_cpu_data->opp_table); kfree(parent_cpu_data); + cpufreq_cpu_put(policy); } - return 0; + return ret; } static int cpufreq_passive_register_notifier(struct devfreq *devfreq) @@ -230,6 +267,9 @@ static int cpufreq_passive_register_notifier(struct devfreq *devfreq) unsigned int cpu; int ret; + p_data->cpu_data_list + = (struct list_head)LIST_HEAD_INIT(p_data->cpu_data_list); + p_data->nb.notifier_call = cpufreq_passive_notifier_call; ret = cpufreq_register_notifier(&p_data->nb, CPUFREQ_TRANSITION_NOTIFIER); if (ret) { @@ -239,15 +279,18 @@ static int cpufreq_passive_register_notifier(struct devfreq *devfreq) } for_each_possible_cpu(cpu) { - if (p_data->parent_cpu_data[cpu]) - continue; - policy = cpufreq_cpu_get(cpu); if (!policy) { ret = -EPROBE_DEFER; goto err; } + parent_cpu_data = get_parent_cpu_data(p_data, policy); + if (parent_cpu_data) { + cpufreq_cpu_put(policy); + continue; + } + parent_cpu_data = kzalloc(sizeof(*parent_cpu_data), GFP_KERNEL); if (!parent_cpu_data) { @@ -276,7 +319,7 @@ static int cpufreq_passive_register_notifier(struct devfreq *devfreq) parent_cpu_data->min_freq = policy->cpuinfo.min_freq; parent_cpu_data->max_freq = policy->cpuinfo.max_freq; - p_data->parent_cpu_data[cpu] = parent_cpu_data; + list_add_tail(&parent_cpu_data->node, &p_data->cpu_data_list); cpufreq_cpu_put(policy); } diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h index b1e4a6f796ce..dc10bee75a72 100644 --- a/include/linux/devfreq.h +++ b/include/linux/devfreq.h @@ -309,7 +309,7 @@ enum devfreq_parent_dev_type { * @this: the devfreq instance of own device. * @nb: the notifier block for DEVFREQ_TRANSITION_NOTIFIER or * CPUFREQ_TRANSITION_NOTIFIER list. - * @parent_cpu_data: the state min/max/current frequency of all online cpu's. + * @cpu_data_list: the list of cpu frequency data for all cpufreq_policy. * * The devfreq_passive_data have to set the devfreq instance of parent * device with governors except for the passive governor. But, don't need to @@ -329,7 +329,7 @@ struct devfreq_passive_data { /* For passive governor's internal use. Don't need to set them */ struct devfreq *this; struct notifier_block nb; - struct devfreq_cpu_data *parent_cpu_data[NR_CPUS]; + struct list_head cpu_data_list; }; #endif -- cgit v1.2.3 From d4627a287e251efed59f2b4bda82c5950768c963 Mon Sep 17 00:00:00 2001 From: Schspa Shi Date: Mon, 16 May 2022 11:02:50 +0800 Subject: cpufreq: Abort show()/store() for half-initialized policies If policy initialization fails after the sysfs files are created, there is a possibility to end up running show()/store() callbacks for half-initialized policies, which may have unpredictable outcomes. Abort show()/store() in such a case by making sure the policy is active. Also dectivate the policy on such failures. Signed-off-by: Schspa Shi Acked-by: Viresh Kumar [ rjw: Subject and changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 1f6667ce43bd..f75197b924ef 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -948,13 +948,14 @@ static ssize_t show(struct kobject *kobj, struct attribute *attr, char *buf) { struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); - ssize_t ret; + ssize_t ret = -EBUSY; if (!fattr->show) return -EIO; down_read(&policy->rwsem); - ret = fattr->show(policy, buf); + if (likely(!policy_is_inactive(policy))) + ret = fattr->show(policy, buf); up_read(&policy->rwsem); return ret; @@ -965,7 +966,7 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, { struct cpufreq_policy *policy = to_policy(kobj); struct freq_attr *fattr = to_attr(attr); - ssize_t ret = -EINVAL; + ssize_t ret = -EBUSY; if (!fattr->store) return -EIO; @@ -979,7 +980,8 @@ static ssize_t store(struct kobject *kobj, struct attribute *attr, if (cpu_online(policy->cpu)) { down_write(&policy->rwsem); - ret = fattr->store(policy, buf, count); + if (likely(!policy_is_inactive(policy))) + ret = fattr->store(policy, buf, count); up_write(&policy->rwsem); } @@ -1535,6 +1537,7 @@ out_destroy_policy: for_each_cpu(j, policy->real_cpus) remove_cpu_dev_symlink(policy, j, get_cpu_device(j)); + cpumask_clear(policy->cpus); up_write(&policy->rwsem); out_offline_policy: -- cgit v1.2.3 From 514ff1bcd98d7f57361025e2200b803d3ddde6c8 Mon Sep 17 00:00:00 2001 From: Schspa Shi Date: Mon, 16 May 2022 11:02:51 +0800 Subject: cpufreq: make interface functions and lock holding state clear cpufreq_offline() calls offline() and exit() under the policy rwsem But they are called outside the rwsem in cpufreq_online(). Make cpufreq_online() call offline() and exit() as well as online() and init() under the policy rwsem to achieve a clear lock relationship. All of the init() and online() implementations in the tree only initialize the policy object without attempting to acquire the policy rwsem and they won't call cpufreq APIs attempting to acquire it. Signed-off-by: Schspa Shi Acked-by: Viresh Kumar [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index f75197b924ef..2cad42774164 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1341,12 +1341,12 @@ static int cpufreq_online(unsigned int cpu) down_write(&policy->rwsem); policy->cpu = cpu; policy->governor = NULL; - up_write(&policy->rwsem); } else { new_policy = true; policy = cpufreq_policy_alloc(cpu); if (!policy) return -ENOMEM; + down_write(&policy->rwsem); } if (!new_policy && cpufreq_driver->online) { @@ -1386,7 +1386,6 @@ static int cpufreq_online(unsigned int cpu) cpumask_copy(policy->related_cpus, policy->cpus); } - down_write(&policy->rwsem); /* * affected cpus must always be the one, which are online. We aren't * managing offline cpus here. @@ -1538,7 +1537,6 @@ out_destroy_policy: remove_cpu_dev_symlink(policy, j, get_cpu_device(j)); cpumask_clear(policy->cpus); - up_write(&policy->rwsem); out_offline_policy: if (cpufreq_driver->offline) @@ -1549,6 +1547,8 @@ out_exit_policy: cpufreq_driver->exit(policy); out_free_policy: + up_write(&policy->rwsem); + cpufreq_policy_free(policy); return ret; } -- cgit v1.2.3 From f125bdbdd6bd4a88f3697e5850359d3ffe43a3f2 Mon Sep 17 00:00:00 2001 From: Zhang Rui Date: Fri, 13 May 2022 13:00:40 +0800 Subject: powercap: intel_rapl: add support for ALDERLAKE_N Add ALDERLAKE_N to the list of supported processor models in the Intel RAPL power capping driver. Signed-off-by: Zhang Rui [ rjw: Changelog ] Signed-off-by: Rafael J. Wysocki --- drivers/powercap/intel_rapl_common.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c index 83da499dd6ec..e45c1c3b0cae 100644 --- a/drivers/powercap/intel_rapl_common.c +++ b/drivers/powercap/intel_rapl_common.c @@ -1107,6 +1107,7 @@ static const struct x86_cpu_id rapl_ids[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(ROCKETLAKE, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &rapl_defaults_core), + X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &rapl_defaults_core), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &rapl_defaults_spr_server), X86_MATCH_INTEL_FAM6_MODEL(LAKEFIELD, &rapl_defaults_core), -- cgit v1.2.3 From 42d2607d91c4ec37ea1970899c2d614824f3014b Mon Sep 17 00:00:00 2001 From: Chanwoo Choi Date: Thu, 19 May 2022 10:07:53 +0900 Subject: PM / devfreq: passive: Return non-error when not-supported event is required Each devfreq governor specifies the supported governor event such as GOV_START and GOV_STOP. When not-supported event is required, just return non-error. But, commit ce9a0d88d97a ("PM / devfreq: Add cpu based scaling support to passive governor") returned the error value. So that return non-error value when not-supported event is required. Fixes: ce9a0d88d97a ("PM / devfreq: Add cpu based scaling support to passive governor") Reported-by: Marek Szyprowski Signed-off-by: Chanwoo Choi Signed-off-by: Rafael J. Wysocki --- drivers/devfreq/governor_passive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/devfreq/governor_passive.c b/drivers/devfreq/governor_passive.c index 7306e943a234..72c67979ebe1 100644 --- a/drivers/devfreq/governor_passive.c +++ b/drivers/devfreq/governor_passive.c @@ -402,7 +402,7 @@ static int devfreq_passive_event_handler(struct devfreq *devfreq, { struct devfreq_passive_data *p_data = (struct devfreq_passive_data *)devfreq->data; - int ret = -EINVAL; + int ret = 0; if (!p_data) return -EINVAL; -- cgit v1.2.3 From 34be27517cb763ea367da21e3cdee5d1bc40f47f Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Sat, 14 May 2022 17:20:32 +0200 Subject: cpuidle: psci: Fix regression leading to no genpd governor While factoring out the PM domain related code from PSCI domain driver into a set of library functions, a regression when initializing the genpds got introduced. More precisely, we fail to assign a genpd governor, so let's fix this. Fixes: 9d976d6721df ("cpuidle: Factor-out power domain related code from PSCI domain driver") Signed-off-by: Ulf Hansson Reviewed-by: Anup Patel Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-psci-domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/cpuidle/cpuidle-psci-domain.c b/drivers/cpuidle/cpuidle-psci-domain.c index 755bbdfc5b82..3db4fca1172b 100644 --- a/drivers/cpuidle/cpuidle-psci-domain.c +++ b/drivers/cpuidle/cpuidle-psci-domain.c @@ -52,7 +52,7 @@ static int psci_pd_init(struct device_node *np, bool use_osi) struct generic_pm_domain *pd; struct psci_pd_provider *pd_provider; struct dev_power_governor *pd_gov; - int ret = -ENOMEM, state_count = 0; + int ret = -ENOMEM; pd = dt_idle_pd_alloc(np, psci_dt_parse_state_node); if (!pd) @@ -71,7 +71,7 @@ static int psci_pd_init(struct device_node *np, bool use_osi) pd->flags |= GENPD_FLAG_ALWAYS_ON; /* Use governor for CPU PM domains if it has some states to manage. */ - pd_gov = state_count > 0 ? &pm_domain_cpu_gov : NULL; + pd_gov = pd->states ? &pm_domain_cpu_gov : NULL; ret = pm_genpd_init(pd, pd_gov, false); if (ret) -- cgit v1.2.3 From a6653fb584b5f6ac60ddd5d86ddd49a1f3945a04 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Sat, 14 May 2022 17:20:44 +0200 Subject: cpuidle: riscv-sbi: Fix code to allow a genpd governor to be used The intent is to use a genpd governor when there are some states that needs to be managed. Although, the current code ends up to never assign a governor, let's fix this. Fixes: 6abf32f1d9c50 ("cpuidle: Add RISC-V SBI CPU idle driver") Signed-off-by: Ulf Hansson Reviewed-by: Anup Patel Tested-by: Anup Patel Signed-off-by: Rafael J. Wysocki --- drivers/cpuidle/cpuidle-riscv-sbi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/cpuidle/cpuidle-riscv-sbi.c b/drivers/cpuidle/cpuidle-riscv-sbi.c index 5c852e671992..1151e5e2ba82 100644 --- a/drivers/cpuidle/cpuidle-riscv-sbi.c +++ b/drivers/cpuidle/cpuidle-riscv-sbi.c @@ -414,7 +414,7 @@ static int sbi_pd_init(struct device_node *np) struct generic_pm_domain *pd; struct sbi_pd_provider *pd_provider; struct dev_power_governor *pd_gov; - int ret = -ENOMEM, state_count = 0; + int ret = -ENOMEM; pd = dt_idle_pd_alloc(np, sbi_dt_parse_state_node); if (!pd) @@ -433,7 +433,7 @@ static int sbi_pd_init(struct device_node *np) pd->flags |= GENPD_FLAG_ALWAYS_ON; /* Use governor for CPU PM domains if it has some states to manage. */ - pd_gov = state_count > 0 ? &pm_domain_cpu_gov : NULL; + pd_gov = pd->states ? &pm_domain_cpu_gov : NULL; ret = pm_genpd_init(pd, pd_gov, false); if (ret) -- cgit v1.2.3 From 0651ab90e4ade17f1d4f4367b70f6120480410f3 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Wed, 18 May 2022 11:08:57 +0200 Subject: ACPI: CPPC: Check _OSC for flexible address space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ACPI 6.2 Section 6.2.11.2 'Platform-Wide OSPM Capabilities': Starting with ACPI Specification 6.2, all _CPC registers can be in PCC, System Memory, System IO, or Functional Fixed Hardware address spaces. OSPM support for this more flexible register space scheme is indicated by the “Flexible Address Space for CPPC Registers” _OSC bit Otherwise (cf ACPI 6.1, s8.4.7.1.1.X), _CPC registers must be in: - PCC or Functional Fixed Hardware address space if defined - SystemMemory address space (NULL register) if not defined Add the corresponding _OSC bit and check it when parsing _CPC objects. Signed-off-by: Pierre Gondois Reviewed-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- drivers/acpi/bus.c | 18 ++++++++++++++++++ drivers/acpi/cppc_acpi.c | 9 +++++++++ include/linux/acpi.h | 2 ++ 3 files changed, 29 insertions(+) (limited to 'drivers') diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 3e58b613a2c4..9eca43d1d941 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -278,6 +278,20 @@ bool osc_sb_apei_support_acked; bool osc_pc_lpi_support_confirmed; EXPORT_SYMBOL_GPL(osc_pc_lpi_support_confirmed); +/* + * ACPI 6.2 Section 6.2.11.2 'Platform-Wide OSPM Capabilities': + * Starting with ACPI Specification 6.2, all _CPC registers can be in + * PCC, System Memory, System IO, or Functional Fixed Hardware address + * spaces. OSPM support for this more flexible register space scheme is + * indicated by the “Flexible Address Space for CPPC Registers” _OSC bit. + * + * Otherwise (cf ACPI 6.1, s8.4.7.1.1.X), _CPC registers must be in: + * - PCC or Functional Fixed Hardware address space if defined + * - SystemMemory address space (NULL register) if not defined + */ +bool osc_cpc_flexible_adr_space_confirmed; +EXPORT_SYMBOL_GPL(osc_cpc_flexible_adr_space_confirmed); + /* * ACPI 6.4 Operating System Capabilities for USB. */ @@ -321,6 +335,8 @@ static void acpi_bus_osc_negotiate_platform_control(void) } #endif + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_FLEXIBLE_ADR_SPACE; + if (IS_ENABLED(CONFIG_SCHED_MC_PRIO)) capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_DIVERSE_HIGH_SUPPORT; @@ -366,6 +382,8 @@ static void acpi_bus_osc_negotiate_platform_control(void) capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; osc_sb_native_usb4_support_confirmed = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; + osc_cpc_flexible_adr_space_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_CPC_FLEXIBLE_ADR_SPACE; } kfree(context.ret.pointer); diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index bc1454789a06..6f09fe011544 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -736,6 +736,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) if (gas_t->address) { void __iomem *addr; + if (!osc_cpc_flexible_adr_space_confirmed) { + pr_debug("Flexible address space capability not supported\n"); + goto out_free; + } + addr = ioremap(gas_t->address, gas_t->bit_width/8); if (!addr) goto out_free; @@ -758,6 +763,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) gas_t->address); goto out_free; } + if (!osc_cpc_flexible_adr_space_confirmed) { + pr_debug("Flexible address space capability not supported\n"); + goto out_free; + } } else { if (gas_t->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE || !cpc_ffh_supported()) { /* Support only PCC, SystemMemory, SystemIO, and FFH type regs. */ diff --git a/include/linux/acpi.h b/include/linux/acpi.h index d7136d13aa44..03465db16b68 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -574,6 +574,7 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context); #define OSC_SB_OSLPI_SUPPORT 0x00000100 #define OSC_SB_CPC_DIVERSE_HIGH_SUPPORT 0x00001000 #define OSC_SB_GENERIC_INITIATOR_SUPPORT 0x00002000 +#define OSC_SB_CPC_FLEXIBLE_ADR_SPACE 0x00004000 #define OSC_SB_NATIVE_USB4_SUPPORT 0x00040000 #define OSC_SB_PRM_SUPPORT 0x00200000 @@ -581,6 +582,7 @@ extern bool osc_sb_apei_support_acked; extern bool osc_pc_lpi_support_confirmed; extern bool osc_sb_native_usb4_support_confirmed; extern bool osc_sb_cppc_not_supported; +extern bool osc_cpc_flexible_adr_space_confirmed; /* USB4 Capabilities */ #define OSC_USB_USB3_TUNNELING 0x00000001 -- cgit v1.2.3 From 72f2ecb7ece7c1d89758d4929d98e95d95fe7199 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Wed, 18 May 2022 11:08:58 +0200 Subject: ACPI: bus: Set CPPC _OSC bits for all and when CPPC_LIB is supported The _OSC method allows the OS and firmware to communicate about supported features/capabitlities. It also allows the OS to take control of some features. In ACPI 6.4, s6.2.11.2 Platform-Wide OSPM Capabilities, the CPPC (resp. v2) bit should be set by the OS if it 'supports controlling processor performance via the interfaces described in the _CPC object'. The OS supports CPPC and parses the _CPC object only if CONFIG_ACPI_CPPC_LIB is set. Replace the x86 specific boot_cpu_has(X86_FEATURE_HWP) dynamic check with an arch generic CONFIG_ACPI_CPPC_LIB build-time check. Note: CONFIG_X86_INTEL_PSTATE selects CONFIG_ACPI_CPPC_LIB. Signed-off-by: Pierre Gondois Reviewed-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- drivers/acpi/bus.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 9eca43d1d941..1fc24f4fbcb4 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -329,10 +329,11 @@ static void acpi_bus_osc_negotiate_platform_control(void) #endif #ifdef CONFIG_X86 capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_GENERIC_INITIATOR_SUPPORT; - if (boot_cpu_has(X86_FEATURE_HWP)) { - capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT; - capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT; - } +#endif + +#ifdef CONFIG_ACPI_CPPC_LIB + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_SUPPORT; + capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPCV2_SUPPORT; #endif capbuf[OSC_SUPPORT_DWORD] |= OSC_SB_CPC_FLEXIBLE_ADR_SPACE; @@ -357,10 +358,9 @@ static void acpi_bus_osc_negotiate_platform_control(void) return; } -#ifdef CONFIG_X86 - if (boot_cpu_has(X86_FEATURE_HWP)) - osc_sb_cppc_not_supported = !(capbuf_ret[OSC_SUPPORT_DWORD] & - (OSC_SB_CPC_SUPPORT | OSC_SB_CPCV2_SUPPORT)); +#ifdef CONFIG_ACPI_CPPC_LIB + osc_sb_cppc_not_supported = !(capbuf_ret[OSC_SUPPORT_DWORD] & + (OSC_SB_CPC_SUPPORT | OSC_SB_CPCV2_SUPPORT)); #endif /* -- cgit v1.2.3 From 6380b7b2b29da9d9c5ab2d4a265901cd93ba3696 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Wed, 18 May 2022 11:08:59 +0200 Subject: ACPI: CPPC: Assume no transition latency if no PCCT The transition_delay_us (struct cpufreq_policy) is currently defined as: Preferred average time interval between consecutive invocations of the driver to set the frequency for this policy. To be set by the scaling driver (0, which is the default, means no preference). The transition_latency represents the amount of time necessary for a CPU to change its frequency. A PCCT table advertises mutliple values: - pcc_nominal: Expected latency to process a command, in microseconds - pcc_mpar: The maximum number of periodic requests that the subspace channel can support, reported in commands per minute. 0 indicates no limitation. - pcc_mrtt: The minimum amount of time that OSPM must wait after the completion of a command before issuing the next command, in microseconds. cppc_get_transition_latency() allows to get the max of them. commit d4f3388afd48 ("cpufreq / CPPC: Set platform specific transition_delay_us") allows to select transition_delay_us based on the platform, and fallbacks to cppc_get_transition_latency() otherwise. If _CPC objects are not using PCC channels (no PPCT table), the transition_delay_us is set to CPUFREQ_ETERNAL, leading to really long periods between frequency updates (~4s). If the desired_reg, where performance requests are written, is in SystemMemory or SystemIo ACPI address space, there is no delay in requests. So return 0 instead of CPUFREQ_ETERNAL, leading to transition_delay_us being set to LATENCY_MULTIPLIER us (1000 us). This patch also adds two macros to check the address spaces. Signed-off-by: Pierre Gondois Reviewed-by: Sudeep Holla Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 6f09fe011544..840223c12540 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -100,6 +100,16 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr); (cpc)->cpc_entry.reg.space_id == \ ACPI_ADR_SPACE_PLATFORM_COMM) +/* Check if a CPC register is in SystemMemory */ +#define CPC_IN_SYSTEM_MEMORY(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \ + (cpc)->cpc_entry.reg.space_id == \ + ACPI_ADR_SPACE_SYSTEM_MEMORY) + +/* Check if a CPC register is in SystemIo */ +#define CPC_IN_SYSTEM_IO(cpc) ((cpc)->type == ACPI_TYPE_BUFFER && \ + (cpc)->cpc_entry.reg.space_id == \ + ACPI_ADR_SPACE_SYSTEM_IO) + /* Evaluates to True if reg is a NULL register descriptor */ #define IS_NULL_REG(reg) ((reg)->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY && \ (reg)->address == 0 && \ @@ -1456,6 +1466,9 @@ EXPORT_SYMBOL_GPL(cppc_set_perf); * transition latency for performance change requests. The closest we have * is the timing information from the PCCT tables which provides the info * on the number and frequency of PCC commands the platform can handle. + * + * If desired_reg is in the SystemMemory or SystemIo ACPI address space, + * then assume there is no latency. */ unsigned int cppc_get_transition_latency(int cpu_num) { @@ -1481,7 +1494,9 @@ unsigned int cppc_get_transition_latency(int cpu_num) return CPUFREQ_ETERNAL; desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF]; - if (!CPC_IN_PCC(desired_reg)) + if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg)) + return 0; + else if (!CPC_IN_PCC(desired_reg)) return CPUFREQ_ETERNAL; if (pcc_ss_id < 0) -- cgit v1.2.3 From 3cc30dd00a580ca0c9c0b01639841cfd72d10129 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Wed, 18 May 2022 11:09:00 +0200 Subject: cpufreq: CPPC: Enable fast_switch The communication mean of the _CPC desired performance can be PCC, System Memory, System IO, or Functional Fixed Hardware. commit b7898fda5bc7 ("cpufreq: Support for fast frequency switching") fast_switching is 'for switching CPU frequencies from interrupt context'. Writes to SystemMemory and SystemIo are fast and suitable this. This is not the case for PCC and might not be the case for FFH. Enable fast_switching for the cppc_cpufreq driver in above cases. Add cppc_allow_fast_switch() to check the desired performance register address space and set fast_switching accordingly. Signed-off-by: Pierre Gondois Reviewed-by: Sudeep Holla Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 18 ++++++++++++++++++ drivers/cpufreq/cppc_cpufreq.c | 24 ++++++++++++++++++++++++ include/acpi/cppc_acpi.h | 5 +++++ 3 files changed, 47 insertions(+) (limited to 'drivers') diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 840223c12540..3b299b28a8af 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -434,6 +434,24 @@ bool acpi_cpc_valid(void) } EXPORT_SYMBOL_GPL(acpi_cpc_valid); +bool cppc_allow_fast_switch(void) +{ + struct cpc_register_resource *desired_reg; + struct cpc_desc *cpc_ptr; + int cpu; + + for_each_possible_cpu(cpu) { + cpc_ptr = per_cpu(cpc_desc_ptr, cpu); + desired_reg = &cpc_ptr->cpc_regs[DESIRED_PERF]; + if (!CPC_IN_SYSTEM_MEMORY(desired_reg) && + !CPC_IN_SYSTEM_IO(desired_reg)) + return false; + } + + return true; +} +EXPORT_SYMBOL_GPL(cppc_allow_fast_switch); + /** * acpi_get_psd_map - Map the CPUs in the freq domain of a given cpu * @cpu: Find all CPUs that share a domain with cpu. diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 3eaa23d1aaf5..bb4118cea2a0 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -389,6 +389,27 @@ static int cppc_cpufreq_set_target(struct cpufreq_policy *policy, return ret; } +static unsigned int cppc_cpufreq_fast_switch(struct cpufreq_policy *policy, + unsigned int target_freq) +{ + struct cppc_cpudata *cpu_data = policy->driver_data; + unsigned int cpu = policy->cpu; + u32 desired_perf; + int ret; + + desired_perf = cppc_cpufreq_khz_to_perf(cpu_data, target_freq); + cpu_data->perf_ctrls.desired_perf = desired_perf; + ret = cppc_set_perf(cpu, &cpu_data->perf_ctrls); + + if (ret) { + pr_debug("Failed to set target on CPU:%d. ret:%d\n", + cpu, ret); + return 0; + } + + return target_freq; +} + static int cppc_verify_policy(struct cpufreq_policy_data *policy) { cpufreq_verify_within_cpu_limits(policy); @@ -721,6 +742,8 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) goto out; } + policy->fast_switch_possible = cppc_allow_fast_switch(); + /* * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost * is supported. @@ -866,6 +889,7 @@ static struct cpufreq_driver cppc_cpufreq_driver = { .verify = cppc_verify_policy, .target = cppc_cpufreq_set_target, .get = cppc_cpufreq_get_rate, + .fast_switch = cppc_cpufreq_fast_switch, .init = cppc_cpufreq_cpu_init, .exit = cppc_cpufreq_cpu_exit, .set_boost = cppc_cpufreq_set_boost, diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h index 92b7ea8d8f5e..c6108581d97d 100644 --- a/include/acpi/cppc_acpi.h +++ b/include/acpi/cppc_acpi.h @@ -141,6 +141,7 @@ extern int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls); extern int cppc_set_enable(int cpu, bool enable); extern int cppc_get_perf_caps(int cpu, struct cppc_perf_caps *caps); extern bool acpi_cpc_valid(void); +extern bool cppc_allow_fast_switch(void); extern int acpi_get_psd_map(unsigned int cpu, struct cppc_cpudata *cpu_data); extern unsigned int cppc_get_transition_latency(int cpu); extern bool cpc_ffh_supported(void); @@ -175,6 +176,10 @@ static inline bool acpi_cpc_valid(void) { return false; } +static inline bool cppc_allow_fast_switch(void) +{ + return false; +} static inline unsigned int cppc_get_transition_latency(int cpu) { return CPUFREQ_ETERNAL; -- cgit v1.2.3 From 2d41dc23804d075e248ff01d16d9d424bcbc1180 Mon Sep 17 00:00:00 2001 From: Pierre Gondois Date: Wed, 18 May 2022 11:09:01 +0200 Subject: cpufreq: CPPC: Enable dvfs_possible_from_any_cpu The communication mean of the _CPC desired performance can be PCC, System Memory, System IO, or Functional Fixed Hardware (FFH). PCC, SystemMemory and SystemIo address spaces are available from any CPU. Thus, dvfs_possible_from_any_cpu should be enabled in such case. For FFH, let the FFH implementation do smp_call_function_*() calls. Signed-off-by: Pierre Gondois Reviewed-by: Sudeep Holla Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cppc_cpufreq.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index bb4118cea2a0..d092c9bb4ba3 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -743,6 +743,7 @@ static int cppc_cpufreq_cpu_init(struct cpufreq_policy *policy) } policy->fast_switch_possible = cppc_allow_fast_switch(); + policy->dvfs_possible_from_any_cpu = true; /* * If 'highest_perf' is greater than 'nominal_perf', we assume CPU Boost -- cgit v1.2.3 From 59cafa728c373551423d67a1369289a717006a4b Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 18 May 2022 09:45:36 +0100 Subject: powercap: intel_rapl: remove redundant store to value after multiply There is no need to store the result of the multiply back to variable value after the multiplication. The store is redundant, replace *= with just *. Cleans up clang scan build warning: warning: Although the value stored to 'value' is used in the enclosing expression, the value is never actually read from 'value' [deadcode.DeadStores] Signed-off-by: Colin Ian King Signed-off-by: Rafael J. Wysocki --- drivers/powercap/intel_rapl_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/powercap/intel_rapl_common.c b/drivers/powercap/intel_rapl_common.c index e45c1c3b0cae..a9c99d9e8b42 100644 --- a/drivers/powercap/intel_rapl_common.c +++ b/drivers/powercap/intel_rapl_common.c @@ -1010,7 +1010,7 @@ static u64 rapl_compute_time_window_atom(struct rapl_package *rp, u64 value, * where time_unit is default to 1 sec. Never 0. */ if (!to_raw) - return (value) ? value *= rp->time_unit : rp->time_unit; + return (value) ? value * rp->time_unit : rp->time_unit; value = div64_u64(value, rp->time_unit); -- cgit v1.2.3 From 27656dcd55f8065aa80dd1f9d948d7d39866a65a Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:51 +0200 Subject: PM: domains: Add GENPD_FLAG_RPM_ALWAYS_ON for the always-on governor Rather than relying on the genpd provider to set the corresponding flag, GENPD_FLAG_RPM_ALWAYS_ON, when the always-on governor is being used, let's add it in pm_genpd_init(). In this way, it starts to benefits all genpd providers immediately. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 18cd7965e5d6..262bda9b56ef 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2016,6 +2016,10 @@ int pm_genpd_init(struct generic_pm_domain *genpd, genpd->dev_ops.start = pm_clk_resume; } + /* The always-on governor works better with the corresponding flag. */ + if (gov == &pm_domain_always_on_gov) + genpd->flags |= GENPD_FLAG_RPM_ALWAYS_ON; + /* Always-on domains must be powered on at initialization. */ if ((genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd)) && !genpd_status_on(genpd)) -- cgit v1.2.3 From fabb1c20b96217e8d0ebb1307985da5599885d6c Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:52 +0200 Subject: PM: domains: Drop redundant code for genpd always-on governor Due to recent changes, the always-on governor is always used with a genpd that has the GENPD_FLAG_RPM_ALWAYS_ON flag being set. This means genpd, doesn't invoke the governor's ->power_down_ok() callback, which makes the code in the governor redundant, so let's drop it. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain_governor.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index cd08c5885190..c13e44823f16 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -327,11 +327,6 @@ static bool default_power_down_ok(struct dev_pm_domain *pd) return _default_power_down_ok(pd, ktime_get()); } -static bool always_on_power_down_ok(struct dev_pm_domain *domain) -{ - return false; -} - #ifdef CONFIG_CPU_IDLE static bool cpu_power_down_ok(struct dev_pm_domain *pd) { @@ -401,6 +396,5 @@ struct dev_power_governor simple_qos_governor = { * pm_genpd_gov_always_on - A governor implementing an always-on policy */ struct dev_power_governor pm_domain_always_on_gov = { - .power_down_ok = always_on_power_down_ok, .suspend_ok = default_suspend_ok, }; -- cgit v1.2.3 From 3f9ee7da724a57d808b52e6e9f53517ef3b00db6 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:53 +0200 Subject: PM: domains: Don't check PM_QOS_FLAG_NO_POWER_OFF in genpd Back in the days when genpd supported intermediate power states of its devices, it made sense to check the PM_QOS_FLAG_NO_POWER_OFF in genpd_power_off(). This because the attached devices were all being put into low power state together when the PM domain was also being powered off. At this point, the flag PM_QOS_FLAG_NO_POWER_OFF is better checked by drivers from their ->runtime_suspend() callbacks, like in the usb_port_runtime_suspend(), for example. Or perhaps an even better option is to set the QoS resume latency constraint for the device to zero, which informs the runtime PM core to prevent the device from being runtime suspended. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 262bda9b56ef..5059c5d2e64a 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -648,12 +648,6 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, } list_for_each_entry(pdd, &genpd->dev_list, list_node) { - enum pm_qos_flags_status stat; - - stat = dev_pm_qos_flags(pdd->dev, PM_QOS_FLAG_NO_POWER_OFF); - if (stat > PM_QOS_FLAGS_NONE) - return -EBUSY; - /* * Do not allow PM domain to be powered off, when an IRQ safe * device is part of a non-IRQ safe domain. -- cgit v1.2.3 From 7a02444b8fc25ac16f90c1e498aeb672651d6f4b Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:54 +0200 Subject: PM: domains: Rename irq_safe_dev_in_no_sleep_domain() in genpd The name "irq_safe_dev_in_no_sleep_domain", doesn't really match the conditions that are being checked in the function, hence the code becomes a bit confusing to read. Let's clarify this by renaming it into "irq_safe_dev_in_sleep_domain" and let's also take the opportunity to clarify a corresponding comment in the code. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 5059c5d2e64a..269bfa3d2303 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -131,7 +131,7 @@ static const struct genpd_lock_ops genpd_spin_ops = { #define genpd_is_cpu_domain(genpd) (genpd->flags & GENPD_FLAG_CPU_DOMAIN) #define genpd_is_rpm_always_on(genpd) (genpd->flags & GENPD_FLAG_RPM_ALWAYS_ON) -static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev, +static inline bool irq_safe_dev_in_sleep_domain(struct device *dev, const struct generic_pm_domain *genpd) { bool ret; @@ -139,9 +139,9 @@ static inline bool irq_safe_dev_in_no_sleep_domain(struct device *dev, ret = pm_runtime_is_irq_safe(dev) && !genpd_is_irq_safe(genpd); /* - * Warn once if an IRQ safe device is attached to a no sleep domain, as - * to indicate a suboptimal configuration for PM. For an always on - * domain this isn't case, thus don't warn. + * Warn once if an IRQ safe device is attached to a domain, which + * callbacks are allowed to sleep. This indicates a suboptimal + * configuration for PM, but it doesn't matter for an always on domain. */ if (ret && !genpd_is_always_on(genpd)) dev_warn_once(dev, "PM domain %s will not be powered off\n", @@ -653,7 +653,7 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on, * device is part of a non-IRQ safe domain. */ if (!pm_runtime_suspended(pdd->dev) || - irq_safe_dev_in_no_sleep_domain(pdd->dev, genpd)) + irq_safe_dev_in_sleep_domain(pdd->dev, genpd)) not_suspended++; } @@ -925,7 +925,7 @@ static int genpd_runtime_suspend(struct device *dev) * If power.irq_safe is set, this routine may be run with * IRQs disabled, so suspend only if the PM domain also is irq_safe. */ - if (irq_safe_dev_in_no_sleep_domain(dev, genpd)) + if (irq_safe_dev_in_sleep_domain(dev, genpd)) return 0; genpd_lock(genpd); @@ -965,7 +965,7 @@ static int genpd_runtime_resume(struct device *dev) * As we don't power off a non IRQ safe domain, which holds * an IRQ safe device, we don't need to restore power to it. */ - if (irq_safe_dev_in_no_sleep_domain(dev, genpd)) { + if (irq_safe_dev_in_sleep_domain(dev, genpd)) { timed = false; goto out; } -- cgit v1.2.3 From bcc19f69f72ab7e3db581b993d6a9fc1bad5ddf4 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:55 +0200 Subject: PM: domains: Skip another warning in irq_safe_dev_in_sleep_domain() In irq_safe_dev_in_sleep_domain() we correctly skip the dev_warn_once() if the corresponding genpd for the device, has the GENPD_FLAG_ALWAYS_ON flag being set. For the same reason (the genpd is always-on in runtime), let's also skip the warning if the GENPD_FLAG_RPM_ALWAYS_ON flag is set for the genpd. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 269bfa3d2303..a3e727c9a4fa 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -143,7 +143,10 @@ static inline bool irq_safe_dev_in_sleep_domain(struct device *dev, * callbacks are allowed to sleep. This indicates a suboptimal * configuration for PM, but it doesn't matter for an always on domain. */ - if (ret && !genpd_is_always_on(genpd)) + if (genpd_is_always_on(genpd) || genpd_is_rpm_always_on(genpd)) + return ret; + + if (ret) dev_warn_once(dev, "PM domain %s will not be powered off\n", genpd->name); -- cgit v1.2.3 From 66d29d802ef3bf55a49b07568b0048823d4a72a6 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:56 +0200 Subject: PM: domains: Allocate gpd_timing_data dynamically based on governor If a genpd doesn't have an associated governor assigned, there's really no point to allocate the per device gpd_timing_data, as the data isn't being used by a governor anyway. To avoid wasting memory, let's therefore convert the corresponding td variable in the struct generic_pm_domain_data into a pointer and manage the allocation of its data dynamically. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 48 +++++++++++++++++++++++++----------- drivers/base/power/domain_governor.c | 9 ++++--- include/linux/pm_domain.h | 2 +- 3 files changed, 40 insertions(+), 19 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index a3e727c9a4fa..ca86fb3db901 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -773,13 +773,16 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb, for (;;) { struct generic_pm_domain *genpd; struct pm_domain_data *pdd; + struct gpd_timing_data *td; spin_lock_irq(&dev->power.lock); pdd = dev->power.subsys_data ? dev->power.subsys_data->domain_data : NULL; if (pdd) { - to_gpd_data(pdd)->td.constraint_changed = true; + td = to_gpd_data(pdd)->td; + if (td) + td->constraint_changed = true; genpd = dev_to_genpd(dev); } else { genpd = ERR_PTR(-ENODATA); @@ -875,7 +878,7 @@ static int genpd_runtime_suspend(struct device *dev) struct generic_pm_domain *genpd; bool (*suspend_ok)(struct device *__dev); struct generic_pm_domain_data *gpd_data = dev_gpd_data(dev); - struct gpd_timing_data *td = &gpd_data->td; + struct gpd_timing_data *td = gpd_data->td; bool runtime_pm = pm_runtime_enabled(dev); ktime_t time_start; s64 elapsed_ns; @@ -915,7 +918,7 @@ static int genpd_runtime_suspend(struct device *dev) /* Update suspend latency value if the measured time exceeds it. */ if (runtime_pm) { elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); - if (elapsed_ns > td->suspend_latency_ns) { + if (td && (elapsed_ns > td->suspend_latency_ns)) { td->suspend_latency_ns = elapsed_ns; dev_dbg(dev, "suspend latency exceeded, %lld ns\n", elapsed_ns); @@ -951,7 +954,7 @@ static int genpd_runtime_resume(struct device *dev) { struct generic_pm_domain *genpd; struct generic_pm_domain_data *gpd_data = dev_gpd_data(dev); - struct gpd_timing_data *td = &gpd_data->td; + struct gpd_timing_data *td = gpd_data->td; bool runtime_pm = pm_runtime_enabled(dev); ktime_t time_start; s64 elapsed_ns; @@ -999,7 +1002,7 @@ static int genpd_runtime_resume(struct device *dev) /* Update resume latency value if the measured time exceeds it. */ if (timed && runtime_pm) { elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); - if (elapsed_ns > td->resume_latency_ns) { + if (td && (elapsed_ns > td->resume_latency_ns)) { td->resume_latency_ns = elapsed_ns; dev_dbg(dev, "resume latency exceeded, %lld ns\n", elapsed_ns); @@ -1496,9 +1499,11 @@ EXPORT_SYMBOL_GPL(dev_pm_genpd_resume); #endif /* CONFIG_PM_SLEEP */ -static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev) +static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev, + bool has_governor) { struct generic_pm_domain_data *gpd_data; + struct gpd_timing_data *td; int ret; ret = dev_pm_get_subsys_data(dev); @@ -1512,26 +1517,38 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev) } gpd_data->base.dev = dev; - gpd_data->td.constraint_changed = true; - gpd_data->td.effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; gpd_data->next_wakeup = KTIME_MAX; - spin_lock_irq(&dev->power.lock); + /* Allocate data used by a governor. */ + if (has_governor) { + td = kzalloc(sizeof(*td), GFP_KERNEL); + if (!td) { + ret = -ENOMEM; + goto err_free; + } - if (dev->power.subsys_data->domain_data) { - ret = -EINVAL; - goto err_free; + td->constraint_changed = true; + td->effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; + gpd_data->td = td; } - dev->power.subsys_data->domain_data = &gpd_data->base; + spin_lock_irq(&dev->power.lock); + + if (dev->power.subsys_data->domain_data) + ret = -EINVAL; + else + dev->power.subsys_data->domain_data = &gpd_data->base; spin_unlock_irq(&dev->power.lock); + if (ret) + goto err_free; + return gpd_data; err_free: - spin_unlock_irq(&dev->power.lock); + kfree(gpd_data->td); kfree(gpd_data); err_put: dev_pm_put_subsys_data(dev); @@ -1547,6 +1564,7 @@ static void genpd_free_dev_data(struct device *dev, spin_unlock_irq(&dev->power.lock); + kfree(gpd_data->td); kfree(gpd_data); dev_pm_put_subsys_data(dev); } @@ -1611,7 +1629,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)) return -EINVAL; - gpd_data = genpd_alloc_dev_data(dev); + gpd_data = genpd_alloc_dev_data(dev, genpd->gov); if (IS_ERR(gpd_data)) return PTR_ERR(gpd_data); diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index c13e44823f16..62be9b3f8c90 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -18,6 +18,8 @@ static int dev_update_qos_constraint(struct device *dev, void *data) s64 constraint_ns; if (dev->power.subsys_data && dev->power.subsys_data->domain_data) { + struct gpd_timing_data *td = dev_gpd_data(dev)->td; + /* * Only take suspend-time QoS constraints of devices into * account, because constraints updated after the device has @@ -25,7 +27,8 @@ static int dev_update_qos_constraint(struct device *dev, void *data) * anyway. In order for them to take effect, the device has to * be resumed and suspended again. */ - constraint_ns = dev_gpd_data(dev)->td.effective_constraint_ns; + constraint_ns = td ? td->effective_constraint_ns : + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; } else { /* * The child is not in a domain and there's no info on its @@ -49,7 +52,7 @@ static int dev_update_qos_constraint(struct device *dev, void *data) */ static bool default_suspend_ok(struct device *dev) { - struct gpd_timing_data *td = &dev_gpd_data(dev)->td; + struct gpd_timing_data *td = dev_gpd_data(dev)->td; unsigned long flags; s64 constraint_ns; @@ -215,7 +218,7 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd, * domain to turn off and on (that's how much time it will * have to wait worst case). */ - td = &to_gpd_data(pdd)->td; + td = to_gpd_data(pdd)->td; constraint_ns = td->effective_constraint_ns; /* * Zero means "no suspend at all" and this runs only when all diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 043d48e4420a..126a4b9ab215 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -193,7 +193,7 @@ struct pm_domain_data { struct generic_pm_domain_data { struct pm_domain_data base; - struct gpd_timing_data td; + struct gpd_timing_data *td; struct notifier_block nb; struct notifier_block *power_nb; int cpu; -- cgit v1.2.3 From 9c74f2ac4801473d03b0e29fab74eb94a9944521 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:57 +0200 Subject: PM: domains: Move the next_wakeup variable into the struct gpd_timing_data If the corresponding genpd for the device doesn't use a governor, the variable next_wakeup within the struct generic_pm_domain_data becomes superfluous. To avoid wasting memory, let's move it into the struct gpd_timing_data, which is already being allocated based upon if there is governor assigned. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 9 +++++---- drivers/base/power/domain_governor.c | 2 +- include/linux/pm_domain.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index ca86fb3db901..4c059a858957 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -478,15 +478,16 @@ EXPORT_SYMBOL_GPL(dev_pm_genpd_set_performance_state); */ void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next) { - struct generic_pm_domain_data *gpd_data; struct generic_pm_domain *genpd; + struct gpd_timing_data *td; genpd = dev_to_genpd_safe(dev); if (!genpd) return; - gpd_data = to_gpd_data(dev->power.subsys_data->domain_data); - gpd_data->next_wakeup = next; + td = to_gpd_data(dev->power.subsys_data->domain_data)->td; + if (td) + td->next_wakeup = next; } EXPORT_SYMBOL_GPL(dev_pm_genpd_set_next_wakeup); @@ -1518,7 +1519,6 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev, gpd_data->base.dev = dev; gpd_data->nb.notifier_call = genpd_dev_pm_qos_notifier; - gpd_data->next_wakeup = KTIME_MAX; /* Allocate data used by a governor. */ if (has_governor) { @@ -1530,6 +1530,7 @@ static struct generic_pm_domain_data *genpd_alloc_dev_data(struct device *dev, td->constraint_changed = true; td->effective_constraint_ns = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS; + td->next_wakeup = KTIME_MAX; gpd_data->td = td; } diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index 62be9b3f8c90..599b3909821c 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -139,7 +139,7 @@ static void update_domain_next_wakeup(struct generic_pm_domain *genpd, ktime_t n * is able to enter its optimal idle state. */ list_for_each_entry(pdd, &genpd->dev_list, list_node) { - next_wakeup = to_gpd_data(pdd)->next_wakeup; + next_wakeup = to_gpd_data(pdd)->td->next_wakeup; if (next_wakeup != KTIME_MAX && !ktime_before(next_wakeup, now)) if (ktime_before(next_wakeup, domain_wakeup)) domain_wakeup = next_wakeup; diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 126a4b9ab215..1f370f074f30 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -182,6 +182,7 @@ struct gpd_timing_data { s64 suspend_latency_ns; s64 resume_latency_ns; s64 effective_constraint_ns; + ktime_t next_wakeup; bool constraint_changed; bool cached_suspend_ok; }; @@ -200,7 +201,6 @@ struct generic_pm_domain_data { unsigned int performance_state; unsigned int default_pstate; unsigned int rpm_pstate; - ktime_t next_wakeup; void *data; }; -- cgit v1.2.3 From 3b84bf3ce94dea82a9451a4c9c2cda37e334333c Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:58 +0200 Subject: PM: domains: Measure suspend/resume latencies in genpd based on governor The QoS latency measurements for devices in genpd_runtime_suspend|resume() are superfluous, unless the corresponding genpd has a governor assigned to it, which would make use of the data. Therefore, let's improve the behaviour in genpd by making the measurements conditional, based upon if there's a governor assigned. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 4c059a858957..2cdfbe48dde0 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -881,7 +881,7 @@ static int genpd_runtime_suspend(struct device *dev) struct generic_pm_domain_data *gpd_data = dev_gpd_data(dev); struct gpd_timing_data *td = gpd_data->td; bool runtime_pm = pm_runtime_enabled(dev); - ktime_t time_start; + ktime_t time_start = 0; s64 elapsed_ns; int ret; @@ -902,8 +902,7 @@ static int genpd_runtime_suspend(struct device *dev) return -EBUSY; /* Measure suspend latency. */ - time_start = 0; - if (runtime_pm) + if (td && runtime_pm) time_start = ktime_get(); ret = __genpd_runtime_suspend(dev); @@ -917,9 +916,9 @@ static int genpd_runtime_suspend(struct device *dev) } /* Update suspend latency value if the measured time exceeds it. */ - if (runtime_pm) { + if (td && runtime_pm) { elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); - if (td && (elapsed_ns > td->suspend_latency_ns)) { + if (elapsed_ns > td->suspend_latency_ns) { td->suspend_latency_ns = elapsed_ns; dev_dbg(dev, "suspend latency exceeded, %lld ns\n", elapsed_ns); @@ -956,11 +955,10 @@ static int genpd_runtime_resume(struct device *dev) struct generic_pm_domain *genpd; struct generic_pm_domain_data *gpd_data = dev_gpd_data(dev); struct gpd_timing_data *td = gpd_data->td; - bool runtime_pm = pm_runtime_enabled(dev); - ktime_t time_start; + bool timed = td && pm_runtime_enabled(dev); + ktime_t time_start = 0; s64 elapsed_ns; int ret; - bool timed = true; dev_dbg(dev, "%s()\n", __func__); @@ -988,8 +986,7 @@ static int genpd_runtime_resume(struct device *dev) out: /* Measure resume latency. */ - time_start = 0; - if (timed && runtime_pm) + if (timed) time_start = ktime_get(); ret = genpd_start_dev(genpd, dev); @@ -1001,9 +998,9 @@ static int genpd_runtime_resume(struct device *dev) goto err_stop; /* Update resume latency value if the measured time exceeds it. */ - if (timed && runtime_pm) { + if (timed) { elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start)); - if (td && (elapsed_ns > td->resume_latency_ns)) { + if (elapsed_ns > td->resume_latency_ns) { td->resume_latency_ns = elapsed_ns; dev_dbg(dev, "resume latency exceeded, %lld ns\n", elapsed_ns); -- cgit v1.2.3 From a294237aea122daa646f02e461877d553439c529 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:56:59 +0200 Subject: PM: domains: Fixup QoS latency measurements for IRQ safe devices in genpd When an IRQ safe device is attached to a non-IRQ safe PM domain, genpd needs to prevent the PM domain from being powered off. However, genpd still allows the device to be runtime suspended/resumed, hence it's also reasonable to think that a governor may be used to validate the QoS latency constraints. Unfortunately, genpd_runtime_resume() treats the configuration above, as a reason to skip measuring the QoS resume latency for the device. This is a legacy behaviour that was earlier correct, but should have been changed when genpd was transformed into its current behaviour around how it manages IRQ safe devices. Luckily, there's no report about problems, so let's just fixup the behaviour. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 2cdfbe48dde0..48101232fcb9 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -970,10 +970,8 @@ static int genpd_runtime_resume(struct device *dev) * As we don't power off a non IRQ safe domain, which holds * an IRQ safe device, we don't need to restore power to it. */ - if (irq_safe_dev_in_sleep_domain(dev, genpd)) { - timed = false; + if (irq_safe_dev_in_sleep_domain(dev, genpd)) goto out; - } genpd_lock(genpd); ret = genpd_power_on(genpd, 0); -- cgit v1.2.3 From 622d9b5577f19a6472db21df042fea8f5fefe244 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:57:00 +0200 Subject: PM: domains: Fix initialization of genpd's next_wakeup In the genpd governor we walk the list of child-domains to take into account their next_wakeup. If the child-domain itself, doesn't have a governor assigned to it, we can end up using the next_wakeup value before it has been properly initialized. To prevent a possible incorrect behaviour in the governor, let's initialize next_wakeup to KTIME_MAX. Fixes: c79aa080fb0f ("PM: domains: use device's next wakeup to determine domain idle state") Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 48101232fcb9..9922fd763739 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -2007,6 +2007,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd, genpd->device_count = 0; genpd->max_off_time_ns = -1; genpd->max_off_time_changed = true; + genpd->next_wakeup = KTIME_MAX; genpd->provider = NULL; genpd->has_provider = false; genpd->accounting_time = ktime_get_mono_fast_ns(); -- cgit v1.2.3 From ba43d6db6e5c6d0b5f58d6487285ed58ba94c5df Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:57:01 +0200 Subject: PM: domains: Clean up some code in pm_genpd_init() and genpd_remove() To improve the readability of the code, let's move the parts that deals with allocation/freeing of data, into two separate functions. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 56 ++++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 18 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 9922fd763739..ac1eb7d2470b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1968,6 +1968,37 @@ static int genpd_set_default_power_state(struct generic_pm_domain *genpd) return 0; } +static int genpd_alloc_data(struct generic_pm_domain *genpd) +{ + int ret; + + if (genpd_is_cpu_domain(genpd) && + !zalloc_cpumask_var(&genpd->cpus, GFP_KERNEL)) + return -ENOMEM; + + /* Use only one "off" state if there were no states declared */ + if (genpd->state_count == 0) { + ret = genpd_set_default_power_state(genpd); + if (ret) + goto free; + } + + return 0; + +free: + if (genpd_is_cpu_domain(genpd)) + free_cpumask_var(genpd->cpus); + return ret; +} + +static void genpd_free_data(struct generic_pm_domain *genpd) +{ + if (genpd_is_cpu_domain(genpd)) + free_cpumask_var(genpd->cpus); + if (genpd->free_states) + genpd->free_states(genpd->states, genpd->state_count); +} + static void genpd_lock_init(struct generic_pm_domain *genpd) { if (genpd->flags & GENPD_FLAG_IRQ_SAFE) { @@ -2037,21 +2068,13 @@ int pm_genpd_init(struct generic_pm_domain *genpd, !genpd_status_on(genpd)) return -EINVAL; - if (genpd_is_cpu_domain(genpd) && - !zalloc_cpumask_var(&genpd->cpus, GFP_KERNEL)) - return -ENOMEM; - - /* Use only one "off" state if there were no states declared */ - if (genpd->state_count == 0) { - ret = genpd_set_default_power_state(genpd); - if (ret) { - if (genpd_is_cpu_domain(genpd)) - free_cpumask_var(genpd->cpus); - return ret; - } - } else if (!gov && genpd->state_count > 1) { + /* Multiple states but no governor doesn't make sense. */ + if (!gov && genpd->state_count > 1) pr_warn("%s: no governor for states\n", genpd->name); - } + + ret = genpd_alloc_data(genpd); + if (ret) + return ret; device_initialize(&genpd->dev); dev_set_name(&genpd->dev, "%s", genpd->name); @@ -2096,10 +2119,7 @@ static int genpd_remove(struct generic_pm_domain *genpd) genpd_unlock(genpd); genpd_debug_remove(genpd); cancel_work_sync(&genpd->power_off_work); - if (genpd_is_cpu_domain(genpd)) - free_cpumask_var(genpd->cpus); - if (genpd->free_states) - genpd->free_states(genpd->states, genpd->state_count); + genpd_free_data(genpd); pr_debug("%s: removed %s\n", __func__, genpd->name); -- cgit v1.2.3 From f38d1a6d002526a4e8840e9bb19733e9d4ce1a67 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:57:02 +0200 Subject: PM: domains: Allocate governor data dynamically based on a genpd governor If a genpd doesn't have an associated governor assigned, several variables in the struct generic_pm_domain becomes superfluous. Rather than wasting memory in allocated genpds, let's move the variables from the struct generic_pm_domain into a new separate struct. In this way, we can instead dynamically decide when we need to allocate the corresponding data for it. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 49 ++++++++++++++++++++++++------------ drivers/base/power/domain_governor.c | 48 ++++++++++++++++++++--------------- include/linux/pm_domain.h | 14 +++++++---- 3 files changed, 70 insertions(+), 41 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index ac1eb7d2470b..a7de5fb8f073 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -527,7 +527,8 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) goto out; genpd->states[state_idx].power_on_latency_ns = elapsed_ns; - genpd->max_off_time_changed = true; + if (genpd->gd) + genpd->gd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "on", elapsed_ns); @@ -576,7 +577,8 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) goto out; genpd->states[state_idx].power_off_latency_ns = elapsed_ns; - genpd->max_off_time_changed = true; + if (genpd->gd) + genpd->gd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "off", elapsed_ns); @@ -772,7 +774,7 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb, dev = gpd_data->base.dev; for (;;) { - struct generic_pm_domain *genpd; + struct generic_pm_domain *genpd = ERR_PTR(-ENODATA); struct pm_domain_data *pdd; struct gpd_timing_data *td; @@ -782,18 +784,17 @@ static int genpd_dev_pm_qos_notifier(struct notifier_block *nb, dev->power.subsys_data->domain_data : NULL; if (pdd) { td = to_gpd_data(pdd)->td; - if (td) + if (td) { td->constraint_changed = true; - genpd = dev_to_genpd(dev); - } else { - genpd = ERR_PTR(-ENODATA); + genpd = dev_to_genpd(dev); + } } spin_unlock_irq(&dev->power.lock); if (!IS_ERR(genpd)) { genpd_lock(genpd); - genpd->max_off_time_changed = true; + genpd->gd->max_off_time_changed = true; genpd_unlock(genpd); } @@ -922,7 +923,7 @@ static int genpd_runtime_suspend(struct device *dev) td->suspend_latency_ns = elapsed_ns; dev_dbg(dev, "suspend latency exceeded, %lld ns\n", elapsed_ns); - genpd->max_off_time_changed = true; + genpd->gd->max_off_time_changed = true; td->constraint_changed = true; } } @@ -1002,7 +1003,7 @@ static int genpd_runtime_resume(struct device *dev) td->resume_latency_ns = elapsed_ns; dev_dbg(dev, "resume latency exceeded, %lld ns\n", elapsed_ns); - genpd->max_off_time_changed = true; + genpd->gd->max_off_time_changed = true; td->constraint_changed = true; } } @@ -1617,6 +1618,7 @@ static int genpd_get_cpu(struct generic_pm_domain *genpd, struct device *dev) static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, struct device *base_dev) { + struct genpd_governor_data *gd = genpd->gd; struct generic_pm_domain_data *gpd_data; int ret; @@ -1625,7 +1627,7 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev)) return -EINVAL; - gpd_data = genpd_alloc_dev_data(dev, genpd->gov); + gpd_data = genpd_alloc_dev_data(dev, gd); if (IS_ERR(gpd_data)) return PTR_ERR(gpd_data); @@ -1641,7 +1643,8 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, dev_pm_domain_set(dev, &genpd->domain); genpd->device_count++; - genpd->max_off_time_changed = true; + if (gd) + gd->max_off_time_changed = true; list_add_tail(&gpd_data->base.list_node, &genpd->dev_list); @@ -1695,7 +1698,8 @@ static int genpd_remove_device(struct generic_pm_domain *genpd, } genpd->device_count--; - genpd->max_off_time_changed = true; + if (genpd->gd) + genpd->gd->max_off_time_changed = true; genpd_clear_cpumask(genpd, gpd_data->cpu); dev_pm_domain_set(dev, NULL); @@ -1970,12 +1974,25 @@ static int genpd_set_default_power_state(struct generic_pm_domain *genpd) static int genpd_alloc_data(struct generic_pm_domain *genpd) { + struct genpd_governor_data *gd = NULL; int ret; if (genpd_is_cpu_domain(genpd) && !zalloc_cpumask_var(&genpd->cpus, GFP_KERNEL)) return -ENOMEM; + if (genpd->gov) { + gd = kzalloc(sizeof(*gd), GFP_KERNEL); + if (!gd) { + ret = -ENOMEM; + goto free; + } + + gd->max_off_time_ns = -1; + gd->max_off_time_changed = true; + gd->next_wakeup = KTIME_MAX; + } + /* Use only one "off" state if there were no states declared */ if (genpd->state_count == 0) { ret = genpd_set_default_power_state(genpd); @@ -1983,11 +2000,13 @@ static int genpd_alloc_data(struct generic_pm_domain *genpd) goto free; } + genpd->gd = gd; return 0; free: if (genpd_is_cpu_domain(genpd)) free_cpumask_var(genpd->cpus); + kfree(gd); return ret; } @@ -1997,6 +2016,7 @@ static void genpd_free_data(struct generic_pm_domain *genpd) free_cpumask_var(genpd->cpus); if (genpd->free_states) genpd->free_states(genpd->states, genpd->state_count); + kfree(genpd->gd); } static void genpd_lock_init(struct generic_pm_domain *genpd) @@ -2036,9 +2056,6 @@ int pm_genpd_init(struct generic_pm_domain *genpd, atomic_set(&genpd->sd_count, 0); genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON; genpd->device_count = 0; - genpd->max_off_time_ns = -1; - genpd->max_off_time_changed = true; - genpd->next_wakeup = KTIME_MAX; genpd->provider = NULL; genpd->has_provider = false; genpd->accounting_time = ktime_get_mono_fast_ns(); diff --git a/drivers/base/power/domain_governor.c b/drivers/base/power/domain_governor.c index 599b3909821c..282a3a135827 100644 --- a/drivers/base/power/domain_governor.c +++ b/drivers/base/power/domain_governor.c @@ -146,19 +146,21 @@ static void update_domain_next_wakeup(struct generic_pm_domain *genpd, ktime_t n } list_for_each_entry(link, &genpd->parent_links, parent_node) { - next_wakeup = link->child->next_wakeup; + struct genpd_governor_data *cgd = link->child->gd; + + next_wakeup = cgd ? cgd->next_wakeup : KTIME_MAX; if (next_wakeup != KTIME_MAX && !ktime_before(next_wakeup, now)) if (ktime_before(next_wakeup, domain_wakeup)) domain_wakeup = next_wakeup; } - genpd->next_wakeup = domain_wakeup; + genpd->gd->next_wakeup = domain_wakeup; } static bool next_wakeup_allows_state(struct generic_pm_domain *genpd, unsigned int state, ktime_t now) { - ktime_t domain_wakeup = genpd->next_wakeup; + ktime_t domain_wakeup = genpd->gd->next_wakeup; s64 idle_time_ns, min_sleep_ns; min_sleep_ns = genpd->states[state].power_off_latency_ns + @@ -188,8 +190,9 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd, * All subdomains have been powered off already at this point. */ list_for_each_entry(link, &genpd->parent_links, parent_node) { - struct generic_pm_domain *sd = link->child; - s64 sd_max_off_ns = sd->max_off_time_ns; + struct genpd_governor_data *cgd = link->child->gd; + + s64 sd_max_off_ns = cgd ? cgd->max_off_time_ns : -1; if (sd_max_off_ns < 0) continue; @@ -247,7 +250,7 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd, * time and the time needed to turn the domain on is the maximum * theoretical time this domain can spend in the "off" state. */ - genpd->max_off_time_ns = min_off_time_ns - + genpd->gd->max_off_time_ns = min_off_time_ns - genpd->states[state].power_on_latency_ns; return true; } @@ -262,6 +265,7 @@ static bool __default_power_down_ok(struct dev_pm_domain *pd, static bool _default_power_down_ok(struct dev_pm_domain *pd, ktime_t now) { struct generic_pm_domain *genpd = pd_to_genpd(pd); + struct genpd_governor_data *gd = genpd->gd; int state_idx = genpd->state_count - 1; struct gpd_link *link; @@ -272,11 +276,11 @@ static bool _default_power_down_ok(struct dev_pm_domain *pd, ktime_t now) * cannot be met. */ update_domain_next_wakeup(genpd, now); - if ((genpd->flags & GENPD_FLAG_MIN_RESIDENCY) && (genpd->next_wakeup != KTIME_MAX)) { + if ((genpd->flags & GENPD_FLAG_MIN_RESIDENCY) && (gd->next_wakeup != KTIME_MAX)) { /* Let's find out the deepest domain idle state, the devices prefer */ while (state_idx >= 0) { if (next_wakeup_allows_state(genpd, state_idx, now)) { - genpd->max_off_time_changed = true; + gd->max_off_time_changed = true; break; } state_idx--; @@ -284,14 +288,14 @@ static bool _default_power_down_ok(struct dev_pm_domain *pd, ktime_t now) if (state_idx < 0) { state_idx = 0; - genpd->cached_power_down_ok = false; + gd->cached_power_down_ok = false; goto done; } } - if (!genpd->max_off_time_changed) { - genpd->state_idx = genpd->cached_power_down_state_idx; - return genpd->cached_power_down_ok; + if (!gd->max_off_time_changed) { + genpd->state_idx = gd->cached_power_down_state_idx; + return gd->cached_power_down_ok; } /* @@ -300,12 +304,16 @@ static bool _default_power_down_ok(struct dev_pm_domain *pd, ktime_t now) * going to be called for any parent until this instance * returns. */ - list_for_each_entry(link, &genpd->child_links, child_node) - link->parent->max_off_time_changed = true; + list_for_each_entry(link, &genpd->child_links, child_node) { + struct genpd_governor_data *pgd = link->parent->gd; + + if (pgd) + pgd->max_off_time_changed = true; + } - genpd->max_off_time_ns = -1; - genpd->max_off_time_changed = false; - genpd->cached_power_down_ok = true; + gd->max_off_time_ns = -1; + gd->max_off_time_changed = false; + gd->cached_power_down_ok = true; /* * Find a state to power down to, starting from the state @@ -313,7 +321,7 @@ static bool _default_power_down_ok(struct dev_pm_domain *pd, ktime_t now) */ while (!__default_power_down_ok(pd, state_idx)) { if (state_idx == 0) { - genpd->cached_power_down_ok = false; + gd->cached_power_down_ok = false; break; } state_idx--; @@ -321,8 +329,8 @@ static bool _default_power_down_ok(struct dev_pm_domain *pd, ktime_t now) done: genpd->state_idx = state_idx; - genpd->cached_power_down_state_idx = genpd->state_idx; - return genpd->cached_power_down_ok; + gd->cached_power_down_state_idx = genpd->state_idx; + return gd->cached_power_down_ok; } static bool default_power_down_ok(struct dev_pm_domain *pd) diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 1f370f074f30..ebc351698090 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -91,6 +91,14 @@ struct gpd_dev_ops { int (*stop)(struct device *dev); }; +struct genpd_governor_data { + s64 max_off_time_ns; + bool max_off_time_changed; + ktime_t next_wakeup; + bool cached_power_down_ok; + bool cached_power_down_state_idx; +}; + struct genpd_power_state { s64 power_off_latency_ns; s64 power_on_latency_ns; @@ -114,6 +122,7 @@ struct generic_pm_domain { struct list_head child_links; /* Links with PM domain as a child */ struct list_head dev_list; /* List of devices */ struct dev_power_governor *gov; + struct genpd_governor_data *gd; /* Data used by a genpd governor. */ struct work_struct power_off_work; struct fwnode_handle *provider; /* Identity of the domain provider */ bool has_provider; @@ -134,11 +143,6 @@ struct generic_pm_domain { int (*set_performance_state)(struct generic_pm_domain *genpd, unsigned int state); struct gpd_dev_ops dev_ops; - s64 max_off_time_ns; /* Maximum allowed "suspended" time. */ - ktime_t next_wakeup; /* Maintained by the domain governor */ - bool max_off_time_changed; - bool cached_power_down_ok; - bool cached_power_down_state_idx; int (*attach_dev)(struct generic_pm_domain *domain, struct device *dev); void (*detach_dev)(struct generic_pm_domain *domain, -- cgit v1.2.3 From 6961795f0d0a9c424915b3d73198c18b3dbd5e85 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:57:03 +0200 Subject: PM: domains: Measure power-on/off latencies in genpd based on a governor The measurements of the power-on|off latencies in genpd for a PM domain are superfluous, unless the corresponding genpd has a governor assigned to it, which would make use of the data. Therefore, let's improve the behaviour in genpd by making the measurements conditional, based upon if there's a governor assigned. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index a7de5fb8f073..83b073cdd0ed 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -509,6 +509,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) if (!genpd->power_on) goto out; + timed = timed && genpd->gd; if (!timed) { ret = genpd->power_on(genpd); if (ret) @@ -527,8 +528,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) goto out; genpd->states[state_idx].power_on_latency_ns = elapsed_ns; - if (genpd->gd) - genpd->gd->max_off_time_changed = true; + genpd->gd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "on", elapsed_ns); @@ -559,6 +559,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) if (!genpd->power_off) goto out; + timed = timed && genpd->gd; if (!timed) { ret = genpd->power_off(genpd); if (ret) @@ -577,8 +578,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) goto out; genpd->states[state_idx].power_off_latency_ns = elapsed_ns; - if (genpd->gd) - genpd->gd->max_off_time_changed = true; + genpd->gd->max_off_time_changed = true; pr_debug("%s: Power-%s latency exceeded, new value %lld ns\n", genpd->name, "off", elapsed_ns); -- cgit v1.2.3 From b2a92f354c5a9436f1f4ac8f6485a3ce1c941b29 Mon Sep 17 00:00:00 2001 From: Ulf Hansson Date: Wed, 11 May 2022 16:57:04 +0200 Subject: PM: domains: Trust domain-idle-states from DT to be correct by genpd If genpd has parsed the domain-idle-states from DT, it's reasonable to believe that the parsed data should be correct for the HW in question. Based upon this, it seem superfluous to let genpd measure the corresponding power-on/off latencies for these states. Therefore, let's improve the behaviour in genpd by avoiding the measurements for the domain-idle-states that have been parsed from DT. Signed-off-by: Ulf Hansson Signed-off-by: Rafael J. Wysocki --- drivers/base/power/domain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 83b073cdd0ed..739e52cd4aba 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -509,7 +509,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed) if (!genpd->power_on) goto out; - timed = timed && genpd->gd; + timed = timed && genpd->gd && !genpd->states[state_idx].fwnode; if (!timed) { ret = genpd->power_on(genpd); if (ret) @@ -559,7 +559,7 @@ static int _genpd_power_off(struct generic_pm_domain *genpd, bool timed) if (!genpd->power_off) goto out; - timed = timed && genpd->gd; + timed = timed && genpd->gd && !genpd->states[state_idx].fwnode; if (!timed) { ret = genpd->power_off(genpd); if (ret) -- cgit v1.2.3