[PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC driver

[PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC driver

[Shyam Sundar S K]

This patch series includes:

1. Fixes to Picasso from Mario
2. Change the SMN pair index for driver probing & STB init
3. New command ID for getting DRAM size from PMFW.
4. Change in smu metrics table data structure.

Mario Limonciello (4):
  platform/x86/amd: pmc: Don't try to read SMU version on Picasso
  platform/x86/amd: pmc: Hide SMU version and program attributes for
    Picasso
  platform/x86/amd: pmc: Don't dump data after resume from s0i3 on
    picasso
  platform/x86/amd: pmc: Move idlemask check into
    `amd_pmc_idlemask_read`

Shyam Sundar S K (4):
  platform/x86/amd: pmc: Utilize SMN index 0 for driver probe
  platform/x86/amd: pmc: Move out of BIOS SMN pair for STB init
  platform/x86/amd: pmc: Get STB DRAM size from PMFW
  platform/x86/amd: pmc: update metrics table info for Pink Sardine

 drivers/platform/x86/amd/Kconfig |   2 +-
 drivers/platform/x86/amd/pmc.c   | 240 ++++++++++++++++++-------------
 2 files changed, 145 insertions(+), 97 deletions(-)

-- 
2.25.1

[PATCH 1/8] platform/x86/amd: pmc: Don't try to read SMU version on Picasso

[Shyam Sundar S K]

From: Mario Limonciello <mario.limonciello@amd.com>

Picasso doesn't support the command in the uPEP mailbox to try to
read the SMU version.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2449
Fixes: f6045de1f532 ("platform/x86: amd-pmc: Export Idlemask values based on the APU")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index ab05b9ee6655..627677c3a335 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -407,6 +407,9 @@ static int amd_pmc_get_smu_version(struct amd_pmc_dev *dev)
 	int rc;
 	u32 val;
 
+	if (dev->cpu_id == AMD_CPU_ID_PCO)
+		return -ENODEV;
+
 	rc = amd_pmc_send_cmd(dev, 0, &val, SMU_MSG_GETSMUVERSION, 1);
 	if (rc)
 		return rc;
-- 
2.25.1

[PATCH 2/8] platform/x86/amd: pmc: Hide SMU version and program attributes for Picasso

[Shyam Sundar S K]

From: Mario Limonciello <mario.limonciello@amd.com>

As the command to get version isn't supported on Picasso, we shouldn't
be exposing this into sysfs either.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2449
Fixes: 7f1ea75d499a ("platform/x86/amd: pmc: Add sysfs files for SMU")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 627677c3a335..9f824ecd84c2 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -456,12 +456,31 @@ static ssize_t smu_program_show(struct device *d, struct device_attribute *attr,
 static DEVICE_ATTR_RO(smu_fw_version);
 static DEVICE_ATTR_RO(smu_program);
 
+static umode_t pmc_attr_is_visible(struct kobject *kobj, struct attribute *attr, int idx)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
+
+	if (pdev->cpu_id == AMD_CPU_ID_PCO)
+		return 0;
+	return 0444;
+}
+
 static struct attribute *pmc_attrs[] = {
 	&dev_attr_smu_fw_version.attr,
 	&dev_attr_smu_program.attr,
 	NULL,
 };
-ATTRIBUTE_GROUPS(pmc);
+
+static struct attribute_group pmc_attr_group = {
+	.attrs = pmc_attrs,
+	.is_visible = pmc_attr_is_visible,
+};
+
+static const struct attribute_group *pmc_groups[] = {
+	&pmc_attr_group,
+	NULL,
+};
 
 static int smu_fw_info_show(struct seq_file *s, void *unused)
 {
-- 
2.25.1

[PATCH 3/8] platform/x86/amd: pmc: Don't dump data after resume from s0i3 on picasso

[Shyam Sundar S K]

From: Mario Limonciello <mario.limonciello@amd.com>

This command isn't supported on Picasso, so guard against running it
to avoid errors like `SMU cmd unknown. err: 0xfe` in the logs.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2449
Fixes: 766205674962 ("platform/x86: amd-pmc: Add support for logging SMU metrics")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 9f824ecd84c2..f3a099550ff1 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -839,6 +839,14 @@ static void amd_pmc_s2idle_check(void)
 		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
 }
 
+static int amd_pmc_dump_data(struct amd_pmc_dev *pdev)
+{
+	if (pdev->cpu_id == AMD_CPU_ID_PCO)
+		return -ENODEV;
+
+	return amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_DUMP_DATA, 0);
+}
+
 static void amd_pmc_s2idle_restore(void)
 {
 	struct amd_pmc_dev *pdev = &pmc;
@@ -851,7 +859,7 @@ static void amd_pmc_s2idle_restore(void)
 		dev_err(pdev->dev, "resume failed: %d\n", rc);
 
 	/* Let SMU know that we are looking for stats */
-	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_DUMP_DATA, 0);
+	amd_pmc_dump_data(pdev);
 
 	rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_S2IDLE_RESTORE);
 	if (rc)
