On 5/15/25 18:40, David (Ming Qiang) Wu wrote: > V2: use common register UVD_STATUS for readback (standard PCI MMIO > behavior, i.e. readback post all writes to let the writes hit > the hardware) > add read-back in ..._stop() for more coverage. > > Similar to the changes made for VCN v4.0.5 where readback to post the > writes to avoid race with the doorbell, the addition of register > readback support in other VCN versions 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. The overhead introduced is negligible. > > Signed-off-by: David (Ming Qiang) Wu <[email protected]>
Exactly for the comment on patch #9 this looks good to me. With my comment cleared up feel free to add Acked-by: Christian König <[email protected]> Regards, Christian. > --- > drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c > b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c > index 21b57c29bf7d..c74947705d77 100644 > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c > @@ -1009,6 +1009,11 @@ static int vcn_v1_0_start_spg_mode(struct > amdgpu_vcn_inst *vinst) > > jpeg_v1_0_start(adev, 0); > > + /* Keeping one read-back to ensure all register writes are done, > + * otherwise it may introduce race conditions. > + */ > + RREG32_SOC15(UVD, 0, mmUVD_STATUS); > + > return 0; > } > > @@ -1154,6 +1159,11 @@ static int vcn_v1_0_start_dpg_mode(struct > amdgpu_vcn_inst *vinst) > > jpeg_v1_0_start(adev, 1); > > + /* Keeping one read-back to ensure all register writes are done, > + * otherwise it may introduce race conditions. > + */ > + RREG32_SOC15(UVD, 0, mmUVD_STATUS); > + > return 0; > } > > @@ -1216,6 +1226,12 @@ static int vcn_v1_0_stop_spg_mode(struct > amdgpu_vcn_inst *vinst) > > vcn_v1_0_enable_clock_gating(vinst); > vcn_1_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(UVD, 0, mmUVD_STATUS); > + > return 0; > } > > @@ -1250,6 +1266,11 @@ static int vcn_v1_0_stop_dpg_mode(struct > amdgpu_vcn_inst *vinst) > WREG32_P(SOC15_REG_OFFSET(UVD, 0, mmUVD_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(UVD, 0, mmUVD_STATUS); > + > return 0; > } >
