AMD Pstate Driver Core Performance Boost

There are some other drivers also need to use the MSR_K7_HWCR_CPB_DIS_BIT for CPB control bit, so it makes sense to move the definition to a common header file to allow other driver to use it.

No intentional functional impact.

Suggested-by Gautham Ranjal Shenoy <[email protected]>

Signed-off-by Perry Yuan <[email protected]>

Acked-by Rafael J. Wysocki <[email protected]>

---
 arch/x86/include/asm/msr-index.h | 2 ++
 drivers/cpufreq/acpi-cpufreq.c   | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

arch/x86/include/asm/msr-index.h +2 -0

index e72c2b872957..8738a7b3917d 100644
@@ -782,6 +782,8 @@
 #define MSR_K7_HWCR_IRPERF_EN		BIT_ULL(MSR_K7_HWCR_IRPERF_EN_BIT)
 #define MSR_K7_FID_VID_CTL		0xc0010041
 #define MSR_K7_FID_VID_STATUS		0xc0010042
+#define MSR_K7_HWCR_CPB_DIS_BIT		25
+#define MSR_K7_HWCR_CPB_DIS		BIT_ULL(MSR_K7_HWCR_CPB_DIS_BIT)
 
 /* K6 MSRs */
 #define MSR_K6_WHCR			0xc0000082

drivers/cpufreq/acpi-cpufreq.c +0 -2

index 37f1cdf46d29..2fc82831bddd 100644
@@ -50,8 +50,6 @@ enum {
 #define AMD_MSR_RANGE		(0x7)
 #define HYGON_MSR_RANGE		(0x7)
 
-#define MSR_K7_HWCR_CPB_DIS	(1ULL << 25)
-
 struct acpi_cpufreq_data {
 	unsigned int resume;
 	unsigned int cpu_feature;

Signature · 1 line

2.34.1

From: Perry Yuan <[email protected]>

Add one global global_params to represent CPU Performance Boost(cpb) state for cpu frequency scaling, both active and passive modes all can support CPU cores frequency boosting control which is based on the BIOS setting, while BIOS turn on the "Core Performance Boost", it will allow OS control each core highest perf limitation from OS side.

The active, guided and passive modes of the amd-pstate driver can support frequency boost control when the "Core Performance Boost" (CPB) feature is enabled in the BIOS. When enabled in BIOS, the user has an option at runtime to allow/disallow the cores from operating in the boost frequency range.

Add an amd_pstate_global_params object to record whether CPB is enabled in BIOS, and if it has been activated by the user

Reported-by Artem S. Tashkinov" <[email protected]>

Cc Oleksandr Natalenko <[email protected]>

Closes https://bugzilla.kernel.org/show_bug.cgi?id=217931

Signed-off-by Perry Yuan <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 39 +++++++++++++++++++++++++++---------
 include/linux/amd-pstate.h   | 13 ++++++++++++
 2 files changed, 42 insertions(+), 10 deletions(-)

drivers/cpufreq/amd-pstate.c +5 -3

index 83a29b257794..3d86cd7c9073 100644
@@ -68,6 +68,8 @@ static int cppc_state = AMD_PSTATE_UNDEFINED;
 static bool cppc_enabled;
 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);
 
 /*
  * AMD Energy Preference Performance (EPP)

@@ -665,18 +667,27 @@ static int amd_pstate_set_boost(struct cpufreq_policy *policy, int state)
 	return 0;
 }
 
-static void amd_pstate_boost_init(struct amd_cpudata *cpudata)
+static int amd_pstate_boost_init(struct amd_cpudata *cpudata)
 {
-	u32 highest_perf, nominal_perf;
+	u64 boost_val;
+	int ret;
 
-	highest_perf = READ_ONCE(cpudata->highest_perf);

drivers/cpufreq/amd-pstate.c +18 -4

+
+	if (amd_pstate_global_params.cpb_supported) {
+		cpudata->boost_supported = true;
+		current_pstate_driver->boost_enabled = true;
+	}
 
-	cpudata->boost_supported = true;
-	current_pstate_driver->boost_enabled = true;
+	amd_pstate_global_params.cpb_boost = amd_pstate_global_params.cpb_supported;
+
+	return ret;
 }
 
 static void amd_perf_ctl_reset(unsigned int cpu)

@@ -900,6 +911,11 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	amd_pstate_init_prefcore(cpudata);
 
+	/* initialize cpu cores boot state */
+	ret = amd_pstate_boost_init(cpudata);
+	if (ret)
+		goto free_cpudata1;
+
 	ret = amd_pstate_init_perf(cpudata);
 	if (ret)
 		goto free_cpudata1;

@@ -956,7 +972,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
 
 	policy->driver_data = cpudata;
 
-	amd_pstate_boost_init(cpudata);
 	if (!current_pstate_driver->adjust_perf)
 		current_pstate_driver->adjust_perf = amd_pstate_adjust_perf;
 

@@ -1363,6 +1378,11 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 
 	amd_pstate_init_prefcore(cpudata);
 
+	/* initialize cpu cores boot state */
+	ret = amd_pstate_boost_init(cpudata);
+	if (ret)
+		goto free_cpudata1;
+
 	ret = amd_pstate_init_perf(cpudata);
 	if (ret)
 		goto free_cpudata1;

@@ -1417,7 +1437,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
 			return ret;
 		WRITE_ONCE(cpudata->cppc_cap1_cached, value);
 	}
