[PATCH 0/9] drm/amdgpu/gfx7: Use GFX IP block soft reset on GFX7

[PATCH 0/9] drm/amdgpu/gfx7: Use GFX IP block soft reset on GFX7

[Timur Kristóf]

GFX IP block soft reset has been implemented as recovery
method so that we can have a way to reset just the GFX
block without resetting the whole GPU or losing the
contents of VRAM.

Fix up various things in the GFX7 code to prepare it
to work with IP block soft reset. The main challenge
was fixing up the MQD/HQD handling to ensure that it
doesn't hang after the soft reset. There are also
other minor fixups.

Rework the pre-existing GFX7 soft reset implementation
like the reworked version on GFX8 to make it more robust.
In practice, this means that it will now reset everything
in the GFX IP block (instead of taking a guess at which
parts need to be reset) to make it consistent, and it now
also handles clock and power gating to make sure not to
degrade GPU functionality after a GFX IP block soft reset.

This improves current user experience on all GFX7 chips:

On Kaveri and Kabini there is currently no working
GPU recovery method so those chips currently require
the user to manually reset the computer when there
was a hang.

On Hawaii and Bonaire, the current GPU recovery method
always clears the contents of VRAM, which means that
a buggy (hanging) app can crash the whole graphical
session, which is less than ideal.

Using GFX IP block soft reset means that we can now
have a working recovery on GFX7 APUs and we can also
move on from GFX hangs on dGPUs without crashing the
whole system.

Tested on the following chips:

Bonaire (Radeon HD 7790)
Hawaii (Radeon R9 390X)
Kaveri (A10-7850K)

Timur Kristóf (9):
  drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7
  drm/amdgpu/gfx7: Refactor MQD initialization and finalization
  drm/amdgpu/gfx7: Return error code when compute ring tests fail
  drm/amdgpu/gfx7: Return error code when failing to start GFX ring
  drm/amdgpu/gfx7: Fixup emitting SWITCH_BUFFER packets
  drm/amdgpu/gfx7: Clean up gfx ring during reset
  drm/amdgpu/gfx7: Use COND_EXEC
  drm/amdgpu/gfx7: Fixup IP block soft reset
  drm/amdgpu/gfx7: Enable IP block soft reset as a GPU recovery method

 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 333 +++++++++++++-----------
 2 files changed, 181 insertions(+), 154 deletions(-)

-- 
2.55.0

[PATCH 1/9] drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7

[Timur Kristóf]

We don't use KIQ on GFX7 but otherwise MQD works the
same way as GFX8 and newer.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 96c9d4f00b27..0f142c156afa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -420,7 +420,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
 #endif
 
 	/* create MQD for KIQ */
