[PATCH v2 0/4] Updates to amd_pmc driver

[PATCH v2 0/4] Updates to amd_pmc driver

[Shyam Sundar S K]

This patch series includes:
- Add num_samples message id support to STB
- Write a dummy postcode in DRAM to get latest data
- Add proper debug print support for STB
- Add linebreak for code readability

v1->v2:
- Fix incorrect pointer handling while memcpy()

Shyam Sundar S K (4):
  platform/x86/amd: pmc: Add num_samples message id support to STB
  platform/x86/amd: pmc: Write dummy postcode into the STB DRAM
  platform/x86/amd: pmc: differentiate STB/SMU messaging prints
  platform/x86/amd: pmc: Add line break for readability

 drivers/platform/x86/amd/pmc.c | 41 +++++++++++++++++++++++++++++-----
 1 file changed, 36 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/4] platform/x86/amd: pmc: Add num_samples message id support to STB

[Shyam Sundar S K]

Recent PMFWs have the support for S2D_NUM_SAMPLES message ID that
can tell the current number of samples present within the STB DRAM.

num_samples returned would let the driver know the start of the read
from the last push location. This way, the driver would emit the
top most region of the STB DRAM.

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 | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 3cbb01ec10e3..b0f98a201a81 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -114,6 +114,7 @@ enum s2d_arg {
 	S2D_TELEMETRY_SIZE = 0x01,
 	S2D_PHYS_ADDR_LOW,
 	S2D_PHYS_ADDR_HIGH,
+	S2D_NUM_SAMPLES,
 };
 
 struct amd_pmc_bit_map {
@@ -246,13 +247,36 @@ static const struct file_operations amd_pmc_stb_debugfs_fops = {
 static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
 {
 	struct amd_pmc_dev *dev = filp->f_inode->i_private;
-	u32 *buf;
+	u32 *buf, fsize, num_samples, stb_rdptr_offset = 0;
+	int ret;
 
 	buf = kzalloc(S2D_TELEMETRY_BYTES_MAX, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
-	memcpy_fromio(buf, dev->stb_virt_addr, S2D_TELEMETRY_BYTES_MAX);
+	/* Spill to DRAM num_samples uses separate SMU message port */
+	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);
+	if (ret) {
+		dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
+		return ret;
+	}
+
+	/* Clear msg_port for other SMU operation */
+	dev->msg_port = 0;
+
+	/* Start capturing data from the last push location */
+	if (num_samples > S2D_TELEMETRY_BYTES_MAX) {
+		fsize  = S2D_TELEMETRY_BYTES_MAX;
+		stb_rdptr_offset = num_samples - fsize;
+	} else {
+		fsize = num_samples;
+		stb_rdptr_offset = 0;
+	}
+
+	memcpy_fromio(buf, dev->stb_virt_addr + stb_rdptr_offset, fsize);
 	filp->private_data = buf;
 
 	return 0;
-- 
2.25.1

[PATCH v2 2/4] platform/x86/amd: pmc: Write dummy postcode into the STB DRAM

[Shyam Sundar S K]

Based on the recommendation from the PMFW team in order to get the
recent telemetry data present on the STB DRAM the driver is required
to send one dummy write to the STB buffer, so it internally triggers
the PMFW to emit the latest telemetry data in the STB DRAM region.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index b0f98a201a81..4073ad9fe63a 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -43,6 +43,7 @@
 #define AMD_PMC_STB_S2IDLE_PREPARE	0xC6000001
 #define AMD_PMC_STB_S2IDLE_RESTORE	0xC6000002
 #define AMD_PMC_STB_S2IDLE_CHECK	0xC6000003
+#define AMD_PMC_STB_DUMMY_PC		0xC6000007
 
 /* STB S2D(Spill to DRAM) has different message port offset */
 #define STB_SPILL_TO_DRAM		0xBE
@@ -250,6 +251,11 @@ static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
 	u32 *buf, fsize, num_samples, stb_rdptr_offset = 0;
 	int ret;
 
+	/* Write dummy postcode while reading the STB buffer */
+	ret = amd_pmc_write_stb(dev, AMD_PMC_STB_DUMMY_PC);
+	if (ret)
+		dev_err(dev->dev, "error writing to STB: %d\n", ret);
+
 	buf = kzalloc(S2D_TELEMETRY_BYTES_MAX, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
-- 
2.25.1

[PATCH v2 3/4] platform/x86/amd: pmc: differentiate STB/SMU messaging prints

[Shyam Sundar S K]

Modify the dynamic debug print to differentiate between the regular
and spill to DRAM usage of the SMU message port.

Suggested-by: Sanket Goswami <Sanket.Goswami@amd.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 4073ad9fe63a..9c8d04a591ec 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -590,13 +590,13 @@ static void amd_pmc_dump_registers(struct amd_pmc_dev *dev)
 	}
 
 	value = amd_pmc_reg_read(dev, response);
-	dev_dbg(dev->dev, "AMD_PMC_REGISTER_RESPONSE:%x\n", value);
+	dev_dbg(dev->dev, "AMD_%s_REGISTER_RESPONSE:%x\n", dev->msg_port ? "S2D" : "PMC", value);
 
 	value = amd_pmc_reg_read(dev, argument);
-	dev_dbg(dev->dev, "AMD_PMC_REGISTER_ARGUMENT:%x\n", value);
+	dev_dbg(dev->dev, "AMD_%s_REGISTER_ARGUMENT:%x\n", dev->msg_port ? "S2D" : "PMC", value);
 
 	value = amd_pmc_reg_read(dev, message);
-	dev_dbg(dev->dev, "AMD_PMC_REGISTER_MESSAGE:%x\n", value);
+	dev_dbg(dev->dev, "AMD_%s_REGISTER_MESSAGE:%x\n", dev->msg_port ? "S2D" : "PMC", value);
 }
 
 static int amd_pmc_send_cmd(struct amd_pmc_dev *dev, u32 arg, u32 *data, u8 msg, bool ret)
-- 
2.25.1

[PATCH v2 4/4] platform/x86/amd: pmc: Add line break for readability

[Shyam Sundar S K]

Add a line break for the code readability.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 9c8d04a591ec..6a5ddd0f1e8c 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -105,6 +105,7 @@
 #define DELAY_MIN_US		2000
 #define DELAY_MAX_US		3000
 #define FIFO_SIZE		4096
+
 enum amd_pmc_def {
 	MSG_TEST = 0x01,
 	MSG_OS_HINT_PCO,
-- 
2.25.1

Re: [PATCH v2 1/4] platform/x86/amd: pmc: Add num_samples message id support to STB

[Hans de Goede]

Hi,

On 1/30/23 17:48, Shyam Sundar S K wrote:
> Recent PMFWs have the support for S2D_NUM_SAMPLES message ID that
> can tell the current number of samples present within the STB DRAM.
> 
> num_samples returned would let the driver know the start of the read
> from the last push location. This way, the driver would emit the
> top most region of the STB DRAM.
> 
> 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 | 28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> index 3cbb01ec10e3..b0f98a201a81 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -114,6 +114,7 @@ enum s2d_arg {
>  	S2D_TELEMETRY_SIZE = 0x01,
>  	S2D_PHYS_ADDR_LOW,
>  	S2D_PHYS_ADDR_HIGH,
> +	S2D_NUM_SAMPLES,
>  };
>  
>  struct amd_pmc_bit_map {
> @@ -246,13 +247,36 @@ static const struct file_operations amd_pmc_stb_debugfs_fops = {
>  static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
>  {
>  	struct amd_pmc_dev *dev = filp->f_inode->i_private;
> -	u32 *buf;
> +	u32 *buf, fsize, num_samples, stb_rdptr_offset = 0;
> +	int ret;
>  
>  	buf = kzalloc(S2D_TELEMETRY_BYTES_MAX, GFP_KERNEL);
>  	if (!buf)
>  		return -ENOMEM;
>  
> -	memcpy_fromio(buf, dev->stb_virt_addr, S2D_TELEMETRY_BYTES_MAX);
> +	/* Spill to DRAM num_samples uses separate SMU message port */
> +	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);
> +	if (ret) {
> +		dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Clear msg_port for other SMU operation */
> +	dev->msg_port = 0;

You are not clearing dev->msg_port on amd_pmc_send_cmd() errors here
which seems wrong ?  (sorry for not catching this before)  How about:

	/* Get the num_samples to calculate the last push location */
	dev->msg_port = 1;
	ret = amd_pmc_send_cmd(dev, S2D_NUM_SAMPLES, &num_samples, STB_SPILL_TO_DRAM, 1);
	dev->msg_port = 0;
	if (ret) {
		dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
		return ret;
	}

?

Regards,

Hans





> +
> +	/* Start capturing data from the last push location */
> +	if (num_samples > S2D_TELEMETRY_BYTES_MAX) {
> +		fsize  = S2D_TELEMETRY_BYTES_MAX;
> +		stb_rdptr_offset = num_samples - fsize;
> +	} else {
> +		fsize = num_samples;
> +		stb_rdptr_offset = 0;
> +	}
> +
> +	memcpy_fromio(buf, dev->stb_virt_addr + stb_rdptr_offset, fsize);
>  	filp->private_data = buf;
>  
>  	return 0;

Re: [PATCH v2 1/4] platform/x86/amd: pmc: Add num_samples message id support to STB

[Shyam Sundar S K]

On 2/6/2023 5:39 PM, Hans de Goede wrote:
> Hi,
> 
> On 1/30/23 17:48, Shyam Sundar S K wrote:
>> Recent PMFWs have the support for S2D_NUM_SAMPLES message ID that
>> can tell the current number of samples present within the STB DRAM.
>>
>> num_samples returned would let the driver know the start of the read
>> from the last push location. This way, the driver would emit the
>> top most region of the STB DRAM.
>>
>> 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 | 28 ++++++++++++++++++++++++++--
>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
>> index 3cbb01ec10e3..b0f98a201a81 100644
>> --- a/drivers/platform/x86/amd/pmc.c
>> +++ b/drivers/platform/x86/amd/pmc.c
>> @@ -114,6 +114,7 @@ enum s2d_arg {
>>  	S2D_TELEMETRY_SIZE = 0x01,
>>  	S2D_PHYS_ADDR_LOW,
>>  	S2D_PHYS_ADDR_HIGH,
>> +	S2D_NUM_SAMPLES,
>>  };
>>  
>>  struct amd_pmc_bit_map {
>> @@ -246,13 +247,36 @@ static const struct file_operations amd_pmc_stb_debugfs_fops = {
>>  static int amd_pmc_stb_debugfs_open_v2(struct inode *inode, struct file *filp)
>>  {
>>  	struct amd_pmc_dev *dev = filp->f_inode->i_private;
>> -	u32 *buf;
>> +	u32 *buf, fsize, num_samples, stb_rdptr_offset = 0;
>> +	int ret;
>>  
>>  	buf = kzalloc(S2D_TELEMETRY_BYTES_MAX, GFP_KERNEL);
>>  	if (!buf)
>>  		return -ENOMEM;
>>  
>> -	memcpy_fromio(buf, dev->stb_virt_addr, S2D_TELEMETRY_BYTES_MAX);
>> +	/* Spill to DRAM num_samples uses separate SMU message port */
>> +	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);
>> +	if (ret) {
>> +		dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* Clear msg_port for other SMU operation */
>> +	dev->msg_port = 0;
> 
> You are not clearing dev->msg_port on amd_pmc_send_cmd() errors here
> which seems wrong ? 

Agreed, that's a miss.

 (sorry for not catching this before)  How about:
> 
> 	/* Get the num_samples to calculate the last push location */
> 	dev->msg_port = 1;
> 	ret = amd_pmc_send_cmd(dev, S2D_NUM_SAMPLES, &num_samples, STB_SPILL_TO_DRAM, 1);
> 	dev->msg_port = 0;
> 	if (ret) {
> 		dev_err(dev->dev, "error: S2D_NUM_SAMPLES not supported : %d\n", ret);
> 		return ret;
> 	}

Made this change now in v3.

Thanks,
Shyam

> 
> ?
> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> 
>> +
>> +	/* Start capturing data from the last push location */
>> +	if (num_samples > S2D_TELEMETRY_BYTES_MAX) {
>> +		fsize  = S2D_TELEMETRY_BYTES_MAX;
>> +		stb_rdptr_offset = num_samples - fsize;
>> +	} else {
>> +		fsize = num_samples;
>> +		stb_rdptr_offset = 0;
>> +	}
>> +
>> +	memcpy_fromio(buf, dev->stb_virt_addr + stb_rdptr_offset, fsize);
>>  	filp->private_data = buf;
>>  
>>  	return 0;
>