[AMD Official Use Only - AMD Internal Distribution Only] With the change,
The series is Reviewed-by: Ruijing Dong <[email protected]> Thanks, Ruijing -----Original Message----- From: Wu, David <[email protected]> Sent: Thursday, May 15, 2025 1:30 PM To: Dong, Ruijing <[email protected]>; Wu, David <[email protected]>; [email protected]; Koenig, Christian <[email protected]> Cc: Deucher, Alexander <[email protected]>; Liu, Leo <[email protected]>; Jiang, Sonny <[email protected]> Subject: Re: [PATCH v2 5/9] drm/amdgpu: read back register after written On 2025-05-15 13:25, Dong, Ruijing wrote: > [AMD Official Use Only - AMD Internal Distribution Only] > > -----Original Message----- > From: Wu, David <[email protected]> > Sent: Thursday, May 15, 2025 12:41 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]> > Subject: [PATCH v2 5/9] drm/amdgpu: read back register after written > > The addition of register read-back in VCN v4.0.0 is intended to prevent > potential race conditions. > > Signed-off-by: David (Ming Qiang) Wu <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c > b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c > index 8fff470bce87..5acdf8fd5a62 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c > @@ -1122,6 +1122,11 @@ 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, regUVD_RB_WPTR); > + > > > > Use the same register regUVD_STATUS? good catch - I will change them. David > > > return 0; > } > > @@ -1303,6 +1308,11 @@ static int vcn_v4_0_start(struct amdgpu_vcn_inst > *vinst) > WREG32_SOC15(VCN, i, regVCN_RB_ENABLE, tmp); > fw_shared->sq.queue_mode &= ~(FW_QUEUE_RING_RESET | > FW_QUEUE_DPG_HOLD_OFF); > > + /* Keeping one read-back to ensure all register writes are done, > + * otherwise it may introduce race conditions. > + */ > + RREG32_SOC15(VCN, i, regUVD_RB_WPTR); > + > return 0; > } > > @@ -1583,6 +1593,11 @@ static void vcn_v4_0_stop_dpg_mode(struct > amdgpu_vcn_inst *vinst) > /* disable dynamic power gating mode */ > WREG32_P(SOC15_REG_OFFSET(VCN, inst_idx, regUVD_POWER_STATUS), 0, > ~UVD_POWER_STATUS__UVD_PG_MODE_MASK); > + > + /* Keeping one read-back to ensure all register writes are done, > + * otherwise it may introduce race conditions. > + */ > + RREG32_SOC15(VCN, inst_idx, regUVD_STATUS); > } > > /** > @@ -1666,6 +1681,11 @@ static int vcn_v4_0_stop(struct amdgpu_vcn_inst *vinst) > /* enable VCN power gating */ > vcn_v4_0_enable_static_power_gating(vinst); > > + /* Keeping one read-back to ensure all register writes are done, > + * otherwise it may introduce race conditions. > + */ > + RREG32_SOC15(VCN, i, regUVD_STATUS); > + > done: > if (adev->pm.dpm_enabled) > amdgpu_dpm_enable_vcn(adev, false, i); > -- > 2.49.0 >
