linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86, sched: Fix the AMD CPPC maximum perf on some specific generations
@ 2021-04-20  8:09 Huang Rui
  2021-04-20  8:22 ` Borislav Petkov
  0 siblings, 1 reply; 3+ messages in thread
From: Huang Rui @ 2021-04-20  8:09 UTC (permalink / raw)
  To: linux-pm, linux-kernel
  Cc: Alex Deucher, Jason Bagavatsingham, Pierre-Loup A . Griffais,
	Huang Rui, Nathan Fontenot, Rafael J . Wysocki, Borislav Petkov,
	x86

Some AMD Ryzen generations has different calculation method on maximum
perf. 255 is not for all asics, some specific generations used 166 as
the maximum perf. This patch is to fix the different maximum perf value
of AMD CPPC.

Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies")

Reported-by: Jason Bagavatsingham <jason.bagavatsingham@gmail.com>
Tested-by: Jason Bagavatsingham <jason.bagavatsingham@gmail.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Nathan Fontenot <nathan.fontenot@amd.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: x86@kernel.org
---
 arch/x86/kernel/smpboot.c      | 33 ++++++++++++++++++++++++++-
 drivers/cpufreq/acpi-cpufreq.c | 41 ++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 02813a7f3a7c..705bc5ceb1ea 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -2033,6 +2033,37 @@ static bool intel_set_max_freq_ratio(void)
 }
 
 #ifdef CONFIG_ACPI_CPPC_LIB
+static u64 amd_get_highest_perf(void)
+{
+	u64 cppc_max_perf;
+
+	switch (boot_cpu_data.x86) {
+	case 0x17:
+		if ((boot_cpu_data.x86_model >= 0x30 &&
+		     boot_cpu_data.x86_model < 0x40) ||
+		    (boot_cpu_data.x86_model >= 0x70 &&
+		     boot_cpu_data.x86_model < 0x80))
+			cppc_max_perf = 166;
+		else
+			cppc_max_perf = 255;
+		break;
+	case 0x19:
+		if ((boot_cpu_data.x86_model >= 0x20 &&
+		     boot_cpu_data.x86_model < 0x30) ||
+		    (boot_cpu_data.x86_model >= 0x40 &&
+		     boot_cpu_data.x86_model < 0x70))
+			cppc_max_perf = 166;
+		else
+			cppc_max_perf = 255;
+		break;
+	default:
+		cppc_max_perf = 255;
+		break;
+	}
+
+	return cppc_max_perf;
+}
+
 static bool amd_set_max_freq_ratio(void)
 {
 	struct cppc_perf_caps perf_caps;
@@ -2046,8 +2077,8 @@ static bool amd_set_max_freq_ratio(void)
 		return false;
 	}
 
-	highest_perf = perf_caps.highest_perf;
 	nominal_perf = perf_caps.nominal_perf;
+	highest_perf = amd_get_highest_perf();
 
 	if (!highest_perf || !nominal_perf) {
 		pr_debug("Could not retrieve highest or nominal performance\n");
diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
index d1bbc16fba4b..e5c03360db20 100644
--- a/drivers/cpufreq/acpi-cpufreq.c
+++ b/drivers/cpufreq/acpi-cpufreq.c
@@ -630,6 +630,44 @@ static int acpi_cpufreq_blacklist(struct cpuinfo_x86 *c)
 #endif
 
 #ifdef CONFIG_ACPI_CPPC_LIB
+
+static u64 get_amd_max_boost_ratio(unsigned int cpu, u64 nominal_perf)
+{
+	u64 boost_ratio, cppc_max_perf;
+
+	if (!nominal_perf)
+		return 0;
+
+	switch (boot_cpu_data.x86) {
+	case 0x17:
+		if ((boot_cpu_data.x86_model >= 0x30 &&
+		     boot_cpu_data.x86_model < 0x40) ||
+		    (boot_cpu_data.x86_model >= 0x70 &&
+		     boot_cpu_data.x86_model < 0x80))
+			cppc_max_perf = 166;
+		else
+			cppc_max_perf = 255;
+		break;
+	case 0x19:
+		if ((boot_cpu_data.x86_model >= 0x20 &&
+		     boot_cpu_data.x86_model < 0x30) ||
+		    (boot_cpu_data.x86_model >= 0x40 &&
+		     boot_cpu_data.x86_model < 0x70))
+			cppc_max_perf = 166;
+		else
+			cppc_max_perf = 255;
+		break;
+	default:
+		cppc_max_perf = 255;
+		break;
+	}
+
+	boost_ratio = div_u64(cppc_max_perf << SCHED_CAPACITY_SHIFT,
+			      nominal_perf);
+
+	return boost_ratio;
+}
+
 static u64 get_max_boost_ratio(unsigned int cpu)
 {
 	struct cppc_perf_caps perf_caps;
@@ -646,6 +684,9 @@ static u64 get_max_boost_ratio(unsigned int cpu)
 		return 0;
 	}
 
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD)
+		return get_amd_max_boost_ratio(cpu, perf_caps.nominal_perf);
+
 	highest_perf = perf_caps.highest_perf;
 	nominal_perf = perf_caps.nominal_perf;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86, sched: Fix the AMD CPPC maximum perf on some specific generations
  2021-04-20  8:09 [PATCH] x86, sched: Fix the AMD CPPC maximum perf on some specific generations Huang Rui
@ 2021-04-20  8:22 ` Borislav Petkov
  2021-04-20 10:48   ` Huang Rui
  0 siblings, 1 reply; 3+ messages in thread
From: Borislav Petkov @ 2021-04-20  8:22 UTC (permalink / raw)
  To: Huang Rui
  Cc: linux-pm, linux-kernel, Alex Deucher, Jason Bagavatsingham,
	Pierre-Loup A . Griffais, Nathan Fontenot, Rafael J . Wysocki,
	x86

On Tue, Apr 20, 2021 at 04:09:43PM +0800, Huang Rui wrote:
> Some AMD Ryzen generations has different calculation method on maximum
> perf. 255 is not for all asics, some specific generations used 166 as
> the maximum perf. This patch is to fix the different maximum perf value

Avoid having "This patch" or "This commit" in the commit message. It is
tautologically useless.

Also, do

$ git grep 'This patch' Documentation/process

for more details.

> of AMD CPPC.
> 
> Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies")
> 
> Reported-by: Jason Bagavatsingham <jason.bagavatsingham@gmail.com>
> Tested-by: Jason Bagavatsingham <jason.bagavatsingham@gmail.com>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Nathan Fontenot <nathan.fontenot@amd.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: x86@kernel.org
> ---
>  arch/x86/kernel/smpboot.c      | 33 ++++++++++++++++++++++++++-
>  drivers/cpufreq/acpi-cpufreq.c | 41 ++++++++++++++++++++++++++++++++++
>  2 files changed, 73 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index 02813a7f3a7c..705bc5ceb1ea 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -2033,6 +2033,37 @@ static bool intel_set_max_freq_ratio(void)
>  }
>  
>  #ifdef CONFIG_ACPI_CPPC_LIB
> +static u64 amd_get_highest_perf(void)
> +{

	struct cpuinfo_x86 *c = &boot_cpu_data;

and then you can use "c" everywhere.

> +	u64 cppc_max_perf;

u64 for something which fits in a byte?

Also,
	max_perf = 255;

and you can remove the else and default branches below.

> +
> +	switch (boot_cpu_data.x86) {
> +	case 0x17:
> +		if ((boot_cpu_data.x86_model >= 0x30 &&
> +		     boot_cpu_data.x86_model < 0x40) ||
> +		    (boot_cpu_data.x86_model >= 0x70 &&
> +		     boot_cpu_data.x86_model < 0x80))
> +			cppc_max_perf = 166;
> +		else
> +			cppc_max_perf = 255;
> +		break;
> +	case 0x19:
> +		if ((boot_cpu_data.x86_model >= 0x20 &&
> +		     boot_cpu_data.x86_model < 0x30) ||
> +		    (boot_cpu_data.x86_model >= 0x40 &&
> +		     boot_cpu_data.x86_model < 0x70))
> +			cppc_max_perf = 166;
> +		else
> +			cppc_max_perf = 255;
> +		break;
> +	default:
> +		cppc_max_perf = 255;
> +		break;
> +	}
> +
> +	return cppc_max_perf;
> +}

Why is this here and not in arch/x86/kernel/cpu/amd.c?

> +
>  static bool amd_set_max_freq_ratio(void)
>  {
>  	struct cppc_perf_caps perf_caps;



> @@ -2046,8 +2077,8 @@ static bool amd_set_max_freq_ratio(void)
>  		return false;
>  	}
>  
> -	highest_perf = perf_caps.highest_perf;
>  	nominal_perf = perf_caps.nominal_perf;
> +	highest_perf = amd_get_highest_perf();
>  
>  	if (!highest_perf || !nominal_perf) {
>  		pr_debug("Could not retrieve highest or nominal performance\n");
> diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> index d1bbc16fba4b..e5c03360db20 100644
> --- a/drivers/cpufreq/acpi-cpufreq.c
> +++ b/drivers/cpufreq/acpi-cpufreq.c
> @@ -630,6 +630,44 @@ static int acpi_cpufreq_blacklist(struct cpuinfo_x86 *c)
>  #endif
>  
>  #ifdef CONFIG_ACPI_CPPC_LIB
> +
> +static u64 get_amd_max_boost_ratio(unsigned int cpu, u64 nominal_perf)
> +{
> +	u64 boost_ratio, cppc_max_perf;
> +
> +	if (!nominal_perf)
> +		return 0;
> +
> +	switch (boot_cpu_data.x86) {
> +	case 0x17:
> +		if ((boot_cpu_data.x86_model >= 0x30 &&
> +		     boot_cpu_data.x86_model < 0x40) ||
> +		    (boot_cpu_data.x86_model >= 0x70 &&
> +		     boot_cpu_data.x86_model < 0x80))
> +			cppc_max_perf = 166;
> +		else
> +			cppc_max_perf = 255;
> +		break;
> +	case 0x19:
> +		if ((boot_cpu_data.x86_model >= 0x20 &&
> +		     boot_cpu_data.x86_model < 0x30) ||
> +		    (boot_cpu_data.x86_model >= 0x40 &&
> +		     boot_cpu_data.x86_model < 0x70))
> +			cppc_max_perf = 166;
> +		else
> +			cppc_max_perf = 255;
> +		break;
> +	default:
> +		cppc_max_perf = 255;
> +		break;

This chunk is repeated here. Why?

-- 
Regards/Gruss,
    Boris.

SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] x86, sched: Fix the AMD CPPC maximum perf on some specific generations
  2021-04-20  8:22 ` Borislav Petkov