-- 
2.25.1

[PATCH 4/8] platform/x86/amd: pmc: Move idlemask check into `amd_pmc_idlemask_read`

[Shyam Sundar S K]

From: Mario Limonciello <mario.limonciello@amd.com>

The version check requirement for idle mask support actually only
applies to RN/CZN/BRC platforms.

So far no issues have happened because the PMFW version string is
bigger on other supported systems.  This can be reset for any new platform
so move the check to only RN/CZN/BRC case.

Fixes: f6045de1f532 ("platform/x86: amd-pmc: Export Idlemask values based on the APU")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 74 +++++++++++++++-------------------
 1 file changed, 33 insertions(+), 41 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index f3a099550ff1..71dc8c80a79a 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -344,33 +344,6 @@ static int amd_pmc_setup_smu_logging(struct amd_pmc_dev *dev)
 	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:
-	case AMD_CPU_ID_CB:
-	case AMD_CPU_ID_PS:
-		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 get_metrics_table(struct amd_pmc_dev *pdev, struct smu_metrics *table)
 {
 	if (!pdev->smu_virt_addr) {
@@ -547,28 +520,47 @@ static int s0ix_stats_show(struct seq_file *s, void *unused)
 }
 DEFINE_SHOW_ATTRIBUTE(s0ix_stats);
 
-static int amd_pmc_idlemask_show(struct seq_file *s, void *unused)
+static int amd_pmc_idlemask_read(struct amd_pmc_dev *pdev, struct device *dev,
+				 struct seq_file *s)
 {
-	struct amd_pmc_dev *dev = s->private;
+	u32 val;
 	int rc;
 
-	/* we haven't yet read SMU version */
-	if (!dev->major) {
-		rc = amd_pmc_get_smu_version(dev);
-		if (rc)
-			return 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
+			return -EINVAL;
+		break;
+	case AMD_CPU_ID_YC:
+	case AMD_CPU_ID_CB:
+	case AMD_CPU_ID_PS:
+		val = amd_pmc_reg_read(pdev, AMD_PMC_SCRATCH_REG_YC);
+		break;
+	default:
+		return -EINVAL;
 	}
 
-	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");
-	}
+	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)
+{
+	return amd_pmc_idlemask_read(s->private, NULL, s);
+}
 DEFINE_SHOW_ATTRIBUTE(amd_pmc_idlemask);
 
 static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev)
-- 
2.25.1

[PATCH 5/8] platform/x86/amd: pmc: Utilize SMN index 0 for driver probe

[Shyam Sundar S K]

The current SMN index used for the driver probe seems to be meant
for the BIOS pair and there are potential concurrency problems that can
occur with an inopportune SMI.

It is been advised to use SMN_INDEX_0 instead of SMN_INDEX_2, which is
what amd_nb.c provides and this function has protections to ensure that
only one caller can use it at a time.

