| Message ID | a6d890f77889555c090db754c9a7b81e91053116.1718787627.git.perry.yuan@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Mario Limonciello |
| Headers | show |
| Series | AMD Pstate Driver Core Performance Boost | expand |
On 6/19/2024 04:16, Perry Yuan wrote: > From: Perry Yuan <Perry.Yuan@amd.com> > > With this new sysfs entry `cpb_boost`created, user can change CPU boost > state dynamically under `active`, `guided` and `passive` modes. > And the highest perf and frequency will also be updated as the boost > state changing. s/changing/changes/ > > 0): check current boost state > cat /sys/devices/system/cpu/amd_pstate/cpb_boost > > 1): disable CPU boost > sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost" > > 2): enable CPU boost > sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost" > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217931 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=217618 > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > --- > drivers/cpufreq/amd-pstate-ut.c | 2 +- > drivers/cpufreq/amd-pstate.c | 112 +++++++++++++++++++++++++++++++- > drivers/cpufreq/amd-pstate.h | 1 + > 3 files changed, 113 insertions(+), 2 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c > index fc275d41d51e..b528f198f4c3 100644 > --- a/drivers/cpufreq/amd-pstate-ut.c > +++ b/drivers/cpufreq/amd-pstate-ut.c > @@ -227,7 +227,7 @@ static void amd_pstate_ut_check_freq(u32 index) > goto skip_test; > } > > - if (cpudata->boost_supported) { > + if (amd_pstate_global_params.cpb_boost) { > if ((policy->max == cpudata->max_freq) || > (policy->max == cpudata->nominal_freq)) > amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 0c50b8ba16b6..1c2320808ae1 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -104,6 +104,7 @@ static bool amd_pstate_prefcore = true; > static struct quirk_entry *quirks; > struct amd_pstate_global_params amd_pstate_global_params; > EXPORT_SYMBOL_GPL(amd_pstate_global_params); > +static int amd_pstate_cpu_boost(int cpu, bool state); > > /* > * AMD Energy Preference Performance (EPP) > @@ -736,6 +737,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata) > if (amd_pstate_global_params.cpb_supported) { > current_pstate_driver->boost_enabled = true; > WRITE_ONCE(cpudata->boost_supported, true); > + WRITE_ONCE(cpudata->boost_state, true); > } > > amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported; > @@ -743,6 +745,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata) > > exit_err: > WRITE_ONCE(cpudata->boost_supported, false); > + WRITE_ONCE(cpudata->boost_state, false); > current_pstate_driver->boost_enabled = false; > amd_pstate_global_params.cpb_boost = false; > return ret; > @@ -1346,6 +1349,111 @@ static ssize_t prefcore_show(struct device *dev, > return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore)); > } > > +static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on) > +{ > + struct amd_cpudata *cpudata = policy->driver_data; > + struct cppc_perf_ctrls perf_ctrls; > + u32 highest_perf, nominal_perf, nominal_freq, max_freq; > + int ret; > + > + highest_perf = READ_ONCE(cpudata->highest_perf); > + nominal_perf = READ_ONCE(cpudata->nominal_perf); > + nominal_freq = READ_ONCE(cpudata->nominal_freq); > + max_freq = READ_ONCE(cpudata->max_freq); > + > + if (boot_cpu_has(X86_FEATURE_CPPC)) { > + u64 value = READ_ONCE(cpudata->cppc_req_cached); > + > + value &= ~GENMASK_ULL(7, 0); > + value |= on ? highest_perf : nominal_perf; > + WRITE_ONCE(cpudata->cppc_req_cached, value); > + > + wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value); > + } else { > + perf_ctrls.max_perf = on ? highest_perf : nominal_perf; > + ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1); > + if (ret) { > + cpufreq_cpu_release(policy); > + pr_debug("failed to set energy perf value (%d)\n", ret); > + return ret; > + } > + } > + > + if (on) > + policy->cpuinfo.max_freq = max_freq; > + else > + policy->cpuinfo.max_freq = nominal_freq * 1000; > + > + policy->max = policy->cpuinfo.max_freq; > + > + if (cppc_state == AMD_PSTATE_PASSIVE) { > + ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq); > + if (ret < 0) > + pr_debug("Failed to update freq constraint: CPU%d\n", cpudata->cpu); > + } > + > + return ret < 0 ? ret : 0; > +} > + > +static int amd_pstate_cpu_boost(int cpu, bool state) > +{ > + int ret; > + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); > + struct amd_cpudata *cpudata = policy->driver_data; > + > + if (!policy) { > + pr_err("policy is NULL\n"); > + ret = -ENODATA; > + goto err_exit; > + } > + > + ret = amd_pstate_cpu_boost_update(policy, state); > + refresh_frequency_limits(policy); > + WRITE_ONCE(cpudata->boost_state, state); > + policy->boost_enabled = state; > + > +err_exit: > + cpufreq_cpu_put(policy); > + return ret < 0 ? ret : 0; > +} > + > +static ssize_t cpb_boost_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sysfs_emit(buf, "%u\n", amd_pstate_global_params.cpb_boost); > +} It is incongruent that this returns a 0/1 but prefcore returns enabled/disabled using str_enabled_disabled(). Can we make this consistent and alos use str_enabled_disabled() please? > + > +static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b, > + const char *buf, size_t count) > +{ > + bool new_state; > + ssize_t ret; > + int cpu; > + > + if (!amd_pstate_global_params.cpb_supported) { > + pr_err("Boost mode is not supported by this processor or SBIOS\n"); > + return -EINVAL; > + } > + > + ret = kstrtobool(buf, &new_state); > + if (ret) > + return ret; > + > + mutex_lock(&amd_pstate_driver_lock); > + for_each_present_cpu(cpu) { > + ret = amd_pstate_cpu_boost(cpu, new_state); > + if (ret < 0) { > + pr_warn("failed to update cpu boost for CPU%d (%zd)\n", cpu, ret); > + goto err_exit; > + } > + } > + amd_pstate_global_params.cpb_boost = !!new_state; > + > +err_exit: > + mutex_unlock(&amd_pstate_driver_lock); > + return ret < 0 ? ret : count; > +} > + > cpufreq_freq_attr_ro(amd_pstate_max_freq); > cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq); > > @@ -1356,6 +1464,7 @@ cpufreq_freq_attr_rw(energy_performance_preference); > cpufreq_freq_attr_ro(energy_performance_available_preferences); > static DEVICE_ATTR_RW(status); > static DEVICE_ATTR_RO(prefcore); > +static DEVICE_ATTR_RW(cpb_boost); > > static struct freq_attr *amd_pstate_attr[] = { > &amd_pstate_max_freq, > @@ -1380,6 +1489,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = { > static struct attribute *pstate_global_attributes[] = { > &dev_attr_status.attr, > &dev_attr_prefcore.attr, > + &dev_attr_cpb_boost.attr, > NULL > }; > > @@ -1418,7 +1528,7 @@ static int amd_pstate_init_boost(struct cpufreq_policy *policy) > if (ret) > return ret; > > - policy->boost_enabled = READ_ONCE(cpudata->boost_supported); > + policy->boost_enabled = READ_ONCE(cpudata->boost_state); > > return 0; > } > diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h > index 133042370a8f..fb240a870289 100644 > --- a/drivers/cpufreq/amd-pstate.h > +++ b/drivers/cpufreq/amd-pstate.h > @@ -100,6 +100,7 @@ struct amd_cpudata { > u64 cppc_cap1_cached; > bool suspended; > s16 epp_default; > + bool boost_state; > }; > > /**
Mario Limonciello <mario.limonciello@amd.com> writes: > On 6/19/2024 04:16, Perry Yuan wrote: >> From: Perry Yuan <Perry.Yuan@amd.com> >> >> With this new sysfs entry `cpb_boost`created, user can change CPU boost >> state dynamically under `active`, `guided` and `passive` modes. >> And the highest perf and frequency will also be updated as the boost >> state changing. > > s/changing/changes/ > >> >> 0): check current boost state >> cat /sys/devices/system/cpu/amd_pstate/cpb_boost >> >> 1): disable CPU boost >> sudo bash -c "echo 0 > /sys/devices/system/cpu/amd_pstate/cpb_boost" >> >> 2): enable CPU boost >> sudo bash -c "echo 1 > /sys/devices/system/cpu/amd_pstate/cpb_boost" From the point of view of the functionality this new interface is no different from /sys/devices/system/cpu/cpufreq/boost, which is currently suppported for the passive mode and the guided mode. Is there anything that prevents us from enabling this interface for the active mode as well? It is easier if we use the common interface because tools such as cpupower will then not need to fiddle with several different sysfs files to enable/disable boost. -- Thanks and Regards gautham.
diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c index fc275d41d51e..b528f198f4c3 100644 --- a/drivers/cpufreq/amd-pstate-ut.c +++ b/drivers/cpufreq/amd-pstate-ut.c @@ -227,7 +227,7 @@ static void amd_pstate_ut_check_freq(u32 index) goto skip_test; } - if (cpudata->boost_supported) { + if (amd_pstate_global_params.cpb_boost) { if ((policy->max == cpudata->max_freq) || (policy->max == cpudata->nominal_freq)) amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS; diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 0c50b8ba16b6..1c2320808ae1 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -104,6 +104,7 @@ static bool amd_pstate_prefcore = true; static struct quirk_entry *quirks; struct amd_pstate_global_params amd_pstate_global_params; EXPORT_SYMBOL_GPL(amd_pstate_global_params); +static int amd_pstate_cpu_boost(int cpu, bool state); /* * AMD Energy Preference Performance (EPP) @@ -736,6 +737,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata) if (amd_pstate_global_params.cpb_supported) { current_pstate_driver->boost_enabled = true; WRITE_ONCE(cpudata->boost_supported, true); + WRITE_ONCE(cpudata->boost_state, true); } amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported; @@ -743,6 +745,7 @@ static int amd_pstate_boost_set(struct amd_cpudata *cpudata) exit_err: WRITE_ONCE(cpudata->boost_supported, false); + WRITE_ONCE(cpudata->boost_state, false); current_pstate_driver->boost_enabled = false; amd_pstate_global_params.cpb_boost = false; return ret; @@ -1346,6 +1349,111 @@ static ssize_t prefcore_show(struct device *dev, return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore)); } +static int amd_pstate_cpu_boost_update(struct cpufreq_policy *policy, bool on) +{ + struct amd_cpudata *cpudata = policy->driver_data; + struct cppc_perf_ctrls perf_ctrls; + u32 highest_perf, nominal_perf, nominal_freq, max_freq; + int ret; + + highest_perf = READ_ONCE(cpudata->highest_perf); + nominal_perf = READ_ONCE(cpudata->nominal_perf); + nominal_freq = READ_ONCE(cpudata->nominal_freq); + max_freq = READ_ONCE(cpudata->max_freq); + + if (boot_cpu_has(X86_FEATURE_CPPC)) { + u64 value = READ_ONCE(cpudata->cppc_req_cached); + + value &= ~GENMASK_ULL(7, 0); + value |= on ? highest_perf : nominal_perf; + WRITE_ONCE(cpudata->cppc_req_cached, value); + + wrmsrl_on_cpu(cpudata->cpu, MSR_AMD_CPPC_REQ, value); + } else { + perf_ctrls.max_perf = on ? highest_perf : nominal_perf; + ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1); + if (ret) { + cpufreq_cpu_release(policy); + pr_debug("failed to set energy perf value (%d)\n", ret); + return ret; + } + } + + if (on) + policy->cpuinfo.max_freq = max_freq; + else + policy->cpuinfo.max_freq = nominal_freq * 1000; + + policy->max = policy->cpuinfo.max_freq; + + if (cppc_state == AMD_PSTATE_PASSIVE) { + ret = freq_qos_update_request(&cpudata->req[1], policy->cpuinfo.max_freq); + if (ret < 0) + pr_debug("Failed to update freq constraint: CPU%d\n", cpudata->cpu); + } + + return ret < 0 ? ret : 0; +} + +static int amd_pstate_cpu_boost(int cpu, bool state) +{ + int ret; + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu); + struct amd_cpudata *cpudata = policy->driver_data; + + if (!policy) { + pr_err("policy is NULL\n"); + ret = -ENODATA; + goto err_exit; + } + + ret = amd_pstate_cpu_boost_update(policy, state); + refresh_frequency_limits(policy); + WRITE_ONCE(cpudata->boost_state, state); + policy->boost_enabled = state; + +err_exit: + cpufreq_cpu_put(policy); + return ret < 0 ? ret : 0; +} + +static ssize_t cpb_boost_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%u\n", amd_pstate_global_params.cpb_boost); +} + +static ssize_t cpb_boost_store(struct device *dev, struct device_attribute *b, + const char *buf, size_t count) +{ + bool new_state; + ssize_t ret; + int cpu; + + if (!amd_pstate_global_params.cpb_supported) { + pr_err("Boost mode is not supported by this processor or SBIOS\n"); + return -EINVAL; + } + + ret = kstrtobool(buf, &new_state); + if (ret) + return ret; + + mutex_lock(&amd_pstate_driver_lock); + for_each_present_cpu(cpu) { + ret = amd_pstate_cpu_boost(cpu, new_state); + if (ret < 0) { + pr_warn("failed to update cpu boost for CPU%d (%zd)\n", cpu, ret); + goto err_exit; + } + } + amd_pstate_global_params.cpb_boost = !!new_state; + +err_exit: + mutex_unlock(&amd_pstate_driver_lock); + return ret < 0 ? ret : count; +} + cpufreq_freq_attr_ro(amd_pstate_max_freq); cpufreq_freq_attr_ro(amd_pstate_lowest_nonlinear_freq); @@ -1356,6 +1464,7 @@ cpufreq_freq_attr_rw(energy_performance_preference); cpufreq_freq_attr_ro(energy_performance_available_preferences); static DEVICE_ATTR_RW(status); static DEVICE_ATTR_RO(prefcore); +static DEVICE_ATTR_RW(cpb_boost); static struct freq_attr *amd_pstate_attr[] = { &amd_pstate_max_freq, @@ -1380,6 +1489,7 @@ static struct freq_attr *amd_pstate_epp_attr[] = { static struct attribute *pstate_global_attributes[] = { &dev_attr_status.attr, &dev_attr_prefcore.attr, + &dev_attr_cpb_boost.attr, NULL }; @@ -1418,7 +1528,7 @@ static int amd_pstate_init_boost(struct cpufreq_policy *policy) if (ret) return ret; - policy->boost_enabled = READ_ONCE(cpudata->boost_supported); + policy->boost_enabled = READ_ONCE(cpudata->boost_state); return 0; } diff --git a/drivers/cpufreq/amd-pstate.h b/drivers/cpufreq/amd-pstate.h index 133042370a8f..fb240a870289 100644 --- a/drivers/cpufreq/amd-pstate.h +++ b/drivers/cpufreq/amd-pstate.h @@ -100,6 +100,7 @@ struct amd_cpudata { u64 cppc_cap1_cached; bool suspended; s16 epp_default; + bool boost_state; }; /**