Messages in this thread | | | Date | Thu, 14 Apr 2022 12:32:48 -0500 | Subject | Re: [PATCH v3 4/6] cpufreq: amd-pstate: Allow replacing acpi-cpufreq when loaded | From | Nathan Fontenot <> |
| |
On 4/14/22 11:47, Mario Limonciello wrote: > `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> > --- > v2->v3: > * Rebase on earlier patches > * Use IS_REACHABLE > * Only add replace parameter if acpu-cpufreq is enabled > * Only show info message once > v1->v2: > * Update to changes from v1. > * Verify the driver being matched is acpi-cpufreq. > * Show a message letting users know they can use amd-pstate. > > drivers/cpufreq/amd-pstate.c | 22 ++++++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index d323f3e3888c..8ae65a2072d6 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -63,6 +63,13 @@ 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)"); > > +#if defined(CONFIG_X86_ACPI_CPUFREQ) || defined(CONFIG_X86_ACPI_CPUFREQ_MODULE) > +static bool replace = false; > +module_param(replace, bool, 0444); > +MODULE_PARM_DESC(replace, > + "replace acpi-cpufreq driver upon init if necessary"); > +#endif > + > static struct cpufreq_driver amd_pstate_driver; > > /** > @@ -643,6 +650,7 @@ static struct cpufreq_driver amd_pstate_driver = { > > static int __init amd_pstate_init(void) > { > + const char *current_driver; > int ret; > > if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD) > @@ -666,9 +674,19 @@ static int __init amd_pstate_init(void) > return -ENODEV; > } > > - /* don't keep reloading if cpufreq_driver exists */ > - if (cpufreq_get_current_driver()) > + current_driver = cpufreq_get_current_driver(); > + if (current_driver) { > +#if IS_REACHABLE(CONFIG_X86_ACPI_CPUFREQ) > + if (replace && strcmp(current_driver, "acpi-cpufreq") == 0) { > + acpi_cpufreq_exit(); > + } else { > + pr_info_once("A processor on this system supports amd-pstate, you can enable it with amd_pstate.replace=1\n"); > + return -EEXIST; > + } > +#else > return -EEXIST; > +#endif > + }
A couple of thoughts. First, should this also provide a path to restore the acpi_cpufreq driver if the amd-pstate driver fails during init some time after calling acpi_cpufreq_exit()?
Which leads me to wonder, should there be a more generic cpufreq_replace_driver() routine that could handle this?
-Nathan
> > /* enable amd pstate feature */ > ret = amd_pstate_enable(true);
| |