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

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

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?

(Same comments go for the GFX12 patch as well.)

Thanks & best regards,
Timur




> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 176 +++++++++++++++++++++++++
>  1 file changed, 176 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c index 315dfd806d56..12b307e4106b
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -363,6 +363,8 @@ static void gfx_v11_0_ring_invalidate_tlbs(struct
> amdgpu_ring *ring, static void gfx_v11_0_set_safe_mode(struct amdgpu_device
> *adev, int xcc_id); static void gfx_v11_0_unset_safe_mode(struct
> amdgpu_device *adev, int xcc_id); static void
> gfx_v11_0_userq_priv_fault_work(struct work_struct *work); +static int
> gfx_v11_0_reset_engine(struct amdgpu_device *adev,
> +				  enum amdgpu_gfx_engine engine);
>  static void gfx_v11_0_update_perf_clk(struct amdgpu_device *adev,
>  				      bool enable);
> 
> @@ -1150,6 +1152,7 @@ static const struct amdgpu_gfx_funcs
> gfx_v11_0_gfx_funcs = { .get_gfx_shadow_info =
> &gfx_v11_0_get_gfx_shadow_info,
>  	.get_hdp_flush_mask = &amdgpu_gfx_get_hdp_flush_mask,
>  	.detect_hung_queue = &gfx_v11_0_detect_hung_queue,
> +	.reset_engine = &gfx_v11_0_reset_engine,
>  };
> 
>  static int gfx_v11_0_gpu_early_init(struct amdgpu_device *adev)
> @@ -6916,6 +6919,179 @@ static void gfx_v11_0_emit_mem_sync(struct
> amdgpu_ring *ring) amdgpu_ring_write(ring, gcr_cntl); /* GCR_CNTL */
>  }
> 
> +/* Restart the PFP and ME pipes from the start of their RS64 firmware. */
> +static void gfx_v11_0_restart_gfx_rs64(struct amdgpu_device *adev)
> +{
> +	const struct gfx_firmware_header_v2_0 *pfp_hdr, *me_hdr;
> +	u32 pipe_id, tmp;
> +
> +	pfp_hdr = (const struct gfx_firmware_header_v2_0 *)adev->gfx.pfp_fw-
>data;
> +	me_hdr = (const struct gfx_firmware_header_v2_0 *)adev->gfx.me_fw-
>data;
> +
> +	mutex_lock(&adev->srbm_mutex);
> +	for (pipe_id = 0; pipe_id < adev->gfx.me.num_pipe_per_me; pipe_id+
+) {
> +		soc21_grbm_select(adev, 0, pipe_id, 0, 0);
> +		WREG32_SOC15(GC, 0, regCP_PFP_PRGRM_CNTR_START,
> +			     (pfp_hdr->ucode_start_addr_hi << 30) |
> +			     (pfp_hdr->ucode_start_addr_lo >> 2));
> +		WREG32_SOC15(GC, 0, regCP_PFP_PRGRM_CNTR_START_HI,
> +			     pfp_hdr->ucode_start_addr_hi >> 2);
> +		WREG32_SOC15(GC, 0, regCP_ME_PRGRM_CNTR_START,
> +			     (me_hdr->ucode_start_addr_hi << 30) |
> +			     (me_hdr->ucode_start_addr_lo >> 2));
> +		WREG32_SOC15(GC, 0, regCP_ME_PRGRM_CNTR_START_HI,
> +			     me_hdr->ucode_start_addr_hi >> 2);
> +
> +		tmp = RREG32_SOC15(GC, 0, regCP_ME_CNTL);
> +		if (pipe_id == 0) {
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
PFP_PIPE0_RESET, 1);
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
ME_PIPE0_RESET, 1);
> +		} else {
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
PFP_PIPE1_RESET, 1);
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
ME_PIPE1_RESET, 1);
> +		}
> +		WREG32_SOC15(GC, 0, regCP_ME_CNTL, tmp);
> +		if (pipe_id == 0) {
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
PFP_PIPE0_RESET, 0);
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
ME_PIPE0_RESET, 0);
> +		} else {
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
PFP_PIPE1_RESET, 0);
> +			tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, 
ME_PIPE1_RESET, 0);
> +		}
> +		WREG32_SOC15(GC, 0, regCP_ME_CNTL, tmp);
> +	}
> +	soc21_grbm_select(adev, 0, 0, 0, 0);
> +	mutex_unlock(&adev->srbm_mutex);
> +}
> +
> +/* The CP DMA and the interfaces its memory requests go through */
> +#define GFX11_CP_STAT_MEM_BUSY	(CP_STAT__DMA_BUSY_MASK | \
> +				 CP_STAT__TCIU_BUSY_MASK | \
> +				 CP_STAT__RCIU_BUSY_MASK | \
> +				 CP_STAT__UTCL2IU_BUSY_MASK)
> +
> +/*
> + * Soft reset the gfx CP and restart PFP and ME. Returns 0, or -ETIMEDOUT
> if the + * CP doesn't go idle after PFP and ME are restarted.
> + */
> +static int gfx_v11_0_soft_reset_gfx_cp(struct amdgpu_device *adev)
> +{
> +	u32 tmp, cntl, me_cntl, ib_control;
> +	int i;
> +
> +	amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
> +
> +	/* MES fetches its queues through CPF: keep it out of the reset. */
> +	amdgpu_mes_lock(&adev->mes);
> +
> +	me_cntl = RREG32_SOC15(GC, 0, regCP_ME_CNTL);
> +	/* Cleared by the reset. */
> +	ib_control = RREG32_SOC15(GC, 0, regCP_PFP_IB_CONTROL);
> +
> +	tmp = REG_SET_FIELD(me_cntl, CP_ME_CNTL, ME_HALT, 1);
> +	tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, PFP_HALT, 1);
> +	WREG32_SOC15(GC, 0, regCP_ME_CNTL, tmp);
> +	udelay(50);
> +
> +	/* A CP DMA in flight during the reset leaves the ME busy after it. 
*/
> +	for (i = 0; i < adev->usec_timeout; i++) {
> +		if (!(RREG32_SOC15(GC, 0, regCP_STAT) & 
GFX11_CP_STAT_MEM_BUSY))
> +			break;
> +		udelay(1);
> +	}
> +
> +	/* Without GFX_ONLY_SOFT_RESET, a CPF reset would also hit compute 
and
> MES. */ +	cntl = RREG32_SOC15(GC, 0, regCP_SOFT_RESET_CNTL);
> +	tmp = REG_SET_FIELD(cntl, CP_SOFT_RESET_CNTL, GFX_ONLY_SOFT_RESET, 
1);
> +	tmp = REG_SET_FIELD(tmp, CP_SOFT_RESET_CNTL, GFX_HQD_REG_RESET, 1);
> +	WREG32_SOC15(GC, 0, regCP_SOFT_RESET_CNTL, tmp);
> +
> +	/* A CPF reset alone leaves the streaming engine stuck. */
> +	tmp = RREG32_SOC15(GC, 0, regGRBM_SOFT_RESET);
> +	tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_CPF, 1);
> +	tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_CPG, 1);
> +	WREG32_SOC15(GC, 0, regGRBM_SOFT_RESET, tmp);
> +	tmp = RREG32_SOC15(GC, 0, regGRBM_SOFT_RESET);
> +	udelay(50);
> +	tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_CPF, 0);
> +	tmp = REG_SET_FIELD(tmp, GRBM_SOFT_RESET, SOFT_RESET_CPG, 0);
> +	WREG32_SOC15(GC, 0, regGRBM_SOFT_RESET, tmp);
> +	tmp = RREG32_SOC15(GC, 0, regGRBM_SOFT_RESET);
> +
> +	WREG32_SOC15(GC, 0, regCP_SOFT_RESET_CNTL, cntl);
> +
> +	/* Not the whole CP: the halted PFP stays busy until restarted. */
> +	for (i = 0; i < adev->usec_timeout; i++) {
> +		if (!(RREG32_SOC15(GC, 0, regCP_STAT) & 
GFX11_CP_STAT_MEM_BUSY))
> +			break;
> +		udelay(1);
> +	}
> +
> +	/* A CP_VMID_RESET the hung PFP never handled survives the reset. 
*/
> +	WREG32_SOC15(GC, 0, regCP_VMID_RESET, 0);
> +	amdgpu_mes_unlock(&adev->mes);
> +
> +	gfx_v11_0_restart_gfx_rs64(adev);
> +	tmp = RREG32_SOC15(GC, 0, regCP_ME_CNTL);
> +	tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, CE_HALT,
> +			    REG_GET_FIELD(me_cntl, CP_ME_CNTL, 
CE_HALT));
> +	WREG32_SOC15(GC, 0, regCP_ME_CNTL, tmp);
> +	WREG32_SOC15(GC, 0, regCP_PFP_IB_CONTROL, ib_control);
> +	amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
> +
> +	tmp = RREG32_SOC15(GC, 0, regCP_ME_CNTL);
> +	tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, ME_HALT, 0);
> +	tmp = REG_SET_FIELD(tmp, CP_ME_CNTL, PFP_HALT, 0);
> +	WREG32_SOC15(GC, 0, regCP_ME_CNTL, tmp);
> +	for (i = 0; i < adev->usec_timeout; i++) {
> +		if (!RREG32_SOC15(GC, 0, regCP_STAT))
> +			return 0;
> +		udelay(1);
> +	}
> +	return -ETIMEDOUT;
> +}
> +
> +/*
> + * Reset the graphics engine: the CP front-end (CPF/CPG), PFP/ME and the
> gfx + * HQDs. MEC, MES and the compute queues keep running.
> + */
> +static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
> +				  enum amdgpu_gfx_engine engine)
> +{
> +	int i, r;
> +
> +	lockdep_assert_held(&adev->gfx.mec.reset_mutex);
> +
> +	if (engine != AMDGPU_GFX_ENGINE_GFX)
> +		return -EOPNOTSUPP;
> +
> +	amdgpu_gfx_off_ctrl(adev, false);
> +
> +	r = gfx_v11_0_soft_reset_gfx_cp(adev);
> +	if (r) {
> +		dev_err(adev->dev, "gfx CP still busy after reset, 
CP_STAT 0x%08x\n",
> +			RREG32_SOC15(GC, 0, regCP_STAT));
> +		goto exit;
> +	}
> +
> +	/* Map the kernel gfx queue again: the reset cleared its HQD. */
> +	r = amdgpu_gfx_disable_kgq(adev, 0);
> +	if (r)
> +		goto exit;
> +
> +	for (i = 0; i < adev->gfx.num_gfx_rings; i++)
> +		amdgpu_gfx_mqd_reset_restore(&adev->gfx.gfx_ring[i]);
> +	r = amdgpu_gfx_enable_kgq(adev, 0);
> +	if (r)
> +		goto exit;
> +
> +	r = gfx_v11_0_cp_gfx_start(adev);
> +
> +exit:
> +	amdgpu_gfx_off_ctrl(adev, true);
> +	return r;
> +}
> +
>  static int gfx_v11_0_reset_kgq(struct amdgpu_ring *ring,
>  			       unsigned int vmid,
>  			       struct amdgpu_fence *timedout_fence)