On 10/6/26 19:03, Timur Kristóf wrote:
It turns out that the VCE_STATUS register bits that we
used to check whether VCE1 is idle are actually not useful:
VCE_STATUS.JOB_BUSY doesn't exist until VCE3
VCE_STATUS.UENC_BUSY seems to be stuck high on Cape Verde

Instead, do the following:
Check that the read and write pointer matches for each ring.
Check VCE_STATUS.VCPU_REPORT bits to check busy status.
Finally, also check SRBM_STATUS2.VCE_BUSY just in case,
although this one seems bogus on VCE1.

Fixes: d4a640d4b9f3 ("drm/amdgpu/vce1: Implement VCE1 IP block (v2)")
Signed-off-by: Timur Kristóf <[email protected]>

There's still occasional warning that VCE is not idle on my cape verde, but it's much better than the current state where it prints it every time.

Acked-by: David Rosca <[email protected]>

---
  drivers/gpu/drm/amd/amdgpu/vce_v1_0.c         | 25 ++++++++++++++-----
  .../include/asic_reg/vce/vce_1_0_sh_mask.h    |  2 --
  2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
index b1428f5bbc81..38e21d1a3ebb 100644
--- a/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vce_v1_0.c
@@ -47,6 +47,9 @@
  #define VCE_V1_0_STACK_SIZE   (64 * 1024)
  #define VCE_V1_0_DATA_SIZE    (ALIGN(7808 * (AMDGPU_MAX_VCE_HANDLES + 1), 
VCE_V1_0_ALIGNMENT))
  #define VCE_STATUS_VCPU_REPORT_FW_LOADED_MASK 0x02
+#define VCE_STATUS_VCPU_REPORT_AUTO_BUSY_MASK   0x08
+#define VCE_STATUS_VCPU_REPORT_RB0_BUSY_MASK    0x10
+#define VCE_STATUS_VCPU_REPORT_RB1_BUSY_MASK    0x20
static void vce_v1_0_set_ring_funcs(struct amdgpu_device *adev);
  static void vce_v1_0_set_irq_funcs(struct amdgpu_device *adev);
@@ -349,19 +352,29 @@ static int vce_v1_0_mc_resume(struct amdgpu_device *adev)
   *
   * @ip_block: amdgpu_ip_block pointer
   *
- * Check whether VCE is busy according to VCE_STATUS.
+ * Check whether VCE is busy according to VCE_STATUS
+ * and whether the ring pointers match.
+ *
   * Also check whether the SRBM thinks VCE is busy, although
- * SRBM_STATUS.VCE_BUSY seems to be bogus because it
+ * SRBM_STATUS2.VCE_BUSY seems to be bogus because it
   * appears to mirror the VCE_STATUS.VCPU_REPORT_FW_LOADED bit.
   */
  static bool vce_v1_0_is_idle(struct amdgpu_ip_block *ip_block)
  {
        struct amdgpu_device *adev = ip_block->adev;
-       bool busy =
-               (RREG32(mmVCE_STATUS) & (VCE_STATUS__JOB_BUSY_MASK | 
VCE_STATUS__UENC_BUSY_MASK)) ||
-               (RREG32(mmSRBM_STATUS2) & SRBM_STATUS2__VCE_BUSY_MASK);
- return !busy;
+       /* Check whether the rings are busy. */
+       if (RREG32(mmVCE_RB_RPTR) != RREG32(mmVCE_RB_WPTR) ||
+           RREG32(mmVCE_RB_RPTR2) != RREG32(mmVCE_RB_WPTR2))
+               return false;
+
+       /* Check whether the VCPU is busy. */
+       if (RREG32(mmVCE_STATUS) & (VCE_STATUS_VCPU_REPORT_AUTO_BUSY_MASK |
+                                   VCE_STATUS_VCPU_REPORT_RB0_BUSY_MASK |
+                                   VCE_STATUS_VCPU_REPORT_RB1_BUSY_MASK))
+               return false;
+
+       return !(RREG32(mmSRBM_STATUS2) & SRBM_STATUS2__VCE_BUSY_MASK);
  }
static int vce_v1_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
diff --git a/drivers/gpu/drm/amd/include/asic_reg/vce/vce_1_0_sh_mask.h 
b/drivers/gpu/drm/amd/include/asic_reg/vce/vce_1_0_sh_mask.h
index 1f82d6f5abde..13c15c2138b3 100644
--- a/drivers/gpu/drm/amd/include/asic_reg/vce/vce_1_0_sh_mask.h
+++ b/drivers/gpu/drm/amd/include/asic_reg/vce/vce_1_0_sh_mask.h
@@ -63,8 +63,6 @@
  #define VCE_SOFT_RESET__ECPU_SOFT_RESET__SHIFT 0x00000000
  #define VCE_SOFT_RESET__FME_SOFT_RESET_MASK 0x00000004L
  #define VCE_SOFT_RESET__FME_SOFT_RESET__SHIFT 0x00000002
-#define VCE_STATUS__JOB_BUSY_MASK 0x00000001L
-#define VCE_STATUS__JOB_BUSY__SHIFT 0x00000000
  #define VCE_STATUS__UENC_BUSY_MASK 0x00000100L
  #define VCE_STATUS__UENC_BUSY__SHIFT 0x00000008
  #define VCE_STATUS__VCPU_REPORT_MASK 0x000000feL

Reply via email to