2021-09-16 12:42:10

by Goswami, Sanket

[permalink] [raw]
Subject: [PATCH v2] platform/x86: amd-pmc: Export Idlemask values based on the APU

IdleMask is the metric used by the PM firmware to know the status of each
of the Hardware IP blocks monitored by the PM firmware.

Knowing this value is key to get the information of s2idle suspend/resume
status. This value is mapped to PMC scratch registers, retrieve them
accordingly based on the CPU family and the underlying firmware support.

Co-developed-by: Shyam Sundar S K <[email protected]>
Signed-off-by: Shyam Sundar S K <[email protected]>
Signed-off-by: Sanket Goswami <[email protected]>
---
Changes in v2:
- Add separate routine amd_pmc_idlemask_read to get the value.
- Address review comments from Mario.

drivers/platform/x86/amd-pmc.c | 76 ++++++++++++++++++++++++++++++++++
1 file changed, 76 insertions(+)

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index 3481479a2942..0c970f613e09 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -29,6 +29,10 @@
#define AMD_PMC_REGISTER_RESPONSE 0x980
#define AMD_PMC_REGISTER_ARGUMENT 0x9BC

+/* PMC Scratch Registers */
+#define AMD_PMC_SCRATCH_REG_CZN 0x94
+#define AMD_PMC_SCRATCH_REG_YC 0xD14
+
/* Base address of SMU for mapping physical address to virtual address */
#define AMD_PMC_SMU_INDEX_ADDRESS 0xB8
#define AMD_PMC_SMU_INDEX_DATA 0xBC
@@ -110,6 +114,10 @@ struct amd_pmc_dev {
u32 base_addr;
u32 cpu_id;
u32 active_ips;
+/* SMU version information */
+ u16 major;
+ u16 minor;
+ u16 rev;
struct device *dev;
struct mutex lock; /* generic mutex lock */
#if IS_ENABLED(CONFIG_DEBUG_FS)
@@ -201,6 +209,66 @@ static int s0ix_stats_show(struct seq_file *s, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(s0ix_stats);

+static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
+{
+ int rc;
+ u32 val;
+
+ rc = amd_pmc_send_cmd(dev, 0, &val, SMU_MSG_GETSMUVERSION, 1);
+ if (rc)
+ return rc;
+
+ dev->major = (val >> 16) & GENMASK(15, 0);
+ dev->minor = (val >> 8) & GENMASK(7, 0);
+ dev->rev = (val >> 0) & GENMASK(7, 0);
+
+ dev_dbg(dev->dev, "SMU version is %u.%u.%u\n", dev->major, dev->minor, dev->rev);
+
+ return 0;
+}
+
+static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
+ struct seq_file *s)
+{
+ u32 val;
+
+ switch (pdev->cpu_id) {
+ case AMD_CPU_ID_CZN:
+ val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN);
+ break;
+ case AMD_CPU_ID_YC:
+ val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_YC);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (dev)
+ dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
+
+ if (s)
+ seq_printf(s, "SMU idlemask : 0x%x\n", val);
+
+ return 0;
+}
+
+static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
+{
+ struct amd_pmc_dev *dev = s->private;
+ int rc;
+
+ if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
+ rc = amd_pmc_idlemask_read(dev, NULL, s);
+ if (rc)
+ return rc;
+ } else {
+ seq_puts(s, "Unsupported SMU version for Idlemask\n");
+ }
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(amd_pmc_idlemask);
+
static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev)
{
debugfs_remove_recursive(dev->dbgfs_dir);
@@ -213,6 +281,8 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
&smu_fw_info_fops);
debugfs_create_file("s0ix_stats", 0644, dev->dbgfs_dir, dev,
&s0ix_stats_fops);
+ debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
+ &amd_pmc_idlemask_fops);
}
#else
static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
@@ -349,6 +419,8 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0);
amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0);

+ /* Dump the IdleMask before we send hint to SMU */
+ amd_pmc_idlemask_read(pdev, dev, NULL);
msg = amd_pmc_get_os_hint(pdev);
rc = amd_pmc_send_cmd(pdev, 1, NULL, msg, 0);
if (rc)
@@ -371,6 +443,9 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
if (rc)
dev_err(pdev->dev, "resume failed\n");

