[Public]
> -----Original Message-----
> From: S-k, Shyam-sundar <Shyam-sundar.S-k@xxxxxxx>
> Sent: Monday, April 10, 2023 12:35
> To: Limonciello, Mario <Mario.Limonciello@xxxxxxx>;
> hdegoede@xxxxxxxxxx; markgross@xxxxxxxxxx
> Cc: Goswami, Sanket <Sanket.Goswami@xxxxxxx>; platform-driver-
> x86@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 7/8] platform/x86/amd: pmc: Get STB DRAM size from
> PMFW
>
>
>
> On 4/10/2023 2:18 AM, Mario Limonciello wrote:
> >
> > On 4/9/23 13:53, Shyam Sundar S K wrote:
> >> Recent PMFW's have support for querying the STB DRAM size. Add this
> >> support to the driver.
> >>
> >> 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.c | 29
> ++++++++++++++++++++++++++++-
> >> 1 file changed, 28 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/platform/x86/amd/pmc.c
> >> b/drivers/platform/x86/amd/pmc.c
> >> index b14d068a6474..86f32b01e3ff 100644
> >> --- a/drivers/platform/x86/amd/pmc.c
> >> +++ b/drivers/platform/x86/amd/pmc.c
> >> @@ -114,6 +114,7 @@ enum s2d_arg {
> >> S2D_PHYS_ADDR_LOW,
> >> S2D_PHYS_ADDR_HIGH,
> >> S2D_NUM_SAMPLES,
> >> + S2D_DRAM_SIZE,
> >> };
> >> struct amd_pmc_bit_map {
> >> @@ -146,6 +147,7 @@ struct amd_pmc_dev {
> >> u32 base_addr;
> >> u32 cpu_id;
> >> u32 active_ips;
> >> + u32 dram_size;
> >> /* SMU version information */
> >> u8 smu_program;
> >> u8 major;
> >> @@ -895,11 +897,31 @@ static const struct pci_device_id pmc_pci_ids[] =
> {
> >> { }
> >> };
> >> +static int amd_pmc_get_dram_size(struct amd_pmc_dev *dev)
> >> +{
> >> + if (dev->cpu_id != AMD_CPU_ID_YC)
> >> + goto err_dram_size;
> >> +
> >> + if (dev->major > 90 || (dev->major == 90 && dev->minor > 39))
> >> + goto err_dram_size;
> >
> > Is this only for YC and not for PS?
>
> Its only YC for now, as this is important of the Chrome work. FW team
> has to still port the same change from YC to PS branch.
>
> Once that is done, we can make other changes too. Agree?
>
Sounds good.
>
> >
> > The version check I think you should make it clear it's only intended
> > for this program.
> >
> > switch(dev->cpu_id) {
> > case AMD_CPU_ID_YC:
> > if (version_check)
> > goto err_dram_size;
> > break;
> >
> > default:
> > goto err_dram_size;
> >
> > }
> >
> > Then when it comes time to add another system it either will need a
> > localized
> > version check or it will just be supported and no check needed. Either
> > way it
> > is cleaner in the code.
> >
> >> +
> >> + amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size,
> >> STB_SPILL_TO_DRAM, 1);
> >> + if (!dev->dram_size)
> >> + goto err_dram_size;
> >> +
> >> + return 0;
> >> +
> >> +err_dram_size:
> >> + dev_err(dev->dev, "DRAM size command not supported for this
> >> platform\n");
> >> + return -EINVAL;
> >> +}
> >> +
> >> static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
> >> {
> >> u32 phys_addr_low, phys_addr_hi;
> >> u64 stb_phys_addr;
> >> u32 size = 0;
> >> + int ret;
> >> /* Spill to DRAM feature uses separate SMU message port */
> >> dev->msg_port = 1;
> >> @@ -908,6 +930,11 @@ static int amd_pmc_s2d_init(struct
> amd_pmc_dev *dev)
> >> if (size != S2D_TELEMETRY_BYTES_MAX)
> >> return -EIO;
> >> + /* Get DRAM size */
> >> + ret = amd_pmc_get_dram_size(dev);
> >> + if (ret)
> >> + dev->dram_size = S2D_TELEMETRY_DRAMBYTES_MAX;
> >> +
> >> /* Get STB DRAM address */
> >> amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_LOW, &phys_addr_low,
> >> STB_SPILL_TO_DRAM, 1);
> >> amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_HIGH, &phys_addr_hi,
> >> STB_SPILL_TO_DRAM, 1);
> >> @@ -917,7 +944,7 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev
> *dev)
> >> /* Clear msg_port for other SMU operation */
> >> dev->msg_port = 0;
> >> - dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr,
> >> S2D_TELEMETRY_DRAMBYTES_MAX);
> >> + dev->stb_virt_addr = devm_ioremap(dev->dev, stb_phys_addr,
> >> dev->dram_size);
> >> if (!dev->stb_virt_addr)
> >> return -ENOMEM;
> >>