-	amd_pstate_boost_init(cpudata);
 
 	return 0;
 

include/linux/amd-pstate.h +13 -0

index 6b832153a126..c5e41de65f70 100644
@@ -134,4 +134,17 @@ struct quirk_entry {
 	u32 lowest_freq;
 };
 
+/**
+ * struct amd_pstate_global_params - Global parameters, mostly tunable via sysfs.
+ * @cpb_boost:		Whether or not to use boost CPU P-states.
+ * @cpb_supported:	Whether or not CPU boost P-states are available
+ *			based on the MSR_K7_HWCR bit[25] state
+ */
+struct amd_pstate_global_params {
+	bool cpb_boost;
+	bool cpb_supported;
+};
+
+extern struct amd_pstate_global_params amd_pstate_global_params;
+
 #endif /* _LINUX_AMD_PSTATE_H */

Signature · 1 line

2.34.1

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, 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.

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 <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 99 ++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

drivers/cpufreq/amd-pstate.c +19 -0

index 3d86cd7c9073..49eeb38fcf20 100644
@@ -1291,6 +1291,103 @@ static ssize_t prefcore_show(struct device *dev,
 	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
 }
 
+static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
+	struct cppc_perf_ctrls perf_ctrls;
+	u32 highest_perf, nominal_perf, nominal_freq, max_freq;
+	int ret;
+
+	if (!policy)
+		return -ENODATA;
+
+	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);

drivers/cpufreq/amd-pstate.c +32 -0

+			return ret;
+		}
+	}
+
+	if (on)
+		policy->cpuinfo.max_freq = max_freq;
+	else
+		policy->cpuinfo.max_freq = nominal_freq;
+
+	policy->max = policy->cpuinfo.max_freq;
+
+	if (cppc_state == AMD_PSTATE_PASSIVE) {
+		ret = freq_qos_update_request(&cpudata->req[1],
+				      policy->cpuinfo.max_freq);
+	}
+
+	cpufreq_cpu_release(policy);
+
+	return ret;
+}
+
+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;

drivers/cpufreq/amd-pstate.c +28 -0

+		return -EINVAL;
+
+	amd_pstate_global_params.cpb_boost = !!new_state;
+
+	for_each_present_cpu(cpu) {
+
+		struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+		struct amd_cpudata *cpudata = policy->driver_data;
+
+		if (!cpudata) {
+			pr_err("cpudata is NULL\n");
+			ret = -ENODATA;
+			cpufreq_cpu_put(policy);
+			goto err_exit;
+		}
+
+		amd_cpu_boost_update(cpudata, amd_pstate_global_params.cpb_boost);
+		refresh_frequency_limits(policy);
+		cpufreq_cpu_put(policy);
+	}
+
+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);
 

@@ -1301,6 +1398,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,

@@ -1325,6 +1423,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
 };
 

Signature · 1 line

