1. Patchew
  2. linux
  3. [PATCH 0/8] Adjustments for preferred core detection
  4. View patch

[PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()

Mario Limonciello posted 8 patches 1 year, 3 months ago
There is a newer version of this series
[PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
Posted by Mario Limonciello 1 year, 3 months ago 
From: Mario Limonciello <mario.limonciello@amd.com>

The special case in amd_pstate_highest_perf_set() is the value used
for calculating the boost numerator.  Merge this into
amd_get_boost_ratio_numerator() and then use that to calculate boost
ratio.

This allows dropping more special casing of the highest perf value.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 arch/x86/kernel/acpi/cppc.c  | 16 ++++++++++++
 drivers/cpufreq/amd-pstate.c | 49 +++++++-----------------------------
 2 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
index 729b35e84f5eb..44b13a4e28740 100644
--- a/arch/x86/kernel/acpi/cppc.c
+++ b/arch/x86/kernel/acpi/cppc.c
@@ -9,6 +9,7 @@
 #include <asm/processor.h>
 #include <asm/topology.h>
 
+#define CPPC_HIGHEST_PERF_PERFORMANCE	196
 #define CPPC_HIGHEST_PERF_PREFCORE	166
 
 enum amd_pref_core {
@@ -244,6 +245,21 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
 		*numerator = boost_numerator;
 		return 0;
 	}
+
+	/*
+	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
+	 * the highest performance level is set to 196.
+	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
+	 */
+	if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
+		switch (boot_cpu_data.x86_model) {
+		case 0x70 ... 0x7f:
+			*numerator = CPPC_HIGHEST_PERF_PERFORMANCE;
+			return 0;
+		default:
+			break;
+		}
+	}
 	*numerator = CPPC_HIGHEST_PERF_PREFCORE;
 
 	return 0;
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index ec32c830abc1d..75568d0f84623 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -52,8 +52,6 @@
 #define AMD_PSTATE_TRANSITION_LATENCY	20000
 #define AMD_PSTATE_TRANSITION_DELAY	1000
 #define AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY 600
-#define CPPC_HIGHEST_PERF_PERFORMANCE	196
-#define CPPC_HIGHEST_PERF_DEFAULT	166
 
 #define AMD_CPPC_EPP_PERFORMANCE		0x00
 #define AMD_CPPC_EPP_BALANCE_PERFORMANCE	0x80
@@ -372,43 +370,17 @@ static inline int amd_pstate_enable(bool enable)
 	return static_call(amd_pstate_enable)(enable);
 }
 
-static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)
-{
-	struct cpuinfo_x86 *c = &cpu_data(0);
-
-	/*
-	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
-	 * the highest performance level is set to 196.
-	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
-	 */
-	if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= 0x7f))
-		return CPPC_HIGHEST_PERF_PERFORMANCE;
-
-	return CPPC_HIGHEST_PERF_DEFAULT;
-}
-
 static int pstate_init_perf(struct amd_cpudata *cpudata)
 {
 	u64 cap1;
-	u32 highest_perf;
 
 	int ret = rdmsrl_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
 				     &cap1);
 	if (ret)
 		return ret;
 
-	/* For platforms that do not support the preferred core feature, the
-	 * highest_pef may be configured with 166 or 255, to avoid max frequency
-	 * calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as
-	 * the default max perf.
-	 */
-	if (cpudata->hw_prefcore)
-		highest_perf = amd_pstate_highest_perf_set(cpudata);
-	else
-		highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
-
-	WRITE_ONCE(cpudata->highest_perf, highest_perf);
-	WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
+	WRITE_ONCE(cpudata->highest_perf, AMD_CPPC_HIGHEST_PERF(cap1));
+	WRITE_ONCE(cpudata->max_limit_perf, AMD_CPPC_HIGHEST_PERF(cap1));
 	WRITE_ONCE(cpudata->nominal_perf, AMD_CPPC_NOMINAL_PERF(cap1));
 	WRITE_ONCE(cpudata->lowest_nonlinear_perf, AMD_CPPC_LOWNONLIN_PERF(cap1));
 	WRITE_ONCE(cpudata->lowest_perf, AMD_CPPC_LOWEST_PERF(cap1));
