Message ID | 20230125113127.3862898-2-Shyam-sundar.S-k@amd.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | Updates to amd_pmc driver | expand |
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;
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; >
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;