On Tue, Jul 14, 2026 at 10:02 PM Liang, Prike <[email protected]> wrote:
>
> AMD General
>
> Regards,
>       Prike
>
> > -----Original Message-----
> > From: Alex Deucher <[email protected]>
> > Sent: Tuesday, July 14, 2026 9:52 PM
> > To: Liang, Prike <[email protected]>
> > Cc: Deucher, Alexander <[email protected]>; amd-
> > [email protected]; SHANMUGAM, SRINIVASAN
> > <[email protected]>; Khatri, Sunil <[email protected]>
> > Subject: Re: [PATCH] drm/amdgpu/userq: properly account for resets
> >
> > On Tue, Jul 14, 2026 at 3:44 AM Liang, Prike <[email protected]> wrote:
> > >
> > > AMD General
> > >
> > > Regards,
> > >       Prike
> > >
> > > > -----Original Message-----
> > > > From: Liang, Prike
> > > > Sent: Tuesday, July 14, 2026 10:49 AM
> > > > To: 'Alex Deucher' <[email protected]>;
> > > > [email protected]
> > > > Cc: Deucher, Alexander <[email protected]>; SHANMUGAM,
> > > > SRINIVASAN <[email protected]>; Khatri, Sunil
> > > > <[email protected]>
> > > > Subject: RE: [PATCH] drm/amdgpu/userq: properly account for resets
> > > >
> > > >
> > > >
> > > > Regards,
> > > >       Prike
> > > >
> > > > > -----Original Message-----
> > > > > From: Alex Deucher <[email protected]>
> > > > > Sent: Tuesday, July 14, 2026 4:39 AM
> > > > > To: [email protected]
> > > > > Cc: Deucher, Alexander <[email protected]>; SHANMUGAM,
> > > > > SRINIVASAN <[email protected]>; Liang, Prike
> > > > > <[email protected]>; Khatri, Sunil <[email protected]>
> > > > > Subject: [PATCH] drm/amdgpu/userq: properly account for resets
> > > > >
> > > > > We need to increment the reset counter, force fence completion,
> > > > > and set the wedged event when a user queue is reset.
> > > > >
> > > > > mes_userq_reset_queue() handles this for collateral damage, but
> > > > > the caller needs to handle this directly for the original guilty 
> > > > > queue.
> > > > >
> > > > > Signed-off-by: Alex Deucher <[email protected]>
> > > > > Cc: Srinivasan Shanmugam <[email protected]>
> > > > > Cc: Prike Liang <[email protected]>
> > > > > Cc: Sunil Khatri <[email protected]>
> > > > > ---
> > > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 7 ++++++-
> > > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > > > index 6aa75da27f912..5e1262636e1e9 100644
> > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> > > > > @@ -146,8 +146,13 @@ static void
> > > > > amdgpu_userq_hang_detect_work(struct
> > > > > work_struct *work)
> > > > >                                                      queue, NULL, 
> > > > > NULL);
> > > > >             else
> > > > >                     r = userq_funcs->reset(queue);
> > > > > -           if (r)
> > > > > +           if (r) {
> > > > >                     gpu_reset = true;
> > > > > +           } else {
> > > > > +                   atomic_inc(&adev->gpu_reset_counter);
> > > > > +                   amdgpu_userq_fence_driver_force_completion(queue);
> > > > > +                   drm_dev_wedged_event(adev_to_drm(adev),
> > > > > DRM_WEDGE_RECOVERY_NONE, NULL);
> > > >
> > > > If we do the userq reset post similar process here just like as
> > > > mes_userq_reset_queue(), so do we need to clear the duplicate the
> > > > code in the mes_userq_reset_queue()? Otherwise, the gpu reset counter
> > should be incorrect.
> > > > Except that, we might input the wedge task info to report the guilty 
> > > > process info.
> > >
> > > As to the first question, I recheck the code and found out the guilty 
> > > user queue in
> > the amdgpu_gfx_reset_mes_compute() was reset separately before
> > mes_userq_reset_queue(), with this current reset sequence then this change 
> > it
> > make sense to update reset counter and  force complete fence.
> > >
> > > But why need to reset the original guilty queue separately, should this 
> > > guilty queue
> > was found by the amdgpu_mes_detect_and_reset_hung_queues() in the MES
> > firmware? If so, we may need to reset the guilty queue through
> > mes_userq_reset_queue() rather than reset separately, then we don't need to 
> > add
> > this change anymore.
> >
> > We still need it for gfx and sdma queues.  Those don't get reset via
> > amdgpu_gfx_reset_mes_compute(),
>
> Yes, I will check whether the original guilty compute queue can be identified 
> by amdgpu_mes_detect_and_reset_hung_queues(). If so, the separate guilty 
> queue reset via mes_userq_reset_queue() becomes redundant and can be removed. 
> The compute userq reset refcount and completed fence handling would then be 
> relocated into amdgpu_userq_hang_detect_work(), consolidating the reset path 
> into a single flow.
>

I don't think we need that; see V3 of the patch I sent out earlier today.

Alex

>
> Thanks,
> Prike
>
> > Alex
> >
> > >
> > > >
> > > > > +           }
> > > > >     } else {
> > > > >             gpu_reset = true;
> > > > >     }
> > > > > --
> > > > > 2.55.0
> > >

Reply via email to