Hi Mario, On 4/11/22 16:38, Mario Limonciello wrote: > SMU logging is setup when the device is probed currently. > > In analyzing boot performance it was observed that amd_pmc_probe is > taking ~116800us on startup on an OEM platform. This is longer than > expected, and is caused by enabling SMU logging at startup. > > As the SMU logging is only needed for debugging, initialize it only upon > use. This decreases the time for amd_pmc_probe to ~28800us. > > Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > v1->v2: > * Drop extra check for dev->active_ips as already validated in get_metrics_table > * Add tag Thank you for your patch-series, I've applied the series to my review-hans branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans Note I had to resolve some trivial conflicts manually because your base did not include the "platform/x86: amd-pmc: Fix compilation without CONFIG_SUSPEND" patch (1), please double check the results. Once I've run some tests on this branch the patches there will be added to the platform-drivers-x86/for-next branch and eventually will be included in the pdx86 pull-request to Linus for the next merge-window. Regards, Hans 1) To avoid this in the future please use the: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans branch as base in the future. > > drivers/platform/x86/amd-pmc.c | 47 ++++++++++++++++++++-------------- > 1 file changed, 28 insertions(+), 19 deletions(-) > > diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c > index e9d0dbbb2887..103ba0729b2a 100644 > --- a/drivers/platform/x86/amd-pmc.c > +++ b/drivers/platform/x86/amd-pmc.c > @@ -162,6 +162,7 @@ static struct amd_pmc_dev pmc; > static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret); > static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data); > static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf); > +static int amd_pmc_setup_smu_logging(struct amd_pmc_dev *dev); > > static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset) > { > @@ -319,6 +320,13 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev, > > static int get_metrics_table(struct amd_pmc_dev *pdev, struct smu_metrics *table) > { > + if (!pdev->smu_virt_addr) { > + int ret = amd_pmc_setup_smu_logging(pdev); > + > + if (ret) > + return ret; > + } > + > if (pdev->cpu_id == AMD_CPU_ID_PCO) > return -ENODEV; > memcpy_fromio(table, pdev->smu_virt_addr, sizeof(struct smu_metrics)); > @@ -447,25 +455,32 @@ static inline void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev) > > static int amd_pmc_setup_smu_logging(struct amd_pmc_dev *dev) > { > - u32 phys_addr_low, phys_addr_hi; > - u64 smu_phys_addr; > - > - if (dev->cpu_id == AMD_CPU_ID_PCO) > + if (dev->cpu_id == AMD_CPU_ID_PCO) { > + dev_warn_once(dev->dev, "SMU debugging info not supported on this platform\n"); > return -EINVAL; > + } > > /* Get Active devices list from SMU */ > - amd_pmc_send_cmd(dev, 0, &dev->active_ips, SMU_MSG_GET_SUP_CONSTRAINTS, 1); > + if (!dev->active_ips) > + amd_pmc_send_cmd(dev, 0, &dev->active_ips, SMU_MSG_GET_SUP_CONSTRAINTS, 1); > > /* Get dram address */ > - amd_pmc_send_cmd(dev, 0, &phys_addr_low, SMU_MSG_LOG_GETDRAM_ADDR_LO, 1); > - amd_pmc_send_cmd(dev, 0, &phys_addr_hi, SMU_MSG_LOG_GETDRAM_ADDR_HI, 1); > - smu_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low); > - > - dev->smu_virt_addr = devm_ioremap(dev->dev, smu_phys_addr, sizeof(struct smu_metrics)); > - if (!dev->smu_virt_addr) > - return -ENOMEM; > + if (!dev->smu_virt_addr) { > + u32 phys_addr_low, phys_addr_hi; > + u64 smu_phys_addr; > + > + amd_pmc_send_cmd(dev, 0, &phys_addr_low, SMU_MSG_LOG_GETDRAM_ADDR_LO, 1); > + amd_pmc_send_cmd(dev, 0, &phys_addr_hi, SMU_MSG_LOG_GETDRAM_ADDR_HI, 1); > + smu_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low); > + > + dev->smu_virt_addr = devm_ioremap(dev->dev, smu_phys_addr, > + sizeof(struct smu_metrics)); > + if (!dev->smu_virt_addr) > + return -ENOMEM; > + } > > /* Start the logging */ > + amd_pmc_send_cmd(dev, 0, NULL, SMU_MSG_LOG_RESET, 0); > amd_pmc_send_cmd(dev, 0, NULL, SMU_MSG_LOG_START, 0); > > return 0; > @@ -634,8 +649,7 @@ static void amd_pmc_s2idle_prepare(void) > u32 arg = 1; > > /* Reset and Start SMU logging - to monitor the s0i3 stats */ > - amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0); > - amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0); > + amd_pmc_setup_smu_logging(pdev); > > /* Activate CZN specific RTC functionality */ > if (pdev->cpu_id == AMD_CPU_ID_CZN) { > @@ -846,11 +860,6 @@ static int amd_pmc_probe(struct platform_device *pdev) > goto err_pci_dev_put; > } > > - /* Use SMU to get the s0i3 debug stats */ > - err = amd_pmc_setup_smu_logging(dev); > - if (err) > - dev_err(dev->dev, "SMU debugging info not supported on this platform\n"); > - > if (enable_stb && dev->cpu_id == AMD_CPU_ID_YC) { > err = amd_pmc_s2d_init(dev); > if (err)
