[PATCH] drm/amd/amdgpu: Fix wave mask in amdgpu_debugfs_wave_read() (v2)

[Christian König]

Am 10.11.2017 um 20:37 schrieb Tom St Denis:
> The bottom two bits of the simd value were being put into
> the upper bits of the wave value which was likely working due
> to the bits being ignored (or aliased).
>
> Eitherway, now we mask it correctly.
>
> Signed-off-by: Tom St Denis <tom.stdenis at amd.com>

Reviewed-by: Christian König <christian.koenig at amd.com>

>
> (v2) Touch up using GENMASK_ULL to a couple of other functions too
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 40 +++++++++++++++---------------
>   1 file changed, 20 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index c1f1b8f15395..4944f8e1577b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3336,9 +3336,9 @@ static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
>   	pm_pg_lock = (*pos >> 23) & 1;
>   
>   	if (*pos & (1ULL << 62)) {
> -		se_bank = (*pos >> 24) & 0x3FF;
> -		sh_bank = (*pos >> 34) & 0x3FF;
> -		instance_bank = (*pos >> 44) & 0x3FF;
> +		se_bank = (*pos & GENMASK_ULL(33, 24)) >> 24;
> +		sh_bank = (*pos & GENMASK_ULL(43, 34)) >> 34;
> +		instance_bank = (*pos & GENMASK_ULL(53, 44)) >> 44;
>   
>   		if (se_bank == 0x3FF)
>   			se_bank = 0xFFFFFFFF;
> @@ -3412,9 +3412,9 @@ static ssize_t amdgpu_debugfs_regs_write(struct file *f, const char __user *buf,
>   	pm_pg_lock = (*pos >> 23) & 1;
>   
>   	if (*pos & (1ULL << 62)) {
> -		se_bank = (*pos >> 24) & 0x3FF;
> -		sh_bank = (*pos >> 34) & 0x3FF;
> -		instance_bank = (*pos >> 44) & 0x3FF;
> +		se_bank = (*pos & GENMASK_ULL(33, 24)) >> 24;
> +		sh_bank = (*pos & GENMASK_ULL(43, 34)) >> 34;
> +		instance_bank = (*pos & GENMASK_ULL(53, 44)) >> 44;
>   
>   		if (se_bank == 0x3FF)
>   			se_bank = 0xFFFFFFFF;
> @@ -3762,12 +3762,12 @@ static ssize_t amdgpu_debugfs_wave_read(struct file *f, char __user *buf,
>   		return -EINVAL;
>   
>   	/* decode offset */
> -	offset = (*pos & 0x7F);
> -	se = ((*pos >> 7) & 0xFF);
> -	sh = ((*pos >> 15) & 0xFF);
> -	cu = ((*pos >> 23) & 0xFF);
> -	wave = ((*pos >> 31) & 0xFF);
> -	simd = ((*pos >> 37) & 0xFF);
> +	offset = (*pos & GENMASK_ULL(6, 0));
> +	se = (*pos & GENMASK_ULL(14, 7)) >> 7;
> +	sh = (*pos & GENMASK_ULL(22, 15)) >> 15;
> +	cu = (*pos & GENMASK_ULL(30, 23)) >> 23;
> +	wave = (*pos & GENMASK_ULL(36, 31)) >> 31;
> +	simd = (*pos & GENMASK_ULL(44, 37)) >> 37;
>   
>   	/* switch to the specific se/sh/cu */
>   	mutex_lock(&adev->grbm_idx_mutex);
> @@ -3812,14 +3812,14 @@ static ssize_t amdgpu_debugfs_gpr_read(struct file *f, char __user *buf,
>   		return -EINVAL;
>   
>   	/* decode offset */
> -	offset = (*pos & 0xFFF);       /* in dwords */
> -	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);
> +	offset = *pos & GENMASK_ULL(11, 0);
> +	se = (*pos & GENMASK_ULL(19, 12)) >> 12;
> +	sh = (*pos & GENMASK_ULL(27, 20)) >> 20;
> +	cu = (*pos & GENMASK_ULL(35, 28)) >> 28;
> +	wave = (*pos & GENMASK_ULL(43, 36)) >> 36;
> +	simd = (*pos & GENMASK_ULL(51, 44)) >> 44;
> +	thread = (*pos & GENMASK_ULL(59, 52)) >> 52;
> +	bank = (*pos & GENMASK_ULL(61, 60)) >> 60;
>   
>   	data = kmalloc_array(1024, sizeof(*data), GFP_KERNEL);
>   	if (!data)