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

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

On Mon, Jan 05, 2026 at 11:14:40PM -0600, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> 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.


Support for raw EPP values was present in the previous patch
itself. It appears that some hunks from the previous patch belong
here.


> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  Documentation/admin-guide/pm/amd-pstate.rst | 16 +++++++++++-----
>  drivers/cpufreq/amd-pstate.c                | 11 +++++++++--
>  drivers/cpufreq/amd-pstate.h                |  1 +
>  3 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index a6745f2358e61..674e76ff9293e 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -280,16 +280,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. Lower epp values shift the bias
> +towards improved performance while a higher epp value shifts the bias towards
> +power-savings. The exact impact can change from one platform to the other.
> +If a valid integer was last written, then a number will be returned on future reads.
> +If a valid string was last written then a string will be returned on future reads.
>  This attribute is read-write.
>  
>  ``boost``
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index e1ccbbdd56d42..14347baf3cefb 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -1333,6 +1333,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
>  {
>  	struct amd_cpudata *cpudata = policy->driver_data;
>  	ssize_t ret;
> +	bool raw_epp = FALSE;

s/FALSE/false ?

false is already defined in <linux/types.h> and rest of the file uses
it.



>  	u8 epp;
>  
>  	if (cpudata->dynamic_epp) {
> @@ -1345,6 +1346,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
>  	 * matches an index in the energy_perf_strings array
>  	 */
>  	ret = kstrtou8(buf, 0, &epp);
> +	raw_epp = !ret;
>  	if (ret) {
>  		ret = sysfs_match_string(energy_perf_strings, buf);
>  		if (ret < 0 || ret == EPP_INDEX_CUSTOM)
> @@ -1364,7 +1366,9 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
>  	if (ret)
>  		return ret;
>  
> -	return ret ? ret : count;
> +	cpudata->raw_epp = raw_epp;
> +
> +	return count;
>  }
>  EXPORT_SYMBOL_GPL(store_energy_performance_preference);
>  
> @@ -1375,6 +1379,9 @@ ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *
>  
>  	epp = FIELD_GET(AMD_CPPC_EPP_PERF_MASK, cpudata->cppc_req_cached);
>  
> +	if (cpudata->raw_epp)
> +		return sysfs_emit(buf, "%u\n", epp);
> +
>  	switch (epp) {
>  	case AMD_CPPC_EPP_PERFORMANCE:
>  		preference = EPP_INDEX_PERFORMANCE;
> @@ -1389,7 +1396,7 @@ ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *
>  		preference = EPP_INDEX_POWERSAVE;
>  		break;
>  	default:
> -		return sysfs_emit(buf, "%u\n", epp);
> +		return -EINVAL;
>  	}
>  
>  	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index 9839c7c6558f4..38b575b8ad959 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -110,6 +110,7 @@ struct amd_cpudata {
>  	u8	epp_default_ac;
>  	u8	epp_default_dc;
>  	bool	dynamic_epp;
> +	bool	raw_epp;
>  	struct notifier_block power_nb;

Looks good otherwise.

-- 
Thanks and Regards
gautham.