[PATCH 0/4] Updates to amd_pmc driver

[PATCH 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

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 | 43 ++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 5 deletions(-)

-- 
2.25.1

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

diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
index 3cbb01ec10e3..01632e6b7820 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,38 @@ 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;
+	}
+
+	dev->stb_virt_addr += stb_rdptr_offset;
+	memcpy_fromio(buf, dev->stb_virt_addr, fsize);
+
 	filp->private_data = buf;
 
 	return 0;
-- 
2.25.1

[PATCH 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.

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 01632e6b7820..0dd9fb576f09 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 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>
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 0dd9fb576f09..467b80ad01a1 100644
--- a/drivers/platform/x86/amd/pmc.c
+++ b/drivers/platform/x86/amd/pmc.c
@@ -592,13 +592,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 4/4] platform/x86/amd: pmc: Add line break for readability

[Shyam Sundar S K]

Add a line break for the code readability.

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 467b80ad01a1..8afe77e443cb 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 2/4] platform/x86/amd: pmc: Write dummy postcode into the STB DRAM

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Sent: Wednesday, January 25, 2023 05:31
> To: hdegoede@redhat.com; markgross@kernel.org
> Cc: Goswami, Sanket <Sanket.Goswami@amd.com>; platform-driver-
> x86@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> Subject: [PATCH 2/4] platform/x86/amd: pmc: Write dummy postcode into
> the STB DRAM
> 
> 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.
> 
> 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 01632e6b7820..0dd9fb576f09 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

Is there significance to this specific code? Any particular reason
not to pick an incremental code like 0xC6000004?

> 
>  /* 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

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

[Shyam Sundar S K]

On 1/25/2023 11:13 PM, Limonciello, Mario wrote:
> [Public]
> 
> 
> 
>> -----Original Message-----
>> From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> Sent: Wednesday, January 25, 2023 05:31
>> To: hdegoede@redhat.com; markgross@kernel.org
>> Cc: Goswami, Sanket <Sanket.Goswami@amd.com>; platform-driver-
>> x86@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
>> Subject: [PATCH 2/4] platform/x86/amd: pmc: Write dummy postcode into
>> the STB DRAM
>>
>> 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.
>>
>> 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 01632e6b7820..0dd9fb576f09 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
> 
> Is there significance to this specific code? Any particular reason
> not to pick an incremental code like 0xC6000004?

Wanted to just keep in inline with Windows code atleast w.r.t to this
dummy postcode. However, if you think it does not make sense I can
change it to incremental one.

Thanks,
Shyam


> 
>>
>>  /* 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

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

[Mario Limonciello]

On 1/25/23 21:29, Shyam Sundar S K wrote:
> 
> 
> On 1/25/2023 11:13 PM, Limonciello, Mario wrote:
>> [Public]
>>
>>
>>
>>> -----Original Message-----
>>> From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> Sent: Wednesday, January 25, 2023 05:31
>>> To: hdegoede@redhat.com; markgross@kernel.org
>>> Cc: Goswami, Sanket <Sanket.Goswami@amd.com>; platform-driver-
>>> x86@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
>>> Subject: [PATCH 2/4] platform/x86/amd: pmc: Write dummy postcode into
>>> the STB DRAM
>>>
>>> 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.
>>>
>>> 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 01632e6b7820..0dd9fb576f09 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
>>
>> Is there significance to this specific code? Any particular reason
>> not to pick an incremental code like 0xC6000004?
> 
> Wanted to just keep in inline with Windows code atleast w.r.t to this
> dummy postcode. However, if you think it does not make sense I can
> change it to incremental one.
> 

That's a great reason to keep it this way.  People who look at the STB 
can have familiarity on the milestones they see.

If there are other ones that are used for Windows in similar vain across 
suspend/restore as the 3 that we have today we should adjust those too.

> Thanks,
> Shyam
> 
> 
>>
>>>
>>>   /* 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

Re: [PATCH 0/4] Updates to amd_pmc driver

[Mario Limonciello]

On 1/25/23 05:31, Shyam Sundar S K wrote:
> 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
> 
> 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 | 43 ++++++++++++++++++++++++++++++----
>   1 file changed, 38 insertions(+), 5 deletions(-)
> 

For the series:

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

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

[Hans de Goede]

Hi,

On 1/25/23 12:31, 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 | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
> index 3cbb01ec10e3..01632e6b7820 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,38 @@ 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;
> +	}
> +
> +	dev->stb_virt_addr += stb_rdptr_offset;
> +	memcpy_fromio(buf, dev->stb_virt_addr, fsize);

This looks wrong, you are moving the pointer stored in the amd_pmc_dev
struct and subsequent uses of stb_virt_addr will now all use the moved
pointer...

I think that instead of these 2 lines you actually want:

	memcpy_fromio(buf, dev->stb_virt_addr + stb_rdptr_offset, fsize);

Regards,

Hans




> +
>  	filp->private_data = buf;
>  
>  	return 0;

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

[Hans de Goede]

Hi,

On 1/25/23 12:31, Shyam Sundar S K wrote:
> 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.
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  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 01632e6b7820..0dd9fb576f09 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;

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

[Hans de Goede]

Hi,

On 1/25/23 12:31, Shyam Sundar S K wrote:
> 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>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  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 0dd9fb576f09..467b80ad01a1 100644
> --- a/drivers/platform/x86/amd/pmc.c
> +++ b/drivers/platform/x86/amd/pmc.c
> @@ -592,13 +592,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)

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

[Hans de Goede]

Hi,

On 1/25/23 12:31, Shyam Sundar S K wrote:
> Add a line break for the code readability.
> 
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  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 467b80ad01a1..8afe77e443cb 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,

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

[Shyam Sundar S K]

Hi Hans,

On 1/30/2023 8:12 PM, Hans de Goede wrote:
> Hi,
> 
> On 1/25/23 12:31, 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 | 30 ++++++++++++++++++++++++++++--
>>  1 file changed, 28 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmc.c b/drivers/platform/x86/amd/pmc.c
>> index 3cbb01ec10e3..01632e6b7820 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,38 @@ 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;
>> +	}
>> +
>> +	dev->stb_virt_addr += stb_rdptr_offset;
>> +	memcpy_fromio(buf, dev->stb_virt_addr, fsize);
> 
> This looks wrong, you are moving the pointer stored in the amd_pmc_dev
> struct and subsequent uses of stb_virt_addr will now all use the moved
> pointer...
> 
> I think that instead of these 2 lines you actually want:
> 
> 	memcpy_fromio(buf, dev->stb_virt_addr + stb_rdptr_offset, fsize);

This is a good catch. Thank you; will respin a v2.

Thanks,
Shyam

> 
> Regards,
> 
> Hans
> 
> 
> 
> 
>> +
>>  	filp->private_data = buf;
>>  
>>  	return 0;
>