[AMD Official Use Only - AMD Internal Distribution Only]

Hi David,

The last register operation in vcn_v4_0_start and vcn_v5_0_0_start is write, I 
guess we can extend this little more to do a dummy read as the last register 
operation.

Thanks,
Ruijing

-----Original Message-----
From: Wu, David <[email protected]>
Sent: Tuesday, May 13, 2025 2:23 PM
To: [email protected]; Koenig, Christian <[email protected]>
Cc: Deucher, Alexander <[email protected]>; Liu, Leo <[email protected]>; 
Jiang, Sonny <[email protected]>; Dong, Ruijing <[email protected]>; 
Limonciello, Mario <[email protected]>
Subject: [PATCH V2 2/2] drm/amdgpu: read back register after written for VCN 
v4.0.0 and v5.0.0

V2: not to add extra read-back in vcn_v4_0_start and vcn_v5_0_0_start as
    there are read-back calls already. New comments for better understanding.

Similar to the previous changes made for VCN v4.0.5, the addition of register 
read-back support in VCN v4.0.0 and v5.0.0 is intended to prevent potential 
race conditions, even though such issues have not been observed yet. This 
change ensures consistency across different VCN variants and helps avoid 
similar issues on newer or closely related GPUs. The overhead introduced by 
this read-back is negligible.

Signed-off-by: David (Ming Qiang) Wu <[email protected]>
Reviewed-by: Mario Limonciello <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c   | 4 ++++
 drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index 8fff470bce873..070a2a8cdf6f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1122,6 +1122,10 @@ static int vcn_v4_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst, bool indirect)
                        ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT |
                        VCN_RB1_DB_CTRL__EN_MASK);

+       /* Keeping one read-back to ensure all register writes are done, 
otherwise
+        * it may introduce race conditions */
+       RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL);
+
        return 0;
 }

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
index 27dcc6f37a730..77c27a317e4c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
@@ -794,6 +794,10 @@ static int vcn_v5_0_0_start_dpg_mode(struct 
amdgpu_vcn_inst *vinst,
                ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT |
                VCN_RB1_DB_CTRL__EN_MASK);

+       /* Keeping one read-back to ensure all register writes are done, 
otherwise
+        * it may introduce race conditions */
+       RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL);
+
        return 0;
 }

--
2.34.1

Reply via email to