-	if (!adev->enable_mes_kiq && !ring->mqd_obj) {
+	if (adev->asic_type >= CHIP_TOPAZ && !adev->enable_mes_kiq && !ring->mqd_obj) {
 		/* originaly the KIQ MQD is put in GTT domain, but for SRIOV VRAM domain is a must
 		 * otherwise hypervisor trigger SAVE_VF fail after driver unloaded which mean MQD
 		 * deallocated and gart_unbind, to strict diverage we decide to use VRAM domain for
-- 
2.55.0

[PATCH 2/9] drm/amdgpu/gfx7: Refactor MQD initialization and finalization

[Timur Kristóf]

Call amdgpu_gfx_mqd_sw_init()/_fini() on GFX7 to initialize and
finalize the MQD, just like GFX8 and newer; instead of doing
an ad-hoc BO allocation. This introduces the possibility of
doing an MQD backup instead of trying to reinitialize the
MQD every time.

This solves an issue with GFX IP block soft reset where
all compute rings would hang after the reset.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 106 +++++++++++++-------------
 1 file changed, 51 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 65b8497ad5f0..9c4b3ac27e1f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2698,25 +2698,6 @@ static int gfx_v7_0_cp_compute_load_microcode(struct amdgpu_device *adev)
 	return 0;
 }
 
-/**
- * gfx_v7_0_cp_compute_fini - stop the compute queues
- *
- * @adev: amdgpu_device pointer
- *
- * Stop the compute queues and tear down the driver queue
- * info.
- */
-static void gfx_v7_0_cp_compute_fini(struct amdgpu_device *adev)
-{
-	int i;
-
-	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
-		struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
-
-		amdgpu_bo_free_kernel(&ring->mqd_obj, NULL, NULL);
-	}
-}
-
 static void gfx_v7_0_mec_fini(struct amdgpu_device *adev)
 {
 	amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL);
@@ -2788,28 +2769,29 @@ static void gfx_v7_0_compute_pipe_init(struct amdgpu_device *adev,
 	mutex_unlock(&adev->srbm_mutex);
 }
 
-static int gfx_v7_0_mqd_deactivate(struct amdgpu_device *adev)
+static int gfx_v7_0_mqd_deactivate(struct amdgpu_device *adev, u32 req)
 {
-	int i;
+	int i, r = 0;
 
 	/* disable the queue if it's active */
-	if (RREG32(mmCP_HQD_ACTIVE) & 1) {
-		WREG32(mmCP_HQD_DEQUEUE_REQUEST, 1);
+	if (RREG32(mmCP_HQD_ACTIVE) & CP_HQD_ACTIVE__ACTIVE_MASK) {
+		WREG32_FIELD(CP_HQD_DEQUEUE_REQUEST, DEQUEUE_REQ, req);
 		for (i = 0; i < adev->usec_timeout; i++) {
-			if (!(RREG32(mmCP_HQD_ACTIVE) & 1))
+			if (!(RREG32(mmCP_HQD_ACTIVE) & CP_HQD_ACTIVE__ACTIVE_MASK))
 				break;
 			udelay(1);
 		}
 
 		if (i == adev->usec_timeout)
-			return -ETIMEDOUT;
+			r = -ETIMEDOUT;
 
-		WREG32(mmCP_HQD_DEQUEUE_REQUEST, 0);
-		WREG32(mmCP_HQD_PQ_RPTR, 0);
-		WREG32(mmCP_HQD_PQ_WPTR, 0);
 	}
 
-	return 0;
+	WREG32(mmCP_HQD_DEQUEUE_REQUEST, 0);
+	WREG32(mmCP_HQD_PQ_RPTR, 0);
+	WREG32(mmCP_HQD_PQ_WPTR, 0);
+
+	return r;
 }
 
 static void gfx_v7_0_mqd_init(struct amdgpu_device *adev,
@@ -2964,31 +2946,42 @@ static int gfx_v7_0_mqd_commit(struct amdgpu_device *adev, struct cik_mqd *mqd)
 
 static int gfx_v7_0_compute_queue_init(struct amdgpu_device *adev, int ring_id)
 {
-	int r;
-	u64 mqd_gpu_addr;
-	struct cik_mqd *mqd;
 	struct amdgpu_ring *ring = &adev->gfx.compute_ring[ring_id];
-
-	r = amdgpu_bo_create_reserved(adev, sizeof(struct cik_mqd), PAGE_SIZE,
-				      AMDGPU_GEM_DOMAIN_GTT, &ring->mqd_obj,
-				      &mqd_gpu_addr, (void **)&mqd);
-	if (r) {
-		dev_warn(adev->dev, "(%d) create MQD bo failed\n", r);
-		return r;
+	struct cik_mqd *mqd = ring->mqd_ptr;
+	int mqd_idx = ring - &adev->gfx.compute_ring[0];
+
+	if (!amdgpu_in_reset(adev) && !adev->in_suspend) {
+		memset((void *)mqd, 0, ring->mqd_size);
+		mutex_lock(&adev->srbm_mutex);
+		cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
+		gfx_v7_0_mqd_init(adev, mqd, ring->mqd_gpu_addr, ring);
+		gfx_v7_0_mqd_deactivate(adev, 1);
+		gfx_v7_0_mqd_commit(adev, mqd);
+		cik_srbm_select(adev, 0, 0, 0, 0);
+		mutex_unlock(&adev->srbm_mutex);
+
+		if (adev->gfx.mec.mqd_backup[mqd_idx])
+			memcpy(adev->gfx.mec.mqd_backup[mqd_idx], mqd, ring->mqd_size);
+	} else {
+		/* restore MQD to a clean status */
+		if (adev->gfx.mec.mqd_backup[mqd_idx])
+			memcpy(mqd, adev->gfx.mec.mqd_backup[mqd_idx], ring->mqd_size);
+
+		/* Re-commit the restored backup */
+		mutex_lock(&adev->srbm_mutex);
+		cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
+		gfx_v7_0_mqd_deactivate(adev, 2);
+		gfx_v7_0_mqd_commit(adev, mqd);
+		cik_srbm_select(adev, 0, 0, 0, 0);
+		mutex_unlock(&adev->srbm_mutex);
+
+		/* reset ring buffer */
+		ring->wptr = 0;
+		atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
+		atomic64_set((atomic64_t *)ring->rptr_cpu_addr, 0);
+		amdgpu_ring_clear_ring(ring);
 	}
 
-	mutex_lock(&adev->srbm_mutex);
-	cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
-
-	gfx_v7_0_mqd_init(adev, mqd, mqd_gpu_addr, ring);
-	gfx_v7_0_mqd_deactivate(adev);
-	gfx_v7_0_mqd_commit(adev, mqd);
-
-	cik_srbm_select(adev, 0, 0, 0, 0);
-	mutex_unlock(&adev->srbm_mutex);
-
-	amdgpu_bo_kunmap(ring->mqd_obj);
-	amdgpu_bo_unreserve(ring->mqd_obj);
 	return 0;
 }
 
@@ -3020,10 +3013,8 @@ static int gfx_v7_0_cp_compute_resume(struct amdgpu_device *adev)
 	/* init the queues */
 	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
 		r = gfx_v7_0_compute_queue_init(adev, i);
-		if (r) {
-			gfx_v7_0_cp_compute_fini(adev);
+		if (r)
 			return r;
-		}
 	}
 
 	gfx_v7_0_cp_compute_enable(adev, true);
@@ -4430,6 +4421,11 @@ static int gfx_v7_0_sw_init(struct amdgpu_ip_block *ip_block)
 		}
 	}
 
+	/* create MQD for all compute queues */
+	r = amdgpu_gfx_mqd_sw_init(adev, sizeof(struct cik_mqd), 0);
+	if (r)
+		return r;
+
 	adev->gfx.ce_ram_size = 0x8000;
 
 	gfx_v7_0_gpu_early_init(adev);
@@ -4452,7 +4448,7 @@ static int gfx_v7_0_sw_fini(struct amdgpu_ip_block *ip_block)
 	for (i = 0; i < adev->gfx.num_compute_rings; i++)
 		amdgpu_ring_fini(&adev->gfx.compute_ring[i]);
 
-	gfx_v7_0_cp_compute_fini(adev);
+	amdgpu_gfx_mqd_sw_fini(adev, 0);
 	amdgpu_gfx_rlc_fini(adev);
 	gfx_v7_0_mec_fini(adev);
 	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,
-- 
2.55.0

[PATCH 3/9] drm/amdgpu/gfx7: Return error code when compute ring tests fail

[Timur Kristóf]

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 <timur.kristof@gmail.com>
---
 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)
-- 
2.55.0

[PATCH 4/9] drm/amdgpu/gfx7: Return error code when failing to start GFX ring

[Timur Kristóf]

Return an error code instead of silently failing.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index a1a9f3fc4567..0ceadb107d26 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2576,7 +2576,10 @@ static int gfx_v7_0_cp_gfx_resume(struct amdgpu_device *adev)
 	WREG32(mmCP_RB0_BASE_HI, upper_32_bits(rb_addr));
 
 	/* start the ring */
-	gfx_v7_0_cp_gfx_start(adev);
+	r = gfx_v7_0_cp_gfx_start(adev);
+	if (r)
+		return r;
+
 	r = amdgpu_ring_test_helper(ring);
 	if (r)
 		return r;
-- 
2.55.0

[PATCH 5/9] drm/amdgpu/gfx7: Fixup emitting SWITCH_BUFFER packets

[Timur Kristóf]

Implement the emit_switch_buffer() function instead of emitting
them duing emit_ib, emit_pipeline_sync and emit_vm_flush.

Note that it isn't necessary to emit these in both
emit_pipeline_sync() and emit_vm_flush() because
amdgpu_vm_flush() already calls these when calling
either of those functions.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 32 +++++++++------------------
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 0ceadb107d26..a93cc02c3400 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2201,12 +2201,6 @@ static void gfx_v7_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 	unsigned vmid = AMDGPU_JOB_GET_VMID(job);
 	u32 header, control = 0;
 
-	/* insert SWITCH_BUFFER packet before first IB in the ring frame */
-	if (flags & AMDGPU_HAVE_CTX_SWITCH) {
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
-
 	if (ib->flags & AMDGPU_IB_FLAG_CE)
 		header = PACKET3(PACKET3_INDIRECT_BUFFER_CONST, 2);
 	else
@@ -2258,6 +2252,12 @@ static void gfx_v7_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
 	amdgpu_ring_write(ring, control);
 }
 
