<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <meta name="Generator" content="Microsoft Word 14 (filtered medium)"> <style></style> </head> <body lang="EN-US" link="blue" vlink="purple"> <div class="WordSection1"> You could make the wave decode a gfx callback and move the code into the IP modules.<o:p></o:p> <o:p> </o:p> Alex<o:p></o:p> <o:p> </o:p> <div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt"> <div> <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in"> From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of StDenis, Tom Sent: Friday, October 14, 2016 6:56 AM To: Nicolai Hähnle Cc: amd-gfx@lists.freedesktop.org Subject: Re: [PATCH 1/3] drm/amd/amdgpu: Add wave reader to debugfs<o:p></o:p> </div> </div> <o:p> </o:p> <div id="divtagdefaultwrapper"> Hi Nicolai,<o:p></o:p> <o:p> </o:p> I was trying to avoid having ASIC specific includes/etc in the amdgpu_device.c file. Agreed that de-numberifying it would be nice. Maybe we can add some /**/ comments to clear it up. I imagine in the future we'll add more fields (upto 32 in this design) anyways.<o:p></o:p> <o:p> </o:p> Cheers,<o:p></o:p> Tom<o:p></o:p> <o:p> </o:p> <div> <div> <div class="MsoNormal" align="center" style="text-align:center"> <hr size="2" width="98%" align="center"> </div> <div id="x_divRplyFwdMsg"> From: Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com">nhaehnle@gmail.com</a>> Sent: Friday, October 14, 2016 03:25 To: Tom St Denis; <a href="mailto:amd-gfx@lists.freedesktop.org">amd-gfx@lists.freedesktop.org</a> Cc: StDenis, Tom Subject: Re: [PATCH 1/3] drm/amd/amdgpu: Add wave reader to debugfs <o:p></o:p> <div> <o:p></o:p> </div> </div> </div> <div> On 11.10.2016 21:18, Tom St Denis wrote: > Currently supports CZ/VI. Allows nearly atomic read > of wave data from GPU. > > Signed-off-by: Tom St Denis <<a href="mailto:tom.stdenis@amd.com">tom.stdenis@amd.com</a>> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 74 ++++++++++++++++++++++++++++++ > 1 file changed, 74 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 89b353418195..b1ab6358fa0f 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2914,6 +2914,72 @@ static ssize_t amdgpu_debugfs_sensor_read(struct file *f, char __user *buf, > return !r ? 4 : r; > } > > +static uint32_t wave_read_ind(struct amdgpu_device *adev, uint32_t SQ_INDEX, uint32_t SQ_DATA, uint32_t simd, uint32_t wave, uint32_t address) > +{ > + WREG32(SQ_INDEX, (wave & 0xF) | ((simd & 0x3) << 4) | (address << 16) | (1 << 13)); > + return RREG32(SQ_DATA); > +} > + > +static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf, > + size_t size, loff_t *pos) > +{ > + struct amdgpu_device *adev = f->f_inode->i_private; > + int r, x; > + ssize_t result=0; > + uint32_t offset, se, sh, cu, wave, simd, data[16]; > + > + if (size & 3 || *pos & 3) > + return -EINVAL; > + > + /* decode offset */ > + offset = (*pos & 0x7F); > + se = ((*pos >> 7) & 0xFF); > + sh = ((*pos >> 15) & 0xFF); > + cu = ((*pos >> 23) & 0xFF); > + wave = ((*pos >> 31) & 0xFF); > + simd = ((*pos >> 37) & 0xFF); > + *pos &= 0x7F; > + > + /* switch to the specific se/sh/cu */ > + mutex_lock(&adev->grbm_idx_mutex); > + amdgpu_gfx_select_se_sh(adev, se, sh, cu); > + > + x = 0; > + if (adev->family == AMDGPU_FAMILY_CZ || adev->family == AMDGPU_FAMILY_VI) { > + /* type 0 wave data */ > + data[x++] = 0; > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x12); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x18); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x19); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x27E); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x27F); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x14); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x1A); > + data[x++] = wave_read_ind(adev, 0x2378, 0x2379, simd, wave, 0x1B); I know this is just debug code, but it's still annoying to have all these magic constants here, when there are perfectly good ixSQ_WAVE_* etc. defines in the asic_reg headers. Nicolai > + } else { > + return -EINVAL; > + } > + > + amdgpu_gfx_select_se_sh(adev, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF); > + mutex_unlock(&adev->grbm_idx_mutex); > + > + while (size && (*pos < x * 4)) { > + uint32_t value; > + > + value = data[*pos >> 2]; > + r = put_user(value, (uint32_t *)buf); > + if (r) > + return r; > + > + result += 4; > + buf += 4; > + *pos += 4; > + size -= 4; > + } > + > + return result; > +} > + > static const struct file_operations amdgpu_debugfs_regs_fops = { > .owner = THIS_MODULE, > .read = amdgpu_debugfs_regs_read, > @@ -2951,6 +3017,12 @@ static const struct file_operations amdgpu_debugfs_sensors_fops = { > .llseek = default_llseek > }; > > +static const struct file_operations amdgpu_debugfs_wave_fops = { > + .owner = THIS_MODULE, > + .read = amdgpu_debugfs_wave_read, > + .llseek = default_llseek > +}; > + > static const struct file_operations *debugfs_regs[] = { > &amdgpu_debugfs_regs_fops, > &amdgpu_debugfs_regs_didt_fops, > @@ -2958,6 +3030,7 @@ static const struct file_operations *debugfs_regs[] = { > &amdgpu_debugfs_regs_smc_fops, > &amdgpu_debugfs_gca_config_fops, > &amdgpu_debugfs_sensors_fops, > + &amdgpu_debugfs_wave_fops, > }; > > static const char *debugfs_regs_names[] = { > @@ -2967,6 +3040,7 @@ static const char *debugfs_regs_names[] = { > "amdgpu_regs_smc", > "amdgpu_gca_config", > "amdgpu_sensors", > + "amdgpu_wave", > }; > > static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) ><o:p></o:p> </div> </div> </div> </div> </div> </body> </html>