Skip to contentSkip to conversation

amd-pstate Dynamic EPP and raw EPP

PATCH Superseded Mario Limonciello 2 participants 15 messages Mar 21, 2025 – Mar 25, 2025

Subsystems (3)

v4 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

v4 cover amd-pstate Dynamic EPP and raw EPP

From: Mario Limonciello <[email protected]>

Dynamic EPP allows the kernel to register amd-pstate as part of a platform profile. It will change EPP modes matching the user's preference to the platform profile sysfs files as well as power adapter state.

Raw EPP allows userspace to write integers to
energy_performance_preference.

This series is based off superm1/linux.git bleeding-edge branch

Mario Limonciello (5):
cpufreq/amd-pstate: Add dynamic energy performance preference cpufreq/amd-pstate: add kernel command line to override dynamic epp cpufreq/amd-pstate: Add support for platform profile class cpufreq/amd-pstate: Add support for raw EPP writes cpufreq/amd-pstate-ut: Add a unit test for raw EPP

 .../admin-guide/kernel-parameters.txt         |   7 +
 Documentation/admin-guide/pm/amd-pstate.rst   |  41 ++-
 drivers/cpufreq/Kconfig.x86                   |  13 +
 drivers/cpufreq/amd-pstate-ut.c               |  58 ++++
 drivers/cpufreq/amd-pstate.c                  | 285 ++++++++++++++++--
 drivers/cpufreq/amd-pstate.h                  |  16 +-
 6 files changed, 396 insertions(+), 24 deletions(-)
Signature · 1 line
2.43.0
#1 lore raw

PATCH v4 1/5 cpufreq/amd-pstate: Add dynamic energy performance preference

From: Mario Limonciello <[email protected]>

Dynamic energy performance preference will change the EPP profile based on whether the machine is running on AC or DC power.

A notification chain from the power supply core is used to adjust EPP values on plug in or plug out events.

For non-server systems:
* the default EPP for AC mode is performance.
* the default EPP for DC mode is balance_performance.

---
v3->v4:
* Handle Kconfig not being set
* Fix dynamic epp default on server
v2-v3:
* Fix typo in Kconfig
v1->v2:
* Change defaults to performance (AC) and balance_performance (DC)
* Default Kconfig to disabled for now
* Rebase on latest branch

---
 Documentation/admin-guide/pm/amd-pstate.rst |  18 ++-
 drivers/cpufreq/Kconfig.x86                 |  12 ++
 drivers/cpufreq/amd-pstate.c                | 135 +++++++++++++++++++-
 drivers/cpufreq/amd-pstate.h                |   5 +-
 4 files changed, 161 insertions(+), 9 deletions(-)
Documentation/admin-guide/pm/amd-pstate.rst +17 -1
index 412423c54f258..2e076650dc77c 100644
@@ -289,7 +289,7 @@ 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, if EPP feature is disabled, driver will ignore the written value
+firmware, but if the dynamic EPP feature is enabled, driver will block writes.
 This attribute is read-write.
 
 ``boost``
@@ -311,6 +311,22 @@ boost or `1` to enable it, for the respective CPU using the sysfs path
 Other performance and frequency values can be read back from
 ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
 
+Dynamic energy performance profile
+==================================
+The amd-pstate driver supports dynamically selecting the energy performance
+profile based on whether the machine is running on AC or DC power.
+
+Whether this behavior is enabled by default with the kernel config option
+`CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP`. This behavior can also be overridden
+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.
+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.
+
+Attempting to manually write to the ``energy_performance_preference`` sysfs
+file will fail when ``dynamic_epp`` is enabled.
 
 ``amd-pstate`` vs ``acpi-cpufreq``
 ======================================
drivers/cpufreq/Kconfig.x86 +12 -0
index 97c2d4f15d76e..3a8bdc35f488a 100644
@@ -68,6 +68,18 @@ config X86_AMD_PSTATE_DEFAULT_MODE
 	  For details, take a look at:
 	  <file:Documentation/admin-guide/pm/amd-pstate.rst>.
 
+config X86_AMD_PSTATE_DYNAMIC_EPP
+	bool "AMD Processor P-State dynamic EPP support"
+	depends on X86_AMD_PSTATE
+	default n
+	help
+	  Allow the kernel to dynamically change the energy performance
+	  value from events like ACPI platform profile and AC adapter plug
+	  events.
+
+	  This feature can also be changed at runtime, this configuration
+	  option only sets the kernel default value behavior.
+
 config X86_AMD_PSTATE_UT
 	tristate "selftest for AMD Processor P-State driver"
 	depends on X86 && ACPI_PROCESSOR
drivers/cpufreq/amd-pstate.c +128 -7

217 lines not on this page · raw message

