On 7/24/2023 10:43 PM, Alex Deucher
wrote:
On Mon, Jul 24, 2023 at 11:54 AM Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx> 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 necessaryHow 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@xxxxxxx> Cc: Alex Deucher <alexander.deucher@xxxxxxx> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx> --- 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
