* [PATCH 1/3] cpufreq: Allow passing NULL as the argument for unregistering a driver
2022-03-25 5:42 [PATCH 0/3] Improve usability for amd-pstate Mario Limonciello
@ 2022-03-25 5:42 ` Mario Limonciello
2022-03-27 13:23 ` Huang Rui
2022-03-25 5:42 ` [PATCH 2/3] cpufreq: amd-pstate: Allow replacing existing cpufreq drivers when loaded Mario Limonciello
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2022-03-25 5:42 UTC (permalink / raw)
To: Huang Rui, Rafael J . Wysocki, Viresh Kumar
Cc: open list:AMD PSTATE DRIVER, open list, Mario Limonciello
Currently the arguments passed to `cpufreq_unregister_driver` are matched
against the currently registered driver.
This means that the only way for a driver to be unregistered is if it's
module is unloaded. Loosen that restriction to allow other kernel modules
to unregister a registered driver.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/cpufreq/cpufreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 80f535cc8a75..4711c17a89bb 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -2885,10 +2885,10 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver)
{
unsigned long flags;
- if (!cpufreq_driver || (driver != cpufreq_driver))
+ if (!cpufreq_driver)
return -EINVAL;
- pr_debug("unregistering driver %s\n", driver->name);
+ pr_debug("unregistering driver %s\n", cpufreq_driver->name);
/* Protect against concurrent cpu hotplug */
cpus_read_lock();
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] cpufreq: Allow passing NULL as the argument for unregistering a driver
2022-03-25 5:42 ` [PATCH 1/3] cpufreq: Allow passing NULL as the argument for unregistering a driver Mario Limonciello
@ 2022-03-27 13:23 ` Huang Rui
0 siblings, 0 replies; 8+ messages in thread
From: Huang Rui @ 2022-03-27 13:23 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Rafael J . Wysocki, Viresh Kumar, open list:AMD PSTATE DRIVER, open list
On Fri, Mar 25, 2022 at 01:42:26PM +0800, Limonciello, Mario wrote:
> Currently the arguments passed to `cpufreq_unregister_driver` are matched
> against the currently registered driver.
>
> This means that the only way for a driver to be unregistered is if it's
> module is unloaded. Loosen that restriction to allow other kernel modules
> to unregister a registered driver.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/cpufreq/cpufreq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 80f535cc8a75..4711c17a89bb 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -2885,10 +2885,10 @@ int cpufreq_unregister_driver(struct cpufreq_driver *driver)
> {
> unsigned long flags;
>
> - if (!cpufreq_driver || (driver != cpufreq_driver))
> + if (!cpufreq_driver)
> return -EINVAL;
>
> - pr_debug("unregistering driver %s\n", driver->name);
> + pr_debug("unregistering driver %s\n", cpufreq_driver->name);
>
This is amd-pstate only use case, I suggest we only modify the amd-pstate
driver, and don't impact generic cpufreq. Actually, only acpi-cpufreq could
be registered earlier than amd-pstate. So how about exposing the
acpi_cpufreq_exit(), and call this to free the acpi_cpufreq_driver in
amd-pstate directly?
Thanks,
Ray
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] cpufreq: amd-pstate: Allow replacing existing cpufreq drivers when loaded
2022-03-25 5:42 [PATCH 0/3] Improve usability for amd-pstate Mario Limonciello
2022-03-25 5:42 ` [PATCH 1/3] cpufreq: Allow passing NULL as the argument for unregistering a driver Mario Limonciello
@ 2022-03-25 5:42 ` Mario Limonciello
2022-03-25 5:42 ` [PATCH 3/3] cpufreq: amd-pstate: Add a module device table Mario Limonciello
2022-03-27 11:56 ` [PATCH 0/3] Improve usability for amd-pstate Huang Rui
3 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2022-03-25 5:42 UTC (permalink / raw)
To: Huang Rui, Rafael J . Wysocki, Viresh Kumar
Cc: open list:AMD PSTATE DRIVER, open list, Mario Limonciello
`amd-pstate` can be compiled as a module. This however can be a
deficiency because `acpi-cpufreq` will be loaded earlier when compiled
into the kernel meaning `amd-pstate` doesn't get a chance.
`acpi-cpufreq` is also unable to be unloaded in this circumstance.
To better improve the usability of `amd-pstate` when compiled as a module,
add an optional module parameter that will force it to replace other
cpufreq drivers at startup.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/cpufreq/amd-pstate.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 9ce75ed11f8e..31a04e818195 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -63,6 +63,11 @@ module_param(shared_mem, bool, 0444);
MODULE_PARM_DESC(shared_mem,
"enable amd-pstate on processors with shared memory solution (false = disabled (default), true = enabled)");
+static bool replace = false;
+module_param(replace, bool, 0444);
+MODULE_PARM_DESC(replace,
+ "replace other cpufreq drivers upon init if necessary");
+
static struct cpufreq_driver amd_pstate_driver;
/**
@@ -598,9 +603,11 @@ static int __init amd_pstate_init(void)
return -ENODEV;
}
- /* don't keep reloading if cpufreq_driver exists */
- if (cpufreq_get_current_driver())
- return -EEXIST;
+ if (cpufreq_get_current_driver()) {
+ ret = replace ? cpufreq_unregister_driver(NULL) : -EEXIST;
+ if (ret)
+ return ret;
+ }
/* capability check */
if (boot_cpu_has(X86_FEATURE_CPPC)) {
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] cpufreq: amd-pstate: Add a module device table
2022-03-25 5:42 [PATCH 0/3] Improve usability for amd-pstate Mario Limonciello
2022-03-25 5:42 ` [PATCH 1/3] cpufreq: Allow passing NULL as the argument for unregistering a driver Mario Limonciello
2022-03-25 5:42 ` [PATCH 2/3] cpufreq: amd-pstate: Allow replacing existing cpufreq drivers when loaded Mario Limonciello
@ 2022-03-25 5:42 ` Mario Limonciello
2022-03-27 13:27 ` Huang Rui
2022-03-27 11:56 ` [PATCH 0/3] Improve usability for amd-pstate Huang Rui
3 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2022-03-25 5:42 UTC (permalink / raw)
To: Huang Rui, Rafael J . Wysocki, Viresh Kumar
Cc: open list:AMD PSTATE DRIVER, open list, Mario Limonciello
`amd-pstate` currently only loads automatically if compiled into the
kernel. To improve the usability, add a module device table that
will load when AMD CPUs that support CPPC are detected.
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/cpufreq/amd-pstate.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index 31a04e818195..44490292fa72 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -644,6 +644,12 @@ static void __exit amd_pstate_exit(void)
amd_pstate_enable(false);
}
+static const struct x86_cpu_id __maybe_unused amd_pstate_ids[] = {
+ X86_MATCH_VENDOR_FEATURE(AMD, X86_FEATURE_CPPC, NULL),
+ {}
+};
+MODULE_DEVICE_TABLE(x86cpu, amd_pstate_ids);
+
module_init(amd_pstate_init);
module_exit(amd_pstate_exit);
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] cpufreq: amd-pstate: Add a module device table
2022-03-25 5:42 ` [PATCH 3/3] cpufreq: amd-pstate: Add a module device table Mario Limonciello
@ 2022-03-27 13:27 ` Huang Rui
0 siblings, 0 replies; 8+ messages in thread
From: Huang Rui @ 2022-03-27 13:27 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Rafael J . Wysocki, Viresh Kumar, open list:AMD PSTATE DRIVER, open list
On Fri, Mar 25, 2022 at 01:42:28PM +0800, Limonciello, Mario wrote:
> `amd-pstate` currently only loads automatically if compiled into the
> kernel. To improve the usability, add a module device table that
> will load when AMD CPUs that support CPPC are detected.
>
There is one AMD semi-custom processor which has the CPPC feature flag, but
the SBIOS doesn't support _CPC objects.
Thanks,
Ray
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> drivers/cpufreq/amd-pstate.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 31a04e818195..44490292fa72 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -644,6 +644,12 @@ static void __exit amd_pstate_exit(void)
> amd_pstate_enable(false);
> }
>
> +static const struct x86_cpu_id __maybe_unused amd_pstate_ids[] = {
> + X86_MATCH_VENDOR_FEATURE(AMD, X86_FEATURE_CPPC, NULL),
> + {}
> +};
> +MODULE_DEVICE_TABLE(x86cpu, amd_pstate_ids);
> +
> module_init(amd_pstate_init);
> module_exit(amd_pstate_exit);
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] Improve usability for amd-pstate
2022-03-25 5:42 [PATCH 0/3] Improve usability for amd-pstate Mario Limonciello
` (2 preceding siblings ...)
2022-03-25 5:42 ` [PATCH 3/3] cpufreq: amd-pstate: Add a module device table Mario Limonciello
@ 2022-03-27 11:56 ` Huang Rui
2022-03-28 22:14 ` Limonciello, Mario
3 siblings, 1 reply; 8+ messages in thread
From: Huang Rui @ 2022-03-27 11:56 UTC (permalink / raw)
To: Limonciello, Mario
Cc: Rafael J . Wysocki, Viresh Kumar, open list:AMD PSTATE DRIVER, open list
On Fri, Mar 25, 2022 at 01:42:25PM +0800, Limonciello, Mario wrote:
> There has recently been some news coverage about `amd-pstate` being in
> 5.17, but this news also mentioned that it's a bit difficult to use.
>
> You need to either block init calls, or compile the module into the kernel
> to force it to take precedence over acpi-cpufreq.
>
> This series aims to improve the usability of amd-pstate so that distros
> can compile as a module, but users can still use it (relatively) easily.
>
> A new module parameter is included that will force amd-pstate to take
> precedence and a module table to let it load automatically on such systems.
>
> With the patches in this series a user can make a file
> /etc/modprobe.d/amd-pstate.conf:
>
> options amd-pstate replace=1
Actually, because the amd-pstate is fairly new for current distos, we can
modify /etc/modules-load.d/modules.conf to add one line "amd_pstate" to
inform the system this module should be loaded at boot time.
But your method also looks fine for me as well, the amd-pstate can force to
replace the acpi-cpufreq. I am not sure whether anyone objects to this way.
Thanks,
Ray
>
> Then upon the next reboot amd-pstate should load automatically even if
> acpi-cpufreq was included on the system.
> Mario Limonciello (3):
> cpufreq: Allow passing NULL as the argument for unregistering a driver
> cpufreq: amd-pstate: Allow replacing existing cpufreq drivers when
> loaded
> cpufreq: amd-pstate: Add a module device table
>
> drivers/cpufreq/amd-pstate.c | 19 ++++++++++++++++---
> drivers/cpufreq/cpufreq.c | 4 ++--
> 2 files changed, 18 insertions(+), 5 deletions(-)
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 0/3] Improve usability for amd-pstate
2022-03-27 11:56 ` [PATCH 0/3] Improve usability for amd-pstate Huang Rui
@ 2022-03-28 22:14 ` Limonciello, Mario
0 siblings, 0 replies; 8+ messages in thread
From: Limonciello, Mario @ 2022-03-28 22:14 UTC (permalink / raw)
To: Huang, Ray
Cc: Rafael J . Wysocki, Viresh Kumar, open list:AMD PSTATE DRIVER, open list
[Public]
> -----Original Message-----
> From: Huang, Ray <Ray.Huang@amd.com>
> Sent: Sunday, March 27, 2022 06:56
> To: Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Rafael J . Wysocki <rafael@kernel.org>; Viresh Kumar
> <viresh.kumar@linaro.org>; open list:AMD PSTATE DRIVER <linux-
> pm@vger.kernel.org>; open list <linux-kernel@vger.kernel.org>
> Subject: Re: [PATCH 0/3] Improve usability for amd-pstate
>
> On Fri, Mar 25, 2022 at 01:42:25PM +0800, Limonciello, Mario wrote:
> > There has recently been some news coverage about `amd-pstate` being in
> > 5.17, but this news also mentioned that it's a bit difficult to use.
> >
> > You need to either block init calls, or compile the module into the kernel
> > to force it to take precedence over acpi-cpufreq.
> >
> > This series aims to improve the usability of amd-pstate so that distros
> > can compile as a module, but users can still use it (relatively) easily.
> >
> > A new module parameter is included that will force amd-pstate to take
> > precedence and a module table to let it load automatically on such systems.
> >
> > With the patches in this series a user can make a file
> > /etc/modprobe.d/amd-pstate.conf:
> >
> > options amd-pstate replace=1
>
> Actually, because the amd-pstate is fairly new for current distos, we can
> modify /etc/modules-load.d/modules.conf to add one line "amd_pstate" to
> inform the system this module should be loaded at boot time.
Actually I don't believe that would work in the case that acpi-cpufreq is built-in
or gets loaded first.
>
> But your method also looks fine for me as well, the amd-pstate can force to
> replace the acpi-cpufreq. I am not sure whether anyone objects to this way.
Thanks for your suggestions in the series, I'll adopt them for v2.
>
> Thanks,
> Ray
>
> >
> > Then upon the next reboot amd-pstate should load automatically even if
> > acpi-cpufreq was included on the system.
> > Mario Limonciello (3):
> > cpufreq: Allow passing NULL as the argument for unregistering a driver
> > cpufreq: amd-pstate: Allow replacing existing cpufreq drivers when
> > loaded
> > cpufreq: amd-pstate: Add a module device table
> >
> > drivers/cpufreq/amd-pstate.c | 19 ++++++++++++++++---
> > drivers/cpufreq/cpufreq.c | 4 ++--
> > 2 files changed, 18 insertions(+), 5 deletions(-)
> >
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 8+ messages in thread