drivers/cpufreq/amd-pstate.h +4 -1
index fbe1c08d3f061..6882876f895de 100644
@@ -104,7 +104,10 @@ struct amd_cpudata {
 	/* EPP feature related attributes*/
 	u32	policy;
 	bool	suspended;
-	u8	epp_default;
+	u8	epp_default_ac;
+	u8	epp_default_dc;
+	bool	dynamic_epp;
+	struct notifier_block power_nb;
 };
 
 /*
Signature · 1 line
2.43.0
#2 lore raw

Re: [PATCH v4 1/5] cpufreq/amd-pstate: Add dynamic energy performance preference

Quoted 45 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Dynamic energy performance preference will change the EPP profile based on whether the machine is running on AC or DC power.

A notification chain from the power supply core is used to adjust EPP values on plug in or plug out events.

For non-server systems:
* the default EPP for AC mode is performance.
* the default EPP for DC mode is balance_performance.

---
v3->v4:
* Handle Kconfig not being set
* Fix dynamic epp default on server
v2-v3:
* Fix typo in Kconfig
v1->v2:
* Change defaults to performance (AC) and balance_performance (DC)
* Default Kconfig to disabled for now
* Rebase on latest branch

---
 Documentation/admin-guide/pm/amd-pstate.rst |  18 ++-
 drivers/cpufreq/Kconfig.x86                 |  12 ++
 drivers/cpufreq/amd-pstate.c                | 135 +++++++++++++++++++-
 drivers/cpufreq/amd-pstate.h                |   5 +-
 4 files changed, 161 insertions(+), 9 deletions(-)
Documentation/admin-guide/pm/amd-pstate.rst +1 -1
index 412423c54f258..2e076650dc77c 100644
@@ -289,7 +289,7 @@ 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, if EPP feature is disabled, driver will ignore the written value
+firmware, but if the dynamic EPP feature is enabled, driver will block writes.
 This attribute is read-write.
 
 ``boost``
@@ -311,6 +311,22 @@ boost or `1` to enable it, for the respective CPU using the sysfs path
 Other performance and frequency values can be read back from

Documentation/admin-guide/pm/amd-pstate.rst boost or `1` to enable it, for the respective CPU using the sysfs path · line 321

Quoted 10 lines
Documentation/admin-guide/pm/amd-pstate.rst +8 -0
 ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
 
+Dynamic energy performance profile
+==================================
+The amd-pstate driver supports dynamically selecting the energy performance
+profile based on whether the machine is running on AC or DC power.
+
+Whether this behavior is enabled by default with the kernel config option
+`CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP`. This behavior can also be overridden
+at runtime by the sysfs file ``/sys/devices/system/cpu/cpufreq/policyX/dynamic_epp``.

The file is actually located at "/sys/devices/system/cpu/amd_pstate/dynamic_epp"

Quoted 11 lines
Documentation/admin-guide/pm/amd-pstate.rst +8 -0
+
+When set to enabled, the driver will select a different energy performance
+profile when the machine is running on battery or AC power.
+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.
+
+Attempting to manually write to the ``energy_performance_preference`` sysfs
+file will fail when ``dynamic_epp`` is enabled.
 
 ``amd-pstate`` vs ``acpi-cpufreq``
 ======================================

[snip]

Documentation/admin-guide/pm/amd-pstate.rst static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) · line 1616

Quoted 8 lines
Documentation/admin-guide/pm/amd-pstate.rst +3 -1
@@ -1502,9 +1611,15 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 			return ret;
 		WRITE_ONCE(cpudata->cppc_req_cached, value);
 	}
-	ret = amd_pstate_set_epp(policy, cpudata->epp_default);
+
+	if (dynamic_epp) {
+		policy->policy = CPUFREQ_POLICY_PERFORMANCE;

So, we are allowing the dynamic EPP framework to modify the EPP value in performance governor as well? Shouldn't we allow dynamic_epp only with powersave governor.

Quoted 46 lines
Documentation/admin-guide/pm/amd-pstate.rst +11 -1
+		ret = amd_pstate_set_dynamic_epp(policy);
+	}
+	else
+		ret = amd_pstate_set_epp(policy, amd_pstate_get_balanced_epp(policy));
 	if (ret)
-		return ret;
+		goto free_cpudata1;
 
 	current_pstate_driver->adjust_perf = NULL;
 
@@ -1521,6 +1636,8 @@ static void amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
 	struct amd_cpudata *cpudata = policy->driver_data;
 
 	if (cpudata) {
+		if (cpudata->dynamic_epp)
+			amd_pstate_clear_dynamic_epp(policy);
 		kfree(cpudata);
 		policy->driver_data = NULL;
 	}
@@ -1556,6 +1673,10 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
 	if (!policy->cpuinfo.max_freq)
 		return -ENODEV;
 
+	/* policy can't be changed to powersave policy while dynamic epp is enabled */
+	if (policy->policy == CPUFREQ_POLICY_POWERSAVE && cpudata->dynamic_epp)
+		return -EBUSY;
+
 	cpudata->policy = policy->policy;
 
 	ret = amd_pstate_epp_update_limit(policy);
drivers/cpufreq/amd-pstate.h +4 -1
index fbe1c08d3f061..6882876f895de 100644
@@ -104,7 +104,10 @@ struct amd_cpudata {
 	/* EPP feature related attributes*/
 	u32	policy;
 	bool	suspended;
-	u8	epp_default;
+	u8	epp_default_ac;
+	u8	epp_default_dc;
+	bool	dynamic_epp;
+	struct notifier_block power_nb;
 };
 
 /*
#3 lore raw

Re: [PATCH v4 1/5] cpufreq/amd-pstate: Add dynamic energy performance preference

Quoted 71 lines · Dhananjay Ugwekar
Quoted 45 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Dynamic energy performance preference will change the EPP profile based on whether the machine is running on AC or DC power.

A notification chain from the power supply core is used to adjust EPP values on plug in or plug out events.

For non-server systems:
* the default EPP for AC mode is performance.
* the default EPP for DC mode is balance_performance.

---
v3->v4:
* Handle Kconfig not being set
* Fix dynamic epp default on server
v2-v3:
* Fix typo in Kconfig
v1->v2:
* Change defaults to performance (AC) and balance_performance (DC)
* Default Kconfig to disabled for now
* Rebase on latest branch

---
  Documentation/admin-guide/pm/amd-pstate.rst |  18 ++-
  drivers/cpufreq/Kconfig.x86                 |  12 ++
  drivers/cpufreq/amd-pstate.c                | 135 +++++++++++++++++++-
  drivers/cpufreq/amd-pstate.h                |   5 +-
  4 files changed, 161 insertions(+), 9 deletions(-)
Documentation/admin-guide/pm/amd-pstate.rst +1 -1
index 412423c54f258..2e076650dc77c 100644
@@ -289,7 +289,7 @@ 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, if EPP feature is disabled, driver will ignore the written value
+firmware, but if the dynamic EPP feature is enabled, driver will block writes.
  This attribute is read-write.
 
  ``boost``
@@ -311,6 +311,22 @@ boost or `1` to enable it, for the respective CPU using the sysfs path
  Other performance and frequency values can be read back from

Documentation/admin-guide/pm/amd-pstate.rst boost or `1` to enable it, for the respective CPU using the sysfs path · line 321

Quoted 10 lines
Documentation/admin-guide/pm/amd-pstate.rst +8 -0
  ``/sys/devices/system/cpu/cpuX/acpi_cppc/``, see :ref:`cppc_sysfs`.
 
+Dynamic energy performance profile
+==================================
+The amd-pstate driver supports dynamically selecting the energy performance
+profile based on whether the machine is running on AC or DC power.
+
+Whether this behavior is enabled by default with the kernel config option
+`CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP`. This behavior can also be overridden
+at runtime by the sysfs file ``/sys/devices/system/cpu/cpufreq/policyX/dynamic_epp``.

The file is actually located at "/sys/devices/system/cpu/amd_pstate/dynamic_epp"

Quoted 11 lines
Documentation/admin-guide/pm/amd-pstate.rst +8 -0
+
+When set to enabled, the driver will select a different energy performance
+profile when the machine is running on battery or AC power.
+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.
+
+Attempting to manually write to the ``energy_performance_preference`` sysfs
+file will fail when ``dynamic_epp`` is enabled.
 
  ``amd-pstate`` vs ``acpi-cpufreq``
  ======================================

[snip]

Quoted 11 lines

Documentation/admin-guide/pm/amd-pstate.rst static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) · line 1616

Quoted 8 lines
Documentation/admin-guide/pm/amd-pstate.rst +3 -1
@@ -1502,9 +1611,15 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
  			return ret;
  		WRITE_ONCE(cpudata->cppc_req_cached, value);
  	}
-	ret = amd_pstate_set_epp(policy, cpudata->epp_default);
+
+	if (dynamic_epp) {
+		policy->policy = CPUFREQ_POLICY_PERFORMANCE;

So, we are allowing the dynamic EPP framework to modify the EPP value in performance governor as well? Shouldn't we allow dynamic_epp only with powersave governor.

You're right; I was trying to avoid issues in amd_pstate_epp_set_policy() but I'll come up with a way to correct this.

Quoted 48 lines
Quoted 47 lines
Documentation/admin-guide/pm/amd-pstate.rst +11 -1
+		ret = amd_pstate_set_dynamic_epp(policy);
+	}
+	else
+		ret = amd_pstate_set_epp(policy, amd_pstate_get_balanced_epp(policy));
  	if (ret)
-		return ret;
+		goto free_cpudata1;
 
  	current_pstate_driver->adjust_perf = NULL;
 
@@ -1521,6 +1636,8 @@ static void amd_pstate_epp_cpu_exit(struct cpufreq_policy *policy)
  	struct amd_cpudata *cpudata = policy->driver_data;
 
  	if (cpudata) {
+		if (cpudata->dynamic_epp)
+			amd_pstate_clear_dynamic_epp(policy);
  		kfree(cpudata);
  		policy->driver_data = NULL;
  	}
@@ -1556,6 +1673,10 @@ static int amd_pstate_epp_set_policy(struct cpufreq_policy *policy)
  	if (!policy->cpuinfo.max_freq)
  		return -ENODEV;
 
+	/* policy can't be changed to powersave policy while dynamic epp is enabled */
+	if (policy->policy == CPUFREQ_POLICY_POWERSAVE && cpudata->dynamic_epp)
+		return -EBUSY;
+
  	cpudata->policy = policy->policy;
 
  	ret = amd_pstate_epp_update_limit(policy);
