Re: [PATCH v6 4/5] cpufreq/amd-pstate: Add support for raw EPP writes

["Gautham R. Shenoy" <gautham.shenoy@amd.com>]

On Sun, Mar 29, 2026 at 03:38:10PM -0500, Mario Limonciello (AMD) wrote:
> The energy performance preference field of the CPPC request MSR
> supports values from 0 to 255, but the strings only offer 4 values.
> 
> The other values are useful for tuning the performance of some
> workloads.
> 
> Add support for writing the raw energy performance preference value
> to the sysfs file.  If the last value written was an integer then
> an integer will be returned.  If the last value written was a string
> then a string will be returned.
> 
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v5->v6:
>  * Move the raw EPP parsing and readback changes into this patch
>  * Use the kernel bool literal `false` for the raw EPP tracking flag
> ---
>  Documentation/admin-guide/pm/amd-pstate.rst | 16 ++++++---
>  drivers/cpufreq/amd-pstate.c                | 36 +++++++++++++++------
>  drivers/cpufreq/amd-pstate.h                |  1 +
>  3 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index 2d92c8072b83c..6ff2b477f0472 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -316,16 +316,22 @@ A list of all the supported EPP preferences that could be used for
>  These profiles represent different hints that are provided
>  to the low-level firmware about the user's desired energy vs efficiency
>  tradeoff.  ``default`` represents the epp value is set by platform
> -firmware. This attribute is read-only.
> +firmware. ``custom`` designates that integer values 0-255 may be written
> +as well.  This attribute is read-only.
>  
>  ``energy_performance_preference``
>  
>  The current energy performance preference can be read from this attribute.
>  and user can change current preference according to energy or performance needs
> -Please get all support profiles list from
> -``energy_performance_available_preferences`` attribute, all the profiles are
> -integer values defined between 0 to 255 when EPP feature is enabled by platform
> -firmware, but if the dynamic EPP feature is enabled, driver will block writes.
> +Coarse named profiles are available in the attribute
> +``energy_performance_available_preferences``.
> +Users can also write individual integer values between 0 to 255.
> +When EPP feature is enabled by platform firmware but if the dynamic EPP feature is
> +enabled, driver will ignore the written value.

Driver isn't ignoring the written value. In this patchset,
store_energy_performance_preference() returns an -EBUSY when the
userspace writes a value to the energy_performance_preference sysfs
file with dynamic EPP enabled.

     if (cpudata->dynamic_epp) {
     		pr_debug("EPP cannot be set when dynamic EPP is enabled\n");
		return -EBUSY;
     }

So, can the last line needs to be reworded as

"When dynamic EPP is enabled, writes to energy_performance_preference
are blocked even when EPP feature is enabled by platform firmware." ?

Otherwise the patch looks good to me.

Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

-- 
Thanks and Regards
gautham.