@@ -426,12 +398,7 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
 	if (ret)
 		return ret;
 
-	if (cpudata->hw_prefcore)
-		highest_perf = amd_pstate_highest_perf_set(cpudata);
-	else
-		highest_perf = cppc_perf.highest_perf;
-
-	WRITE_ONCE(cpudata->highest_perf, highest_perf);
+	WRITE_ONCE(cpudata->highest_perf, cppc_perf.highest_perf);
 	WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
 	WRITE_ONCE(cpudata->nominal_perf, cppc_perf.nominal_perf);
 	WRITE_ONCE(cpudata->lowest_nonlinear_perf,
@@ -905,8 +872,8 @@ static u32 amd_pstate_get_transition_latency(unsigned int cpu)
 static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
 {
 	int ret;
-	u32 min_freq;
-	u32 highest_perf, max_freq;
+	u32 min_freq, max_freq;
+	u64 numerator;
 	u32 nominal_perf, nominal_freq;
 	u32 lowest_nonlinear_perf, lowest_nonlinear_freq;
 	u32 boost_ratio, lowest_nonlinear_ratio;
@@ -928,8 +895,10 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
 
 	nominal_perf = READ_ONCE(cpudata->nominal_perf);
 
-	highest_perf = READ_ONCE(cpudata->highest_perf);
-	boost_ratio = div_u64(highest_perf << SCHED_CAPACITY_SHIFT, nominal_perf);
+	ret = amd_get_boost_ratio_numerator(cpudata->cpu, &numerator);
+	if (ret)
+		return ret;
+	boost_ratio = div_u64(numerator << SCHED_CAPACITY_SHIFT, nominal_perf);
 	max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT) * 1000;
 
 	lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
-- 
2.43.0
Re: [PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
Posted by kernel test robot 1 year, 3 months ago 
Hi Mario,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge tip/x86/core tip/master linus/master v6.11-rc5 next-20240827]
[cannot apply to tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/x86-amd-Move-amd_get_highest_perf-from-amd-c-to-cppc-c/20240827-051648
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20240826211358.2694603-7-superm1%40kernel.org
patch subject: [PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20240828/202408280553.k4hXRtZy-lkp@intel.com/config)
compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280553.k4hXRtZy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408280553.k4hXRtZy-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/cpufreq/amd-pstate.c:402:38: warning: variable 'highest_perf' is uninitialized when used here [-Wuninitialized]
     402 |         WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
         |                                             ^~~~~~~~~~~~
   include/asm-generic/rwonce.h:61:18: note: expanded from macro 'WRITE_ONCE'
      61 |         __WRITE_ONCE(x, val);                                           \
         |                         ^~~
   include/asm-generic/rwonce.h:55:33: note: expanded from macro '__WRITE_ONCE'
      55 |         *(volatile typeof(x) *)&(x) = (val);                            \
         |                                        ^~~
   drivers/cpufreq/amd-pstate.c:395:18: note: initialize the variable 'highest_perf' to silence this warning
     395 |         u32 highest_perf;
         |                         ^
         |                          = 0
   1 warning generated.


vim +/highest_perf +402 drivers/cpufreq/amd-pstate.c

ec437d71db77a1 Huang Rui         2021-12-24  391  
e059c184da47e9 Huang Rui         2021-12-24  392  static int cppc_init_perf(struct amd_cpudata *cpudata)
e059c184da47e9 Huang Rui         2021-12-24  393  {
e059c184da47e9 Huang Rui         2021-12-24  394  	struct cppc_perf_caps cppc_perf;
bedadcfb011fef Perry Yuan        2022-08-30  395  	u32 highest_perf;
e059c184da47e9 Huang Rui         2021-12-24  396  
e059c184da47e9 Huang Rui         2021-12-24  397  	int ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
e059c184da47e9 Huang Rui         2021-12-24  398  	if (ret)
e059c184da47e9 Huang Rui         2021-12-24  399  		return ret;
e059c184da47e9 Huang Rui         2021-12-24  400  
347b3754cc9780 Mario Limonciello 2024-08-26  401  	WRITE_ONCE(cpudata->highest_perf, cppc_perf.highest_perf);
febab20caebac9 Wyes Karny        2023-11-17 @402  	WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
e059c184da47e9 Huang Rui         2021-12-24  403  	WRITE_ONCE(cpudata->nominal_perf, cppc_perf.nominal_perf);
e059c184da47e9 Huang Rui         2021-12-24  404  	WRITE_ONCE(cpudata->lowest_nonlinear_perf,
e059c184da47e9 Huang Rui         2021-12-24  405  		   cppc_perf.lowest_nonlinear_perf);
e059c184da47e9 Huang Rui         2021-12-24  406  	WRITE_ONCE(cpudata->lowest_perf, cppc_perf.lowest_perf);
e571a5e2068ef5 Meng Li           2024-01-19  407  	WRITE_ONCE(cpudata->prefcore_ranking, cppc_perf.highest_perf);
febab20caebac9 Wyes Karny        2023-11-17  408  	WRITE_ONCE(cpudata->min_limit_perf, cppc_perf.lowest_perf);
e059c184da47e9 Huang Rui         2021-12-24  409  
2dd6d0ebf74049 Wyes Karny        2023-03-07  410  	if (cppc_state == AMD_PSTATE_ACTIVE)
2dd6d0ebf74049 Wyes Karny        2023-03-07  411  		return 0;
2dd6d0ebf74049 Wyes Karny        2023-03-07  412  
2dd6d0ebf74049 Wyes Karny        2023-03-07  413  	ret = cppc_get_auto_sel_caps(cpudata->cpu, &cppc_perf);
2dd6d0ebf74049 Wyes Karny        2023-03-07  414  	if (ret) {
2dd6d0ebf74049 Wyes Karny        2023-03-07  415  		pr_warn("failed to get auto_sel, ret: %d\n", ret);
e059c184da47e9 Huang Rui         2021-12-24  416  		return 0;
e059c184da47e9 Huang Rui         2021-12-24  417  	}
e059c184da47e9 Huang Rui         2021-12-24  418  
2dd6d0ebf74049 Wyes Karny        2023-03-07  419  	ret = cppc_set_auto_sel(cpudata->cpu,
2dd6d0ebf74049 Wyes Karny        2023-03-07  420  			(cppc_state == AMD_PSTATE_PASSIVE) ? 0 : 1);
2dd6d0ebf74049 Wyes Karny        2023-03-07  421  
2dd6d0ebf74049 Wyes Karny        2023-03-07  422  	if (ret)
2dd6d0ebf74049 Wyes Karny        2023-03-07  423  		pr_warn("failed to set auto_sel, ret: %d\n", ret);
2dd6d0ebf74049 Wyes Karny        2023-03-07  424  
2dd6d0ebf74049 Wyes Karny        2023-03-07  425  	return ret;
2dd6d0ebf74049 Wyes Karny        2023-03-07  426  }
2dd6d0ebf74049 Wyes Karny        2023-03-07  427  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
Posted by Gautham R. Shenoy 1 year, 3 months ago 
Hello Mario,


On Mon, Aug 26, 2024 at 04:13:56PM -0500, Mario Limonciello wrote:
> From: Mario Limonciello <mario.limonciello@amd.com>
> 
> The special case in amd_pstate_highest_perf_set() is the value used
> for calculating the boost numerator.  Merge this into
> amd_get_boost_ratio_numerator() and then use that to calculate boost
> ratio.
> 
> This allows dropping more special casing of the highest perf value.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  arch/x86/kernel/acpi/cppc.c  | 16 ++++++++++++
>  drivers/cpufreq/amd-pstate.c | 49 +++++++-----------------------------
>  2 files changed, 25 insertions(+), 40 deletions(-)
> 
> diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
> index 729b35e84f5eb..44b13a4e28740 100644
> --- a/arch/x86/kernel/acpi/cppc.c
> +++ b/arch/x86/kernel/acpi/cppc.c
> @@ -9,6 +9,7 @@
>  #include <asm/processor.h>
>  #include <asm/topology.h>
>  
> +#define CPPC_HIGHEST_PERF_PERFORMANCE	196
>  #define CPPC_HIGHEST_PERF_PREFCORE	166
>  
>  enum amd_pref_core {
> @@ -244,6 +245,21 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
>  		*numerator = boost_numerator;
>  		return 0;
>  	}
> +
> +	/*
> +	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
> +	 * the highest performance level is set to 196.
> +	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
> +	 */
> +	if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
> +		switch (boot_cpu_data.x86_model) {
> +		case 0x70 ... 0x7f:
> +			*numerator = CPPC_HIGHEST_PERF_PERFORMANCE;
> +			return 0;
> +		default:
> +			break;
> +		}
> +	}
>  	*numerator = CPPC_HIGHEST_PERF_PREFCORE;
>  
>  	return 0;
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index ec32c830abc1d..75568d0f84623 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -52,8 +52,6 @@
>  #define AMD_PSTATE_TRANSITION_LATENCY	20000
>  #define AMD_PSTATE_TRANSITION_DELAY	1000
>  #define AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY 600
> -#define CPPC_HIGHEST_PERF_PERFORMANCE	196
> -#define CPPC_HIGHEST_PERF_DEFAULT	166
>  
>  #define AMD_CPPC_EPP_PERFORMANCE		0x00
>  #define AMD_CPPC_EPP_BALANCE_PERFORMANCE	0x80
> @@ -372,43 +370,17 @@ static inline int amd_pstate_enable(bool enable)
>  	return static_call(amd_pstate_enable)(enable);
>  }
>  
> -static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)
> -{
> -	struct cpuinfo_x86 *c = &cpu_data(0);
> -
> -	/*
> -	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
> -	 * the highest performance level is set to 196.
> -	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
> -	 */
> -	if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= 0x7f))
> -		return CPPC_HIGHEST_PERF_PERFORMANCE;
> -
> -	return CPPC_HIGHEST_PERF_DEFAULT;
> -}
> -
>  static int pstate_init_perf(struct amd_cpudata *cpudata)
>  {
>  	u64 cap1;
> -	u32 highest_perf;
>  
>  	int ret = rdmsrl_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
>  				     &cap1);
>  	if (ret)
>  		return ret;
>  
> -	/* For platforms that do not support the preferred core feature, the
> -	 * highest_pef may be configured with 166 or 255, to avoid max frequency
> -	 * calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as
> -	 * the default max perf.
> -	 */
> -	if (cpudata->hw_prefcore)
> -		highest_perf = amd_pstate_highest_perf_set(cpudata);
> -	else
> -		highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
> -
> -	WRITE_ONCE(cpudata->highest_perf, highest_perf);
> -	WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
> +	WRITE_ONCE(cpudata->highest_perf, AMD_CPPC_HIGHEST_PERF(cap1));
> +	WRITE_ONCE(cpudata->max_limit_perf, AMD_CPPC_HIGHEST_PERF(cap1));


