<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Arial,Helvetica,sans-serif;" dir="ltr"> Hi Edward, I don't think 'f' could be NULL there nor could the private data be NULL because it's initialized to point to the amdgpu_device when the entry is added to the vfs. I can sort out the whitespace though. Tom <div style="color: rgb(0, 0, 0);"> <div> <hr tabindex="-1" style="display:inline-block; width:98%"> <div id="x_divRplyFwdMsg" dir="ltr">From: Edward O'Callaghan <funfunctor@folklore1984.net> Sent: Monday, December 5, 2016 14:59 To: Tom St Denis; amd-gfx@lists.freedesktop.org Cc: StDenis, Tom Subject: Re: [PATCH 1/3] drm/amd/amdgpu: Add debugfs support for reading GPRs <div> </div> </div> </div> <div class="PlainText">Hi Tom, On 12/06/2016 05:36 AM, Tom St Denis wrote: > Implemented for SGPRs for GFX v8 initially. > > Signed-off-by: Tom St Denis <tom.stdenis@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 72 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 25 +++++++++++ > 3 files changed, 99 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index 4f64bb16a8d3..2834451eef8a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -844,6 +844,8 @@ struct amdgpu_gfx_funcs { > uint64_t (*get_gpu_clock_counter)(struct amdgpu_device *adev); > void (*select_se_sh)(struct amdgpu_device *adev, u32 se_num, u32 sh_num, u32 instance); > void (*read_wave_data)(struct amdgpu_device *adev, uint32_t simd, uint32_t wave, uint32_t *dst, int *no_fields); > + void (*read_wave_vgprs)(struct amdgpu_device *adev, uint32_t simd, uint32_t wave, uint32_t thread, uint32_t start, uint32_t size, uint32_t *dst); > + void (*read_wave_sgprs)(struct amdgpu_device *adev, uint32_t simd, uint32_t wave, uint32_t start, uint32_t size, uint32_t *dst); > }; > > struct amdgpu_gfx { > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 25ad736b5ca5..7f9cb1c85d75 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -3055,6 +3055,71 @@ static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf, > return result; > } > > +static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf, > + size_t size, loff_t *pos) > +{ > + struct amdgpu_device *adev = f->f_inode->i_private; Could f potentially be null here? > + int r; > + ssize_t result=0; result = 0 > + uint32_t offset, se, sh, cu, wave, simd, thread, bank, *data; > + > + if (size & 3 || *pos & 3) > + return -EINVAL; > + > + /* decode offset */ > + offset = (*pos & 0xFFF); > + se = ((*pos >> 12) & 0xFF); > + sh = ((*pos >> 20) & 0xFF); > + cu = ((*pos >> 28) & 0xFF); > + wave = ((*pos >> 36) & 0xFF); > + simd = ((*pos >> 44) & 0xFF); > + thread = ((*pos >> 52) & 0xFF); > + bank = ((*pos >> 60) & 1); > + > + /* sanity check offset/size */ > + if (offset + size > (adev->gfx.config.max_gprs << 2)) > + return -EINVAL; > + > + data = kmalloc_array(1024, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + /* switch to the specific se/sh/cu */ > + mutex_lock(&adev->grbm_idx_mutex); > + amdgpu_gfx_select_se_sh(adev, se, sh, cu); > + > + if (bank == 0) { > + if (adev->gfx.funcs->read_wave_vgprs) > + adev->gfx.funcs->read_wave_vgprs(adev, simd, wave, thread, offset>>2, size>>2, data); > + } else { > + if (adev->gfx.funcs->read_wave_sgprs) > + adev->gfx.funcs->read_wave_sgprs(adev, simd, wave, offset>>2, size>>2, data); > + } > + > + amdgpu_gfx_select_se_sh(adev, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF); > + mutex_unlock(&adev->grbm_idx_mutex); > + > + while (size) { > + uint32_t value; > + > + value = data[offset >> 2]; > + r = put_user(value, (uint32_t *)buf); > + if (r) { > + result = r; > + goto err; > + } > + > + result += 4; > + buf += 4; > + offset += 4; > + size -= 4; > + } > + > +err: > + kfree(data); > + return result; > +} > + > static const struct file_operations amdgpu_debugfs_regs_fops = { > .owner = THIS_MODULE, > .read = amdgpu_debugfs_regs_read, > @@ -3097,6 +3162,11 @@ static const struct file_operations amdgpu_debugfs_wave_fops = { > .read = amdgpu_debugfs_wave_read, > .llseek = default_llseek > }; > +static const struct file_operations amdgpu_debugfs_gpr_fops = { > + .owner = THIS_MODULE, > + .read = amdgpu_debugfs_gpr_read, > + .llseek = default_llseek > +}; > > static const struct file_operations *debugfs_regs[] = { > &amdgpu_debugfs_regs_fops, > @@ -3106,6 +3176,7 @@ static const struct file_operations *debugfs_regs[] = { > &amdgpu_debugfs_gca_config_fops, > &amdgpu_debugfs_sensors_fops, > &amdgpu_debugfs_wave_fops, > + &amdgpu_debugfs_gpr_fops, > }; > > static const char *debugfs_regs_names[] = { > @@ -3116,6 +3187,7 @@ static const char *debugfs_regs_names[] = { > "amdgpu_gca_config", > "amdgpu_sensors", > "amdgpu_wave", > + "amdgpu_gpr", > }; > > static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev) > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > index 9b8d3fe67adb..180309f5784c 100644 > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c > @@ -5184,6 +5184,21 @@ static uint32_t wave_read_ind(struct amdgpu_device *adev, uint32_t simd, uint32_ > return RREG32(mmSQ_IND_DATA); > } > > +static void wave_read_regs(struct amdgpu_device *adev, uint32_t simd, > + uint32_t wave, uint32_t thread, > + uint32_t regno, uint32_t num, uint32_t *out) > +{ > + WREG32(mmSQ_IND_INDEX, > + (wave << SQ_IND_INDEX__WAVE_ID__SHIFT) | > + (simd << SQ_IND_INDEX__SIMD_ID__SHIFT) | > + (regno << SQ_IND_INDEX__INDEX__SHIFT) | > + (thread << SQ_IND_INDEX__THREAD_ID__SHIFT) | > + (SQ_IND_INDEX__FORCE_READ_MASK) | > + (SQ_IND_INDEX__AUTO_INCR_MASK)); > + while (num--) > + *(out++) = RREG32(mmSQ_IND_DATA); > +} > + > static void gfx_v8_0_read_wave_data(struct amdgpu_device *adev, uint32_t simd, uint32_t wave, uint32_t *dst, int *no_fields) > { > /* type 0 wave data */ > @@ -5208,11 +5223,21 @@ static void gfx_v8_0_read_wave_data(struct amdgpu_device *adev, uint32_t simd, u > dst[(*no_fields)++] = wave_read_ind(adev, simd, wave, ixSQ_WAVE_M0); > } > > +static void gfx_v8_0_read_wave_sgprs(struct amdgpu_device *adev, uint32_t simd, > + uint32_t wave, uint32_t start, > + uint32_t size, uint32_t *dst) > +{ > + wave_read_regs( > + adev, simd, wave, 0, > + start + SQIND_WAVE_SGPRS_OFFSET, size, dst); > +} > + > > static const struct amdgpu_gfx_funcs gfx_v8_0_gfx_funcs = { > .get_gpu_clock_counter = &gfx_v8_0_get_gpu_clock_counter, > .select_se_sh = &gfx_v8_0_select_se_sh, > .read_wave_data = &gfx_v8_0_read_wave_data, > + .read_wave_sgprs = &gfx_v8_0_read_wave_sgprs, Just fyi, these & are actually redundant when it comes to func ptr's. > }; > > static int gfx_v8_0_early_init(void *handle) > Kind Regards, Edward. </div> </div> </div> </body> </html>