All of lore.kernel.org
 help / color / mirror / Atom feed
* [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-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

* [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

* [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

* [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 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 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

* [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 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: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

* 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

* 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 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 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

* 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

* 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

* [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

end of thread, other threads:[~2019-08-28  8:50 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-23  6:14 [PATCH V2 00/10] cpufreq: Migrate users of policy notifiers to QoS requests Viresh Kumar
2019-07-23  6:26 ` Viresh Kumar
2019-07-23  6:14 ` Viresh Kumar
2019-07-23  6:14 ` [PATCH V2 01/10] cpufreq: Add policy create/remove notifiers Viresh Kumar
2019-07-23  6:14 ` [PATCH V2 02/10] thermal: cpu_cooling: Switch to QoS requests instead of cpufreq notifier Viresh Kumar
2019-07-23  6:14 ` [PATCH V2 03/10] powerpc: macintosh: " Viresh Kumar
2019-07-23  6:14   ` Viresh Kumar
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-19  2:26       ` Viresh Kumar
2019-08-19  6:42         ` Rafael J. Wysocki
2019-08-19  6:47           ` Viresh Kumar
2019-08-10 12:19     ` Rafael J. Wysocki
2019-07-23  6:14 ` [PATCH V2 05/10] ACPI: cpufreq: " Viresh Kumar
2019-08-05  9:42   ` Rafael J. Wysocki
2019-08-05  9:42     ` [Devel] " Rafael J. Wysocki
2019-08-06  4:39     ` Viresh Kumar
2019-08-06  8:01       ` Rafael J. Wysocki
2019-08-06  8:01         ` [Devel] " Rafael J. Wysocki
2019-08-06  8:47         ` Viresh Kumar
2019-08-09  2:33           ` Viresh Kumar
2019-08-10 12:13             ` Rafael J. Wysocki
2019-08-10 12:13               ` [Devel] " Rafael J. Wysocki
2019-08-28  8:50   ` [PATCH V3 " Viresh Kumar
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
2019-07-23  6:14 ` [PATCH V2 07/10] video: sa1100fb: Remove cpufreq policy notifier Viresh Kumar
2019-07-23  6:26   ` Viresh Kumar
2019-07-23  6:14 ` [PATCH V2 08/10] video: pxafb: " Viresh Kumar
2019-07-23  6:26   ` Viresh Kumar
2019-07-23  6:14 ` [PATCH V2 09/10] cpufreq: Remove CPUFREQ_ADJUST and CPUFREQ_NOTIFY policy notifier events Viresh Kumar
2019-07-23  6:14 ` [PATCH V2 10/10] Documentation: cpufreq: Update policy notifier documentation Viresh Kumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.