Fixes: 156ec4731cb2 ("platform/x86: amd-pmc: Add AMD platform support for S2Idle")
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/Kconfig |  2 +-
 drivers/platform/x86/amd/pmc.c   | 23 +++++------------------
 2 files changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/platform/x86/amd/Kconfig b/drivers/platform/x86/amd/Kconfig
index 2ce8cb2170df..d9685aef0887 100644
--- a/drivers/platform/x86/amd/Kconfig
+++ b/drivers/platform/x86/amd/Kconfig
@@ -7,7 +7,7 @@ source "drivers/platform/x86/amd/pmf/Kconfig"
 
 config AMD_PMC
 	tristate "AMD SoC PMC driver"
-	depends on ACPI && PCI && RTC_CLASS
+	depends on ACPI && PCI && RTC_CLASS && AMD_NB
 	select SERIO
 	help
 	  The driver provides support for AMD Power Management Controller
diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 71dc8c80a79a..b7f736e7cd0e 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -10,6 +10,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <asm/amd_nb.h>
 #include <linux/acpi.h>
 #include <linux/bitfield.h>
 #include <linux/bits.h>
@@ -56,8 +57,6 @@
 #define S2D_TELEMETRY_DRAMBYTES_MAX	0x1000000
 
 /* 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
 #define AMD_PMC_MAPPING_SIZE		0x01000
 #define AMD_PMC_BASE_ADDR_OFFSET	0x10000
 #define AMD_PMC_BASE_ADDR_LO		0x13B102E8
@@ -992,30 +991,18 @@ static int amd_pmc_probe(struct platform_device *pdev)
 
 	dev->cpu_id = rdev->device;
 	dev->rdev = rdev;
-	err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS, AMD_PMC_BASE_ADDR_LO);
-	if (err) {
-		dev_err(dev->dev, "error writing to 0x%x\n", AMD_PMC_SMU_INDEX_ADDRESS);
-		err = pcibios_err_to_errno(err);
-		goto err_pci_dev_put;
-	}
-
-	err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, &val);
+	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_LO, &val);
 	if (err) {
+		dev_err(dev->dev, "error reading 0x%x\n", AMD_PMC_BASE_ADDR_LO);
 		err = pcibios_err_to_errno(err);
 		goto err_pci_dev_put;
 	}
 
 	base_addr_lo = val & AMD_PMC_BASE_ADDR_HI_MASK;
 
-	err = pci_write_config_dword(rdev, AMD_PMC_SMU_INDEX_ADDRESS, AMD_PMC_BASE_ADDR_HI);
-	if (err) {
-		dev_err(dev->dev, "error writing to 0x%x\n", AMD_PMC_SMU_INDEX_ADDRESS);
-		err = pcibios_err_to_errno(err);
-		goto err_pci_dev_put;
-	}
-
-	err = pci_read_config_dword(rdev, AMD_PMC_SMU_INDEX_DATA, &val);
+	err = amd_smn_read(0, AMD_PMC_BASE_ADDR_HI, &val);
 	if (err) {
+		dev_err(dev->dev, "error reading 0x%x\n", AMD_PMC_BASE_ADDR_HI);
 		err = pcibios_err_to_errno(err);
 		goto err_pci_dev_put;
 	}
-- 
2.25.1

[PATCH 6/8] platform/x86/amd: pmc: Move out of BIOS SMN pair for STB init

[Shyam Sundar S K]

The current SMN index used for the driver probe seems to be meant
for the BIOS pair and there are potential concurrency problems that can
occur with an inopportune SMI.

It is been advised to use SMN_INDEX_0 instead of SMN_INDEX_6, which is
what amd_nb.c provides and this function has protections to ensure that
only one caller can use it at a time.

Fixes: 426c0ff27b83 ("platform/x86: amd-pmc: Add support for AMD Smart Trace Buffer")
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.c | 26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index b7f736e7cd0e..b14d068a6474 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -38,8 +38,6 @@
 #define AMD_PMC_SCRATCH_REG_YC		0xD14
 
 /* STB Registers */
