| Message ID | 20250304152327.1561017-1-superm1@kernel.org |
|---|---|
| Headers | show |
| Series | amd-pstate Dynamic EPP and raw EPP | expand |
Hello Mario, On Tue, Mar 04, 2025 at 09:23:23AM -0600, Mario Limonciello wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > 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`. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > 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 | 129 ++++++++++++++++++-- > drivers/cpufreq/amd-pstate.h | 5 +- > 4 files changed, 155 insertions(+), 9 deletions(-) > > diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst > index 412423c54f258..2e076650dc77c 100644 > --- a/Documentation/admin-guide/pm/amd-pstate.rst > +++ b/Documentation/admin-guide/pm/amd-pstate.rst [...snip..] > @@ -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`` > ====================================== > diff --git a/drivers/cpufreq/Kconfig.x86 b/drivers/cpufreq/Kconfig.x86 > index 97c2d4f15d76e..c5ef92634ddf4 100644 > --- a/drivers/cpufreq/Kconfig.x86 > +++ b/drivers/cpufreq/Kconfig.x86 > @@ -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 kenrel to dynamically change the energy performance ^^^^^^ kernel > + 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 > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index e98ef41083ba1..f00fb4ba9f26e 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -36,6 +36,7 @@ > #include <linux/io.h> > #include <linux/delay.h> > #include <linux/uaccess.h> > +#include <linux/power_supply.h> > #include <linux/static_call.h> > #include <linux/topology.h> > > @@ -86,6 +87,7 @@ static struct cpufreq_driver amd_pstate_driver; > static struct cpufreq_driver amd_pstate_epp_driver; > static int cppc_state = AMD_PSTATE_UNDEFINED; > static bool amd_pstate_prefcore = true; > +static bool dynamic_epp = CONFIG_X86_AMD_PSTATE_DYNAMIC_EPP; > static struct quirk_entry *quirks; > > /* > @@ -1050,6 +1052,73 @@ static void amd_pstate_cpu_exit(struct cpufreq_policy *policy) > kfree(cpudata); > } > > +static int amd_pstate_get_balanced_epp(struct cpufreq_policy *policy) Can we rename this function as amd_pstate_get_dynamic_epp() ? Because the values being returned may not be balance_performance/balance_power. > +{ > + struct amd_cpudata *cpudata = policy->driver_data; > + > + if (power_supply_is_system_supplied()) > + 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); > + 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; > +} > +static 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); > + cpudata->dynamic_epp = false; > +} > + > +static int amd_pstate_set_dynamic_epp(struct cpufreq_policy *policy) > +{ > + struct amd_cpudata *cpudata = policy->driver_data; > + int ret; > + u8 epp; > + > + epp = amd_pstate_get_balanced_epp(policy); > + ret = amd_pstate_set_epp(policy, epp); > + if (ret) > + return ret; > + > + /* 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); > + if (ret) > + goto cleanup; > + cpudata->power_nb.notifier_call = amd_pstate_power_supply_notifier; > + } > + > + cpudata->dynamic_epp = true; > + > + return 0; > + > +cleanup: > + amd_pstate_clear_dynamic_epp(policy); > + > + return ret; > +} > + > /* Sysfs attributes */ > > /* > @@ -1146,6 +1215,11 @@ static ssize_t store_energy_performance_preference( > ssize_t ret; > u8 epp; > > + if (cpudata->dynamic_epp) { > + pr_debug("EPP cannot be set when dynamic EPP is enabled\n"); > + return -EBUSY; > + } > + > ret = sscanf(buf, "%20s", str_preference); > if (ret != 1) > return -EINVAL; > @@ -1154,10 +1228,10 @@ static ssize_t store_energy_performance_preference( > if (ret < 0) > return -EINVAL; > > - if (!ret) > - epp = cpudata->epp_default; > - else > + if (ret) > epp = epp_values[ret]; > + else > + epp = amd_pstate_get_balanced_epp(policy); > > if (epp > 0 && policy->policy == CPUFREQ_POLICY_PERFORMANCE) { > pr_debug("EPP cannot be set under performance policy\n"); > @@ -1165,6 +1239,8 @@ static ssize_t store_energy_performance_preference( > } > > ret = amd_pstate_set_epp(policy, epp); > + if (ret) > + return ret; Isn't this unecessary given that the next line is doing exactly this ? > > return ret ? ret : count; > } Rest of the patch looks good to me.
On Tue, Mar 04, 2025 at 09:23:25AM -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. > > 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 | 142 +++++++++++++++++--- > drivers/cpufreq/amd-pstate.h | 10 ++ > 4 files changed, 140 insertions(+), 17 deletions(-) > > diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst > index 8424e7119dd7e..36950fb6568c0 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 c5ef92634ddf4..905eab998b836 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 9911808fe0bcf..28c02edf6e40b 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -105,6 +105,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, > @@ -112,6 +113,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[] = { > @@ -120,6 +122,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 > }; > > @@ -1073,6 +1076,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); > @@ -1081,14 +1088,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; So prior to the patch, cpudata->policy is supposed to mirror policy->policy. With this patch, this assumption is no longer true. So it is possible for the user to again override the choice of EPP set via platform profile by changing the cpufreq governor ? Is this the expected behaviour? The bigger concern is, if the governor was previously "performance" and then the platform profile requested "low power", "cat /sys/devices/system/cpu/cpuX/cpufreq/scaling_governor" would still show "performance", which is inconsistent with the behaviour.
On 3/7/2025 10:22, Gautham R. Shenoy wrote: > On Tue, Mar 04, 2025 at 09:23:25AM -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. >> >> 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 | 142 +++++++++++++++++--- >> drivers/cpufreq/amd-pstate.h | 10 ++ >> 4 files changed, 140 insertions(+), 17 deletions(-) >> >> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/Documentation/admin-guide/pm/amd-pstate.rst >> index 8424e7119dd7e..36950fb6568c0 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 c5ef92634ddf4..905eab998b836 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 9911808fe0bcf..28c02edf6e40b 100644 >> --- a/drivers/cpufreq/amd-pstate.c >> +++ b/drivers/cpufreq/amd-pstate.c >> @@ -105,6 +105,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, >> @@ -112,6 +113,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[] = { >> @@ -120,6 +122,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 >> }; >> >> @@ -1073,6 +1076,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); >> @@ -1081,14 +1088,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; > > So prior to the patch, cpudata->policy is supposed to mirror > policy->policy. With this patch, this assumption is no longer > true. So it is possible for the user to again override the choice of > EPP set via platform profile by changing the cpufreq governor ? > > Is this the expected behaviour? > > The bigger concern is, if the governor was previously "performance" > and then the platform profile requested "low power", "cat > /sys/devices/system/cpu/cpuX/cpufreq/scaling_governor" would still > show "performance", which is inconsistent with the behaviour. > > This ties back to the previous patches for dynamic EPP. My expectation was that when dynamic EPP is enabled that users can't manually set the EPP anymore (it will return -EBUSY) and likewise turning on dynamic EPP should keep the governor as powersave. I'll double check all those are properly enforced; but that's at least the intent. IMO this "should" all work because turning on Dynamic EPP sysfs file forces the driver to go through a state transition that it will tear everything down and back up. The policy will come back up in "powersave" even if it was previously in "performance" when the dynamic EPP sysfs file was turned on. Longer term; I also envision the scheduler influencing EPP values when dynamic_epp is turned on. The "platform profile" would be an "input" to that decision making process (maybe giving a weighting?), not the only lever. I haven't given any serious look at how to do this with the scheduler, I wanted to lay the foundation first being dynamic EPP and raw EPP. So even if dynamic_epp isn't interesting "right now" for server because the focus is around behavior for AC/DC, don't write it off just yet.
On 3/7/2025 10:55, Mario Limonciello wrote: > On 3/7/2025 10:22, Gautham R. Shenoy wrote: >> On Tue, Mar 04, 2025 at 09:23:25AM -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. >>> >>> 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 | 142 +++++++++++++++++--- >>> drivers/cpufreq/amd-pstate.h | 10 ++ >>> 4 files changed, 140 insertions(+), 17 deletions(-) >>> >>> diff --git a/Documentation/admin-guide/pm/amd-pstate.rst b/ >>> Documentation/admin-guide/pm/amd-pstate.rst >>> index 8424e7119dd7e..36950fb6568c0 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 c5ef92634ddf4..905eab998b836 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 9911808fe0bcf..28c02edf6e40b 100644 >>> --- a/drivers/cpufreq/amd-pstate.c >>> +++ b/drivers/cpufreq/amd-pstate.c >>> @@ -105,6 +105,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, >>> @@ -112,6 +113,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[] = { >>> @@ -120,6 +122,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 >>> }; >>> @@ -1073,6 +1076,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); >>> @@ -1081,14 +1088,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; >> >> So prior to the patch, cpudata->policy is supposed to mirror >> policy->policy. With this patch, this assumption is no longer >> true. So it is possible for the user to again override the choice of >> EPP set via platform profile by changing the cpufreq governor ? >> >> Is this the expected behaviour? >> >> The bigger concern is, if the governor was previously "performance" >> and then the platform profile requested "low power", "cat >> /sys/devices/system/cpu/cpuX/cpufreq/scaling_governor" would still >> show "performance", which is inconsistent with the behaviour. >> >> > > This ties back to the previous patches for dynamic EPP. My expectation > was that when dynamic EPP is enabled that users can't manually set the > EPP anymore (it will return -EBUSY) and likewise turning on dynamic EPP > should keep the governor as powersave. > > I'll double check all those are properly enforced; but that's at least > the intent. FWIW - I double checked and confirmed that this is working as intended. * I couldn't change from powersave to performance when dynamic_epp was enabled (-EBUSY) * I couldn't change energy_performance_preference when dynamic_epp was enabled (-EBUSY) > > IMO this "should" all work because turning on Dynamic EPP sysfs file > forces the driver to go through a state transition that it will tear > everything down and back up. The policy will come back up in > "powersave" even if it was previously in "performance" when the dynamic > EPP sysfs file was turned on. > > Longer term; I also envision the scheduler influencing EPP values when > dynamic_epp is turned on. The "platform profile" would be an "input" to > that decision making process (maybe giving a weighting?), not the only > lever. > > I haven't given any serious look at how to do this with the scheduler, I > wanted to lay the foundation first being dynamic EPP and raw EPP. > > So even if dynamic_epp isn't interesting "right now" for server because > the focus is around behavior for AC/DC, don't write it off just yet.
From: Mario Limonciello <mario.limonciello@amd.com> 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 v1->v2: * Rebase * Change some defaults 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 | 279 ++++++++++++++++-- drivers/cpufreq/amd-pstate.h | 16 +- 6 files changed, 390 insertions(+), 24 deletions(-)