+ /* Dump the IdleMask to see the blockers */
+ amd_pmc_idlemask_read(pdev, dev, NULL);
+
return 0;
}

@@ -457,6 +532,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
if (err)
dev_err(dev->dev, "SMU debugging info not supported on this platform\n");

+ amd_pmc_get_smu_version(dev);
platform_set_drvdata(pdev, dev);
amd_pmc_dbgfs_register(dev);
return 0;
--
2.25.1


2021-09-16 13:06:11

by Shyam Sundar S K

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: amd-pmc: Export Idlemask values based on the APU

+Mario

On 9/16/2021 6:10 PM, Sanket Goswami wrote:
> IdleMask is the metric used by the PM firmware to know the status of each
> of the Hardware IP blocks monitored by the PM firmware.
>
> Knowing this value is key to get the information of s2idle suspend/resume
> status. This value is mapped to PMC scratch registers, retrieve them
> accordingly based on the CPU family and the underlying firmware support.
>
> Co-developed-by: Shyam Sundar S K <[email protected]>
> Signed-off-by: Shyam Sundar S K <[email protected]>
> Signed-off-by: Sanket Goswami <[email protected]>
> ---
> Changes in v2:
> - Add separate routine amd_pmc_idlemask_read to get the value.
> - Address review comments from Mario.
>
> drivers/platform/x86/amd-pmc.c | 76 ++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
> index 3481479a2942..0c970f613e09 100644
> --- a/drivers/platform/x86/amd-pmc.c
> +++ b/drivers/platform/x86/amd-pmc.c
> @@ -29,6 +29,10 @@
> #define AMD_PMC_REGISTER_RESPONSE 0x980
> #define AMD_PMC_REGISTER_ARGUMENT 0x9BC
>
> +/* PMC Scratch Registers */
> +#define AMD_PMC_SCRATCH_REG_CZN 0x94
> +#define AMD_PMC_SCRATCH_REG_YC 0xD14
> +
> /* Base address of SMU for mapping physical address to virtual address */
> #define AMD_PMC_SMU_INDEX_ADDRESS 0xB8
> #define AMD_PMC_SMU_INDEX_DATA 0xBC
> @@ -110,6 +114,10 @@ struct amd_pmc_dev {
> u32 base_addr;
> u32 cpu_id;
> u32 active_ips;
> +/* SMU version information */
> + u16 major;
> + u16 minor;
> + u16 rev;
> struct device *dev;
> struct mutex lock; /* generic mutex lock */
> #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -201,6 +209,66 @@ static int s0ix_stats_show(struct seq_file *s, void *unused)
> }
> DEFINE_SHOW_ATTRIBUTE(s0ix_stats);
>
> +static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
> +{
> + int rc;
> + u32 val;
> +
> + rc = amd_pmc_send_cmd(dev, 0, &val, SMU_MSG_GETSMUVERSION, 1);
> + if (rc)
> + return rc;
> +
> + dev->major = (val >> 16) & GENMASK(15, 0);
> + dev->minor = (val >> 8) & GENMASK(7, 0);
> + dev->rev = (val >> 0) & GENMASK(7, 0);
> +
> + dev_dbg(dev->dev, "SMU version is %u.%u.%u\n", dev->major, dev->minor, dev->rev);
> +
> + return 0;
> +}
> +
> +static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
> + struct seq_file *s)
> +{
> + u32 val;
> +
> + switch (pdev->cpu_id) {
> + case AMD_CPU_ID_CZN:
> + val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN);
> + break;
> + case AMD_CPU_ID_YC:
> + val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_YC);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (dev)
> + dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
> +
> + if (s)
> + seq_printf(s, "SMU idlemask : 0x%x\n", val);
> +
> + return 0;
> +}
> +
> +static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
> +{
> + struct amd_pmc_dev *dev = s->private;
> + int rc;
> +
> + if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
> + rc = amd_pmc_idlemask_read(dev, NULL, s);
> + if (rc)
> + return rc;
> + } else {
> + seq_puts(s, "Unsupported SMU version for Idlemask\n");
> + }
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(amd_pmc_idlemask);
> +
> static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev)
> {
> debugfs_remove_recursive(dev->dbgfs_dir);
> @@ -213,6 +281,8 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> &smu_fw_info_fops);
> debugfs_create_file("s0ix_stats", 0644, dev->dbgfs_dir, dev,
> &s0ix_stats_fops);
> + debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
> + &amd_pmc_idlemask_fops);
> }
> #else
> static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> @@ -349,6 +419,8 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
> amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0);
> amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0);
>
> + /* Dump the IdleMask before we send hint to SMU */
> + amd_pmc_idlemask_read(pdev, dev, NULL);
> msg = amd_pmc_get_os_hint(pdev);
> rc = amd_pmc_send_cmd(pdev, 1, NULL, msg, 0);
> if (rc)
> @@ -371,6 +443,9 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
> if (rc)
> dev_err(pdev->dev, "resume failed\n");
>
> + /* Dump the IdleMask to see the blockers */
> + amd_pmc_idlemask_read(pdev, dev, NULL);
> +
> return 0;
> }
>
> @@ -457,6 +532,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
> if (err)
> dev_err(dev->dev, "SMU debugging info not supported on this platform\n");
>
> + amd_pmc_get_smu_version(dev);
> platform_set_drvdata(pdev, dev);
> amd_pmc_dbgfs_register(dev);
> return 0;
>

