diff mbox series

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

Message ID 20230130164855.168437-2-Shyam-sundar.S-k@amd.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Updates to amd_pmc driver | expand

Commit Message

Shyam Sundar S K Jan. 30, 2023, 4:48 p.m. UTC 
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(-)

Comments

Hans de Goede Feb. 6, 2023, 12:09 p.m. UTC | #1 
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;
Shyam Sundar S K Feb. 6, 2023, 3:09 p.m. UTC | #2 
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;
>
diff mbox series

Patch

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;