* [PATCH V2 00/10] cpufreq: Migrate users of policy notifiers to QoS requests
@ 2019-07-23 6:14 ` Viresh Kumar
0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:26 UTC (permalink / raw)
To: Rafael Wysocki, Amit Daniel Kachhap, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Daniel Lezcano, Eduardo Valentin,
Erik Schmauss, Greg Kroah-Hartman, Javi Merino, Jonathan Corbet,
Len Brown, Rafael J. Wysocki, Robert Moore, Viresh Kumar,
Zhang Rui
Cc: linux-pm, Vincent Guittot, devel, dri-devel, linux-acpi,
linux-doc, linux-fbdev, linux-kernel, linuxppc-dev
Hello,
Now that cpufreq core supports taking QoS requests for min/max cpu
frequencies, lets migrate rest of the users to using them instead of the
policy notifiers.
The CPUFREQ_NOTIFY and CPUFREQ_ADJUST events of the policy notifiers are
removed as a result, but we have to add CPUFREQ_CREATE_POLICY and
CPUFREQ_REMOVE_POLICY events to it for the acpi stuff specifically,
though they are also used by arch_topology stuff now. So the policy
notifiers aren't completely removed.
Boot tested on my x86 PC and ARM hikey board.
This has already gone through build bot for a few days now.
V1->V2:
- Added Acked-by tags
- Reordered to keep cleanups at the bottom
- Rebased over 5.3-rc1
--
viresh
Viresh Kumar (10):
cpufreq: Add policy create/remove notifiers
thermal: cpu_cooling: Switch to QoS requests instead of cpufreq
notifier
powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier
cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq
notifier
ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY
video: sa1100fb: Remove cpufreq policy notifier
video: pxafb: Remove cpufreq policy notifier
cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier
events
Documentation: cpufreq: Update policy notifier documentation
Documentation/cpu-freq/core.txt | 16 +--
drivers/acpi/processor_driver.c | 44 ++++++++-
drivers/acpi/processor_perflib.c | 106 +++++++++-----------
drivers/acpi/processor_thermal.c | 81 ++++++++-------
drivers/base/arch_topology.c | 2 +-
drivers/cpufreq/cpufreq.c | 51 ++++------
drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++-
drivers/cpufreq/ppc_cbe_cpufreq.h | 8 ++
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++-------
drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++++++++++-----
drivers/thermal/cpu_cooling.c | 110 +++++----------------
drivers/video/fbdev/pxafb.c | 21 ----
drivers/video/fbdev/pxafb.h | 1 -
drivers/video/fbdev/sa1100fb.c | 27 -----
drivers/video/fbdev/sa1100fb.h | 1 -
include/acpi/processor.h | 22 +++--
include/linux/cpufreq.h | 4 +-
17 files changed, 327 insertions(+), 359 deletions(-)
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH V2 00/10] cpufreq: Migrate users of policy notifiers to QoS requests
@ 2019-07-23 6:14 ` Viresh Kumar
0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Amit Daniel Kachhap, Bartlomiej Zolnierkiewicz,
Benjamin Herrenschmidt, Daniel Lezcano, Eduardo Valentin,
Erik Schmauss, Greg Kroah-Hartman, Javi Merino, Jonathan Corbet,
Len Brown, Rafael J. Wysocki, Robert Moore, Viresh Kumar,
Zhang Rui
Cc: linux-fbdev, Vincent Guittot, linux-doc, linux-pm, linux-kernel,
dri-devel, linux-acpi, linuxppc-dev, devel
Hello,
Now that cpufreq core supports taking QoS requests for min/max cpu
frequencies, lets migrate rest of the users to using them instead of the
policy notifiers.
The CPUFREQ_NOTIFY and CPUFREQ_ADJUST events of the policy notifiers are
removed as a result, but we have to add CPUFREQ_CREATE_POLICY and
CPUFREQ_REMOVE_POLICY events to it for the acpi stuff specifically,
though they are also used by arch_topology stuff now. So the policy
notifiers aren't completely removed.
Boot tested on my x86 PC and ARM hikey board.
This has already gone through build bot for a few days now.
V1->V2:
- Added Acked-by tags
- Reordered to keep cleanups at the bottom
- Rebased over 5.3-rc1
--
viresh
Viresh Kumar (10):
cpufreq: Add policy create/remove notifiers
thermal: cpu_cooling: Switch to QoS requests instead of cpufreq
notifier
powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier
cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq
notifier
ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY
video: sa1100fb: Remove cpufreq policy notifier
video: pxafb: Remove cpufreq policy notifier
cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier
events
Documentation: cpufreq: Update policy notifier documentation
Documentation/cpu-freq/core.txt | 16 +--
drivers/acpi/processor_driver.c | 44 ++++++++-
drivers/acpi/processor_perflib.c | 106 +++++++++-----------
drivers/acpi/processor_thermal.c | 81 ++++++++-------
drivers/base/arch_topology.c | 2 +-
drivers/cpufreq/cpufreq.c | 51 ++++------
drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++-
drivers/cpufreq/ppc_cbe_cpufreq.h | 8 ++
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++-------
drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++++++++++-----
drivers/thermal/cpu_cooling.c | 110 +++++----------------
drivers/video/fbdev/pxafb.c | 21 ----
drivers/video/fbdev/pxafb.h | 1 -
drivers/video/fbdev/sa1100fb.c | 27 -----
drivers/video/fbdev/sa1100fb.h | 1 -
include/acpi/processor.h | 22 +++--
include/linux/cpufreq.h | 4 +-
17 files changed, 327 insertions(+), 359 deletions(-)
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply [flat|nested] 33+ messages in thread* [PATCH V2 01/10] cpufreq: Add policy create/remove notifiers
2019-07-23 6:14 ` Viresh Kumar
(?)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar; +Cc: linux-pm, Vincent Guittot, linux-kernel
This reverts commit f9f41e3ef99ac9d4e91b07634362e393fb929aad.
We have a new use case for policy create/remove notifiers (for
allocating/freeing QoS requests per policy), lets add them back.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 15 ++++++++++++++-
include/linux/cpufreq.h | 2 ++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 8dda62367816..c13dcb59b30c 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -1266,7 +1266,17 @@ static void cpufreq_policy_free(struct cpufreq_policy *policy)
DEV_PM_QOS_MAX_FREQUENCY);
dev_pm_qos_remove_notifier(dev, &policy->nb_min,
DEV_PM_QOS_MIN_FREQUENCY);
- dev_pm_qos_remove_request(policy->max_freq_req);
+
+ if (policy->max_freq_req) {
+ /*
+ * CPUFREQ_CREATE_POLICY notification is sent only after
+ * successfully adding max_freq_req request.
+ */
+ blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
+ CPUFREQ_REMOVE_POLICY, policy);
+ dev_pm_qos_remove_request(policy->max_freq_req);
+ }
+
dev_pm_qos_remove_request(policy->min_freq_req);
kfree(policy->min_freq_req);
@@ -1391,6 +1401,9 @@ static int cpufreq_online(unsigned int cpu)
ret);
goto out_destroy_policy;
}
+
+ blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
+ CPUFREQ_CREATE_POLICY, policy);
}
if (cpufreq_driver->get && has_target()) {
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index 536a049d7ecc..afc10384a681 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -458,6 +458,8 @@ static inline void cpufreq_resume(void) {}
/* Policy Notifiers */
#define CPUFREQ_ADJUST (0)
#define CPUFREQ_NOTIFY (1)
+#define CPUFREQ_CREATE_POLICY (2)
+#define CPUFREQ_REMOVE_POLICY (3)
#ifdef CONFIG_CPU_FREQ
int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V2 02/10] thermal: cpu_cooling: Switch to QoS requests instead of cpufreq notifier
2019-07-23 6:14 ` Viresh Kumar
` (2 preceding siblings ...)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Amit Daniel Kachhap, Viresh Kumar, Javi Merino,
Zhang Rui, Eduardo Valentin, Daniel Lezcano
Cc: linux-pm, Vincent Guittot, linux-kernel
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for cpu_cooling driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/thermal/cpu_cooling.c | 110 ++++++++--------------------------
1 file changed, 26 insertions(+), 84 deletions(-)
diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index 4c5db59a619b..391f39776c6a 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -16,6 +16,7 @@
#include <linux/err.h>
#include <linux/idr.h>
#include <linux/pm_opp.h>
+#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/cpu.h>
#include <linux/cpu_cooling.h>
@@ -66,8 +67,6 @@ struct time_in_idle {
* @last_load: load measured by the latest call to cpufreq_get_requested_power()
* @cpufreq_state: integer value representing the current state of cpufreq
* cooling devices.
- * @clipped_freq: integer value representing the absolute value of the clipped
- * frequency.
* @max_level: maximum cooling level. One less than total number of valid
* cpufreq frequencies.
* @freq_table: Freq table in descending order of frequencies
@@ -84,12 +83,12 @@ struct cpufreq_cooling_device {
int id;
u32 last_load;
unsigned int cpufreq_state;
- unsigned int clipped_freq;
unsigned int max_level;
struct freq_table *freq_table; /* In descending order */
struct cpufreq_policy *policy;
struct list_head node;
struct time_in_idle *idle_time;
+ struct dev_pm_qos_request qos_req;
};
static DEFINE_IDA(cpufreq_ida);
@@ -118,59 +117,6 @@ static unsigned long get_level(struct cpufreq_cooling_device *cpufreq_cdev,
return level - 1;
}
-/**
- * cpufreq_thermal_notifier - notifier callback for cpufreq policy change.
- * @nb: struct notifier_block * with callback info.
- * @event: value showing cpufreq event for which this function invoked.
- * @data: callback-specific data
- *
- * Callback to hijack the notification on cpufreq policy transition.
- * Every time there is a change in policy, we will intercept and
- * update the cpufreq policy with thermal constraints.
- *
- * Return: 0 (success)
- */
-static int cpufreq_thermal_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- unsigned long clipped_freq;
- struct cpufreq_cooling_device *cpufreq_cdev;
-
- if (event != CPUFREQ_ADJUST)
- return NOTIFY_DONE;
-
- mutex_lock(&cooling_list_lock);
- list_for_each_entry(cpufreq_cdev, &cpufreq_cdev_list, node) {
- /*
- * A new copy of the policy is sent to the notifier and can't
- * compare that directly.
- */
- if (policy->cpu != cpufreq_cdev->policy->cpu)
- continue;
-
- /*
- * policy->max is the maximum allowed frequency defined by user
- * and clipped_freq is the maximum that thermal constraints
- * allow.
- *
- * If clipped_freq is lower than policy->max, then we need to
- * readjust policy->max.
- *
- * But, if clipped_freq is greater than policy->max, we don't
- * need to do anything.
- */
- clipped_freq = cpufreq_cdev->clipped_freq;
-
- if (policy->max > clipped_freq)
- cpufreq_verify_within_limits(policy, 0, clipped_freq);
- break;
- }
- mutex_unlock(&cooling_list_lock);
-
- return NOTIFY_OK;
-}
-
/**
* update_freq_table() - Update the freq table with power numbers
* @cpufreq_cdev: the cpufreq cooling device in which to update the table
@@ -374,7 +320,6 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
unsigned long state)
{
struct cpufreq_cooling_device *cpufreq_cdev = cdev->devdata;
- unsigned int clip_freq;
/* Request state should be less than max_level */
if (WARN_ON(state > cpufreq_cdev->max_level))
@@ -384,13 +329,10 @@ static int cpufreq_set_cur_state(struct thermal_cooling_device *cdev,
if (cpufreq_cdev->cpufreq_state == state)
return 0;
- clip_freq = cpufreq_cdev->freq_table[state].frequency;
cpufreq_cdev->cpufreq_state = state;
- cpufreq_cdev->clipped_freq = clip_freq;
-
- cpufreq_update_policy(cpufreq_cdev->policy->cpu);
- return 0;
+ return dev_pm_qos_update_request(&cpufreq_cdev->qos_req,
+ cpufreq_cdev->freq_table[state].frequency);
}
/**
@@ -554,11 +496,6 @@ static struct thermal_cooling_device_ops cpufreq_power_cooling_ops = {
.power2state = cpufreq_power2state,
};
-/* Notifier for cpufreq policy change */
-static struct notifier_block thermal_cpufreq_notifier_block = {
- .notifier_call = cpufreq_thermal_notifier,
-};
-
static unsigned int find_next_max(struct cpufreq_frequency_table *table,
unsigned int prev_max)
{
@@ -596,9 +533,16 @@ __cpufreq_cooling_register(struct device_node *np,
struct cpufreq_cooling_device *cpufreq_cdev;
char dev_name[THERMAL_NAME_LENGTH];
unsigned int freq, i, num_cpus;
+ struct device *dev;
int ret;
struct thermal_cooling_device_ops *cooling_ops;
- bool first;
+
+ dev = get_cpu_device(policy->cpu);
+ if (unlikely(!dev)) {
+ pr_warn("No cpu device for cpu %d\n", policy->cpu);
+ return ERR_PTR(-ENODEV);
+ }
+
if (IS_ERR_OR_NULL(policy)) {
pr_err("%s: cpufreq policy isn't valid: %p\n", __func__, policy);
@@ -671,25 +615,29 @@ __cpufreq_cooling_register(struct device_node *np,
cooling_ops = &cpufreq_cooling_ops;
}
+ ret = dev_pm_qos_add_request(dev, &cpufreq_cdev->qos_req,
+ DEV_PM_QOS_MAX_FREQUENCY,
+ cpufreq_cdev->freq_table[0].frequency);
+ if (ret < 0) {
+ pr_err("%s: Failed to add freq constraint (%d)\n", __func__,
+ ret);
+ cdev = ERR_PTR(ret);
+ goto remove_ida;
+ }
+
cdev = thermal_of_cooling_device_register(np, dev_name, cpufreq_cdev,
cooling_ops);
if (IS_ERR(cdev))
- goto remove_ida;
-
- cpufreq_cdev->clipped_freq = cpufreq_cdev->freq_table[0].frequency;
+ goto remove_qos_req;
mutex_lock(&cooling_list_lock);
- /* Register the notifier for first cpufreq cooling device */
- first = list_empty(&cpufreq_cdev_list);
list_add(&cpufreq_cdev->node, &cpufreq_cdev_list);
mutex_unlock(&cooling_list_lock);
- if (first)
- cpufreq_register_notifier(&thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
-
return cdev;
+remove_qos_req:
+ dev_pm_qos_remove_request(&cpufreq_cdev->qos_req);
remove_ida:
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
free_table:
@@ -777,7 +725,6 @@ EXPORT_SYMBOL_GPL(of_cpufreq_cooling_register);
void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
{
struct cpufreq_cooling_device *cpufreq_cdev;
- bool last;
if (!cdev)
return;
@@ -786,15 +733,10 @@ void cpufreq_cooling_unregister(struct thermal_cooling_device *cdev)
mutex_lock(&cooling_list_lock);
list_del(&cpufreq_cdev->node);
- /* Unregister the notifier for the last cpufreq cooling device */
- last = list_empty(&cpufreq_cdev_list);
mutex_unlock(&cooling_list_lock);
- if (last)
- cpufreq_unregister_notifier(&thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
-
thermal_cooling_device_unregister(cdev);
+ dev_pm_qos_remove_request(&cpufreq_cdev->qos_req);
ida_simple_remove(&cpufreq_ida, cpufreq_cdev->id);
kfree(cpufreq_cdev->idle_time);
kfree(cpufreq_cdev->freq_table);
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V2 03/10] powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier
2019-07-23 6:14 ` Viresh Kumar
@ 2019-07-23 6:14 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Benjamin Herrenschmidt
Cc: Viresh Kumar, linux-pm, Vincent Guittot, linuxppc-dev, linux-kernel
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for windfarm_cpufreq_clamp driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++++++++++++++--------
1 file changed, 50 insertions(+), 27 deletions(-)
diff --git a/drivers/macintosh/windfarm_cpufreq_clamp.c b/drivers/macintosh/windfarm_cpufreq_clamp.c
index 52fd5fca89a0..705c6200814b 100644
--- a/drivers/macintosh/windfarm_cpufreq_clamp.c
+++ b/drivers/macintosh/windfarm_cpufreq_clamp.c
@@ -3,9 +3,11 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/delay.h>
+#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/wait.h>
+#include <linux/cpu.h>
#include <linux/cpufreq.h>
#include <asm/prom.h>
@@ -16,36 +18,24 @@
static int clamped;
static struct wf_control *clamp_control;
-
-static int clamp_notifier_call(struct notifier_block *self,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *p = data;
- unsigned long max_freq;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- max_freq = clamped ? (p->cpuinfo.min_freq) : (p->cpuinfo.max_freq);
- cpufreq_verify_within_limits(p, 0, max_freq);
-
- return 0;
-}
-
-static struct notifier_block clamp_notifier = {
- .notifier_call = clamp_notifier_call,
-};
+static struct dev_pm_qos_request qos_req;
+static unsigned int min_freq, max_freq;
static int clamp_set(struct wf_control *ct, s32 value)
{
- if (value)
+ unsigned int freq;
+
+ if (value) {
+ freq = min_freq;
printk(KERN_INFO "windfarm: Clamping CPU frequency to "
"minimum !\n");
- else
+ } else {
+ freq = max_freq;
printk(KERN_INFO "windfarm: CPU frequency unclamped !\n");
+ }
clamped = value;
- cpufreq_update_policy(0);
- return 0;
+
+ return dev_pm_qos_update_request(&qos_req, freq);
}
static int clamp_get(struct wf_control *ct, s32 *value)
@@ -74,27 +64,60 @@ static const struct wf_control_ops clamp_ops = {
static int __init wf_cpufreq_clamp_init(void)
{
+ struct cpufreq_policy *policy;
struct wf_control *clamp;
+ struct device *dev;
+ int ret;
+
+ policy = cpufreq_cpu_get(0);
+ if (!policy) {
+ pr_warn("%s: cpufreq policy not found cpu0\n", __func__);
+ return -EPROBE_DEFER;
+ }
+
+ min_freq = policy->cpuinfo.min_freq;
+ max_freq = policy->cpuinfo.max_freq;
+ cpufreq_cpu_put(policy);
+
+ dev = get_cpu_device(0);
+ if (unlikely(!dev)) {
+ pr_warn("%s: No cpu device for cpu0\n", __func__);
+ return -ENODEV;
+ }
clamp = kmalloc(sizeof(struct wf_control), GFP_KERNEL);
if (clamp == NULL)
return -ENOMEM;
- cpufreq_register_notifier(&clamp_notifier, CPUFREQ_POLICY_NOTIFIER);
+
+ ret = dev_pm_qos_add_request(dev, &qos_req, DEV_PM_QOS_MAX_FREQUENCY,
+ max_freq);
+ if (ret < 0) {
+ pr_err("%s: Failed to add freq constraint (%d)\n", __func__,
+ ret);
+ goto free;
+ }
+
clamp->ops = &clamp_ops;
clamp->name = "cpufreq-clamp";
- if (wf_register_control(clamp))
+ ret = wf_register_control(clamp);
+ if (ret)
goto fail;
clamp_control = clamp;
return 0;
fail:
+ dev_pm_qos_remove_request(&qos_req);
+
+ free:
kfree(clamp);
- return -ENODEV;
+ return ret;
}
static void __exit wf_cpufreq_clamp_exit(void)
{
- if (clamp_control)
+ if (clamp_control) {
wf_unregister_control(clamp_control);
+ dev_pm_qos_remove_request(&qos_req);
+ }
}
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V2 03/10] powerpc: macintosh: Switch to QoS requests instead of cpufreq notifier
@ 2019-07-23 6:14 ` Viresh Kumar
0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Benjamin Herrenschmidt
Cc: Viresh Kumar, Vincent Guittot, linuxppc-dev, linux-kernel, linux-pm
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for windfarm_cpufreq_clamp driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/macintosh/windfarm_cpufreq_clamp.c | 77 ++++++++++++++--------
1 file changed, 50 insertions(+), 27 deletions(-)
diff --git a/drivers/macintosh/windfarm_cpufreq_clamp.c b/drivers/macintosh/windfarm_cpufreq_clamp.c
index 52fd5fca89a0..705c6200814b 100644
--- a/drivers/macintosh/windfarm_cpufreq_clamp.c
+++ b/drivers/macintosh/windfarm_cpufreq_clamp.c
@@ -3,9 +3,11 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/delay.h>
+#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/init.h>
#include <linux/wait.h>
+#include <linux/cpu.h>
#include <linux/cpufreq.h>
#include <asm/prom.h>
@@ -16,36 +18,24 @@
static int clamped;
static struct wf_control *clamp_control;
-
-static int clamp_notifier_call(struct notifier_block *self,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *p = data;
- unsigned long max_freq;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- max_freq = clamped ? (p->cpuinfo.min_freq) : (p->cpuinfo.max_freq);
- cpufreq_verify_within_limits(p, 0, max_freq);
-
- return 0;
-}
-
-static struct notifier_block clamp_notifier = {
- .notifier_call = clamp_notifier_call,
-};
+static struct dev_pm_qos_request qos_req;
+static unsigned int min_freq, max_freq;
static int clamp_set(struct wf_control *ct, s32 value)
{
- if (value)
+ unsigned int freq;
+
+ if (value) {
+ freq = min_freq;
printk(KERN_INFO "windfarm: Clamping CPU frequency to "
"minimum !\n");
- else
+ } else {
+ freq = max_freq;
printk(KERN_INFO "windfarm: CPU frequency unclamped !\n");
+ }
clamped = value;
- cpufreq_update_policy(0);
- return 0;
+
+ return dev_pm_qos_update_request(&qos_req, freq);
}
static int clamp_get(struct wf_control *ct, s32 *value)
@@ -74,27 +64,60 @@ static const struct wf_control_ops clamp_ops = {
static int __init wf_cpufreq_clamp_init(void)
{
+ struct cpufreq_policy *policy;
struct wf_control *clamp;
+ struct device *dev;
+ int ret;
+
+ policy = cpufreq_cpu_get(0);
+ if (!policy) {
+ pr_warn("%s: cpufreq policy not found cpu0\n", __func__);
+ return -EPROBE_DEFER;
+ }
+
+ min_freq = policy->cpuinfo.min_freq;
+ max_freq = policy->cpuinfo.max_freq;
+ cpufreq_cpu_put(policy);
+
+ dev = get_cpu_device(0);
+ if (unlikely(!dev)) {
+ pr_warn("%s: No cpu device for cpu0\n", __func__);
+ return -ENODEV;
+ }
clamp = kmalloc(sizeof(struct wf_control), GFP_KERNEL);
if (clamp == NULL)
return -ENOMEM;
- cpufreq_register_notifier(&clamp_notifier, CPUFREQ_POLICY_NOTIFIER);
+
+ ret = dev_pm_qos_add_request(dev, &qos_req, DEV_PM_QOS_MAX_FREQUENCY,
+ max_freq);
+ if (ret < 0) {
+ pr_err("%s: Failed to add freq constraint (%d)\n", __func__,
+ ret);
+ goto free;
+ }
+
clamp->ops = &clamp_ops;
clamp->name = "cpufreq-clamp";
- if (wf_register_control(clamp))
+ ret = wf_register_control(clamp);
+ if (ret)
goto fail;
clamp_control = clamp;
return 0;
fail:
+ dev_pm_qos_remove_request(&qos_req);
+
+ free:
kfree(clamp);
- return -ENODEV;
+ return ret;
}
static void __exit wf_cpufreq_clamp_exit(void)
{
- if (clamp_control)
+ if (clamp_control) {
wf_unregister_control(clamp_control);
+ dev_pm_qos_remove_request(&qos_req);
+ }
}
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-07-23 6:14 ` Viresh Kumar
` (4 preceding siblings ...)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
2019-08-09 2:34 ` Viresh Kumar
-1 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar; +Cc: linux-pm, Vincent Guittot, linux-kernel
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for ppc_cbe_cpufreq driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
3 files changed, 86 insertions(+), 37 deletions(-)
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c
index b83f36febf03..c58abb4cca3a 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
@@ -110,6 +110,13 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
#endif
policy->freq_table = cbe_freqs;
+ cbe_cpufreq_pmi_policy_init(policy);
+ return 0;
+}
+
+static int cbe_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+ cbe_cpufreq_pmi_policy_exit(policy);
return 0;
}
@@ -129,6 +136,7 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
.verify = cpufreq_generic_frequency_table_verify,
.target_index = cbe_cpufreq_target,
.init = cbe_cpufreq_cpu_init,
+ .exit = cbe_cpufreq_cpu_exit,
.name = "cbe-cpufreq",
.flags = CPUFREQ_CONST_LOOPS,
};
@@ -139,15 +147,24 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
static int __init cbe_cpufreq_init(void)
{
+ int ret;
+
if (!machine_is(cell))
return -ENODEV;
- return cpufreq_register_driver(&cbe_cpufreq_driver);
+ cbe_cpufreq_pmi_init();
+
+ ret = cpufreq_register_driver(&cbe_cpufreq_driver);
+ if (ret)
+ cbe_cpufreq_pmi_exit();
+
+ return ret;
}
static void __exit cbe_cpufreq_exit(void)
{
cpufreq_unregister_driver(&cbe_cpufreq_driver);
+ cbe_cpufreq_pmi_exit();
}
module_init(cbe_cpufreq_init);
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.h b/drivers/cpufreq/ppc_cbe_cpufreq.h
index 9d973519d669..00cd8633b0d9 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq.h
+++ b/drivers/cpufreq/ppc_cbe_cpufreq.h
@@ -20,6 +20,14 @@ int cbe_cpufreq_set_pmode_pmi(int cpu, unsigned int pmode);
#if IS_ENABLED(CONFIG_CPU_FREQ_CBE_PMI)
extern bool cbe_cpufreq_has_pmi;
+void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy);
+void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy);
+void cbe_cpufreq_pmi_init(void);
+void cbe_cpufreq_pmi_exit(void);
#else
#define cbe_cpufreq_has_pmi (0)
+static inline void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy) {}
+static inline void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy) {}
+static inline void cbe_cpufreq_pmi_init(void) {}
+static inline void cbe_cpufreq_pmi_exit(void) {}
#endif
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
index 97c8ee4614b7..0babb27c1c72 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
@@ -12,6 +12,7 @@
#include <linux/timer.h>
#include <linux/init.h>
#include <linux/of_platform.h>
+#include <linux/pm_qos.h>
#include <asm/processor.h>
#include <asm/prom.h>
@@ -24,8 +25,6 @@
#include "ppc_cbe_cpufreq.h"
-static u8 pmi_slow_mode_limit[MAX_CBE];
-
bool cbe_cpufreq_has_pmi = false;
EXPORT_SYMBOL_GPL(cbe_cpufreq_has_pmi);
@@ -65,64 +64,89 @@ EXPORT_SYMBOL_GPL(cbe_cpufreq_set_pmode_pmi);
static void cbe_cpufreq_handle_pmi(pmi_message_t pmi_msg)
{
+ struct cpufreq_policy *policy;
+ struct dev_pm_qos_request *req;
u8 node, slow_mode;
+ int cpu, ret;
BUG_ON(pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
node = pmi_msg.data1;
slow_mode = pmi_msg.data2;
- pmi_slow_mode_limit[node] = slow_mode;
+ cpu = cbe_node_to_cpu(node);
pr_debug("cbe_handle_pmi: node: %d max_freq: %d\n", node, slow_mode);
-}
-
-static int pmi_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct cpufreq_frequency_table *cbe_freqs = policy->freq_table;
- u8 node;
-
- /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
- * policy events?)
- */
- node = cbe_cpu_to_node(policy->cpu);
-
- pr_debug("got notified, event=%lu, node=%u\n", event, node);
- if (pmi_slow_mode_limit[node] != 0) {
- pr_debug("limiting node %d to slow mode %d\n",
- node, pmi_slow_mode_limit[node]);
+ policy = cpufreq_cpu_get(cpu);
+ if (!policy) {
+ pr_warn("cpufreq policy not found cpu%d\n", cpu);
+ return;
+ }
- cpufreq_verify_within_limits(policy, 0,
+ req = policy->driver_data;
- cbe_freqs[pmi_slow_mode_limit[node]].frequency);
- }
+ ret = dev_pm_qos_update_request(req,
+ policy->freq_table[slow_mode].frequency);
+ if (ret)
+ pr_warn("Failed to update freq constraint: %d\n", ret);
+ else
+ pr_debug("limiting node %d to slow mode %d\n", node, slow_mode);
- return 0;
+ cpufreq_cpu_put(policy);
}
-static struct notifier_block pmi_notifier_block = {
- .notifier_call = pmi_notifier,
-};
-
static struct pmi_handler cbe_pmi_handler = {
.type = PMI_TYPE_FREQ_CHANGE,
.handle_pmi_message = cbe_cpufreq_handle_pmi,
};
+void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy)
+{
+ struct dev_pm_qos_request *req;
+ int ret;
+
+ if (!cbe_cpufreq_has_pmi)
+ return;
+
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu), req,
+ DEV_PM_QOS_MAX_FREQUENCY,
+ policy->freq_table[0].frequency);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint (%d)\n", ret);
+ kfree(req);
+ return;
+ }
+ policy->driver_data = req;
+}
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_init);
-static int __init cbe_cpufreq_pmi_init(void)
+void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy)
{
- cbe_cpufreq_has_pmi = pmi_register_handler(&cbe_pmi_handler) == 0;
+ struct dev_pm_qos_request *req = policy->driver_data;
- if (!cbe_cpufreq_has_pmi)
- return -ENODEV;
+ if (cbe_cpufreq_has_pmi) {
+ dev_pm_qos_remove_request(req);
+ kfree(req);
+ }
+}
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_exit);
- cpufreq_register_notifier(&pmi_notifier_block, CPUFREQ_POLICY_NOTIFIER);
+void cbe_cpufreq_pmi_init(void)
+{
+ if (!pmi_register_handler(&cbe_pmi_handler))
+ cbe_cpufreq_has_pmi = true;
+}
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_init);
- return 0;
+void cbe_cpufreq_pmi_exit(void)
+{
+ pmi_unregister_handler(&cbe_pmi_handler);
+ cbe_cpufreq_has_pmi = false;
}
-device_initcall(cbe_cpufreq_pmi_init);
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_exit);
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-07-23 6:14 ` [PATCH V2 04/10] cpufreq: powerpc_cbe: " Viresh Kumar
@ 2019-08-09 2:34 ` Viresh Kumar
2019-08-09 9:01 ` Rafael J. Wysocki
2019-08-10 12:19 ` Rafael J. Wysocki
0 siblings, 2 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-08-09 2:34 UTC (permalink / raw)
To: Rafael Wysocki; +Cc: linux-pm, Vincent Guittot, linux-kernel
On 23-07-19, 11:44, Viresh Kumar wrote:
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for ppc_cbe_cpufreq driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> 3 files changed, 86 insertions(+), 37 deletions(-)
-------------------------8<-------------------------
From b84e1c119d63ab842c9e4f3acbc3aec22efa866d Mon Sep 17 00:00:00 2001
Message-Id: <b84e1c119d63ab842c9e4f3acbc3aec22efa866d.1565318034.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Fri, 5 Jul 2019 15:49:48 +0530
Subject: [PATCH] cpufreq: powerpc_cbe: Switch to QoS requests instead of
cpufreq notifier
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for ppc_cbe_cpufreq driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
- dev_pm_qos_update_request() can return 1 on success
drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
3 files changed, 86 insertions(+), 37 deletions(-)
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c
index b83f36febf03..c58abb4cca3a 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
@@ -110,6 +110,13 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
#endif
policy->freq_table = cbe_freqs;
+ cbe_cpufreq_pmi_policy_init(policy);
+ return 0;
+}
+
+static int cbe_cpufreq_cpu_exit(struct cpufreq_policy *policy)
+{
+ cbe_cpufreq_pmi_policy_exit(policy);
return 0;
}
@@ -129,6 +136,7 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
.verify = cpufreq_generic_frequency_table_verify,
.target_index = cbe_cpufreq_target,
.init = cbe_cpufreq_cpu_init,
+ .exit = cbe_cpufreq_cpu_exit,
.name = "cbe-cpufreq",
.flags = CPUFREQ_CONST_LOOPS,
};
@@ -139,15 +147,24 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
static int __init cbe_cpufreq_init(void)
{
+ int ret;
+
if (!machine_is(cell))
return -ENODEV;
- return cpufreq_register_driver(&cbe_cpufreq_driver);
+ cbe_cpufreq_pmi_init();
+
+ ret = cpufreq_register_driver(&cbe_cpufreq_driver);
+ if (ret)
+ cbe_cpufreq_pmi_exit();
+
+ return ret;
}
static void __exit cbe_cpufreq_exit(void)
{
cpufreq_unregister_driver(&cbe_cpufreq_driver);
+ cbe_cpufreq_pmi_exit();
}
module_init(cbe_cpufreq_init);
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.h b/drivers/cpufreq/ppc_cbe_cpufreq.h
index 9d973519d669..00cd8633b0d9 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq.h
+++ b/drivers/cpufreq/ppc_cbe_cpufreq.h
@@ -20,6 +20,14 @@ int cbe_cpufreq_set_pmode_pmi(int cpu, unsigned int pmode);
#if IS_ENABLED(CONFIG_CPU_FREQ_CBE_PMI)
extern bool cbe_cpufreq_has_pmi;
+void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy);
+void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy);
+void cbe_cpufreq_pmi_init(void);
+void cbe_cpufreq_pmi_exit(void);
#else
#define cbe_cpufreq_has_pmi (0)
+static inline void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy) {}
+static inline void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy) {}
+static inline void cbe_cpufreq_pmi_init(void) {}
+static inline void cbe_cpufreq_pmi_exit(void) {}
#endif
diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
index 97c8ee4614b7..bc9dd30395c4 100644
--- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
+++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
@@ -12,6 +12,7 @@
#include <linux/timer.h>
#include <linux/init.h>
#include <linux/of_platform.h>
+#include <linux/pm_qos.h>
#include <asm/processor.h>
#include <asm/prom.h>
@@ -24,8 +25,6 @@
#include "ppc_cbe_cpufreq.h"
-static u8 pmi_slow_mode_limit[MAX_CBE];
-
bool cbe_cpufreq_has_pmi = false;
EXPORT_SYMBOL_GPL(cbe_cpufreq_has_pmi);
@@ -65,64 +64,89 @@ EXPORT_SYMBOL_GPL(cbe_cpufreq_set_pmode_pmi);
static void cbe_cpufreq_handle_pmi(pmi_message_t pmi_msg)
{
+ struct cpufreq_policy *policy;
+ struct dev_pm_qos_request *req;
u8 node, slow_mode;
+ int cpu, ret;
BUG_ON(pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
node = pmi_msg.data1;
slow_mode = pmi_msg.data2;
- pmi_slow_mode_limit[node] = slow_mode;
+ cpu = cbe_node_to_cpu(node);
pr_debug("cbe_handle_pmi: node: %d max_freq: %d\n", node, slow_mode);
-}
-
-static int pmi_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct cpufreq_frequency_table *cbe_freqs = policy->freq_table;
- u8 node;
-
- /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
- * policy events?)
- */
- node = cbe_cpu_to_node(policy->cpu);
-
- pr_debug("got notified, event=%lu, node=%u\n", event, node);
- if (pmi_slow_mode_limit[node] != 0) {
- pr_debug("limiting node %d to slow mode %d\n",
- node, pmi_slow_mode_limit[node]);
+ policy = cpufreq_cpu_get(cpu);
+ if (!policy) {
+ pr_warn("cpufreq policy not found cpu%d\n", cpu);
+ return;
+ }
- cpufreq_verify_within_limits(policy, 0,
+ req = policy->driver_data;
- cbe_freqs[pmi_slow_mode_limit[node]].frequency);
- }
+ ret = dev_pm_qos_update_request(req,
+ policy->freq_table[slow_mode].frequency);
+ if (ret < 0)
+ pr_warn("Failed to update freq constraint: %d\n", ret);
+ else
+ pr_debug("limiting node %d to slow mode %d\n", node, slow_mode);
- return 0;
+ cpufreq_cpu_put(policy);
}
-static struct notifier_block pmi_notifier_block = {
- .notifier_call = pmi_notifier,
-};
-
static struct pmi_handler cbe_pmi_handler = {
.type = PMI_TYPE_FREQ_CHANGE,
.handle_pmi_message = cbe_cpufreq_handle_pmi,
};
+void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy)
+{
+ struct dev_pm_qos_request *req;
+ int ret;
+
+ if (!cbe_cpufreq_has_pmi)
+ return;
+
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu), req,
+ DEV_PM_QOS_MAX_FREQUENCY,
+ policy->freq_table[0].frequency);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint (%d)\n", ret);
+ kfree(req);
+ return;
+ }
+ policy->driver_data = req;
+}
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_init);
-static int __init cbe_cpufreq_pmi_init(void)
+void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy)
{
- cbe_cpufreq_has_pmi = pmi_register_handler(&cbe_pmi_handler) == 0;
+ struct dev_pm_qos_request *req = policy->driver_data;
- if (!cbe_cpufreq_has_pmi)
- return -ENODEV;
+ if (cbe_cpufreq_has_pmi) {
+ dev_pm_qos_remove_request(req);
+ kfree(req);
+ }
+}
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_exit);
- cpufreq_register_notifier(&pmi_notifier_block, CPUFREQ_POLICY_NOTIFIER);
+void cbe_cpufreq_pmi_init(void)
+{
+ if (!pmi_register_handler(&cbe_pmi_handler))
+ cbe_cpufreq_has_pmi = true;
+}
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_init);
- return 0;
+void cbe_cpufreq_pmi_exit(void)
+{
+ pmi_unregister_handler(&cbe_pmi_handler);
+ cbe_cpufreq_has_pmi = false;
}
-device_initcall(cbe_cpufreq_pmi_init);
+EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_exit);
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-08-09 2:34 ` Viresh Kumar
@ 2019-08-09 9:01 ` Rafael J. Wysocki
2019-08-19 2:26 ` Viresh Kumar
2019-08-10 12:19 ` Rafael J. Wysocki
1 sibling, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-09 9:01 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Linux PM, Vincent Guittot, Linux Kernel Mailing List
On Fri, Aug 9, 2019 at 4:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 23-07-19, 11:44, Viresh Kumar wrote:
> > The cpufreq core now takes the min/max frequency constraints via QoS
> > requests and the CPUFREQ_ADJUST notifier shall get removed later on.
> >
> > Switch over to using the QoS request for maximum frequency constraint
> > for ppc_cbe_cpufreq driver.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> > drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> > drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> > 3 files changed, 86 insertions(+), 37 deletions(-)
>
> -------------------------8<-------------------------
If you do it this way, Patchwork will not pick up the patch.
Please send afresh with "[Update]" or bumped up version number in the
subject (or both).
> From b84e1c119d63ab842c9e4f3acbc3aec22efa866d Mon Sep 17 00:00:00 2001
> Message-Id: <b84e1c119d63ab842c9e4f3acbc3aec22efa866d.1565318034.git.viresh.kumar@linaro.org>
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Date: Fri, 5 Jul 2019 15:49:48 +0530
> Subject: [PATCH] cpufreq: powerpc_cbe: Switch to QoS requests instead of
> cpufreq notifier
>
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for ppc_cbe_cpufreq driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
And here you can say that it replaces an earlier patch.
> ---
> - dev_pm_qos_update_request() can return 1 on success
> drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> 3 files changed, 86 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c
> index b83f36febf03..c58abb4cca3a 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
> @@ -110,6 +110,13 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
> #endif
>
> policy->freq_table = cbe_freqs;
> + cbe_cpufreq_pmi_policy_init(policy);
> + return 0;
> +}
> +
> +static int cbe_cpufreq_cpu_exit(struct cpufreq_policy *policy)
> +{
> + cbe_cpufreq_pmi_policy_exit(policy);
> return 0;
> }
>
> @@ -129,6 +136,7 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = cbe_cpufreq_target,
> .init = cbe_cpufreq_cpu_init,
> + .exit = cbe_cpufreq_cpu_exit,
> .name = "cbe-cpufreq",
> .flags = CPUFREQ_CONST_LOOPS,
> };
> @@ -139,15 +147,24 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
>
> static int __init cbe_cpufreq_init(void)
> {
> + int ret;
> +
> if (!machine_is(cell))
> return -ENODEV;
>
> - return cpufreq_register_driver(&cbe_cpufreq_driver);
> + cbe_cpufreq_pmi_init();
> +
> + ret = cpufreq_register_driver(&cbe_cpufreq_driver);
> + if (ret)
> + cbe_cpufreq_pmi_exit();
> +
> + return ret;
> }
>
> static void __exit cbe_cpufreq_exit(void)
> {
> cpufreq_unregister_driver(&cbe_cpufreq_driver);
> + cbe_cpufreq_pmi_exit();
> }
>
> module_init(cbe_cpufreq_init);
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.h b/drivers/cpufreq/ppc_cbe_cpufreq.h
> index 9d973519d669..00cd8633b0d9 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq.h
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq.h
> @@ -20,6 +20,14 @@ int cbe_cpufreq_set_pmode_pmi(int cpu, unsigned int pmode);
>
> #if IS_ENABLED(CONFIG_CPU_FREQ_CBE_PMI)
> extern bool cbe_cpufreq_has_pmi;
> +void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy);
> +void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy);
> +void cbe_cpufreq_pmi_init(void);
> +void cbe_cpufreq_pmi_exit(void);
> #else
> #define cbe_cpufreq_has_pmi (0)
> +static inline void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy) {}
> +static inline void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy) {}
> +static inline void cbe_cpufreq_pmi_init(void) {}
> +static inline void cbe_cpufreq_pmi_exit(void) {}
> #endif
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> index 97c8ee4614b7..bc9dd30395c4 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> @@ -12,6 +12,7 @@
> #include <linux/timer.h>
> #include <linux/init.h>
> #include <linux/of_platform.h>
> +#include <linux/pm_qos.h>
>
> #include <asm/processor.h>
> #include <asm/prom.h>
> @@ -24,8 +25,6 @@
>
> #include "ppc_cbe_cpufreq.h"
>
> -static u8 pmi_slow_mode_limit[MAX_CBE];
> -
> bool cbe_cpufreq_has_pmi = false;
> EXPORT_SYMBOL_GPL(cbe_cpufreq_has_pmi);
>
> @@ -65,64 +64,89 @@ EXPORT_SYMBOL_GPL(cbe_cpufreq_set_pmode_pmi);
>
> static void cbe_cpufreq_handle_pmi(pmi_message_t pmi_msg)
> {
> + struct cpufreq_policy *policy;
> + struct dev_pm_qos_request *req;
> u8 node, slow_mode;
> + int cpu, ret;
>
> BUG_ON(pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
>
> node = pmi_msg.data1;
> slow_mode = pmi_msg.data2;
>
> - pmi_slow_mode_limit[node] = slow_mode;
> + cpu = cbe_node_to_cpu(node);
>
> pr_debug("cbe_handle_pmi: node: %d max_freq: %d\n", node, slow_mode);
> -}
> -
> -static int pmi_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - struct cpufreq_frequency_table *cbe_freqs = policy->freq_table;
> - u8 node;
> -
> - /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
> - * policy events?)
> - */
> - node = cbe_cpu_to_node(policy->cpu);
> -
> - pr_debug("got notified, event=%lu, node=%u\n", event, node);
>
> - if (pmi_slow_mode_limit[node] != 0) {
> - pr_debug("limiting node %d to slow mode %d\n",
> - node, pmi_slow_mode_limit[node]);
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy) {
> + pr_warn("cpufreq policy not found cpu%d\n", cpu);
> + return;
> + }
>
> - cpufreq_verify_within_limits(policy, 0,
> + req = policy->driver_data;
>
> - cbe_freqs[pmi_slow_mode_limit[node]].frequency);
> - }
> + ret = dev_pm_qos_update_request(req,
> + policy->freq_table[slow_mode].frequency);
> + if (ret < 0)
> + pr_warn("Failed to update freq constraint: %d\n", ret);
> + else
> + pr_debug("limiting node %d to slow mode %d\n", node, slow_mode);
>
> - return 0;
> + cpufreq_cpu_put(policy);
> }
>
> -static struct notifier_block pmi_notifier_block = {
> - .notifier_call = pmi_notifier,
> -};
> -
> static struct pmi_handler cbe_pmi_handler = {
> .type = PMI_TYPE_FREQ_CHANGE,
> .handle_pmi_message = cbe_cpufreq_handle_pmi,
> };
>
> +void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy)
> +{
> + struct dev_pm_qos_request *req;
> + int ret;
> +
> + if (!cbe_cpufreq_has_pmi)
> + return;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return;
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu), req,
> + DEV_PM_QOS_MAX_FREQUENCY,
> + policy->freq_table[0].frequency);
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint (%d)\n", ret);
> + kfree(req);
> + return;
> + }
>
> + policy->driver_data = req;
> +}
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_init);
>
> -static int __init cbe_cpufreq_pmi_init(void)
> +void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy)
> {
> - cbe_cpufreq_has_pmi = pmi_register_handler(&cbe_pmi_handler) == 0;
> + struct dev_pm_qos_request *req = policy->driver_data;
>
> - if (!cbe_cpufreq_has_pmi)
> - return -ENODEV;
> + if (cbe_cpufreq_has_pmi) {
> + dev_pm_qos_remove_request(req);
> + kfree(req);
> + }
> +}
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_exit);
>
> - cpufreq_register_notifier(&pmi_notifier_block, CPUFREQ_POLICY_NOTIFIER);
> +void cbe_cpufreq_pmi_init(void)
> +{
> + if (!pmi_register_handler(&cbe_pmi_handler))
> + cbe_cpufreq_has_pmi = true;
> +}
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_init);
>
> - return 0;
> +void cbe_cpufreq_pmi_exit(void)
> +{
> + pmi_unregister_handler(&cbe_pmi_handler);
> + cbe_cpufreq_has_pmi = false;
> }
> -device_initcall(cbe_cpufreq_pmi_init);
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_exit);
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-08-09 9:01 ` Rafael J. Wysocki
@ 2019-08-19 2:26 ` Viresh Kumar
2019-08-19 6:42 ` Rafael J. Wysocki
0 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2019-08-19 2:26 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael Wysocki, Linux PM, Vincent Guittot, Linux Kernel Mailing List
On 09-08-19, 11:01, Rafael J. Wysocki wrote:
> On Fri, Aug 9, 2019 at 4:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> >
> > On 23-07-19, 11:44, Viresh Kumar wrote:
> > > The cpufreq core now takes the min/max frequency constraints via QoS
> > > requests and the CPUFREQ_ADJUST notifier shall get removed later on.
> > >
> > > Switch over to using the QoS request for maximum frequency constraint
> > > for ppc_cbe_cpufreq driver.
> > >
> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > ---
> > > drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> > > drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> > > drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> > > 3 files changed, 86 insertions(+), 37 deletions(-)
> >
> > -------------------------8<-------------------------
>
> If you do it this way, Patchwork will not pick up the patch.
>
> Please send afresh with "[Update]" or bumped up version number in the
> subject (or both).
Okay, will take care of this in future. Was away on holidays and so
the late reply. Thanks.
--
viresh
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-08-19 2:26 ` Viresh Kumar
@ 2019-08-19 6:42 ` Rafael J. Wysocki
2019-08-19 6:47 ` Viresh Kumar
0 siblings, 1 reply; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-19 6:42 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Rafael Wysocki, Linux PM, Vincent Guittot,
Linux Kernel Mailing List
On Mon, Aug 19, 2019 at 4:26 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 09-08-19, 11:01, Rafael J. Wysocki wrote:
> > On Fri, Aug 9, 2019 at 4:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > >
> > > On 23-07-19, 11:44, Viresh Kumar wrote:
> > > > The cpufreq core now takes the min/max frequency constraints via QoS
> > > > requests and the CPUFREQ_ADJUST notifier shall get removed later on.
> > > >
> > > > Switch over to using the QoS request for maximum frequency constraint
> > > > for ppc_cbe_cpufreq driver.
> > > >
> > > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > > ---
> > > > drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> > > > drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> > > > drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> > > > 3 files changed, 86 insertions(+), 37 deletions(-)
> > >
> > > -------------------------8<-------------------------
> >
> > If you do it this way, Patchwork will not pick up the patch.
> >
> > Please send afresh with "[Update]" or bumped up version number in the
> > subject (or both).
>
> Okay, will take care of this in future. Was away on holidays and so
> the late reply. Thanks.
OK, thanks!
The series is on hold, though, because the acpi-cpufreq patch turned
out to be problematic and I didn't have the time to have a deeper look
at the problem last week.
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-08-19 6:42 ` Rafael J. Wysocki
@ 2019-08-19 6:47 ` Viresh Kumar
0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-08-19 6:47 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael Wysocki, Linux PM, Vincent Guittot, Linux Kernel Mailing List
On 19-08-19, 08:42, Rafael J. Wysocki wrote:
> OK, thanks!
>
> The series is on hold, though, because the acpi-cpufreq patch turned
> out to be problematic and I didn't have the time to have a deeper look
> at the problem last week.
And so was I wondering why it is present in bleeding-edge only :)
I don't think I am cc'd to any bug reports on that, can you please help me with
some information on it ?
--
viresh
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH V2 04/10] cpufreq: powerpc_cbe: Switch to QoS requests instead of cpufreq notifier
2019-08-09 2:34 ` Viresh Kumar
2019-08-09 9:01 ` Rafael J. Wysocki
@ 2019-08-10 12:19 ` Rafael J. Wysocki
1 sibling, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-10 12:19 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Linux PM, Vincent Guittot, Linux Kernel Mailing List
On Fri, Aug 9, 2019 at 4:34 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 23-07-19, 11:44, Viresh Kumar wrote:
> > The cpufreq core now takes the min/max frequency constraints via QoS
> > requests and the CPUFREQ_ADJUST notifier shall get removed later on.
> >
> > Switch over to using the QoS request for maximum frequency constraint
> > for ppc_cbe_cpufreq driver.
> >
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > ---
> > drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> > drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> > drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> > 3 files changed, 86 insertions(+), 37 deletions(-)
OK, picked up from email this time, but in the future please let
Patchwork pick up new versions of patches.
Thanks!
> -------------------------8<-------------------------
> From b84e1c119d63ab842c9e4f3acbc3aec22efa866d Mon Sep 17 00:00:00 2001
> Message-Id: <b84e1c119d63ab842c9e4f3acbc3aec22efa866d.1565318034.git.viresh.kumar@linaro.org>
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Date: Fri, 5 Jul 2019 15:49:48 +0530
> Subject: [PATCH] cpufreq: powerpc_cbe: Switch to QoS requests instead of
> cpufreq notifier
>
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for ppc_cbe_cpufreq driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> - dev_pm_qos_update_request() can return 1 on success
> drivers/cpufreq/ppc_cbe_cpufreq.c | 19 +++++-
> drivers/cpufreq/ppc_cbe_cpufreq.h | 8 +++
> drivers/cpufreq/ppc_cbe_cpufreq_pmi.c | 96 +++++++++++++++++----------
> 3 files changed, 86 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.c b/drivers/cpufreq/ppc_cbe_cpufreq.c
> index b83f36febf03..c58abb4cca3a 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq.c
> @@ -110,6 +110,13 @@ static int cbe_cpufreq_cpu_init(struct cpufreq_policy *policy)
> #endif
>
> policy->freq_table = cbe_freqs;
> + cbe_cpufreq_pmi_policy_init(policy);
> + return 0;
> +}
> +
> +static int cbe_cpufreq_cpu_exit(struct cpufreq_policy *policy)
> +{
> + cbe_cpufreq_pmi_policy_exit(policy);
> return 0;
> }
>
> @@ -129,6 +136,7 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
> .verify = cpufreq_generic_frequency_table_verify,
> .target_index = cbe_cpufreq_target,
> .init = cbe_cpufreq_cpu_init,
> + .exit = cbe_cpufreq_cpu_exit,
> .name = "cbe-cpufreq",
> .flags = CPUFREQ_CONST_LOOPS,
> };
> @@ -139,15 +147,24 @@ static struct cpufreq_driver cbe_cpufreq_driver = {
>
> static int __init cbe_cpufreq_init(void)
> {
> + int ret;
> +
> if (!machine_is(cell))
> return -ENODEV;
>
> - return cpufreq_register_driver(&cbe_cpufreq_driver);
> + cbe_cpufreq_pmi_init();
> +
> + ret = cpufreq_register_driver(&cbe_cpufreq_driver);
> + if (ret)
> + cbe_cpufreq_pmi_exit();
> +
> + return ret;
> }
>
> static void __exit cbe_cpufreq_exit(void)
> {
> cpufreq_unregister_driver(&cbe_cpufreq_driver);
> + cbe_cpufreq_pmi_exit();
> }
>
> module_init(cbe_cpufreq_init);
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq.h b/drivers/cpufreq/ppc_cbe_cpufreq.h
> index 9d973519d669..00cd8633b0d9 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq.h
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq.h
> @@ -20,6 +20,14 @@ int cbe_cpufreq_set_pmode_pmi(int cpu, unsigned int pmode);
>
> #if IS_ENABLED(CONFIG_CPU_FREQ_CBE_PMI)
> extern bool cbe_cpufreq_has_pmi;
> +void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy);
> +void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy);
> +void cbe_cpufreq_pmi_init(void);
> +void cbe_cpufreq_pmi_exit(void);
> #else
> #define cbe_cpufreq_has_pmi (0)
> +static inline void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy) {}
> +static inline void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy) {}
> +static inline void cbe_cpufreq_pmi_init(void) {}
> +static inline void cbe_cpufreq_pmi_exit(void) {}
> #endif
> diff --git a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> index 97c8ee4614b7..bc9dd30395c4 100644
> --- a/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> +++ b/drivers/cpufreq/ppc_cbe_cpufreq_pmi.c
> @@ -12,6 +12,7 @@
> #include <linux/timer.h>
> #include <linux/init.h>
> #include <linux/of_platform.h>
> +#include <linux/pm_qos.h>
>
> #include <asm/processor.h>
> #include <asm/prom.h>
> @@ -24,8 +25,6 @@
>
> #include "ppc_cbe_cpufreq.h"
>
> -static u8 pmi_slow_mode_limit[MAX_CBE];
> -
> bool cbe_cpufreq_has_pmi = false;
> EXPORT_SYMBOL_GPL(cbe_cpufreq_has_pmi);
>
> @@ -65,64 +64,89 @@ EXPORT_SYMBOL_GPL(cbe_cpufreq_set_pmode_pmi);
>
> static void cbe_cpufreq_handle_pmi(pmi_message_t pmi_msg)
> {
> + struct cpufreq_policy *policy;
> + struct dev_pm_qos_request *req;
> u8 node, slow_mode;
> + int cpu, ret;
>
> BUG_ON(pmi_msg.type != PMI_TYPE_FREQ_CHANGE);
>
> node = pmi_msg.data1;
> slow_mode = pmi_msg.data2;
>
> - pmi_slow_mode_limit[node] = slow_mode;
> + cpu = cbe_node_to_cpu(node);
>
> pr_debug("cbe_handle_pmi: node: %d max_freq: %d\n", node, slow_mode);
> -}
> -
> -static int pmi_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - struct cpufreq_frequency_table *cbe_freqs = policy->freq_table;
> - u8 node;
> -
> - /* Should this really be called for CPUFREQ_ADJUST and CPUFREQ_NOTIFY
> - * policy events?)
> - */
> - node = cbe_cpu_to_node(policy->cpu);
> -
> - pr_debug("got notified, event=%lu, node=%u\n", event, node);
>
> - if (pmi_slow_mode_limit[node] != 0) {
> - pr_debug("limiting node %d to slow mode %d\n",
> - node, pmi_slow_mode_limit[node]);
> + policy = cpufreq_cpu_get(cpu);
> + if (!policy) {
> + pr_warn("cpufreq policy not found cpu%d\n", cpu);
> + return;
> + }
>
> - cpufreq_verify_within_limits(policy, 0,
> + req = policy->driver_data;
>
> - cbe_freqs[pmi_slow_mode_limit[node]].frequency);
> - }
> + ret = dev_pm_qos_update_request(req,
> + policy->freq_table[slow_mode].frequency);
> + if (ret < 0)
> + pr_warn("Failed to update freq constraint: %d\n", ret);
> + else
> + pr_debug("limiting node %d to slow mode %d\n", node, slow_mode);
>
> - return 0;
> + cpufreq_cpu_put(policy);
> }
>
> -static struct notifier_block pmi_notifier_block = {
> - .notifier_call = pmi_notifier,
> -};
> -
> static struct pmi_handler cbe_pmi_handler = {
> .type = PMI_TYPE_FREQ_CHANGE,
> .handle_pmi_message = cbe_cpufreq_handle_pmi,
> };
>
> +void cbe_cpufreq_pmi_policy_init(struct cpufreq_policy *policy)
> +{
> + struct dev_pm_qos_request *req;
> + int ret;
> +
> + if (!cbe_cpufreq_has_pmi)
> + return;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return;
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu), req,
> + DEV_PM_QOS_MAX_FREQUENCY,
> + policy->freq_table[0].frequency);
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint (%d)\n", ret);
> + kfree(req);
> + return;
> + }
>
> + policy->driver_data = req;
> +}
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_init);
>
> -static int __init cbe_cpufreq_pmi_init(void)
> +void cbe_cpufreq_pmi_policy_exit(struct cpufreq_policy *policy)
> {
> - cbe_cpufreq_has_pmi = pmi_register_handler(&cbe_pmi_handler) == 0;
> + struct dev_pm_qos_request *req = policy->driver_data;
>
> - if (!cbe_cpufreq_has_pmi)
> - return -ENODEV;
> + if (cbe_cpufreq_has_pmi) {
> + dev_pm_qos_remove_request(req);
> + kfree(req);
> + }
> +}
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_policy_exit);
>
> - cpufreq_register_notifier(&pmi_notifier_block, CPUFREQ_POLICY_NOTIFIER);
> +void cbe_cpufreq_pmi_init(void)
> +{
> + if (!pmi_register_handler(&cbe_pmi_handler))
> + cbe_cpufreq_has_pmi = true;
> +}
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_init);
>
> - return 0;
> +void cbe_cpufreq_pmi_exit(void)
> +{
> + pmi_unregister_handler(&cbe_pmi_handler);
> + cbe_cpufreq_has_pmi = false;
> }
> -device_initcall(cbe_cpufreq_pmi_init);
> +EXPORT_SYMBOL_GPL(cbe_cpufreq_pmi_exit);
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
2019-07-23 6:14 ` Viresh Kumar
` (5 preceding siblings ...)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
2019-08-05 9:42 ` [Devel] " Rafael J. Wysocki
2019-08-28 8:50 ` [PATCH V3 " Viresh Kumar
-1 siblings, 2 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Len Brown, Zhang Rui, Robert Moore, Erik Schmauss
Cc: Viresh Kumar, linux-pm, Vincent Guittot, linux-acpi, linux-kernel, devel
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for acpi driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/acpi/processor_driver.c | 44 +++++++++++--
drivers/acpi/processor_perflib.c | 106 +++++++++++++------------------
drivers/acpi/processor_thermal.c | 81 ++++++++++++-----------
include/acpi/processor.h | 22 ++++---
4 files changed, 137 insertions(+), 116 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index aea8d674a33d..e7a3f07e9879 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -284,6 +284,35 @@ static int acpi_processor_stop(struct device *dev)
return 0;
}
+bool acpi_processor_cpufreq_init;
+
+static int acpi_processor_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu;
+
+ if (event == CPUFREQ_CREATE_POLICY) {
+ for_each_cpu(cpu, policy->cpus)
+ per_cpu(processors, cpu)->policy = policy;
+
+ acpi_thermal_cpufreq_init(policy);
+ acpi_processor_ppc_init(policy);
+ } else if (event == CPUFREQ_REMOVE_POLICY) {
+ acpi_processor_ppc_exit(policy);
+ acpi_thermal_cpufreq_exit(policy);
+
+ for_each_cpu(cpu, policy->cpus)
+ per_cpu(processors, cpu)->policy = NULL;
+ }
+
+ return 0;
+}
+
+static struct notifier_block acpi_processor_notifier_block = {
+ .notifier_call = acpi_processor_notifier,
+};
+
/*
* We keep the driver loaded even when ACPI is not running.
* This is needed for the powernow-k8 driver, that works even without
@@ -310,8 +339,11 @@ static int __init acpi_processor_driver_init(void)
cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
NULL, acpi_soft_cpu_dead);
- acpi_thermal_cpufreq_init();
- acpi_processor_ppc_init();
+ if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER)) {
+ acpi_processor_cpufreq_init = true;
+ }
+
acpi_processor_throttling_init();
return 0;
err:
@@ -324,8 +356,12 @@ static void __exit acpi_processor_driver_exit(void)
if (acpi_disabled)
return;
- acpi_processor_ppc_exit();
- acpi_thermal_cpufreq_exit();
+ if (acpi_processor_cpufreq_init) {
+ cpufreq_unregister_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER);
+ acpi_processor_cpufreq_init = false;
+ }
+
cpuhp_remove_state_nocalls(hp_online);
cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
driver_unregister(&acpi_processor_driver);
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index ee87cb6f6e59..1a22b2415a8b 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
"limited by BIOS, this should help");
-#define PPC_REGISTERED 1
-#define PPC_IN_USE 2
-
-static int acpi_processor_ppc_status;
-
-static int acpi_processor_ppc_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct acpi_processor *pr;
- unsigned int ppc = 0;
-
- if (ignore_ppc < 0)
- ignore_ppc = 0;
-
- if (ignore_ppc)
- return 0;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- mutex_lock(&performance_mutex);
-
- pr = per_cpu(processors, policy->cpu);
- if (!pr || !pr->performance)
- goto out;
-
- ppc = (unsigned int)pr->performance_platform_limit;
-
- if (ppc >= pr->performance->state_count)
- goto out;
-
- cpufreq_verify_within_limits(policy, 0,
- pr->performance->states[ppc].
- core_frequency * 1000);
-
- out:
- mutex_unlock(&performance_mutex);
-
- return 0;
-}
-
-static struct notifier_block acpi_ppc_notifier_block = {
- .notifier_call = acpi_processor_ppc_notifier,
-};
+static int acpi_processor_ppc_in_use;
static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
{
acpi_status status = 0;
unsigned long long ppc = 0;
-
+ int ret;
if (!pr)
return -EINVAL;
@@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
if (status != AE_NOT_FOUND)
- acpi_processor_ppc_status |= PPC_IN_USE;
+ acpi_processor_ppc_in_use = 1;
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
@@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
pr->performance_platform_limit = (int)ppc;
+ if (ignore_ppc || ppc >= pr->performance->state_count)
+ return 0;
+
+ ret = dev_pm_qos_update_request(pr->perflib_req,
+ pr->performance->states[ppc].core_frequency * 1000);
+ if (ret) {
+ pr_warn("Failed to update perflib freq constraint: cpu%d (%d)\n",
+ pr->id, ret);
+ }
+
return 0;
}
@@ -184,23 +150,39 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
}
EXPORT_SYMBOL(acpi_processor_get_bios_limit);
-void acpi_processor_ppc_init(void)
+void acpi_processor_ppc_init(struct cpufreq_policy *policy)
{
- if (!cpufreq_register_notifier
- (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
- acpi_processor_ppc_status |= PPC_REGISTERED;
- else
- printk(KERN_DEBUG
- "Warning: Processor Platform Limit not supported.\n");
+ struct acpi_processor *pr = per_cpu(processors, policy->cpu);
+ struct dev_pm_qos_request *req;
+ int ret;
+
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu),
+ req, DEV_PM_QOS_MAX_FREQUENCY,
+ policy->cpuinfo.max_freq);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for cpu%d (%d)\n",
+ policy->cpu, ret);
+ kfree(req);
+ return;
+ }
+
+ pr->perflib_req = req;
+
+ if (ignore_ppc < 0)
+ ignore_ppc = 0;
}
-void acpi_processor_ppc_exit(void)
+void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
{
- if (acpi_processor_ppc_status & PPC_REGISTERED)
- cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, policy->cpu);
- acpi_processor_ppc_status &= ~PPC_REGISTERED;
+ dev_pm_qos_remove_request(pr->perflib_req);
+ kfree(pr->perflib_req);
+ pr->perflib_req = NULL;
}
static int acpi_processor_get_performance_control(struct acpi_processor *pr)
@@ -477,7 +459,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
static int is_done = 0;
int result;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EBUSY;
if (!try_module_get(calling_module))
@@ -513,7 +495,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
* we can allow the cpufreq driver to be rmmod'ed. */
is_done = 1;
- if (!(acpi_processor_ppc_status & PPC_IN_USE))
+ if (!acpi_processor_ppc_in_use)
module_put(calling_module);
return 0;
@@ -742,7 +724,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
{
struct acpi_processor *pr;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EINVAL;
mutex_lock(&performance_mutex);
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 50fb0107375e..02407b33b874 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
#define CPUFREQ_THERMAL_MAX_STEP 3
static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
-static unsigned int acpi_thermal_cpufreq_is_init = 0;
#define reduction_pctg(cpu) \
per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
@@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
static int cpu_has_cpufreq(unsigned int cpu)
{
struct cpufreq_policy policy;
- if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
+ if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
return 0;
return 1;
}
-static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- unsigned long max_freq = 0;
-
- if (event != CPUFREQ_ADJUST)
- goto out;
-
- max_freq = (
- policy->cpuinfo.max_freq *
- (100 - reduction_pctg(policy->cpu) * 20)
- ) / 100;
-
- cpufreq_verify_within_limits(policy, 0, max_freq);
-
- out:
- return 0;
-}
-
-static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
- .notifier_call = acpi_thermal_cpufreq_notifier,
-};
-
static int cpufreq_get_max_state(unsigned int cpu)
{
if (!cpu_has_cpufreq(cpu))
@@ -108,7 +83,9 @@ static int cpufreq_get_cur_state(unsigned int cpu)
static int cpufreq_set_cur_state(unsigned int cpu, int state)
{
- int i;
+ struct acpi_processor *pr;
+ unsigned long max_freq;
+ int i, ret;
if (!cpu_has_cpufreq(cpu))
return 0;
@@ -121,33 +98,53 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
* frequency.
*/
for_each_online_cpu(i) {
- if (topology_physical_package_id(i) ==
+ if (topology_physical_package_id(i) !=
topology_physical_package_id(cpu))
- cpufreq_update_policy(i);
+ continue;
+
+ pr = per_cpu(processors, i);
+ max_freq = (pr->policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
+
+ ret = dev_pm_qos_update_request(pr->thermal_req, max_freq);
+ if (ret) {
+ pr_warn("Failed to update thermal freq constraint: cpu%d (%d)\n",
+ pr->id, ret);
+ }
}
return 0;
}
-void acpi_thermal_cpufreq_init(void)
+void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
{
- int i;
+ struct acpi_processor *pr = per_cpu(processors, policy->cpu);
+ struct dev_pm_qos_request *req;
+ int ret;
+
+ req = kzalloc(sizeof(*req), GFP_KERNEL);
+ if (!req)
+ return;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu),
+ req, DEV_PM_QOS_MAX_FREQUENCY,
+ policy->cpuinfo.max_freq);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for cpu%d (%d)\n",
+ policy->cpu, ret);
+ kfree(req);
+ return;
+ }
- i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
- if (!i)
- acpi_thermal_cpufreq_is_init = 1;
+ pr->thermal_req = req;
}
-void acpi_thermal_cpufreq_exit(void)
+void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
{
- if (acpi_thermal_cpufreq_is_init)
- cpufreq_unregister_notifier
- (&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, policy->cpu);
- acpi_thermal_cpufreq_is_init = 0;
+ dev_pm_qos_remove_request(pr->thermal_req);
+ kfree(pr->thermal_req);
+ pr->thermal_req = NULL;
}
-
#else /* ! CONFIG_CPU_FREQ */
static int cpufreq_get_max_state(unsigned int cpu)
{
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 1194a4c78d55..a1a7966bb755 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -4,6 +4,8 @@
#include <linux/kernel.h>
#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/pm_qos.h>
#include <linux/thermal.h>
#include <asm/acpi.h>
@@ -230,6 +232,9 @@ struct acpi_processor {
struct acpi_processor_limit limit;
struct thermal_cooling_device *cdev;
struct device *dev; /* Processor device. */
+ struct cpufreq_policy *policy;
+ struct dev_pm_qos_request *perflib_req;
+ struct dev_pm_qos_request *thermal_req;
};
struct acpi_processor_errata {
@@ -296,16 +301,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
/* in processor_perflib.c */
#ifdef CONFIG_CPU_FREQ
-void acpi_processor_ppc_init(void);
-void acpi_processor_ppc_exit(void);
+extern bool acpi_processor_cpufreq_init;
+void acpi_processor_ppc_init(struct cpufreq_policy *policy);
+void acpi_processor_ppc_exit(struct cpufreq_policy *policy);
void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
#else
-static inline void acpi_processor_ppc_init(void)
+static inline void acpi_processor_ppc_init(struct cpufreq_policy *policy)
{
return;
}
-static inline void acpi_processor_ppc_exit(void)
+static inline void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
{
return;
}
@@ -421,14 +427,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
int acpi_processor_get_limit_info(struct acpi_processor *pr);
extern const struct thermal_cooling_device_ops processor_cooling_ops;
#if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
-void acpi_thermal_cpufreq_init(void);
-void acpi_thermal_cpufreq_exit(void);
+void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy);
+void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy);
#else
-static inline void acpi_thermal_cpufreq_init(void)
+static inline void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
{
return;
}
-static inline void acpi_thermal_cpufreq_exit(void)
+static inline void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
{
return;
}
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
@ 2019-08-05 9:42 ` Rafael J. Wysocki
0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-05 9:42 UTC (permalink / raw)
To: Viresh Kumar
Cc: Len Brown, Zhang Rui, Robert Moore, Erik Schmauss, linux-pm,
Vincent Guittot, linux-acpi, linux-kernel, devel
On Tuesday, July 23, 2019 8:14:05 AM CEST Viresh Kumar wrote:
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for acpi driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/acpi/processor_driver.c | 44 +++++++++++--
> drivers/acpi/processor_perflib.c | 106 +++++++++++++------------------
> drivers/acpi/processor_thermal.c | 81 ++++++++++++-----------
> include/acpi/processor.h | 22 ++++---
> 4 files changed, 137 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index aea8d674a33d..e7a3f07e9879 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -284,6 +284,35 @@ static int acpi_processor_stop(struct device *dev)
> return 0;
> }
>
> +bool acpi_processor_cpufreq_init;
> +
> +static int acpi_processor_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct cpufreq_policy *policy = data;
> + int cpu;
> +
> + if (event == CPUFREQ_CREATE_POLICY) {
> + for_each_cpu(cpu, policy->cpus)
> + per_cpu(processors, cpu)->policy = policy;
> +
> + acpi_thermal_cpufreq_init(policy);
> + acpi_processor_ppc_init(policy);
> + } else if (event == CPUFREQ_REMOVE_POLICY) {
> + acpi_processor_ppc_exit(policy);
> + acpi_thermal_cpufreq_exit(policy);
> +
> + for_each_cpu(cpu, policy->cpus)
> + per_cpu(processors, cpu)->policy = NULL;
> + }
It doesn't look like it is necessary to pass policy to the
functions here, just the CPU number.
Also I don't think it is necessary to squirrel the policy pointer.
> +
> + return 0;
> +}
> +
> +static struct notifier_block acpi_processor_notifier_block = {
> + .notifier_call = acpi_processor_notifier,
> +};
> +
> /*
> * We keep the driver loaded even when ACPI is not running.
> * This is needed for the powernow-k8 driver, that works even without
> @@ -310,8 +339,11 @@ static int __init acpi_processor_driver_init(void)
> cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> NULL, acpi_soft_cpu_dead);
>
> - acpi_thermal_cpufreq_init();
> - acpi_processor_ppc_init();
> + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER)) {
> + acpi_processor_cpufreq_init = true;
Can't that be set/cleared by acpi_processor_notifier() itself?
> + }
Redundant braces.
> +
> acpi_processor_throttling_init();
> return 0;
> err:
> @@ -324,8 +356,12 @@ static void __exit acpi_processor_driver_exit(void)
> if (acpi_disabled)
> return;
>
> - acpi_processor_ppc_exit();
> - acpi_thermal_cpufreq_exit();
> + if (acpi_processor_cpufreq_init) {
> + cpufreq_unregister_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER);
> + acpi_processor_cpufreq_init = false;
> + }
> +
> cpuhp_remove_state_nocalls(hp_online);
> cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
> driver_unregister(&acpi_processor_driver);
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index ee87cb6f6e59..1a22b2415a8b 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
> MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> "limited by BIOS, this should help");
>
> -#define PPC_REGISTERED 1
> -#define PPC_IN_USE 2
> -
> -static int acpi_processor_ppc_status;
> -
> -static int acpi_processor_ppc_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - struct acpi_processor *pr;
> - unsigned int ppc = 0;
> -
> - if (ignore_ppc < 0)
> - ignore_ppc = 0;
> -
> - if (ignore_ppc)
> - return 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - return 0;
> -
> - mutex_lock(&performance_mutex);
> -
> - pr = per_cpu(processors, policy->cpu);
> - if (!pr || !pr->performance)
> - goto out;
> -
> - ppc = (unsigned int)pr->performance_platform_limit;
> -
> - if (ppc >= pr->performance->state_count)
> - goto out;
> -
> - cpufreq_verify_within_limits(policy, 0,
> - pr->performance->states[ppc].
> - core_frequency * 1000);
> -
> - out:
> - mutex_unlock(&performance_mutex);
> -
> - return 0;
> -}
> -
> -static struct notifier_block acpi_ppc_notifier_block = {
> - .notifier_call = acpi_processor_ppc_notifier,
> -};
> +static int acpi_processor_ppc_in_use;
Why not bool?
>
> static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> {
> acpi_status status = 0;
> unsigned long long ppc = 0;
> -
> + int ret;
>
> if (!pr)
> return -EINVAL;
> @@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
>
> if (status != AE_NOT_FOUND)
> - acpi_processor_ppc_status |= PPC_IN_USE;
> + acpi_processor_ppc_in_use = 1;
>
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
> @@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
>
> pr->performance_platform_limit = (int)ppc;
>
> + if (ignore_ppc || ppc >= pr->performance->state_count)
> + return 0;
> +
> + ret = dev_pm_qos_update_request(pr->perflib_req,
> + pr->performance->states[ppc].core_frequency * 1000);
> + if (ret) {
> + pr_warn("Failed to update perflib freq constraint: cpu%d (%d)\n",
> + pr->id, ret);
> + }
> +
> return 0;
> }
>
> @@ -184,23 +150,39 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> }
> EXPORT_SYMBOL(acpi_processor_get_bios_limit);
>
> -void acpi_processor_ppc_init(void)
> +void acpi_processor_ppc_init(struct cpufreq_policy *policy)
> {
> - if (!cpufreq_register_notifier
> - (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
> - acpi_processor_ppc_status |= PPC_REGISTERED;
> - else
> - printk(KERN_DEBUG
> - "Warning: Processor Platform Limit not supported.\n");
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
> + struct dev_pm_qos_request *req;
> + int ret;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return;
The above wouldn't be necessary if the request struct was part of struct acpi_processor.
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu),
> + req, DEV_PM_QOS_MAX_FREQUENCY,
> + policy->cpuinfo.max_freq);
The last argument doesn't need to be policy->cpuinfo.max_freq AFAICS. It might as well
be a large constant ("infinity"), so the function doesn't seem to need a policy pointer,
just the CPU number.
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for cpu%d (%d)\n",
> + policy->cpu, ret);
> + kfree(req);
> + return;
> + }
> +
> + pr->perflib_req = req;
> +
> + if (ignore_ppc < 0)
> + ignore_ppc = 0;
> }
>
> -void acpi_processor_ppc_exit(void)
> +void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
> {
> - if (acpi_processor_ppc_status & PPC_REGISTERED)
> - cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
>
> - acpi_processor_ppc_status &= ~PPC_REGISTERED;
> + dev_pm_qos_remove_request(pr->perflib_req);
> + kfree(pr->perflib_req);
> + pr->perflib_req = NULL;
> }
>
> static int acpi_processor_get_performance_control(struct acpi_processor *pr)
> @@ -477,7 +459,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> static int is_done = 0;
> int result;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EBUSY;
>
> if (!try_module_get(calling_module))
> @@ -513,7 +495,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> * we can allow the cpufreq driver to be rmmod'ed. */
> is_done = 1;
>
> - if (!(acpi_processor_ppc_status & PPC_IN_USE))
> + if (!acpi_processor_ppc_in_use)
> module_put(calling_module);
>
> return 0;
> @@ -742,7 +724,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
> {
> struct acpi_processor *pr;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EINVAL;
>
> mutex_lock(&performance_mutex);
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index 50fb0107375e..02407b33b874 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
> #define CPUFREQ_THERMAL_MAX_STEP 3
>
> static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
> -static unsigned int acpi_thermal_cpufreq_is_init = 0;
>
> #define reduction_pctg(cpu) \
> per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
> @@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
> static int cpu_has_cpufreq(unsigned int cpu)
> {
> struct cpufreq_policy policy;
> - if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
> + if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
> return 0;
> return 1;
> }
>
> -static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - unsigned long max_freq = 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - goto out;
> -
> - max_freq = (
> - policy->cpuinfo.max_freq *
> - (100 - reduction_pctg(policy->cpu) * 20)
> - ) / 100;
> -
> - cpufreq_verify_within_limits(policy, 0, max_freq);
> -
> - out:
> - return 0;
> -}
> -
> -static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
> - .notifier_call = acpi_thermal_cpufreq_notifier,
> -};
> -
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> if (!cpu_has_cpufreq(cpu))
> @@ -108,7 +83,9 @@ static int cpufreq_get_cur_state(unsigned int cpu)
>
> static int cpufreq_set_cur_state(unsigned int cpu, int state)
> {
> - int i;
> + struct acpi_processor *pr;
> + unsigned long max_freq;
> + int i, ret;
>
> if (!cpu_has_cpufreq(cpu))
> return 0;
> @@ -121,33 +98,53 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
> * frequency.
> */
> for_each_online_cpu(i) {
> - if (topology_physical_package_id(i) ==
> + if (topology_physical_package_id(i) !=
> topology_physical_package_id(cpu))
> - cpufreq_update_policy(i);
> + continue;
> +
> + pr = per_cpu(processors, i);
This could use cpufreq_cpu_get() to get to the policy (given the CPU number).
> + max_freq = (pr->policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
> +
> + ret = dev_pm_qos_update_request(pr->thermal_req, max_freq);
> + if (ret) {
> + pr_warn("Failed to update thermal freq constraint: cpu%d (%d)\n",
> + pr->id, ret);
Please spell CPU in capitals in messages.
> + }
> }
> return 0;
> }
>
> -void acpi_thermal_cpufreq_init(void)
> +void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
> {
> - int i;
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
> + struct dev_pm_qos_request *req;
> + int ret;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return;
Again, the request struct could be part of struct acpi_processor and the code
would be simpler then.
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu),
> + req, DEV_PM_QOS_MAX_FREQUENCY,
> + policy->cpuinfo.max_freq);
And again, the last argument here could be a large ("inifinity") constant here, so
no need to take the policy pointer.
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for cpu%d (%d)\n",
> + policy->cpu, ret);
> + kfree(req);
> + return;
> + }
>
> - i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> - if (!i)
> - acpi_thermal_cpufreq_is_init = 1;
> + pr->thermal_req = req;
> }
>
> -void acpi_thermal_cpufreq_exit(void)
> +void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
> {
> - if (acpi_thermal_cpufreq_is_init)
> - cpufreq_unregister_notifier
> - (&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
>
> - acpi_thermal_cpufreq_is_init = 0;
> + dev_pm_qos_remove_request(pr->thermal_req);
> + kfree(pr->thermal_req);
> + pr->thermal_req = NULL;
> }
> -
> #else /* ! CONFIG_CPU_FREQ */
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 1194a4c78d55..a1a7966bb755 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -4,6 +4,8 @@
>
> #include <linux/kernel.h>
> #include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/pm_qos.h>
> #include <linux/thermal.h>
> #include <asm/acpi.h>
>
> @@ -230,6 +232,9 @@ struct acpi_processor {
> struct acpi_processor_limit limit;
> struct thermal_cooling_device *cdev;
> struct device *dev; /* Processor device. */
> + struct cpufreq_policy *policy;
> + struct dev_pm_qos_request *perflib_req;
> + struct dev_pm_qos_request *thermal_req;
> };
>
> struct acpi_processor_errata {
> @@ -296,16 +301,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
> /* in processor_perflib.c */
>
> #ifdef CONFIG_CPU_FREQ
> -void acpi_processor_ppc_init(void);
> -void acpi_processor_ppc_exit(void);
> +extern bool acpi_processor_cpufreq_init;
> +void acpi_processor_ppc_init(struct cpufreq_policy *policy);
> +void acpi_processor_ppc_exit(struct cpufreq_policy *policy);
> void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
> extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
> #else
> -static inline void acpi_processor_ppc_init(void)
> +static inline void acpi_processor_ppc_init(struct cpufreq_policy *policy)
> {
> return;
> }
> -static inline void acpi_processor_ppc_exit(void)
> +static inline void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
> {
> return;
> }
> @@ -421,14 +427,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> int acpi_processor_get_limit_info(struct acpi_processor *pr);
> extern const struct thermal_cooling_device_ops processor_cooling_ops;
> #if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
> -void acpi_thermal_cpufreq_init(void);
> -void acpi_thermal_cpufreq_exit(void);
> +void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy);
> +void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy);
> #else
> -static inline void acpi_thermal_cpufreq_init(void)
> +static inline void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
> {
> return;
> }
> -static inline void acpi_thermal_cpufreq_exit(void)
> +static inline void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
> {
> return;
> }
>
Thanks!
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Devel] [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
@ 2019-08-05 9:42 ` Rafael J. Wysocki
0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-05 9:42 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 15717 bytes --]
On Tuesday, July 23, 2019 8:14:05 AM CEST Viresh Kumar wrote:
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for acpi driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
> ---
> drivers/acpi/processor_driver.c | 44 +++++++++++--
> drivers/acpi/processor_perflib.c | 106 +++++++++++++------------------
> drivers/acpi/processor_thermal.c | 81 ++++++++++++-----------
> include/acpi/processor.h | 22 ++++---
> 4 files changed, 137 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index aea8d674a33d..e7a3f07e9879 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -284,6 +284,35 @@ static int acpi_processor_stop(struct device *dev)
> return 0;
> }
>
> +bool acpi_processor_cpufreq_init;
> +
> +static int acpi_processor_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct cpufreq_policy *policy = data;
> + int cpu;
> +
> + if (event == CPUFREQ_CREATE_POLICY) {
> + for_each_cpu(cpu, policy->cpus)
> + per_cpu(processors, cpu)->policy = policy;
> +
> + acpi_thermal_cpufreq_init(policy);
> + acpi_processor_ppc_init(policy);
> + } else if (event == CPUFREQ_REMOVE_POLICY) {
> + acpi_processor_ppc_exit(policy);
> + acpi_thermal_cpufreq_exit(policy);
> +
> + for_each_cpu(cpu, policy->cpus)
> + per_cpu(processors, cpu)->policy = NULL;
> + }
It doesn't look like it is necessary to pass policy to the
functions here, just the CPU number.
Also I don't think it is necessary to squirrel the policy pointer.
> +
> + return 0;
> +}
> +
> +static struct notifier_block acpi_processor_notifier_block = {
> + .notifier_call = acpi_processor_notifier,
> +};
> +
> /*
> * We keep the driver loaded even when ACPI is not running.
> * This is needed for the powernow-k8 driver, that works even without
> @@ -310,8 +339,11 @@ static int __init acpi_processor_driver_init(void)
> cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> NULL, acpi_soft_cpu_dead);
>
> - acpi_thermal_cpufreq_init();
> - acpi_processor_ppc_init();
> + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER)) {
> + acpi_processor_cpufreq_init = true;
Can't that be set/cleared by acpi_processor_notifier() itself?
> + }
Redundant braces.
> +
> acpi_processor_throttling_init();
> return 0;
> err:
> @@ -324,8 +356,12 @@ static void __exit acpi_processor_driver_exit(void)
> if (acpi_disabled)
> return;
>
> - acpi_processor_ppc_exit();
> - acpi_thermal_cpufreq_exit();
> + if (acpi_processor_cpufreq_init) {
> + cpufreq_unregister_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER);
> + acpi_processor_cpufreq_init = false;
> + }
> +
> cpuhp_remove_state_nocalls(hp_online);
> cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
> driver_unregister(&acpi_processor_driver);
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index ee87cb6f6e59..1a22b2415a8b 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
> MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> "limited by BIOS, this should help");
>
> -#define PPC_REGISTERED 1
> -#define PPC_IN_USE 2
> -
> -static int acpi_processor_ppc_status;
> -
> -static int acpi_processor_ppc_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - struct acpi_processor *pr;
> - unsigned int ppc = 0;
> -
> - if (ignore_ppc < 0)
> - ignore_ppc = 0;
> -
> - if (ignore_ppc)
> - return 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - return 0;
> -
> - mutex_lock(&performance_mutex);
> -
> - pr = per_cpu(processors, policy->cpu);
> - if (!pr || !pr->performance)
> - goto out;
> -
> - ppc = (unsigned int)pr->performance_platform_limit;
> -
> - if (ppc >= pr->performance->state_count)
> - goto out;
> -
> - cpufreq_verify_within_limits(policy, 0,
> - pr->performance->states[ppc].
> - core_frequency * 1000);
> -
> - out:
> - mutex_unlock(&performance_mutex);
> -
> - return 0;
> -}
> -
> -static struct notifier_block acpi_ppc_notifier_block = {
> - .notifier_call = acpi_processor_ppc_notifier,
> -};
> +static int acpi_processor_ppc_in_use;
Why not bool?
>
> static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> {
> acpi_status status = 0;
> unsigned long long ppc = 0;
> -
> + int ret;
>
> if (!pr)
> return -EINVAL;
> @@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
>
> if (status != AE_NOT_FOUND)
> - acpi_processor_ppc_status |= PPC_IN_USE;
> + acpi_processor_ppc_in_use = 1;
>
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
> @@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
>
> pr->performance_platform_limit = (int)ppc;
>
> + if (ignore_ppc || ppc >= pr->performance->state_count)
> + return 0;
> +
> + ret = dev_pm_qos_update_request(pr->perflib_req,
> + pr->performance->states[ppc].core_frequency * 1000);
> + if (ret) {
> + pr_warn("Failed to update perflib freq constraint: cpu%d (%d)\n",
> + pr->id, ret);
> + }
> +
> return 0;
> }
>
> @@ -184,23 +150,39 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> }
> EXPORT_SYMBOL(acpi_processor_get_bios_limit);
>
> -void acpi_processor_ppc_init(void)
> +void acpi_processor_ppc_init(struct cpufreq_policy *policy)
> {
> - if (!cpufreq_register_notifier
> - (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
> - acpi_processor_ppc_status |= PPC_REGISTERED;
> - else
> - printk(KERN_DEBUG
> - "Warning: Processor Platform Limit not supported.\n");
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
> + struct dev_pm_qos_request *req;
> + int ret;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return;
The above wouldn't be necessary if the request struct was part of struct acpi_processor.
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu),
> + req, DEV_PM_QOS_MAX_FREQUENCY,
> + policy->cpuinfo.max_freq);
The last argument doesn't need to be policy->cpuinfo.max_freq AFAICS. It might as well
be a large constant ("infinity"), so the function doesn't seem to need a policy pointer,
just the CPU number.
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for cpu%d (%d)\n",
> + policy->cpu, ret);
> + kfree(req);
> + return;
> + }
> +
> + pr->perflib_req = req;
> +
> + if (ignore_ppc < 0)
> + ignore_ppc = 0;
> }
>
> -void acpi_processor_ppc_exit(void)
> +void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
> {
> - if (acpi_processor_ppc_status & PPC_REGISTERED)
> - cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
>
> - acpi_processor_ppc_status &= ~PPC_REGISTERED;
> + dev_pm_qos_remove_request(pr->perflib_req);
> + kfree(pr->perflib_req);
> + pr->perflib_req = NULL;
> }
>
> static int acpi_processor_get_performance_control(struct acpi_processor *pr)
> @@ -477,7 +459,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> static int is_done = 0;
> int result;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EBUSY;
>
> if (!try_module_get(calling_module))
> @@ -513,7 +495,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> * we can allow the cpufreq driver to be rmmod'ed. */
> is_done = 1;
>
> - if (!(acpi_processor_ppc_status & PPC_IN_USE))
> + if (!acpi_processor_ppc_in_use)
> module_put(calling_module);
>
> return 0;
> @@ -742,7 +724,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
> {
> struct acpi_processor *pr;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EINVAL;
>
> mutex_lock(&performance_mutex);
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index 50fb0107375e..02407b33b874 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
> #define CPUFREQ_THERMAL_MAX_STEP 3
>
> static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
> -static unsigned int acpi_thermal_cpufreq_is_init = 0;
>
> #define reduction_pctg(cpu) \
> per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
> @@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
> static int cpu_has_cpufreq(unsigned int cpu)
> {
> struct cpufreq_policy policy;
> - if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
> + if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
> return 0;
> return 1;
> }
>
> -static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - unsigned long max_freq = 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - goto out;
> -
> - max_freq = (
> - policy->cpuinfo.max_freq *
> - (100 - reduction_pctg(policy->cpu) * 20)
> - ) / 100;
> -
> - cpufreq_verify_within_limits(policy, 0, max_freq);
> -
> - out:
> - return 0;
> -}
> -
> -static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
> - .notifier_call = acpi_thermal_cpufreq_notifier,
> -};
> -
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> if (!cpu_has_cpufreq(cpu))
> @@ -108,7 +83,9 @@ static int cpufreq_get_cur_state(unsigned int cpu)
>
> static int cpufreq_set_cur_state(unsigned int cpu, int state)
> {
> - int i;
> + struct acpi_processor *pr;
> + unsigned long max_freq;
> + int i, ret;
>
> if (!cpu_has_cpufreq(cpu))
> return 0;
> @@ -121,33 +98,53 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
> * frequency.
> */
> for_each_online_cpu(i) {
> - if (topology_physical_package_id(i) ==
> + if (topology_physical_package_id(i) !=
> topology_physical_package_id(cpu))
> - cpufreq_update_policy(i);
> + continue;
> +
> + pr = per_cpu(processors, i);
This could use cpufreq_cpu_get() to get to the policy (given the CPU number).
> + max_freq = (pr->policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
> +
> + ret = dev_pm_qos_update_request(pr->thermal_req, max_freq);
> + if (ret) {
> + pr_warn("Failed to update thermal freq constraint: cpu%d (%d)\n",
> + pr->id, ret);
Please spell CPU in capitals in messages.
> + }
> }
> return 0;
> }
>
> -void acpi_thermal_cpufreq_init(void)
> +void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
> {
> - int i;
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
> + struct dev_pm_qos_request *req;
> + int ret;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return;
Again, the request struct could be part of struct acpi_processor and the code
would be simpler then.
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(policy->cpu),
> + req, DEV_PM_QOS_MAX_FREQUENCY,
> + policy->cpuinfo.max_freq);
And again, the last argument here could be a large ("inifinity") constant here, so
no need to take the policy pointer.
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for cpu%d (%d)\n",
> + policy->cpu, ret);
> + kfree(req);
> + return;
> + }
>
> - i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> - if (!i)
> - acpi_thermal_cpufreq_is_init = 1;
> + pr->thermal_req = req;
> }
>
> -void acpi_thermal_cpufreq_exit(void)
> +void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
> {
> - if (acpi_thermal_cpufreq_is_init)
> - cpufreq_unregister_notifier
> - (&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, policy->cpu);
>
> - acpi_thermal_cpufreq_is_init = 0;
> + dev_pm_qos_remove_request(pr->thermal_req);
> + kfree(pr->thermal_req);
> + pr->thermal_req = NULL;
> }
> -
> #else /* ! CONFIG_CPU_FREQ */
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 1194a4c78d55..a1a7966bb755 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -4,6 +4,8 @@
>
> #include <linux/kernel.h>
> #include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/pm_qos.h>
> #include <linux/thermal.h>
> #include <asm/acpi.h>
>
> @@ -230,6 +232,9 @@ struct acpi_processor {
> struct acpi_processor_limit limit;
> struct thermal_cooling_device *cdev;
> struct device *dev; /* Processor device. */
> + struct cpufreq_policy *policy;
> + struct dev_pm_qos_request *perflib_req;
> + struct dev_pm_qos_request *thermal_req;
> };
>
> struct acpi_processor_errata {
> @@ -296,16 +301,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
> /* in processor_perflib.c */
>
> #ifdef CONFIG_CPU_FREQ
> -void acpi_processor_ppc_init(void);
> -void acpi_processor_ppc_exit(void);
> +extern bool acpi_processor_cpufreq_init;
> +void acpi_processor_ppc_init(struct cpufreq_policy *policy);
> +void acpi_processor_ppc_exit(struct cpufreq_policy *policy);
> void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
> extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
> #else
> -static inline void acpi_processor_ppc_init(void)
> +static inline void acpi_processor_ppc_init(struct cpufreq_policy *policy)
> {
> return;
> }
> -static inline void acpi_processor_ppc_exit(void)
> +static inline void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
> {
> return;
> }
> @@ -421,14 +427,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> int acpi_processor_get_limit_info(struct acpi_processor *pr);
> extern const struct thermal_cooling_device_ops processor_cooling_ops;
> #if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
> -void acpi_thermal_cpufreq_init(void);
> -void acpi_thermal_cpufreq_exit(void);
> +void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy);
> +void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy);
> #else
> -static inline void acpi_thermal_cpufreq_init(void)
> +static inline void acpi_thermal_cpufreq_init(struct cpufreq_policy *policy)
> {
> return;
> }
> -static inline void acpi_thermal_cpufreq_exit(void)
> +static inline void acpi_thermal_cpufreq_exit(struct cpufreq_policy *policy)
> {
> return;
> }
>
Thanks!
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
2019-08-05 9:42 ` [Devel] " Rafael J. Wysocki
(?)
@ 2019-08-06 4:39 ` Viresh Kumar
2019-08-06 8:01 ` [Devel] " Rafael J. Wysocki
-1 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2019-08-06 4:39 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Len Brown, Zhang Rui, Robert Moore, Erik Schmauss, linux-pm,
Vincent Guittot, linux-acpi, linux-kernel, devel
On 05-08-19, 11:42, Rafael J. Wysocki wrote:
> On Tuesday, July 23, 2019 8:14:05 AM CEST Viresh Kumar wrote:
> > @@ -310,8 +339,11 @@ static int __init acpi_processor_driver_init(void)
> > cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> > NULL, acpi_soft_cpu_dead);
> >
> > - acpi_thermal_cpufreq_init();
> > - acpi_processor_ppc_init();
> > + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> > + CPUFREQ_POLICY_NOTIFIER)) {
> > + acpi_processor_cpufreq_init = true;
>
> Can't that be set/cleared by acpi_processor_notifier() itself?
This is required to be done only once at initialization and setting it
to true again and again on every invocation of the notifier callback
doesn't look right.
I have updated the patch based on rest of your suggestions, please see
if it looks okay now.
--
viresh
-------------------------8<-------------------------
From 7998742113d93f22078d5f267a50c91bd87a3e24 Mon Sep 17 00:00:00 2001
Message-Id: <7998742113d93f22078d5f267a50c91bd87a3e24.1565066143.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Mon, 15 Jul 2019 15:06:02 +0530
Subject: [PATCH] ACPI: cpufreq: Switch to QoS requests instead of cpufreq
notifier
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for acpi driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/acpi/processor_driver.c | 37 ++++++++++--
drivers/acpi/processor_perflib.c | 96 +++++++++++---------------------
drivers/acpi/processor_thermal.c | 81 +++++++++++++--------------
include/acpi/processor.h | 21 ++++---
4 files changed, 118 insertions(+), 117 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index aea8d674a33d..2c911fcaa4b4 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -284,6 +284,29 @@ static int acpi_processor_stop(struct device *dev)
return 0;
}
+bool acpi_processor_cpufreq_init;
+
+static int acpi_processor_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu = policy->cpu;
+
+ if (event == CPUFREQ_CREATE_POLICY) {
+ acpi_thermal_cpufreq_init(cpu);
+ acpi_processor_ppc_init(cpu);
+ } else if (event == CPUFREQ_REMOVE_POLICY) {
+ acpi_processor_ppc_exit(cpu);
+ acpi_thermal_cpufreq_exit(cpu);
+ }
+
+ return 0;
+}
+
+static struct notifier_block acpi_processor_notifier_block = {
+ .notifier_call = acpi_processor_notifier,
+};
+
/*
* We keep the driver loaded even when ACPI is not running.
* This is needed for the powernow-k8 driver, that works even without
@@ -310,8 +333,10 @@ static int __init acpi_processor_driver_init(void)
cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
NULL, acpi_soft_cpu_dead);
- acpi_thermal_cpufreq_init();
- acpi_processor_ppc_init();
+ if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER))
+ acpi_processor_cpufreq_init = true;
+
acpi_processor_throttling_init();
return 0;
err:
@@ -324,8 +349,12 @@ static void __exit acpi_processor_driver_exit(void)
if (acpi_disabled)
return;
- acpi_processor_ppc_exit();
- acpi_thermal_cpufreq_exit();
+ if (acpi_processor_cpufreq_init) {
+ cpufreq_unregister_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER);
+ acpi_processor_cpufreq_init = false;
+ }
+
cpuhp_remove_state_nocalls(hp_online);
cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
driver_unregister(&acpi_processor_driver);
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index ee87cb6f6e59..f3801d7c0d36 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
"limited by BIOS, this should help");
-#define PPC_REGISTERED 1
-#define PPC_IN_USE 2
-
-static int acpi_processor_ppc_status;
-
-static int acpi_processor_ppc_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct acpi_processor *pr;
- unsigned int ppc = 0;
-
- if (ignore_ppc < 0)
- ignore_ppc = 0;
-
- if (ignore_ppc)
- return 0;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- mutex_lock(&performance_mutex);
-
- pr = per_cpu(processors, policy->cpu);
- if (!pr || !pr->performance)
- goto out;
-
- ppc = (unsigned int)pr->performance_platform_limit;
-
- if (ppc >= pr->performance->state_count)
- goto out;
-
- cpufreq_verify_within_limits(policy, 0,
- pr->performance->states[ppc].
- core_frequency * 1000);
-
- out:
- mutex_unlock(&performance_mutex);
-
- return 0;
-}
-
-static struct notifier_block acpi_ppc_notifier_block = {
- .notifier_call = acpi_processor_ppc_notifier,
-};
+static bool acpi_processor_ppc_in_use;
static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
{
acpi_status status = 0;
unsigned long long ppc = 0;
-
+ int ret;
if (!pr)
return -EINVAL;
@@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
if (status != AE_NOT_FOUND)
- acpi_processor_ppc_status |= PPC_IN_USE;
+ acpi_processor_ppc_in_use = true;
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
@@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
pr->performance_platform_limit = (int)ppc;
+ if (ignore_ppc || ppc >= pr->performance->state_count)
+ return 0;
+
+ ret = dev_pm_qos_update_request(&pr->perflib_req,
+ pr->performance->states[ppc].core_frequency * 1000);
+ if (ret) {
+ pr_warn("Failed to update perflib freq constraint: cpu%d (%d)\n",
+ pr->id, ret);
+ }
+
return 0;
}
@@ -184,23 +150,29 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
}
EXPORT_SYMBOL(acpi_processor_get_bios_limit);
-void acpi_processor_ppc_init(void)
+void acpi_processor_ppc_init(int cpu)
{
- if (!cpufreq_register_notifier
- (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
- acpi_processor_ppc_status |= PPC_REGISTERED;
- else
- printk(KERN_DEBUG
- "Warning: Processor Platform Limit not supported.\n");
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for cpu%d (%d)\n", cpu,
+ ret);
+ return;
+ }
+
+ if (ignore_ppc < 0)
+ ignore_ppc = 0;
}
-void acpi_processor_ppc_exit(void)
+void acpi_processor_ppc_exit(int cpu)
{
- if (acpi_processor_ppc_status & PPC_REGISTERED)
- cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_processor_ppc_status &= ~PPC_REGISTERED;
+ dev_pm_qos_remove_request(&pr->perflib_req);
}
static int acpi_processor_get_performance_control(struct acpi_processor *pr)
@@ -477,7 +449,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
static int is_done = 0;
int result;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EBUSY;
if (!try_module_get(calling_module))
@@ -513,7 +485,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
* we can allow the cpufreq driver to be rmmod'ed. */
is_done = 1;
- if (!(acpi_processor_ppc_status & PPC_IN_USE))
+ if (!acpi_processor_ppc_in_use)
module_put(calling_module);
return 0;
@@ -742,7 +714,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
{
struct acpi_processor *pr;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EINVAL;
mutex_lock(&performance_mutex);
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 50fb0107375e..e38dba339dfb 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
#define CPUFREQ_THERMAL_MAX_STEP 3
static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
-static unsigned int acpi_thermal_cpufreq_is_init = 0;
#define reduction_pctg(cpu) \
per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
@@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
static int cpu_has_cpufreq(unsigned int cpu)
{
struct cpufreq_policy policy;
- if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
+ if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
return 0;
return 1;
}
-static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- unsigned long max_freq = 0;
-
- if (event != CPUFREQ_ADJUST)
- goto out;
-
- max_freq = (
- policy->cpuinfo.max_freq *
- (100 - reduction_pctg(policy->cpu) * 20)
- ) / 100;
-
- cpufreq_verify_within_limits(policy, 0, max_freq);
-
- out:
- return 0;
-}
-
-static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
- .notifier_call = acpi_thermal_cpufreq_notifier,
-};
-
static int cpufreq_get_max_state(unsigned int cpu)
{
if (!cpu_has_cpufreq(cpu))
@@ -108,7 +83,10 @@ static int cpufreq_get_cur_state(unsigned int cpu)
static int cpufreq_set_cur_state(unsigned int cpu, int state)
{
- int i;
+ struct cpufreq_policy *policy;
+ struct acpi_processor *pr;
+ unsigned long max_freq;
+ int i, ret;
if (!cpu_has_cpufreq(cpu))
return 0;
@@ -121,33 +99,50 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
* frequency.
*/
for_each_online_cpu(i) {
- if (topology_physical_package_id(i) ==
+ if (topology_physical_package_id(i) !=
topology_physical_package_id(cpu))
- cpufreq_update_policy(i);
+ continue;
+
+ pr = per_cpu(processors, i);
+
+ policy = cpufreq_cpu_get(i);
+ if (!policy)
+ return -EINVAL;
+
+ max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
+
+ cpufreq_cpu_put(policy);
+
+ ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
+ if (ret) {
+ pr_warn("Failed to update thermal freq constraint: cpu%d (%d)\n",
+ pr->id, ret);
+ }
}
return 0;
}
-void acpi_thermal_cpufreq_init(void)
+void acpi_thermal_cpufreq_init(int cpu)
{
- int i;
-
- i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
- if (!i)
- acpi_thermal_cpufreq_is_init = 1;
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for cpu%d (%d)\n", cpu,
+ ret);
+ return;
+ }
}
-void acpi_thermal_cpufreq_exit(void)
+void acpi_thermal_cpufreq_exit(int cpu)
{
- if (acpi_thermal_cpufreq_is_init)
- cpufreq_unregister_notifier
- (&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_thermal_cpufreq_is_init = 0;
+ dev_pm_qos_remove_request(&pr->thermal_req);
}
-
#else /* ! CONFIG_CPU_FREQ */
static int cpufreq_get_max_state(unsigned int cpu)
{
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 1194a4c78d55..ea21a6d20da7 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -4,6 +4,8 @@
#include <linux/kernel.h>
#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/pm_qos.h>
#include <linux/thermal.h>
#include <asm/acpi.h>
@@ -230,6 +232,8 @@ struct acpi_processor {
struct acpi_processor_limit limit;
struct thermal_cooling_device *cdev;
struct device *dev; /* Processor device. */
+ struct dev_pm_qos_request perflib_req;
+ struct dev_pm_qos_request thermal_req;
};
struct acpi_processor_errata {
@@ -296,16 +300,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
/* in processor_perflib.c */
#ifdef CONFIG_CPU_FREQ
-void acpi_processor_ppc_init(void);
-void acpi_processor_ppc_exit(void);
+extern bool acpi_processor_cpufreq_init;
+void acpi_processor_ppc_init(int cpu);
+void acpi_processor_ppc_exit(int cpu);
void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
#else
-static inline void acpi_processor_ppc_init(void)
+static inline void acpi_processor_ppc_init(int cpu)
{
return;
}
-static inline void acpi_processor_ppc_exit(void)
+static inline void acpi_processor_ppc_exit(int cpu)
{
return;
}
@@ -421,14 +426,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
int acpi_processor_get_limit_info(struct acpi_processor *pr);
extern const struct thermal_cooling_device_ops processor_cooling_ops;
#if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
-void acpi_thermal_cpufreq_init(void);
-void acpi_thermal_cpufreq_exit(void);
+void acpi_thermal_cpufreq_init(int cpu);
+void acpi_thermal_cpufreq_exit(int cpu);
#else
-static inline void acpi_thermal_cpufreq_init(void)
+static inline void acpi_thermal_cpufreq_init(int cpu)
{
return;
}
-static inline void acpi_thermal_cpufreq_exit(void)
+static inline void acpi_thermal_cpufreq_exit(int cpu)
{
return;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
@ 2019-08-06 8:01 ` Rafael J. Wysocki
0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-06 8:01 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Len Brown, Zhang Rui, Robert Moore,
Erik Schmauss, Linux PM, Vincent Guittot, ACPI Devel Maling List,
Linux Kernel Mailing List,
open list:ACPI COMPONENT ARCHITECTURE (ACPICA)
On Tue, Aug 6, 2019 at 6:39 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 05-08-19, 11:42, Rafael J. Wysocki wrote:
> > On Tuesday, July 23, 2019 8:14:05 AM CEST Viresh Kumar wrote:
> > > @@ -310,8 +339,11 @@ static int __init acpi_processor_driver_init(void)
> > > cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> > > NULL, acpi_soft_cpu_dead);
> > >
> > > - acpi_thermal_cpufreq_init();
> > > - acpi_processor_ppc_init();
> > > + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> > > + CPUFREQ_POLICY_NOTIFIER)) {
> > > + acpi_processor_cpufreq_init = true;
> >
> > Can't that be set/cleared by acpi_processor_notifier() itself?
>
> This is required to be done only once at initialization and setting it
> to true again and again on every invocation of the notifier callback
> doesn't look right.
>
> I have updated the patch based on rest of your suggestions, please see
> if it looks okay now.
Yes, it does, thanks!
[No need to resend, I'll take it from this message.]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Devel] [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
@ 2019-08-06 8:01 ` Rafael J. Wysocki
0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-06 8:01 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]
On Tue, Aug 6, 2019 at 6:39 AM Viresh Kumar <viresh.kumar(a)linaro.org> wrote:
>
> On 05-08-19, 11:42, Rafael J. Wysocki wrote:
> > On Tuesday, July 23, 2019 8:14:05 AM CEST Viresh Kumar wrote:
> > > @@ -310,8 +339,11 @@ static int __init acpi_processor_driver_init(void)
> > > cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> > > NULL, acpi_soft_cpu_dead);
> > >
> > > - acpi_thermal_cpufreq_init();
> > > - acpi_processor_ppc_init();
> > > + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> > > + CPUFREQ_POLICY_NOTIFIER)) {
> > > + acpi_processor_cpufreq_init = true;
> >
> > Can't that be set/cleared by acpi_processor_notifier() itself?
>
> This is required to be done only once at initialization and setting it
> to true again and again on every invocation of the notifier callback
> doesn't look right.
>
> I have updated the patch based on rest of your suggestions, please see
> if it looks okay now.
Yes, it does, thanks!
[No need to resend, I'll take it from this message.]
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
2019-08-06 8:01 ` [Devel] " Rafael J. Wysocki
(?)
@ 2019-08-06 8:47 ` Viresh Kumar
2019-08-09 2:33 ` Viresh Kumar
-1 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2019-08-06 8:47 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Len Brown, Zhang Rui, Robert Moore,
Erik Schmauss, Linux PM, Vincent Guittot, ACPI Devel Maling List,
Linux Kernel Mailing List,
open list:ACPI COMPONENT ARCHITECTURE (ACPICA)
On 06-08-19, 10:01, Rafael J. Wysocki wrote:
> Yes, it does, thanks!
>
> [No need to resend, I'll take it from this message.]
Forgot to write CPU in caps in print messages, updated now.
--
viresh
-------------------------8<-------------------------
From 5761009323fde6bbbef90f9ecdd3bf7c191672cb Mon Sep 17 00:00:00 2001
Message-Id: <5761009323fde6bbbef90f9ecdd3bf7c191672cb.1565081202.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Mon, 15 Jul 2019 15:06:02 +0530
Subject: [PATCH] ACPI: cpufreq: Switch to QoS requests instead of cpufreq
notifier
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for acpi driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/acpi/processor_driver.c | 37 ++++++++++--
drivers/acpi/processor_perflib.c | 96 +++++++++++---------------------
drivers/acpi/processor_thermal.c | 81 +++++++++++++--------------
include/acpi/processor.h | 21 ++++---
4 files changed, 118 insertions(+), 117 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index aea8d674a33d..2c911fcaa4b4 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -284,6 +284,29 @@ static int acpi_processor_stop(struct device *dev)
return 0;
}
+bool acpi_processor_cpufreq_init;
+
+static int acpi_processor_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu = policy->cpu;
+
+ if (event == CPUFREQ_CREATE_POLICY) {
+ acpi_thermal_cpufreq_init(cpu);
+ acpi_processor_ppc_init(cpu);
+ } else if (event == CPUFREQ_REMOVE_POLICY) {
+ acpi_processor_ppc_exit(cpu);
+ acpi_thermal_cpufreq_exit(cpu);
+ }
+
+ return 0;
+}
+
+static struct notifier_block acpi_processor_notifier_block = {
+ .notifier_call = acpi_processor_notifier,
+};
+
/*
* We keep the driver loaded even when ACPI is not running.
* This is needed for the powernow-k8 driver, that works even without
@@ -310,8 +333,10 @@ static int __init acpi_processor_driver_init(void)
cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
NULL, acpi_soft_cpu_dead);
- acpi_thermal_cpufreq_init();
- acpi_processor_ppc_init();
+ if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER))
+ acpi_processor_cpufreq_init = true;
+
acpi_processor_throttling_init();
return 0;
err:
@@ -324,8 +349,12 @@ static void __exit acpi_processor_driver_exit(void)
if (acpi_disabled)
return;
- acpi_processor_ppc_exit();
- acpi_thermal_cpufreq_exit();
+ if (acpi_processor_cpufreq_init) {
+ cpufreq_unregister_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER);
+ acpi_processor_cpufreq_init = false;
+ }
+
cpuhp_remove_state_nocalls(hp_online);
cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
driver_unregister(&acpi_processor_driver);
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index ee87cb6f6e59..fc74b8075c36 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
"limited by BIOS, this should help");
-#define PPC_REGISTERED 1
-#define PPC_IN_USE 2
-
-static int acpi_processor_ppc_status;
-
-static int acpi_processor_ppc_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct acpi_processor *pr;
- unsigned int ppc = 0;
-
- if (ignore_ppc < 0)
- ignore_ppc = 0;
-
- if (ignore_ppc)
- return 0;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- mutex_lock(&performance_mutex);
-
- pr = per_cpu(processors, policy->cpu);
- if (!pr || !pr->performance)
- goto out;
-
- ppc = (unsigned int)pr->performance_platform_limit;
-
- if (ppc >= pr->performance->state_count)
- goto out;
-
- cpufreq_verify_within_limits(policy, 0,
- pr->performance->states[ppc].
- core_frequency * 1000);
-
- out:
- mutex_unlock(&performance_mutex);
-
- return 0;
-}
-
-static struct notifier_block acpi_ppc_notifier_block = {
- .notifier_call = acpi_processor_ppc_notifier,
-};
+static bool acpi_processor_ppc_in_use;
static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
{
acpi_status status = 0;
unsigned long long ppc = 0;
-
+ int ret;
if (!pr)
return -EINVAL;
@@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
if (status != AE_NOT_FOUND)
- acpi_processor_ppc_status |= PPC_IN_USE;
+ acpi_processor_ppc_in_use = true;
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
@@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
pr->performance_platform_limit = (int)ppc;
+ if (ignore_ppc || ppc >= pr->performance->state_count)
+ return 0;
+
+ ret = dev_pm_qos_update_request(&pr->perflib_req,
+ pr->performance->states[ppc].core_frequency * 1000);
+ if (ret) {
+ pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
+ pr->id, ret);
+ }
+
return 0;
}
@@ -184,23 +150,29 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
}
EXPORT_SYMBOL(acpi_processor_get_bios_limit);
-void acpi_processor_ppc_init(void)
+void acpi_processor_ppc_init(int cpu)
{
- if (!cpufreq_register_notifier
- (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
- acpi_processor_ppc_status |= PPC_REGISTERED;
- else
- printk(KERN_DEBUG
- "Warning: Processor Platform Limit not supported.\n");
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
+ ret);
+ return;
+ }
+
+ if (ignore_ppc < 0)
+ ignore_ppc = 0;
}
-void acpi_processor_ppc_exit(void)
+void acpi_processor_ppc_exit(int cpu)
{
- if (acpi_processor_ppc_status & PPC_REGISTERED)
- cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_processor_ppc_status &= ~PPC_REGISTERED;
+ dev_pm_qos_remove_request(&pr->perflib_req);
}
static int acpi_processor_get_performance_control(struct acpi_processor *pr)
@@ -477,7 +449,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
static int is_done = 0;
int result;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EBUSY;
if (!try_module_get(calling_module))
@@ -513,7 +485,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
* we can allow the cpufreq driver to be rmmod'ed. */
is_done = 1;
- if (!(acpi_processor_ppc_status & PPC_IN_USE))
+ if (!acpi_processor_ppc_in_use)
module_put(calling_module);
return 0;
@@ -742,7 +714,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
{
struct acpi_processor *pr;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EINVAL;
mutex_lock(&performance_mutex);
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 50fb0107375e..1483e24fdea9 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
#define CPUFREQ_THERMAL_MAX_STEP 3
static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
-static unsigned int acpi_thermal_cpufreq_is_init = 0;
#define reduction_pctg(cpu) \
per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
@@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
static int cpu_has_cpufreq(unsigned int cpu)
{
struct cpufreq_policy policy;
- if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
+ if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
return 0;
return 1;
}
-static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- unsigned long max_freq = 0;
-
- if (event != CPUFREQ_ADJUST)
- goto out;
-
- max_freq = (
- policy->cpuinfo.max_freq *
- (100 - reduction_pctg(policy->cpu) * 20)
- ) / 100;
-
- cpufreq_verify_within_limits(policy, 0, max_freq);
-
- out:
- return 0;
-}
-
-static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
- .notifier_call = acpi_thermal_cpufreq_notifier,
-};
-
static int cpufreq_get_max_state(unsigned int cpu)
{
if (!cpu_has_cpufreq(cpu))
@@ -108,7 +83,10 @@ static int cpufreq_get_cur_state(unsigned int cpu)
static int cpufreq_set_cur_state(unsigned int cpu, int state)
{
- int i;
+ struct cpufreq_policy *policy;
+ struct acpi_processor *pr;
+ unsigned long max_freq;
+ int i, ret;
if (!cpu_has_cpufreq(cpu))
return 0;
@@ -121,33 +99,50 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
* frequency.
*/
for_each_online_cpu(i) {
- if (topology_physical_package_id(i) ==
+ if (topology_physical_package_id(i) !=
topology_physical_package_id(cpu))
- cpufreq_update_policy(i);
+ continue;
+
+ pr = per_cpu(processors, i);
+
+ policy = cpufreq_cpu_get(i);
+ if (!policy)
+ return -EINVAL;
+
+ max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
+
+ cpufreq_cpu_put(policy);
+
+ ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
+ if (ret) {
+ pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n",
+ pr->id, ret);
+ }
}
return 0;
}
-void acpi_thermal_cpufreq_init(void)
+void acpi_thermal_cpufreq_init(int cpu)
{
- int i;
-
- i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
- if (!i)
- acpi_thermal_cpufreq_is_init = 1;
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
+ ret);
+ return;
+ }
}
-void acpi_thermal_cpufreq_exit(void)
+void acpi_thermal_cpufreq_exit(int cpu)
{
- if (acpi_thermal_cpufreq_is_init)
- cpufreq_unregister_notifier
- (&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_thermal_cpufreq_is_init = 0;
+ dev_pm_qos_remove_request(&pr->thermal_req);
}
-
#else /* ! CONFIG_CPU_FREQ */
static int cpufreq_get_max_state(unsigned int cpu)
{
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 1194a4c78d55..ea21a6d20da7 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -4,6 +4,8 @@
#include <linux/kernel.h>
#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/pm_qos.h>
#include <linux/thermal.h>
#include <asm/acpi.h>
@@ -230,6 +232,8 @@ struct acpi_processor {
struct acpi_processor_limit limit;
struct thermal_cooling_device *cdev;
struct device *dev; /* Processor device. */
+ struct dev_pm_qos_request perflib_req;
+ struct dev_pm_qos_request thermal_req;
};
struct acpi_processor_errata {
@@ -296,16 +300,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
/* in processor_perflib.c */
#ifdef CONFIG_CPU_FREQ
-void acpi_processor_ppc_init(void);
-void acpi_processor_ppc_exit(void);
+extern bool acpi_processor_cpufreq_init;
+void acpi_processor_ppc_init(int cpu);
+void acpi_processor_ppc_exit(int cpu);
void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
#else
-static inline void acpi_processor_ppc_init(void)
+static inline void acpi_processor_ppc_init(int cpu)
{
return;
}
-static inline void acpi_processor_ppc_exit(void)
+static inline void acpi_processor_ppc_exit(int cpu)
{
return;
}
@@ -421,14 +426,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
int acpi_processor_get_limit_info(struct acpi_processor *pr);
extern const struct thermal_cooling_device_ops processor_cooling_ops;
#if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
-void acpi_thermal_cpufreq_init(void);
-void acpi_thermal_cpufreq_exit(void);
+void acpi_thermal_cpufreq_init(int cpu);
+void acpi_thermal_cpufreq_exit(int cpu);
#else
-static inline void acpi_thermal_cpufreq_init(void)
+static inline void acpi_thermal_cpufreq_init(int cpu)
{
return;
}
-static inline void acpi_thermal_cpufreq_exit(void)
+static inline void acpi_thermal_cpufreq_exit(int cpu)
{
return;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
2019-08-06 8:47 ` Viresh Kumar
@ 2019-08-09 2:33 ` Viresh Kumar
2019-08-10 12:13 ` [Devel] " Rafael J. Wysocki
0 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2019-08-09 2:33 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Len Brown, Zhang Rui, Robert Moore,
Erik Schmauss, Linux PM, Vincent Guittot, ACPI Devel Maling List,
Linux Kernel Mailing List,
open list:ACPI COMPONENT ARCHITECTURE (ACPICA)
On 06-08-19, 14:17, Viresh Kumar wrote:
> On 06-08-19, 10:01, Rafael J. Wysocki wrote:
> > Yes, it does, thanks!
> >
> > [No need to resend, I'll take it from this message.]
>
> Forgot to write CPU in caps in print messages, updated now.
And here is another version.
-------------------------8<-------------------------
From 6d2c1e8034562043a758524d6078e2dd1624195c Mon Sep 17 00:00:00 2001
Message-Id: <6d2c1e8034562043a758524d6078e2dd1624195c.1565317925.git.viresh.kumar@linaro.org>
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Mon, 15 Jul 2019 15:06:02 +0530
Subject: [PATCH] ACPI: cpufreq: Switch to QoS requests instead of cpufreq
notifier
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for acpi driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
- dev_pm_qos_update_request() can return 1 on success
drivers/acpi/processor_driver.c | 37 ++++++++++--
drivers/acpi/processor_perflib.c | 96 +++++++++++---------------------
drivers/acpi/processor_thermal.c | 81 +++++++++++++--------------
include/acpi/processor.h | 21 ++++---
4 files changed, 118 insertions(+), 117 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index aea8d674a33d..2c911fcaa4b4 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -284,6 +284,29 @@ static int acpi_processor_stop(struct device *dev)
return 0;
}
+bool acpi_processor_cpufreq_init;
+
+static int acpi_processor_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu = policy->cpu;
+
+ if (event == CPUFREQ_CREATE_POLICY) {
+ acpi_thermal_cpufreq_init(cpu);
+ acpi_processor_ppc_init(cpu);
+ } else if (event == CPUFREQ_REMOVE_POLICY) {
+ acpi_processor_ppc_exit(cpu);
+ acpi_thermal_cpufreq_exit(cpu);
+ }
+
+ return 0;
+}
+
+static struct notifier_block acpi_processor_notifier_block = {
+ .notifier_call = acpi_processor_notifier,
+};
+
/*
* We keep the driver loaded even when ACPI is not running.
* This is needed for the powernow-k8 driver, that works even without
@@ -310,8 +333,10 @@ static int __init acpi_processor_driver_init(void)
cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
NULL, acpi_soft_cpu_dead);
- acpi_thermal_cpufreq_init();
- acpi_processor_ppc_init();
+ if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER))
+ acpi_processor_cpufreq_init = true;
+
acpi_processor_throttling_init();
return 0;
err:
@@ -324,8 +349,12 @@ static void __exit acpi_processor_driver_exit(void)
if (acpi_disabled)
return;
- acpi_processor_ppc_exit();
- acpi_thermal_cpufreq_exit();
+ if (acpi_processor_cpufreq_init) {
+ cpufreq_unregister_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER);
+ acpi_processor_cpufreq_init = false;
+ }
+
cpuhp_remove_state_nocalls(hp_online);
cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
driver_unregister(&acpi_processor_driver);
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index ee87cb6f6e59..277fcbbe3be4 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
"limited by BIOS, this should help");
-#define PPC_REGISTERED 1
-#define PPC_IN_USE 2
-
-static int acpi_processor_ppc_status;
-
-static int acpi_processor_ppc_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct acpi_processor *pr;
- unsigned int ppc = 0;
-
- if (ignore_ppc < 0)
- ignore_ppc = 0;
-
- if (ignore_ppc)
- return 0;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- mutex_lock(&performance_mutex);
-
- pr = per_cpu(processors, policy->cpu);
- if (!pr || !pr->performance)
- goto out;
-
- ppc = (unsigned int)pr->performance_platform_limit;
-
- if (ppc >= pr->performance->state_count)
- goto out;
-
- cpufreq_verify_within_limits(policy, 0,
- pr->performance->states[ppc].
- core_frequency * 1000);
-
- out:
- mutex_unlock(&performance_mutex);
-
- return 0;
-}
-
-static struct notifier_block acpi_ppc_notifier_block = {
- .notifier_call = acpi_processor_ppc_notifier,
-};
+static bool acpi_processor_ppc_in_use;
static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
{
acpi_status status = 0;
unsigned long long ppc = 0;
-
+ int ret;
if (!pr)
return -EINVAL;
@@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
if (status != AE_NOT_FOUND)
- acpi_processor_ppc_status |= PPC_IN_USE;
+ acpi_processor_ppc_in_use = true;
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
@@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
pr->performance_platform_limit = (int)ppc;
+ if (ignore_ppc || ppc >= pr->performance->state_count)
+ return 0;
+
+ ret = dev_pm_qos_update_request(&pr->perflib_req,
+ pr->performance->states[ppc].core_frequency * 1000);
+ if (ret < 0) {
+ pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
+ pr->id, ret);
+ }
+
return 0;
}
@@ -184,23 +150,29 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
}
EXPORT_SYMBOL(acpi_processor_get_bios_limit);
-void acpi_processor_ppc_init(void)
+void acpi_processor_ppc_init(int cpu)
{
- if (!cpufreq_register_notifier
- (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
- acpi_processor_ppc_status |= PPC_REGISTERED;
- else
- printk(KERN_DEBUG
- "Warning: Processor Platform Limit not supported.\n");
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
+ ret);
+ return;
+ }
+
+ if (ignore_ppc < 0)
+ ignore_ppc = 0;
}
-void acpi_processor_ppc_exit(void)
+void acpi_processor_ppc_exit(int cpu)
{
- if (acpi_processor_ppc_status & PPC_REGISTERED)
- cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_processor_ppc_status &= ~PPC_REGISTERED;
+ dev_pm_qos_remove_request(&pr->perflib_req);
}
static int acpi_processor_get_performance_control(struct acpi_processor *pr)
@@ -477,7 +449,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
static int is_done = 0;
int result;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EBUSY;
if (!try_module_get(calling_module))
@@ -513,7 +485,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
* we can allow the cpufreq driver to be rmmod'ed. */
is_done = 1;
- if (!(acpi_processor_ppc_status & PPC_IN_USE))
+ if (!acpi_processor_ppc_in_use)
module_put(calling_module);
return 0;
@@ -742,7 +714,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
{
struct acpi_processor *pr;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EINVAL;
mutex_lock(&performance_mutex);
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 50fb0107375e..eb552ee086ad 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
#define CPUFREQ_THERMAL_MAX_STEP 3
static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
-static unsigned int acpi_thermal_cpufreq_is_init = 0;
#define reduction_pctg(cpu) \
per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
@@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
static int cpu_has_cpufreq(unsigned int cpu)
{
struct cpufreq_policy policy;
- if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
+ if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
return 0;
return 1;
}
-static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- unsigned long max_freq = 0;
-
- if (event != CPUFREQ_ADJUST)
- goto out;
-
- max_freq = (
- policy->cpuinfo.max_freq *
- (100 - reduction_pctg(policy->cpu) * 20)
- ) / 100;
-
- cpufreq_verify_within_limits(policy, 0, max_freq);
-
- out:
- return 0;
-}
-
-static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
- .notifier_call = acpi_thermal_cpufreq_notifier,
-};
-
static int cpufreq_get_max_state(unsigned int cpu)
{
if (!cpu_has_cpufreq(cpu))
@@ -108,7 +83,10 @@ static int cpufreq_get_cur_state(unsigned int cpu)
static int cpufreq_set_cur_state(unsigned int cpu, int state)
{
- int i;
+ struct cpufreq_policy *policy;
+ struct acpi_processor *pr;
+ unsigned long max_freq;
+ int i, ret;
if (!cpu_has_cpufreq(cpu))
return 0;
@@ -121,33 +99,50 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
* frequency.
*/
for_each_online_cpu(i) {
- if (topology_physical_package_id(i) ==
+ if (topology_physical_package_id(i) !=
topology_physical_package_id(cpu))
- cpufreq_update_policy(i);
+ continue;
+
+ pr = per_cpu(processors, i);
+
+ policy = cpufreq_cpu_get(i);
+ if (!policy)
+ return -EINVAL;
+
+ max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
+
+ cpufreq_cpu_put(policy);
+
+ ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
+ if (ret < 0) {
+ pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n",
+ pr->id, ret);
+ }
}
return 0;
}
-void acpi_thermal_cpufreq_init(void)
+void acpi_thermal_cpufreq_init(int cpu)
{
- int i;
-
- i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
- if (!i)
- acpi_thermal_cpufreq_is_init = 1;
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
+ ret);
+ return;
+ }
}
-void acpi_thermal_cpufreq_exit(void)
+void acpi_thermal_cpufreq_exit(int cpu)
{
- if (acpi_thermal_cpufreq_is_init)
- cpufreq_unregister_notifier
- (&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_thermal_cpufreq_is_init = 0;
+ dev_pm_qos_remove_request(&pr->thermal_req);
}
-
#else /* ! CONFIG_CPU_FREQ */
static int cpufreq_get_max_state(unsigned int cpu)
{
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 1194a4c78d55..ea21a6d20da7 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -4,6 +4,8 @@
#include <linux/kernel.h>
#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/pm_qos.h>
#include <linux/thermal.h>
#include <asm/acpi.h>
@@ -230,6 +232,8 @@ struct acpi_processor {
struct acpi_processor_limit limit;
struct thermal_cooling_device *cdev;
struct device *dev; /* Processor device. */
+ struct dev_pm_qos_request perflib_req;
+ struct dev_pm_qos_request thermal_req;
};
struct acpi_processor_errata {
@@ -296,16 +300,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
/* in processor_perflib.c */
#ifdef CONFIG_CPU_FREQ
-void acpi_processor_ppc_init(void);
-void acpi_processor_ppc_exit(void);
+extern bool acpi_processor_cpufreq_init;
+void acpi_processor_ppc_init(int cpu);
+void acpi_processor_ppc_exit(int cpu);
void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
#else
-static inline void acpi_processor_ppc_init(void)
+static inline void acpi_processor_ppc_init(int cpu)
{
return;
}
-static inline void acpi_processor_ppc_exit(void)
+static inline void acpi_processor_ppc_exit(int cpu)
{
return;
}
@@ -421,14 +426,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
int acpi_processor_get_limit_info(struct acpi_processor *pr);
extern const struct thermal_cooling_device_ops processor_cooling_ops;
#if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
-void acpi_thermal_cpufreq_init(void);
-void acpi_thermal_cpufreq_exit(void);
+void acpi_thermal_cpufreq_init(int cpu);
+void acpi_thermal_cpufreq_exit(int cpu);
#else
-static inline void acpi_thermal_cpufreq_init(void)
+static inline void acpi_thermal_cpufreq_init(int cpu)
{
return;
}
-static inline void acpi_thermal_cpufreq_exit(void)
+static inline void acpi_thermal_cpufreq_exit(int cpu)
{
return;
}
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
@ 2019-08-10 12:13 ` Rafael J. Wysocki
0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-10 12:13 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Len Brown, Zhang Rui,
Robert Moore, Erik Schmauss, Linux PM, Vincent Guittot,
ACPI Devel Maling List, Linux Kernel Mailing List,
open list:ACPI COMPONENT ARCHITECTURE (ACPICA)
On Fri, Aug 9, 2019 at 4:33 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 06-08-19, 14:17, Viresh Kumar wrote:
> > On 06-08-19, 10:01, Rafael J. Wysocki wrote:
> > > Yes, it does, thanks!
> > >
> > > [No need to resend, I'll take it from this message.]
> >
> > Forgot to write CPU in caps in print messages, updated now.
>
> And here is another version.
Queuing up for v5.4, thanks!
>
> -------------------------8<-------------------------
> From 6d2c1e8034562043a758524d6078e2dd1624195c Mon Sep 17 00:00:00 2001
> Message-Id: <6d2c1e8034562043a758524d6078e2dd1624195c.1565317925.git.viresh.kumar@linaro.org>
> From: Viresh Kumar <viresh.kumar@linaro.org>
> Date: Mon, 15 Jul 2019 15:06:02 +0530
> Subject: [PATCH] ACPI: cpufreq: Switch to QoS requests instead of cpufreq
> notifier
>
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for acpi driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> - dev_pm_qos_update_request() can return 1 on success
>
> drivers/acpi/processor_driver.c | 37 ++++++++++--
> drivers/acpi/processor_perflib.c | 96 +++++++++++---------------------
> drivers/acpi/processor_thermal.c | 81 +++++++++++++--------------
> include/acpi/processor.h | 21 ++++---
> 4 files changed, 118 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index aea8d674a33d..2c911fcaa4b4 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -284,6 +284,29 @@ static int acpi_processor_stop(struct device *dev)
> return 0;
> }
>
> +bool acpi_processor_cpufreq_init;
> +
> +static int acpi_processor_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct cpufreq_policy *policy = data;
> + int cpu = policy->cpu;
> +
> + if (event == CPUFREQ_CREATE_POLICY) {
> + acpi_thermal_cpufreq_init(cpu);
> + acpi_processor_ppc_init(cpu);
> + } else if (event == CPUFREQ_REMOVE_POLICY) {
> + acpi_processor_ppc_exit(cpu);
> + acpi_thermal_cpufreq_exit(cpu);
> + }
> +
> + return 0;
> +}
> +
> +static struct notifier_block acpi_processor_notifier_block = {
> + .notifier_call = acpi_processor_notifier,
> +};
> +
> /*
> * We keep the driver loaded even when ACPI is not running.
> * This is needed for the powernow-k8 driver, that works even without
> @@ -310,8 +333,10 @@ static int __init acpi_processor_driver_init(void)
> cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> NULL, acpi_soft_cpu_dead);
>
> - acpi_thermal_cpufreq_init();
> - acpi_processor_ppc_init();
> + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER))
> + acpi_processor_cpufreq_init = true;
> +
> acpi_processor_throttling_init();
> return 0;
> err:
> @@ -324,8 +349,12 @@ static void __exit acpi_processor_driver_exit(void)
> if (acpi_disabled)
> return;
>
> - acpi_processor_ppc_exit();
> - acpi_thermal_cpufreq_exit();
> + if (acpi_processor_cpufreq_init) {
> + cpufreq_unregister_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER);
> + acpi_processor_cpufreq_init = false;
> + }
> +
> cpuhp_remove_state_nocalls(hp_online);
> cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
> driver_unregister(&acpi_processor_driver);
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index ee87cb6f6e59..277fcbbe3be4 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
> MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> "limited by BIOS, this should help");
>
> -#define PPC_REGISTERED 1
> -#define PPC_IN_USE 2
> -
> -static int acpi_processor_ppc_status;
> -
> -static int acpi_processor_ppc_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - struct acpi_processor *pr;
> - unsigned int ppc = 0;
> -
> - if (ignore_ppc < 0)
> - ignore_ppc = 0;
> -
> - if (ignore_ppc)
> - return 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - return 0;
> -
> - mutex_lock(&performance_mutex);
> -
> - pr = per_cpu(processors, policy->cpu);
> - if (!pr || !pr->performance)
> - goto out;
> -
> - ppc = (unsigned int)pr->performance_platform_limit;
> -
> - if (ppc >= pr->performance->state_count)
> - goto out;
> -
> - cpufreq_verify_within_limits(policy, 0,
> - pr->performance->states[ppc].
> - core_frequency * 1000);
> -
> - out:
> - mutex_unlock(&performance_mutex);
> -
> - return 0;
> -}
> -
> -static struct notifier_block acpi_ppc_notifier_block = {
> - .notifier_call = acpi_processor_ppc_notifier,
> -};
> +static bool acpi_processor_ppc_in_use;
>
> static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> {
> acpi_status status = 0;
> unsigned long long ppc = 0;
> -
> + int ret;
>
> if (!pr)
> return -EINVAL;
> @@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
>
> if (status != AE_NOT_FOUND)
> - acpi_processor_ppc_status |= PPC_IN_USE;
> + acpi_processor_ppc_in_use = true;
>
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
> @@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
>
> pr->performance_platform_limit = (int)ppc;
>
> + if (ignore_ppc || ppc >= pr->performance->state_count)
> + return 0;
> +
> + ret = dev_pm_qos_update_request(&pr->perflib_req,
> + pr->performance->states[ppc].core_frequency * 1000);
> + if (ret < 0) {
> + pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
> + pr->id, ret);
> + }
> +
> return 0;
> }
>
> @@ -184,23 +150,29 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> }
> EXPORT_SYMBOL(acpi_processor_get_bios_limit);
>
> -void acpi_processor_ppc_init(void)
> +void acpi_processor_ppc_init(int cpu)
> {
> - if (!cpufreq_register_notifier
> - (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
> - acpi_processor_ppc_status |= PPC_REGISTERED;
> - else
> - printk(KERN_DEBUG
> - "Warning: Processor Platform Limit not supported.\n");
> + struct acpi_processor *pr = per_cpu(processors, cpu);
> + int ret;
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(cpu),
> + &pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
> + INT_MAX);
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
> + ret);
> + return;
> + }
> +
> + if (ignore_ppc < 0)
> + ignore_ppc = 0;
> }
>
> -void acpi_processor_ppc_exit(void)
> +void acpi_processor_ppc_exit(int cpu)
> {
> - if (acpi_processor_ppc_status & PPC_REGISTERED)
> - cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, cpu);
>
> - acpi_processor_ppc_status &= ~PPC_REGISTERED;
> + dev_pm_qos_remove_request(&pr->perflib_req);
> }
>
> static int acpi_processor_get_performance_control(struct acpi_processor *pr)
> @@ -477,7 +449,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> static int is_done = 0;
> int result;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EBUSY;
>
> if (!try_module_get(calling_module))
> @@ -513,7 +485,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> * we can allow the cpufreq driver to be rmmod'ed. */
> is_done = 1;
>
> - if (!(acpi_processor_ppc_status & PPC_IN_USE))
> + if (!acpi_processor_ppc_in_use)
> module_put(calling_module);
>
> return 0;
> @@ -742,7 +714,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
> {
> struct acpi_processor *pr;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EINVAL;
>
> mutex_lock(&performance_mutex);
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index 50fb0107375e..eb552ee086ad 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
> #define CPUFREQ_THERMAL_MAX_STEP 3
>
> static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
> -static unsigned int acpi_thermal_cpufreq_is_init = 0;
>
> #define reduction_pctg(cpu) \
> per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
> @@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
> static int cpu_has_cpufreq(unsigned int cpu)
> {
> struct cpufreq_policy policy;
> - if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
> + if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
> return 0;
> return 1;
> }
>
> -static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - unsigned long max_freq = 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - goto out;
> -
> - max_freq = (
> - policy->cpuinfo.max_freq *
> - (100 - reduction_pctg(policy->cpu) * 20)
> - ) / 100;
> -
> - cpufreq_verify_within_limits(policy, 0, max_freq);
> -
> - out:
> - return 0;
> -}
> -
> -static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
> - .notifier_call = acpi_thermal_cpufreq_notifier,
> -};
> -
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> if (!cpu_has_cpufreq(cpu))
> @@ -108,7 +83,10 @@ static int cpufreq_get_cur_state(unsigned int cpu)
>
> static int cpufreq_set_cur_state(unsigned int cpu, int state)
> {
> - int i;
> + struct cpufreq_policy *policy;
> + struct acpi_processor *pr;
> + unsigned long max_freq;
> + int i, ret;
>
> if (!cpu_has_cpufreq(cpu))
> return 0;
> @@ -121,33 +99,50 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
> * frequency.
> */
> for_each_online_cpu(i) {
> - if (topology_physical_package_id(i) ==
> + if (topology_physical_package_id(i) !=
> topology_physical_package_id(cpu))
> - cpufreq_update_policy(i);
> + continue;
> +
> + pr = per_cpu(processors, i);
> +
> + policy = cpufreq_cpu_get(i);
> + if (!policy)
> + return -EINVAL;
> +
> + max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
> +
> + cpufreq_cpu_put(policy);
> +
> + ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
> + if (ret < 0) {
> + pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n",
> + pr->id, ret);
> + }
> }
> return 0;
> }
>
> -void acpi_thermal_cpufreq_init(void)
> +void acpi_thermal_cpufreq_init(int cpu)
> {
> - int i;
> -
> - i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> - if (!i)
> - acpi_thermal_cpufreq_is_init = 1;
> + struct acpi_processor *pr = per_cpu(processors, cpu);
> + int ret;
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(cpu),
> + &pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
> + INT_MAX);
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
> + ret);
> + return;
> + }
> }
>
> -void acpi_thermal_cpufreq_exit(void)
> +void acpi_thermal_cpufreq_exit(int cpu)
> {
> - if (acpi_thermal_cpufreq_is_init)
> - cpufreq_unregister_notifier
> - (&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, cpu);
>
> - acpi_thermal_cpufreq_is_init = 0;
> + dev_pm_qos_remove_request(&pr->thermal_req);
> }
> -
> #else /* ! CONFIG_CPU_FREQ */
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 1194a4c78d55..ea21a6d20da7 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -4,6 +4,8 @@
>
> #include <linux/kernel.h>
> #include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/pm_qos.h>
> #include <linux/thermal.h>
> #include <asm/acpi.h>
>
> @@ -230,6 +232,8 @@ struct acpi_processor {
> struct acpi_processor_limit limit;
> struct thermal_cooling_device *cdev;
> struct device *dev; /* Processor device. */
> + struct dev_pm_qos_request perflib_req;
> + struct dev_pm_qos_request thermal_req;
> };
>
> struct acpi_processor_errata {
> @@ -296,16 +300,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
> /* in processor_perflib.c */
>
> #ifdef CONFIG_CPU_FREQ
> -void acpi_processor_ppc_init(void);
> -void acpi_processor_ppc_exit(void);
> +extern bool acpi_processor_cpufreq_init;
> +void acpi_processor_ppc_init(int cpu);
> +void acpi_processor_ppc_exit(int cpu);
> void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
> extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
> #else
> -static inline void acpi_processor_ppc_init(void)
> +static inline void acpi_processor_ppc_init(int cpu)
> {
> return;
> }
> -static inline void acpi_processor_ppc_exit(void)
> +static inline void acpi_processor_ppc_exit(int cpu)
> {
> return;
> }
> @@ -421,14 +426,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> int acpi_processor_get_limit_info(struct acpi_processor *pr);
> extern const struct thermal_cooling_device_ops processor_cooling_ops;
> #if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
> -void acpi_thermal_cpufreq_init(void);
> -void acpi_thermal_cpufreq_exit(void);
> +void acpi_thermal_cpufreq_init(int cpu);
> +void acpi_thermal_cpufreq_exit(int cpu);
> #else
> -static inline void acpi_thermal_cpufreq_init(void)
> +static inline void acpi_thermal_cpufreq_init(int cpu)
> {
> return;
> }
> -static inline void acpi_thermal_cpufreq_exit(void)
> +static inline void acpi_thermal_cpufreq_exit(int cpu)
> {
> return;
> }
^ permalink raw reply [flat|nested] 33+ messages in thread* Re: [Devel] [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
@ 2019-08-10 12:13 ` Rafael J. Wysocki
0 siblings, 0 replies; 33+ messages in thread
From: Rafael J. Wysocki @ 2019-08-10 12:13 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 16662 bytes --]
On Fri, Aug 9, 2019 at 4:33 AM Viresh Kumar <viresh.kumar(a)linaro.org> wrote:
>
> On 06-08-19, 14:17, Viresh Kumar wrote:
> > On 06-08-19, 10:01, Rafael J. Wysocki wrote:
> > > Yes, it does, thanks!
> > >
> > > [No need to resend, I'll take it from this message.]
> >
> > Forgot to write CPU in caps in print messages, updated now.
>
> And here is another version.
Queuing up for v5.4, thanks!
>
> -------------------------8<-------------------------
> From 6d2c1e8034562043a758524d6078e2dd1624195c Mon Sep 17 00:00:00 2001
> Message-Id: <6d2c1e8034562043a758524d6078e2dd1624195c.1565317925.git.viresh.kumar(a)linaro.org>
> From: Viresh Kumar <viresh.kumar(a)linaro.org>
> Date: Mon, 15 Jul 2019 15:06:02 +0530
> Subject: [PATCH] ACPI: cpufreq: Switch to QoS requests instead of cpufreq
> notifier
>
> The cpufreq core now takes the min/max frequency constraints via QoS
> requests and the CPUFREQ_ADJUST notifier shall get removed later on.
>
> Switch over to using the QoS request for maximum frequency constraint
> for acpi driver.
>
> Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org>
> ---
> - dev_pm_qos_update_request() can return 1 on success
>
> drivers/acpi/processor_driver.c | 37 ++++++++++--
> drivers/acpi/processor_perflib.c | 96 +++++++++++---------------------
> drivers/acpi/processor_thermal.c | 81 +++++++++++++--------------
> include/acpi/processor.h | 21 ++++---
> 4 files changed, 118 insertions(+), 117 deletions(-)
>
> diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
> index aea8d674a33d..2c911fcaa4b4 100644
> --- a/drivers/acpi/processor_driver.c
> +++ b/drivers/acpi/processor_driver.c
> @@ -284,6 +284,29 @@ static int acpi_processor_stop(struct device *dev)
> return 0;
> }
>
> +bool acpi_processor_cpufreq_init;
> +
> +static int acpi_processor_notifier(struct notifier_block *nb,
> + unsigned long event, void *data)
> +{
> + struct cpufreq_policy *policy = data;
> + int cpu = policy->cpu;
> +
> + if (event == CPUFREQ_CREATE_POLICY) {
> + acpi_thermal_cpufreq_init(cpu);
> + acpi_processor_ppc_init(cpu);
> + } else if (event == CPUFREQ_REMOVE_POLICY) {
> + acpi_processor_ppc_exit(cpu);
> + acpi_thermal_cpufreq_exit(cpu);
> + }
> +
> + return 0;
> +}
> +
> +static struct notifier_block acpi_processor_notifier_block = {
> + .notifier_call = acpi_processor_notifier,
> +};
> +
> /*
> * We keep the driver loaded even when ACPI is not running.
> * This is needed for the powernow-k8 driver, that works even without
> @@ -310,8 +333,10 @@ static int __init acpi_processor_driver_init(void)
> cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
> NULL, acpi_soft_cpu_dead);
>
> - acpi_thermal_cpufreq_init();
> - acpi_processor_ppc_init();
> + if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER))
> + acpi_processor_cpufreq_init = true;
> +
> acpi_processor_throttling_init();
> return 0;
> err:
> @@ -324,8 +349,12 @@ static void __exit acpi_processor_driver_exit(void)
> if (acpi_disabled)
> return;
>
> - acpi_processor_ppc_exit();
> - acpi_thermal_cpufreq_exit();
> + if (acpi_processor_cpufreq_init) {
> + cpufreq_unregister_notifier(&acpi_processor_notifier_block,
> + CPUFREQ_POLICY_NOTIFIER);
> + acpi_processor_cpufreq_init = false;
> + }
> +
> cpuhp_remove_state_nocalls(hp_online);
> cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
> driver_unregister(&acpi_processor_driver);
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index ee87cb6f6e59..277fcbbe3be4 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
> MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
> "limited by BIOS, this should help");
>
> -#define PPC_REGISTERED 1
> -#define PPC_IN_USE 2
> -
> -static int acpi_processor_ppc_status;
> -
> -static int acpi_processor_ppc_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - struct acpi_processor *pr;
> - unsigned int ppc = 0;
> -
> - if (ignore_ppc < 0)
> - ignore_ppc = 0;
> -
> - if (ignore_ppc)
> - return 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - return 0;
> -
> - mutex_lock(&performance_mutex);
> -
> - pr = per_cpu(processors, policy->cpu);
> - if (!pr || !pr->performance)
> - goto out;
> -
> - ppc = (unsigned int)pr->performance_platform_limit;
> -
> - if (ppc >= pr->performance->state_count)
> - goto out;
> -
> - cpufreq_verify_within_limits(policy, 0,
> - pr->performance->states[ppc].
> - core_frequency * 1000);
> -
> - out:
> - mutex_unlock(&performance_mutex);
> -
> - return 0;
> -}
> -
> -static struct notifier_block acpi_ppc_notifier_block = {
> - .notifier_call = acpi_processor_ppc_notifier,
> -};
> +static bool acpi_processor_ppc_in_use;
>
> static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> {
> acpi_status status = 0;
> unsigned long long ppc = 0;
> -
> + int ret;
>
> if (!pr)
> return -EINVAL;
> @@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
> status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
>
> if (status != AE_NOT_FOUND)
> - acpi_processor_ppc_status |= PPC_IN_USE;
> + acpi_processor_ppc_in_use = true;
>
> if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
> ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
> @@ -124,6 +80,16 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
>
> pr->performance_platform_limit = (int)ppc;
>
> + if (ignore_ppc || ppc >= pr->performance->state_count)
> + return 0;
> +
> + ret = dev_pm_qos_update_request(&pr->perflib_req,
> + pr->performance->states[ppc].core_frequency * 1000);
> + if (ret < 0) {
> + pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
> + pr->id, ret);
> + }
> +
> return 0;
> }
>
> @@ -184,23 +150,29 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
> }
> EXPORT_SYMBOL(acpi_processor_get_bios_limit);
>
> -void acpi_processor_ppc_init(void)
> +void acpi_processor_ppc_init(int cpu)
> {
> - if (!cpufreq_register_notifier
> - (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
> - acpi_processor_ppc_status |= PPC_REGISTERED;
> - else
> - printk(KERN_DEBUG
> - "Warning: Processor Platform Limit not supported.\n");
> + struct acpi_processor *pr = per_cpu(processors, cpu);
> + int ret;
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(cpu),
> + &pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
> + INT_MAX);
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
> + ret);
> + return;
> + }
> +
> + if (ignore_ppc < 0)
> + ignore_ppc = 0;
> }
>
> -void acpi_processor_ppc_exit(void)
> +void acpi_processor_ppc_exit(int cpu)
> {
> - if (acpi_processor_ppc_status & PPC_REGISTERED)
> - cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, cpu);
>
> - acpi_processor_ppc_status &= ~PPC_REGISTERED;
> + dev_pm_qos_remove_request(&pr->perflib_req);
> }
>
> static int acpi_processor_get_performance_control(struct acpi_processor *pr)
> @@ -477,7 +449,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> static int is_done = 0;
> int result;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EBUSY;
>
> if (!try_module_get(calling_module))
> @@ -513,7 +485,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
> * we can allow the cpufreq driver to be rmmod'ed. */
> is_done = 1;
>
> - if (!(acpi_processor_ppc_status & PPC_IN_USE))
> + if (!acpi_processor_ppc_in_use)
> module_put(calling_module);
>
> return 0;
> @@ -742,7 +714,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
> {
> struct acpi_processor *pr;
>
> - if (!(acpi_processor_ppc_status & PPC_REGISTERED))
> + if (!acpi_processor_cpufreq_init)
> return -EINVAL;
>
> mutex_lock(&performance_mutex);
> diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
> index 50fb0107375e..eb552ee086ad 100644
> --- a/drivers/acpi/processor_thermal.c
> +++ b/drivers/acpi/processor_thermal.c
> @@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
> #define CPUFREQ_THERMAL_MAX_STEP 3
>
> static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
> -static unsigned int acpi_thermal_cpufreq_is_init = 0;
>
> #define reduction_pctg(cpu) \
> per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
> @@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
> static int cpu_has_cpufreq(unsigned int cpu)
> {
> struct cpufreq_policy policy;
> - if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
> + if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
> return 0;
> return 1;
> }
>
> -static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
> - unsigned long event, void *data)
> -{
> - struct cpufreq_policy *policy = data;
> - unsigned long max_freq = 0;
> -
> - if (event != CPUFREQ_ADJUST)
> - goto out;
> -
> - max_freq = (
> - policy->cpuinfo.max_freq *
> - (100 - reduction_pctg(policy->cpu) * 20)
> - ) / 100;
> -
> - cpufreq_verify_within_limits(policy, 0, max_freq);
> -
> - out:
> - return 0;
> -}
> -
> -static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
> - .notifier_call = acpi_thermal_cpufreq_notifier,
> -};
> -
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> if (!cpu_has_cpufreq(cpu))
> @@ -108,7 +83,10 @@ static int cpufreq_get_cur_state(unsigned int cpu)
>
> static int cpufreq_set_cur_state(unsigned int cpu, int state)
> {
> - int i;
> + struct cpufreq_policy *policy;
> + struct acpi_processor *pr;
> + unsigned long max_freq;
> + int i, ret;
>
> if (!cpu_has_cpufreq(cpu))
> return 0;
> @@ -121,33 +99,50 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
> * frequency.
> */
> for_each_online_cpu(i) {
> - if (topology_physical_package_id(i) ==
> + if (topology_physical_package_id(i) !=
> topology_physical_package_id(cpu))
> - cpufreq_update_policy(i);
> + continue;
> +
> + pr = per_cpu(processors, i);
> +
> + policy = cpufreq_cpu_get(i);
> + if (!policy)
> + return -EINVAL;
> +
> + max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
> +
> + cpufreq_cpu_put(policy);
> +
> + ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
> + if (ret < 0) {
> + pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n",
> + pr->id, ret);
> + }
> }
> return 0;
> }
>
> -void acpi_thermal_cpufreq_init(void)
> +void acpi_thermal_cpufreq_init(int cpu)
> {
> - int i;
> -
> - i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> - if (!i)
> - acpi_thermal_cpufreq_is_init = 1;
> + struct acpi_processor *pr = per_cpu(processors, cpu);
> + int ret;
> +
> + ret = dev_pm_qos_add_request(get_cpu_device(cpu),
> + &pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
> + INT_MAX);
> + if (ret < 0) {
> + pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
> + ret);
> + return;
> + }
> }
>
> -void acpi_thermal_cpufreq_exit(void)
> +void acpi_thermal_cpufreq_exit(int cpu)
> {
> - if (acpi_thermal_cpufreq_is_init)
> - cpufreq_unregister_notifier
> - (&acpi_thermal_cpufreq_notifier_block,
> - CPUFREQ_POLICY_NOTIFIER);
> + struct acpi_processor *pr = per_cpu(processors, cpu);
>
> - acpi_thermal_cpufreq_is_init = 0;
> + dev_pm_qos_remove_request(&pr->thermal_req);
> }
> -
> #else /* ! CONFIG_CPU_FREQ */
> static int cpufreq_get_max_state(unsigned int cpu)
> {
> diff --git a/include/acpi/processor.h b/include/acpi/processor.h
> index 1194a4c78d55..ea21a6d20da7 100644
> --- a/include/acpi/processor.h
> +++ b/include/acpi/processor.h
> @@ -4,6 +4,8 @@
>
> #include <linux/kernel.h>
> #include <linux/cpu.h>
> +#include <linux/cpufreq.h>
> +#include <linux/pm_qos.h>
> #include <linux/thermal.h>
> #include <asm/acpi.h>
>
> @@ -230,6 +232,8 @@ struct acpi_processor {
> struct acpi_processor_limit limit;
> struct thermal_cooling_device *cdev;
> struct device *dev; /* Processor device. */
> + struct dev_pm_qos_request perflib_req;
> + struct dev_pm_qos_request thermal_req;
> };
>
> struct acpi_processor_errata {
> @@ -296,16 +300,17 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
> /* in processor_perflib.c */
>
> #ifdef CONFIG_CPU_FREQ
> -void acpi_processor_ppc_init(void);
> -void acpi_processor_ppc_exit(void);
> +extern bool acpi_processor_cpufreq_init;
> +void acpi_processor_ppc_init(int cpu);
> +void acpi_processor_ppc_exit(int cpu);
> void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
> extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
> #else
> -static inline void acpi_processor_ppc_init(void)
> +static inline void acpi_processor_ppc_init(int cpu)
> {
> return;
> }
> -static inline void acpi_processor_ppc_exit(void)
> +static inline void acpi_processor_ppc_exit(int cpu)
> {
> return;
> }
> @@ -421,14 +426,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
> int acpi_processor_get_limit_info(struct acpi_processor *pr);
> extern const struct thermal_cooling_device_ops processor_cooling_ops;
> #if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
> -void acpi_thermal_cpufreq_init(void);
> -void acpi_thermal_cpufreq_exit(void);
> +void acpi_thermal_cpufreq_init(int cpu);
> +void acpi_thermal_cpufreq_exit(int cpu);
> #else
> -static inline void acpi_thermal_cpufreq_init(void)
> +static inline void acpi_thermal_cpufreq_init(int cpu)
> {
> return;
> }
> -static inline void acpi_thermal_cpufreq_exit(void)
> +static inline void acpi_thermal_cpufreq_exit(int cpu)
> {
> return;
> }
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V3 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
2019-07-23 6:14 ` [PATCH V2 05/10] ACPI: cpufreq: " Viresh Kumar
2019-08-05 9:42 ` [Devel] " Rafael J. Wysocki
@ 2019-08-28 8:50 ` Viresh Kumar
1 sibling, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-08-28 8:50 UTC (permalink / raw)
To: Rafael Wysocki, Len Brown, Zhang Rui, Robert Moore, Erik Schmauss
Cc: Viresh Kumar, linux-pm, Vincent Guittot, linux-acpi, linux-kernel, devel
The cpufreq core now takes the min/max frequency constraints via QoS
requests and the CPUFREQ_ADJUST notifier shall get removed later on.
Switch over to using the QoS request for maximum frequency constraint
for acpi driver.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V3:
- Before updating qos request, make sure they are successfully added in
the first place using dev_pm_qos_request_active() helper.
- reset ignore_ppc from acpi_processor_ignore_ppc_init().
drivers/acpi/processor_driver.c | 39 ++++++++++--
drivers/acpi/processor_perflib.c | 100 ++++++++++++-------------------
drivers/acpi/processor_thermal.c | 84 +++++++++++++-------------
include/acpi/processor.h | 26 +++++---
4 files changed, 132 insertions(+), 117 deletions(-)
diff --git a/drivers/acpi/processor_driver.c b/drivers/acpi/processor_driver.c
index aea8d674a33d..08da9c29f1e9 100644
--- a/drivers/acpi/processor_driver.c
+++ b/drivers/acpi/processor_driver.c
@@ -284,6 +284,29 @@ static int acpi_processor_stop(struct device *dev)
return 0;
}
+bool acpi_processor_cpufreq_init;
+
+static int acpi_processor_notifier(struct notifier_block *nb,
+ unsigned long event, void *data)
+{
+ struct cpufreq_policy *policy = data;
+ int cpu = policy->cpu;
+
+ if (event == CPUFREQ_CREATE_POLICY) {
+ acpi_thermal_cpufreq_init(cpu);
+ acpi_processor_ppc_init(cpu);
+ } else if (event == CPUFREQ_REMOVE_POLICY) {
+ acpi_processor_ppc_exit(cpu);
+ acpi_thermal_cpufreq_exit(cpu);
+ }
+
+ return 0;
+}
+
+static struct notifier_block acpi_processor_notifier_block = {
+ .notifier_call = acpi_processor_notifier,
+};
+
/*
* We keep the driver loaded even when ACPI is not running.
* This is needed for the powernow-k8 driver, that works even without
@@ -310,8 +333,12 @@ static int __init acpi_processor_driver_init(void)
cpuhp_setup_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD, "acpi/cpu-drv:dead",
NULL, acpi_soft_cpu_dead);
- acpi_thermal_cpufreq_init();
- acpi_processor_ppc_init();
+ if (!cpufreq_register_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER)) {
+ acpi_processor_cpufreq_init = true;
+ acpi_processor_ignore_ppc_init();
+ }
+
acpi_processor_throttling_init();
return 0;
err:
@@ -324,8 +351,12 @@ static void __exit acpi_processor_driver_exit(void)
if (acpi_disabled)
return;
- acpi_processor_ppc_exit();
- acpi_thermal_cpufreq_exit();
+ if (acpi_processor_cpufreq_init) {
+ cpufreq_unregister_notifier(&acpi_processor_notifier_block,
+ CPUFREQ_POLICY_NOTIFIER);
+ acpi_processor_cpufreq_init = false;
+ }
+
cpuhp_remove_state_nocalls(hp_online);
cpuhp_remove_state_nocalls(CPUHP_ACPI_CPUDRV_DEAD);
driver_unregister(&acpi_processor_driver);
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index ee87cb6f6e59..2261713d1aec 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -50,57 +50,13 @@ module_param(ignore_ppc, int, 0644);
MODULE_PARM_DESC(ignore_ppc, "If the frequency of your machine gets wrongly" \
"limited by BIOS, this should help");
-#define PPC_REGISTERED 1
-#define PPC_IN_USE 2
-
-static int acpi_processor_ppc_status;
-
-static int acpi_processor_ppc_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- struct acpi_processor *pr;
- unsigned int ppc = 0;
-
- if (ignore_ppc < 0)
- ignore_ppc = 0;
-
- if (ignore_ppc)
- return 0;
-
- if (event != CPUFREQ_ADJUST)
- return 0;
-
- mutex_lock(&performance_mutex);
-
- pr = per_cpu(processors, policy->cpu);
- if (!pr || !pr->performance)
- goto out;
-
- ppc = (unsigned int)pr->performance_platform_limit;
-
- if (ppc >= pr->performance->state_count)
- goto out;
-
- cpufreq_verify_within_limits(policy, 0,
- pr->performance->states[ppc].
- core_frequency * 1000);
-
- out:
- mutex_unlock(&performance_mutex);
-
- return 0;
-}
-
-static struct notifier_block acpi_ppc_notifier_block = {
- .notifier_call = acpi_processor_ppc_notifier,
-};
+static bool acpi_processor_ppc_in_use;
static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
{
acpi_status status = 0;
unsigned long long ppc = 0;
-
+ int ret;
if (!pr)
return -EINVAL;
@@ -112,7 +68,7 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
status = acpi_evaluate_integer(pr->handle, "_PPC", NULL, &ppc);
if (status != AE_NOT_FOUND)
- acpi_processor_ppc_status |= PPC_IN_USE;
+ acpi_processor_ppc_in_use = true;
if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) {
ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PPC"));
@@ -124,6 +80,17 @@ static int acpi_processor_get_platform_limit(struct acpi_processor *pr)
pr->performance_platform_limit = (int)ppc;
+ if (ppc >= pr->performance->state_count ||
+ unlikely(!dev_pm_qos_request_active(&pr->perflib_req)))
+ return 0;
+
+ ret = dev_pm_qos_update_request(&pr->perflib_req,
+ pr->performance->states[ppc].core_frequency * 1000);
+ if (ret < 0) {
+ pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n",
+ pr->id, ret);
+ }
+
return 0;
}
@@ -184,23 +151,32 @@ int acpi_processor_get_bios_limit(int cpu, unsigned int *limit)
}
EXPORT_SYMBOL(acpi_processor_get_bios_limit);
-void acpi_processor_ppc_init(void)
+void acpi_processor_ignore_ppc_init(void)
{
- if (!cpufreq_register_notifier
- (&acpi_ppc_notifier_block, CPUFREQ_POLICY_NOTIFIER))
- acpi_processor_ppc_status |= PPC_REGISTERED;
- else
- printk(KERN_DEBUG
- "Warning: Processor Platform Limit not supported.\n");
+ if (ignore_ppc < 0)
+ ignore_ppc = 0;
+}
+
+void acpi_processor_ppc_init(int cpu)
+{
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->perflib_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
+ ret);
+ return;
+ }
}
-void acpi_processor_ppc_exit(void)
+void acpi_processor_ppc_exit(int cpu)
{
- if (acpi_processor_ppc_status & PPC_REGISTERED)
- cpufreq_unregister_notifier(&acpi_ppc_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_processor_ppc_status &= ~PPC_REGISTERED;
+ dev_pm_qos_remove_request(&pr->perflib_req);
}
static int acpi_processor_get_performance_control(struct acpi_processor *pr)
@@ -477,7 +453,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
static int is_done = 0;
int result;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EBUSY;
if (!try_module_get(calling_module))
@@ -513,7 +489,7 @@ int acpi_processor_notify_smm(struct module *calling_module)
* we can allow the cpufreq driver to be rmmod'ed. */
is_done = 1;
- if (!(acpi_processor_ppc_status & PPC_IN_USE))
+ if (!acpi_processor_ppc_in_use)
module_put(calling_module);
return 0;
@@ -742,7 +718,7 @@ acpi_processor_register_performance(struct acpi_processor_performance
{
struct acpi_processor *pr;
- if (!(acpi_processor_ppc_status & PPC_REGISTERED))
+ if (!acpi_processor_cpufreq_init)
return -EINVAL;
mutex_lock(&performance_mutex);
diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c
index 50fb0107375e..ec2638f1df4f 100644
--- a/drivers/acpi/processor_thermal.c
+++ b/drivers/acpi/processor_thermal.c
@@ -35,7 +35,6 @@ ACPI_MODULE_NAME("processor_thermal");
#define CPUFREQ_THERMAL_MAX_STEP 3
static DEFINE_PER_CPU(unsigned int, cpufreq_thermal_reduction_pctg);
-static unsigned int acpi_thermal_cpufreq_is_init = 0;
#define reduction_pctg(cpu) \
per_cpu(cpufreq_thermal_reduction_pctg, phys_package_first_cpu(cpu))
@@ -61,35 +60,11 @@ static int phys_package_first_cpu(int cpu)
static int cpu_has_cpufreq(unsigned int cpu)
{
struct cpufreq_policy policy;
- if (!acpi_thermal_cpufreq_is_init || cpufreq_get_policy(&policy, cpu))
+ if (!acpi_processor_cpufreq_init || cpufreq_get_policy(&policy, cpu))
return 0;
return 1;
}
-static int acpi_thermal_cpufreq_notifier(struct notifier_block *nb,
- unsigned long event, void *data)
-{
- struct cpufreq_policy *policy = data;
- unsigned long max_freq = 0;
-
- if (event != CPUFREQ_ADJUST)
- goto out;
-
- max_freq = (
- policy->cpuinfo.max_freq *
- (100 - reduction_pctg(policy->cpu) * 20)
- ) / 100;
-
- cpufreq_verify_within_limits(policy, 0, max_freq);
-
- out:
- return 0;
-}
-
-static struct notifier_block acpi_thermal_cpufreq_notifier_block = {
- .notifier_call = acpi_thermal_cpufreq_notifier,
-};
-
static int cpufreq_get_max_state(unsigned int cpu)
{
if (!cpu_has_cpufreq(cpu))
@@ -108,7 +83,10 @@ static int cpufreq_get_cur_state(unsigned int cpu)
static int cpufreq_set_cur_state(unsigned int cpu, int state)
{
- int i;
+ struct cpufreq_policy *policy;
+ struct acpi_processor *pr;
+ unsigned long max_freq;
+ int i, ret;
if (!cpu_has_cpufreq(cpu))
return 0;
@@ -121,33 +99,53 @@ static int cpufreq_set_cur_state(unsigned int cpu, int state)
* frequency.
*/
for_each_online_cpu(i) {
- if (topology_physical_package_id(i) ==
+ if (topology_physical_package_id(i) !=
topology_physical_package_id(cpu))
- cpufreq_update_policy(i);
+ continue;
+
+ pr = per_cpu(processors, i);
+
+ if (unlikely(!dev_pm_qos_request_active(&pr->thermal_req)))
+ continue;
+
+ policy = cpufreq_cpu_get(i);
+ if (!policy)
+ return -EINVAL;
+
+ max_freq = (policy->cpuinfo.max_freq * (100 - reduction_pctg(i) * 20)) / 100;
+
+ cpufreq_cpu_put(policy);
+
+ ret = dev_pm_qos_update_request(&pr->thermal_req, max_freq);
+ if (ret < 0) {
+ pr_warn("Failed to update thermal freq constraint: CPU%d (%d)\n",
+ pr->id, ret);
+ }
}
return 0;
}
-void acpi_thermal_cpufreq_init(void)
+void acpi_thermal_cpufreq_init(int cpu)
{
- int i;
-
- i = cpufreq_register_notifier(&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
- if (!i)
- acpi_thermal_cpufreq_is_init = 1;
+ struct acpi_processor *pr = per_cpu(processors, cpu);
+ int ret;
+
+ ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+ &pr->thermal_req, DEV_PM_QOS_MAX_FREQUENCY,
+ INT_MAX);
+ if (ret < 0) {
+ pr_err("Failed to add freq constraint for CPU%d (%d)\n", cpu,
+ ret);
+ return;
+ }
}
-void acpi_thermal_cpufreq_exit(void)
+void acpi_thermal_cpufreq_exit(int cpu)
{
- if (acpi_thermal_cpufreq_is_init)
- cpufreq_unregister_notifier
- (&acpi_thermal_cpufreq_notifier_block,
- CPUFREQ_POLICY_NOTIFIER);
+ struct acpi_processor *pr = per_cpu(processors, cpu);
- acpi_thermal_cpufreq_is_init = 0;
+ dev_pm_qos_remove_request(&pr->thermal_req);
}
-
#else /* ! CONFIG_CPU_FREQ */
static int cpufreq_get_max_state(unsigned int cpu)
{
diff --git a/include/acpi/processor.h b/include/acpi/processor.h
index 1194a4c78d55..f936033cb9e6 100644
--- a/include/acpi/processor.h
+++ b/include/acpi/processor.h
@@ -4,6 +4,8 @@
#include <linux/kernel.h>
#include <linux/cpu.h>
+#include <linux/cpufreq.h>
+#include <linux/pm_qos.h>
#include <linux/thermal.h>
#include <asm/acpi.h>
@@ -230,6 +232,8 @@ struct acpi_processor {
struct acpi_processor_limit limit;
struct thermal_cooling_device *cdev;
struct device *dev; /* Processor device. */
+ struct dev_pm_qos_request perflib_req;
+ struct dev_pm_qos_request thermal_req;
};
struct acpi_processor_errata {
@@ -296,16 +300,22 @@ static inline void acpi_processor_ffh_cstate_enter(struct acpi_processor_cx
/* in processor_perflib.c */
#ifdef CONFIG_CPU_FREQ
-void acpi_processor_ppc_init(void);
-void acpi_processor_ppc_exit(void);
+extern bool acpi_processor_cpufreq_init;
+void acpi_processor_ignore_ppc_init(void);
+void acpi_processor_ppc_init(int cpu);
+void acpi_processor_ppc_exit(int cpu);
void acpi_processor_ppc_has_changed(struct acpi_processor *pr, int event_flag);
extern int acpi_processor_get_bios_limit(int cpu, unsigned int *limit);
#else
-static inline void acpi_processor_ppc_init(void)
+static inline void acpi_processor_ignore_ppc_init(void)
{
return;
}
-static inline void acpi_processor_ppc_exit(void)
+static inline void acpi_processor_ppc_init(int cpu)
+{
+ return;
+}
+static inline void acpi_processor_ppc_exit(int cpu)
{
return;
}
@@ -421,14 +431,14 @@ static inline int acpi_processor_hotplug(struct acpi_processor *pr)
int acpi_processor_get_limit_info(struct acpi_processor *pr);
extern const struct thermal_cooling_device_ops processor_cooling_ops;
#if defined(CONFIG_ACPI_CPU_FREQ_PSS) & defined(CONFIG_CPU_FREQ)
-void acpi_thermal_cpufreq_init(void);
-void acpi_thermal_cpufreq_exit(void);
+void acpi_thermal_cpufreq_init(int cpu);
+void acpi_thermal_cpufreq_exit(int cpu);
#else
-static inline void acpi_thermal_cpufreq_init(void)
+static inline void acpi_thermal_cpufreq_init(int cpu)
{
return;
}
-static inline void acpi_thermal_cpufreq_exit(void)
+static inline void acpi_thermal_cpufreq_exit(int cpu)
{
return;
}
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 06/10] arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY
2019-07-23 6:14 ` Viresh Kumar
` (6 preceding siblings ...)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
2019-07-25 13:36 ` Greg Kroah-Hartman
-1 siblings, 1 reply; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Greg Kroah-Hartman, Rafael J. Wysocki
Cc: Viresh Kumar, linux-pm, Vincent Guittot, linux-kernel
CPUFREQ_NOTIFY is going to get removed soon, lets use
CPUFREQ_CREATE_POLICY instead of that here. CPUFREQ_CREATE_POLICY is
called only once (which is exactly what we want here) for each cpufreq
policy when it is first created.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/base/arch_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 63c1e76739f1..8cab1f5a8e0c 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -174,7 +174,7 @@ init_cpu_capacity_callback(struct notifier_block *nb,
if (!raw_capacity)
return 0;
- if (val != CPUFREQ_NOTIFY)
+ if (val != CPUFREQ_CREATE_POLICY)
return 0;
pr_debug("cpu_capacity: init cpu capacity for CPUs [%*pbl] (to_visit=%*pbl)\n",
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* Re: [PATCH V2 06/10] arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY
2019-07-23 6:14 ` [PATCH V2 06/10] arch_topology: Use CPUFREQ_CREATE_POLICY instead of CPUFREQ_NOTIFY Viresh Kumar
@ 2019-07-25 13:36 ` Greg Kroah-Hartman
0 siblings, 0 replies; 33+ messages in thread
From: Greg Kroah-Hartman @ 2019-07-25 13:36 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Rafael J. Wysocki, linux-pm, Vincent Guittot,
linux-kernel
On Tue, Jul 23, 2019 at 11:44:06AM +0530, Viresh Kumar wrote:
> CPUFREQ_NOTIFY is going to get removed soon, lets use
> CPUFREQ_CREATE_POLICY instead of that here. CPUFREQ_CREATE_POLICY is
> called only once (which is exactly what we want here) for each cpufreq
> policy when it is first created.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/base/arch_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH V2 07/10] video: sa1100fb: Remove cpufreq policy notifier
2019-07-23 6:14 ` Viresh Kumar
@ 2019-07-23 6:26 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Bartlomiej Zolnierkiewicz
Cc: Viresh Kumar, linux-pm, Vincent Guittot, dri-devel, linux-fbdev,
linux-kernel
The cpufreq policy notifier's CPUFREQ_ADJUST notification is going to
get removed soon.
The notifier callback sa1100fb_freq_policy() isn't doing anything apart
from printing a debug message on CPUFREQ_ADJUST notification. There is
no point in keeping an otherwise empty callback and registering the
notifier.
Remove it.
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/video/fbdev/sa1100fb.c | 27 ---------------------------
drivers/video/fbdev/sa1100fb.h | 1 -
2 files changed, 28 deletions(-)
diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c
index f7f8dee044b1..ae2bcfee338a 100644
--- a/drivers/video/fbdev/sa1100fb.c
+++ b/drivers/video/fbdev/sa1100fb.c
@@ -1005,31 +1005,6 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val,
}
return 0;
}
-
-static int
-sa1100fb_freq_policy(struct notifier_block *nb, unsigned long val,
- void *data)
-{
- struct sa1100fb_info *fbi = TO_INF(nb, freq_policy);
- struct cpufreq_policy *policy = data;
-
- switch (val) {
- case CPUFREQ_ADJUST:
- dev_dbg(fbi->dev, "min dma period: %d ps, "
- "new clock %d kHz\n", sa1100fb_min_dma_period(fbi),
- policy->max);
- /* todo: fill in min/max values */
- break;
- case CPUFREQ_NOTIFY:
- do {} while(0);
- /* todo: panic if min/max values aren't fulfilled
- * [can't really happen unless there's a bug in the
- * CPU policy verififcation process *
- */
- break;
- }
- return 0;
-}
#endif
#ifdef CONFIG_PM
@@ -1242,9 +1217,7 @@ static int sa1100fb_probe(struct platform_device *pdev)
#ifdef CONFIG_CPU_FREQ
fbi->freq_transition.notifier_call = sa1100fb_freq_transition;
- fbi->freq_policy.notifier_call = sa1100fb_freq_policy;
cpufreq_register_notifier(&fbi->freq_transition, CPUFREQ_TRANSITION_NOTIFIER);
- cpufreq_register_notifier(&fbi->freq_policy, CPUFREQ_POLICY_NOTIFIER);
#endif
/* This driver cannot be unloaded at the moment */
diff --git a/drivers/video/fbdev/sa1100fb.h b/drivers/video/fbdev/sa1100fb.h
index 7a1a9ca33cec..d0aa33b0b88a 100644
--- a/drivers/video/fbdev/sa1100fb.h
+++ b/drivers/video/fbdev/sa1100fb.h
@@ -64,7 +64,6 @@ struct sa1100fb_info {
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
- struct notifier_block freq_policy;
#endif
const struct sa1100fb_mach_info *inf;
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V2 07/10] video: sa1100fb: Remove cpufreq policy notifier
@ 2019-07-23 6:26 ` Viresh Kumar
0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:26 UTC (permalink / raw)
To: Rafael Wysocki, Bartlomiej Zolnierkiewicz
Cc: Viresh Kumar, linux-pm, Vincent Guittot, dri-devel, linux-fbdev,
linux-kernel
The cpufreq policy notifier's CPUFREQ_ADJUST notification is going to
get removed soon.
The notifier callback sa1100fb_freq_policy() isn't doing anything apart
from printing a debug message on CPUFREQ_ADJUST notification. There is
no point in keeping an otherwise empty callback and registering the
notifier.
Remove it.
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/video/fbdev/sa1100fb.c | 27 ---------------------------
drivers/video/fbdev/sa1100fb.h | 1 -
2 files changed, 28 deletions(-)
diff --git a/drivers/video/fbdev/sa1100fb.c b/drivers/video/fbdev/sa1100fb.c
index f7f8dee044b1..ae2bcfee338a 100644
--- a/drivers/video/fbdev/sa1100fb.c
+++ b/drivers/video/fbdev/sa1100fb.c
@@ -1005,31 +1005,6 @@ sa1100fb_freq_transition(struct notifier_block *nb, unsigned long val,
}
return 0;
}
-
-static int
-sa1100fb_freq_policy(struct notifier_block *nb, unsigned long val,
- void *data)
-{
- struct sa1100fb_info *fbi = TO_INF(nb, freq_policy);
- struct cpufreq_policy *policy = data;
-
- switch (val) {
- case CPUFREQ_ADJUST:
- dev_dbg(fbi->dev, "min dma period: %d ps, "
- "new clock %d kHz\n", sa1100fb_min_dma_period(fbi),
- policy->max);
- /* todo: fill in min/max values */
- break;
- case CPUFREQ_NOTIFY:
- do {} while(0);
- /* todo: panic if min/max values aren't fulfilled
- * [can't really happen unless there's a bug in the
- * CPU policy verififcation process *
- */
- break;
- }
- return 0;
-}
#endif
#ifdef CONFIG_PM
@@ -1242,9 +1217,7 @@ static int sa1100fb_probe(struct platform_device *pdev)
#ifdef CONFIG_CPU_FREQ
fbi->freq_transition.notifier_call = sa1100fb_freq_transition;
- fbi->freq_policy.notifier_call = sa1100fb_freq_policy;
cpufreq_register_notifier(&fbi->freq_transition, CPUFREQ_TRANSITION_NOTIFIER);
- cpufreq_register_notifier(&fbi->freq_policy, CPUFREQ_POLICY_NOTIFIER);
#endif
/* This driver cannot be unloaded at the moment */
diff --git a/drivers/video/fbdev/sa1100fb.h b/drivers/video/fbdev/sa1100fb.h
index 7a1a9ca33cec..d0aa33b0b88a 100644
--- a/drivers/video/fbdev/sa1100fb.h
+++ b/drivers/video/fbdev/sa1100fb.h
@@ -64,7 +64,6 @@ struct sa1100fb_info {
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
- struct notifier_block freq_policy;
#endif
const struct sa1100fb_mach_info *inf;
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 08/10] video: pxafb: Remove cpufreq policy notifier
2019-07-23 6:14 ` Viresh Kumar
@ 2019-07-23 6:26 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Bartlomiej Zolnierkiewicz
Cc: Viresh Kumar, linux-pm, Vincent Guittot, dri-devel, linux-fbdev,
linux-kernel
The cpufreq policy notifier's CPUFREQ_ADJUST notification is going to
get removed soon.
The notifier callback pxafb_freq_policy() isn't doing anything apart
from printing a debug message on CPUFREQ_ADJUST notification. There is
no point in keeping an otherwise empty callback and registering the
notifier.
Remove it.
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/video/fbdev/pxafb.c | 21 ---------------------
drivers/video/fbdev/pxafb.h | 1 -
2 files changed, 22 deletions(-)
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 4282cb117b92..f70c9f79622e 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -1678,24 +1678,6 @@ pxafb_freq_transition(struct notifier_block *nb, unsigned long val, void *data)
}
return 0;
}
-
-static int
-pxafb_freq_policy(struct notifier_block *nb, unsigned long val, void *data)
-{
- struct pxafb_info *fbi = TO_INF(nb, freq_policy);
- struct fb_var_screeninfo *var = &fbi->fb.var;
- struct cpufreq_policy *policy = data;
-
- switch (val) {
- case CPUFREQ_ADJUST:
- pr_debug("min dma period: %d ps, "
- "new clock %d kHz\n", pxafb_display_dma_period(var),
- policy->max);
- /* TODO: fill in min/max values */
- break;
- }
- return 0;
-}
#endif
#ifdef CONFIG_PM
@@ -2400,11 +2382,8 @@ static int pxafb_probe(struct platform_device *dev)
#ifdef CONFIG_CPU_FREQ
fbi->freq_transition.notifier_call = pxafb_freq_transition;
- fbi->freq_policy.notifier_call = pxafb_freq_policy;
cpufreq_register_notifier(&fbi->freq_transition,
CPUFREQ_TRANSITION_NOTIFIER);
- cpufreq_register_notifier(&fbi->freq_policy,
- CPUFREQ_POLICY_NOTIFIER);
#endif
/*
diff --git a/drivers/video/fbdev/pxafb.h b/drivers/video/fbdev/pxafb.h
index b641289c8a99..86b1e9ab1a38 100644
--- a/drivers/video/fbdev/pxafb.h
+++ b/drivers/video/fbdev/pxafb.h
@@ -162,7 +162,6 @@ struct pxafb_info {
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
- struct notifier_block freq_policy;
#endif
struct regulator *lcd_supply;
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V2 08/10] video: pxafb: Remove cpufreq policy notifier
@ 2019-07-23 6:26 ` Viresh Kumar
0 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:26 UTC (permalink / raw)
To: Rafael Wysocki, Bartlomiej Zolnierkiewicz
Cc: Viresh Kumar, linux-pm, Vincent Guittot, dri-devel, linux-fbdev,
linux-kernel
The cpufreq policy notifier's CPUFREQ_ADJUST notification is going to
get removed soon.
The notifier callback pxafb_freq_policy() isn't doing anything apart
from printing a debug message on CPUFREQ_ADJUST notification. There is
no point in keeping an otherwise empty callback and registering the
notifier.
Remove it.
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/video/fbdev/pxafb.c | 21 ---------------------
drivers/video/fbdev/pxafb.h | 1 -
2 files changed, 22 deletions(-)
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 4282cb117b92..f70c9f79622e 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -1678,24 +1678,6 @@ pxafb_freq_transition(struct notifier_block *nb, unsigned long val, void *data)
}
return 0;
}
-
-static int
-pxafb_freq_policy(struct notifier_block *nb, unsigned long val, void *data)
-{
- struct pxafb_info *fbi = TO_INF(nb, freq_policy);
- struct fb_var_screeninfo *var = &fbi->fb.var;
- struct cpufreq_policy *policy = data;
-
- switch (val) {
- case CPUFREQ_ADJUST:
- pr_debug("min dma period: %d ps, "
- "new clock %d kHz\n", pxafb_display_dma_period(var),
- policy->max);
- /* TODO: fill in min/max values */
- break;
- }
- return 0;
-}
#endif
#ifdef CONFIG_PM
@@ -2400,11 +2382,8 @@ static int pxafb_probe(struct platform_device *dev)
#ifdef CONFIG_CPU_FREQ
fbi->freq_transition.notifier_call = pxafb_freq_transition;
- fbi->freq_policy.notifier_call = pxafb_freq_policy;
cpufreq_register_notifier(&fbi->freq_transition,
CPUFREQ_TRANSITION_NOTIFIER);
- cpufreq_register_notifier(&fbi->freq_policy,
- CPUFREQ_POLICY_NOTIFIER);
#endif
/*
diff --git a/drivers/video/fbdev/pxafb.h b/drivers/video/fbdev/pxafb.h
index b641289c8a99..86b1e9ab1a38 100644
--- a/drivers/video/fbdev/pxafb.h
+++ b/drivers/video/fbdev/pxafb.h
@@ -162,7 +162,6 @@ struct pxafb_info {
#ifdef CONFIG_CPU_FREQ
struct notifier_block freq_transition;
- struct notifier_block freq_policy;
#endif
struct regulator *lcd_supply;
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH V2 09/10] cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier events
2019-07-23 6:14 ` Viresh Kumar
` (9 preceding siblings ...)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar; +Cc: linux-pm, Vincent Guittot, linux-kernel
No driver makes reference to these events now, remove them and the code
related to them.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/cpufreq.c | 36 +++++++-----------------------------
include/linux/cpufreq.h | 6 ++----
2 files changed, 9 insertions(+), 33 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index c13dcb59b30c..e0ee23895497 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2360,15 +2360,13 @@ EXPORT_SYMBOL(cpufreq_get_policy);
* @policy: Policy object to modify.
* @new_policy: New policy data.
*
- * Pass @new_policy to the cpufreq driver's ->verify() callback, run the
- * installed policy notifiers for it with the CPUFREQ_ADJUST value, pass it to
- * the driver's ->verify() callback again and run the notifiers for it again
- * with the CPUFREQ_NOTIFY value. Next, copy the min and max parameters
- * of @new_policy to @policy and either invoke the driver's ->setpolicy()
- * callback (if present) or carry out a governor update for @policy. That is,
- * run the current governor's ->limits() callback (if the governor field in
- * @new_policy points to the same object as the one in @policy) or replace the
- * governor for @policy with the new one stored in @new_policy.
+ * Pass @new_policy to the cpufreq driver's ->verify() callback. Next, copy the
+ * min and max parameters of @new_policy to @policy and either invoke the
+ * driver's ->setpolicy() callback (if present) or carry out a governor update
+ * for @policy. That is, run the current governor's ->limits() callback (if the
+ * governor field in @new_policy points to the same object as the one in
+ * @policy) or replace the governor for @policy with the new one stored in
+ * @new_policy.
*
* The cpuinfo part of @policy is not updated by this function.
*/
@@ -2396,26 +2394,6 @@ int cpufreq_set_policy(struct cpufreq_policy *policy,
if (ret)
return ret;
- /*
- * The notifier-chain shall be removed once all the users of
- * CPUFREQ_ADJUST are moved to use the QoS framework.
- */
- /* adjust if necessary - all reasons */
- blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
- CPUFREQ_ADJUST, new_policy);
-
- /*
- * verify the cpu speed can be set within this limit, which might be
- * different to the first one
- */
- ret = cpufreq_driver->verify(new_policy);
- if (ret)
- return ret;
-
- /* notification of the new policy */
- blocking_notifier_call_chain(&cpufreq_policy_notifier_list,
- CPUFREQ_NOTIFY, new_policy);
-
policy->min = new_policy->min;
policy->max = new_policy->max;
trace_cpu_frequency_limits(policy);
diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index afc10384a681..c57e88e85c41 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -456,10 +456,8 @@ static inline void cpufreq_resume(void) {}
#define CPUFREQ_POSTCHANGE (1)
/* Policy Notifiers */
-#define CPUFREQ_ADJUST (0)
-#define CPUFREQ_NOTIFY (1)
-#define CPUFREQ_CREATE_POLICY (2)
-#define CPUFREQ_REMOVE_POLICY (3)
+#define CPUFREQ_CREATE_POLICY (0)
+#define CPUFREQ_REMOVE_POLICY (1)
#ifdef CONFIG_CPU_FREQ
int cpufreq_register_notifier(struct notifier_block *nb, unsigned int list);
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread* [PATCH V2 10/10] Documentation: cpufreq: Update policy notifier documentation
2019-07-23 6:14 ` Viresh Kumar
` (10 preceding siblings ...)
(?)
@ 2019-07-23 6:14 ` Viresh Kumar
-1 siblings, 0 replies; 33+ messages in thread
From: Viresh Kumar @ 2019-07-23 6:14 UTC (permalink / raw)
To: Rafael Wysocki, Viresh Kumar, Jonathan Corbet
Cc: linux-pm, Vincent Guittot, linux-doc, linux-kernel
Update documentation with the recent policy notifier updates.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
Documentation/cpu-freq/core.txt | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/Documentation/cpu-freq/core.txt b/Documentation/cpu-freq/core.txt
index 55193e680250..ed577d9c154b 100644
--- a/Documentation/cpu-freq/core.txt
+++ b/Documentation/cpu-freq/core.txt
@@ -57,19 +57,11 @@ transition notifiers.
2.1 CPUFreq policy notifiers
----------------------------
-These are notified when a new policy is intended to be set. Each
-CPUFreq policy notifier is called twice for a policy transition:
+These are notified when a new policy is created or removed.
-1.) During CPUFREQ_ADJUST all CPUFreq notifiers may change the limit if
- they see a need for this - may it be thermal considerations or
- hardware limitations.
-
-2.) And during CPUFREQ_NOTIFY all notifiers are informed of the new policy
- - if two hardware drivers failed to agree on a new policy before this
- stage, the incompatible hardware shall be shut down, and the user
- informed of this.
-
-The phase is specified in the second argument to the notifier.
+The phase is specified in the second argument to the notifier. The phase is
+CPUFREQ_CREATE_POLICY when the policy is first created and it is
+CPUFREQ_REMOVE_POLICY when the policy is removed.
The third argument, a void *pointer, points to a struct cpufreq_policy
consisting of several values, including min, max (the lower and upper
--
2.21.0.rc0.269.g1a574e7a288b
^ permalink raw reply related [flat|nested] 33+ messages in thread