On Tue, Jul 14, 2026 at 4:14 PM Tvrtko Ursulin <[email protected]> wrote:
>
>
> On 13/07/2026 13:58, Timur Kristóf wrote:
> > The gfx_v7_0_cp_compute_resume() function should only return
> > success when all compute rings are actually functional.
> > This will be especially important for soft reset which needs
> > this to know whether the reset was successful.
> >
> > Signed-off-by: Timur Kristóf <[email protected]>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index 9c4b3ac27e1f..a1a9f3fc4567 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -3019,12 +3019,14 @@ static int gfx_v7_0_cp_compute_resume(struct 
> > amdgpu_device *adev)
> >
> >       gfx_v7_0_cp_compute_enable(adev, true);
> >
> > +     r = 0;
> > +
> >       for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> >               ring = &adev->gfx.compute_ring[i];
> > -             amdgpu_ring_test_helper(ring);
> > +             r |= amdgpu_ring_test_helper(ring);
> >       }
> >
> > -     return 0;
> > +     return r;
> >   }
> >
> >   static void gfx_v7_0_cp_enable(struct amdgpu_device *adev, bool enable)
>
> Gfx8 and 9 (did not look further) do not do it like that. Should they?
> Or there is more work there to be done first?
>
> I actually might like this because maybe it gets us closer to removing
> the ring->sched.ready hack but what I am just not sure if the idea was
> to allow driver to function with some non-functional rings after resume.
> Under the premise that if they initialized during init, then after
> resume they must too, or if they don't, it is a transient glitch. I
> don't know.. I am being imaginative here thinking about silly driver
> workarounds for weird hardware glitches. It is much more likely this was
> just an oversight and it is completely fine to to error out.
>
> I have to defer to Alex and Christian on this one.

The reason for not checking the errors was because compute queue
failure was not seen as fatal.  There are a lot of compute queues
(relative to other engines), so if something happened, it seemed
better to just continue in a degraded mode with fewer compute queues
than to fail to resume in general.

Alex

>
> Regards,
>
> Tvrtko
>

Reply via email to