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@xxxxxxx>
>> Signed-off-by: Sanket Goswami <Sanket.Goswami@xxxxxxx>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@xxxxxxx>
>> ---
>> 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;
>
