[PATCH] drm/amdgpu: Remove volatile from 'wb' & from 'ptr' in amdgpu.h

[SHANMUGAM, SRINIVASAN]

On 7/24/2023 10:43 PM, Alex Deucher wrote:
> On Mon, Jul 24, 2023 at 11:54 AM Srinivasan Shanmugam
> <srinivasan.shanmugam at amd.com>  wrote:
>> Fixes the following from checkpatch.pl:
>>
>> WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
>> +       volatile uint32_t       *wb;
>>
>> WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst
>> +       volatile uint32_t               *ptr;
>>
>> 'wb' field from 'amdgpu_wb' struct & 'ptr' field from
>> 'amdgpu_mem_scratch', is not used to access h/w directly, neither they
>> are shared variables, so volatile is not necessary
> How did you come to that determination?  Both are GPU accessible
> memory allocations.  The writeback (wb) allocation happens to be in
> GTT so it's system memory, but the the mem_scratch allocation can be
> in device memory.
>
> Alex

Hi Alex,

Thanks for your feedbacks!

Commit message is misleading, I presumed that this volatile modifiers 
are used for monitoring HW status registers due to external events & for 
some shared variables - may be volatile might be needed for *wb pointer 
variable - as they may be used for caches in between (on surface level 
info), can we split this patch into two, I felt volatile for *ptr is 
unnecessary as it is type casted with void type  [(void 
**)&adev->mem_scratch.ptr); in amdgpu_device.c]- Any advises onto this 
please?

Best regards,

Srini

>> Cc: Christian König<christian.koenig at amd.com>
>> Cc: Alex Deucher<alexander.deucher at amd.com>
>> Signed-off-by: Srinivasan Shanmugam<srinivasan.shanmugam at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index a046160b6a0e..06f79a84ff4b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -502,7 +502,7 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
>>
>>   struct amdgpu_wb {
>>          struct amdgpu_bo        *wb_obj;
>> -       volatile uint32_t       *wb;
>> +       u32                     *wb;
>>          uint64_t                gpu_addr;
>>          u32                     num_wb; /* Number of wb slots actually reserved for amdgpu. */
>>          unsigned long           used[DIV_ROUND_UP(AMDGPU_MAX_WB, BITS_PER_LONG)];
>> @@ -621,7 +621,7 @@ int amdgpu_cs_wait_fences_ioctl(struct drm_device *dev, void *data,
>>   /* VRAM scratch page for HDP bug, default vram page */
>>   struct amdgpu_mem_scratch {
>>          struct amdgpu_bo                *robj;
>> -       volatile uint32_t               *ptr;
>> +       u32                             *ptr;
>>          u64                             gpu_addr;
>>   };
>>
>> --
>> 2.25.1
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230803/b77af224/attachment.htm>