On Sunday, October 25, 2026 7:22:04 PM Central European Standard Time David 
Rosca wrote:
> 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.
> 

Hi David,

Can you say what the remaining warning says?
Do you think we could fix the remaining warning by removing the check for the 
SRBM_STATUS bit entirely?

Thanks,
Timur


> 
> > ---
> > 
> >   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