So henceforth, cpudata->highest_perf is expected to cache the value of
CPPC.highest_perf and not the boost_ratio_numerator. There are couple
of user-visible changes due to this.


1.  On platforms where preferred-core is supported, previously the
    sysfs file
    /sys/devices/system/cpu/cpuX/cpufreq/amd_pstate_highest_perf would
    report the boost_ratio_numerator. Henceforth it will report
    CPPC.highest_perf.

    I hope there are no userspace tools that compute the boost_ratio
    using the syfs amd_pstate_highest_perf/amd_pstate_nominal_perf.

2. The amd_pstate_prefcore_ranking and amd_pstate_highest_perf will
   show the same values on all platforms, and henceforth are
   redundant.


Shouldn't this be documented?

The rest of the patch looks good to me.



--
Thanks and Regards
gautham.
Re: [PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
Posted by Mario Limonciello 1 year, 3 months ago 
On 8/27/2024 11:52, Gautham R. Shenoy wrote:
> Hello Mario,
> 
> 
> On Mon, Aug 26, 2024 at 04:13:56PM -0500, Mario Limonciello wrote:
>> From: Mario Limonciello <mario.limonciello@amd.com>
>>
>> The special case in amd_pstate_highest_perf_set() is the value used
>> for calculating the boost numerator.  Merge this into
>> amd_get_boost_ratio_numerator() and then use that to calculate boost
>> ratio.
>>
>> This allows dropping more special casing of the highest perf value.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   arch/x86/kernel/acpi/cppc.c  | 16 ++++++++++++
>>   drivers/cpufreq/amd-pstate.c | 49 +++++++-----------------------------
>>   2 files changed, 25 insertions(+), 40 deletions(-)
>>
>> diff --git a/arch/x86/kernel/acpi/cppc.c b/arch/x86/kernel/acpi/cppc.c
>> index 729b35e84f5eb..44b13a4e28740 100644
>> --- a/arch/x86/kernel/acpi/cppc.c
>> +++ b/arch/x86/kernel/acpi/cppc.c
>> @@ -9,6 +9,7 @@
>>   #include <asm/processor.h>
>>   #include <asm/topology.h>
>>   
>> +#define CPPC_HIGHEST_PERF_PERFORMANCE	196
>>   #define CPPC_HIGHEST_PERF_PREFCORE	166
>>   
>>   enum amd_pref_core {
>> @@ -244,6 +245,21 @@ int amd_get_boost_ratio_numerator(unsigned int cpu, u64 *numerator)
>>   		*numerator = boost_numerator;
>>   		return 0;
>>   	}
>> +
>> +	/*
>> +	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
>> +	 * the highest performance level is set to 196.
>> +	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
>> +	 */
>> +	if (cpu_feature_enabled(X86_FEATURE_ZEN4)) {
>> +		switch (boot_cpu_data.x86_model) {
>> +		case 0x70 ... 0x7f:
>> +			*numerator = CPPC_HIGHEST_PERF_PERFORMANCE;
>> +			return 0;
>> +		default:
>> +			break;
>> +		}
>> +	}
>>   	*numerator = CPPC_HIGHEST_PERF_PREFCORE;
>>   
>>   	return 0;
>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
>> index ec32c830abc1d..75568d0f84623 100644
>> --- a/drivers/cpufreq/amd-pstate.c
>> +++ b/drivers/cpufreq/amd-pstate.c
>> @@ -52,8 +52,6 @@
>>   #define AMD_PSTATE_TRANSITION_LATENCY	20000
>>   #define AMD_PSTATE_TRANSITION_DELAY	1000
>>   #define AMD_PSTATE_FAST_CPPC_TRANSITION_DELAY 600
>> -#define CPPC_HIGHEST_PERF_PERFORMANCE	196
>> -#define CPPC_HIGHEST_PERF_DEFAULT	166
>>   
>>   #define AMD_CPPC_EPP_PERFORMANCE		0x00
>>   #define AMD_CPPC_EPP_BALANCE_PERFORMANCE	0x80
>> @@ -372,43 +370,17 @@ static inline int amd_pstate_enable(bool enable)
>>   	return static_call(amd_pstate_enable)(enable);
>>   }
>>   
>> -static u32 amd_pstate_highest_perf_set(struct amd_cpudata *cpudata)
>> -{
>> -	struct cpuinfo_x86 *c = &cpu_data(0);
>> -
>> -	/*
>> -	 * For AMD CPUs with Family ID 19H and Model ID range 0x70 to 0x7f,
>> -	 * the highest performance level is set to 196.
>> -	 * https://bugzilla.kernel.org/show_bug.cgi?id=218759
>> -	 */
>> -	if (c->x86 == 0x19 && (c->x86_model >= 0x70 && c->x86_model <= 0x7f))
>> -		return CPPC_HIGHEST_PERF_PERFORMANCE;
>> -
>> -	return CPPC_HIGHEST_PERF_DEFAULT;
>> -}
>> -
>>   static int pstate_init_perf(struct amd_cpudata *cpudata)
>>   {
>>   	u64 cap1;
>> -	u32 highest_perf;
>>   
>>   	int ret = rdmsrl_safe_on_cpu(cpudata->cpu, MSR_AMD_CPPC_CAP1,
>>   				     &cap1);
>>   	if (ret)
>>   		return ret;
>>   
>> -	/* For platforms that do not support the preferred core feature, the
>> -	 * highest_pef may be configured with 166 or 255, to avoid max frequency
>> -	 * calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as
>> -	 * the default max perf.
>> -	 */
>> -	if (cpudata->hw_prefcore)
>> -		highest_perf = amd_pstate_highest_perf_set(cpudata);
>> -	else
>> -		highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
>> -
>> -	WRITE_ONCE(cpudata->highest_perf, highest_perf);
>> -	WRITE_ONCE(cpudata->max_limit_perf, highest_perf);
>> +	WRITE_ONCE(cpudata->highest_perf, AMD_CPPC_HIGHEST_PERF(cap1));
>> +	WRITE_ONCE(cpudata->max_limit_perf, AMD_CPPC_HIGHEST_PERF(cap1));
> 
> 
> So henceforth, cpudata->highest_perf is expected to cache the value of
> CPPC.highest_perf and not the boost_ratio_numerator. There are couple
> of user-visible changes due to this.
> 
> 
> 1.  On platforms where preferred-core is supported, previously the
>      sysfs file
>      /sys/devices/system/cpu/cpuX/cpufreq/amd_pstate_highest_perf would
>      report the boost_ratio_numerator. Henceforth it will report
>      CPPC.highest_perf.
> 
>      I hope there are no userspace tools that compute the boost_ratio
>      using the syfs amd_pstate_highest_perf/amd_pstate_nominal_perf.
> 
> 2. The amd_pstate_prefcore_ranking and amd_pstate_highest_perf will
>     show the same values on all platforms, and henceforth are
>     redundant.
> 

Good observations here.  I'm not aware of any tools trying to replicate 
this calculation.
With the redundancy I would actually argue we should just drop the sysfs 
file 'amd_pstate_prefcore_ranking'.

Thoughts?

> 
> Shouldn't this be documented?

I noticed amd_pstate_prefcore_ranking wasn't properly documented in 
amd-pstate.rst in the first place.  If the decision is not to drop the 
sysfs file, then I'll add a section for it.

> 
> The rest of the patch looks good to me.
> 
> 
> 
> --
> Thanks and Regards
> gautham.
>
Re: [PATCH 6/8] cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()
Posted by Gautham R. Shenoy 1 year, 3 months ago 
On Tue, Aug 27, 2024 at 01:36:47PM -0500, Mario Limonciello wrote:
> On 8/27/2024 11:52, Gautham R. Shenoy wrote:

[..snip..]

> > 
> > 
> > So henceforth, cpudata->highest_perf is expected to cache the value of
> > CPPC.highest_perf and not the boost_ratio_numerator. There are couple
> > of user-visible changes due to this.
> > 
> > 
> > 1.  On platforms where preferred-core is supported, previously the
> >      sysfs file
> >      /sys/devices/system/cpu/cpuX/cpufreq/amd_pstate_highest_perf would
> >      report the boost_ratio_numerator. Henceforth it will report
> >      CPPC.highest_perf.

One other side effect is that the highest_perf sysfs file will now
reveal the differential highest_perf, even when "amd_prefcore=false",
while earlier all the cores would report CPPC_HIGHEST_PERF_DEFAULT.

I think we may be better off reporting the boost-numerator here, but
that's really not the highest_perf :(

> > 
> >      I hope there are no userspace tools that compute the boost_ratio
> >      using the syfs amd_pstate_highest_perf/amd_pstate_nominal_perf.
> > 
> > 2. The amd_pstate_prefcore_ranking and amd_pstate_highest_perf will
> >     show the same values on all platforms, and henceforth are
> >     redundant.
> > 
> 
> Good observations here.  I'm not aware of any tools trying to replicate this
> calculation.
> With the redundancy I would actually argue we should just drop the sysfs
> file 'amd_pstate_prefcore_ranking'.
> 
> Thoughts?

Looking at the code again, I realize that I was
wrong. cpudata->prefcore_ranking also gets updated in
amd_pstate_update_min_max_limits() and reflects the dynamic
preference.

While cpudata->highest_perf value indicates the initial boot-time
preference.

Hence it makes sense to amd_pstate_prefcore_ranking.

> 
> > 
> > Shouldn't this be documented?
> 
> I noticed amd_pstate_prefcore_ranking wasn't properly documented in
> amd-pstate.rst in the first place.  If the decision is not to drop the sysfs
> file, then I'll add a section for it.

Thanks.

> 
> > 
> > The rest of the patch looks good to me.
> > 
> > 
> > 
--
Thanks and Regards
gautham.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.