Looks good to me.

Acked-by: Shyam Sundar S K <[email protected]>

2021-09-16 13:27:38

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: amd-pmc: Export Idlemask values based on the APU

On 9/16/2021 08:00, Shyam Sundar S K wrote:
> +Mario
>
> On 9/16/2021 6:10 PM, Sanket Goswami wrote:
>> IdleMask is the metric used by the PM firmware to know the status of each
>> of the Hardware IP blocks monitored by the PM firmware.
>>
>> Knowing this value is key to get the information of s2idle suspend/resume
>> status. This value is mapped to PMC scratch registers, retrieve them
>> accordingly based on the CPU family and the underlying firmware support.
>>
>> Co-developed-by: Shyam Sundar S K <[email protected]>
>> Signed-off-by: Shyam Sundar S K <[email protected]>
>> Signed-off-by: Sanket Goswami <[email protected]>
>> ---
>> Changes in v2:
>> - Add separate routine amd_pmc_idlemask_read to get the value.
>> - Address review comments from Mario.
>>
>> drivers/platform/x86/amd-pmc.c | 76 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 76 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
>> index 3481479a2942..0c970f613e09 100644
>> --- a/drivers/platform/x86/amd-pmc.c
>> +++ b/drivers/platform/x86/amd-pmc.c
>> @@ -29,6 +29,10 @@
>> #define AMD_PMC_REGISTER_RESPONSE 0x980
>> #define AMD_PMC_REGISTER_ARGUMENT 0x9BC
>>
>> +/* PMC Scratch Registers */
>> +#define AMD_PMC_SCRATCH_REG_CZN 0x94
>> +#define AMD_PMC_SCRATCH_REG_YC 0xD14
>> +
>> /* Base address of SMU for mapping physical address to virtual address */
>> #define AMD_PMC_SMU_INDEX_ADDRESS 0xB8
>> #define AMD_PMC_SMU_INDEX_DATA 0xBC
>> @@ -110,6 +114,10 @@ struct amd_pmc_dev {
>> u32 base_addr;
>> u32 cpu_id;
>> u32 active_ips;
>> +/* SMU version information */
>> + u16 major;
>> + u16 minor;
>> + u16 rev;
>> struct device *dev;
>> struct mutex lock; /* generic mutex lock */
>> #if IS_ENABLED(CONFIG_DEBUG_FS)
>> @@ -201,6 +209,66 @@ static int s0ix_stats_show(struct seq_file *s, void *unused)
>> }
>> DEFINE_SHOW_ATTRIBUTE(s0ix_stats);
>>
>> +static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
>> +{
>> + int rc;
>> + u32 val;
>> +
>> + rc = amd_pmc_send_cmd(dev, 0, &val, SMU_MSG_GETSMUVERSION, 1);
>> + if (rc)
>> + return rc;
>> +
>> + dev->major = (val >> 16) & GENMASK(15, 0);
>> + dev->minor = (val >> 8) & GENMASK(7, 0);
>> + dev->rev = (val >> 0) & GENMASK(7, 0);
>> +
>> + dev_dbg(dev->dev, "SMU version is %u.%u.%u\n", dev->major, dev->minor, dev->rev);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
>> + struct seq_file *s)
>> +{
>> + u32 val;
>> +
>> + switch (pdev->cpu_id) {
>> + case AMD_CPU_ID_CZN:
>> + val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN);
>> + break;
>> + case AMD_CPU_ID_YC:
>> + val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_YC);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + if (dev)
>> + dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
>> +
>> + if (s)
>> + seq_printf(s, "SMU idlemask : 0x%x\n", val);
>> +
>> + return 0;
>> +}
>> +
>> +static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
>> +{
>> + struct amd_pmc_dev *dev = s->private;
>> + int rc;
>> +
>> + if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
>> + rc = amd_pmc_idlemask_read(dev, NULL, s);
>> + if (rc)
>> + return rc;
>> + } else {
>> + seq_puts(s, "Unsupported SMU version for Idlemask\n");
>> + }
>> +
>> + return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(amd_pmc_idlemask);
>> +
>> static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev)
>> {
>> debugfs_remove_recursive(dev->dbgfs_dir);
>> @@ -213,6 +281,8 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
>> &smu_fw_info_fops);
>> debugfs_create_file("s0ix_stats", 0644, dev->dbgfs_dir, dev,
>> &s0ix_stats_fops);
>> + debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
>> + &amd_pmc_idlemask_fops);
>> }
>> #else
>> static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
>> @@ -349,6 +419,8 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
>> amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0);
>> amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0);
>>
>> + /* Dump the IdleMask before we send hint to SMU */
>> + amd_pmc_idlemask_read(pdev, dev, NULL);
>> msg = amd_pmc_get_os_hint(pdev);
>> rc = amd_pmc_send_cmd(pdev, 1, NULL, msg, 0);
>> if (rc)
>> @@ -371,6 +443,9 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
>> if (rc)
>> dev_err(pdev->dev, "resume failed\n");
>>
>> + /* Dump the IdleMask to see the blockers */
>> + amd_pmc_idlemask_read(pdev, dev, NULL);
>> +
>> return 0;
>> }
>>
>> @@ -457,6 +532,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
>> if (err)
>> dev_err(dev->dev, "SMU debugging info not supported on this platform\n");
>>
>> + amd_pmc_get_smu_version(dev);
>> platform_set_drvdata(pdev, dev);
>> amd_pmc_dbgfs_register(dev);
>> return 0;
>>
>
> Looks good to me.
>
> Acked-by: Shyam Sundar S K <[email protected]>
>

