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

[StDenis, Tom]

Bike shedding accepted.  [😊]  Of course now I have to circle back and fix the debug tool...


Tom


________________________________
From: Nicolai Hähnle <nhaehnle at gmail.com>
Sent: Wednesday, June 29, 2016 09:50
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

On 29.06.2016 12:15, Tom St Denis wrote:
> (v2) Added INSTANCE selector
>
> 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..bea5f90f479e 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)) {
> +             sh_bank = (*pos >> 24) & 0x3FF;
> +             se_bank = (*pos >> 34) & 0x3FF;
> +             instance_bank = (*pos >> 44) & 0x3FF;

It's a bit of an bike-shedding quibble, but the bit shift should be
ordered SE -> SH -> INSTANCE, since the SE is the highest-level concept
and the instance number is the lowest level.

Apart from that and the comment on #1, patch 1 and 3 look good to me.
(Can't really say much about patch 2.)

Cheers,
Nicolai

> +             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/c94d0971/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OutlookEmoji-😊.png
Type: image/png
Size: 488 bytes
Desc: OutlookEmoji-😊.png
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20160629/c94d0971/attachment.png>