Re: [PATCH v5 11/18] xen/x86: implement EPP support for the amd-cppc driver in active mode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.05.2025 10:48, Penny Zheng wrote:
> @@ -537,6 +537,14 @@ choice of `dom0-kernel` is deprecated and not supported 
> by all Dom0 kernels.
>  * `amd-cppc` selects ACPI Collaborative Performance and Power Control (CPPC)
>    on supported AMD hardware to provide finer grained frequency control
>    mechanism. The default is disabled.
> +* `active` is a boolean to enable amd-cppc driver in active(autonomous) mode.
> +  In this mode, users could write to energy performance preference
> +  register(epp) to tell hardware if they want to bias toward performance or
> +  energy efficiency.

Who or what is "users" here? Certainly not the person reading the doc, trying
to determine whether to use this command line sub-option. That person can't
write to the EPP register. (Or really they maybe can, but they shouldn't.)
It's the driver which is supposed to be carrying out such writes, isn't it?

> +static int cf_check amd_cppc_epp_set_policy(struct cpufreq_policy *policy)
> +{
> +    const struct amd_cppc_drv_data *data = per_cpu(amd_cppc_drv_data,
> +                                                   policy->cpu);
> +    uint8_t max_perf, min_perf, epp;
> +
> +    /*
> +     * min_perf represents the idle frequency, while max_perf represents
> +     * the maximum frequency
> +     */
> +    max_perf = data->caps.highest_perf;
> +    min_perf = data->caps.lowest_perf;
> +
> +    /*
> +     * We set min_perf with highest_perf in performance mode
> +     * and max_perf with lowest_perf in powersave mode, to achieve
> +     * ultmost performance and power savings.
> +     */
> +    switch ( policy->policy )
> +    {
> +    case CPUFREQ_POLICY_PERFORMANCE:
> +        /* Force the epp value to be zero for performance policy */
> +        epp = CPPC_ENERGY_PERF_MAX_PERFORMANCE;
> +        min_perf = data->caps.highest_perf;
> +        break;
> +    case CPUFREQ_POLICY_POWERSAVE:
> +        /* Force the epp value to be 0xff for powersave policy */
> +        epp = CPPC_ENERGY_PERF_MAX_POWERSAVE;
> +        max_perf = data->caps.lowest_perf;
> +        break;
> +    default:
> +        epp = per_cpu(epp_init, policy->cpu);
> +        break;
> +    }

Blank lines please between non-fall-through case blocks.

Jan