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

[Tom St Denis]

On 10/11/17 01:20 PM, Christian König wrote:
> Am 10.11.2017 um 19:03 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>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index c1f1b8f15395..cc59020d5874 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -3766,7 +3766,7 @@ static ssize_t amdgpu_debugfs_wave_read(struct 
>> file *f, char __user *buf,
>>       se = ((*pos >> 7) & 0xFF);
>>       sh = ((*pos >> 15) & 0xFF);
>>       cu = ((*pos >> 23) & 0xFF);
>> -    wave = ((*pos >> 31) & 0xFF);
>> +    wave = ((*pos >> 31) & 0x3F);
>>       simd = ((*pos >> 37) & 0xFF);
> 
> Have you considered using GENMASK_ULL instead of fiddling with the hex 
> masks manually?
> 
> E.g. that would look like this
> 
> 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;
> 
> if you ask me makes it totally obvious what is happening here and avoids 
> such typos.
> 
> Might be that we even have some GENMASK_ULL + shift macro somewhere in 
> the kernel, but I don't know of anything of hand.
> 
> Christian.


I'll take a gander it's a good suggestion.  Though this could apply to 3 
other functions in the debugfs side.  Maybe do a fix patch first and 
then another patch to switch all 4 functions over to a cleaner approach 
(using GENMASK_ULL/etc)?

Tom