| Message ID | 20241107072714.943423-2-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86/amd/pmc: Updates to AMD PMC driver | expand |
On Thu, 7 Nov 2024, Shyam Sundar S K wrote: > Transfer the support for STB-related file operations to the > amd_pmc_s2d_init() function, thereby consolidating the STB and S2D > (Spill to DRAM) functionality in one location. > > For older platforms that supported S2D, exit immediately after creating > debugfs. These platforms may not support the PMFW messages available on > newer platforms. This adjustment is necessary due to the relocation of > debugfs creation into amd_pmc_s2d_init(). > > Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> > Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com> > Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/platform/x86/amd/pmc/pmc.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c > index bbb8edb62e00..54ceb2f9bf56 100644 > --- a/drivers/platform/x86/amd/pmc/pmc.c > +++ b/drivers/platform/x86/amd/pmc/pmc.c > @@ -648,15 +648,6 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev) > &s0ix_stats_fops); > debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev, > &amd_pmc_idlemask_fops); > - /* Enable STB only when the module_param is set */ > - if (enable_stb) { > - if (amd_pmc_is_stb_supported(dev)) > - debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, > - &amd_pmc_stb_debugfs_fops_v2); > - else > - debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, > - &amd_pmc_stb_debugfs_fops); > - } > } > > static void amd_pmc_dump_registers(struct amd_pmc_dev *dev) > @@ -982,6 +973,15 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) > u32 size = 0; > int ret; > > + if (amd_pmc_is_stb_supported(dev)) { > + debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, > + &amd_pmc_stb_debugfs_fops_v2); > + } else { > + debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, > + &amd_pmc_stb_debugfs_fops); > + return 0; > + } > + > /* Spill to DRAM feature uses separate SMU message port */ > dev->msg_port = 1; This now runs afoul the other issue you even mentioned yourself (IIRC), that is, dev->dbgfs_dir is initialized inside amd_pmc_dbgfs_register() which is only called after amd_pmc_s2d_init() until it is moved in patch 2. Thus, you need to combine patches 1 & 2 so you don't get a broken kernel after this patch. Please also move the enable_stb check inside amd_pmc_s2d_init() in this patch since that's another thing you've now broken in between patches 1 & 3. So to reiterate, in the first patch combine: Patch 1 + 2 + the if () logic move from amd_pmc_probe() into amd_pmc_s2d_init().
On 11/7/2024 16:10, Ilpo Järvinen wrote: > On Thu, 7 Nov 2024, Shyam Sundar S K wrote: > >> Transfer the support for STB-related file operations to the >> amd_pmc_s2d_init() function, thereby consolidating the STB and S2D >> (Spill to DRAM) functionality in one location. >> >> For older platforms that supported S2D, exit immediately after creating >> debugfs. These platforms may not support the PMFW messages available on >> newer platforms. This adjustment is necessary due to the relocation of >> debugfs creation into amd_pmc_s2d_init(). >> >> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> >> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com> >> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >> --- >> drivers/platform/x86/amd/pmc/pmc.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c >> index bbb8edb62e00..54ceb2f9bf56 100644 >> --- a/drivers/platform/x86/amd/pmc/pmc.c >> +++ b/drivers/platform/x86/amd/pmc/pmc.c >> @@ -648,15 +648,6 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev) >> &s0ix_stats_fops); >> debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev, >> &amd_pmc_idlemask_fops); >> - /* Enable STB only when the module_param is set */ >> - if (enable_stb) { >> - if (amd_pmc_is_stb_supported(dev)) >> - debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, >> - &amd_pmc_stb_debugfs_fops_v2); >> - else >> - debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, >> - &amd_pmc_stb_debugfs_fops); >> - } >> } >> >> static void amd_pmc_dump_registers(struct amd_pmc_dev *dev) >> @@ -982,6 +973,15 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) >> u32 size = 0; >> int ret; >> >> + if (amd_pmc_is_stb_supported(dev)) { >> + debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, >> + &amd_pmc_stb_debugfs_fops_v2); >> + } else { >> + debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, >> + &amd_pmc_stb_debugfs_fops); >> + return 0; >> + } >> + >> /* Spill to DRAM feature uses separate SMU message port */ >> dev->msg_port = 1; > > This now runs afoul the other issue you even mentioned yourself (IIRC), > that is, dev->dbgfs_dir is initialized inside amd_pmc_dbgfs_register() > which is only called after amd_pmc_s2d_init() until it is moved in patch > 2. > > Thus, you need to combine patches 1 & 2 so you don't get a broken kernel > after this patch. > > Please also move the enable_stb check inside amd_pmc_s2d_init() in this > patch since that's another thing you've now broken in between patches 1 & > 3. > > So to reiterate, in the first patch combine: Patch 1 + 2 + the if () logic > move from amd_pmc_probe() into amd_pmc_s2d_init(). > OK. Will do it. Thanks, Shyam
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c index bbb8edb62e00..54ceb2f9bf56 100644 --- a/drivers/platform/x86/amd/pmc/pmc.c +++ b/drivers/platform/x86/amd/pmc/pmc.c @@ -648,15 +648,6 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev) &s0ix_stats_fops); debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev, &amd_pmc_idlemask_fops); - /* Enable STB only when the module_param is set */ - if (enable_stb) { - if (amd_pmc_is_stb_supported(dev)) - debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, - &amd_pmc_stb_debugfs_fops_v2); - else - debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, - &amd_pmc_stb_debugfs_fops); - } } static void amd_pmc_dump_registers(struct amd_pmc_dev *dev) @@ -982,6 +973,15 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev) u32 size = 0; int ret; + if (amd_pmc_is_stb_supported(dev)) { + debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, + &amd_pmc_stb_debugfs_fops_v2); + } else { + debugfs_create_file("stb_read", 0644, dev->dbgfs_dir, dev, + &amd_pmc_stb_debugfs_fops); + return 0; + } + /* Spill to DRAM feature uses separate SMU message port */ dev->msg_port = 1;