linux-acpi.vger.kernel.org archive mirror
 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
  2019-07-23  6:14 ` [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier Viresh Kumar
  0 siblings, 1 reply; 9+ 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] 9+ messages in thread

* [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
  2019-07-23  6:14 [PATCH V2 00/10] cpufreq: Migrate users of policy notifiers to QoS requests Viresh Kumar
@ 2019-07-23  6:14 ` Viresh Kumar
  2019-08-05  9:42   ` Rafael J. Wysocki
  2019-08-28  8:50   ` [PATCH V3 " Viresh Kumar
  0 siblings, 2 replies; 9+ 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] 9+ messages in thread

* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
  2019-07-23  6:14 ` [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier Viresh Kumar
@ 2019-08-05  9:42   ` Rafael J. Wysocki
  2019-08-06  4:39     ` Viresh Kumar
  2019-08-28  8:50   ` [PATCH V3 " Viresh Kumar
  1 sibling, 1 reply; 9+ 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] 9+ 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
@ 2019-08-06  4:39     ` Viresh Kumar
  2019-08-06  8:01       ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ 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] 9+ messages in thread

* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
  2019-08-06  4:39     ` Viresh Kumar
@ 2019-08-06  8:01       ` Rafael J. Wysocki
  2019-08-06  8:47         ` Viresh Kumar
  0 siblings, 1 reply; 9+ 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] 9+ 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
@ 2019-08-06  8:47         ` Viresh Kumar
  2019-08-09  2:33           ` Viresh Kumar
  0 siblings, 1 reply; 9+ 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] 9+ 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             ` Rafael J. Wysocki
  0 siblings, 1 reply; 9+ 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] 9+ messages in thread

* Re: [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier
  2019-08-09  2:33           ` Viresh Kumar
@ 2019-08-10 12:13             ` Rafael J. Wysocki
  0 siblings, 0 replies; 9+ 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] 9+ 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: Switch to QoS requests instead of cpufreq notifier Viresh Kumar
  2019-08-05  9:42   ` Rafael J. Wysocki
@ 2019-08-28  8:50   ` Viresh Kumar
  1 sibling, 0 replies; 9+ 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] 9+ messages in thread

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

Thread overview: 9+ 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:14 ` [PATCH V2 05/10] ACPI: cpufreq: Switch to QoS requests instead of cpufreq notifier Viresh Kumar
2019-08-05  9:42   ` Rafael J. Wysocki
2019-08-06  4:39     ` Viresh Kumar
2019-08-06  8:01       ` 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-28  8:50   ` [PATCH V3 " Viresh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).