2.34.1

-----Original Message-----
From: Huang, Ray <[email protected]>
Sent: Tuesday, April 23, 2024 7:00 PM
To: Yuan, Perry <[email protected]>
Cc: [email protected]; Limonciello, Mario <[email protected]>; [email protected]; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>; Deucher, Alexander <[email protected]>; Huang, Shimmer <[email protected]>; [email protected]; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; linux- [email protected]
Subject: Re: [PATCH v7 3/6] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control

Quoted 56 lines · Perry

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, 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.

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 <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 99
++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

(quoted diff) +0 -0

b/drivers/cpufreq/amd-pstate.c index 3d86cd7c9073..49eeb38fcf20 100644

drivers/cpufreq/amd-pstate.c +23 -0

@@ -1291,6 +1291,103 @@ static ssize_t prefcore_show(struct device *dev,
    return sysfs_emit(buf, "%s\n",
 str_enabled_disabled(amd_pstate_prefcore));
 }
 
+static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
+{
+   struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
+   struct cppc_perf_ctrls perf_ctrls;
+   u32 highest_perf, nominal_perf, nominal_freq, max_freq;
+   int ret;
+
+   if (!policy)
+           return -ENODATA;
+
+   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);

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

Quoted 6 lines

drivers/cpufreq/amd-pstate.c +6 -0

+
+   } else {
+           perf_ctrls.max_perf = on ? highest_perf : nominal_perf;
+           ret = cppc_set_epp_perf(cpudata->cpu, &perf_ctrls, 1);
+           if (ret) {
+                   pr_debug("failed to set energy perf value (%d)\n", ret);

Do we need cpufreq_cpu_release here?

Quoted 25 lines

drivers/cpufreq/amd-pstate.c +25 -0

+                   return ret;
+           }
+   }
+
+   if (on)
+           policy->cpuinfo.max_freq = max_freq;
+   else
+           policy->cpuinfo.max_freq = nominal_freq;
+
+   policy->max = policy->cpuinfo.max_freq;
+
+   if (cppc_state == AMD_PSTATE_PASSIVE) {
+           ret = freq_qos_update_request(&cpudata->req[1],
+                                 policy->cpuinfo.max_freq);
+   }
+
+   cpufreq_cpu_release(policy);
+
+   return ret;
+}
+
+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);
+}

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

Quoted 10 lines

drivers/cpufreq/amd-pstate.c +10 -0

+
+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;
+
+   mutex_lock(&amd_pstate_driver_lock);
+   if (!amd_pstate_global_params.cpb_supported) {
+           pr_err("Boost mode is not supported by this processor or

SBIOS\n");

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

Quoted 5 lines

drivers/cpufreq/amd-pstate.c +5 -0

+           return -EINVAL;
+   }
+
+   ret = kstrtobool(buf, &new_state);
+   if (ret)

If get a falure, amd_pstate_driver_lock will be always locked.

Thanks,
Ray

Quoted 7 lines

drivers/cpufreq/amd-pstate.c +7 -0

+           return -EINVAL;
+
+   amd_pstate_global_params.cpb_boost = !!new_state;
+
+   for_each_present_cpu(cpu) {
+
+           struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);

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

Quoted 10 lines

drivers/cpufreq/amd-pstate.c +10 -0

+           struct amd_cpudata *cpudata = policy->driver_data;
+
+           if (!cpudata) {
+                   pr_err("cpudata is NULL\n");
+                   ret = -ENODATA;
+                   cpufreq_cpu_put(policy);
+                   goto err_exit;
+           }
+
+           amd_cpu_boost_update(cpudata,

amd_pstate_global_params.cpb_boost);

Quoted 32 lines

drivers/cpufreq/amd-pstate.c +10 -0

+           refresh_frequency_limits(policy);
+           cpufreq_cpu_put(policy);
+   }
+
+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);

@@ -1301,6 +1398,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,

drivers/cpufreq/amd-pstate.c +1 -1

@@ -1325,6 +1423,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
 };
 
--

2.34.1

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, 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.

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 <[email protected]>

---
  drivers/cpufreq/amd-pstate.c | 99 ++++++++++++++++++++++++++++++++++++
  1 file changed, 99 insertions(+)

