[PATCH 1/3] drm/amd/amdgpu: Add debugfs support for reading GPRs
StDenis, Tom
Tom.StDenis at amd.com
Tue Dec 6 12:40:19 UTC 2016
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
________________________________
From: Edward O'Callaghan <funfunctor at folklore1984.net>
Sent: Monday, December 5, 2016 14:59
To: Tom St Denis; amd-gfx at lists.freedesktop.org
Cc: StDenis, Tom
Subject: Re: [PATCH 1/3] drm/amd/amdgpu: Add debugfs support for reading GPRs
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 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20161206/f33acf38/attachment.html>
More information about the amd-gfx
mailing list