[PATCH 3/3] drm/amd/amdgpu: Add bank selection for MMIO debugfs (v3)

[StDenis, Tom]

Thanks.  I use vmalloc first to avoid putting the array on the stack (as good practice) then I chose vmalloc because there really isn't a need for contiguous memory and in theory it should be just as fast to allocate.  Wouldn't you typically use kmalloc when you need contiguous memory?  Though this all boils down to a single page since it's so small anyways.


Tom




________________________________
From: Nicolai Hähnle <nhaehnle at gmail.com>
Sent: Wednesday, June 29, 2016 10:20
To: Tom St Denis; amd-gfx at lists.freedesktop.org
Cc: StDenis, Tom
Subject: Re: [PATCH 3/3] drm/amd/amdgpu: Add bank selection for MMIO debugfs (v3)

Patch 1 & 3 are

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>

Patch 2 looks mostly fine to me, too, but I'm not sure about the
vmalloc/vfree vs. kmalloc/kfree.

Cheers,
Nicolai

On 29.06.2016 15:56, Tom St Denis wrote:
> (v2) Added INSTANCE selector
> (v3) Changed order of bank selectors
>
> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 35 +++++++++++++++++++++++++++---
>   1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7f08ac02f9de..4a8f66c5ff43 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2244,20 +2244,43 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
>        struct amdgpu_device *adev = f->f_inode->i_private;
>        ssize_t result = 0;
>        int r;
> +     bool use_bank;
> +     unsigned instance_bank, sh_bank, se_bank;
>
>        if (size & 0x3 || *pos & 0x3)
>                return -EINVAL;
>
> +     if (*pos & (1ULL << 62)) {
> +             se_bank = (*pos >> 24) & 0x3FF;
> +             sh_bank = (*pos >> 34) & 0x3FF;
> +             instance_bank = (*pos >> 44) & 0x3FF;
> +             use_bank = 1;
> +             *pos &= 0xFFFFFF;
> +     } else {
> +             use_bank = 0;
> +     }
> +
> +     if (use_bank) {
> +             if (sh_bank >= adev->gfx.config.max_sh_per_se ||
> +                 se_bank >= adev->gfx.config.max_shader_engines)
> +                     return -EINVAL;
> +             mutex_lock(&adev->grbm_idx_mutex);
> +             amdgpu_gfx_select_se_sh(adev, se_bank,
> +                                     sh_bank, instance_bank);
> +     }
> +
>        while (size) {
>                uint32_t value;
>
>                if (*pos > adev->rmmio_size)
> -                     return result;
> +                     goto end;
>
>                value = RREG32(*pos >> 2);
>                r = put_user(value, (uint32_t *)buf);
> -             if (r)
> -                     return r;
> +             if (r) {
> +                     result = r;
> +                     goto end;
> +             }
>
>                result += 4;
>                buf += 4;
> @@ -2265,6 +2288,12 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
>                size -= 4;
>        }
>
> +end:
> +     if (use_bank) {
> +             amdgpu_gfx_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
> +             mutex_unlock(&adev->grbm_idx_mutex);
> +     }
> +
>        return result;
>   }
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160629/62a98d65/attachment-0001.html>