On 11/7/2024 16:32, Ilpo Järvinen wrote: > On Thu, 7 Nov 2024, Shyam Sundar S K wrote: > >> The latest AMD processors include additional IP blocks that must be turned >> off before transitioning to low power. PMFW provides an interface to >> retrieve debug information from each IP block, which is useful for >> diagnosing issues if the system fails to enter or exit low power states, >> or for profiling which IP block takes more time. Add support for using >> this information within the driver. >> >> Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx> >> Co-developed-by: Sanket Goswami <Sanket.Goswami@xxxxxxx> >> Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx> >> --- >> drivers/platform/x86/amd/pmc/pmc.c | 42 +++++++++++++++++++++++++++--- >> drivers/platform/x86/amd/pmc/pmc.h | 1 + >> 2 files changed, 40 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c >> index 7c3204110bf8..5b99845d0914 100644 >> --- a/drivers/platform/x86/amd/pmc/pmc.c >> +++ b/drivers/platform/x86/amd/pmc/pmc.c >> @@ -95,6 +95,34 @@ struct amd_pmc_bit_map { >> u32 bit_mask; >> }; >> >> +static const struct amd_pmc_bit_map soc15_ip_blk_v2[] = { >> + {"DISPLAY", BIT(0)}, >> + {"CPU", BIT(1)}, >> + {"GFX", BIT(2)}, >> + {"VDD", BIT(3)}, >> + {"VDD_CCX", BIT(4)}, >> + {"ACP", BIT(5)}, >> + {"VCN_0", BIT(6)}, >> + {"VCN_1", BIT(7)}, >> + {"ISP", BIT(8)}, >> + {"NBIO", BIT(9)}, >> + {"DF", BIT(10)}, >> + {"USB3_0", BIT(11)}, >> + {"USB3_1", BIT(12)}, >> + {"LAPIC", BIT(13)}, >> + {"USB3_2", BIT(14)}, >> + {"USB4_RT0", BIT(15)}, >> + {"USB4_RT1", BIT(16)}, >> + {"USB4_0", BIT(17)}, >> + {"USB4_1", BIT(18)}, >> + {"MPM", BIT(19)}, >> + {"JPEG_0", BIT(20)}, >> + {"JPEG_1", BIT(21)}, >> + {"IPU", BIT(22)}, >> + {"UMSCH", BIT(23)}, >> + {"VPE", BIT(24)}, >> +}; >> + >> static const struct amd_pmc_bit_map soc15_ip_blk[] = { >> {"DISPLAY", BIT(0)}, >> {"CPU", BIT(1)}, >> @@ -162,14 +190,22 @@ static void amd_pmc_get_ip_info(struct amd_pmc_dev *dev) >> case AMD_CPU_ID_CB: >> dev->num_ips = 12; >> dev->smu_msg = 0x538; >> + dev->ips_ptr = (struct amd_pmc_bit_map *)soc15_ip_blk; >> break; >> case AMD_CPU_ID_PS: >> dev->num_ips = 21; >> dev->smu_msg = 0x538; >> + dev->ips_ptr = (struct amd_pmc_bit_map *)soc15_ip_blk; > > Could you please put num_ips and ips_ptr lines next to each other as they > kind of belong together. > > I'm a bit surprised you need the casts in these assignments. ...Surprised > enough I finally went to check what's going on. > > And sadly... It turns out you needed the cast only to get rid of const > which is very bad practice. You really should almost never do that. > ah! I missed this badly.. :-( Thanks for pointing this out. Thanks, Shyam > The correct solution is highlighted below. > >> break; >> case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT: >> case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT: >> - dev->num_ips = ARRAY_SIZE(soc15_ip_blk); >> + if (boot_cpu_data.x86_model == 0x70) { >> + dev->num_ips = ARRAY_SIZE(soc15_ip_blk_v2); >> + dev->ips_ptr = (struct amd_pmc_bit_map *)soc15_ip_blk_v2; >> + } else { >> + dev->num_ips = ARRAY_SIZE(soc15_ip_blk); >> + dev->ips_ptr = (struct amd_pmc_bit_map *)soc15_ip_blk; >> + } >> dev->smu_msg = 0x938; >> break; >> } >> @@ -337,8 +373,8 @@ static int smu_fw_info_show(struct seq_file *s, void *unused) >> >> seq_puts(s, "\n=== Active time (in us) ===\n"); >> for (idx = 0 ; idx < dev->num_ips ; idx++) { >> - if (soc15_ip_blk[idx].bit_mask & dev->active_ips) >> - seq_printf(s, "%-8s : %lld\n", soc15_ip_blk[idx].name, >> + if (dev->ips_ptr[idx].bit_mask & dev->active_ips) >> + seq_printf(s, "%-8s : %lld\n", dev->ips_ptr[idx].name, >> table.timecondition_notmet_lastcapture[idx]); >> } >> >> diff --git a/drivers/platform/x86/amd/pmc/pmc.h b/drivers/platform/x86/amd/pmc/pmc.h >> index 69870013c0fc..f6d9a7c37588 100644 >> --- a/drivers/platform/x86/amd/pmc/pmc.h >> +++ b/drivers/platform/x86/amd/pmc/pmc.h >> @@ -62,6 +62,7 @@ struct amd_pmc_dev { >> bool disable_8042_wakeup; >> struct amd_mp2_dev *mp2; >> struct stb_arg stb_arg; >> + struct amd_pmc_bit_map *ips_ptr; > > const struct > > -- > i. > >> }; >> >> void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev); >>