drivers/cpufreq/amd-pstate.h +4 -1
index fbe1c08d3f061..6882876f895de 100644
@@ -104,7 +104,10 @@ struct amd_cpudata {
  	/* EPP feature related attributes*/
  	u32	policy;
  	bool	suspended;
-	u8	epp_default;
+	u8	epp_default_ac;
+	u8	epp_default_dc;
+	bool	dynamic_epp;
+	struct notifier_block power_nb;
  };
 
  /*
#4 lore raw

Re: [PATCH v4 1/5] cpufreq/amd-pstate: Add dynamic energy performance preference

In reply to Mario Limonciello
Quoted 30 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Dynamic energy performance preference will change the EPP profile based on whether the machine is running on AC or DC power.

A notification chain from the power supply core is used to adjust EPP values on plug in or plug out events.

For non-server systems:
* the default EPP for AC mode is performance.
* the default EPP for DC mode is balance_performance.

---
v3->v4:
* Handle Kconfig not being set
* Fix dynamic epp default on server
v2-v3:
* Fix typo in Kconfig
v1->v2:
* Change defaults to performance (AC) and balance_performance (DC)
* Default Kconfig to disabled for now
* Rebase on latest branch

---
 Documentation/admin-guide/pm/amd-pstate.rst |  18 ++-
 drivers/cpufreq/Kconfig.x86                 |  12 ++
 drivers/cpufreq/amd-pstate.c                | 135 +++++++++++++++++++-
 drivers/cpufreq/amd-pstate.h                |   5 +-
 4 files changed, 161 insertions(+), 9 deletions(-)

[snip]

Quoted 9 lines
drivers/cpufreq/amd-pstate.c +5 -0
@@ -1050,6 +1056,73 @@ static void amd_pstate_cpu_exit(struct cpufreq_policy *policy)
 	kfree(cpudata);
 }
 
+static int amd_pstate_get_balanced_epp(struct cpufreq_policy *policy)
+{
+	struct amd_cpudata *cpudata = policy->driver_data;
+
+	if (power_supply_is_system_supplied())

drivers/cpufreq/amd-pstate.c static void amd_pstate_cpu_exit(struct cpufreq_policy *policy) · line 1073

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+		return cpudata->epp_default_ac;
+	else
+		return cpudata->epp_default_dc;
+}
+
+static int amd_pstate_power_supply_notifier(struct notifier_block *nb,
+					    unsigned long event, void *data)
+{
+	struct amd_cpudata *cpudata = container_of(nb, struct amd_cpudata, power_nb);
+	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpudata->cpu);

For consistency, we should add "if (!policy)" check I think

Quoted 14 lines
drivers/cpufreq/amd-pstate.c +14 -0
+	u8 epp;
+	int ret;
+
+	if (event != PSY_EVENT_PROP_CHANGED)
+		return NOTIFY_OK;
+
+	epp = amd_pstate_get_balanced_epp(policy);
+
+	ret = amd_pstate_set_epp(policy, epp);
+	if (ret)
+		pr_warn("Failed to set CPU %d EPP %u: %d\n", cpudata->cpu, epp, ret);
+
+	return NOTIFY_OK;
+}

[snip]

Quoted 16 lines
drivers/cpufreq/amd-pstate.c +12 -0
@@ -1364,6 +1444,32 @@ static ssize_t prefcore_show(struct device *dev,
 	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
 }
 
+static ssize_t dynamic_epp_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%s\n", str_enabled_disabled(dynamic_epp));
+}
+
+static ssize_t dynamic_epp_store(struct device *a, struct device_attribute *b,
+				 const char *buf, size_t count)
+{
+	bool enabled;
+	int ret;
+

drivers/cpufreq/amd-pstate.c static ssize_t prefcore_show(struct device *dev, · line 1468

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+	ret = kstrtobool(buf, &enabled);
+	if (ret)
+		return ret;
+
+	if (dynamic_epp == enabled)
+		return -EINVAL;
+
+	/* reinitialize with desired dynamic EPP value */
+	dynamic_epp = enabled;
+	ret = amd_pstate_change_driver_mode(cppc_state);

