| Message ID | 20230811112116.2279419-3-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Updates to amd-pmc driver | expand |
On 8/11/2023 6:21 AM, Shyam Sundar S K wrote: > Currently the SMU version is being read at multiple places, unify all > of them and get the SMU version at the time of probe. > > 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 | 39 +++++------------------------- > 1 file changed, 6 insertions(+), 33 deletions(-) > It's actually quite intentional that the SMU version isn't read at probe but rather at first use. The reason is that it increased boot time in a measurable way. > diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c > index c1e788b67a74..043451fabbbe 100644 > --- a/drivers/platform/x86/amd/pmc/pmc.c > +++ b/drivers/platform/x86/amd/pmc/pmc.c > @@ -405,12 +405,6 @@ static ssize_t smu_fw_version_show(struct device *d, struct device_attribute *at > { > struct amd_pmc_dev *dev = dev_get_drvdata(d); > > - if (!dev->major) { > - int rc = amd_pmc_get_smu_version(dev); > - > - if (rc) > - return rc; > - } > return sysfs_emit(buf, "%u.%u.%u\n", dev->major, dev->minor, dev->rev); > } > > @@ -419,12 +413,6 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr, > { > struct amd_pmc_dev *dev = dev_get_drvdata(d); > > - if (!dev->major) { > - int rc = amd_pmc_get_smu_version(dev); > - > - if (rc) > - return rc; > - } > return sysfs_emit(buf, "%u\n", dev->smu_program); > } > > @@ -526,16 +514,9 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev, > struct seq_file *s) > { > u32 val; > - int rc; > > switch (pdev->cpu_id) { > case AMD_CPU_ID_CZN: > - /* we haven't yet read SMU version */ > - if (!pdev->major) { > - rc = amd_pmc_get_smu_version(pdev); > - if (rc) > - return rc; > - } > if (pdev->major > 56 || (pdev->major >= 55 && pdev->minor >= 37)) > val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN); > else > @@ -717,13 +698,6 @@ static int amd_pmc_get_os_hint(struct amd_pmc_dev *dev) > static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) > { > struct device *d; > - int rc; > - > - if (!pdev->major) { > - rc = amd_pmc_get_smu_version(pdev); > - if (rc) > - return rc; > - } > > if (pdev->major > 64 || (pdev->major == 64 && pdev->minor > 65)) > return 0; > @@ -749,13 +723,6 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) > struct rtc_time tm; > int rc; > > - /* we haven't yet read SMU version */ > - if (!pdev->major) { > - rc = amd_pmc_get_smu_version(pdev); > - if (rc) > - return rc; > - } > - > if (pdev->major < 64 || (pdev->major == 64 && pdev->minor < 53)) > return 0; > > @@ -1059,6 +1026,12 @@ static int amd_pmc_probe(struct platform_device *pdev) > > mutex_init(&dev->lock); > > + err = amd_pmc_get_smu_version(dev); > + if (err) { > + dev_err(dev->dev, "error reading SMU version\n"); > + goto err_pci_dev_put; > + } > + > if (enable_stb && amd_pmc_is_stb_supported(dev)) { > err = amd_pmc_s2d_init(dev); > if (err)
Hi Mario, On 8/11/2023 5:34 PM, Limonciello, Mario wrote: > > > On 8/11/2023 6:21 AM, Shyam Sundar S K wrote: >> Currently the SMU version is being read at multiple places, unify all >> of them and get the SMU version at the time of probe. >> >> 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 | 39 +++++------------------------- >> 1 file changed, 6 insertions(+), 33 deletions(-) >> > > It's actually quite intentional that the SMU version isn't read at probe > but rather at first use. The reason is that it increased boot time in a > measurable way. Apologies. I missed to respond back on this. I will drop this patch in v2. Thanks, Shyam > >> diff --git a/drivers/platform/x86/amd/pmc/pmc.c >> b/drivers/platform/x86/amd/pmc/pmc.c >> index c1e788b67a74..043451fabbbe 100644 >> --- a/drivers/platform/x86/amd/pmc/pmc.c >> +++ b/drivers/platform/x86/amd/pmc/pmc.c >> @@ -405,12 +405,6 @@ static ssize_t smu_fw_version_show(struct device >> *d, struct device_attribute *at >> { >> struct amd_pmc_dev *dev = dev_get_drvdata(d); >> - if (!dev->major) { >> - int rc = amd_pmc_get_smu_version(dev); >> - >> - if (rc) >> - return rc; >> - } >> return sysfs_emit(buf, "%u.%u.%u\n", dev->major, dev->minor, >> dev->rev); >> } >> @@ -419,12 +413,6 @@ static ssize_t smu_program_show(struct device >> *d, struct device_attribute *attr, >> { >> struct amd_pmc_dev *dev = dev_get_drvdata(d); >> - if (!dev->major) { >> - int rc = amd_pmc_get_smu_version(dev); >> - >> - if (rc) >> - return rc; >> - } >> return sysfs_emit(buf, "%u\n", dev->smu_program); >> } >> @@ -526,16 +514,9 @@ static int amd_pmc_idlemask_read(struct >> amd_pmc_dev *pdev, struct device *dev, >> struct seq_file *s) >> { >> u32 val; >> - int rc; >> switch (pdev->cpu_id) { >> case AMD_CPU_ID_CZN: >> - /* we haven't yet read SMU version */ >> - if (!pdev->major) { >> - rc = amd_pmc_get_smu_version(pdev); >> - if (rc) >> - return rc; >> - } >> if (pdev->major > 56 || (pdev->major >= 55 && pdev->minor >= >> 37)) >> val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN); >> else >> @@ -717,13 +698,6 @@ static int amd_pmc_get_os_hint(struct amd_pmc_dev >> *dev) >> static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) >> { >> struct device *d; >> - int rc; >> - >> - if (!pdev->major) { >> - rc = amd_pmc_get_smu_version(pdev); >> - if (rc) >> - return rc; >> - } >> if (pdev->major > 64 || (pdev->major == 64 && pdev->minor > 65)) >> return 0; >> @@ -749,13 +723,6 @@ static int amd_pmc_verify_czn_rtc(struct >> amd_pmc_dev *pdev, u32 *arg) >> struct rtc_time tm; >> int rc; >> - /* we haven't yet read SMU version */ >> - if (!pdev->major) { >> - rc = amd_pmc_get_smu_version(pdev); >> - if (rc) >> - return rc; >> - } >> - >> if (pdev->major < 64 || (pdev->major == 64 && pdev->minor < 53)) >> return 0; >> @@ -1059,6 +1026,12 @@ static int amd_pmc_probe(struct >> platform_device *pdev) >> mutex_init(&dev->lock); >> + err = amd_pmc_get_smu_version(dev); >> + if (err) { >> + dev_err(dev->dev, "error reading SMU version\n"); >> + goto err_pci_dev_put; >> + } >> + >> if (enable_stb && amd_pmc_is_stb_supported(dev)) { >> err = amd_pmc_s2d_init(dev); >> if (err)
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c index c1e788b67a74..043451fabbbe 100644 --- a/drivers/platform/x86/amd/pmc/pmc.c +++ b/drivers/platform/x86/amd/pmc/pmc.c @@ -405,12 +405,6 @@ static ssize_t smu_fw_version_show(struct device *d, struct device_attribute *at { struct amd_pmc_dev *dev = dev_get_drvdata(d); - if (!dev->major) { - int rc = amd_pmc_get_smu_version(dev); - - if (rc) - return rc; - } return sysfs_emit(buf, "%u.%u.%u\n", dev->major, dev->minor, dev->rev); } @@ -419,12 +413,6 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr, { struct amd_pmc_dev *dev = dev_get_drvdata(d); - if (!dev->major) { - int rc = amd_pmc_get_smu_version(dev); - - if (rc) - return rc; - } return sysfs_emit(buf, "%u\n", dev->smu_program); } @@ -526,16 +514,9 @@ static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev, struct seq_file *s) { u32 val; - int rc; switch (pdev->cpu_id) { case AMD_CPU_ID_CZN: - /* we haven't yet read SMU version */ - if (!pdev->major) { - rc = amd_pmc_get_smu_version(pdev); - if (rc) - return rc; - } if (pdev->major > 56 || (pdev->major >= 55 && pdev->minor >= 37)) val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN); else @@ -717,13 +698,6 @@ static int amd_pmc_get_os_hint(struct amd_pmc_dev *dev) static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) { struct device *d; - int rc; - - if (!pdev->major) { - rc = amd_pmc_get_smu_version(pdev); - if (rc) - return rc; - } if (pdev->major > 64 || (pdev->major == 64 && pdev->minor > 65)) return 0; @@ -749,13 +723,6 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) struct rtc_time tm; int rc; - /* we haven't yet read SMU version */ - if (!pdev->major) { - rc = amd_pmc_get_smu_version(pdev); - if (rc) - return rc; - } - if (pdev->major < 64 || (pdev->major == 64 && pdev->minor < 53)) return 0; @@ -1059,6 +1026,12 @@ static int amd_pmc_probe(struct platform_device *pdev) mutex_init(&dev->lock); + err = amd_pmc_get_smu_version(dev); + if (err) { + dev_err(dev->dev, "error reading SMU version\n"); + goto err_pci_dev_put; + } + if (enable_stb && amd_pmc_is_stb_supported(dev)) { err = amd_pmc_s2d_init(dev); if (err)