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.bagavatsing...@gmail.com> > > Tested-by: Jason Bagavatsingham <jason.bagavatsing...@gmail.com> > > Signed-off-by: Huang Rui <ray.hu...@amd.com> > > Cc: Alex Deucher <alexander.deuc...@amd.com> > > Cc: Nathan Fontenot <nathan.fonte...@amd.com> > > Cc: Rafael J. Wysocki <rafael.j.wyso...@intel.com> > > Cc: Borislav Petkov <b...@suse.de> > > Cc: x...@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