[PATCH 36/36] drm/amdgpu/vcn3: implement ring reset

[Sundararaju, Sathishkumar]

On 6/18/2025 1:44 AM, Alex Deucher wrote:
> On Tue, Jun 17, 2025 at 4:02 PM Sundararaju, Sathishkumar
> <sasundar at amd.com> wrote:
>> Hi Alex,
>>
>> On 6/17/2025 8:38 AM, Alex Deucher wrote:
>>> Use the new helpers to handle engine resets for VCN.
>>>
>>> Untested.
>>>
>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 24 ++++++++++++++++++++++++
>>>    1 file changed, 24 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>>> index 9fb0d53805892..ec4d2ab75fc4d 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>>> @@ -110,6 +110,7 @@ static int vcn_v3_0_set_pg_state(struct amdgpu_vcn_inst *vinst,
>>>                                 enum amd_powergating_state state);
>>>    static int vcn_v3_0_pause_dpg_mode(struct amdgpu_vcn_inst *vinst,
>>>                                   struct dpg_pause_state *new_state);
>>> +static int vcn_v3_0_reset(struct amdgpu_vcn_inst *vinst);
>>>
>>>    static void vcn_v3_0_dec_ring_set_wptr(struct amdgpu_ring *ring);
>>>    static void vcn_v3_0_enc_ring_set_wptr(struct amdgpu_ring *ring);
>>> @@ -289,6 +290,7 @@ static int vcn_v3_0_sw_init(struct amdgpu_ip_block *ip_block)
>>>
>>>                if (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG)
>>>                        adev->vcn.inst[i].pause_dpg_mode = vcn_v3_0_pause_dpg_mode;
>>> +             adev->vcn.inst[i].reset = vcn_v3_0_reset;
>>>        }
>>>
>>>        if (amdgpu_sriov_vf(adev)) {
>>> @@ -1869,6 +1871,7 @@ static const struct amdgpu_ring_funcs vcn_v3_0_dec_sw_ring_vm_funcs = {
>>>        .emit_wreg = vcn_dec_sw_ring_emit_wreg,
>>>        .emit_reg_wait = vcn_dec_sw_ring_emit_reg_wait,
>>>        .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
>>> +     .reset = amdgpu_vcn_ring_reset,
>> You probably wanted to add reset callback to vcn_v3_0_enc_ring_vm_funcs
>> instead ofvcn_v3_0_dec_sw_ring_vm_funcs.
> I'll fix that up.
>
>> With that, the vcn and jpeg changes in this series are :-
>>
>> Reviewed-by: Sathishkumar S <sathishkumar.sundararaju at amd.com>
>> Tested-by: Sathishkumar S <sathishkumar.sundararaju at amd.com>
> You mentioned that the start/stop sequence didn't work for some chips.
> What sequence should I use for those?
It is for vcn3 and vcn2 (non unified), I am testing on vcn3.
Your changes as it is, works for encode hang, but failed to reset decode 
hang on vcn3.
The workaround is (works for both dec and enc on vcn3) :-

vcn_v3_0_stop(vinst);
vcn_v3_0_enable_clock_gating(vinst);
vcn_v3_0_enable_static_power_gating(vinst);
vcn_v3_0_start(vinst);

If you are okay to add the workaround, that would be good, until we have 
the firmware
also handle this properly or clarify the requirements to reset the 
rings, even without it
it's a good first iteration to start, leave it your decision to add this 
or not.

Have also requested for a vcn2 machine from Lab, which I think will get 
by EOD,
I am hoping this works on vcn2 as well, since they are similar, will 
keep you updated of the result.

Regards,
Sathish
>
> Alex
>
>> Test exceptions: VCN/JPEG 4_0_3 and VCN/JPEG 5_0_1.
>>
>> Regards,
>> Sathish
>>
>>>    };
>>>
>>>    static int vcn_v3_0_limit_sched(struct amdgpu_cs_parser *p,
>>> @@ -2033,6 +2036,7 @@ static const struct amdgpu_ring_funcs vcn_v3_0_dec_ring_vm_funcs = {
>>>        .emit_wreg = vcn_v2_0_dec_ring_emit_wreg,
>>>        .emit_reg_wait = vcn_v2_0_dec_ring_emit_reg_wait,
>>>        .emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
>>> +     .reset = amdgpu_vcn_ring_reset,
>>>    };
>>>
>>>    /**
>>> @@ -2164,6 +2168,26 @@ static void vcn_v3_0_set_enc_ring_funcs(struct amdgpu_device *adev)
>>>        }
>>>    }
>>>
>>> +static int vcn_v3_0_reset(struct amdgpu_vcn_inst *vinst)
>>> +{
>>> +     int i, r;
>>> +
>>> +     vcn_v3_0_stop(vinst);
>>> +     vcn_v3_0_start(vinst);
>>> +     r = amdgpu_ring_test_ring(&vinst->ring_dec);
>>> +     if (r)
>>> +             return r;
>>> +     for (i = 0; i < vinst->num_enc_rings; i++) {
>>> +             r = amdgpu_ring_test_ring(&vinst->ring_enc[i]);
>>> +             if (r)
>>> +                     return r;
>>> +     }
>>> +     amdgpu_fence_driver_force_completion(&vinst->ring_dec);
>>> +     for (i = 0; i < vinst->num_enc_rings; i++)
>>> +             amdgpu_fence_driver_force_completion(&vinst->ring_enc[i]);
>>> +     return 0;
>>> +}
>>> +
>>>    static bool vcn_v3_0_is_idle(struct amdgpu_ip_block *ip_block)
>>>    {
>>>        struct amdgpu_device *adev = ip_block->adev;