I think implicitly changing the driver mode when we write to dynamic_epp file might lead to some confusions.

Quoted 6 lines
drivers/cpufreq/amd-pstate.c +4 -0
+
+	return ret ? ret : count;
+}
+
 cpufreq_freq_attr_ro(amd_pstate_max_freq);
 cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
#5 lore raw

Re: [PATCH v4 1/5] cpufreq/amd-pstate: Add dynamic energy performance preference

Quoted 89 lines · Dhananjay Ugwekar
Quoted 30 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Dynamic energy performance preference will change the EPP profile based on whether the machine is running on AC or DC power.

A notification chain from the power supply core is used to adjust EPP values on plug in or plug out events.

For non-server systems:
* the default EPP for AC mode is performance.
* the default EPP for DC mode is balance_performance.

---
v3->v4:
* Handle Kconfig not being set
* Fix dynamic epp default on server
v2-v3:
* Fix typo in Kconfig
v1->v2:
* Change defaults to performance (AC) and balance_performance (DC)
* Default Kconfig to disabled for now
* Rebase on latest branch

---
  Documentation/admin-guide/pm/amd-pstate.rst |  18 ++-
  drivers/cpufreq/Kconfig.x86                 |  12 ++
  drivers/cpufreq/amd-pstate.c                | 135 +++++++++++++++++++-
  drivers/cpufreq/amd-pstate.h                |   5 +-
  4 files changed, 161 insertions(+), 9 deletions(-)

[snip]

Quoted 9 lines
drivers/cpufreq/amd-pstate.c +5 -0
@@ -1050,6 +1056,73 @@ static void amd_pstate_cpu_exit(struct cpufreq_policy *policy)
  	kfree(cpudata);
  }
 
+static int amd_pstate_get_balanced_epp(struct cpufreq_policy *policy)
+{
+	struct amd_cpudata *cpudata = policy->driver_data;
+
+	if (power_supply_is_system_supplied())

drivers/cpufreq/amd-pstate.c static void amd_pstate_cpu_exit(struct cpufreq_policy *policy) · line 1073

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+		return cpudata->epp_default_ac;
+	else
+		return cpudata->epp_default_dc;
+}
+
+static int amd_pstate_power_supply_notifier(struct notifier_block *nb,
+					    unsigned long event, void *data)
+{
+	struct amd_cpudata *cpudata = container_of(nb, struct amd_cpudata, power_nb);
+	struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpudata->cpu);

For consistency, we should add "if (!policy)" check I think

Quoted 14 lines
drivers/cpufreq/amd-pstate.c +14 -0
+	u8 epp;
+	int ret;
+
+	if (event != PSY_EVENT_PROP_CHANGED)
+		return NOTIFY_OK;
+
+	epp = amd_pstate_get_balanced_epp(policy);
+
+	ret = amd_pstate_set_epp(policy, epp);
+	if (ret)
+		pr_warn("Failed to set CPU %d EPP %u: %d\n", cpudata->cpu, epp, ret);
+
+	return NOTIFY_OK;
+}

[snip]

Quoted 20 lines
drivers/cpufreq/amd-pstate.c +16 -0
@@ -1364,6 +1444,32 @@ static ssize_t prefcore_show(struct device *dev,
  	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
  }
 
+static ssize_t dynamic_epp_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%s\n", str_enabled_disabled(dynamic_epp));
+}
+
+static ssize_t dynamic_epp_store(struct device *a, struct device_attribute *b,
+				 const char *buf, size_t count)
+{
+	bool enabled;
+	int ret;
+
+	ret = kstrtobool(buf, &enabled);
+	if (ret)
+		return ret;
+
Quoted 8 lines

drivers/cpufreq/amd-pstate.c static ssize_t prefcore_show(struct device *dev, · line 1468

Quoted 6 lines
drivers/cpufreq/amd-pstate.c +6 -0
+	if (dynamic_epp == enabled)
+		return -EINVAL;
+
+	/* reinitialize with desired dynamic EPP value */
+	dynamic_epp = enabled;
+	ret = amd_pstate_change_driver_mode(cppc_state);

I think implicitly changing the driver mode when we write to dynamic_epp file might lead to some confusions.

How about only allowing to write dynamic_epp attribute when in active mode already?

Quoted 7 lines
Quoted 7 lines
drivers/cpufreq/amd-pstate.c +4 -0
+
+	return ret ? ret : count;
+}
+
  cpufreq_freq_attr_ro(amd_pstate_max_freq);
  cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
#6 lore raw

Re: [PATCH v4 1/5] cpufreq/amd-pstate: Add dynamic energy performance preference

Quoted 97 lines · Mario Limonciello
Quoted 87 lines · Dhananjay Ugwekar
Quoted 30 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Dynamic energy performance preference will change the EPP profile based on whether the machine is running on AC or DC power.

A notification chain from the power supply core is used to adjust EPP values on plug in or plug out events.

For non-server systems:
     * the default EPP for AC mode is performance.
     * the default EPP for DC mode is balance_performance.

---
v3->v4:
  * Handle Kconfig not being set
  * Fix dynamic epp default on server
v2-v3:
  * Fix typo in Kconfig
v1->v2:
  * Change defaults to performance (AC) and balance_performance (DC)
  * Default Kconfig to disabled for now
  * Rebase on latest branch
---
  Documentation/admin-guide/pm/amd-pstate.rst |  18 ++- drivers/cpufreq/Kconfig.x86                 |  12 ++ drivers/cpufreq/amd-pstate.c                | 135 +++++++++++++++++++- drivers/cpufreq/amd-pstate.h                |   5 +-

  4 files changed, 161 insertions(+), 9 deletions(-)

[snip]

Quoted 8 lines
drivers/cpufreq/amd-pstate.c +4 -0
@@ -1050,6 +1056,73 @@ static void amd_pstate_cpu_exit(struct cpufreq_policy *policy)
       kfree(cpudata);
   }
   +static int amd_pstate_get_balanced_epp(struct cpufreq_policy *policy)
