public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
From: "Gautham R. Shenoy" <gautham.shenoy@amd.com>
To: Mario Limonciello <superm1@kernel.org>
Cc: Perry Yuan <perry.yuan@amd.com>,
	"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
	<linux-kernel@vger.kernel.org>,
	"open list:CPU FREQUENCY SCALING FRAMEWORK"
	<linux-pm@vger.kernel.org>,
	Mario Limonciello <mario.limonciello@amd.com>
Subject: Re: [PATCH v5 3/5] cpufreq/amd-pstate: Add support for platform profile class
Date: Fri, 27 Mar 2026 12:28:53 +0530	[thread overview]
Message-ID: <acYqrfB6Lh3LG8UY@BLRRASHENOY1.amd.com> (raw)
In-Reply-To: <20260106051441.60093-4-superm1@kernel.org>

On Mon, Jan 05, 2026 at 11:14:39PM -0600, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> The platform profile core allows multiple drivers and devices to
> register platform profile support.
> 
> When the legacy platform profile interface is used all drivers will
> adjust the platform profile as well.
> 
> Add support for registering every CPU with the platform profile handler
> when dynamic EPP is enabled.
> 
> The end result will be that changing the platform profile will modify
> EPP accordingly.

The commit message only describes the platform profile integration.

However, the patch also does the following other things

 * introduces raw integer EPP parsing via in store_energy_performance_preference()
 
 * adds EPP_INDEX_CUSTOM
 
 * exports store_energy_performance_preference(),
   show_energy_performance_preference(), and
   amd_pstate_clear_dynamic_epp()

 * changes the performance policy check from policy->policy to
   cpudata->policy.


These are separate functional changes that aren't mentioned in the
changelog.

Or perhaps split the suport for raw epp as a separate patch, and the
policy check from policy->policy to use cpudata->policy as a separate
check.

-- 
Thanks and Regards
gautham.

> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  Documentation/admin-guide/pm/amd-pstate.rst |   4 +-
>  drivers/cpufreq/Kconfig.x86                 |   1 +
>  drivers/cpufreq/amd-pstate.c                | 144 +++++++++++++++++---
>  drivers/cpufreq/amd-pstate.h                |  10 ++
>  4 files changed, 141 insertions(+), 18 deletions(-)
> 
> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst
> index df4607b6a5f62..a6745f2358e61 100644
> --- a/Documentation/admin-guide/pm/amd-pstate.rst
> +++ b/Documentation/admin-guide/pm/amd-pstate.rst
> @@ -321,7 +321,9 @@ Whether this behavior is enabled by default with the kernel config option
>  at runtime by the sysfs file ``/sys/devices/system/cpu/cpufreq/policyX/dynamic_epp``.
>  
>  When set to enabled, the driver will select a different energy performance
> -profile when the machine is running on battery or AC power.
> +profile when the machine is running on battery or AC power. The driver will
> +also register with the platform profile handler to receive notifications of
> +user desired power state and react to those.
>  When set to disabled, the driver will not change the energy performance profile
>  based on the power source and will not react to user desired power state.
>  
> diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86
> index cdaa8d858045a..a0dbb9808ae99 100644
> --- a/drivers/cpufreq/Kconfig.x86
> +++ b/drivers/cpufreq/Kconfig.x86
> @@ -40,6 +40,7 @@ config X86_AMD_PSTATE
>  	select ACPI_PROCESSOR
>  	select ACPI_CPPC_LIB if X86_64
>  	select CPU_FREQ_GOV_SCHEDUTIL if SMP
> +	select ACPI_PLATFORM_PROFILE
>  	help
>  	  This driver adds a CPUFreq driver which utilizes a fine grain
>  	  processor performance frequency control range instead of legacy
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 7dd50b5825d78..e1ccbbdd56d42 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -109,6 +109,7 @@ static struct quirk_entry *quirks;
>   *	2		balance_performance
>   *	3		balance_power
>   *	4		power
> + *	5		custom (for raw EPP values)
>   */
>  enum energy_perf_value_index {
>  	EPP_INDEX_DEFAULT = 0,
> @@ -116,6 +117,7 @@ enum energy_perf_value_index {
>  	EPP_INDEX_BALANCE_PERFORMANCE,
>  	EPP_INDEX_BALANCE_POWERSAVE,
>  	EPP_INDEX_POWERSAVE,
> +	EPP_INDEX_CUSTOM,
>  	EPP_INDEX_MAX,
>  };
>  
> @@ -125,6 +127,7 @@ static const char * const energy_perf_strings[] = {
>  	[EPP_INDEX_BALANCE_PERFORMANCE] = "balance_performance",
>  	[EPP_INDEX_BALANCE_POWERSAVE] = "balance_power",
>  	[EPP_INDEX_POWERSAVE] = "power",
> +	[EPP_INDEX_CUSTOM] = "custom",
>  };
>  static_assert(ARRAY_SIZE(energy_perf_strings) == EPP_INDEX_MAX);
>  
> @@ -135,7 +138,7 @@ static unsigned int epp_values[] = {
>  	[EPP_INDEX_BALANCE_POWERSAVE] = AMD_CPPC_EPP_BALANCE_POWERSAVE,
>  	[EPP_INDEX_POWERSAVE] = AMD_CPPC_EPP_POWERSAVE,
>  };
> -static_assert(ARRAY_SIZE(epp_values) == EPP_INDEX_MAX);
> +static_assert(ARRAY_SIZE(epp_values) == EPP_INDEX_MAX - 1);
>  
>  typedef int (*cppc_mode_transition_fn)(int);
>  
> @@ -1094,6 +1097,10 @@ static int amd_pstate_power_supply_notifier(struct notifier_block *nb,
>  	if (event != PSY_EVENT_PROP_CHANGED)
>  		return NOTIFY_OK;
>  
> +	/* dynamic actions are only applied while platform profile is in balanced */
> +	if (cpudata->current_profile != PLATFORM_PROFILE_BALANCED)
> +		return 0;
> +
>  	epp = amd_pstate_get_balanced_epp(policy);
>  
>  	ret = amd_pstate_set_epp(policy, epp);
> @@ -1102,14 +1109,84 @@ static int amd_pstate_power_supply_notifier(struct notifier_block *nb,
>  
>  	return NOTIFY_OK;
>  }
> -static void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy)
> +
> +static int amd_pstate_profile_probe(void *drvdata, unsigned long *choices)
> +{
> +	set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
> +	set_bit(PLATFORM_PROFILE_BALANCED, choices);
> +	set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
> +
> +	return 0;
> +}
> +
> +static int amd_pstate_profile_get(struct device *dev,
> +				  enum platform_profile_option *profile)
> +{
> +	struct amd_cpudata *cpudata = dev_get_drvdata(dev);
> +
> +	*profile = cpudata->current_profile;
> +
> +	return 0;
> +}
> +
> +static int amd_pstate_profile_set(struct device *dev,
> +				  enum platform_profile_option profile)
> +{
> +	struct amd_cpudata *cpudata = dev_get_drvdata(dev);
> +	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpudata->cpu);
> +	int ret;
> +
> +	switch (profile) {
> +	case PLATFORM_PROFILE_LOW_POWER:
> +		if (cpudata->policy != CPUFREQ_POLICY_POWERSAVE)
> +			cpudata->policy = CPUFREQ_POLICY_POWERSAVE;
> +		ret = amd_pstate_set_epp(policy, AMD_CPPC_EPP_POWERSAVE);
> +		if (ret)
> +			return ret;
> +		break;
> +	case PLATFORM_PROFILE_BALANCED:
> +		if (cpudata->policy != CPUFREQ_POLICY_POWERSAVE)
> +			cpudata->policy = CPUFREQ_POLICY_POWERSAVE;
> +		ret = amd_pstate_set_epp(policy,
> +					 amd_pstate_get_balanced_epp(policy));
> +		if (ret)
> +			return ret;
> +		break;
> +	case PLATFORM_PROFILE_PERFORMANCE:
> +		ret = amd_pstate_set_epp(policy, AMD_CPPC_EPP_PERFORMANCE);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		pr_err("Unknown Platform Profile %d\n", profile);
> +		return -EOPNOTSUPP;
> +	}
> +
> +	cpudata->current_profile = profile;
> +
> +	return 0;
> +}
> +
> +static const struct platform_profile_ops amd_pstate_profile_ops = {
> +	.probe = amd_pstate_profile_probe,
> +	.profile_set = amd_pstate_profile_set,
> +	.profile_get = amd_pstate_profile_get,
> +};
> +
> +void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy)
>  {
>  	struct amd_cpudata *cpudata = policy->driver_data;
>  
>  	if (cpudata->power_nb.notifier_call)
>  		power_supply_unreg_notifier(&cpudata->power_nb);
> +	if (cpudata->ppdev) {
> +		platform_profile_remove(cpudata->ppdev);
> +		cpudata->ppdev = NULL;
> +	}
> +	kfree(cpudata->profile_name);
>  	cpudata->dynamic_epp = false;
>  }
> +EXPORT_SYMBOL_GPL(amd_pstate_clear_dynamic_epp);
>  
>  static int amd_pstate_set_dynamic_epp(struct cpufreq_policy *policy)
>  {
> @@ -1117,11 +1194,35 @@ static int amd_pstate_set_dynamic_epp(struct cpufreq_policy *policy)
>  	int ret;
>  	u8 epp;
>  
> -	epp = amd_pstate_get_balanced_epp(policy);
> +	switch (cpudata->current_profile) {
> +	case PLATFORM_PROFILE_PERFORMANCE:
> +		epp = AMD_CPPC_EPP_PERFORMANCE;
> +		break;
> +	case PLATFORM_PROFILE_LOW_POWER:
> +		epp = AMD_CPPC_EPP_POWERSAVE;
> +		break;
> +	case PLATFORM_PROFILE_BALANCED:
> +		epp = amd_pstate_get_balanced_epp(policy);
> +		break;
> +	default:
> +		pr_err("Unknown Platform Profile %d\n", cpudata->current_profile);
> +		return -EOPNOTSUPP;
> +	}
>  	ret = amd_pstate_set_epp(policy, epp);
>  	if (ret)
>  		return ret;
>  
> +	cpudata->profile_name = kasprintf(GFP_KERNEL, "amd-pstate-epp-cpu%d", cpudata->cpu);
> +
> +	cpudata->ppdev = platform_profile_register(get_cpu_device(policy->cpu),
> +						   cpudata->profile_name,
> +						   policy->driver_data,
> +						   &amd_pstate_profile_ops);
> +	if (IS_ERR(cpudata->ppdev)) {
> +		ret = PTR_ERR(cpudata->ppdev);
> +		goto cleanup;
> +	}
> +
>  	/* only enable notifier if things will actually change */
>  	if (cpudata->epp_default_ac != cpudata->epp_default_dc) {
>  		ret = power_supply_reg_notifier(&cpudata->power_nb);
> @@ -1227,8 +1328,8 @@ static ssize_t show_energy_performance_available_preferences(
>  	return offset;
>  }
>  
> -static ssize_t store_energy_performance_preference(
> -		struct cpufreq_policy *policy, const char *buf, size_t count)
> +ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
> +					    const char *buf, size_t count)
>  {
>  	struct amd_cpudata *cpudata = policy->driver_data;
>  	ssize_t ret;
> @@ -1239,16 +1340,22 @@ static ssize_t store_energy_performance_preference(
>  		return -EBUSY;
>  	}
>  
> -	ret = sysfs_match_string(energy_perf_strings, buf);
> -	if (ret < 0)
> -		return -EINVAL;
> -
> -	if (ret)
> -		epp = epp_values[ret];
> -	else
> -		epp = amd_pstate_get_balanced_epp(policy);
> +	/*
> +	 * if the value matches a number, use that, otherwise see if
> +	 * matches an index in the energy_perf_strings array
> +	 */
> +	ret = kstrtou8(buf, 0, &epp);
> +	if (ret) {
> +		ret = sysfs_match_string(energy_perf_strings, buf);
> +		if (ret < 0 || ret == EPP_INDEX_CUSTOM)
> +			return -EINVAL;
> +		if (ret)
> +			epp = epp_values[ret];
> +		else
> +			epp = amd_pstate_get_balanced_epp(policy);
> +	}
>  
> -	if (epp > 0 && policy->policy == CPUFREQ_POLICY_PERFORMANCE) {
> +	if (epp > 0 && cpudata->policy == CPUFREQ_POLICY_PERFORMANCE) {
>  		pr_debug("EPP cannot be set under performance policy\n");
>  		return -EBUSY;
>  	}
> @@ -1259,9 +1366,9 @@ static ssize_t store_energy_performance_preference(
>  
>  	return ret ? ret : count;
>  }
> +EXPORT_SYMBOL_GPL(store_energy_performance_preference);
>  
> -static ssize_t show_energy_performance_preference(
> -				struct cpufreq_policy *policy, char *buf)
> +ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *buf)
>  {
>  	struct amd_cpudata *cpudata = policy->driver_data;
>  	u8 preference, epp;
> @@ -1282,11 +1389,12 @@ static ssize_t show_energy_performance_preference(
>  		preference = EPP_INDEX_POWERSAVE;
>  		break;
>  	default:
> -		return -EINVAL;
> +		return sysfs_emit(buf, "%u\n", epp);
>  	}
>  
>  	return sysfs_emit(buf, "%s\n", energy_perf_strings[preference]);
>  }
> +EXPORT_SYMBOL_GPL(show_energy_performance_preference);
>  
>  static void amd_pstate_driver_cleanup(void)
>  {
> @@ -1621,10 +1729,12 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>  	    amd_pstate_acpi_pm_profile_undefined()) {
>  		policy->policy = CPUFREQ_POLICY_PERFORMANCE;
>  		cpudata->epp_default_ac = cpudata->epp_default_dc = amd_pstate_get_epp(cpudata);
> +		cpudata->current_profile = PLATFORM_PROFILE_PERFORMANCE;
>  	} else {
>  		policy->policy = CPUFREQ_POLICY_POWERSAVE;
>  		cpudata->epp_default_ac = AMD_CPPC_EPP_PERFORMANCE;
>  		cpudata->epp_default_dc = AMD_CPPC_EPP_BALANCE_PERFORMANCE;
> +		cpudata->current_profile = PLATFORM_PROFILE_BALANCED;
>  	}
>  
>  	if (dynamic_epp) {
> diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h
> index 7bfe5f8115623..9839c7c6558f4 100644
> --- a/drivers/cpufreq/amd-pstate.h
> +++ b/drivers/cpufreq/amd-pstate.h
> @@ -9,6 +9,7 @@
>  #define _LINUX_AMD_PSTATE_H
>  
>  #include <linux/pm_qos.h>
> +#include <linux/platform_profile.h>
>  
>  /*********************************************************************
>   *                        AMD P-state INTERFACE                       *
> @@ -110,6 +111,11 @@ struct amd_cpudata {
>  	u8	epp_default_dc;
>  	bool	dynamic_epp;
>  	struct notifier_block power_nb;
> +
> +	/* platform profile */
> +	enum platform_profile_option current_profile;
> +	struct device *ppdev;
> +	char *profile_name;
>  };
>  
>  /*
> @@ -126,5 +132,9 @@ enum amd_pstate_mode {
>  const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
>  int amd_pstate_get_status(void);
>  int amd_pstate_update_status(const char *buf, size_t size);
> +ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
> +					    const char *buf, size_t count);
> +ssize_t show_energy_performance_preference(struct cpufreq_policy *policy, char *buf);
> +void amd_pstate_clear_dynamic_epp(struct cpufreq_policy *policy);
>  
>  #endif /* _LINUX_AMD_PSTATE_H */
> 
> -- 
> 2.43.0
> 

  reply	other threads:[~2026-03-27  6:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-06  5:14 [PATCH v5 0/5] amd-pstate Dynamic EPP and raw EPP Mario Limonciello (AMD)
2026-01-06  5:14 ` [PATCH v5 1/5] cpufreq/amd-pstate: Add dynamic energy performance preference Mario Limonciello (AMD)
2026-03-27  6:43   ` Gautham R. Shenoy
2026-01-06  5:14 ` [PATCH v5 2/5] cpufreq/amd-pstate: add kernel command line to override dynamic epp Mario Limonciello (AMD)
2026-03-27  6:45   ` Gautham R. Shenoy
2026-01-06  5:14 ` [PATCH v5 3/5] cpufreq/amd-pstate: Add support for platform profile class Mario Limonciello (AMD)
2026-03-27  6:58   ` Gautham R. Shenoy [this message]
2026-01-06  5:14 ` [PATCH v5 4/5] cpufreq/amd-pstate: Add support for raw EPP writes Mario Limonciello (AMD)
2026-03-27  8:41   ` Gautham R. Shenoy
2026-01-06  5:14 ` [PATCH v5 5/5] cpufreq/amd-pstate-ut: Add a unit test for raw EPP Mario Limonciello (AMD)
2026-03-27  8:59   ` Gautham R. Shenoy
2026-03-31 12:09 ` [PATCH v5 0/5] amd-pstate Dynamic EPP and " Mario Limonciello

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=acYqrfB6Lh3LG8UY@BLRRASHENOY1.amd.com \
    --to=gautham.shenoy@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mario.limonciello@amd.com \
    --cc=perry.yuan@amd.com \
    --cc=superm1@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox