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

[Sundararaju, Sathishkumar]

On 6/18/2025 1:05 PM, Sundararaju, Sathishkumar wrote:
>
>
> 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.

Got a vcn2 machine to test, reset isn't working even with the workaround 
on vcn2,
nothing looks consistent w.r.t vcn non unified queues reset functionality.

But having it enabled as it is in V9 will help in debug and work with 
firmware team on this.

Regards,
Sathish
>
> 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;
>