On 03.12.2024 09:11, Penny Zheng wrote:
> From: Penny Zheng <penny.zh...@amd.com>
>
> The amd-pstate driver may support multiple working modes, passive and active.
>
> Introduce a new variable to keep track of which mode is currently enabled.
> This variable will also help to choose which cpufreq driver to be registered.
>
> Signed-off-by: Penny Zheng <penny.zh...@amd.com>
> ---
> docs/misc/xen-command-line.pandoc | 9 ++++++++-
> xen/arch/x86/acpi/cpufreq/amd-pstate.c | 12 +++++++++++-
> 2 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.pandoc
> b/docs/misc/xen-command-line.pandoc
> index 30f855fa18..a9a3e0ef79 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -499,7 +499,8 @@ If set, force use of the performance counters for
> oprofile, rather than detectin
> available support.
>
> ### cpufreq
> -> `= none | {{ <boolean> | xen } {
> [:[powersave|performance|ondemand|userspace][,[<maxfreq>]][,[<minfreq>]]] }
> [,verbose]} | dom0-kernel | hwp[:[<hdc>][,verbose]] | amd-pstate[:[verbose]]`
> +> `= none | {{ <boolean> | xen } {
> [:[powersave|performance|ondemand|userspace][,[<maxfreq>]][,[<minfreq>]]] }
> +[,verbose]} | dom0-kernel | hwp[:[<hdc>][,verbose]] |
> amd-pstate[:[active][,verbose]]`
>
> > Default: `xen`
>
> @@ -522,6 +523,12 @@ choice of `dom0-kernel` is deprecated and not supported
> by all Dom0 kernels.
> on supported AMD hardware to provide a finer grained frequency control
> mechanism.
> The default is disabled. If `amd-pstate` is selected, but hardware support
> is not available, Xen will fallback to cpufreq=xen.
> +* `active` is a boolean to enable amd-pstate driver in active(autonomous)
> mode.
> + In this mode, users could provide a hint with energy performance preference
> + register to the hardware if they want to bias toward performance(0x0) or
> + energy efficiency(0xff), then CPPC power algorithm will calculate the
> runtime
> + workload and adjust the realtime cores frequency according to the power
> supply
> + and themal, core voltage and some other hardware conditions.
Nit: thermal
> --- a/xen/arch/x86/acpi/cpufreq/amd-pstate.c
> +++ b/xen/arch/x86/acpi/cpufreq/amd-pstate.c
> @@ -27,6 +27,8 @@
> #define amd_pstate_warn(fmt, args...) \
> printk(XENLOG_WARNING "AMD_PSTATE: CPU%u warning: " fmt, cpu, ## args)
>
> +static bool __ro_after_init opt_cpufreq_active = false;
Pointless initializer.
> @@ -398,5 +407,6 @@ int __init amd_pstate_register_driver(void)
> if ( !cpu_has_cppc )
> return -ENODEV;
>
> - return cpufreq_register_driver(&amd_pstate_cpufreq_driver);
> + if ( !opt_cpufreq_active )
> + return cpufreq_register_driver(&amd_pstate_cpufreq_driver);
> }
I'm afraid the description is of no help in determining why this is a
correct change to make (here). How would the user provided hint (see
cmdline option description) be communicated to hardware when the driver
isn't even registered?
Finally I don't think the change above would build, as it leaves a
return from the function without return value.
Jan
