On Tue, May 13, 2025 at 1:54 PM Dong, Ruijing <[email protected]> wrote: > > [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.
Right. Any register will do. Alex > > 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 > >>