Looks good to me too, thanks.

Reviewed-by: Mario Limonciello <[email protected]>

2021-09-16 13:32:32

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: amd-pmc: Export Idlemask values based on the APU

Hi,

On 9/16/21 2:40 PM, Sanket Goswami wrote:
> IdleMask is the metric used by the PM firmware to know the status of each
> of the Hardware IP blocks monitored by the PM firmware.
>
> Knowing this value is key to get the information of s2idle suspend/resume
> status. This value is mapped to PMC scratch registers, retrieve them
> accordingly based on the CPU family and the underlying firmware support.
>
> Co-developed-by: Shyam Sundar S K <[email protected]>
> Signed-off-by: Shyam Sundar S K <[email protected]>
> Signed-off-by: Sanket Goswami <[email protected]>

Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my


local branch there, which might take a while.

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

> ---
> Changes in v2:
> - Add separate routine amd_pmc_idlemask_read to get the value.
> - Address review comments from Mario.
>
> drivers/platform/x86/amd-pmc.c | 76 ++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
> index 3481479a2942..0c970f613e09 100644
> --- a/drivers/platform/x86/amd-pmc.c
> +++ b/drivers/platform/x86/amd-pmc.c
> @@ -29,6 +29,10 @@
> #define AMD_PMC_REGISTER_RESPONSE 0x980
> #define AMD_PMC_REGISTER_ARGUMENT 0x9BC
>
> +/* PMC Scratch Registers */
> +#define AMD_PMC_SCRATCH_REG_CZN 0x94
> +#define AMD_PMC_SCRATCH_REG_YC 0xD14
> +
> /* Base address of SMU for mapping physical address to virtual address */
> #define AMD_PMC_SMU_INDEX_ADDRESS 0xB8
> #define AMD_PMC_SMU_INDEX_DATA 0xBC
> @@ -110,6 +114,10 @@ struct amd_pmc_dev {
> u32 base_addr;
> u32 cpu_id;
> u32 active_ips;
> +/* SMU version information */
> + u16 major;
> + u16 minor;
> + u16 rev;
> struct device *dev;
> struct mutex lock; /* generic mutex lock */
> #if IS_ENABLED(CONFIG_DEBUG_FS)
> @@ -201,6 +209,66 @@ static int s0ix_stats_show(struct seq_file *s, void *unused)
> }
> DEFINE_SHOW_ATTRIBUTE(s0ix_stats);
>
> +static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
> +{
> + int rc;
> + u32 val;
> +
> + rc = amd_pmc_send_cmd(dev, 0, &val, SMU_MSG_GETSMUVERSION, 1);
> + if (rc)
> + return rc;
> +
> + dev->major = (val >> 16) & GENMASK(15, 0);
> + dev->minor = (val >> 8) & GENMASK(7, 0);
> + dev->rev = (val >> 0) & GENMASK(7, 0);
> +
> + dev_dbg(dev->dev, "SMU version is %u.%u.%u\n", dev->major, dev->minor, dev->rev);
> +
> + return 0;
> +}
> +
> +static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
> + struct seq_file *s)
> +{
> + u32 val;
> +
> + switch (pdev->cpu_id) {
> + case AMD_CPU_ID_CZN:
> + val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_CZN);
> + break;
> + case AMD_CPU_ID_YC:
> + val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_YC);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (dev)
> + dev_dbg(pdev->dev, "SMU idlemask s0i3: 0x%x\n", val);
> +
> + if (s)
> + seq_printf(s, "SMU idlemask : 0x%x\n", val);
> +
> + return 0;
> +}
> +
> +static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
> +{
> + struct amd_pmc_dev *dev = s->private;
> + int rc;
> +
> + if (dev->major > 56 || (dev->major >= 55 && dev->minor >= 37)) {
> + rc = amd_pmc_idlemask_read(dev, NULL, s);
> + if (rc)
> + return rc;
> + } else {
> + seq_puts(s, "Unsupported SMU version for Idlemask\n");
> + }
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(amd_pmc_idlemask);
> +
> static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev)
> {
> debugfs_remove_recursive(dev->dbgfs_dir);
> @@ -213,6 +281,8 @@ static void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> &smu_fw_info_fops);
> debugfs_create_file("s0ix_stats", 0644, dev->dbgfs_dir, dev,
> &s0ix_stats_fops);
> + debugfs_create_file("amd_pmc_idlemask", 0644, dev->dbgfs_dir, dev,
> + &amd_pmc_idlemask_fops);
> }
> #else
> static inline void amd_pmc_dbgfs_register(struct amd_pmc_dev *dev)
> @@ -349,6 +419,8 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
> amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0);
> amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0);
>
> + /* Dump the IdleMask before we send hint to SMU */
> + amd_pmc_idlemask_read(pdev, dev, NULL);
> msg = amd_pmc_get_os_hint(pdev);
> rc = amd_pmc_send_cmd(pdev, 1, NULL, msg, 0);
> if (rc)
> @@ -371,6 +443,9 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
> if (rc)
> dev_err(pdev->dev, "resume failed\n");
>
> + /* Dump the IdleMask to see the blockers */
> + amd_pmc_idlemask_read(pdev, dev, NULL);
> +
> return 0;
> }
>
> @@ -457,6 +532,7 @@ static int amd_pmc_probe(struct platform_device *pdev)
> if (err)
> dev_err(dev->dev, "SMU debugging info not supported on this platform\n");
>
> + amd_pmc_get_smu_version(dev);
> platform_set_drvdata(pdev, dev);
> amd_pmc_dbgfs_register(dev);
> return 0;
>