+static void gfx_v7_0_ring_emit_sb(struct amdgpu_ring *ring)
+{
+	amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
+	amdgpu_ring_write(ring, 0);
+}
+
 static void gfx_v7_ring_emit_cntxcntl(struct amdgpu_ring *ring, uint32_t flags)
 {
 	uint32_t dw2 = 0;
@@ -3111,14 +3111,6 @@ static void gfx_v7_0_ring_emit_pipeline_sync(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, seq);
 	amdgpu_ring_write(ring, 0xffffffff);
 	amdgpu_ring_write(ring, 4); /* poll interval */
-
-	if (usepfp) {
-		/* sync CE with ME to prevent CE fetch CEIB before context switch done */
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-	}
 }
 
 /*
@@ -3160,12 +3152,6 @@ static void gfx_v7_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 		/* sync PFP to ME, otherwise we might get invalid PFP reads */
 		amdgpu_ring_write(ring, PACKET3(PACKET3_PFP_SYNC_ME, 0));
 		amdgpu_ring_write(ring, 0x0);
-
-		/* synce CE with ME to prevent CE fetch CEIB before context switch done */
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
-		amdgpu_ring_write(ring, PACKET3(PACKET3_SWITCH_BUFFER, 0));
-		amdgpu_ring_write(ring, 0);
 	}
 }
 