-#define AMD_PMC_STB_INDEX_ADDRESS	0xF8
-#define AMD_PMC_STB_INDEX_DATA		0xFC
 #define AMD_PMC_STB_PMI_0		0x03E30600
 #define AMD_PMC_STB_S2IDLE_PREPARE	0xC6000001
 #define AMD_PMC_STB_S2IDLE_RESTORE	0xC6000002
@@ -931,17 +929,9 @@ static int amd_pmc_write_stb(struct amd_pmc_dev *dev, u32 data)
 {
 	int err;
 
-	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_ADDRESS, AMD_PMC_STB_PMI_0);
+	err = amd_smn_write(0, AMD_PMC_STB_PMI_0, data);
 	if (err) {
-		dev_err(dev->dev, "failed to write addr in stb: 0x%X\n",
-			AMD_PMC_STB_INDEX_ADDRESS);
-		return pcibios_err_to_errno(err);
-	}
-
-	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_DATA, data);
-	if (err) {
-		dev_err(dev->dev, "failed to write data in stb: 0x%X\n",
-			AMD_PMC_STB_INDEX_DATA);
+		dev_err(dev->dev, "failed to write data in stb: 0x%X\n", AMD_PMC_STB_PMI_0);
 		return pcibios_err_to_errno(err);
 	}
 