drivers/cpufreq/amd-pstate.c +54 -0

index 3d86cd7c9073..49eeb38fcf20 100644
@@ -1291,6 +1291,103 @@ static ssize_t prefcore_show(struct device *dev,
  	return sysfs_emit(buf, "%s\n", str_enabled_disabled(amd_pstate_prefcore));
  }
 
+static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
+{
+	struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
+	struct cppc_perf_ctrls perf_ctrls;
+	u32 highest_perf, nominal_perf, nominal_freq, max_freq;
+	int ret;
+
+	if (!policy)
+		return -ENODATA;
+
+	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) {
+			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;
+
+	policy->max = policy->cpuinfo.max_freq;
+
+	if (cppc_state == AMD_PSTATE_PASSIVE) {
+		ret = freq_qos_update_request(&cpudata->req[1],
+				      policy->cpuinfo.max_freq);
+	}
+
+	cpufreq_cpu_release(policy);
+
+	return ret;
+}
+
+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);

drivers/cpufreq/amd-pstate.c +35 -0

+	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 -EINVAL;
+
+	amd_pstate_global_params.cpb_boost = !!new_state;
+
+	for_each_present_cpu(cpu) {
+
+		struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
+		struct amd_cpudata *cpudata = policy->driver_data;
+
+		if (!cpudata) {
+			pr_err("cpudata is NULL\n");
+			ret = -ENODATA;
+			cpufreq_cpu_put(policy);
+			goto err_exit;
+		}
+
+		amd_cpu_boost_update(cpudata, amd_pstate_global_params.cpb_boost);
+		refresh_frequency_limits(policy);
+		cpufreq_cpu_put(policy);
+	}
+
+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);
 

@@ -1301,6 +1398,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,

@@ -1325,6 +1423,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
  };
 

-----Original Message-----
From: Limonciello, Mario <[email protected]>
Sent: Friday, April 26, 2024 5:17 AM
To: Yuan, Perry <[email protected]>; [email protected]; [email protected]; Shenoy, Gautham Ranjal <[email protected]>; Petkov, Borislav <[email protected]>; Huang, Ray <[email protected]>; Deucher, Alexander <[email protected]>
Cc: Huang, Shimmer <[email protected]>; [email protected]; Du, Xiaojian <[email protected]>; Meng, Li (Jassmine) <[email protected]>; [email protected]; [email protected]
Subject: Re: [PATCH v7 3/6] cpufreq: amd-pstate: implement cpb_boost sysfs entry for boost control

Quoted 13 lines · Perry Yuan

From: Perry Yuan <[email protected]>

With this new sysfs entry cpb_boostcreated, 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.

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"

Quoted 8 lines

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 <[email protected]>

---
  drivers/cpufreq/amd-pstate.c | 99

++++++++++++++++++++++++++++++++++++

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

Quoted 8 lines

  1 file changed, 99 insertions(+)

(quoted diff) +0 -0

b/drivers/cpufreq/amd-pstate.c index 3d86cd7c9073..49eeb38fcf20 100644

drivers/cpufreq/amd-pstate.c +0 -0

@@ -1291,6 +1291,103 @@ static ssize_t prefcore_show(struct device *dev,
    return sysfs_emit(buf, "%s\n",

str_enabled_disabled(amd_pstate_prefcore));

Quoted 59 lines

drivers/cpufreq/amd-pstate.c +57 -0

  }
 
