Bike shedding accepted. [ð???] Of course now I have to circle back and fix the debug tool...
Tom
________________________________
From: Nicolai Hähnle <nhaehnle@xxxxxxxxx>
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>