@@ -953,18 +943,10 @@ static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf)
 {
 	int i, err;
 
-	err = pci_write_config_dword(dev->rdev, AMD_PMC_STB_INDEX_ADDRESS, AMD_PMC_STB_PMI_0);
-	if (err) {
-		dev_err(dev->dev, "error writing addr to stb: 0x%X\n",
-			AMD_PMC_STB_INDEX_ADDRESS);
-		return pcibios_err_to_errno(err);
-	}
-
 	for (i = 0; i < FIFO_SIZE; i++) {
-		err = pci_read_config_dword(dev->rdev, AMD_PMC_STB_INDEX_DATA, buf++);
+		err = amd_smn_read(0, AMD_PMC_STB_PMI_0, buf++);
 		if (err) {
-			dev_err(dev->dev, "error reading data from stb: 0x%X\n",
-				AMD_PMC_STB_INDEX_DATA);
+			dev_err(dev->dev, "error reading data from stb: 0x%X\n", AMD_PMC_STB_PMI_0);
 			return pcibios_err_to_errno(err);
 		}
 	}
-- 
2.25.1

[PATCH 7/8] platform/x86/amd: pmc: Get STB DRAM size from PMFW

[Shyam Sundar S K]

Recent PMFW's have support for querying the STB DRAM size. Add this
support to the driver.

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.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;
+
+	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;
 
-- 
2.25.1

[PATCH 8/8] platform/x86/amd: pmc: update metrics table info for Pink Sardine

[Shyam Sundar S K]

Starting from Pink Sardine, number of IP blocks were added to the
SoC and the PMFW has the ability to give debug stats on each the IP blocks
after a S0ix cycle within part of the SMU metrics table. Add this new
capability to the driver.

Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 56 ++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 86f32b01e3ff..b27e93cfbe61 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -45,7 +45,6 @@
 #define AMD_PMC_STB_DUMMY_PC		0xC6000007
 
 /* STB S2D(Spill to DRAM) has different message port offset */
-#define STB_SPILL_TO_DRAM		0xBE
 #define AMD_S2D_REGISTER_MESSAGE	0xA20
 #define AMD_S2D_REGISTER_RESPONSE	0xA80
 #define AMD_S2D_REGISTER_ARGUMENT	0xA88
@@ -98,7 +97,6 @@
 #define PMC_MSG_DELAY_MIN_US		50
 #define RESPONSE_REGISTER_LOOP_MAX	20000
 
-#define SOC_SUBSYSTEM_IP_MAX	12
 #define DELAY_MIN_US		2000
 #define DELAY_MAX_US		3000
 #define FIFO_SIZE		4096
@@ -132,9 +130,18 @@ static const struct amd_pmc_bit_map soc15_ip_blk[] = {
 	{"ISP",		BIT(6)},
 	{"NBIO",	BIT(7)},
 	{"DF",		BIT(8)},
-	{"USB0",	BIT(9)},
-	{"USB1",	BIT(10)},
+	{"USB3_0",	BIT(9)},
+	{"USB3_1",	BIT(10)},
 	{"LAPIC",	BIT(11)},
+	{"USB3_2",	BIT(12)},
+	{"USB3_3",	BIT(13)},
+	{"USB3_4",	BIT(14)},
+	{"USB4_0",	BIT(15)},
+	{"USB4_1",	BIT(16)},
+	{"MPM",		BIT(17)},
+	{"JPEG",	BIT(18)},
+	{"IPU",		BIT(19)},
+	{"UMSCH",	BIT(20)},
 	{}
 };
 
@@ -148,6 +155,8 @@ struct amd_pmc_dev {
 	u32 cpu_id;
 	u32 active_ips;
 	u32 dram_size;
+	u32 num_ips;
+	u32 s2d_msg_id;
 /* SMU version information */
 	u8 smu_program;
 	u8 major;
@@ -197,8 +206,8 @@ struct smu_metrics {
 	u64 timein_s0i3_totaltime;
 	u64 timein_swdrips_lastcapture;
 	u64 timein_swdrips_totaltime;
-	u64 timecondition_notmet_lastcapture[SOC_SUBSYSTEM_IP_MAX];
-	u64 timecondition_notmet_totaltime[SOC_SUBSYSTEM_IP_MAX];
+	u64 timecondition_notmet_lastcapture[32];
+	u64 timecondition_notmet_totaltime[32];
 } __packed;
 
 static int amd_pmc_stb_debugfs_open(struct inode *inode, struct file *filp)
@@ -264,7 +273,7 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
 	dev->msg_port = 1;
 
 	/* Get the num_samples to calculate the last push location */
-	ret = amd_pmc_send_cmd(dev, S2D_NUM_SAMPLES, &num_samples, STB_SPILL_TO_DRAM, 1);
+	ret = amd_pmc_send_cmd(dev, S2D_NUM_SAMPLES, &num_samples, dev->s2d_msg_id, 1);
 	/* Clear msg_port for other SMU operation */
 	dev->msg_port = 0;
 	if (ret) {
@@ -310,6 +319,23 @@ static const struct file_operations amd_pmc_stb_debugfs_fops_v2 = {
 	.release = amd_pmc_stb_debugfs_release_v2,
 };
 
+static void amd_pmc_get_ip_info(struct amd_pmc_dev *dev)
+{
+	switch (dev->cpu_id) {
+	case AMD_CPU_ID_PCO:
+	case AMD_CPU_ID_RN:
+	case AMD_CPU_ID_YC:
+	case AMD_CPU_ID_CB:
+		dev->num_ips = 12;
+		dev->s2d_msg_id = 0xBE;
+		break;
+	case AMD_CPU_ID_PS:
+		dev->num_ips = 21;
+		dev->s2d_msg_id = 0x85;
+		break;
+	}
+}
+
 static int amd_pmc_setup_smu_logging(struct amd_pmc_dev *dev)
 {
 	if (dev->cpu_id == AMD_CPU_ID_PCO) {
@@ -474,7 +500,7 @@ static int smu_fw_info_show(struct seq_file *s, void *unused)
 		   table.timeto_resume_to_os_lastcapture);
 
 	seq_puts(s, "\n=== Active time (in us) ===\n");
-	for (idx = 0 ; idx < SOC_SUBSYSTEM_IP_MAX ; idx++) {
+	for (idx = 0 ; idx < dev->num_ips ; idx++) {
 		if (soc15_ip_blk[idx].bit_mask & dev->active_ips)
 			seq_printf(s, "%-8s : %lld\n", soc15_ip_blk[idx].name,
 				   table.timecondition_notmet_lastcapture[idx]);
@@ -905,7 +931,7 @@ static int amd_pmc_get_dram_size(struct amd_pmc_dev *dev)
 	if (dev->major > 90 || (dev->major == 90 && dev->minor > 39))
 		goto err_dram_size;
 
-	amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, STB_SPILL_TO_DRAM, 1);
+	amd_pmc_send_cmd(dev, S2D_DRAM_SIZE, &dev->dram_size, dev->s2d_msg_id, 1);
 	if (!dev->dram_size)
 		goto err_dram_size;
 
@@ -926,7 +952,10 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
 	/* Spill to DRAM feature uses separate SMU message port */
 	dev->msg_port = 1;
 
-	amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, STB_SPILL_TO_DRAM, 1);
+	/* Get num of IP blocks within the SoC */
+	amd_pmc_get_ip_info(dev);
+
+	amd_pmc_send_cmd(dev, S2D_TELEMETRY_SIZE, &size, dev->s2d_msg_id, 1);
 	if (size != S2D_TELEMETRY_BYTES_MAX)
 		return -EIO;
 
@@ -936,8 +965,8 @@ static int amd_pmc_s2d_init(struct amd_pmc_dev *dev)
 		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);
+	amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_LOW, &phys_addr_low, dev->s2d_msg_id, 1);
+	amd_pmc_send_cmd(dev, S2D_PHYS_ADDR_HIGH, &phys_addr_hi, dev->s2d_msg_id, 1);
 
 	stb_phys_addr = ((u64)phys_addr_hi << 32 | phys_addr_low);
 
@@ -1028,7 +1057,8 @@ static int amd_pmc_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->lock);
 
-	if (enable_stb && (dev->cpu_id == AMD_CPU_ID_YC || dev->cpu_id == AMD_CPU_ID_CB)) {
+	if (enable_stb && (dev->cpu_id == AMD_CPU_ID_YC || dev->cpu_id == AMD_CPU_ID_CB ||
+			   dev->cpu_id == AMD_CPU_ID_PS)) {
 		err = amd_pmc_s2d_init(dev);
 		if (err)
 			goto err_pci_dev_put;
-- 
2.25.1

Re: [PATCH 7/8] platform/x86/amd: pmc: Get STB DRAM size from PMFW

[Mario Limonciello]

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@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.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?

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;
>   

Re: [PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC driver

[Hans de Goede]

Hi Shyam, Mario,

On 4/9/23 20:53, Shyam Sundar S K wrote:
> This patch series includes:
> 
> 1. Fixes to Picasso from Mario
> 2. Change the SMN pair index for driver probing & STB init
> 3. New command ID for getting DRAM size from PMFW.
> 4. Change in smu metrics table data structure.

So patches 1-6 all seem to be bugfixes, but they don't seem
to be very urgent.  At least AFAICT they mostly just fix
some (harmless) warnings getting logged + future proof some
of the code for newer firmware revisions.

Given that rc6 has already been released it would be my
preference to just merge these into linux-next so that they
get merged into Linus tree for 6.4-rc1.

Either way (urgent should go to fixes, or -next is fine)
please let me know how to proceed with merging these.

Regards,

Hans






> 
> Mario Limonciello (4):
>   platform/x86/amd: pmc: Don't try to read SMU version on Picasso
>   platform/x86/amd: pmc: Hide SMU version and program attributes for
>     Picasso
>   platform/x86/amd: pmc: Don't dump data after resume from s0i3 on
>     picasso
>   platform/x86/amd: pmc: Move idlemask check into
>     `amd_pmc_idlemask_read`
> 
> Shyam Sundar S K (4):
>   platform/x86/amd: pmc: Utilize SMN index 0 for driver probe
>   platform/x86/amd: pmc: Move out of BIOS SMN pair for STB init
>   platform/x86/amd: pmc: Get STB DRAM size from PMFW
>   platform/x86/amd: pmc: update metrics table info for Pink Sardine
> 
>  drivers/platform/x86/amd/Kconfig |   2 +-
>  drivers/platform/x86/amd/pmc.c   | 240 ++++++++++++++++++-------------
>  2 files changed, 145 insertions(+), 97 deletions(-)
> 

RE: [PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC driver

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, April 10, 2023 05:37
> To: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>;
> markgross@kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
> Cc: Goswami, Sanket <Sanket.Goswami@amd.com>; platform-driver-
> x86@vger.kernel.org
> Subject: Re: [PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC
> driver
> 
> Hi Shyam, Mario,
> 
> On 4/9/23 20:53, Shyam Sundar S K wrote:
> > This patch series includes:
> >
> > 1. Fixes to Picasso from Mario
> > 2. Change the SMN pair index for driver probing & STB init
> > 3. New command ID for getting DRAM size from PMFW.
> > 4. Change in smu metrics table data structure.
> 
> So patches 1-6 all seem to be bugfixes, but they don't seem
> to be very urgent.  At least AFAICT they mostly just fix
> some (harmless) warnings getting logged + future proof some
> of the code for newer firmware revisions.
> 
> Given that rc6 has already been released it would be my
> preference to just merge these into linux-next so that they
> get merged into Linus tree for 6.4-rc1.
> 
> Either way (urgent should go to fixes, or -next is fine)
> please let me know how to proceed with merging these.
> 

IMO they can wait for 6.4.  There is a matching change needed
for NVME for the Picasso stuff which isn't landing until 6.4 also.

> Regards,
> 
> Hans
> 
> 
> 
> 
> 
> 
> >
> > Mario Limonciello (4):
> >   platform/x86/amd: pmc: Don't try to read SMU version on Picasso
> >   platform/x86/amd: pmc: Hide SMU version and program attributes for
> >     Picasso
> >   platform/x86/amd: pmc: Don't dump data after resume from s0i3 on
> >     picasso
> >   platform/x86/amd: pmc: Move idlemask check into
> >     `amd_pmc_idlemask_read`
> >
> > Shyam Sundar S K (4):
> >   platform/x86/amd: pmc: Utilize SMN index 0 for driver probe
> >   platform/x86/amd: pmc: Move out of BIOS SMN pair for STB init
> >   platform/x86/amd: pmc: Get STB DRAM size from PMFW
> >   platform/x86/amd: pmc: update metrics table info for Pink Sardine
> >
> >  drivers/platform/x86/amd/Kconfig |   2 +-
> >  drivers/platform/x86/amd/pmc.c   | 240 ++++++++++++++++++-------------
> >  2 files changed, 145 insertions(+), 97 deletions(-)
> >

Re: [PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC driver

[Shyam Sundar S K]

Hi Hans,

On 4/10/2023 10:19 PM, Limonciello, Mario wrote:
> [Public]
> 
> 
> 
>> -----Original Message-----
>> From: Hans de Goede <hdegoede@redhat.com>
>> Sent: Monday, April 10, 2023 05:37
>> To: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>;
>> markgross@kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>
>> Cc: Goswami, Sanket <Sanket.Goswami@amd.com>; platform-driver-
>> x86@vger.kernel.org
>> Subject: Re: [PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC
>> driver
>>
>> Hi Shyam, Mario,
>>
>> On 4/9/23 20:53, Shyam Sundar S K wrote:
>>> This patch series includes:
>>>
>>> 1. Fixes to Picasso from Mario
>>> 2. Change the SMN pair index for driver probing & STB init
>>> 3. New command ID for getting DRAM size from PMFW.
>>> 4. Change in smu metrics table data structure.
>>
>> So patches 1-6 all seem to be bugfixes, but they don't seem
>> to be very urgent.  At least AFAICT they mostly just fix
>> some (harmless) warnings getting logged + future proof some
>> of the code for newer firmware revisions.
>>
>> Given that rc6 has already been released it would be my
>> preference to just merge these into linux-next so that they
>> get merged into Linus tree for 6.4-rc1.
>>
>> Either way (urgent should go to fixes, or -next is fine)
>> please let me know how to proceed with merging these.
>>
> 
> IMO they can wait for 6.4.  There is a matching change needed
> for NVME for the Picasso stuff which isn't landing until 6.4 also.

Agree with Mario.

Thanks,
Shyam

> 
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>>
>>
>>>
>>> Mario Limonciello (4):
>>>   platform/x86/amd: pmc: Don't try to read SMU version on Picasso
>>>   platform/x86/amd: pmc: Hide SMU version and program attributes for
>>>     Picasso
>>>   platform/x86/amd: pmc: Don't dump data after resume from s0i3 on
>>>     picasso
>>>   platform/x86/amd: pmc: Move idlemask check into
>>>     `amd_pmc_idlemask_read`
>>>
>>> Shyam Sundar S K (4):
>>>   platform/x86/amd: pmc: Utilize SMN index 0 for driver probe
>>>   platform/x86/amd: pmc: Move out of BIOS SMN pair for STB init
>>>   platform/x86/amd: pmc: Get STB DRAM size from PMFW
>>>   platform/x86/amd: pmc: update metrics table info for Pink Sardine
>>>
>>>  drivers/platform/x86/amd/Kconfig |   2 +-
>>>  drivers/platform/x86/amd/pmc.c   | 240 ++++++++++++++++++-------------
>>>  2 files changed, 145 insertions(+), 97 deletions(-)
>>>

Re: [PATCH 7/8] platform/x86/amd: pmc: Get STB DRAM size from PMFW

[Shyam Sundar S K]

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@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.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?

Thanks,
Shyam

> 
> 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;
>>   

RE: [PATCH 7/8] platform/x86/amd: pmc: Get STB DRAM size from PMFW

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> Sent: Monday, April 10, 2023 12:35
> To: Limonciello, Mario <Mario.Limonciello@amd.com>;
> hdegoede@redhat.com; markgross@kernel.org
> Cc: Goswami, Sanket <Sanket.Goswami@amd.com>; platform-driver-
> x86@vger.kernel.org
> 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@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.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;
> >>

Re: [PATCH 0/8] platform/x86/amd/pmc: Updates to AMD PMC driver

[Hans de Goede]

Hi Shyam,

On 4/9/23 20:53, Shyam Sundar S K wrote:
> This patch series includes:
> 
> 1. Fixes to Picasso from Mario
> 2. Change the SMN pair index for driver probing & STB init
> 3. New command ID for getting DRAM size from PMFW.
> 4. Change in smu metrics table data structure.
> 
> Mario Limonciello (4):
>   platform/x86/amd: pmc: Don't try to read SMU version on Picasso
>   platform/x86/amd: pmc: Hide SMU version and program attributes for
>     Picasso
>   platform/x86/amd: pmc: Don't dump data after resume from s0i3 on
>     picasso
>   platform/x86/amd: pmc: Move idlemask check into
>     `amd_pmc_idlemask_read`
> 
> Shyam Sundar S K (4):
>   platform/x86/amd: pmc: Utilize SMN index 0 for driver probe
>   platform/x86/amd: pmc: Move out of BIOS SMN pair for STB init
>   platform/x86/amd: pmc: Get STB DRAM size from PMFW
>   platform/x86/amd: pmc: update metrics table info for Pink Sardine
> 
>  drivers/platform/x86/amd/Kconfig |   2 +-
>  drivers/platform/x86/amd/pmc.c   | 240 ++++++++++++++++++-------------
>  2 files changed, 145 insertions(+), 97 deletions(-)

Thanks for the patches I have merged patches 1-6 into my
review-hans (soon to be for-next) branch now.

I agree with Mario that the firmware version check in 7/8
should look like this:

"""
Is this only for YC and not for PS?

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;

} 
"""

Since we have seen in earlier patches in this series that we will otherwise need to make this a per CPU-id check later we might just as well do it now.

Please send a new version of patches 7 + 8 based on top of my review-hans branch.

Regards,

Hans