| Message ID | 20240813095115.2078-3-gautham.shenoy@amd.com |
|---|---|
| State | Accepted |
| Commit | 0d8584d288a9b4132e945d76bcc04395d158b2e7 |
| Headers | show |
| Series | cpufreq/amd-pstate: A set of fixes | expand |
Hi Gautham, On 2024/8/13 17:54, Gautham R. Shenoy wrote: > ... > > This feature bit corresponds to CPUID 0x80000008.ebx[27] which is a > reserved bit on the Zen1 and Zen2 platforms, and is expected to be > cleared on these platforms. Thus printing the warning message for Zen1 > and Zen2 models when X86_FEATURE_CPPC is incorrect. Fix this. > > ... > if (!cpu_feature_enabled(X86_FEATURE_CPPC)) { > - if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) { > - if (c->x86_model > 0x60 && c->x86_model < 0xaf) > - warn = true; Some models of ZEN2 APU/CPU require this warning info, like Renoir (Ryzen 7 4800H mobile APU/4750G desktop APU, Ryzen 5 4600 desktop CPU), Lucienne (Ryzen 5 5500U mobile APU) and Aerith (APU of Steam Deck console). So it has to use model ID to narrow down the coverage. For ZEN1 APU/CPU, this warning can be removed completely, because ZEN1 doesn't support CPPC. Thanks, Xiaojian > - } else if (cpu_feature_enabled(X86_FEATURE_ZEN3) || cpu_feature_enabled(X86_FEATURE_ZEN4)) { > + if (cpu_feature_enabled(X86_FEATURE_ZEN3) || > + cpu_feature_enabled(X86_FEATURE_ZEN4)) { > if ((c->x86_model > 0x10 && c->x86_model < 0x1F) || > (c->x86_model > 0x40 && c->x86_model < 0xaf)) > warn = true;
Hello Xiaojian, On Wed, Aug 14, 2024 at 05:29:04PM +0800, Xiaojian Du wrote: > Hi Gautham, > > On 2024/8/13 17:54, Gautham R. Shenoy wrote: > > ... > > > > This feature bit corresponds to CPUID 0x80000008.ebx[27] which is a > > reserved bit on the Zen1 and Zen2 platforms, and is expected to be > > cleared on these platforms. Thus printing the warning message for Zen1 > > and Zen2 models when X86_FEATURE_CPPC is incorrect. Fix this. > > > > ... > > if (!cpu_feature_enabled(X86_FEATURE_CPPC)) { > > - if (cpu_feature_enabled(X86_FEATURE_ZEN1) || cpu_feature_enabled(X86_FEATURE_ZEN2)) { > > - if (c->x86_model > 0x60 && c->x86_model < 0xaf) > > - warn = true; > > Some models of ZEN2 APU/CPU require this warning info, like Renoir (Ryzen 7 > 4800H mobile APU/4750G desktop APU, > Ryzen 5 4600 desktop CPU), Lucienne (Ryzen 5 5500U mobile APU) and Aerith > (APU of Steam Deck console). > > So it has to use model ID to narrow down the coverage. > I checked the publicly available PPRs of the Family 17h models 0x60 (Renoir) [1] and Family 17h model 0x71 (Matisse) [2]. In both these PPRs, CPUID 0x80000008 EBX[27] is a reserved bit. In fact, David reported this issue on Matisse system. I am happy to retain the warnings for specific models which are known to have support for the CPPC MSRs (which is what CPUID 0x80000008 EBX[27] advertises). Could you please share the model numbers of those that you are aware of? [1] PPR Family 17h Model 0x60 : https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/55922-A1-PUB.zip [2] PPR Family 17h Model 0x71 : https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/programmer-references/56176_ppr_Family_17h_Model_71h_B0_pub_Rev_3_06.zip > For ZEN1 APU/CPU, this warning can be removed completely, because ZEN1 > doesn't support CPPC. Agreed. > > Thanks, > Xiaojian -- Thanks and Regards gautham.
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 358bd88cd0c5..89bda7a2bb8d 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -321,7 +321,7 @@ static inline int pstate_enable(bool enable) return 0; for_each_present_cpu(cpu) { - unsigned long logical_id = topology_logical_die_id(cpu); + unsigned long logical_id = topology_logical_package_id(cpu); if (test_bit(logical_id, &logical_proc_id_mask)) continue;
After the commit 63edbaa48a57 ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf"), the topolgy_logical_die_id() function returns the logical Core Chiplet Die (CCD) ID instead of the logical socket ID. Since this is currently used to set MSR_AMD_CPPC_ENABLE, which needs to be set on any one of the threads of the socket, it is prudent to use topology_logical_package_id() in place of topology_logical_die_id(). Fixes: 63edbaa48a57 ("x86/cpu/topology: Add support for the AMD 0x80000026 leaf") cc: stable@vger.kernel.org # 6.10 Signed-off-by: Gautham R. Shenoy <gautham.shenoy@amd.com> Tested-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> Link: https://lore.kernel.org/lkml/20240801124509.3650-1-Dhananjay.Ugwekar@amd.com/ Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@amd.com> --- drivers/cpufreq/amd-pstate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)