[AMD Official Use Only - AMD Internal Distribution Only] Then dummy read should not be limited to this register regVCN_RB1_DB_CTRL, it can be any valid readable register just for posting the previous write operations.
Thanks, Ruijing -----Original Message----- From: Wu, David <[email protected]> Sent: Tuesday, May 13, 2025 12:48 PM To: Alex Deucher <[email protected]>; Wu, David <[email protected]> Cc: [email protected]; Koenig, Christian <[email protected]>; Deucher, Alexander <[email protected]>; Liu, Leo <[email protected]>; Jiang, Sonny <[email protected]>; Dong, Ruijing <[email protected]>; [email protected] Subject: Re: [PATCH 1/2] drm/amdgpu: read back DB_CTRL register after written for VCN v4.0.5 sounds great! will adjust accordingly. David On 2025-05-13 12:44, Alex Deucher wrote: > On Tue, May 13, 2025 at 12:38 PM David (Ming Qiang) Wu > <[email protected]> wrote: >> On VCN v4.0.5 there is a race condition where the WPTR is not updated >> after starting from idle when doorbell is used. The read-back of >> regVCN_RB1_DB_CTRL register after written is to ensure the >> doorbell_index is updated before it can work properly. >> >> Link: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12528 >> Signed-off-by: David (Ming Qiang) Wu <[email protected]> >> --- >> drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c >> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c >> index ed00d35039c1..d6be8b05d7a2 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c >> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c >> @@ -1033,6 +1033,8 @@ static int vcn_v4_0_5_start_dpg_mode(struct >> amdgpu_vcn_inst *vinst, >> WREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL, >> ring->doorbell_index << >> VCN_RB1_DB_CTRL__OFFSET__SHIFT | >> VCN_RB1_DB_CTRL__EN_MASK); >> + /* Read DB_CTRL to flush the write DB_CTRL command. */ >> + RREG32_SOC15(VCN, inst_idx, regVCN_RB1_DB_CTRL); >> >> return 0; >> } >> @@ -1195,6 +1197,8 @@ static int vcn_v4_0_5_start(struct amdgpu_vcn_inst >> *vinst) >> WREG32_SOC15(VCN, i, regVCN_RB1_DB_CTRL, >> ring->doorbell_index << VCN_RB1_DB_CTRL__OFFSET__SHIFT >> | >> VCN_RB1_DB_CTRL__EN_MASK); >> + /* Read DB_CTRL to flush the write DB_CTRL command. */ >> + RREG32_SOC15(VCN, i, regVCN_RB1_DB_CTRL); > You might want to move this one down to the end of the function to > post the other subsequent writes. Arguably all of the VCNs should do > something similar. If you want to make sure a PCIe write goes > through, you need to issue a subsequent read. Doing this at the end > of each function should post all previous writes. > > Alex > >> WREG32_SOC15(VCN, i, regUVD_RB_BASE_LO, ring->gpu_addr); >> WREG32_SOC15(VCN, i, regUVD_RB_BASE_HI, >> upper_32_bits(ring->gpu_addr)); >> -- >> 2.49.0 >>