+{
+    struct amd_cpudata *cpudata = policy->driver_data;
+
+    if (power_supply_is_system_supplied())

drivers/cpufreq/amd-pstate.c static void amd_pstate_cpu_exit(struct cpufreq_policy *policy) · line 1072

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+        return cpudata->epp_default_ac;
+    else
+        return cpudata->epp_default_dc;
+}
+
+static int amd_pstate_power_supply_notifier(struct notifier_block *nb,
+                        unsigned long event, void *data)
+{
+    struct amd_cpudata *cpudata = container_of(nb, struct amd_cpudata, power_nb);
+    struct cpufreq_policy *policy __free(put_cpufreq_policy) = cpufreq_cpu_get(cpudata->cpu);

For consistency, we should add "if (!policy)" check I think

Quoted 14 lines
drivers/cpufreq/amd-pstate.c +14 -0
+    u8 epp;
+    int ret;
+
+    if (event != PSY_EVENT_PROP_CHANGED)
+        return NOTIFY_OK;
+
+    epp = amd_pstate_get_balanced_epp(policy);
+
+    ret = amd_pstate_set_epp(policy, epp);
+    if (ret)
+        pr_warn("Failed to set CPU %d EPP %u: %d\n", cpudata->cpu, epp, ret);
+
+    return NOTIFY_OK;
+}

[snip]

Quoted 19 lines
drivers/cpufreq/amd-pstate.c +15 -0
@@ -1364,6 +1444,32 @@ static ssize_t prefcore_show(struct device *dev,
       return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
   }
   +static ssize_t dynamic_epp_show(struct device *dev,
+                struct device_attribute *attr, char *buf)
+{
+    return sysfs_emit(buf, "%s\n", str_enabled_disabled(dynamic_epp));
+}
+
+static ssize_t dynamic_epp_store(struct device *a, struct device_attribute *b,
+                 const char *buf, size_t count)
+{
+    bool enabled;
+    int ret;
+
+    ret = kstrtobool(buf, &enabled);
+    if (ret)
+        return ret;
+
Quoted 8 lines

drivers/cpufreq/amd-pstate.c static ssize_t prefcore_show(struct device *dev, · line 1468

Quoted 6 lines
drivers/cpufreq/amd-pstate.c +6 -0
+    if (dynamic_epp == enabled)
+        return -EINVAL;
+
+    /* reinitialize with desired dynamic EPP value */
+    dynamic_epp = enabled;
+    ret = amd_pstate_change_driver_mode(cppc_state);

I think implicitly changing the driver mode when we write to dynamic_epp file might lead to some confusions.

Quoted 1 line

How about only allowing to write dynamic_epp attribute when in active mode already?

Yes, I think we should allow, dynamic_epp only with "active mode + powersave governor". And when user tries to enable dynamic_epp in the wrong combination, we should fail, right?

Quoted 9 lines
Quoted 8 lines
Quoted 8 lines
drivers/cpufreq/amd-pstate.c +4 -0
+
+    return ret ? ret : count;
+}
+
   cpufreq_freq_attr_ro(amd_pstate_max_freq);
   cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq);
#7 lore raw

PATCH v4 2/5 cpufreq/amd-pstate: add kernel command line to override dynamic epp

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

Add amd_dynamic_epp=enable and amd_dynamic_epp=disable to override the kernel configuration option CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP locally.

---
 Documentation/admin-guide/kernel-parameters.txt |  7 +++++++
 Documentation/admin-guide/pm/amd-pstate.rst     |  7 +++++++
 drivers/cpufreq/amd-pstate.c                    | 11 +++++++++++
 3 files changed, 25 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +7 -0
index fb8752b42ec85..1afe6d8ab09bb 100644
@@ -392,6 +392,13 @@
 			disable
 			  Disable amd-pstate preferred core.
 
+	amd_dynamic_epp=
+			[X86]
+			disable
+			  Disable amd-pstate dynamic EPP.
+			enable
+			  Enable amd-pstate dynamic EPP.
+
 	amijoy.map=	[HW,JOY] Amiga joystick support
 			Map of devices attached to JOY0DAT and JOY1DAT
 			Format: <a>,<b>
Documentation/admin-guide/pm/amd-pstate.rst +7 -0
index 2e076650dc77c..8424e7119dd7e 100644
@@ -438,6 +438,13 @@ For systems that support ``amd-pstate`` preferred core, the core rankings will
 always be advertised by the platform. But OS can choose to ignore that via the
 kernel parameter ``amd_prefcore=disable``.
 
+``amd_dynamic_epp``
+
+When AMD pstate is in auto mode, dynamic EPP will control whether the kernel
+autonomously changes the EPP mode. The default is configured by
+``CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP`` but can be explicitly enabled with
+``amd_pstate_epp=enable`` or disabled with ``amd_pstate_epp=disable``.
+
 User Space Interface in ``sysfs`` - General
 ===========================================
 
drivers/cpufreq/amd-pstate.c +11 -0
index 7f203495f60e3..8172bd4b5952f 100644
@@ -1934,8 +1934,19 @@ static int __init amd_prefcore_param(char *str)
 	return 0;
 }
 
+static int __init amd_dynamic_epp_param(char *str)
+{
+	if (!strcmp(str, "disable"))
+		dynamic_epp = false;
+	if (!strcmp(str, "enable"))
+		dynamic_epp = true;
+
+	return 0;
+}
+
 early_param("amd_pstate", amd_pstate_param);
 early_param("amd_prefcore", amd_prefcore_param);
+early_param("amd_dynamic_epp", amd_dynamic_epp_param);
 
 MODULE_AUTHOR("Huang Rui <[email protected]>");
 MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
Signature · 1 line
2.43.0
Reviewed-by on v4 2/5
#8 lore raw

Re: [PATCH v4 2/5] cpufreq/amd-pstate: add kernel command line to override dynamic epp

Quoted 6 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Add amd_dynamic_epp=enable and amd_dynamic_epp=disable to override the kernel configuration option CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP locally.

Small correction below, apart from that LGTM

Quoted 30 lines
---
 Documentation/admin-guide/kernel-parameters.txt |  7 +++++++
 Documentation/admin-guide/pm/amd-pstate.rst     |  7 +++++++
 drivers/cpufreq/amd-pstate.c                    | 11 +++++++++++
 3 files changed, 25 insertions(+)
Documentation/admin-guide/kernel-parameters.txt +7 -0
index fb8752b42ec85..1afe6d8ab09bb 100644
@@ -392,6 +392,13 @@
 			disable
 			  Disable amd-pstate preferred core.
 
+	amd_dynamic_epp=
+			[X86]
+			disable
+			  Disable amd-pstate dynamic EPP.
+			enable
+			  Enable amd-pstate dynamic EPP.
+
 	amijoy.map=	[HW,JOY] Amiga joystick support
 			Map of devices attached to JOY0DAT and JOY1DAT
 			Format: <a>,<b>