@ 2021-04-20 10:48   ` Huang Rui
  0 siblings, 0 replies; 3+ messages in thread
From: Huang Rui @ 2021-04-20 10:48 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: linux-pm, linux-kernel, Deucher, Alexander, Jason Bagavatsingham,
	Pierre-Loup A . Griffais, Fontenot, Nathan, Rafael J . Wysocki,
	x86

On Tue, Apr 20, 2021 at 04:22:31PM +0800, Borislav Petkov wrote:
> On Tue, Apr 20, 2021 at 04:09:43PM +0800, Huang Rui wrote:
> > Some AMD Ryzen generations has different calculation method on maximum
> > perf. 255 is not for all asics, some specific generations used 166 as
> > the maximum perf. This patch is to fix the different maximum perf value
> 
> Avoid having "This patch" or "This commit" in the commit message. It is
> tautologically useless.
> 
> Also, do
> 
> $ git grep 'This patch' Documentation/process
> 
> for more details.

Thanks and good to know, I will enhance the commit message in V2.

> 
> > of AMD CPPC.
> > 
> > Fixes: 41ea667227ba ("x86, sched: Calculate frequency invariance for AMD systems")
> > Fixes: 3c55e94c0ade ("cpufreq: ACPI: Extend frequency tables to cover boost frequencies")
> > 
> > Reported-by: Jason Bagavatsingham <jason.bagavatsingham@gmail.com>
> > Tested-by: Jason Bagavatsingham <jason.bagavatsingham@gmail.com>
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Nathan Fontenot <nathan.fontenot@amd.com>
> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Cc: Borislav Petkov <bp@suse.de>
> > Cc: x86@kernel.org
> > ---
> >  arch/x86/kernel/smpboot.c      | 33 ++++++++++++++++++++++++++-
> >  drivers/cpufreq/acpi-cpufreq.c | 41 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 73 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> > index 02813a7f3a7c..705bc5ceb1ea 100644
> > --- a/arch/x86/kernel/smpboot.c
> > +++ b/arch/x86/kernel/smpboot.c
> > @@ -2033,6 +2033,37 @@ static bool intel_set_max_freq_ratio(void)
> >  }
> >  
> >  #ifdef CONFIG_ACPI_CPPC_LIB
> > +static u64 amd_get_highest_perf(void)
> > +{
> 
> 	struct cpuinfo_x86 *c = &boot_cpu_data;
> 
> and then you can use "c" everywhere.
> 
> > +	u64 cppc_max_perf;
> 
> u64 for something which fits in a byte?
> 
> Also,
> 	max_perf = 255;
> 
> and you can remove the else and default branches below.

I aligned with highest_perf type in get_max_boost_ratio() funciton.

Will clean the "else" and "default" branches in V2.

> 
> > +
> > +	switch (boot_cpu_data.x86) {
> > +	case 0x17:
> > +		if ((boot_cpu_data.x86_model >= 0x30 &&
> > +		     boot_cpu_data.x86_model < 0x40) ||
> > +		    (boot_cpu_data.x86_model >= 0x70 &&
> > +		     boot_cpu_data.x86_model < 0x80))
> > +			cppc_max_perf = 166;
> > +		else
> > +			cppc_max_perf = 255;
> > +		break;
> > +	case 0x19:
> > +		if ((boot_cpu_data.x86_model >= 0x20 &&
> > +		     boot_cpu_data.x86_model < 0x30) ||
> > +		    (boot_cpu_data.x86_model >= 0x40 &&
> > +		     boot_cpu_data.x86_model < 0x70))
> > +			cppc_max_perf = 166;
> > +		else
> > +			cppc_max_perf = 255;
> > +		break;
> > +	default:
> > +		cppc_max_perf = 255;
> > +		break;
> > +	}
> > +
> > +	return cppc_max_perf;
> > +}
> 
> Why is this here and not in arch/x86/kernel/cpu/amd.c?

OK, I will modify to abstract this function in amd.c and then call it both
on smpboot and acpi-cpufreq.

> 
> > +
> >  static bool amd_set_max_freq_ratio(void)
> >  {
> >  	struct cppc_perf_caps perf_caps;
> 
> 
> 
> > @@ -2046,8 +2077,8 @@ static bool amd_set_max_freq_ratio(void)
> >  		return false;
> >  	}
> >  
> > -	highest_perf = perf_caps.highest_perf;
> >  	nominal_perf = perf_caps.nominal_perf;
> > +	highest_perf = amd_get_highest_perf();
> >  
> >  	if (!highest_perf || !nominal_perf) {
> >  		pr_debug("Could not retrieve highest or nominal performance\n");
> > diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c
> > index d1bbc16fba4b..e5c03360db20 100644
> > --- a/drivers/cpufreq/acpi-cpufreq.c
> > +++ b/drivers/cpufreq/acpi-cpufreq.c
> > @@ -630,6 +630,44 @@ static int acpi_cpufreq_blacklist(struct cpuinfo_x86 *c)
> >  #endif
> >  
> >  #ifdef CONFIG_ACPI_CPPC_LIB
> > +
> > +static u64 get_amd_max_boost_ratio(unsigned int cpu, u64 nominal_perf)
> > +{
> > +	u64 boost_ratio, cppc_max_perf;
> > +
> > +	if (!nominal_perf)
> > +		return 0;
> > +
> > +	switch (boot_cpu_data.x86) {
> > +	case 0x17:
> > +		if ((boot_cpu_data.x86_model >= 0x30 &&
> > +		     boot_cpu_data.x86_model < 0x40) ||
> > +		    (boot_cpu_data.x86_model >= 0x70 &&
> > +		     boot_cpu_data.x86_model < 0x80))
> > +			cppc_max_perf = 166;
> > +		else
> > +			cppc_max_perf = 255;
> > +		break;
> > +	case 0x19:
> > +		if ((boot_cpu_data.x86_model >= 0x20 &&
> > +		     boot_cpu_data.x86_model < 0x30) ||
> > +		    (boot_cpu_data.x86_model >= 0x40 &&
> > +		     boot_cpu_data.x86_model < 0x70))
> > +			cppc_max_perf = 166;
> > +		else
> > +			cppc_max_perf = 255;
> > +		break;
> > +	default:
> > +		cppc_max_perf = 255;
> > +		break;
> 
> This chunk is repeated here. Why?
> 

Yes, I should abstract the funciton in amd.c and avoid the repeated
implementation.

Thanks,
Ray

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-20 10:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-20  8:09 [PATCH] x86, sched: Fix the AMD CPPC maximum perf on some specific generations Huang Rui
2021-04-20  8:22 ` Borislav Petkov
2021-04-20 10:48   ` Huang Rui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).