+static int amd_cpu_boost_update(struct amd_cpudata *cpudata, u32 on)
+{
+   struct cpufreq_policy *policy = cpufreq_cpu_acquire(cpudata->cpu);
+   struct cppc_perf_ctrls perf_ctrls;
+   u32 highest_perf, nominal_perf, nominal_freq, max_freq;
+   int ret;
+
+   if (!policy)
+           return -ENODATA;
+
+   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) {
+                   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;
+
+   policy->max = policy->cpuinfo.max_freq;
+
+   if (cppc_state == AMD_PSTATE_PASSIVE) {
+           ret = freq_qos_update_request(&cpudata->req[1],
+                                 policy->cpuinfo.max_freq);
+   }
+
+   cpufreq_cpu_release(policy);
+
+   return ret;
+}
+
+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) {

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

Quoted 5 lines

drivers/cpufreq/amd-pstate.c +5 -0

+   bool new_state;
+   ssize_t ret;
+   int cpu;
+
+   mutex_lock(&amd_pstate_driver_lock);

This mutex lock should be after the check for cpb_supported and kstrtobool check. Right now you have two cases that the lock doesn't get released.

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

Quoted 3 lines

drivers/cpufreq/amd-pstate.c +2 -0

+   if (!amd_pstate_global_params.cpb_supported) {
+           pr_err("Boost mode is not supported by this processor or

SBIOS\n");

Quoted 12 lines

drivers/cpufreq/amd-pstate.c +12 -0

+           return -EINVAL;
+   }
+
+   ret = kstrtobool(buf, &new_state);
+   if (ret)
+           return -EINVAL;
+
+   amd_pstate_global_params.cpb_boost = !!new_state;
+
+   for_each_present_cpu(cpu) {
+
+           struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);

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

Quoted 10 lines

drivers/cpufreq/amd-pstate.c +10 -0

+           struct amd_cpudata *cpudata = policy->driver_data;
+
+           if (!cpudata) {
+                   pr_err("cpudata is NULL\n");
+                   ret = -ENODATA;
+                   cpufreq_cpu_put(policy);
+                   goto err_exit;
+           }
+
+           amd_cpu_boost_update(cpudata,

amd_pstate_global_params.cpb_boost);

drivers/cpufreq/amd-pstate.c

Quoted 13 lines

drivers/cpufreq/amd-pstate.c +9 -0

+           refresh_frequency_limits(policy);
+           cpufreq_cpu_put(policy);
+   }
+
+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);
 

@@ -1301,6 +1398,7 @@

cpufreq_freq_attr_rw(energy_performance_preference);

Quoted 15 lines

drivers/cpufreq/amd-pstate.c +2 -0

  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,

@@ -1325,6 +1423,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
  };
 

From: Perry Yuan <[email protected]>

Select the min perf to fix the highest perf value while update pstate CPPC request MSR register, here we need to limit the max perf value when CPU boost is disabled in case of that highest perf value in the MSR will be reset to original highest perf value which cause the BOOST control failed.

Signed-off-by Perry Yuan <[email protected]>

---
 drivers/cpufreq/amd-pstate.c | 5 +++++
 1 file changed, 5 insertions(+)

drivers/cpufreq/amd-pstate.c +5 -0

index 49eeb38fcf20..22e5b84dbe28 100644
@@ -476,6 +476,7 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 			      u32 des_perf, u32 max_perf, bool fast_switch, int gov_flags)
 {
 	u64 prev = READ_ONCE(cpudata->cppc_req_cached);
+	u32 nominal_perf = READ_ONCE(cpudata->nominal_perf);
 	u64 value = prev;
 
 	min_perf = clamp_t(unsigned long, min_perf, cpudata->min_limit_perf,

@@ -495,6 +496,10 @@ static void amd_pstate_update(struct amd_cpudata *cpudata, u32 min_perf,
 	value &= ~AMD_CPPC_DES_PERF(~0L);
 	value |= AMD_CPPC_DES_PERF(des_perf);
 
+	/* limit the max perf when core performance boost feature is disabled */
+	if (!amd_pstate_global_params.cpb_boost)
+		max_perf = min_t(unsigned long, nominal_perf, max_perf);
+
 	value &= ~AMD_CPPC_MAX_PERF(~0L);
 	value |= AMD_CPPC_MAX_PERF(max_perf);
 

Signature · 1 line

2.34.1

As the new CPB boost control is enabled, pstate unit test needs to remove legacy boost_supported check and start to use new CPB boost control interface amd_pstate_global_params.cpb_boost.

Signed-off-by Perry Yuan <[email protected]>

---
 drivers/cpufreq/amd-pstate-ut.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/cpufreq/amd-pstate-ut.c +1 -1

index f04ae67dda37..b3601b0e6dd3 100644
@@ -226,7 +226,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;

Signature · 1 line

2.34.1