Re: [PATCH v2 03/15] drm/amdgpu/gfx11: add a gfx engine reset

["Timur Kristóf" <timur.kristof@gmail.com>]

On Mon, 2026-10-26 at 08:35 +0000, Zhang, Jesse(Jie) wrote:
> > 
> > On 2026. október 22., csütörtök 10:19:43 közép-európai téli idő
> > Jesse Zhang
> > wrote:
> > > Implement the reset_engine callback for the graphics engine of
> > > gfx11,
> > > as for gfx12:
> > > - halt PFP/ME and wait for the memory requests of the CP DMA to
> > > complete;
> > > - with CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET and
> > GFX_HQD_REG_RESET set,
> > >   pulse GRBM_SOFT_RESET.SOFT_RESET_CPF/CPG, holding the MES lock,
> > > and
> > >   wait for the memory requests again;
> > > - clear CP_VMID_RESET;
> > > - restart PFP/ME from the start of their RS64 firmware and
> > > restore
> > >   CP_PFP_IB_CONTROL; if the CP doesn't go idle then, fail the
> > > reset so
> > >   that a full reset follows;
> > > - remap the kernel gfx queue through MES from its MQD backup and
> > > restart
> > >   it with gfx_v11_0_cp_gfx_start().
> > > 
> > > MES resets a gfx queue by writing CP_VMID_RESET, which the PFP
> > > handles.
> > > When the PFP hung, eg. on a bad opcode, the request stays pending
> > > across the CPF/CPG reset, and the restarted PFP then waits on it
> > > forever for queues the reset already cleared.
> > > 
> > > gfx11 has no helper to restart PFP/ME from their start addresses
> > > outside of the firmware load: add one. A later patch enables the
> > > reset
> > > with MES, async gfx rings and the RS64 firmware.
> > 
> > I don't think it's enough to soft-reset just the CP and expect the
> > GFX queue to
> > continue. There are other parts of the GFX block which can hang,
> > for example a
> > badly written NGG shader can easily hang the PA. I think we need to
> > use the
> > SOFT_RESET_GFX bit as well.
> > 
> > I noticed that the soft reset implementation needs to also re-
> > initialize the clock
> > and power gating for the GFX block, otherwise the GPU can consume
> > more
> > power after the reset than before on older GPU generations (and
> > some GPUs
> > just fall off the PCIe bus). We need to check how GFX11-12 behaves.
> > I think it
> > would be best to re-initialize the clock and power gating here too.
> > 
> > Additionally, as I said on the other patch, I think the cleanest
> > would be to reset
> > the entire GFX block instead of just parts of it, for two reasons:
> > (1) gang submit can hang both gfx and compute at the same time, and
> > those
> > hangs can only be resolved by resetting both gfx and compute.
> > (2) if we choose to reset the whole gfx block, that allows us to
> > reuse the code
> > from the suspend() and resume() functions, which means better code
> > reuse
> > and more consistency.
> > 
> > What do you think?
> > 
> 
> Hi Timur,
> 
> Thanks for the review. I'm glad you find the idea useful.
> 
> I would keep the gfx-only CP reset as a separate recovery level. Its
> purpose is to recover an isolated gfx CP hang without resetting
> independent compute workloads.
> Replacing it with a whole GFX IP reset would lose that benefit.

I see your point. I'm skeptical because the GFX7-8 soft reset code was
also supposed to do that, but it never actually worked and nobody
noticed it for many years until I fixed it recently.

> 
> There are several reasons for keeping this distinction:
> 
> - The hardware supports this narrower scope through
>   CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET and CMP_ONLY_SOFT_RESET.
>   This gives us an intermediate recovery step when a queue or pipe
> reset
>   cannot clear the hang.

We need to test each code path very thoroughly to make sure it can
recover from all possible hangs on both graphics and compute queues.
There should also be a code path that resets all engines at the same
time if needed.

> - A whole GFX IP reset also affects compute resources and requires
>   recovery of the affected MES queue state and all the KFD/ROCm
> queues.

The soft reset code already handles that for kernel queues.
It just needs to be extended to also consider user queues.

>   The gfx-only CP reset preserves that state and the pending compute
> work, although MES
>   suspend/resume can temporarily pause scheduling. We have successful
>   HIP and compute test runs across these resets, demonstrating the
> value
>   of preserving independent compute workloads.

Nice! That's impressive.

> 
> - The locking doesn't fit either: amdgpu_device_ip_soft_reset() takes
>   the reset domain for write. [...]

We can change that.

> It is also reached from the user queue
> reset path, not only from a kernel ring timeout.

I was planning to extend the soft reset code to be reachable when user
queues hang the GPU. Just haven't done it yet as currently user queues
don't yet work well on any supported GPU.

> - gfx12 has no soft_reset callback, and the gfx11 one isn't enabled
> as
>   a recovery step. A whole GFX reset for these generations needs new
>   work and validation in any case.

If we really want to have the capability to reset graphics and compute
separately, then I think the right way to go about it is to make soft
reset more fine-grained instead of introducing another reset type.
I think that would simplify the code and benefit both implementations.

In my opinion, we should do the following:

1. amdgpu_device_ip_soft_reset() needs to be extended to handle user
queues. It should be able to save and recover non-guilty user queues
(it already does that for kernel queues), and it should be reachable
when user queues hang.

2. We should add an IP block function called soft_reset_engine() and
move your engine reset implementation there. The argument should be a
bit mask, each bit representing an engine so that the function should
be able to reset multiple engines at once if needed (when all bits are
set, it should reset all engines at the same time, ie. equivalent to a
full IP block reset).

3. amdgpu_device_ip_soft_reset() should call soft_reset_engine() if it
exists.

I'm happy to help with the code if you want. Can you share what is your
test case and how to reproduce it?

Best regards,
Timur