Documentation/admin-guide/pm/amd-pstate.rst +0 -0
index 2e076650dc77c..8424e7119dd7e 100644

Documentation/admin-guide/pm/amd-pstate.rst For systems that support ``amd-pstate`` preferred core, the core rankings will · line 446

Quoted 9 lines
Documentation/admin-guide/pm/amd-pstate.rst +6 -0
@@ -438,6 +438,13 @@ For systems that support ``amd-pstate`` preferred core, the core rankings will
 always be advertised by the platform. But OS can choose to ignore that via the
 kernel parameter ``amd_prefcore=disable``.
 
+``amd_dynamic_epp``
+
+When AMD pstate is in auto mode, dynamic EPP will control whether the kernel
+autonomously changes the EPP mode. The default is configured by
+``CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP`` but can be explicitly enabled with
+``amd_pstate_epp=enable`` or disabled with ``amd_pstate_epp=disable``.

/s/amd_pstate_epp/amd_dynamic_epp/

Quoted 28 lines
Documentation/admin-guide/pm/amd-pstate.rst +1 -0
+
 User Space Interface in ``sysfs`` - General
 ===========================================
 
drivers/cpufreq/amd-pstate.c +11 -0
index 7f203495f60e3..8172bd4b5952f 100644
@@ -1934,8 +1934,19 @@ static int __init amd_prefcore_param(char *str)
 	return 0;
 }
 
+static int __init amd_dynamic_epp_param(char *str)
+{
+	if (!strcmp(str, "disable"))
+		dynamic_epp = false;
+	if (!strcmp(str, "enable"))
+		dynamic_epp = true;
+
+	return 0;
+}
+
 early_param("amd_pstate", amd_pstate_param);
 early_param("amd_prefcore", amd_prefcore_param);
+early_param("amd_dynamic_epp", amd_dynamic_epp_param);
 
 MODULE_AUTHOR("Huang Rui <[email protected]>");
 MODULE_DESCRIPTION("AMD Processor P-state Frequency Driver");
#9 lore raw

PATCH v4 3/5 cpufreq/amd-pstate: Add support for platform profile class

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

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.

---
 Documentation/admin-guide/pm/amd-pstate.rst |   4 +-
 drivers/cpufreq/Kconfig.x86                 |   1 +
 drivers/cpufreq/amd-pstate.c                | 142 +++++++++++++++++---
 drivers/cpufreq/amd-pstate.h                |  10 ++
 4 files changed, 140 insertions(+), 17 deletions(-)
Documentation/admin-guide/pm/amd-pstate.rst +3 -1
index 8424e7119dd7e..36950fb6568c0 100644
@@ -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.
 
drivers/cpufreq/Kconfig.x86 +1 -0
index 3a8bdc35f488a..8fc8319861bdf 100644
@@ -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
drivers/cpufreq/amd-pstate.c +126 -16

229 lines not on this page · raw message

drivers/cpufreq/amd-pstate.h +10 -0
index 6882876f895de..b4c5374762110 100644
@@ -9,6 +9,7 @@
 #define _LINUX_AMD_PSTATE_H
 
 #include <linux/pm_qos.h>
+#include <linux/platform_profile.h>
 
 /*********************************************************************
  *                        AMD P-state INTERFACE                       *
@@ -108,6 +109,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;
 };
 
 /*
@@ -123,5 +129,9 @@ enum amd_pstate_mode {
 };
 const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
 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 */
Signature · 1 line
2.43.0
#10 lore raw

Re: [PATCH v4 3/5] cpufreq/amd-pstate: Add support for platform profile class

Quoted 54 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

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.

---
 Documentation/admin-guide/pm/amd-pstate.rst |   4 +-
 drivers/cpufreq/Kconfig.x86                 |   1 +
 drivers/cpufreq/amd-pstate.c                | 142 +++++++++++++++++---
 drivers/cpufreq/amd-pstate.h                |  10 ++
 4 files changed, 140 insertions(+), 17 deletions(-)
Documentation/admin-guide/pm/amd-pstate.rst +3 -1
index 8424e7119dd7e..36950fb6568c0 100644
@@ -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.
 
drivers/cpufreq/Kconfig.x86 +1 -0
index 3a8bdc35f488a..8fc8319861bdf 100644
@@ -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
drivers/cpufreq/amd-pstate.c +0 -0
index 8172bd4b5952f..2a62b12148544 100644

drivers/cpufreq/amd-pstate.c static struct quirk_entry *quirks; · line 112

Quoted 4 lines
drivers/cpufreq/amd-pstate.c +1 -0
@@ -109,6 +109,7 @@ static struct quirk_entry *quirks;
  *	2		balance_performance
  *	3		balance_power
  *	4		power
+ *	5		custom (for raw EPP values)

Should we move these "custom" changes to the raw EPP patch ?

Quoted 50 lines
drivers/cpufreq/amd-pstate.c +21 -1
  */
 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,
 };
 
 static const char * const energy_perf_strings[] = {
@@ -124,6 +126,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",
 	NULL
 };
 
@@ -1077,6 +1080,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);
@@ -1085,14 +1092,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);
+

drivers/cpufreq/amd-pstate.c static int amd_pstate_power_supply_notifier(struct notifier_block *nb, · line 1119

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +10 -0
+	*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);

We should add "if (!policy)" check here?

Quoted 33 lines
drivers/cpufreq/amd-pstate.c +33 -0
+	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;
+}
+

[snip]

Quoted 19 lines
drivers/cpufreq/amd-pstate.c +7 -8
@@ -1228,16 +1329,22 @@ static ssize_t store_energy_performance_preference(
 	if (ret != 1)
 		return -EINVAL;
 
-	ret = match_string(energy_perf_strings, -1, str_preference);
-	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(str_preference, 0, &epp);
+	if (ret) {
+		ret = match_string(energy_perf_strings, -1, str_preference);

drivers/cpufreq/amd-pstate.c static ssize_t store_energy_performance_preference( · line 1347

Quoted 10 lines
drivers/cpufreq/amd-pstate.c +8 -1
+		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) {

^^^^^^^ this change wont make any difference right?

Quoted 30 lines
drivers/cpufreq/amd-pstate.c +4 -3
 		pr_debug("EPP cannot be set under performance policy\n");
 		return -EBUSY;
 	}
@@ -1248,9 +1355,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;
@@ -1271,11 +1378,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)
 {

drivers/cpufreq/amd-pstate.c static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy) · line 1709

Quoted 3 lines
drivers/cpufreq/amd-pstate.c +0 -0
@@ -1599,10 +1707,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);

