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