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

[Nicolai Hähnle]

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;
>   }
>
>