@@ -4954,8 +4940,9 @@ static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_gfx = {
 		7 + /* gfx_v7_0_ring_emit_hdp_flush */
 		5 + /* hdp invalidate */
 		12 + 12 + 12 + /* gfx_v7_0_ring_emit_fence_gfx x3 for user fence, vm fence */
-		7 + 4 + /* gfx_v7_0_ring_emit_pipeline_sync */
-		CIK_FLUSH_GPU_TLB_NUM_WREG * 5 + 7 + 6 + /* gfx_v7_0_ring_emit_vm_flush */
+		7 + /* gfx_v7_0_ring_emit_pipeline_sync */
+		CIK_FLUSH_GPU_TLB_NUM_WREG * 5 + 7 + 2 + /* gfx_v7_0_ring_emit_vm_flush */
+		3 * 2 + /* gfx_v7_0_ring_emit_sb x3 (from amdgpu_vm_flush, amdgpu_ib_schedule) */
 		3 + 4 + /* gfx_v7_ring_emit_cntxcntl including vgt flush*/
 		5, /* SURFACE_SYNC */
 	.emit_ib_size = 4, /* gfx_v7_0_ring_emit_ib_gfx */
@@ -4969,6 +4956,7 @@ static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_gfx = {
 	.test_ib = gfx_v7_0_ring_test_ib,
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
+	.emit_switch_buffer = gfx_v7_0_ring_emit_sb,
 	.emit_cntxcntl = gfx_v7_ring_emit_cntxcntl,
 	.emit_wreg = gfx_v7_0_ring_emit_wreg,
 	.soft_recovery = gfx_v7_0_ring_soft_recovery,
-- 
2.55.0

[PATCH 6/9] drm/amdgpu/gfx7: Clean up gfx ring during reset

[Timur Kristóf]

Clear the WPTR and RPTR at ring initialization.
Additionally clear the ring contents during reset.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index a93cc02c3400..915612628f9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -2546,8 +2546,14 @@ static int gfx_v7_0_cp_gfx_resume(struct amdgpu_device *adev)
 	WREG32(mmSCRATCH_ADDR, 0);
 
 	/* ring 0 - compute and gfx */
-	/* Set ring buffer size */
 	ring = &adev->gfx.gfx_ring[0];
+	atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
+	atomic64_set((atomic64_t *)ring->rptr_cpu_addr, 0);
+
+	if (amdgpu_in_reset(adev))
+		amdgpu_ring_clear_ring(ring);
+
+	/* Set ring buffer size */
 	rb_bufsz = order_base_2(ring->ring_size / 8);
 	tmp = (order_base_2(AMDGPU_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
 #ifdef __BIG_ENDIAN
@@ -2559,6 +2565,7 @@ static int gfx_v7_0_cp_gfx_resume(struct amdgpu_device *adev)
 	WREG32(mmCP_RB0_CNTL, tmp | CP_RB0_CNTL__RB_RPTR_WR_ENA_MASK);
 	ring->wptr = 0;
 	WREG32(mmCP_RB0_WPTR, lower_32_bits(ring->wptr));
+	WREG32(mmCP_RB0_RPTR, lower_32_bits(ring->wptr));
 
 	/* set the wb address whether it's enabled or not */
 	rptr_addr = ring->rptr_gpu_addr;
-- 
2.55.0

[PATCH 7/9] drm/amdgpu/gfx7: Use COND_EXEC

[Timur Kristóf]

COND_EXEC tells the CP to discard the dwords following it
when its condition is zero (false).

This is useful for GPU recovery because it can help reduce
collateral damage during GFX IP block soft reset, meaning
that it reduces the likelyhood that we fail some jobs which
are not guilty of the hang as the IP block soft reset
mechanism clears the condition before doing the reset.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 915612628f9a..6d52b8710437 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -3162,6 +3162,22 @@ static void gfx_v7_0_ring_emit_vm_flush(struct amdgpu_ring *ring,
 	}
 }
 
+static unsigned int gfx_v7_0_ring_emit_init_cond_exec(struct amdgpu_ring *ring,
+						      uint64_t gpu_addr)
+{
+	unsigned int ret;
+
+	/* Discard following DWs after this packet when gpu_addr==0 */
+	amdgpu_ring_write(ring, PACKET3(PACKET3_COND_EXEC, 3));
+	amdgpu_ring_write(ring, lower_32_bits(gpu_addr));
+	amdgpu_ring_write(ring, upper_32_bits(gpu_addr));
+	amdgpu_ring_write(ring, 0);
+	ret = ring->wptr & ring->buf_mask;
+	/* patch dummy value later */
+	amdgpu_ring_write(ring, 0);
+	return ret;
+}
+
 static void gfx_v7_0_ring_emit_wreg(struct amdgpu_ring *ring,
 				    uint32_t reg, uint32_t val)
 {
@@ -4943,6 +4959,8 @@ static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_gfx = {
 	.get_wptr = gfx_v7_0_ring_get_wptr_gfx,
 	.set_wptr = gfx_v7_0_ring_set_wptr_gfx,
 	.emit_frame_size =
+		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from amdgpu_ib_schedule) */
+		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from amdgpu_vm_flush) */
 		20 + /* gfx_v7_0_ring_emit_gds_switch */
 		7 + /* gfx_v7_0_ring_emit_hdp_flush */
 		5 + /* hdp invalidate */
@@ -4965,6 +4983,7 @@ static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_gfx = {
 	.pad_ib = amdgpu_ring_generic_pad_ib,
 	.emit_switch_buffer = gfx_v7_0_ring_emit_sb,
 	.emit_cntxcntl = gfx_v7_ring_emit_cntxcntl,
+	.init_cond_exec = gfx_v7_0_ring_emit_init_cond_exec,
 	.emit_wreg = gfx_v7_0_ring_emit_wreg,
 	.soft_recovery = gfx_v7_0_ring_soft_recovery,
 	.emit_mem_sync = gfx_v7_0_emit_mem_sync,
@@ -4979,6 +4998,8 @@ static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_compute = {
 	.get_wptr = gfx_v7_0_ring_get_wptr_compute,
 	.set_wptr = gfx_v7_0_ring_set_wptr_compute,
 	.emit_frame_size =
+		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from amdgpu_ib_schedule) */
+		5 + /* gfx_v7_0_ring_emit_init_cond_exec (from amdgpu_vm_flush) */
 		20 + /* gfx_v7_0_ring_emit_gds_switch */
 		7 + /* gfx_v7_0_ring_emit_hdp_flush */
 		5 + /* hdp invalidate */
@@ -4997,6 +5018,7 @@ static const struct amdgpu_ring_funcs gfx_v7_0_ring_funcs_compute = {
 	.test_ib = gfx_v7_0_ring_test_ib,
 	.insert_nop = amdgpu_ring_insert_nop,
 	.pad_ib = amdgpu_ring_generic_pad_ib,
+	.init_cond_exec = gfx_v7_0_ring_emit_init_cond_exec,
 	.emit_wreg = gfx_v7_0_ring_emit_wreg,
 	.soft_recovery = gfx_v7_0_ring_soft_recovery,
 	.emit_mem_sync = gfx_v7_0_emit_mem_sync_compute,
-- 
2.55.0

[PATCH 8/9] drm/amdgpu/gfx7: Fixup IP block soft reset

[Timur Kristóf]

Use basically the same implementation as GFX8,
except for the GFX7 specific MQD functions.

Reset every block using the GRBM, then proceed
to reset the GRBM and SEM blocks using the SRBM.

Remove the redundant gfx_v7_0_update_cg() function.
The soft reset now calls the clock and powergating
functions of the IP block instead.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 148 +++++++++++++-------------
 1 file changed, 76 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 6d52b8710437..825e3d7d5f0f 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -3618,21 +3618,6 @@ static void gfx_v7_0_enable_mgcg(struct amdgpu_device *adev, bool enable)
 	}
 }
 
-static void gfx_v7_0_update_cg(struct amdgpu_device *adev,
-			       bool enable)
-{
-	gfx_v7_0_enable_gui_idle_interrupt(adev, false);
-	/* order matters! */
-	if (enable) {
-		gfx_v7_0_enable_mgcg(adev, true);
-		gfx_v7_0_enable_cgcg(adev, true);
-	} else {
-		gfx_v7_0_enable_cgcg(adev, false);
-		gfx_v7_0_enable_mgcg(adev, false);
-	}
-	gfx_v7_0_enable_gui_idle_interrupt(adev, true);
-}
-
 static void gfx_v7_0_enable_sclk_slowdown_on_pu(struct amdgpu_device *adev,
 						bool enable)
 {
@@ -4551,80 +4536,99 @@ static int gfx_v7_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
 
 static int gfx_v7_0_soft_reset(struct amdgpu_ip_block *ip_block)
 {
+	struct amdgpu_device *adev = ip_block->adev;
 	u32 grbm_soft_reset = 0, srbm_soft_reset = 0;
 	u32 tmp;
-	struct amdgpu_device *adev = ip_block->adev;
+	int i;
+	int r;
 
-	/* GRBM_STATUS */
-	tmp = RREG32(mmGRBM_STATUS);
-	if (tmp & (GRBM_STATUS__PA_BUSY_MASK | GRBM_STATUS__SC_BUSY_MASK |
-		   GRBM_STATUS__BCI_BUSY_MASK | GRBM_STATUS__SX_BUSY_MASK |
-		   GRBM_STATUS__TA_BUSY_MASK | GRBM_STATUS__VGT_BUSY_MASK |
-		   GRBM_STATUS__DB_BUSY_MASK | GRBM_STATUS__CB_BUSY_MASK |
-		   GRBM_STATUS__GDS_BUSY_MASK | GRBM_STATUS__SPI_BUSY_MASK |
-		   GRBM_STATUS__IA_BUSY_MASK | GRBM_STATUS__IA_BUSY_NO_DMA_MASK))
-		grbm_soft_reset |= GRBM_SOFT_RESET__SOFT_RESET_CP_MASK |
-			GRBM_SOFT_RESET__SOFT_RESET_GFX_MASK;
-
-	if (tmp & (GRBM_STATUS__CP_BUSY_MASK | GRBM_STATUS__CP_COHERENCY_BUSY_MASK)) {
-		grbm_soft_reset |= GRBM_SOFT_RESET__SOFT_RESET_CP_MASK;
-		srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_GRBM_MASK;
-	}
+	grbm_soft_reset =
+		REG_SET_FIELD(0, GRBM_SOFT_RESET, SOFT_RESET_RLC, 1) |
+		REG_SET_FIELD(0, GRBM_SOFT_RESET, SOFT_RESET_GFX, 1) |
+		REG_SET_FIELD(0, GRBM_SOFT_RESET, SOFT_RESET_CP, 1) |
+		REG_SET_FIELD(0, GRBM_SOFT_RESET, SOFT_RESET_CPF, 1) |
+		REG_SET_FIELD(0, GRBM_SOFT_RESET, SOFT_RESET_CPC, 1) |
+		REG_SET_FIELD(0, GRBM_SOFT_RESET, SOFT_RESET_CPG, 1);
+
+	srbm_soft_reset =
+		REG_SET_FIELD(0, SRBM_SOFT_RESET, SOFT_RESET_GRBM, 1) |
+		REG_SET_FIELD(0, SRBM_SOFT_RESET, SOFT_RESET_SEM, 1);
+
+	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
+		struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
+
+		mutex_lock(&adev->srbm_mutex);
+		cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
+		gfx_v7_0_mqd_deactivate(adev, 2);
+		cik_srbm_select(adev, 0, 0, 0, 0);
+		mutex_unlock(&adev->srbm_mutex);
 
-	/* GRBM_STATUS2 */
-	tmp = RREG32(mmGRBM_STATUS2);
-	if (tmp & GRBM_STATUS2__RLC_BUSY_MASK)
-		grbm_soft_reset |= GRBM_SOFT_RESET__SOFT_RESET_RLC_MASK;
+		udelay(50);
+	}
 
-	/* SRBM_STATUS */
-	tmp = RREG32(mmSRBM_STATUS);
-	if (tmp & SRBM_STATUS__GRBM_RQ_PENDING_MASK)
-		srbm_soft_reset |= SRBM_SOFT_RESET__SOFT_RESET_GRBM_MASK;
+	ip_block->version->funcs->set_clockgating_state(ip_block, AMD_CG_STATE_UNGATE);
+	ip_block->version->funcs->set_powergating_state(ip_block, AMD_PG_STATE_UNGATE);
+	ip_block->version->funcs->suspend(ip_block);
 
 	if (grbm_soft_reset || srbm_soft_reset) {
-		/* disable CG/PG */
-		gfx_v7_0_fini_pg(adev);
-		gfx_v7_0_update_cg(adev, false);
+		tmp = RREG32(mmGMCON_DEBUG);
+		tmp = REG_SET_FIELD(tmp, GMCON_DEBUG, GFX_STALL, 1);
+		tmp = REG_SET_FIELD(tmp, GMCON_DEBUG, GFX_CLEAR, 1);
+		WREG32(mmGMCON_DEBUG, tmp);
 
-		/* stop the rlc */
-		adev->gfx.rlc.funcs->stop(adev);
+		udelay(100);
+	}
 
-		/* Disable GFX parsing/prefetching */
-		WREG32(mmCP_ME_CNTL, CP_ME_CNTL__ME_HALT_MASK | CP_ME_CNTL__PFP_HALT_MASK | CP_ME_CNTL__CE_HALT_MASK);
+	if (grbm_soft_reset) {
+		tmp = RREG32(mmGRBM_SOFT_RESET);
+		tmp |= grbm_soft_reset;
+		dev_info(adev->dev, "GRBM_SOFT_RESET=0x%08X\n", tmp);
+		WREG32(mmGRBM_SOFT_RESET, tmp);
+		tmp = RREG32(mmGRBM_SOFT_RESET);
 
-		/* Disable MEC parsing/prefetching */
-		WREG32(mmCP_MEC_CNTL, CP_MEC_CNTL__MEC_ME1_HALT_MASK | CP_MEC_CNTL__MEC_ME2_HALT_MASK);
+		udelay(100);
 
-		if (grbm_soft_reset) {
-			tmp = RREG32(mmGRBM_SOFT_RESET);
-			tmp |= grbm_soft_reset;
-			dev_info(adev->dev, "GRBM_SOFT_RESET=0x%08X\n", tmp);
-			WREG32(mmGRBM_SOFT_RESET, tmp);
-			tmp = RREG32(mmGRBM_SOFT_RESET);
+		tmp &= ~grbm_soft_reset;
+		WREG32(mmGRBM_SOFT_RESET, tmp);
+		tmp = RREG32(mmGRBM_SOFT_RESET);
 
-			udelay(50);
+		udelay(100);
+	}
 
-			tmp &= ~grbm_soft_reset;
-			WREG32(mmGRBM_SOFT_RESET, tmp);
-			tmp = RREG32(mmGRBM_SOFT_RESET);
-		}
+	if (srbm_soft_reset) {
+		tmp = RREG32(mmSRBM_SOFT_RESET);
+		tmp |= srbm_soft_reset;
+		dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
+		WREG32(mmSRBM_SOFT_RESET, tmp);
+		tmp = RREG32(mmSRBM_SOFT_RESET);
 
-		if (srbm_soft_reset) {
-			tmp = RREG32(mmSRBM_SOFT_RESET);
-			tmp |= srbm_soft_reset;
-			dev_info(adev->dev, "SRBM_SOFT_RESET=0x%08X\n", tmp);
-			WREG32(mmSRBM_SOFT_RESET, tmp);
-			tmp = RREG32(mmSRBM_SOFT_RESET);
+		udelay(100);
 
-			udelay(50);
+		tmp &= ~srbm_soft_reset;
+		WREG32(mmSRBM_SOFT_RESET, tmp);
+		tmp = RREG32(mmSRBM_SOFT_RESET);
 
-			tmp &= ~srbm_soft_reset;
-			WREG32(mmSRBM_SOFT_RESET, tmp);
-			tmp = RREG32(mmSRBM_SOFT_RESET);
-		}
-		/* Wait a little for things to settle down */
-		udelay(50);
+		udelay(100);
 	}
+
+	if (grbm_soft_reset || srbm_soft_reset) {
+		tmp = RREG32(mmGMCON_DEBUG);
+		tmp = REG_SET_FIELD(tmp, GMCON_DEBUG, GFX_STALL, 0);
+		tmp = REG_SET_FIELD(tmp, GMCON_DEBUG, GFX_CLEAR, 0);
+		WREG32(mmGMCON_DEBUG, tmp);
+	}
+
+	/* Wait a little for things to settle down */
+	udelay(100);
+
+	r = ip_block->version->funcs->resume(ip_block);
+	r |= ip_block->version->funcs->late_init(ip_block);
+	if (r)
+		return r;
+
+	ip_block->version->funcs->set_clockgating_state(ip_block, AMD_CG_STATE_GATE);
+	ip_block->version->funcs->set_powergating_state(ip_block, AMD_PG_STATE_GATE);
+
 	return 0;
 }
 
-- 
2.55.0

[PATCH 9/9] drm/amdgpu/gfx7: Enable IP block soft reset as a GPU recovery method

[Timur Kristóf]

Enable IP block soft reset as a GPU recovery method for GFX7
graphics and compute rings.

This improves current user experience on all GFX7 chips:

* On Kaveri and Kabini there is currently no working
  GPU recovery method so those chips currently require
  the user to manually reset the computer when there
  was a hang.

* On Hawaii and Bonaire, the current GPU recovery method
  always clears the contents of VRAM, which means that
  a buggy (hanging) app can crash the whole graphical
  session, which is less than ideal.

Using GFX IP block soft reset means that we can now
have a working recovery on GFX7 APUs and we can also
move on from GFX hangs on dGPUs without crashing the
whole system.

Tested with the "hard_reset_cp_wait" test case from the
Hang Test Suite created by Natalie Vock and Konstantin Seurer.
This Vulkan testcase waits for an event that never occurs,
effectively a WAIT_REG_MEM packet that intentionally hangs.
IP block soft reset can resolve that hang and allow
the rest of the system to move on and keep functioning
without needing a full ASIC reset.

Tested on the following chips:

Bonaire (Radeon HD 7790)
Hawaii (Radeon R9 390X)
Kaveri (A10-7850K)

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index 825e3d7d5f0f..1a7a6962303e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -4434,6 +4434,11 @@ static int gfx_v7_0_sw_init(struct amdgpu_ip_block *ip_block)
 	adev->gfx.compute_supported_reset =
 		amdgpu_get_soft_full_reset_mask(&adev->gfx.compute_ring[0]);
 
+	if (!amdgpu_sriov_vf(adev) && !adev->debug_disable_ip_block_soft_reset) {
+		adev->gfx.compute_supported_reset |= AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
+		adev->gfx.gfx_supported_reset |= AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
+	}
+
 	return r;
 }
 
-- 
2.55.0

Re: [PATCH 1/9] drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7

[Tvrtko Ursulin]

On 13/07/2026 13:58, Timur Kristóf wrote:
> We don't use KIQ on GFX7 but otherwise MQD works the
> same way as GFX8 and newer.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 96c9d4f00b27..0f142c156afa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -420,7 +420,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
>   #endif
>   
>   	/* create MQD for KIQ */
> -	if (!adev->enable_mes_kiq && !ring->mqd_obj) {
> +	if (adev->asic_type >= CHIP_TOPAZ && !adev->enable_mes_kiq && !ring->mqd_obj) {

CHIP_TOPAZ is gfx7? Hm if it is then the branch would already run there. 
So the change is limiting the branch to a subset of platforms, while the 
patch title made me think it is enabling something on gfx7. Perhaps 
somehow indirectly or what am I not understanding?

Regards,

Tvrtko

>   		/* originaly the KIQ MQD is put in GTT domain, but for SRIOV VRAM domain is a must
>   		 * otherwise hypervisor trigger SAVE_VF fail after driver unloaded which mean MQD
>   		 * deallocated and gart_unbind, to strict diverage we decide to use VRAM domain for

Re: [PATCH 1/9] drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7

[Alex Deucher]

On Tue, Jul 14, 2026 at 10:59 AM Tvrtko Ursulin <tursulin@ursulin.net> wrote:
>
>
> On 13/07/2026 13:58, Timur Kristóf wrote:
> > We don't use KIQ on GFX7 but otherwise MQD works the
> > same way as GFX8 and newer.
> >
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > index 96c9d4f00b27..0f142c156afa 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> > @@ -420,7 +420,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
> >   #endif
> >
> >       /* create MQD for KIQ */
> > -     if (!adev->enable_mes_kiq && !ring->mqd_obj) {
> > +     if (adev->asic_type >= CHIP_TOPAZ && !adev->enable_mes_kiq && !ring->mqd_obj) {
>
> CHIP_TOPAZ is gfx7? Hm if it is then the branch would already run there.
> So the change is limiting the branch to a subset of platforms, while the
> patch title made me think it is enabling something on gfx7. Perhaps
> somehow indirectly or what am I not understanding?

TOPAZ is gfx8.

Alex

>
> Regards,
>
> Tvrtko
>
> >               /* originaly the KIQ MQD is put in GTT domain, but for SRIOV VRAM domain is a must
> >                * otherwise hypervisor trigger SAVE_VF fail after driver unloaded which mean MQD
> >                * deallocated and gart_unbind, to strict diverage we decide to use VRAM domain for
>

Re: [PATCH 1/9] drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7

[Tvrtko Ursulin]

On 14/07/2026 16:05, Alex Deucher wrote:
> On Tue, Jul 14, 2026 at 10:59 AM Tvrtko Ursulin <tursulin@ursulin.net> wrote:
>>
>>
>> On 13/07/2026 13:58, Timur Kristóf wrote:
>>> We don't use KIQ on GFX7 but otherwise MQD works the
>>> same way as GFX8 and newer.
>>>
>>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> index 96c9d4f00b27..0f142c156afa 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> @@ -420,7 +420,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device *adev,
>>>    #endif
>>>
>>>        /* create MQD for KIQ */
>>> -     if (!adev->enable_mes_kiq && !ring->mqd_obj) {
>>> +     if (adev->asic_type >= CHIP_TOPAZ && !adev->enable_mes_kiq && !ring->mqd_obj) {
>>
>> CHIP_TOPAZ is gfx7? Hm if it is then the branch would already run there.
>> So the change is limiting the branch to a subset of platforms, while the
>> patch title made me think it is enabling something on gfx7. Perhaps
>> somehow indirectly or what am I not understanding?
> 
> TOPAZ is gfx8.

Ah now I get it, thank you! Could maybe adev->gfx[0].kiq.something or be 
used to make it a bit self-documenting?
Regards,

Tvrtko

>>
>>>                /* originaly the KIQ MQD is put in GTT domain, but for SRIOV VRAM domain is a must
>>>                 * otherwise hypervisor trigger SAVE_VF fail after driver unloaded which mean MQD
>>>                 * deallocated and gart_unbind, to strict diverage we decide to use VRAM domain for
>>

Re: [PATCH 1/9] drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7

[Timur Kristóf]

On Tuesday, July 14, 2026 5:19:57 PM Central European Summer Time Tvrtko 
Ursulin wrote:
> On 14/07/2026 16:05, Alex Deucher wrote:
> > On Tue, Jul 14, 2026 at 10:59 AM Tvrtko Ursulin <tursulin@ursulin.net> 
wrote:
> >> On 13/07/2026 13:58, Timur Kristóf wrote:
> >>> We don't use KIQ on GFX7 but otherwise MQD works the
> >>> same way as GFX8 and newer.
> >>> 
> >>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> >>> ---
> >>> 
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
> >>>    1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c index
> >>> 96c9d4f00b27..0f142c156afa 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> >>> @@ -420,7 +420,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device
> >>> *adev,
> >>> 
> >>>    #endif
> >>>    
> >>>        /* create MQD for KIQ */
> >>> 
> >>> -     if (!adev->enable_mes_kiq && !ring->mqd_obj) {
> >>> +     if (adev->asic_type >= CHIP_TOPAZ && !adev->enable_mes_kiq &&
> >>> !ring->mqd_obj) {>> 
> >> CHIP_TOPAZ is gfx7? Hm if it is then the branch would already run there.
> >> So the change is limiting the branch to a subset of platforms, while the
> >> patch title made me think it is enabling something on gfx7. Perhaps
> >> somehow indirectly or what am I not understanding?
> > 
> > TOPAZ is gfx8.

We currently don't use KIQ on GFX7 so the patch changes the code to allocate 
the BO only on GFX8 and newer. Topaz is the first GFX8 chip in the enum, so 
that's why the code checks >= TOPAZ here.

> Ah now I get it, thank you! Could maybe adev->gfx[0].kiq.something or be
> used to make it a bit self-documenting?

Technically, GFX7 supports the KIQ, amdgpu just doesn't use it. So, I fear 
that adding a field would mislead the reader into thinking that the HW support 
is missing when it really is just the kernel doesn't use it.

How would you feel about just updating the comment above the changed line?
Maybe like this?

/* create MQD for KIQ - only on GFX8+ GPUs where we use the KIQ */

I think that would make it self-explanatory.

> 
> >>>                /* originaly the KIQ MQD is put in GTT domain, but for
> >>>                SRIOV VRAM domain is a must>>>                
> >>>                 * otherwise hypervisor trigger SAVE_VF fail after driver
> >>>                 unloaded which mean MQD * deallocated and gart_unbind,
> >>>                 to strict diverage we decide to use VRAM domain for

Re: [PATCH 1/9] drm/amdgpu/gfx7: Make amdgpu_gfx_mqd_sw_init() usable on GFX7

[Tvrtko Ursulin]

On 14/07/2026 16:39, Timur Kristóf wrote:
> On Tuesday, July 14, 2026 5:19:57 PM Central European Summer Time Tvrtko
> Ursulin wrote:
>> On 14/07/2026 16:05, Alex Deucher wrote:
>>> On Tue, Jul 14, 2026 at 10:59 AM Tvrtko Ursulin <tursulin@ursulin.net>
> wrote:
>>>> On 13/07/2026 13:58, Timur Kristóf wrote:
>>>>> We don't use KIQ on GFX7 but otherwise MQD works the
>>>>> same way as GFX8 and newer.
>>>>>
>>>>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
>>>>> ---
>>>>>
>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c index
>>>>> 96c9d4f00b27..0f142c156afa 100644
>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>>>> @@ -420,7 +420,7 @@ int amdgpu_gfx_mqd_sw_init(struct amdgpu_device
>>>>> *adev,
>>>>>
>>>>>     #endif
>>>>>     
>>>>>         /* create MQD for KIQ */
>>>>>
>>>>> -     if (!adev->enable_mes_kiq && !ring->mqd_obj) {
>>>>> +     if (adev->asic_type >= CHIP_TOPAZ && !adev->enable_mes_kiq &&
>>>>> !ring->mqd_obj) {>>
>>>> CHIP_TOPAZ is gfx7? Hm if it is then the branch would already run there.
>>>> So the change is limiting the branch to a subset of platforms, while the
>>>> patch title made me think it is enabling something on gfx7. Perhaps
>>>> somehow indirectly or what am I not understanding?
>>>
>>> TOPAZ is gfx8.
> 
> We currently don't use KIQ on GFX7 so the patch changes the code to allocate
> the BO only on GFX8 and newer. Topaz is the first GFX8 chip in the enum, so
> that's why the code checks >= TOPAZ here.
> 
>> Ah now I get it, thank you! Could maybe adev->gfx[0].kiq.something or be
>> used to make it a bit self-documenting?
> 
> Technically, GFX7 supports the KIQ, amdgpu just doesn't use it. So, I fear
> that adding a field would mislead the reader into thinking that the HW support
> is missing when it really is just the kernel doesn't use it.
> 
> How would you feel about just updating the comment above the changed line?
> Maybe like this?
> 
> /* create MQD for KIQ - only on GFX8+ GPUs where we use the KIQ */
> 
> I think that would make it self-explanatory.

Your call if you think that adds value or not much. I was simply 
wondering if there is some sort of a "central" source of knowledge, like 
a function or variable under gfx or adev, which is colloquially used as 
"do we use kiq". If there isn't that's fine. The patch LGTM.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

> 
>>
>>>>>                 /* originaly the KIQ MQD is put in GTT domain, but for
>>>>>                 SRIOV VRAM domain is a must>>>
>>>>>                  * otherwise hypervisor trigger SAVE_VF fail after driver
>>>>>                  unloaded which mean MQD * deallocated and gart_unbind,
>>>>>                  to strict diverage we decide to use VRAM domain for
> 
> 
> 
> 

Re: [PATCH 2/9] drm/amdgpu/gfx7: Refactor MQD initialization and finalization

[Tvrtko Ursulin]

On 13/07/2026 13:58, Timur Kristóf wrote:
> Call amdgpu_gfx_mqd_sw_init()/_fini() on GFX7 to initialize and
> finalize the MQD, just like GFX8 and newer; instead of doing
> an ad-hoc BO allocation. This introduces the possibility of
> doing an MQD backup instead of trying to reinitialize the
> MQD every time.
> 
> This solves an issue with GFX IP block soft reset where
> all compute rings would hang after the reset.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 106 +++++++++++++-------------
>   1 file changed, 51 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 65b8497ad5f0..9c4b3ac27e1f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2698,25 +2698,6 @@ static int gfx_v7_0_cp_compute_load_microcode(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> -/**
> - * gfx_v7_0_cp_compute_fini - stop the compute queues
> - *
> - * @adev: amdgpu_device pointer
> - *
> - * Stop the compute queues and tear down the driver queue
> - * info.
> - */
> -static void gfx_v7_0_cp_compute_fini(struct amdgpu_device *adev)
> -{
> -	int i;
> -
> -	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
> -		struct amdgpu_ring *ring = &adev->gfx.compute_ring[i];
> -
> -		amdgpu_bo_free_kernel(&ring->mqd_obj, NULL, NULL);
> -	}
> -}
> -
>   static void gfx_v7_0_mec_fini(struct amdgpu_device *adev)
>   {
>   	amdgpu_bo_free_kernel(&adev->gfx.mec.hpd_eop_obj, NULL, NULL);
> @@ -2788,28 +2769,29 @@ static void gfx_v7_0_compute_pipe_init(struct amdgpu_device *adev,
>   	mutex_unlock(&adev->srbm_mutex);
>   }
>   
> -static int gfx_v7_0_mqd_deactivate(struct amdgpu_device *adev)
> +static int gfx_v7_0_mqd_deactivate(struct amdgpu_device *adev, u32 req)
>   {
> -	int i;
> +	int i, r = 0;
>   
>   	/* disable the queue if it's active */
> -	if (RREG32(mmCP_HQD_ACTIVE) & 1) {
> -		WREG32(mmCP_HQD_DEQUEUE_REQUEST, 1);
> +	if (RREG32(mmCP_HQD_ACTIVE) & CP_HQD_ACTIVE__ACTIVE_MASK) {
> +		WREG32_FIELD(CP_HQD_DEQUEUE_REQUEST, DEQUEUE_REQ, req);
>   		for (i = 0; i < adev->usec_timeout; i++) {
> -			if (!(RREG32(mmCP_HQD_ACTIVE) & 1))
> +			if (!(RREG32(mmCP_HQD_ACTIVE) & CP_HQD_ACTIVE__ACTIVE_MASK))
>   				break;
>   			udelay(1);
>   		}
>   
>   		if (i == adev->usec_timeout)
> -			return -ETIMEDOUT;
> +			r = -ETIMEDOUT;
>   
> -		WREG32(mmCP_HQD_DEQUEUE_REQUEST, 0);
> -		WREG32(mmCP_HQD_PQ_RPTR, 0);
> -		WREG32(mmCP_HQD_PQ_WPTR, 0);
>   	}
>   
> -	return 0;
> +	WREG32(mmCP_HQD_DEQUEUE_REQUEST, 0);
> +	WREG32(mmCP_HQD_PQ_RPTR, 0);
> +	WREG32(mmCP_HQD_PQ_WPTR, 0);
> +
> +	return r;

I can see this matches gfx_v8_0_deactivate_hqd. If I am not missing 
anything only to replace the hardcoded 1 with CP_HQD_ACTIVE__ACTIVE_MASK?

Is it okay to call the function mqd if the registers are hqd and is v7 
or v8 (which calls it hqd) more correct? Not saying either way, just 
observing a curiosity.

>   }
>   
>   static void gfx_v7_0_mqd_init(struct amdgpu_device *adev,
> @@ -2964,31 +2946,42 @@ static int gfx_v7_0_mqd_commit(struct amdgpu_device *adev, struct cik_mqd *mqd)
>   
>   static int gfx_v7_0_compute_queue_init(struct amdgpu_device *adev, int ring_id)
>   {
> -	int r;
> -	u64 mqd_gpu_addr;
> -	struct cik_mqd *mqd;
>   	struct amdgpu_ring *ring = &adev->gfx.compute_ring[ring_id];
> -
> -	r = amdgpu_bo_create_reserved(adev, sizeof(struct cik_mqd), PAGE_SIZE,
> -				      AMDGPU_GEM_DOMAIN_GTT, &ring->mqd_obj,
> -				      &mqd_gpu_addr, (void **)&mqd);
> -	if (r) {
> -		dev_warn(adev->dev, "(%d) create MQD bo failed\n", r);
> -		return r;
> +	struct cik_mqd *mqd = ring->mqd_ptr;
> +	int mqd_idx = ring - &adev->gfx.compute_ring[0];
> +
> +	if (!amdgpu_in_reset(adev) && !adev->in_suspend) {
> +		memset((void *)mqd, 0, ring->mqd_size);
> +		mutex_lock(&adev->srbm_mutex);
> +		cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
> +		gfx_v7_0_mqd_init(adev, mqd, ring->mqd_gpu_addr, ring);
> +		gfx_v7_0_mqd_deactivate(adev, 1);
> +		gfx_v7_0_mqd_commit(adev, mqd);
> +		cik_srbm_select(adev, 0, 0, 0, 0);
> +		mutex_unlock(&adev->srbm_mutex);
> +
> +		if (adev->gfx.mec.mqd_backup[mqd_idx])
> +			memcpy(adev->gfx.mec.mqd_backup[mqd_idx], mqd, ring->mqd_size);
> +	} else {
> +		/* restore MQD to a clean status */
> +		if (adev->gfx.mec.mqd_backup[mqd_idx])
> +			memcpy(mqd, adev->gfx.mec.mqd_backup[mqd_idx], ring->mqd_size);
> +
> +		/* Re-commit the restored backup */
> +		mutex_lock(&adev->srbm_mutex);
> +		cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
> +		gfx_v7_0_mqd_deactivate(adev, 2);
> +		gfx_v7_0_mqd_commit(adev, mqd);
> +		cik_srbm_select(adev, 0, 0, 0, 0);
> +		mutex_unlock(&adev->srbm_mutex);
> +
> +		/* reset ring buffer */
> +		ring->wptr = 0;
> +		atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
> +		atomic64_set((atomic64_t *)ring->rptr_cpu_addr, 0);
> +		amdgpu_ring_clear_ring(ring);
>   	}
>   
> -	mutex_lock(&adev->srbm_mutex);
> -	cik_srbm_select(adev, ring->me, ring->pipe, ring->queue, 0);
> -
> -	gfx_v7_0_mqd_init(adev, mqd, mqd_gpu_addr, ring);
> -	gfx_v7_0_mqd_deactivate(adev);
> -	gfx_v7_0_mqd_commit(adev, mqd);
> -
> -	cik_srbm_select(adev, 0, 0, 0, 0);
> -	mutex_unlock(&adev->srbm_mutex);
> -
> -	amdgpu_bo_kunmap(ring->mqd_obj);
> -	amdgpu_bo_unreserve(ring->mqd_obj);
>   	return 0;
>   }

I think I can follow this - only the wptr and rptr reset is a bit 
different than what v8 does it. Any specific reason? Gfx9 then reverts 
back to a single ring->wptr = 0. I guess v8 is somehow special?

>   
> @@ -3020,10 +3013,8 @@ static int gfx_v7_0_cp_compute_resume(struct amdgpu_device *adev)
>   	/* init the queues */
>   	for (i = 0; i < adev->gfx.num_compute_rings; i++) {
>   		r = gfx_v7_0_compute_queue_init(adev, i);
> -		if (r) {
> -			gfx_v7_0_cp_compute_fini(adev);
> +		if (r)
>   			return r;
> -		}
>   	}
>   
>   	gfx_v7_0_cp_compute_enable(adev, true);
> @@ -4430,6 +4421,11 @@ static int gfx_v7_0_sw_init(struct amdgpu_ip_block *ip_block)
>   		}
>   	}
>   
> +	/* create MQD for all compute queues */
> +	r = amdgpu_gfx_mqd_sw_init(adev, sizeof(struct cik_mqd), 0);
> +	if (r)
> +		return r;
> +
>   	adev->gfx.ce_ram_size = 0x8000;
>   
>   	gfx_v7_0_gpu_early_init(adev);
> @@ -4452,7 +4448,7 @@ static int gfx_v7_0_sw_fini(struct amdgpu_ip_block *ip_block)
>   	for (i = 0; i < adev->gfx.num_compute_rings; i++)
>   		amdgpu_ring_fini(&adev->gfx.compute_ring[i]);
>   
> -	gfx_v7_0_cp_compute_fini(adev);
> +	amdgpu_gfx_mqd_sw_fini(adev, 0);
>   	amdgpu_gfx_rlc_fini(adev);
>   	gfx_v7_0_mec_fini(adev);
>   	amdgpu_bo_free_kernel(&adev->gfx.rlc.clear_state_obj,

I am assuming all this applies only to compute because gfx is single 
instance on v7?

Anyway, it looks plausible to me so assuming you were able to exercise 
both paths:

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

Re: [PATCH 3/9] drm/amdgpu/gfx7: Return error code when compute ring tests fail

[Tvrtko Ursulin]

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 <timur.kristof@gmail.com>
> ---
>   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.

Regards,

Tvrtko

Re: [PATCH 4/9] drm/amdgpu/gfx7: Return error code when failing to start GFX ring

[Tvrtko Ursulin]

On 13/07/2026 13:58, Timur Kristóf wrote:
> Return an error code instead of silently failing.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index a1a9f3fc4567..0ceadb107d26 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -2576,7 +2576,10 @@ static int gfx_v7_0_cp_gfx_resume(struct amdgpu_device *adev)
>   	WREG32(mmCP_RB0_BASE_HI, upper_32_bits(rb_addr));
>   
>   	/* start the ring */
> -	gfx_v7_0_cp_gfx_start(adev);
> +	r = gfx_v7_0_cp_gfx_start(adev);
> +	if (r)
> +		return r;
> +
>   	r = amdgpu_ring_test_helper(ring);
>   	if (r)
>   		return r;

Hm this one looks less "controversial" than the previous one since it 
already can error out. And surely test ring helper would error out if 
cp_gfx_start failed..

I think it also means the previous one is likely fine.

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>

Regards,

Tvrtko

Re: [PATCH 3/9] drm/amdgpu/gfx7: Return error code when compute ring tests fail

[Alex Deucher]

On Tue, Jul 14, 2026 at 4:14 PM Tvrtko Ursulin <tursulin@ursulin.net> 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 <timur.kristof@gmail.com>
> > ---
> >   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
>