Why dont we just set the defaults to "AMD_CPPC_EPP_BALANCE_PERFORMANCE" for server systems, so that the governor, profile and the EPP values are aligned at initialization. Any historical context for using amd_pstate_get_epp() to init the default epp value?, do we want to preserve the initial value set by the SMU?

Quoted 43 lines
drivers/cpufreq/amd-pstate.c +2 -0
+		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 (cpu_feature_enabled(X86_FEATURE_CPPC)) {
drivers/cpufreq/amd-pstate.h +10 -0
index 6882876f895de..b4c5374762110 100644
@@ -9,6 +9,7 @@
 #define _LINUX_AMD_PSTATE_H
 
 #include <linux/pm_qos.h>
+#include <linux/platform_profile.h>
 
 /*********************************************************************
  *                        AMD P-state INTERFACE                       *
@@ -108,6 +109,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;
 };
 
 /*
@@ -123,5 +129,9 @@ enum amd_pstate_mode {
 };
 const char *amd_pstate_get_mode_string(enum amd_pstate_mode mode);
 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 */
#11 lore raw

PATCH v4 4/5 cpufreq/amd-pstate: Add support for raw EPP writes

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

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.

---
 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(-)
Documentation/admin-guide/pm/amd-pstate.rst +11 -5
index 36950fb6568c0..0e4d2e0aaeff7 100644
@@ -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``
drivers/cpufreq/amd-pstate.c +9 -2
index 2a62b12148544..b0de50f390e07 100644
@@ -1318,6 +1318,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 	struct amd_cpudata *cpudata = policy->driver_data;
 	char str_preference[21];
 	ssize_t ret;
+	bool raw_epp = FALSE;
 	u8 epp;
 
 	if (cpudata->dynamic_epp) {
@@ -1334,6 +1335,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 	 * matches an index in the energy_perf_strings array
 	 */
 	ret = kstrtou8(str_preference, 0, &epp);
+	raw_epp = !ret;
 	if (ret) {
 		ret = match_string(energy_perf_strings, -1, str_preference);
 		if (ret < 0 || ret == EPP_INDEX_CUSTOM)
@@ -1353,7 +1355,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);
 
@@ -1364,6 +1368,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;
@@ -1378,7 +1385,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]);
drivers/cpufreq/amd-pstate.h +1 -0
index b4c5374762110..b6be2b8fbffbf 100644
@@ -108,6 +108,7 @@ struct amd_cpudata {
 	u8	epp_default_ac;
 	u8	epp_default_dc;
 	bool	dynamic_epp;
+	bool	raw_epp;
 	struct notifier_block power_nb;
 
 	/* platform profile */
Signature · 1 line
2.43.0
Reviewed-by on v4 4/5
#12 lore raw

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

Quoted 5 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

The energy performance preference field of the CPPC request MSR supports values from 0 to 255, but the strings only offer 4 values.

Quoted 7 lines

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.

LGTM

Quoted 102 lines
---
 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(-)
Documentation/admin-guide/pm/amd-pstate.rst +11 -5
index 36950fb6568c0..0e4d2e0aaeff7 100644
@@ -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``
drivers/cpufreq/amd-pstate.c +9 -2
index 2a62b12148544..b0de50f390e07 100644
@@ -1318,6 +1318,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 	struct amd_cpudata *cpudata = policy->driver_data;
 	char str_preference[21];
 	ssize_t ret;
+	bool raw_epp = FALSE;
 	u8 epp;
 
 	if (cpudata->dynamic_epp) {
@@ -1334,6 +1335,7 @@ ssize_t store_energy_performance_preference(struct cpufreq_policy *policy,
 	 * matches an index in the energy_perf_strings array
 	 */
 	ret = kstrtou8(str_preference, 0, &epp);
+	raw_epp = !ret;
 	if (ret) {
 		ret = match_string(energy_perf_strings, -1, str_preference);
 		if (ret < 0 || ret == EPP_INDEX_CUSTOM)
@@ -1353,7 +1355,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);
 
@@ -1364,6 +1368,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;
@@ -1378,7 +1385,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]);
drivers/cpufreq/amd-pstate.h +1 -0
index b4c5374762110..b6be2b8fbffbf 100644
@@ -108,6 +108,7 @@ struct amd_cpudata {
 	u8	epp_default_ac;
 	u8	epp_default_dc;
 	bool	dynamic_epp;
+	bool	raw_epp;
 	struct notifier_block power_nb;
 
 	/* platform profile */
#13 lore raw

PATCH v4 5/5 cpufreq/amd-pstate-ut: Add a unit test for raw EPP

In reply to Mario Limonciello

From: Mario Limonciello <[email protected]>

Ensure that all supported raw EPP values work properly.

---
 drivers/cpufreq/amd-pstate-ut.c | 58 +++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
drivers/cpufreq/amd-pstate-ut.c +58 -0
index e671bc7d15508..d0c5c0aa3cc94 100644
@@ -26,6 +26,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/mm.h>
 #include <linux/fs.h>
 #include <linux/cleanup.h>
 
@@ -33,6 +34,7 @@
 
 #include "amd-pstate.h"
 
+DEFINE_FREE(free_page, void *, if (_T) free_page((unsigned long)_T))
 
 struct amd_pstate_ut_struct {
 	const char *name;
@@ -46,6 +48,7 @@ static int amd_pstate_ut_acpi_cpc_valid(u32 index);
 static int amd_pstate_ut_check_enabled(u32 index);
 static int amd_pstate_ut_check_perf(u32 index);
 static int amd_pstate_ut_check_freq(u32 index);
+static int amd_pstate_ut_epp(u32 index);
 static int amd_pstate_ut_check_driver(u32 index);
 
 static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
@@ -53,6 +56,7 @@ static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
 	{"amd_pstate_ut_check_enabled",    amd_pstate_ut_check_enabled    },
 	{"amd_pstate_ut_check_perf",       amd_pstate_ut_check_perf       },
 	{"amd_pstate_ut_check_freq",       amd_pstate_ut_check_freq       },
+	{"amd_pstate_ut_epp",              amd_pstate_ut_epp              },
 	{"amd_pstate_ut_check_driver",	   amd_pstate_ut_check_driver     }
 };
 
@@ -239,6 +243,60 @@ static int amd_pstate_set_mode(enum amd_pstate_mode mode)
 	return amd_pstate_update_status(mode_str, strlen(mode_str));
 }
 
+static int amd_pstate_ut_epp(u32 index)
+{
+	struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
+	void *buf __free(free_page) = NULL;
+	struct amd_cpudata *cpudata;
+	int ret, cpu = 0;
+	u16 epp;
+
+	policy = cpufreq_cpu_get(cpu);
+	if (!policy)
+		return -ENODEV;
+
+	cpudata = policy->driver_data;
+
+	/* disable dynamic EPP before running test */
+	if (cpudata->dynamic_epp) {
+		pr_debug("Dynamic EPP is enabled, disabling it\n");
+		amd_pstate_clear_dynamic_epp(policy);
+	}
+
+	buf = (void *)__get_free_page(GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = amd_pstate_set_mode(AMD_PSTATE_ACTIVE);
+	if (ret)
+		return ret;
+
+	for (epp = 0; epp <= U8_MAX; epp++) {
+		u8 val;
+
+		/* write all EPP values */
+		memset(buf, 0, sizeof(*buf));
+		snprintf(buf, PAGE_SIZE, "%d", epp);
+		ret = store_energy_performance_preference(policy, buf, sizeof(*buf));
+		if (ret < 0)
+			return ret;
+
+		/* check if the EPP value reads back correctly for raw numbers */
+		memset(buf, 0, sizeof(*buf));
+		ret = show_energy_performance_preference(policy, buf);
+		if (ret < 0)
+			return ret;
+		strreplace(buf, '\n', '\0');
+		ret = kstrtou8(buf, 0, &val);
+		if (!ret && epp != val) {
+			pr_err("Raw EPP value mismatch: %d != %d\n", epp, val);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int amd_pstate_ut_check_driver(u32 index)
 {
 	enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE;
Signature · 1 line
2.43.0
#14 lore raw

Re: [PATCH v4 5/5] cpufreq/amd-pstate-ut: Add a unit test for raw EPP

Quoted 64 lines · Mario Limonciello

From: Mario Limonciello <[email protected]>

Ensure that all supported raw EPP values work properly.

---
 drivers/cpufreq/amd-pstate-ut.c | 58 +++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
drivers/cpufreq/amd-pstate-ut.c +19 -0
index e671bc7d15508..d0c5c0aa3cc94 100644
@@ -26,6 +26,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/mm.h>
 #include <linux/fs.h>
 #include <linux/cleanup.h>
 
@@ -33,6 +34,7 @@
 
 #include "amd-pstate.h"
 
+DEFINE_FREE(free_page, void *, if (_T) free_page((unsigned long)_T))
 
 struct amd_pstate_ut_struct {
 	const char *name;
@@ -46,6 +48,7 @@ static int amd_pstate_ut_acpi_cpc_valid(u32 index);
 static int amd_pstate_ut_check_enabled(u32 index);
 static int amd_pstate_ut_check_perf(u32 index);
 static int amd_pstate_ut_check_freq(u32 index);
+static int amd_pstate_ut_epp(u32 index);
 static int amd_pstate_ut_check_driver(u32 index);
 
 static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
@@ -53,6 +56,7 @@ static struct amd_pstate_ut_struct amd_pstate_ut_cases[] = {
 	{"amd_pstate_ut_check_enabled",    amd_pstate_ut_check_enabled    },
 	{"amd_pstate_ut_check_perf",       amd_pstate_ut_check_perf       },
 	{"amd_pstate_ut_check_freq",       amd_pstate_ut_check_freq       },
+	{"amd_pstate_ut_epp",              amd_pstate_ut_epp              },
 	{"amd_pstate_ut_check_driver",	   amd_pstate_ut_check_driver     }
 };
 
@@ -239,6 +243,60 @@ static int amd_pstate_set_mode(enum amd_pstate_mode mode)
 	return amd_pstate_update_status(mode_str, strlen(mode_str));
 }
 
+static int amd_pstate_ut_epp(u32 index)
+{
+	struct cpufreq_policy *policy __free(put_cpufreq_policy) = NULL;
+	void *buf __free(free_page) = NULL;
+	struct amd_cpudata *cpudata;
+	int ret, cpu = 0;
+	u16 epp;
+
+	policy = cpufreq_cpu_get(cpu);
+	if (!policy)
+		return -ENODEV;
+
+	cpudata = policy->driver_data;
+
+	/* disable dynamic EPP before running test */

drivers/cpufreq/amd-pstate-ut.c static int amd_pstate_set_mode(enum amd_pstate_mode mode) · line 270

Quoted 10 lines
drivers/cpufreq/amd-pstate-ut.c +10 -0
+	if (cpudata->dynamic_epp) {
+		pr_debug("Dynamic EPP is enabled, disabling it\n");
+		amd_pstate_clear_dynamic_epp(policy);
+	}
+
+	buf = (void *)__get_free_page(GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	ret = amd_pstate_set_mode(AMD_PSTATE_ACTIVE);

This causes a deadlock, as we are holding cpufreq_policy reference and calling set_mode(), which needs to unregister the cpufreq subsystem (i.e. needs all cpufreq_policy references freed).

We might need to drop and retake the reference around set_mode(). Also make cpudata variable NULL to avoid anyone using a stale cpudata pointer later on by mistake.
OR
Directly call set_epp() instead of store_energy_performance_preference() and eliminate the need of policy ref after set_mode().

Quoted 32 lines
drivers/cpufreq/amd-pstate-ut.c +29 -0
+	if (ret)
+		return ret;
+
+	for (epp = 0; epp <= U8_MAX; epp++) {
+		u8 val;
+
+		/* write all EPP values */
+		memset(buf, 0, sizeof(*buf));
+		snprintf(buf, PAGE_SIZE, "%d", epp);
+		ret = store_energy_performance_preference(policy, buf, sizeof(*buf));
+		if (ret < 0)
+			return ret;
+
+		/* check if the EPP value reads back correctly for raw numbers */
+		memset(buf, 0, sizeof(*buf));
+		ret = show_energy_performance_preference(policy, buf);
+		if (ret < 0)
+			return ret;
+		strreplace(buf, '\n', '\0');
+		ret = kstrtou8(buf, 0, &val);
+		if (!ret && epp != val) {
+			pr_err("Raw EPP value mismatch: %d != %d\n", epp, val);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static int amd_pstate_ut_check_driver(u32 index)
 {
 	enum amd_pstate_mode mode1, mode2 = AMD_PSTATE_DISABLE;