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 David.Wu3@amd.com --- 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);
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));
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 David.Wu3@amd.com --- 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 8fff470bce87..24d4077254df 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c @@ -1121,6 +1121,8 @@ static int vcn_v4_0_start_dpg_mode(struct amdgpu_vcn_inst *vinst, bool indirect) 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; } @@ -1282,6 +1284,8 @@ static int vcn_v4_0_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);
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)); 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 27dcc6f37a73..d873128862e4 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c @@ -793,6 +793,8 @@ static int vcn_v5_0_0_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; } @@ -925,6 +927,8 @@ static int vcn_v5_0_0_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);
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));
On 5/13/2025 11:29 AM, David (Ming Qiang) Wu wrote:
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 David.Wu3@amd.com
Reviewed-by: Mario Limonciello mario.limonciello@amd.com
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 8fff470bce87..24d4077254df 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c @@ -1121,6 +1121,8 @@ static int vcn_v4_0_start_dpg_mode(struct amdgpu_vcn_inst *vinst, bool indirect) 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; } @@ -1282,6 +1284,8 @@ static int vcn_v4_0_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);
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)); 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 27dcc6f37a73..d873128862e4 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c @@ -793,6 +793,8 @@ static int vcn_v5_0_0_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; } @@ -925,6 +927,8 @@ static int vcn_v5_0_0_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);
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));
On Tue, May 13, 2025 at 12:38 PM David (Ming Qiang) Wu David.Wu3@amd.com wrote:
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.
Same comment here as on the previous patch.
Alex
Signed-off-by: David (Ming Qiang) Wu David.Wu3@amd.com
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 8fff470bce87..24d4077254df 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c @@ -1121,6 +1121,8 @@ static int vcn_v4_0_start_dpg_mode(struct amdgpu_vcn_inst *vinst, bool indirect) 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;} @@ -1282,6 +1284,8 @@ static int vcn_v4_0_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); 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));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 27dcc6f37a73..d873128862e4 100644 --- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c @@ -793,6 +793,8 @@ static int vcn_v5_0_0_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;} @@ -925,6 +927,8 @@ static int vcn_v5_0_0_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); 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
On 5/13/2025 11:29 AM, David (Ming Qiang) Wu 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
As this is a proper fix this tag can be:
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/12528
Signed-off-by: David (Ming Qiang) Wu David.Wu3@amd.com
Make sure the commit message has the stable tag not just the patch email.
Cc: stable@vger.kernel.org
Otherwise:
Reviewed-by: Mario Limonciello mario.limonciello@amd.com Tested-by: Mario Limonciello mario.limonciello@amd.com
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);
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));
On Tue, May 13, 2025 at 12:38 PM David (Ming Qiang) Wu David.Wu3@amd.com 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 David.Wu3@amd.com
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
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 David.Wu3@amd.com 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 David.Wu3@amd.com
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
[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 David.Wu3@amd.com Sent: Tuesday, May 13, 2025 12:48 PM To: Alex Deucher alexdeucher@gmail.com; Wu, David David.Wu3@amd.com Cc: amd-gfx@lists.freedesktop.org; Koenig, Christian Christian.Koenig@amd.com; Deucher, Alexander Alexander.Deucher@amd.com; Liu, Leo Leo.Liu@amd.com; Jiang, Sonny Sonny.Jiang@amd.com; Dong, Ruijing Ruijing.Dong@amd.com; stable@vger.kernel.org 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 David.Wu3@amd.com 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 David.Wu3@amd.com
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
On Tue, May 13, 2025 at 1:54 PM Dong, Ruijing Ruijing.Dong@amd.com 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 David.Wu3@amd.com Sent: Tuesday, May 13, 2025 12:48 PM To: Alex Deucher alexdeucher@gmail.com; Wu, David David.Wu3@amd.com Cc: amd-gfx@lists.freedesktop.org; Koenig, Christian Christian.Koenig@amd.com; Deucher, Alexander Alexander.Deucher@amd.com; Liu, Leo Leo.Liu@amd.com; Jiang, Sonny Sonny.Jiang@amd.com; Dong, Ruijing Ruijing.Dong@amd.com; stable@vger.kernel.org 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 David.Wu3@amd.com 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 David.Wu3@amd.com
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
linux-stable-mirror@lists.linaro.org
-
Alex Deucher -
David (Ming Qiang) Wu -
David Wu -
Dong, Ruijing -
Mario Limonciello