2021-09-23 21:53:05

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: amd-pmc: Export Idlemask values based on the APU

On Thu, Sep 16, 2021 at 06:10:02PM +0530, Sanket Goswami wrote:
> IdleMask is the metric used by the PM firmware to know the status of each
> of the Hardware IP blocks monitored by the PM firmware.
>
> Knowing this value is key to get the information of s2idle suspend/resume
> status. This value is mapped to PMC scratch registers, retrieve them
> accordingly based on the CPU family and the underlying firmware support.
>
> Co-developed-by: Shyam Sundar S K <[email protected]>
> Signed-off-by: Shyam Sundar S K <[email protected]>
> Signed-off-by: Sanket Goswami <[email protected]>

This patch as commit f6045de1f532 ("platform/x86: amd-pmc: Export
Idlemask values based on the APU") in -next causes the following errors
when CONFIG_DEBUG_FS is disabled:

drivers/platform/x86/amd-pmc.c:424:2: error: implicit declaration of function 'amd_pmc_idlemask_read' [-Werror,-Wimplicit-function-declaration]
amd_pmc_idlemask_read(pdev, dev, NULL);
^
drivers/platform/x86/amd-pmc.c:424:2: note: did you mean 'amd_pmc_reg_read'?
drivers/platform/x86/amd-pmc.c:131:19: note: 'amd_pmc_reg_read' declared here
static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset)
^
drivers/platform/x86/amd-pmc.c:448:2: error: implicit declaration of function 'amd_pmc_idlemask_read' [-Werror,-Wimplicit-function-declaration]
amd_pmc_idlemask_read(pdev, dev, NULL);
^
drivers/platform/x86/amd-pmc.c:536:2: error: implicit declaration of function 'amd_pmc_get_smu_version' [-Werror,-Wimplicit-function-declaration]
amd_pmc_get_smu_version(dev);
^
3 errors generated.

Should these functions be stubbed or should there be a different fix?

Cheers,
Nathan

2021-09-28 14:19:59

by Hans de Goede

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: amd-pmc: Export Idlemask values based on the APU

Hi,

On 9/23/21 11:50 PM, Nathan Chancellor wrote:
> On Thu, Sep 16, 2021 at 06:10:02PM +0530, Sanket Goswami wrote:
>> IdleMask is the metric used by the PM firmware to know the status of each
>> of the Hardware IP blocks monitored by the PM firmware.
>>
>> Knowing this value is key to get the information of s2idle suspend/resume
>> status. This value is mapped to PMC scratch registers, retrieve them
>> accordingly based on the CPU family and the underlying firmware support.
>>
>> Co-developed-by: Shyam Sundar S K <[email protected]>
>> Signed-off-by: Shyam Sundar S K <[email protected]>
>> Signed-off-by: Sanket Goswami <[email protected]>
>
> This patch as commit f6045de1f532 ("platform/x86: amd-pmc: Export
> Idlemask values based on the APU") in -next causes the following errors
> when CONFIG_DEBUG_FS is disabled:
>
> drivers/platform/x86/amd-pmc.c:424:2: error: implicit declaration of function 'amd_pmc_idlemask_read' [-Werror,-Wimplicit-function-declaration]
> amd_pmc_idlemask_read(pdev, dev, NULL);
> ^
> drivers/platform/x86/amd-pmc.c:424:2: note: did you mean 'amd_pmc_reg_read'?
> drivers/platform/x86/amd-pmc.c:131:19: note: 'amd_pmc_reg_read' declared here
> static inline u32 amd_pmc_reg_read(struct amd_pmc_dev *dev, int reg_offset)
> ^
> drivers/platform/x86/amd-pmc.c:448:2: error: implicit declaration of function 'amd_pmc_idlemask_read' [-Werror,-Wimplicit-function-declaration]
> amd_pmc_idlemask_read(pdev, dev, NULL);
> ^
> drivers/platform/x86/amd-pmc.c:536:2: error: implicit declaration of function 'amd_pmc_get_smu_version' [-Werror,-Wimplicit-function-declaration]
> amd_pmc_get_smu_version(dev);
> ^
> 3 errors generated.
>
> Should these functions be stubbed or should there be a different fix?

Thank you for the bug report.

Since these functions are also used outside of the debugfs show functions
they simply need to be moved outside of the #ifdef CONFIG_DEBUG_FS
block. I'll add a patch fixing this to pdx86/review-hans and include
it in my next pdx86-fixes pull-req to Linus.

Regards,

Hans