[PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring reset helpers

[PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring reset helpers

[Jesse Zhang]

amdgpu_multi_ring_reset_helper_begin() and _end() take the device from
the guilty ring. A reset triggered by a hung user queue has no guilty
kernel ring, so pass the device explicitly and allow a NULL guilty ring.

No functional change.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   |  4 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 14 ++++++++------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++--
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
index 734f2a0bc79a..d0b0d3eaffc5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
@@ -465,11 +465,11 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
 	ring_type_mask = amdgpu_ring_mask_from_ip(ip_type);
 
 	amdgpu_device_lock_reset_domain(adev->reset_domain);
-	amdgpu_multi_ring_reset_helper_begin(ring_type_mask, guilty_ring, guilty_fence);
+	amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask, guilty_ring, guilty_fence);
 
 	r = ip_block->version->funcs->soft_reset(ip_block);
 
-	r = amdgpu_multi_ring_reset_helper_end(ring_type_mask, guilty_ring, r);
+	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask, guilty_ring, r);
 	amdgpu_device_unlock_reset_domain(adev->reset_domain);
 
 	if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index 7b67e8cecca2..edae764eb0c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -919,8 +919,9 @@ int amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
 /**
  * amdgpu_multi_ring_reset_helper_begin() - Prepare multiple rings for a reset.
  *
+ * @adev: amdgpu device
  * @ring_type_mask: Bitmask of affected ring types
- * @guilty_ring: The ring which is guilty of causing a reset.
+ * @guilty_ring: The ring which is guilty of causing a reset, or NULL.
  * @guilty_fence: The fence which didn't signal on the guilty ring.
  *
  * Useful when performing a GPU reset method that affects
@@ -938,11 +939,11 @@ int amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
  * After the reset is complete, the caller should then call
  * amdgpu_multi_ring_reset_helper_end() to restore the rings.
  */
-void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
+void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
+					  const u32 ring_type_mask,
 					  struct amdgpu_ring *guilty_ring,
 					  struct amdgpu_fence *guilty_fence)
 {
-	struct amdgpu_device *adev = guilty_ring->adev;
 	struct amdgpu_fence *ring_guilty_fence;
 	struct amdgpu_ring *ring;
 	bool rings_busy;
@@ -1034,8 +1035,9 @@ void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
 /**
  * amdgpu_multi_ring_reset_helper_end() - Prepare multiple rings for a reset.
  *
+ * @adev: amdgpu device
  * @ring_type_mask: Bitmask of affected ring types
- * @guilty_ring: The ring which is guilty of causing a reset.
+ * @guilty_ring: The ring which is guilty of causing a reset, or NULL.
  * @ret: Return code from the reset function.
  *
  * After calling amdgpu_multi_ring_reset_helper_begin()
@@ -1046,10 +1048,10 @@ void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
  * be called to restore preemption state, but it won't attempt to
  * fully restore the ring contents.
  */
-int amdgpu_multi_ring_reset_helper_end(const u32 ring_type_mask,
+int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
+				       const u32 ring_type_mask,
 				       struct amdgpu_ring *guilty_ring, int ret)
 {
-	struct amdgpu_device *adev = guilty_ring->adev;
 	struct amdgpu_ring *ring;
 	int i, r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index b2040f80b7f2..7e6ed43771f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -637,10 +637,12 @@ void amdgpu_ring_reset_helper_begin(struct amdgpu_ring *ring,
 				    struct amdgpu_fence *guilty_fence);
 int amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
 				 struct amdgpu_fence *guilty_fence);
-void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
+void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
+					  const u32 ring_type_mask,
 					  struct amdgpu_ring *guilty_ring,
 					  struct amdgpu_fence *guilty_fence);
-int amdgpu_multi_ring_reset_helper_end(const u32 ring_type_mask,
+int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
+				       const u32 ring_type_mask,
 				       struct amdgpu_ring *guilty_ring, int ret);
 bool amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring,
 					 u32 reset_type);
-- 
2.50.1

[PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

[Jesse Zhang]

amdgpu_ring_backup_unprocessed_commands() skips the backup of a ring and
clears ring->guilty_fence when it sees the same guilty fence again, to
avoid replay loops. That happens when the IP block soft reset follows a
failed ring reset of the same job. amdgpu_multi_ring_reset_helper_end()
then restored the guilty ring with ring->guilty_fence, NULL by then, so
neither the guilty fence nor the rest of the ring was completed: the hung
job stayed pending until it timed out again.

Pass the guilty fence to the end helper, and use it for the guilty ring.
For the other rings, and when the backup didn't drop it, nothing changes.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   |  3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 13 ++++++++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 ++-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
index d0b0d3eaffc5..2eea18965fd1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
@@ -469,7 +469,8 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
 
 	r = ip_block->version->funcs->soft_reset(ip_block);
 
-	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask, guilty_ring, r);
+	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask, guilty_ring,
+					       guilty_fence, r);
 	amdgpu_device_unlock_reset_domain(adev->reset_domain);
 
 	if (r) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index edae764eb0c4..7f0f0ae6650a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -1038,6 +1038,7 @@ void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
  * @adev: amdgpu device
  * @ring_type_mask: Bitmask of affected ring types
  * @guilty_ring: The ring which is guilty of causing a reset, or NULL.
+ * @guilty_fence: The fence which didn't signal on the guilty ring.
  * @ret: Return code from the reset function.
  *
  * After calling amdgpu_multi_ring_reset_helper_begin()
@@ -1050,7 +1051,8 @@ void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
  */
 int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
 				       const u32 ring_type_mask,
-				       struct amdgpu_ring *guilty_ring, int ret)
+				       struct amdgpu_ring *guilty_ring,
+				       struct amdgpu_fence *guilty_fence, int ret)
 {
 	struct amdgpu_ring *ring;
 	int i, r;
@@ -1080,8 +1082,13 @@ int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
 		if (!(BIT(ring->funcs->type) & ring_type_mask))
 			continue;
 
-		/* Restore contents of the ring */
-		r = amdgpu_ring_reset_helper_end(ring, ring->guilty_fence);
+		/*
+		 * Restore contents of the ring. The backup of the guilty ring
+		 * may have dropped its guilty fence, eg. after a failed ring
+		 * reset with the same fence: still complete it.
+		 */
+		r = amdgpu_ring_reset_helper_end(ring, ring == guilty_ring ?
+						 guilty_fence : ring->guilty_fence);
 		if (r) {
 			dev_err(adev->dev,
 				"Failed to recover ring %s after soft reset\n",
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 7e6ed43771f2..349c7d503e5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -643,7 +643,8 @@ void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
 					  struct amdgpu_fence *guilty_fence);
 int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
 				       const u32 ring_type_mask,
-				       struct amdgpu_ring *guilty_ring, int ret);
+				       struct amdgpu_ring *guilty_ring,
+				       struct amdgpu_fence *guilty_fence, int ret);
 bool amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring,
 					 u32 reset_type);
 #endif
-- 
2.50.1

[PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP block soft reset

[Jesse Zhang]

Some IP blocks have engines that the hardware can reset on their own,
eg. the graphics and compute engines of the GFX IP
(CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET / CMP_ONLY_SOFT_RESET). Resetting
only the engine of a hung ring leaves the work on the other engines
alone, which the IP block soft reset can't do, as it resets every ring
of the IP block.

Add an optional soft_reset_engine() IP block callback, which resets one
engine of the IP block, or all of them, and recovers the user queues on
it, and amdgpu_device_ip_soft_reset_engine() around it. It only stops,
backs up and replays the rings of that engine, and it is also reachable
without a guilty kernel ring, for a hung user queue.

For a single engine, it only takes the reset domain for read: KFD only
try-locks it for read before it adds or removes queues, and would fail
those calls with the write lock although the other engines aren't
reset.

amdgpu_device_ip_soft_reset() uses it when the IP block implements it,
for the engine of the guilty ring.

v3: replaces the separate engine reset type and helper
    (Timur).

Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   | 116 +++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h   |   6 ++
 drivers/gpu/drm/amd/include/amd_shared.h |  18 ++++
 3 files changed, 140 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
index 2eea18965fd1..e95f69dd3142 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
@@ -418,6 +418,111 @@ static u32 amdgpu_ring_mask_from_ip(const enum amd_ip_block_type ip_type)
 	}
 }
 
+/**
+ * amdgpu_ring_mask_from_engine() - Find mask of ring types of an engine.
+ *
+ * @ip_type: The IP block type of the engine.
+ * @engine: The engine whose rings you are looking for.
+ */
+static u32 amdgpu_ring_mask_from_engine(const enum amd_ip_block_type ip_type,
+					const enum amd_ip_engine engine)
+{
+	switch (engine) {
+	case AMD_IP_ENGINE_GFX:
+		return BIT(AMDGPU_RING_TYPE_GFX);
+
+	case AMD_IP_ENGINE_COMPUTE:
+		return BIT(AMDGPU_RING_TYPE_COMPUTE);
+
+	default:
+		return amdgpu_ring_mask_from_ip(ip_type);
+	}
+}
+
+/**
+ * amdgpu_engine_from_ring() - Find the engine of a ring type.
+ *
+ * @ring_type: The ring type whose engine you are looking for.
+ */
+static enum amd_ip_engine amdgpu_engine_from_ring(const enum amdgpu_ring_type ring_type)
+{
+	switch (ring_type) {
+	case AMDGPU_RING_TYPE_GFX:
+		return AMD_IP_ENGINE_GFX;
+
+	case AMDGPU_RING_TYPE_COMPUTE:
+		return AMD_IP_ENGINE_COMPUTE;
+
+	default:
+		return AMD_IP_ENGINE_ALL;
+	}
+}
+
+static const char *const amdgpu_ip_engine_names[] = {
+	[AMD_IP_ENGINE_ALL] = "all",
+	[AMD_IP_ENGINE_GFX] = "gfx",
+	[AMD_IP_ENGINE_COMPUTE] = "compute",
+};
+
+/**
+ * amdgpu_device_ip_soft_reset_engine() - Soft reset one engine of an IP block.
+ *
+ * @adev: amdgpu device
+ * @ip_type: The IP block type.
+ * @engine: The engine to reset, or AMD_IP_ENGINE_ALL for the whole IP block.
+ * @guilty_ring: The ring which is guilty of causing a reset, or NULL.
+ * @guilty_fence: The fence which didn't signal on @guilty_ring.
+ * @guilty_queue: The user queue which is guilty of causing a reset, or NULL.
+ */
+int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device *adev,
+				       enum amd_ip_block_type ip_type,
+				       enum amd_ip_engine engine,
+				       struct amdgpu_ring *guilty_ring,
+				       struct amdgpu_fence *guilty_fence,
+				       struct amdgpu_usermode_queue *guilty_queue)
+{
+	struct amdgpu_ip_block *ip_block;
+	u32 ring_type_mask;
+	int r;
+
+	ip_block = amdgpu_device_ip_get_ip_block(adev, ip_type);
+	if (!ip_block || !ip_block->version->funcs->soft_reset_engine)
+		return -EOPNOTSUPP;
+
+	dev_err(adev->dev, "Starting %s %s engine soft reset\n",
+		ip_block->version->funcs->name, amdgpu_ip_engine_names[engine]);
+
+	ring_type_mask = amdgpu_ring_mask_from_engine(ip_type, engine);
+
+	if (engine == AMD_IP_ENGINE_ALL)
+		amdgpu_device_lock_reset_domain(adev->reset_domain);
+	else
+		down_read(&adev->reset_domain->sem);
+	amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask, guilty_ring,
+					     guilty_fence);
+
+	r = ip_block->version->funcs->soft_reset_engine(ip_block, engine,
+							guilty_queue);
+
+	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask, guilty_ring,
+					       guilty_fence, r);
+	if (engine == AMD_IP_ENGINE_ALL)
+		amdgpu_device_unlock_reset_domain(adev->reset_domain);
+	else
+		up_read(&adev->reset_domain->sem);
+
+	if (r) {
+		dev_err(adev->dev, "Failed %s %s engine soft reset: %d\n",
+			ip_block->version->funcs->name,
+			amdgpu_ip_engine_names[engine], r);
+		return r;
+	}
+
+	dev_err(adev->dev, "Successful %s %s engine soft reset\n",
+		ip_block->version->funcs->name, amdgpu_ip_engine_names[engine]);
+	return 0;
+}
+
 /**
  * amdgpu_device_ip_soft_reset() - Perform a graceful soft reset on an IP block.
  *
@@ -430,6 +535,9 @@ static u32 amdgpu_ring_mask_from_ip(const enum amd_ip_block_type ip_type)
  * resets all rings that belong to the same device IP block
  * and re-initializes the device IP block.
  *
+ * IP blocks with engines that can be reset separately only reset the engine
+ * of @guilty_ring, see amdgpu_device_ip_soft_reset_engine().
+ *
  * The reset is handled gracefully, meaning that we try to
  * minimize collateral damage (ie. avoid rejecting non-guilty jobs)
  * as well as back up and restore the contents of all rings
@@ -441,6 +549,7 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
 	struct amdgpu_device *adev = guilty_ring->adev;
 	struct amdgpu_ip_block *ip_block;
 	enum amd_ip_block_type ip_type;
+	enum amd_ip_engine engine;
 	u32 ring_type_mask;
 	int r;
 
@@ -453,6 +562,13 @@ int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
 		return -EOPNOTSUPP;
 	}
 
+	if (ip_block->version->funcs->soft_reset_engine) {
+		engine = amdgpu_engine_from_ring(guilty_ring->funcs->type);
+		return amdgpu_device_ip_soft_reset_engine(adev, ip_type, engine,
+							  guilty_ring,
+							  guilty_fence, NULL);
+	}
+
 	if (!ip_block->version->funcs->soft_reset) {
 		dev_warn(adev->dev, "IP block soft reset not supported on %s\n",
 			 ip_block->version->funcs->name);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
index 5dd6273982c4..856297d1c8e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
@@ -160,5 +160,11 @@ bool amdgpu_device_ip_is_valid(struct amdgpu_device *adev,
 			       enum amd_ip_block_type block_type);
 int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
 				struct amdgpu_fence *guilty_fence);
+int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device *adev,
+				       enum amd_ip_block_type ip_type,
+				       enum amd_ip_engine engine,
+				       struct amdgpu_ring *guilty_ring,
+				       struct amdgpu_fence *guilty_fence,
+				       struct amdgpu_usermode_queue *guilty_queue);
 
 #endif /* __AMDGPU_IP_H__ */
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h b/drivers/gpu/drm/amd/include/amd_shared.h
index e81ce9d79009..95cdd0fc191e 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -29,6 +29,7 @@
 
 #define AMD_MAX_USEC_TIMEOUT		1000000  /* 1000 ms */
 struct amdgpu_ip_block;
+struct amdgpu_usermode_queue;
 
 
 /*
@@ -422,6 +423,18 @@ enum DC_DEBUG_MASK {
 
 enum amd_dpm_forced_level;
 
+/**
+ * enum amd_ip_engine - the engines of an IP block that can be soft reset
+ * @AMD_IP_ENGINE_ALL: all the engines of the IP block
+ * @AMD_IP_ENGINE_GFX: the graphics engine of the GFX IP block
+ * @AMD_IP_ENGINE_COMPUTE: the compute engine of the GFX IP block
+ */
+enum amd_ip_engine {
+	AMD_IP_ENGINE_ALL,
+	AMD_IP_ENGINE_GFX,
+	AMD_IP_ENGINE_COMPUTE,
+};
+
 /**
  * struct amd_ip_funcs - general hooks for managing amdgpu IP Blocks
  * @name: Name of IP block
@@ -441,6 +454,8 @@ enum amd_dpm_forced_level;
  * @complete: handles IP specific changes after resume
  * @wait_for_idle: poll for idle
  * @soft_reset: soft reset the IP block
+ * @soft_reset_engine: soft reset one engine of the IP block, or all of them,
+ *                     and recover the user queues on it
  * @set_clockgating_state: enable/disable cg for the IP block
  * @set_powergating_state: enable/disable pg for the IP block
  * @get_clockgating_state: get current clockgating status
@@ -469,6 +484,9 @@ struct amd_ip_funcs {
 	void (*complete)(struct amdgpu_ip_block *ip_block);
 	int (*wait_for_idle)(struct amdgpu_ip_block *ip_block);
 	int (*soft_reset)(struct amdgpu_ip_block *ip_block);
+	int (*soft_reset_engine)(struct amdgpu_ip_block *ip_block,
+				 enum amd_ip_engine engine,
+				 struct amdgpu_usermode_queue *guilty_queue);
 	int (*set_clockgating_state)(struct amdgpu_ip_block *ip_block,
 				     enum amd_clockgating_state state);
 	int (*set_powergating_state)(struct amdgpu_ip_block *ip_block,
-- 
2.50.1

[PATCH v3 04/15] drm/amdgpu/gfx: add a reset_engine callback

[Jesse Zhang]

Some gfx hangs can't be recovered by a queue or a pipe reset, eg. a CP
front-end stuck in the middle of a packet. They currently end in a full
GPU reset, and VRAM is lost. Resetting only the graphics engine is
enough for them, and the compute engine isn't reset.

Add an optional reset_engine callback to amdgpu_gfx_funcs for the
hardware part of such a reset: it resets the CP of one engine of the GFX
IP and maps the kernel queues of that engine again. A later patch builds
the soft_reset_engine() IP block callback of the gfx IPs around it.

v3: Take enum amd_ip_engine, called by the soft_reset_engine() IP block
    callback (Timur).

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 5939e1fb3b24..8d7bd7057666 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -35,6 +35,7 @@
 #include "amdgpu_ras.h"
 #include "amdgpu_ring_mux.h"
 #include "amdgpu_xcp.h"
+#include "amd_shared.h"
 
 struct amdgpu_usermode_queue;
 
@@ -376,6 +377,9 @@ struct amdgpu_gfx_funcs {
 	/* find the gfx HQD slot (me/pipe/queue) for a doorbell */
 	bool (*detect_hung_queue)(struct amdgpu_device *adev, u32 doorbell_index,
 				  u32 *me, u32 *pipe, u32 *queue);
+	/* reset the CP of one engine and map its kernel queues again */
+	int (*reset_engine)(struct amdgpu_device *adev,
+			    enum amd_ip_engine engine);
 };
 
 struct sq_work {
-- 
2.50.1

[PATCH v3 05/15] drm/amdgpu/gfx12: add a gfx engine reset

[Jesse Zhang]

Some hangs leave the gfx streaming engine of the CP front-end stuck,
eg. waiting for packet data that never arrives. Neither a queue reset
nor a pipe reset can stop it, so the recovery currently ends in a full
GPU reset, and VRAM is lost.

Implement the reset_engine callback for the graphics engine:
- halt PFP/ME;
- 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. A CPF reset alone does not
  clear the stuck stream;
- restart PFP/ME and restore CP_PFP_IB_CONTROL;
- remap the kernel gfx queue through MES from its MQD backup and restart
  it with gfx_v12_0_cp_gfx_start().

MEC and the compute part of CPF/CPG aren't reset, so the compute queues
aren't reset either.

MES fetches its own queues through CPF, and a MES command in flight
during the reset doesn't complete: hold the MES lock across the reset.

A CP DMA request in flight during the reset leaves the ME busy after it,
and MES then fails its next commands. Wait for those requests before
the reset, and give up after the usual timeout: the DMA of a hung stream
may never complete. Wait for them after the reset too, but not for the
whole CP, as the halted PFP stays busy until it is restarted. If the CP
doesn't go idle once PFP/ME run again, fail the reset so that a full
reset follows.

The reset also clears the HQDs of the gfx user queues, the caller has to
handle them.

The callback relies on the caller to only use it when the IP reports
AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET for the gfx rings, which a later
patch enables with MES and async gfx rings.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 129 +++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index 184e009c8e50..defa5ca84fc2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -274,6 +274,8 @@ static void gfx_v12_0_set_imu_funcs(struct amdgpu_device *adev);
 static int gfx_v12_0_get_cu_info(struct amdgpu_device *adev,
 				 struct amdgpu_cu_info *cu_info);
 static uint64_t gfx_v12_0_get_gpu_clock_counter(struct amdgpu_device *adev);
+static int gfx_v12_0_reset_engine(struct amdgpu_device *adev,
+				  enum amd_ip_engine engine);
 static void gfx_v12_0_select_se_sh(struct amdgpu_device *adev, u32 se_num,
 				   u32 sh_num, u32 instance, int xcc_id);
 static u32 gfx_v12_0_get_wgp_active_bitmap_per_sh(struct amdgpu_device *adev);
@@ -964,6 +966,7 @@ static const struct amdgpu_gfx_funcs gfx_v12_0_gfx_funcs = {
 	.update_perfmon_mgcg = &gfx_v12_0_update_perf_clk,
 	.get_gfx_shadow_info = &gfx_v12_0_get_gfx_shadow_info,
 	.get_hdp_flush_mask = &amdgpu_gfx_get_hdp_flush_mask,
+	.reset_engine = &gfx_v12_0_reset_engine,
 };
 
 static int gfx_v12_0_gpu_early_init(struct amdgpu_device *adev)
@@ -3849,6 +3852,132 @@ static int gfx_v12_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
+/* The CP DMA and the interfaces its memory requests go through */
+#define GFX12_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_v12_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) & GFX12_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) & GFX12_CP_STAT_MEM_BUSY))
+			break;
+		udelay(1);
+	}
+	amdgpu_mes_unlock(&adev->mes);
+
+	gfx_v12_0_set_pfp_ucode_start_addr(adev);
+	gfx_v12_0_set_me_ucode_start_addr(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 aren't reset.
+ */
+static int gfx_v12_0_reset_engine(struct amdgpu_device *adev,
+				  enum amd_ip_engine engine)
+{
+	int i, r;
+
+	lockdep_assert_held(&adev->gfx.mec.reset_mutex);
+
+	if (engine != AMD_IP_ENGINE_GFX)
+		return -EOPNOTSUPP;
+
+	amdgpu_gfx_off_ctrl(adev, false);
+
+	r = gfx_v12_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_v12_0_cp_gfx_start(adev);
+
+exit:
+	amdgpu_gfx_off_ctrl(adev, true);
+	return r;
+}
+
 static uint64_t gfx_v12_0_get_gpu_clock_counter(struct amdgpu_device *adev)
 {
 	uint64_t clock = 0;
-- 
2.50.1

[PATCH v3 06/15] drm/amdgpu/gfx11: add a gfx engine reset

[Jesse Zhang]

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.

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..e533a5939d02 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 amd_ip_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 aren't reset.
+ */
+static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
+				  enum amd_ip_engine engine)
+{
+	int i, r;
+
+	lockdep_assert_held(&adev->gfx.mec.reset_mutex);
+
+	if (engine != AMD_IP_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)
-- 
2.50.1

[PATCH v3 07/15] drm/amdgpu/gfx: add callbacks to read and restore the ring state of a gfx queue

[Jesse Zhang]

A gfx engine reset clears the HQD registers of the gfx queues, including
the only copy of the ring position of the queues MES didn't preempt.
MES doesn't save them in the MQD either when the engine hangs.

Add struct amdgpu_gfx_hqd_ptrs for that ring state, and gfx callbacks
for the gfx IP, which knows its registers and the layout of its MQD:
- read_gfx_hqd_ptrs() reads the ring state of the gfx HQD of a doorbell;
- restore_gfx_mqd_ring() puts it in the MQD after the reset, for MES to
  map the queue again from it.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 8d7bd7057666..f750b11f4ad4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -346,6 +346,17 @@ struct amdgpu_gfx_shadow_info {
 	u32 eop_alignment;
 };
 
+/* Ring state of a gfx HQD that a gfx engine reset clears. */
+struct amdgpu_gfx_hqd_ptrs {
+	u32	doorbell_index;
+	u64	mqd_addr;	/* the MQD loaded in the HQD */
+	bool	on_hqd;
+	u64	wptr;
+	u32	offset;
+	u32	csmd_rptr;
+	bool	connected;	/* the queue the pipe executes */
+};
+
 struct amdgpu_gfx_funcs {
 	/* get the gpu clock counter */
 	uint64_t (*get_gpu_clock_counter)(struct amdgpu_device *adev);
@@ -380,6 +391,16 @@ struct amdgpu_gfx_funcs {
 	/* reset the CP of one engine and map its kernel queues again */
 	int (*reset_engine)(struct amdgpu_device *adev,
 			    enum amd_ip_engine engine);
+	/* read the ring state of the gfx HQD of a doorbell, if it has one */
+	bool (*read_gfx_hqd_ptrs)(struct amdgpu_device *adev, u32 doorbell_index,
+				  struct amdgpu_gfx_hqd_ptrs *ptrs);
+	/*
+	 * write the ring state read by read_gfx_hqd_ptrs() into a gfx MQD;
+	 * true if the work up to ptrs->wptr was dropped
+	 */
+	bool (*restore_gfx_mqd_ring)(struct amdgpu_device *adev, void *mqd,
+				     u32 ring_size,
+				     struct amdgpu_gfx_hqd_ptrs *ptrs);
 };
 
 struct sq_work {
-- 
2.50.1

[PATCH v3 08/15] drm/amdgpu/gfx12: read and restore the ring state of a gfx queue

[Jesse Zhang]

Implement the gfx callbacks to read and restore the ring state of a gfx
queue for gfx12:
- read_gfx_hqd_ptrs() reads the HQD offset, CSMD read pointer and write
  pointer of the gfx HQD of a doorbell, and whether the queue is the one
  connected to its pipe;
- restore_gfx_mqd_ring() puts them in the MQD after a reset, for MES to
  map the queue again where it was. A queue switched out of the pipe
  resumes from the preemption status the PFP saved in its MQD, also in
  the middle of an IB. The queue the pipe was executing has its state in
  the PFP, which the reset cleared: it restarts after the work submitted
  so far instead, and that work is dropped.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 85 ++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index defa5ca84fc2..708325e6bc7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -276,6 +276,12 @@ static int gfx_v12_0_get_cu_info(struct amdgpu_device *adev,
 static uint64_t gfx_v12_0_get_gpu_clock_counter(struct amdgpu_device *adev);
 static int gfx_v12_0_reset_engine(struct amdgpu_device *adev,
 				  enum amd_ip_engine engine);
+static bool gfx_v12_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
+					u32 doorbell_index,
+					struct amdgpu_gfx_hqd_ptrs *ptrs);
+static bool gfx_v12_0_restore_gfx_mqd_ring(struct amdgpu_device *adev, void *m,
+					   u32 ring_size,
+					   struct amdgpu_gfx_hqd_ptrs *ptrs);
 static void gfx_v12_0_select_se_sh(struct amdgpu_device *adev, u32 se_num,
 				   u32 sh_num, u32 instance, int xcc_id);
 static u32 gfx_v12_0_get_wgp_active_bitmap_per_sh(struct amdgpu_device *adev);
@@ -967,6 +973,8 @@ static const struct amdgpu_gfx_funcs gfx_v12_0_gfx_funcs = {
 	.get_gfx_shadow_info = &gfx_v12_0_get_gfx_shadow_info,
 	.get_hdp_flush_mask = &amdgpu_gfx_get_hdp_flush_mask,
 	.reset_engine = &gfx_v12_0_reset_engine,
+	.read_gfx_hqd_ptrs = &gfx_v12_0_read_gfx_hqd_ptrs,
+	.restore_gfx_mqd_ring = &gfx_v12_0_restore_gfx_mqd_ring,
 };
 
 static int gfx_v12_0_gpu_early_init(struct amdgpu_device *adev)
@@ -3978,6 +3986,83 @@ static int gfx_v12_0_reset_engine(struct amdgpu_device *adev,
 	return r;
 }
 
+#define GFX12_HPD_QUEUE_STATE_CONNECTED_MIN	3
+#define GFX12_HPD_QUEUE_STATE_CONNECTED_MAX	15
+
+static bool gfx_v12_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
+					u32 doorbell_index,
+					struct amdgpu_gfx_hqd_ptrs *ptrs)
+{
+	u32 me, pipe, queue, status, state;
+
+	memset(ptrs, 0, sizeof(*ptrs));
+	ptrs->doorbell_index = doorbell_index;
+	if (!gfx_v12_0_detect_hung_queue(adev, doorbell_index, &me, &pipe, &queue))
+		return true;
+
+	amdgpu_gfx_off_ctrl(adev, false);
+	mutex_lock(&adev->srbm_mutex);
+	soc24_grbm_select(adev, me, pipe, queue, 0);
+	ptrs->on_hqd = RREG32_SOC15(GC, 0, regCP_GFX_HQD_ACTIVE) &
+		       CP_GFX_HQD_ACTIVE__ACTIVE_MASK;
+	if (!ptrs->on_hqd)
+		goto out;
+	ptrs->mqd_addr = RREG32_SOC15(GC, 0, regCP_GFX_MQD_BASE_ADDR) |
+			 (u64)RREG32_SOC15(GC, 0, regCP_GFX_MQD_BASE_ADDR_HI) << 32;
+	ptrs->offset = RREG32_SOC15(GC, 0, regCP_GFX_HQD_OFFSET);
+	ptrs->csmd_rptr = RREG32_SOC15(GC, 0, regCP_GFX_HQD_CSMD_RPTR);
+	ptrs->wptr = RREG32_SOC15(GC, 0, regCP_GFX_HQD_WPTR) |
+		     (u64)RREG32_SOC15(GC, 0, regCP_GFX_HQD_WPTR_HI) << 32;
+	status = RREG32_SOC15(GC, 0, regCP_GFX_HPD_STATUS0);
+	state = REG_GET_FIELD(status, CP_GFX_HPD_STATUS0, QUEUE_STATE);
+	ptrs->connected =
+		state >= GFX12_HPD_QUEUE_STATE_CONNECTED_MIN &&
+		state <= GFX12_HPD_QUEUE_STATE_CONNECTED_MAX &&
+		REG_GET_FIELD(status, CP_GFX_HPD_STATUS0, MAPPED_QUEUE) == queue;
+out:
+	soc24_grbm_select(adev, 0, 0, 0, 0);
+	mutex_unlock(&adev->srbm_mutex);
+	amdgpu_gfx_off_ctrl(adev, true);
+
+	return true;
+}
+
+static bool gfx_v12_0_restore_gfx_mqd_ring(struct amdgpu_device *adev, void *m,
+					   u32 ring_size,
+					   struct amdgpu_gfx_hqd_ptrs *ptrs)
+{
+	struct v12_gfx_mqd *mqd = m;
+	u32 wptr = lower_32_bits(ptrs->wptr) & ((ring_size >> 2) - 1);
+	u32 *status;
+
+	/*
+	 * MQD dwords 32-41 (reserved_32..41) are the PFP preemption status,
+	 * mcbp_status_pfp: the offset and chained IB size and address of IB1
+	 * (32-35) and IB2 (36-39), rb_preempt (40: preempted in the ring) and
+	 * completion_status (41: 0 IB not started, 1 preempted inside an IB,
+	 * 2 IB completed, 3 IB resumed). The PFP saves it when the queue is
+	 * switched out of the pipe and resumes from it when the queue runs
+	 * again, at the HQD offset. That survives the reset for a queue that
+	 * was switched out. The queue the pipe executed has its state in the
+	 * PFP, which the reset cleared: restart it at the write pointer with
+	 * no IB to resume, and drop its work in flight.
+	 */
+	if (ptrs->connected) {
+		mqd->cp_gfx_hqd_offset = wptr;
+		mqd->cp_gfx_hqd_csmd_rptr = wptr;
+		for (status = &mqd->reserved_32; status <= &mqd->reserved_41; status++)
+			*status = 0;
+	} else {
+		mqd->cp_gfx_hqd_offset = ptrs->offset;
+		mqd->cp_gfx_hqd_csmd_rptr = ptrs->csmd_rptr;
+	}
+	mqd->cp_gfx_hqd_wptr = lower_32_bits(ptrs->wptr);
+	mqd->cp_gfx_hqd_wptr_hi = upper_32_bits(ptrs->wptr);
+	mqd->cp_gfx_hqd_active = 0;
+
+	return ptrs->connected;
+}
+
 static uint64_t gfx_v12_0_get_gpu_clock_counter(struct amdgpu_device *adev)
 {
 	uint64_t clock = 0;
-- 
2.50.1

[PATCH v3 09/15] drm/amdgpu/gfx11: read and restore the ring state of a gfx queue

[Jesse Zhang]

Implement the gfx callbacks to read and restore the ring state of a gfx
queue for gfx11:
- read_gfx_hqd_ptrs() reads the HQD offset, CSMD read pointer and write
  pointer of the gfx HQD of a doorbell, and whether the queue is the one
  connected to its pipe;
- restore_gfx_mqd_ring() puts them in the MQD after a reset, for MES to
  map the queue again where it was. A queue switched out of the pipe
  resumes from the preemption status the PFP saved in its MQD, also in
  the middle of an IB. The queue the pipe was executing has its state in
  the PFP, which the reset cleared: it restarts after the work submitted
  so far instead, and that work is dropped.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 85 ++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index e533a5939d02..a71dae5d0d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -365,6 +365,12 @@ 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 amd_ip_engine engine);
+static bool gfx_v11_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
+					u32 doorbell_index,
+					struct amdgpu_gfx_hqd_ptrs *ptrs);
+static bool gfx_v11_0_restore_gfx_mqd_ring(struct amdgpu_device *adev, void *m,
+					   u32 ring_size,
+					   struct amdgpu_gfx_hqd_ptrs *ptrs);
 static void gfx_v11_0_update_perf_clk(struct amdgpu_device *adev,
 				      bool enable);
 
@@ -1153,6 +1159,8 @@ static const struct amdgpu_gfx_funcs gfx_v11_0_gfx_funcs = {
 	.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,
+	.read_gfx_hqd_ptrs = &gfx_v11_0_read_gfx_hqd_ptrs,
+	.restore_gfx_mqd_ring = &gfx_v11_0_restore_gfx_mqd_ring,
 };
 
 static int gfx_v11_0_gpu_early_init(struct amdgpu_device *adev)
@@ -7092,6 +7100,83 @@ static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
 	return r;
 }
 
+#define GFX11_HPD_QUEUE_STATE_CONNECTED_MIN	3
+#define GFX11_HPD_QUEUE_STATE_CONNECTED_MAX	15
+
+static bool gfx_v11_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
+					u32 doorbell_index,
+					struct amdgpu_gfx_hqd_ptrs *ptrs)
+{
+	u32 me, pipe, queue, status, state;
+
+	memset(ptrs, 0, sizeof(*ptrs));
+	ptrs->doorbell_index = doorbell_index;
+	if (!gfx_v11_0_detect_hung_queue(adev, doorbell_index, &me, &pipe, &queue))
+		return true;
+
+	amdgpu_gfx_off_ctrl(adev, false);
+	mutex_lock(&adev->srbm_mutex);
+	soc21_grbm_select(adev, me, pipe, queue, 0);
+	ptrs->on_hqd = RREG32_SOC15(GC, 0, regCP_GFX_HQD_ACTIVE) &
+		       CP_GFX_HQD_ACTIVE__ACTIVE_MASK;
+	if (!ptrs->on_hqd)
+		goto out;
+	ptrs->mqd_addr = RREG32_SOC15(GC, 0, regCP_GFX_MQD_BASE_ADDR) |
+			 (u64)RREG32_SOC15(GC, 0, regCP_GFX_MQD_BASE_ADDR_HI) << 32;
+	ptrs->offset = RREG32_SOC15(GC, 0, regCP_GFX_HQD_OFFSET);
+	ptrs->csmd_rptr = RREG32_SOC15(GC, 0, regCP_GFX_HQD_CSMD_RPTR);
+	ptrs->wptr = RREG32_SOC15(GC, 0, regCP_GFX_HQD_WPTR) |
+		     (u64)RREG32_SOC15(GC, 0, regCP_GFX_HQD_WPTR_HI) << 32;
+	status = RREG32_SOC15(GC, 0, regCP_GFX_HPD_STATUS0);
+	state = REG_GET_FIELD(status, CP_GFX_HPD_STATUS0, QUEUE_STATE);
+	ptrs->connected =
+		state >= GFX11_HPD_QUEUE_STATE_CONNECTED_MIN &&
+		state <= GFX11_HPD_QUEUE_STATE_CONNECTED_MAX &&
+		REG_GET_FIELD(status, CP_GFX_HPD_STATUS0, MAPPED_QUEUE) == queue;
+out:
+	soc21_grbm_select(adev, 0, 0, 0, 0);
+	mutex_unlock(&adev->srbm_mutex);
+	amdgpu_gfx_off_ctrl(adev, true);
+
+	return true;
+}
+
+static bool gfx_v11_0_restore_gfx_mqd_ring(struct amdgpu_device *adev, void *m,
+					   u32 ring_size,
+					   struct amdgpu_gfx_hqd_ptrs *ptrs)
+{
+	struct v11_gfx_mqd *mqd = m;
+	u32 wptr = lower_32_bits(ptrs->wptr) & ((ring_size >> 2) - 1);
+	u32 *status;
+
+	/*
+	 * MQD dwords 32-41 (reserved_32..41) are the PFP preemption status,
+	 * mcbp_status_pfp: the offset and chained IB size and address of IB1
+	 * (32-35) and IB2 (36-39), rb_preempt (40: preempted in the ring) and
+	 * completion_status (41: 0 IB not started, 1 preempted inside an IB,
+	 * 2 IB completed, 3 IB resumed). The PFP saves it when the queue is
+	 * switched out of the pipe and resumes from it when the queue runs
+	 * again, at the HQD offset. That survives the reset for a queue that
+	 * was switched out. The queue the pipe executed has its state in the
+	 * PFP, which the reset cleared: restart it at the write pointer with
+	 * no IB to resume, and drop its work in flight.
+	 */
+	if (ptrs->connected) {
+		mqd->cp_gfx_hqd_offset = wptr;
+		mqd->cp_gfx_hqd_csmd_rptr = wptr;
+		for (status = &mqd->reserved_32; status <= &mqd->reserved_41; status++)
+			*status = 0;
+	} else {
+		mqd->cp_gfx_hqd_offset = ptrs->offset;
+		mqd->cp_gfx_hqd_csmd_rptr = ptrs->csmd_rptr;
+	}
+	mqd->cp_gfx_hqd_wptr = lower_32_bits(ptrs->wptr);
+	mqd->cp_gfx_hqd_wptr_hi = upper_32_bits(ptrs->wptr);
+	mqd->cp_gfx_hqd_active = 0;
+
+	return ptrs->connected;
+}
+
 static int gfx_v11_0_reset_kgq(struct amdgpu_ring *ring,
 			       unsigned int vmid,
 			       struct amdgpu_fence *timedout_fence)
-- 
2.50.1

[PATCH v3 10/15] drm/amdgpu/userq: add a helper to signal the fences of a queue up to a point

[Jesse Zhang]

The fence value of a user queue is the write pointer of the queue when
the work was submitted. amdgpu_userq_fence_driver_force_completion()
signals all the fences of a queue, as after a reset that dropped all its
work.

Add amdgpu_userq_fence_driver_force_completion_with_seqno(), which only
signals the fences of the work up to a write pointer, with an error, for
a reset that drops only part of the work of a queue: the work submitted
after that write pointer still runs.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c   | 31 +++++++++++++++++++
 .../gpu/drm/amd/amdgpu/amdgpu_userq_fence.h   |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index a2fa59a9609e..e76249c43b7a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -459,6 +459,37 @@ amdgpu_userq_fence_driver_force_completion(struct amdgpu_usermode_queue *userq)
 	}
 }
 
+/*
+ * Signal the fences of @userq with a seqno up to @seqno with -ECANCELED, eg.
+ * when a reset dropped the work that would have signaled them.
+ */
+void
+amdgpu_userq_fence_driver_force_completion_with_seqno(struct amdgpu_usermode_queue *userq,
+						      u64 seqno)
+{
+	struct amdgpu_userq_fence_driver *fence_drv = userq->fence_drv;
+	struct amdgpu_userq_fence *fence;
+	unsigned long flags;
+
+	if (!fence_drv)
+		return;
+
+	spin_lock_irqsave(&fence_drv->fence_list_lock, flags);
+	list_for_each_entry(fence, &fence_drv->fences, link) {
+		if (fence->base.seqno > seqno)
+			break;
+		spin_lock(dma_fence_spinlock(&fence->base));
+		if (!dma_fence_is_signaled_locked(&fence->base))
+			dma_fence_set_error(&fence->base, -ECANCELED);
+		spin_unlock(dma_fence_spinlock(&fence->base));
+	}
+	if (amdgpu_userq_fence_read(fence_drv) < seqno)
+		amdgpu_userq_fence_write(fence_drv, seqno);
+	spin_unlock_irqrestore(&fence_drv->fence_list_lock, flags);
+
+	amdgpu_userq_fence_driver_process(fence_drv);
+}
+
 int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *filp)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
index 0bd51616cef1..0407c8c2575f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.h
@@ -65,6 +65,8 @@ int amdgpu_userq_fence_driver_alloc(struct amdgpu_device *adev,
 void amdgpu_userq_fence_driver_free(struct amdgpu_usermode_queue *userq);
 int amdgpu_userq_fence_driver_process(struct amdgpu_userq_fence_driver *fence_drv);
 void amdgpu_userq_fence_driver_force_completion(struct amdgpu_usermode_queue *userq);
+void amdgpu_userq_fence_driver_force_completion_with_seqno(struct amdgpu_usermode_queue *userq,
+							   u64 seqno);
 void amdgpu_userq_fence_driver_destroy(struct kref *ref);
 int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
 			      struct drm_file *filp);
-- 
2.50.1

[PATCH v3 11/15] drm/amdgpu/userq: add helpers to keep the user queues of one engine across a reset

[Jesse Zhang]

An engine reset, eg. of the gfx engine, only resets the queues of one
engine of the GFX IP; the queues of the other engines aren't reset.
amdgpu_userq_pre_reset() and amdgpu_userq_post_reset() handle all the
user queues for a full reset. Add separate helpers for the queues of one
engine, which keep them in the state they had:

- amdgpu_userq_pre_engine_reset(), called after the MES suspend, saves
  the ring state of the mapped queues still on a HQD: the reset clears
  their HQD, and MES couldn't preempt them into their MQD. The queues
  MES preempted have their state in their MQD already.
- amdgpu_userq_post_engine_reset(), called after the reset but before
  MES resumes, unmaps those queues, puts the saved ring state in their
  MQD and maps them again: MES doesn't map a queue whose HQD the reset
  cleared again on its own. The queues keep their state and their work,
  except the one the pipe executed: its work in flight depends on pipe
  state the reset cleared, it is dropped and its fences signaled. The
  queue that hung, if any, is unmapped and hung.
- amdgpu_userq_cancel_engine_reset() drops the saved ring state for the
  full reset that follows a failed engine reset.

The helpers walk the queues and unmap and map them as
amdgpu_userq_pre_reset() and amdgpu_userq_post_reset() do. The saved
ring state is kept in the gfx IP, per HQD, and matched to a queue by its
doorbell and MQD address.

v2: Drop the user queue reset lock and the walk over the queues of one
    type (Christian).

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 108 ++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |   5 +
 3 files changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index f750b11f4ad4..6bbf53359a49 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -518,6 +518,9 @@ struct amdgpu_gfx {
 
 	/* reset mask */
 	uint32_t 			gfx_supported_reset;
+	/* ring state of the gfx user queues on a HQD, saved by an engine reset */
+	struct amdgpu_gfx_hqd_ptrs	engine_reset_hqds[AMDGPU_MAX_GFX_QUEUES];
+	u32				num_engine_reset_hqds;
 	uint32_t 			compute_supported_reset;
 
 	/* gfx off */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
index 1a2aa85c504e..353c5f7cfcdd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
@@ -1597,3 +1597,111 @@ int amdgpu_userq_post_reset(struct amdgpu_device *adev, bool vram_lost)
 
 	return ret;
 }
+
+/**
+ * amdgpu_userq_pre_engine_reset - save the ring state of the user queues
+ * @adev: amdgpu device
+ * @queue_type: AMDGPU_HW_IP_* type of the queues on the reset engine
+ */
+void amdgpu_userq_pre_engine_reset(struct amdgpu_device *adev, u32 queue_type)
+{
+	struct amdgpu_gfx_hqd_ptrs ptrs;
+	struct amdgpu_usermode_queue *queue;
+	unsigned long queue_id;
+
+	adev->gfx.num_engine_reset_hqds = 0;
+	if (!adev->gfx.funcs->read_gfx_hqd_ptrs)
+		return;
+
+	xa_for_each(&adev->userq_doorbell_xa, queue_id, queue) {
+		if (adev->gfx.num_engine_reset_hqds >= AMDGPU_MAX_GFX_QUEUES)
+			break;
+		if (queue->queue_type != queue_type ||
+		    queue->state != AMDGPU_USERQ_STATE_MAPPED)
+			continue;
+		if (adev->gfx.funcs->read_gfx_hqd_ptrs(adev, queue->doorbell_index,
+						       &ptrs) && ptrs.on_hqd)
+			adev->gfx.engine_reset_hqds[adev->gfx.num_engine_reset_hqds++] = ptrs;
+	}
+}
+
+static struct amdgpu_gfx_hqd_ptrs *
+amdgpu_userq_engine_reset_hqd(struct amdgpu_device *adev,
+			      struct amdgpu_usermode_queue *queue)
+{
+	struct amdgpu_gfx_hqd_ptrs *ptrs;
+	u32 i;
+
+	for (i = 0; i < adev->gfx.num_engine_reset_hqds; i++) {
+		ptrs = &adev->gfx.engine_reset_hqds[i];
+		/* Check the MQD too: a new queue may reuse the doorbell. */
+		if (ptrs->doorbell_index == queue->doorbell_index &&
+		    ptrs->mqd_addr == queue->mqd.gpu_addr)
+			return ptrs;
+	}
+	return NULL;
+}
+
+/**
+ * amdgpu_userq_post_engine_reset - restore the user queues after an engine reset
+ * @adev: amdgpu device
+ * @queue_type: AMDGPU_HW_IP_* type of the queues on the reset engine
+ * @guilty: the queue that hung, or NULL
+ *
+ * MES doesn't map a queue whose HQD the reset cleared again on its own: unmap
+ * it from MES, restore its MQD, and map it again.
+ */
+void amdgpu_userq_post_engine_reset(struct amdgpu_device *adev, u32 queue_type,
+				    struct amdgpu_usermode_queue *guilty)
+{
+	const struct amdgpu_userq_funcs *userq_funcs = adev->userq_funcs[queue_type];
+	struct amdgpu_usermode_queue *queue;
+	struct amdgpu_gfx_hqd_ptrs *ptrs;
+	unsigned long queue_id;
+	int r;
+
+	xa_for_each(&adev->userq_doorbell_xa, queue_id, queue) {
+		if (queue->queue_type != queue_type ||
+		    queue->state != AMDGPU_USERQ_STATE_MAPPED)
+			continue;
+
+		if (queue == guilty) {
+			userq_funcs->unmap(queue);
+			amdgpu_userq_fence_driver_force_completion(queue);
+			trace_amdgpu_userq_state_changed(queue, AMDGPU_USERQ_STATE_HUNG);
+			queue->state = AMDGPU_USERQ_STATE_HUNG;
+			continue;
+		}
+
+		ptrs = amdgpu_userq_engine_reset_hqd(adev, queue);
+		if (!ptrs)
+			continue;
+
+		userq_funcs->unmap(queue);
+		if (adev->gfx.funcs->restore_gfx_mqd_ring(adev, queue->mqd.cpu_ptr,
+							  queue->userq_prop->queue_size,
+							  ptrs))
+			amdgpu_userq_fence_driver_force_completion_with_seqno(queue,
+									      ptrs->wptr);
+		r = userq_funcs->map(queue);
+		if (r) {
+			dev_err(adev->dev, "Failed to remap user queue %llu: %d\n",
+				queue->doorbell_index, r);
+			amdgpu_userq_fence_driver_force_completion(queue);
+			trace_amdgpu_userq_state_changed(queue, AMDGPU_USERQ_STATE_HUNG);
+			queue->state = AMDGPU_USERQ_STATE_HUNG;
+		}
+	}
+	adev->gfx.num_engine_reset_hqds = 0;
+}
+
+/**
+ * amdgpu_userq_cancel_engine_reset - give up an engine reset
+ * @adev: amdgpu device
+ * @queue_type: AMDGPU_HW_IP_* type of the queues on the reset engine
+ */
+void amdgpu_userq_cancel_engine_reset(struct amdgpu_device *adev,
+				      u32 queue_type)
+{
+	adev->gfx.num_engine_reset_hqds = 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
index 488dc21d7c81..e07c71cd5af5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
@@ -181,6 +181,11 @@ int amdgpu_userq_start_sched_for_enforce_isolation(struct amdgpu_device *adev,
 void amdgpu_userq_reset_work(struct work_struct *work);
 void amdgpu_userq_pre_reset(struct amdgpu_device *adev);
 int amdgpu_userq_post_reset(struct amdgpu_device *adev, bool vram_lost);
+void amdgpu_userq_pre_engine_reset(struct amdgpu_device *adev, u32 queue_type);
+void amdgpu_userq_post_engine_reset(struct amdgpu_device *adev, u32 queue_type,
+				    struct amdgpu_usermode_queue *guilty);
+void amdgpu_userq_cancel_engine_reset(struct amdgpu_device *adev,
+				      u32 queue_type);
 void amdgpu_userq_start_hang_detect_work(struct amdgpu_usermode_queue *queue);
 void amdgpu_userq_process_fence_irq(struct amdgpu_device *adev, u32 doorbell);
 
-- 
2.50.1

[PATCH v3 12/15] drm/amdgpu/gfx: add a gfx engine soft reset

[Jesse Zhang]

Add amdgpu_gfx_soft_reset_engine(), the soft_reset_engine() IP block
callback of the gfx IPs, around the reset_engine callback. It only
supports the graphics engine so far. The IP block soft reset stops,
backs up and replays the kernel gfx rings around it; the compute rings
aren't stopped.

MES is suspended across the reset, so that it doesn't schedule or map
queues while the CP front-end is reset. The compute queues are preempted
meanwhile: their work is only paused. MES
fetches its commands through the CP front-end though, and a hung gfx
stream can keep it from completing the suspend. Reset the engine anyway:
the reset frees the front-end.

The reset also clears the HQDs of the gfx user queues. Call
amdgpu_userq_pre_engine_reset() after the MES suspend, to save the ring
state of the queues still on a HQD, and amdgpu_userq_post_engine_reset()
after the reset, before MES resumes, to restore them. @guilty_queue is
the gfx user queue that hung, if any: it is hung instead. If the engine
reset fails, amdgpu_userq_cancel_engine_reset() drops the saved ring
state for the full reset that follows.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 41 +++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  3 ++
 2 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index 0ae6e990c310..f1a26e4eb06c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -2181,6 +2181,47 @@ int amdgpu_gfx_reset_mes_gfx(struct amdgpu_device *adev,
 	return r;
 }
 
+/**
+ * amdgpu_gfx_soft_reset_engine() - Soft reset one engine of the GFX IP.
+ *
+ * @ip_block: The GFX IP block.
+ * @engine: The engine to reset.
+ * @guilty_queue: The user queue that hung, or NULL.
+ *
+ * soft_reset_engine() IP block callback around the reset_engine callback,
+ * which resets the CP of the engine. Only the graphics engine is supported
+ * so far.
+ */
+int amdgpu_gfx_soft_reset_engine(struct amdgpu_ip_block *ip_block,
+				 enum amd_ip_engine engine,
+				 struct amdgpu_usermode_queue *guilty_queue)
+{
+	struct amdgpu_device *adev = ip_block->adev;
+	int r;
+
+	if (engine != AMD_IP_ENGINE_GFX || !adev->gfx.funcs->reset_engine)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&adev->gfx.mec.reset_mutex);
+
+	/* The suspend may time out while the gfx engine hangs: reset anyway. */
+	amdgpu_mes_suspend(adev, 0);
+	amdgpu_userq_pre_engine_reset(adev, AMDGPU_HW_IP_GFX);
+	r = adev->gfx.funcs->reset_engine(adev, engine);
+	if (r) {
+		amdgpu_userq_cancel_engine_reset(adev, AMDGPU_HW_IP_GFX);
+		amdgpu_mes_resume(adev, 0);
+		goto exit;
+	}
+
+	amdgpu_userq_post_engine_reset(adev, AMDGPU_HW_IP_GFX, guilty_queue);
+	r = amdgpu_mes_resume(adev, 0);
+
+exit:
+	mutex_unlock(&adev->gfx.mec.reset_mutex);
+	return r;
+}
+
 int amdgpu_gfx_mes_reset_queue(struct amdgpu_ring *ring,
 			       unsigned int vmid,
 			       struct amdgpu_fence *timedout_fence,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 6bbf53359a49..d63d9f5f5b96 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -740,6 +740,9 @@ void amdgpu_gfx_pipe_reset_prepare(struct amdgpu_device *adev,
 				   struct amdgpu_ring *guilty_ring,
 				   u32 me, u32 pipe,
 				   struct amdgpu_gfx_pipe_reset_ctx *ctx);
+int amdgpu_gfx_soft_reset_engine(struct amdgpu_ip_block *ip_block,
+				 enum amd_ip_engine engine,
+				 struct amdgpu_usermode_queue *guilty_queue);
 int amdgpu_gfx_reset_mes_gfx(struct amdgpu_device *adev,
 			     struct amdgpu_ring *ring,
 			     struct amdgpu_fence *guilty_fence,
-- 
2.50.1

[PATCH v3 13/15] drm/amdgpu/userq: reset the gfx engine when a gfx user queue reset fails

[Jesse Zhang]

When neither the queue nor the pipe reset recovered a gfx user queue,
fall back to a soft reset of the gfx engine before a full reset, when
the GFX IP supports it. A hung kernel gfx queue gets there through the
IP block soft reset of the job timeout handler; a user queue has no
kernel ring, so call amdgpu_device_ip_soft_reset_engine() directly.

The hung queue can't be unmapped before the reset: pass it to the engine
reset, which unmaps it once the reset cleared its HQD, and hangs it,
while the other gfx user queues keep their state.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/mes_userqueue.c | 29 +++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
index 7abb9d95ef59..5589be68d600 100644
--- a/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
+++ b/drivers/gpu/drm/amd/amdgpu/mes_userqueue.c
@@ -235,11 +235,27 @@ int mes_userq_reset_hw(struct amdgpu_usermode_queue *queue,
 	return 0;
 }
 
+/* The hung queue can't be unmapped now: the engine reset unmaps it after. */
+static int mes_userq_reset_gfx_engine(struct amdgpu_usermode_queue *queue)
+{
+	return amdgpu_device_ip_soft_reset_engine(queue->userq_mgr->adev,
+						  AMD_IP_BLOCK_TYPE_GFX,
+						  AMD_IP_ENGINE_GFX, NULL, NULL,
+						  queue);
+}
+
+static bool mes_userq_gfx_engine_reset_supported(struct amdgpu_device *adev)
+{
+	return adev->gfx.gfx_supported_reset &
+	       AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
+}
+
 int mes_userq_reset(struct amdgpu_usermode_queue *queue)
 {
 	struct amdgpu_userq_mgr *uq_mgr = queue->userq_mgr;
 	struct amdgpu_device *adev = uq_mgr->adev;
 	struct mes_reset_queue_input queue_input;
+	int r;
 
 	/* already reset by an earlier job's hang-detect; just signal and bail */
 	if (queue->state == AMDGPU_USERQ_STATE_HUNG)
@@ -248,8 +264,10 @@ int mes_userq_reset(struct amdgpu_usermode_queue *queue)
 	/* A GFX pipe reset also affects the kernel rings on that pipe. */
 	if (queue->queue_type == AMDGPU_HW_IP_GFX &&
 	    adev->gfx.funcs->detect_hung_queue &&
-	    amdgpu_gfx_me_pipe_reset_supported(adev))
-		return amdgpu_gfx_reset_mes_gfx(adev, NULL, NULL, queue, 0, true);
+	    amdgpu_gfx_me_pipe_reset_supported(adev)) {
+		r = amdgpu_gfx_reset_mes_gfx(adev, NULL, NULL, queue, 0, true);
+		goto out;
+	}
 
 	memset(&queue_input, 0x0, sizeof(struct mes_reset_queue_input));
 	queue_input.doorbell_offset = queue->doorbell_index;
@@ -277,7 +295,12 @@ int mes_userq_reset(struct amdgpu_usermode_queue *queue)
 		}
 	}
 
-	return mes_userq_reset_hw(queue, &queue_input);
+	r = mes_userq_reset_hw(queue, &queue_input);
+out:
+	if (r && queue->queue_type == AMDGPU_HW_IP_GFX &&
+	    mes_userq_gfx_engine_reset_supported(adev))
+		r = mes_userq_reset_gfx_engine(queue);
+	return r;
 }
 
 int mes_userq_reset_queue(struct amdgpu_device *adev,
-- 
2.50.1

[PATCH v3 14/15] drm/amdgpu/gfx12: enable the gfx engine soft reset

[Jesse Zhang]

Report AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET for the gfx rings, with
amdgpu_gfx_soft_reset_engine() as the soft_reset_engine() callback, when
the reset_engine callback can be used: with MES and async gfx rings. The
reset remaps the kernel gfx queue through MES from its MQD, and without
async gfx rings the kernel gfx queue has no MQD. It is programmed
through the CP_RB0 registers, which the reset clears. Not on SR-IOV VFs.

The kernel gfx queue and gfx user queue resets then fall back to a soft
reset of the gfx engine before a full reset.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
index 708325e6bc7b..d1fe1ee71bcc 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v12_0.c
@@ -274,6 +274,7 @@ static void gfx_v12_0_set_imu_funcs(struct amdgpu_device *adev);
 static int gfx_v12_0_get_cu_info(struct amdgpu_device *adev,
 				 struct amdgpu_cu_info *cu_info);
 static uint64_t gfx_v12_0_get_gpu_clock_counter(struct amdgpu_device *adev);
+static bool gfx_v12_0_engine_reset_supported(struct amdgpu_device *adev);
 static int gfx_v12_0_reset_engine(struct amdgpu_device *adev,
 				  enum amd_ip_engine engine);
 static bool gfx_v12_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
@@ -1599,6 +1600,10 @@ static int gfx_v12_0_sw_init(struct amdgpu_ip_block *ip_block)
 		break;
 	}
 
+	if (gfx_v12_0_engine_reset_supported(adev) &&
+	    !adev->debug_disable_ip_block_soft_reset)
+		adev->gfx.gfx_supported_reset |= AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
+
 	if (!adev->enable_mes_kiq) {
 		r = amdgpu_gfx_kiq_init(adev, GFX12_MEC_HPD_SIZE, 0);
 		if (r) {
@@ -3860,6 +3865,12 @@ static int gfx_v12_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
+/* The reset remaps the kernel gfx queue from its MQD: needs async rings. */
+static bool gfx_v12_0_engine_reset_supported(struct amdgpu_device *adev)
+{
+	return !amdgpu_sriov_vf(adev) && adev->enable_mes && amdgpu_async_gfx_ring;
+}
+
 /* The CP DMA and the interfaces its memory requests go through */
 #define GFX12_CP_STAT_MEM_BUSY	(CP_STAT__DMA_BUSY_MASK | \
 				 CP_STAT__TCIU_BUSY_MASK | \
@@ -5500,6 +5511,7 @@ static const struct amd_ip_funcs gfx_v12_0_ip_funcs = {
 	.suspend = gfx_v12_0_suspend,
 	.resume = gfx_v12_0_resume,
 	.wait_for_idle = gfx_v12_0_wait_for_idle,
+	.soft_reset_engine = amdgpu_gfx_soft_reset_engine,
 	.set_clockgating_state = gfx_v12_0_set_clockgating_state,
 	.set_powergating_state = gfx_v12_0_set_powergating_state,
 	.get_clockgating_state = gfx_v12_0_get_clockgating_state,
-- 
2.50.1

[PATCH v3 15/15] drm/amdgpu/gfx11: enable the gfx engine soft reset

[Jesse Zhang]

Report AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET for the gfx rings, with
amdgpu_gfx_soft_reset_engine() as the soft_reset_engine() callback, when
the reset_engine callback can be used: with MES, async gfx rings and the
RS64 firmware. The reset restarts PFP and ME from the start of their
RS64 firmware, and remaps the kernel gfx queue through MES from its
MQD, which the kernel gfx queue only has with async gfx rings. Not on
SR-IOV VFs.

The kernel gfx queue and gfx user queue resets then fall back to a soft
reset of the gfx engine before a full reset. The IP block soft reset of
gfx11 only resets the gfx engine then; the whole IP block soft_reset()
isn't used.

Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
index a71dae5d0d3a..41ebd2ead99b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
@@ -363,6 +363,7 @@ 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 bool gfx_v11_0_engine_reset_supported(struct amdgpu_device *adev);
 static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
 				  enum amd_ip_engine engine);
 static bool gfx_v11_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
@@ -1945,6 +1946,10 @@ static int gfx_v11_0_sw_init(struct amdgpu_ip_block *ip_block)
 		break;
 	}
 
+	if (gfx_v11_0_engine_reset_supported(adev) &&
+	    !adev->debug_disable_ip_block_soft_reset)
+		adev->gfx.gfx_supported_reset |= AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
+
 	if (!adev->enable_mes_kiq) {
 		r = amdgpu_gfx_kiq_init(adev, GFX11_MEC_HPD_SIZE, 0);
 		if (r) {
@@ -6927,6 +6932,16 @@ static void gfx_v11_0_emit_mem_sync(struct amdgpu_ring *ring)
 	amdgpu_ring_write(ring, gcr_cntl); /* GCR_CNTL */
 }
 
+/*
+ * The reset restarts PFP/ME through their RS64 start addresses, and remaps the
+ * kernel gfx queue from its MQD, which needs async rings.
+ */
+static bool gfx_v11_0_engine_reset_supported(struct amdgpu_device *adev)
+{
+	return !amdgpu_sriov_vf(adev) && adev->enable_mes &&
+	       amdgpu_async_gfx_ring && adev->gfx.rs64_enable;
+}
+
 /* 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)
 {
@@ -7370,6 +7385,7 @@ static const struct amd_ip_funcs gfx_v11_0_ip_funcs = {
 	.resume = gfx_v11_0_resume,
 	.wait_for_idle = gfx_v11_0_wait_for_idle,
 	.soft_reset = gfx_v11_0_soft_reset,
+	.soft_reset_engine = amdgpu_gfx_soft_reset_engine,
 	.set_clockgating_state = gfx_v11_0_set_clockgating_state,
 	.set_powergating_state = gfx_v11_0_set_powergating_state,
 	.get_clockgating_state = gfx_v11_0_get_clockgating_state,
-- 
2.50.1

RE: [PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring reset helpers

[Zhang, Jesse(Jie)]

AMD General

Ping on the series ?

> -----Original Message-----
> From: Jesse Zhang <Jesse.Zhang@amd.com>
> Sent: Wednesday, October 28, 2026 4:57 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>;
> timur.kristof@gmail.com; Mario Sopena-Novales
> <Mario.Novales@amd.com>; Rastogi, Manu <Manu.Rastogi@amd.com>;
> Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>; Zhang, Jesse(Jie)
> <Jesse.Zhang@amd.com>
> Subject: [PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring
> reset helpers
>
> amdgpu_multi_ring_reset_helper_begin() and _end() take the device from the
> guilty ring. A reset triggered by a hung user queue has no guilty kernel ring, so
> pass the device explicitly and allow a NULL guilty ring.
>
> No functional change.
>
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 14 ++++++++------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  6 ++++--
>  3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> index 734f2a0bc79a..d0b0d3eaffc5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> @@ -465,11 +465,11 @@ int amdgpu_device_ip_soft_reset(struct
> amdgpu_ring *guilty_ring,
>       ring_type_mask = amdgpu_ring_mask_from_ip(ip_type);
>
>       amdgpu_device_lock_reset_domain(adev->reset_domain);
> -     amdgpu_multi_ring_reset_helper_begin(ring_type_mask, guilty_ring,
> guilty_fence);
> +     amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask,
> +guilty_ring, guilty_fence);
>
>       r = ip_block->version->funcs->soft_reset(ip_block);
>
> -     r = amdgpu_multi_ring_reset_helper_end(ring_type_mask,
> guilty_ring, r);
> +     r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> +guilty_ring, r);
>       amdgpu_device_unlock_reset_domain(adev->reset_domain);
>
>       if (r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index 7b67e8cecca2..edae764eb0c4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -919,8 +919,9 @@ int amdgpu_ring_reset_helper_end(struct
> amdgpu_ring *ring,
>  /**
>   * amdgpu_multi_ring_reset_helper_begin() - Prepare multiple rings for a
> reset.
>   *
> + * @adev: amdgpu device
>   * @ring_type_mask: Bitmask of affected ring types
> - * @guilty_ring: The ring which is guilty of causing a reset.
> + * @guilty_ring: The ring which is guilty of causing a reset, or NULL.
>   * @guilty_fence: The fence which didn't signal on the guilty ring.
>   *
>   * Useful when performing a GPU reset method that affects @@ -938,11
> +939,11 @@ int amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
>   * After the reset is complete, the caller should then call
>   * amdgpu_multi_ring_reset_helper_end() to restore the rings.
>   */
> -void amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
> +void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
> +                                       const u32 ring_type_mask,
>                                         struct amdgpu_ring *guilty_ring,
>                                         struct amdgpu_fence *guilty_fence)
> {
> -     struct amdgpu_device *adev = guilty_ring->adev;
>       struct amdgpu_fence *ring_guilty_fence;
>       struct amdgpu_ring *ring;
>       bool rings_busy;
> @@ -1034,8 +1035,9 @@ void
> amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
>  /**
>   * amdgpu_multi_ring_reset_helper_end() - Prepare multiple rings for a reset.
>   *
> + * @adev: amdgpu device
>   * @ring_type_mask: Bitmask of affected ring types
> - * @guilty_ring: The ring which is guilty of causing a reset.
> + * @guilty_ring: The ring which is guilty of causing a reset, or NULL.
>   * @ret: Return code from the reset function.
>   *
>   * After calling amdgpu_multi_ring_reset_helper_begin()
> @@ -1046,10 +1048,10 @@ void
> amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
>   * be called to restore preemption state, but it won't attempt to
>   * fully restore the ring contents.
>   */
> -int amdgpu_multi_ring_reset_helper_end(const u32 ring_type_mask,
> +int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
> +                                    const u32 ring_type_mask,
>                                      struct amdgpu_ring *guilty_ring, int ret)  {
> -     struct amdgpu_device *adev = guilty_ring->adev;
>       struct amdgpu_ring *ring;
>       int i, r;
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index b2040f80b7f2..7e6ed43771f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -637,10 +637,12 @@ void amdgpu_ring_reset_helper_begin(struct
> amdgpu_ring *ring,
>                                   struct amdgpu_fence *guilty_fence);  int
> amdgpu_ring_reset_helper_end(struct amdgpu_ring *ring,
>                                struct amdgpu_fence *guilty_fence); -void
> amdgpu_multi_ring_reset_helper_begin(const u32 ring_type_mask,
> +void amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
> +                                       const u32 ring_type_mask,
>                                         struct amdgpu_ring *guilty_ring,
>                                         struct amdgpu_fence *guilty_fence);
> -int amdgpu_multi_ring_reset_helper_end(const u32 ring_type_mask,
> +int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
> +                                    const u32 ring_type_mask,
>                                      struct amdgpu_ring *guilty_ring, int ret);
> bool amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring,
>                                        u32 reset_type);
> --
> 2.50.1

Re: [PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring reset helpers

[Timur Kristóf]

On Mon, 2026-11-02 at 01:28 +0000, Zhang, Jesse(Jie) wrote:
> AMD General
> 
> Ping on the series ?

Hi,

I'm reviewing the series, just need a little more time to finish my
review and test it.

Thanks,
Timur

Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> amdgpu_ring_backup_unprocessed_commands() skips the backup of a ring
> and
> clears ring->guilty_fence when it sees the same guilty fence again,
> to
> avoid replay loops. That happens when the IP block soft reset follows
> a
> failed ring reset of the same job.
> amdgpu_multi_ring_reset_helper_end()
> then restored the guilty ring with ring->guilty_fence, NULL by then,
> so
> neither the guilty fence nor the rest of the ring was completed: the
> hung
> job stayed pending until it timed out again.
> 
> Pass the guilty fence to the end helper, and use it for the guilty
> ring.
> For the other rings, and when the backup didn't drop it, nothing
> changes.
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 13 ++++++++++---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 ++-
>  3 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> index d0b0d3eaffc5..2eea18965fd1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> @@ -469,7 +469,8 @@ int amdgpu_device_ip_soft_reset(struct
> amdgpu_ring *guilty_ring,
>  
>  	r = ip_block->version->funcs->soft_reset(ip_block);
>  
> -	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> guilty_ring, r);
> +	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> guilty_ring,
> +					       guilty_fence, r);
>  	amdgpu_device_unlock_reset_domain(adev->reset_domain);
>  
>  	if (r) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> index edae764eb0c4..7f0f0ae6650a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -1038,6 +1038,7 @@ void
> amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
>   * @adev: amdgpu device
>   * @ring_type_mask: Bitmask of affected ring types
>   * @guilty_ring: The ring which is guilty of causing a reset, or
> NULL.
> + * @guilty_fence: The fence which didn't signal on the guilty ring.
>   * @ret: Return code from the reset function.
>   *
>   * After calling amdgpu_multi_ring_reset_helper_begin()
> @@ -1050,7 +1051,8 @@ void
> amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
>   */
>  int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
>  				       const u32 ring_type_mask,
> -				       struct amdgpu_ring
> *guilty_ring, int ret)
> +				       struct amdgpu_ring
> *guilty_ring,
> +				       struct amdgpu_fence
> *guilty_fence, int ret)
>  {
>  	struct amdgpu_ring *ring;
>  	int i, r;
> @@ -1080,8 +1082,13 @@ int amdgpu_multi_ring_reset_helper_end(struct
> amdgpu_device *adev,
>  		if (!(BIT(ring->funcs->type) & ring_type_mask))
>  			continue;
>  
> -		/* Restore contents of the ring */
> -		r = amdgpu_ring_reset_helper_end(ring, ring-
> >guilty_fence);
> +		/*
> +		 * Restore contents of the ring. The backup of the
> guilty ring
> +		 * may have dropped its guilty fence, eg. after a
> failed ring
> +		 * reset with the same fence: still complete it.
> +		 */
> +		r = amdgpu_ring_reset_helper_end(ring, ring ==
> guilty_ring ?
> +						 guilty_fence :
> ring->guilty_fence);

Can you help me understand what was wrong with the line before?
I think guilty_fence == ring->guilty_fence when ring == guilty_ring no?


>  		if (r) {
>  			dev_err(adev->dev,
>  				"Failed to recover ring %s after
> soft reset\n",
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 7e6ed43771f2..349c7d503e5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -643,7 +643,8 @@ void amdgpu_multi_ring_reset_helper_begin(struct
> amdgpu_device *adev,
>  					  struct amdgpu_fence
> *guilty_fence);
>  int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
>  				       const u32 ring_type_mask,
> -				       struct amdgpu_ring
> *guilty_ring, int ret);
> +				       struct amdgpu_ring
> *guilty_ring,
> +				       struct amdgpu_fence
> *guilty_fence, int ret);
>  bool amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring,
>  					 u32 reset_type);
>  #endif

Re: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP block soft reset

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> Some IP blocks have engines that the hardware can reset on their own,
> eg. the graphics and compute engines of the GFX IP
> (CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET / CMP_ONLY_SOFT_RESET).
> Resetting
> only the engine of a hung ring leaves the work on the other engines
> alone, which the IP block soft reset can't do, as it resets every
> ring
> of the IP block.
> 
> Add an optional soft_reset_engine() IP block callback, which resets
> one
> engine of the IP block, or all of them, and recovers the user queues
> on
> it, and amdgpu_device_ip_soft_reset_engine() around it. It only
> stops,
> backs up and replays the rings of that engine, and it is also
> reachable
> without a guilty kernel ring, for a hung user queue.
> 
> For a single engine, it only takes the reset domain for read: KFD
> only
> try-locks it for read before it adds or removes queues, and would
> fail
> those calls with the write lock although the other engines aren't
> reset.
> 
> amdgpu_device_ip_soft_reset() uses it when the IP block implements
> it,
> for the engine of the guilty ring.
> 
> v3: replaces the separate engine reset type and helper
>     (Timur).
> 
> Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   | 116
> +++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h   |   6 ++
>  drivers/gpu/drm/amd/include/amd_shared.h |  18 ++++
>  3 files changed, 140 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> index 2eea18965fd1..e95f69dd3142 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> @@ -418,6 +418,111 @@ static u32 amdgpu_ring_mask_from_ip(const enum
> amd_ip_block_type ip_type)
>  	}
>  }
>  
> +/**
> + * amdgpu_ring_mask_from_engine() - Find mask of ring types of an
> engine.
> + *
> + * @ip_type: The IP block type of the engine.
> + * @engine: The engine whose rings you are looking for.
> + */
> +static u32 amdgpu_ring_mask_from_engine(const enum amd_ip_block_type
> ip_type,
> +					const enum amd_ip_engine
> engine)
> +{
> +	switch (engine) {
> +	case AMD_IP_ENGINE_GFX:
> +		return BIT(AMDGPU_RING_TYPE_GFX);
> +
> +	case AMD_IP_ENGINE_COMPUTE:
> +		return BIT(AMDGPU_RING_TYPE_COMPUTE);
> +
> +	default:
> +		return amdgpu_ring_mask_from_ip(ip_type);
> +	}
> +}
> +
> +/**
> + * amdgpu_engine_from_ring() - Find the engine of a ring type.
> + *
> + * @ring_type: The ring type whose engine you are looking for.
> + */
> +static enum amd_ip_engine amdgpu_engine_from_ring(const enum
> amdgpu_ring_type ring_type)
> +{
> +	switch (ring_type) {
> +	case AMDGPU_RING_TYPE_GFX:
> +		return AMD_IP_ENGINE_GFX;
> +
> +	case AMDGPU_RING_TYPE_COMPUTE:
> +		return AMD_IP_ENGINE_COMPUTE;
> +
> +	default:
> +		return AMD_IP_ENGINE_ALL;
> +	}
> +}
> +
> +static const char *const amdgpu_ip_engine_names[] = {
> +	[AMD_IP_ENGINE_ALL] = "all",
> +	[AMD_IP_ENGINE_GFX] = "gfx",
> +	[AMD_IP_ENGINE_COMPUTE] = "compute",
> +};

No need to add a new enum.
This is redundant with amdgpu_ring_type.


> +
> +/**
> + * amdgpu_device_ip_soft_reset_engine() - Soft reset one engine of
> an IP block.
> + *
> + * @adev: amdgpu device
> + * @ip_type: The IP block type.
> + * @engine: The engine to reset, or AMD_IP_ENGINE_ALL for the whole
> IP block.
> + * @guilty_ring: The ring which is guilty of causing a reset, or
> NULL.
> + * @guilty_fence: The fence which didn't signal on @guilty_ring.
> + * @guilty_queue: The user queue which is guilty of causing a reset,
> or NULL.
> + */
> +int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device *adev,
> +				       enum amd_ip_block_type
> ip_type,
> +				       enum amd_ip_engine engine,
> +				       struct amdgpu_ring
> *guilty_ring,
> +				       struct amdgpu_fence
> *guilty_fence,
> +				       struct amdgpu_usermode_queue
> *guilty_queue)

Please don't add a separate function.
This duplicates a lot of code that shouldn't be duplicated.

Instead, I suggest to just add the new functionality to
amdgpu_device_ip_soft_reset(), for example like this:

if (ip_block->version->funcs->soft_reset_engine)
  ip_block->version->funcs->soft_reset_engine(...);
else if (ip_block->version->funcs->soft_reset)
  ip_block->version->funcs->soft_reset(...);


> +{
> +	struct amdgpu_ip_block *ip_block;
> +	u32 ring_type_mask;
> +	int r;
> +
> +	ip_block = amdgpu_device_ip_get_ip_block(adev, ip_type);
> +	if (!ip_block || !ip_block->version->funcs-
> >soft_reset_engine)
> +		return -EOPNOTSUPP;
> +
> +	dev_err(adev->dev, "Starting %s %s engine soft reset\n",
> +		ip_block->version->funcs->name,
> amdgpu_ip_engine_names[engine]);
> +
> +	ring_type_mask = amdgpu_ring_mask_from_engine(ip_type,
> engine);
> +
> +	if (engine == AMD_IP_ENGINE_ALL)
> +		amdgpu_device_lock_reset_domain(adev->reset_domain);
> +	else
> +		down_read(&adev->reset_domain->sem);
> +	amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask,
> guilty_ring,
> +					     guilty_fence);
> +
> +	r = ip_block->version->funcs->soft_reset_engine(ip_block,
> engine,
> +							guilty_queue
> );
> +
> +	r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> guilty_ring,
> +					       guilty_fence, r);
> +	if (engine == AMD_IP_ENGINE_ALL)
> +		amdgpu_device_unlock_reset_domain(adev-
> >reset_domain);
> +	else
> +		up_read(&adev->reset_domain->sem);
> +
> +	if (r) {
> +		dev_err(adev->dev, "Failed %s %s engine soft reset:
> %d\n",
> +			ip_block->version->funcs->name,
> +			amdgpu_ip_engine_names[engine], r);
> +		return r;
> +	}
> +
> +	dev_err(adev->dev, "Successful %s %s engine soft reset\n",
> +		ip_block->version->funcs->name,
> amdgpu_ip_engine_names[engine]);
> +	return 0;
> +}
> +
>  /**
>   * amdgpu_device_ip_soft_reset() - Perform a graceful soft reset on
> an IP block.
>   *
> @@ -430,6 +535,9 @@ static u32 amdgpu_ring_mask_from_ip(const enum
> amd_ip_block_type ip_type)
>   * resets all rings that belong to the same device IP block
>   * and re-initializes the device IP block.
>   *
> + * IP blocks with engines that can be reset separately only reset
> the engine
> + * of @guilty_ring, see amdgpu_device_ip_soft_reset_engine().
> + *
>   * The reset is handled gracefully, meaning that we try to
>   * minimize collateral damage (ie. avoid rejecting non-guilty jobs)
>   * as well as back up and restore the contents of all rings
> @@ -441,6 +549,7 @@ int amdgpu_device_ip_soft_reset(struct
> amdgpu_ring *guilty_ring,
>  	struct amdgpu_device *adev = guilty_ring->adev;
>  	struct amdgpu_ip_block *ip_block;
>  	enum amd_ip_block_type ip_type;
> +	enum amd_ip_engine engine;
>  	u32 ring_type_mask;
>  	int r;
>  
> @@ -453,6 +562,13 @@ int amdgpu_device_ip_soft_reset(struct
> amdgpu_ring *guilty_ring,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	if (ip_block->version->funcs->soft_reset_engine) {
> +		engine = amdgpu_engine_from_ring(guilty_ring->funcs-
> >type);
> +		return amdgpu_device_ip_soft_reset_engine(adev,
> ip_type, engine,
> +							 
> guilty_ring,
> +							 
> guilty_fence, NULL);
> +	}
> +
>  	if (!ip_block->version->funcs->soft_reset) {
>  		dev_warn(adev->dev, "IP block soft reset not
> supported on %s\n",
>  			 ip_block->version->funcs->name);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> index 5dd6273982c4..856297d1c8e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> @@ -160,5 +160,11 @@ bool amdgpu_device_ip_is_valid(struct
> amdgpu_device *adev,
>  			       enum amd_ip_block_type block_type);
>  int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
>  				struct amdgpu_fence *guilty_fence);
> +int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device *adev,
> +				       enum amd_ip_block_type
> ip_type,
> +				       enum amd_ip_engine engine,
> +				       struct amdgpu_ring
> *guilty_ring,
> +				       struct amdgpu_fence
> *guilty_fence,
> +				       struct amdgpu_usermode_queue
> *guilty_queue);
>  
>  #endif /* __AMDGPU_IP_H__ */
> diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index e81ce9d79009..95cdd0fc191e 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -29,6 +29,7 @@
>  
>  #define AMD_MAX_USEC_TIMEOUT		1000000  /* 1000 ms */
>  struct amdgpu_ip_block;
> +struct amdgpu_usermode_queue;
>  
>  
>  /*
> @@ -422,6 +423,18 @@ enum DC_DEBUG_MASK {
>  
>  enum amd_dpm_forced_level;
>  
> +/**
> + * enum amd_ip_engine - the engines of an IP block that can be soft
> reset
> + * @AMD_IP_ENGINE_ALL: all the engines of the IP block
> + * @AMD_IP_ENGINE_GFX: the graphics engine of the GFX IP block
> + * @AMD_IP_ENGINE_COMPUTE: the compute engine of the GFX IP block
> + */
> +enum amd_ip_engine {
> +	AMD_IP_ENGINE_ALL,
> +	AMD_IP_ENGINE_GFX,
> +	AMD_IP_ENGINE_COMPUTE,
> +};
> +
>  /**
>   * struct amd_ip_funcs - general hooks for managing amdgpu IP Blocks
>   * @name: Name of IP block
> @@ -441,6 +454,8 @@ enum amd_dpm_forced_level;
>   * @complete: handles IP specific changes after resume
>   * @wait_for_idle: poll for idle
>   * @soft_reset: soft reset the IP block
> + * @soft_reset_engine: soft reset one engine of the IP block, or all
> of them,
> + *                     and recover the user queues on it
>   * @set_clockgating_state: enable/disable cg for the IP block
>   * @set_powergating_state: enable/disable pg for the IP block
>   * @get_clockgating_state: get current clockgating status
> @@ -469,6 +484,9 @@ struct amd_ip_funcs {
>  	void (*complete)(struct amdgpu_ip_block *ip_block);
>  	int (*wait_for_idle)(struct amdgpu_ip_block *ip_block);
>  	int (*soft_reset)(struct amdgpu_ip_block *ip_block);
> +	int (*soft_reset_engine)(struct amdgpu_ip_block *ip_block,
> +				 enum amd_ip_engine engine,
> +				 struct amdgpu_usermode_queue
> *guilty_queue);
>  	int (*set_clockgating_state)(struct amdgpu_ip_block
> *ip_block,
>  				     enum amd_clockgating_state
> state);
>  	int (*set_powergating_state)(struct amdgpu_ip_block
> *ip_block,

Re: [PATCH v3 04/15] drm/amdgpu/gfx: add a reset_engine callback

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> Some gfx hangs can't be recovered by a queue or a pipe reset, eg. a
> CP
> front-end stuck in the middle of a packet. They currently end in a
> full
> GPU reset, and VRAM is lost. Resetting only the graphics engine is
> enough for them, and the compute engine isn't reset.
> 
> Add an optional reset_engine callback to amdgpu_gfx_funcs for the
> hardware part of such a reset: it resets the CP of one engine of the
> GFX
> IP and maps the kernel queues of that engine again. A later patch
> builds
> the soft_reset_engine() IP block callback of the gfx IPs around it.
> 
> v3: Take enum amd_ip_engine, called by the soft_reset_engine() IP
> block
>     callback (Timur).
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 5939e1fb3b24..8d7bd7057666 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -35,6 +35,7 @@
>  #include "amdgpu_ras.h"
>  #include "amdgpu_ring_mux.h"
>  #include "amdgpu_xcp.h"
> +#include "amd_shared.h"
>  
>  struct amdgpu_usermode_queue;
>  
> @@ -376,6 +377,9 @@ struct amdgpu_gfx_funcs {
>  	/* find the gfx HQD slot (me/pipe/queue) for a doorbell */
>  	bool (*detect_hung_queue)(struct amdgpu_device *adev, u32
> doorbell_index,
>  				  u32 *me, u32 *pipe, u32 *queue);
> +	/* reset the CP of one engine and map its kernel queues
> again */
> +	int (*reset_engine)(struct amdgpu_device *adev,
> +			    enum amd_ip_engine engine);

No need to add this.
The soft_reset_engine callback on the IP block is enough.

>  };
>  
>  struct sq_work {

Re: [PATCH v3 06/15] drm/amdgpu/gfx11: add a gfx engine reset

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, 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.
> 
> 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..e533a5939d02 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 amd_ip_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,

Instead, this should be hooked up to the soft_reset_engine callback of
the IP block.

>  };
>  
>  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);

Why not also set the SOFT_RESET_GFX bit?

> +	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);

As far as I know, rs64 is for Navi3x dGPUs only, right?
We need an alternative code path that works on APUs too.

> +	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 aren't reset.
> + */
> +static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
> +				  enum amd_ip_engine engine)
> +{
> +	int i, r;
> +
> +	lockdep_assert_held(&adev->gfx.mec.reset_mutex);
> +
> +	if (engine != AMD_IP_ENGINE_GFX)
> +		return -EOPNOTSUPP;

It's OK if we don't support resetting compute initially, but we need a
plan to implement that eventually (and/or full GFX IP block reset).

> +
> +	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)

Re: [PATCH v3 07/15] drm/amdgpu/gfx: add callbacks to read and restore the ring state of a gfx queue

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> A gfx engine reset clears the HQD registers of the gfx queues,
> including
> the only copy of the ring position of the queues MES didn't preempt.
> MES doesn't save them in the MQD either when the engine hangs.
> 
> Add struct amdgpu_gfx_hqd_ptrs for that ring state, and gfx callbacks
> for the gfx IP, which knows its registers and the layout of its MQD:
> - read_gfx_hqd_ptrs() reads the ring state of the gfx HQD of a
> doorbell;
> - restore_gfx_mqd_ring() puts it in the MQD after the reset, for MES
> to
>   map the queue again from it.
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 8d7bd7057666..f750b11f4ad4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -346,6 +346,17 @@ struct amdgpu_gfx_shadow_info {
>  	u32 eop_alignment;
>  };
>  
> +/* Ring state of a gfx HQD that a gfx engine reset clears. */
> +struct amdgpu_gfx_hqd_ptrs {
> +	u32	doorbell_index;
> +	u64	mqd_addr;	/* the MQD loaded in the HQD */
> +	bool	on_hqd;
> +	u64	wptr;
> +	u32	offset;
> +	u32	csmd_rptr;
> +	bool	connected;	/* the queue the pipe executes */
> +};
> +
>  struct amdgpu_gfx_funcs {
>  	/* get the gpu clock counter */
>  	uint64_t (*get_gpu_clock_counter)(struct amdgpu_device
> *adev);
> @@ -380,6 +391,16 @@ struct amdgpu_gfx_funcs {
>  	/* reset the CP of one engine and map its kernel queues
> again */
>  	int (*reset_engine)(struct amdgpu_device *adev,
>  			    enum amd_ip_engine engine);
> +	/* read the ring state of the gfx HQD of a doorbell, if it
> has one */
> +	bool (*read_gfx_hqd_ptrs)(struct amdgpu_device *adev, u32
> doorbell_index,
> +				  struct amdgpu_gfx_hqd_ptrs *ptrs);

Why is this called "ptrs" (plural) when the function seems to just read
a single HQD?

> +	/*
> +	 * write the ring state read by read_gfx_hqd_ptrs() into a
> gfx MQD;
> +	 * true if the work up to ptrs->wptr was dropped
> +	 */
> +	bool (*restore_gfx_mqd_ring)(struct amdgpu_device *adev,
> void *mqd,
> +				     u32 ring_size,
> +				     struct amdgpu_gfx_hqd_ptrs
> *ptrs);

Seems inconsistent to me that this one is named "mqd" while the other
one is named "hqd", and "ring" vs. "ptrs".

I suggest the following naming: save_hqd() and restore_hqd()

>  };
>  
>  struct sq_work {

Re: [PATCH v3 11/15] drm/amdgpu/userq: add helpers to keep the user queues of one engine across a reset

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> An engine reset, eg. of the gfx engine, only resets the queues of one
> engine of the GFX IP; the queues of the other engines aren't reset.
> amdgpu_userq_pre_reset() and amdgpu_userq_post_reset() handle all the
> user queues for a full reset. Add separate helpers for the queues of
> one
> engine, which keep them in the state they had:
> 
> - amdgpu_userq_pre_engine_reset(), called after the MES suspend,
> saves
>   the ring state of the mapped queues still on a HQD: the reset
> clears
>   their HQD, and MES couldn't preempt them into their MQD. The queues
>   MES preempted have their state in their MQD already.
> - amdgpu_userq_post_engine_reset(), called after the reset but before
>   MES resumes, unmaps those queues, puts the saved ring state in
> their
>   MQD and maps them again: MES doesn't map a queue whose HQD the
> reset
>   cleared again on its own. The queues keep their state and their
> work,
>   except the one the pipe executed: its work in flight depends on
> pipe
>   state the reset cleared, it is dropped and its fences signaled. The
>   queue that hung, if any, is unmapped and hung.
> - amdgpu_userq_cancel_engine_reset() drops the saved ring state for
> the
>   full reset that follows a failed engine reset.
> 
> The helpers walk the queues and unmap and map them as
> amdgpu_userq_pre_reset() and amdgpu_userq_post_reset() do. The saved
> ring state is kept in the gfx IP, per HQD, and matched to a queue by
> its
> doorbell and MQD address.
> 
> v2: Drop the user queue reset lock and the walk over the queues of
> one
>     type (Christian).
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h   |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 108
> ++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h |   5 +
>  3 files changed, 116 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index f750b11f4ad4..6bbf53359a49 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -518,6 +518,9 @@ struct amdgpu_gfx {
>  
>  	/* reset mask */
>  	uint32_t 			gfx_supported_reset;
> +	/* ring state of the gfx user queues on a HQD, saved by an
> engine reset */
> +	struct
> amdgpu_gfx_hqd_ptrs	engine_reset_hqds[AMDGPU_MAX_GFX_QUEUES];
> +	u32				num_engine_reset_hqds;

Instead of "engine_reset_hqds", I suggest: "saved_gfx_hqds",
assuming GFX is different enough from a compute HQD that we won't be
able to reuse this for compute.

>  	uint32_t 			compute_supported_reset;
>  
>  	/* gfx off */
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> index 1a2aa85c504e..353c5f7cfcdd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c
> @@ -1597,3 +1597,111 @@ int amdgpu_userq_post_reset(struct
> amdgpu_device *adev, bool vram_lost)
>  
>  	return ret;
>  }
> +
> +/**
> + * amdgpu_userq_pre_engine_reset - save the ring state of the user
> queues
> + * @adev: amdgpu device
> + * @queue_type: AMDGPU_HW_IP_* type of the queues on the reset
> engine
> + */
> +void amdgpu_userq_pre_engine_reset(struct amdgpu_device *adev, u32
> queue_type)

I suggest to rename this to: amdgpu_userq_save_hqds()

> +{
> +	struct amdgpu_gfx_hqd_ptrs ptrs;
> +	struct amdgpu_usermode_queue *queue;
> +	unsigned long queue_id;
> +
> +	adev->gfx.num_engine_reset_hqds = 0;
> +	if (!adev->gfx.funcs->read_gfx_hqd_ptrs)
> +		return;
> +
> +	xa_for_each(&adev->userq_doorbell_xa, queue_id, queue) {
> +		if (adev->gfx.num_engine_reset_hqds >=
> AMDGPU_MAX_GFX_QUEUES)
> +			break;
> +		if (queue->queue_type != queue_type ||
> +		    queue->state != AMDGPU_USERQ_STATE_MAPPED)
> +			continue;
> +		if (adev->gfx.funcs->read_gfx_hqd_ptrs(adev, queue-
> >doorbell_index,
> +						       &ptrs) &&
> ptrs.on_hqd)
> +			adev->gfx.engine_reset_hqds[adev-
> >gfx.num_engine_reset_hqds++] = ptrs;
> +	}
> +}
> +
> +static struct amdgpu_gfx_hqd_ptrs *
> +amdgpu_userq_engine_reset_hqd(struct amdgpu_device *adev,
> +			      struct amdgpu_usermode_queue *queue)

This function seems to not reset anything, but just searches in the
array of saved HQDs. I suggest to rename to:
"amdgpu_userq_find_saved_hqd"

> +{
> +	struct amdgpu_gfx_hqd_ptrs *ptrs;
> +	u32 i;
> +
> +	for (i = 0; i < adev->gfx.num_engine_reset_hqds; i++) {
> +		ptrs = &adev->gfx.engine_reset_hqds[i];
> +		/* Check the MQD too: a new queue may reuse the
> doorbell. */
> +		if (ptrs->doorbell_index == queue->doorbell_index &&
> +		    ptrs->mqd_addr == queue->mqd.gpu_addr)
> +			return ptrs;
> +	}
> +	return NULL;
> +}
> +
> +/**
> + * amdgpu_userq_post_engine_reset - restore the user queues after an
> engine reset
> + * @adev: amdgpu device
> + * @queue_type: AMDGPU_HW_IP_* type of the queues on the reset
> engine
> + * @guilty: the queue that hung, or NULL
> + *
> + * MES doesn't map a queue whose HQD the reset cleared again on its
> own: unmap
> + * it from MES, restore its MQD, and map it again.
> + */
> +void amdgpu_userq_post_engine_reset(struct amdgpu_device *adev, u32
> queue_type,
> +				    struct amdgpu_usermode_queue
> *guilty)

I suggest to rename this to: amdgpu_userq_restore_hqds() to better
describe what this is doing.

> +{
> +	const struct amdgpu_userq_funcs *userq_funcs = adev-
> >userq_funcs[queue_type];
> +	struct amdgpu_usermode_queue *queue;
> +	struct amdgpu_gfx_hqd_ptrs *ptrs;
> +	unsigned long queue_id;
> +	int r;
> +
> +	xa_for_each(&adev->userq_doorbell_xa, queue_id, queue) {
> +		if (queue->queue_type != queue_type ||
> +		    queue->state != AMDGPU_USERQ_STATE_MAPPED)
> +			continue;
> +
> +		if (queue == guilty) {
> +			userq_funcs->unmap(queue);
> +			amdgpu_userq_fence_driver_force_completion(q
> ueue);
> +			trace_amdgpu_userq_state_changed(queue,
> AMDGPU_USERQ_STATE_HUNG);
> +			queue->state = AMDGPU_USERQ_STATE_HUNG;
> +			continue;
> +		}
> +
> +		ptrs = amdgpu_userq_engine_reset_hqd(adev, queue);
> +		if (!ptrs)
> +			continue;
> +
> +		userq_funcs->unmap(queue);
> +		if (adev->gfx.funcs->restore_gfx_mqd_ring(adev,
> queue->mqd.cpu_ptr,
> +							  queue-
> >userq_prop->queue_size,
> +							  ptrs))
> +			amdgpu_userq_fence_driver_force_completion_w
> ith_seqno(queue,
> +								
> 	      ptrs->wptr);
> +		r = userq_funcs->map(queue);
> +		if (r) {
> +			dev_err(adev->dev, "Failed to remap user
> queue %llu: %d\n",
> +				queue->doorbell_index, r);
> +			amdgpu_userq_fence_driver_force_completion(q
> ueue);
> +			trace_amdgpu_userq_state_changed(queue,
> AMDGPU_USERQ_STATE_HUNG);
> +			queue->state = AMDGPU_USERQ_STATE_HUNG;
> +		}
> +	}
> +	adev->gfx.num_engine_reset_hqds = 0;
> +}
> +
> +/**
> + * amdgpu_userq_cancel_engine_reset - give up an engine reset
> + * @adev: amdgpu device
> + * @queue_type: AMDGPU_HW_IP_* type of the queues on the reset
> engine
> + */
> +void amdgpu_userq_cancel_engine_reset(struct amdgpu_device *adev,
> +				      u32 queue_type)

amdgpu_userq_clear_hqds

> +{
> +	adev->gfx.num_engine_reset_hqds = 0;
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> index 488dc21d7c81..e07c71cd5af5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.h
> @@ -181,6 +181,11 @@ int
> amdgpu_userq_start_sched_for_enforce_isolation(struct amdgpu_device
> *adev,
>  void amdgpu_userq_reset_work(struct work_struct *work);
>  void amdgpu_userq_pre_reset(struct amdgpu_device *adev);
>  int amdgpu_userq_post_reset(struct amdgpu_device *adev, bool
> vram_lost);
> +void amdgpu_userq_pre_engine_reset(struct amdgpu_device *adev, u32
> queue_type);
> +void amdgpu_userq_post_engine_reset(struct amdgpu_device *adev, u32
> queue_type,
> +				    struct amdgpu_usermode_queue
> *guilty);
> +void amdgpu_userq_cancel_engine_reset(struct amdgpu_device *adev,
> +				      u32 queue_type);
>  void amdgpu_userq_start_hang_detect_work(struct
> amdgpu_usermode_queue *queue);
>  void amdgpu_userq_process_fence_irq(struct amdgpu_device *adev, u32
> doorbell);
>  

Re: [PATCH v3 12/15] drm/amdgpu/gfx: add a gfx engine soft reset

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> Add amdgpu_gfx_soft_reset_engine(), the soft_reset_engine() IP block
> callback of the gfx IPs, around the reset_engine callback. It only
> supports the graphics engine so far. The IP block soft reset stops,
> backs up and replays the kernel gfx rings around it; the compute
> rings
> aren't stopped.
> 
> MES is suspended across the reset, so that it doesn't schedule or map
> queues while the CP front-end is reset. The compute queues are
> preempted
> meanwhile: their work is only paused. MES
> fetches its commands through the CP front-end though, and a hung gfx
> stream can keep it from completing the suspend. Reset the engine
> anyway:
> the reset frees the front-end.
> 
> The reset also clears the HQDs of the gfx user queues. Call
> amdgpu_userq_pre_engine_reset() after the MES suspend, to save the
> ring
> state of the queues still on a HQD, and
> amdgpu_userq_post_engine_reset()
> after the reset, before MES resumes, to restore them. @guilty_queue
> is
> the gfx user queue that hung, if any: it is hung instead. If the
> engine
> reset fails, amdgpu_userq_cancel_engine_reset() drops the saved ring
> state for the full reset that follows.
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 41
> +++++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |  3 ++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 0ae6e990c310..f1a26e4eb06c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -2181,6 +2181,47 @@ int amdgpu_gfx_reset_mes_gfx(struct
> amdgpu_device *adev,
>  	return r;
>  }
>  
> +/**
> + * amdgpu_gfx_soft_reset_engine() - Soft reset one engine of the GFX
> IP.
> + *
> + * @ip_block: The GFX IP block.
> + * @engine: The engine to reset.
> + * @guilty_queue: The user queue that hung, or NULL.
> + *
> + * soft_reset_engine() IP block callback around the reset_engine
> callback,
> + * which resets the CP of the engine. Only the graphics engine is
> supported
> + * so far.
> + */
> +int amdgpu_gfx_soft_reset_engine(struct amdgpu_ip_block *ip_block,
> +				 enum amd_ip_engine engine,
> +				 struct amdgpu_usermode_queue
> *guilty_queue)
> +{
> +	struct amdgpu_device *adev = ip_block->adev;
> +	int r;
> +
> +	if (engine != AMD_IP_ENGINE_GFX || !adev->gfx.funcs-
> >reset_engine)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&adev->gfx.mec.reset_mutex);
> +
> +	/* The suspend may time out while the gfx engine hangs:
> reset anyway. */
> +	amdgpu_mes_suspend(adev, 0);
> +	amdgpu_userq_pre_engine_reset(adev, AMDGPU_HW_IP_GFX);
> +	r = adev->gfx.funcs->reset_engine(adev, engine);
> +	if (r) {
> +		amdgpu_userq_cancel_engine_reset(adev,
> AMDGPU_HW_IP_GFX);
> +		amdgpu_mes_resume(adev, 0);
> +		goto exit;
> +	}
> +
> +	amdgpu_userq_post_engine_reset(adev, AMDGPU_HW_IP_GFX,
> guilty_queue);
> +	r = amdgpu_mes_resume(adev, 0);
> +
> +exit:
> +	mutex_unlock(&adev->gfx.mec.reset_mutex);
> +	return r;
> +}

I think the additional functionality should be added to
the amdgpu_device_ip_soft_reset() function as well.

> +
>  int amdgpu_gfx_mes_reset_queue(struct amdgpu_ring *ring,
>  			       unsigned int vmid,
>  			       struct amdgpu_fence *timedout_fence,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 6bbf53359a49..d63d9f5f5b96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -740,6 +740,9 @@ void amdgpu_gfx_pipe_reset_prepare(struct
> amdgpu_device *adev,
>  				   struct amdgpu_ring *guilty_ring,
>  				   u32 me, u32 pipe,
>  				   struct amdgpu_gfx_pipe_reset_ctx
> *ctx);
> +int amdgpu_gfx_soft_reset_engine(struct amdgpu_ip_block *ip_block,
> +				 enum amd_ip_engine engine,
> +				 struct amdgpu_usermode_queue
> *guilty_queue);
>  int amdgpu_gfx_reset_mes_gfx(struct amdgpu_device *adev,
>  			     struct amdgpu_ring *ring,
>  			     struct amdgpu_fence *guilty_fence,

Re: [PATCH v3 15/15] drm/amdgpu/gfx11: enable the gfx engine soft reset

[Timur Kristóf]

On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> Report AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET for the gfx rings, with
> amdgpu_gfx_soft_reset_engine() as the soft_reset_engine() callback,
> when
> the reset_engine callback can be used: with MES, async gfx rings and
> the
> RS64 firmware. The reset restarts PFP and ME from the start of their
> RS64 firmware, and remaps the kernel gfx queue through MES from its
> MQD, which the kernel gfx queue only has with async gfx rings. Not on
> SR-IOV VFs.
> 
> The kernel gfx queue and gfx user queue resets then fall back to a
> soft
> reset of the gfx engine before a full reset. The IP block soft reset
> of
> gfx11 only resets the gfx engine then; the whole IP block
> soft_reset()
> isn't used.
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> index a71dae5d0d3a..41ebd2ead99b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> @@ -363,6 +363,7 @@ 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 bool gfx_v11_0_engine_reset_supported(struct amdgpu_device
> *adev);
>  static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
>  				  enum amd_ip_engine engine);
>  static bool gfx_v11_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
> @@ -1945,6 +1946,10 @@ static int gfx_v11_0_sw_init(struct
> amdgpu_ip_block *ip_block)
>  		break;
>  	}
>  
> +	if (gfx_v11_0_engine_reset_supported(adev) &&
> +	    !adev->debug_disable_ip_block_soft_reset)
> +		adev->gfx.gfx_supported_reset |=
> AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
> +
>  	if (!adev->enable_mes_kiq) {
>  		r = amdgpu_gfx_kiq_init(adev, GFX11_MEC_HPD_SIZE,
> 0);
>  		if (r) {
> @@ -6927,6 +6932,16 @@ static void gfx_v11_0_emit_mem_sync(struct
> amdgpu_ring *ring)
>  	amdgpu_ring_write(ring, gcr_cntl); /* GCR_CNTL */
>  }
>  
> +/*
> + * The reset restarts PFP/ME through their RS64 start addresses, and
> remaps the
> + * kernel gfx queue from its MQD, which needs async rings.
> + */
> +static bool gfx_v11_0_engine_reset_supported(struct amdgpu_device
> *adev)
> +{
> +	return !amdgpu_sriov_vf(adev) && adev->enable_mes &&
> +	       amdgpu_async_gfx_ring && adev->gfx.rs64_enable;

I tried to test this on Strix Halo but it seems it doesn't work, maybe
due to rs64_enable?

Is there any reason to not enable this on GFX11.5 (and GFX11.7)?

> +}
> +
>  /* 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)
>  {
> @@ -7370,6 +7385,7 @@ static const struct amd_ip_funcs
> gfx_v11_0_ip_funcs = {
>  	.resume = gfx_v11_0_resume,
>  	.wait_for_idle = gfx_v11_0_wait_for_idle,
>  	.soft_reset = gfx_v11_0_soft_reset,
> +	.soft_reset_engine = amdgpu_gfx_soft_reset_engine,
>  	.set_clockgating_state = gfx_v11_0_set_clockgating_state,
>  	.set_powergating_state = gfx_v11_0_set_powergating_state,
>  	.get_clockgating_state = gfx_v11_0_get_clockgating_state,

RE: [PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring reset helpers

[Zhang, Jesse(Jie)]

AMD General

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Tuesday, November 3, 2026 12:24 AM
> To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> Subject: Re: [PATCH v3 01/15] drm/amdgpu: pass the device to the multi ring
> reset helpers
>
> On Mon, 2026-11-02 at 01:28 +0000, Zhang, Jesse(Jie) wrote:
> > AMD General
> >
> > Ping on the series ?
>
> Hi,
>
> I'm reviewing the series, just need a little more time to finish my review and
> test it.
Got it, thanks very much Timur.
>
> Thanks,
> Timur

RE: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

[Zhang, Jesse(Jie)]

AMD General

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Wednesday, November 4, 2026 4:12 AM
> To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> Subject: Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the
> end of a multi ring reset
>
> On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> > amdgpu_ring_backup_unprocessed_commands() skips the backup of a ring
> > and clears ring->guilty_fence when it sees the same guilty fence
> > again, to avoid replay loops. That happens when the IP block soft
> > reset follows a failed ring reset of the same job.
> > amdgpu_multi_ring_reset_helper_end()
> > then restored the guilty ring with ring->guilty_fence, NULL by then,
> > so neither the guilty fence nor the rest of the ring was completed:
> > the hung job stayed pending until it timed out again.
> >
> > Pass the guilty fence to the end helper, and use it for the guilty
> > ring.
> > For the other rings, and when the backup didn't drop it, nothing
> > changes.
> >
> > Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   |  3 ++-
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 13 ++++++++++---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  3 ++-
> >  3 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > index d0b0d3eaffc5..2eea18965fd1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > @@ -469,7 +469,8 @@ int amdgpu_device_ip_soft_reset(struct
> > amdgpu_ring *guilty_ring,
> >
> >     r = ip_block->version->funcs->soft_reset(ip_block);
> >
> > -   r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> > guilty_ring, r);
> > +   r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> > guilty_ring,
> > +                                          guilty_fence, r);
> >     amdgpu_device_unlock_reset_domain(adev->reset_domain);
> >
> >     if (r) {
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > index edae764eb0c4..7f0f0ae6650a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> > @@ -1038,6 +1038,7 @@ void
> > amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
> >   * @adev: amdgpu device
> >   * @ring_type_mask: Bitmask of affected ring types
> >   * @guilty_ring: The ring which is guilty of causing a reset, or
> > NULL.
> > + * @guilty_fence: The fence which didn't signal on the guilty ring.
> >   * @ret: Return code from the reset function.
> >   *
> >   * After calling amdgpu_multi_ring_reset_helper_begin()
> > @@ -1050,7 +1051,8 @@ void
> > amdgpu_multi_ring_reset_helper_begin(struct amdgpu_device *adev,
> >   */
> >  int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
> >                                    const u32 ring_type_mask,
> > -                                  struct amdgpu_ring
> > *guilty_ring, int ret)
> > +                                  struct amdgpu_ring
> > *guilty_ring,
> > +                                  struct amdgpu_fence
> > *guilty_fence, int ret)
> >  {
> >     struct amdgpu_ring *ring;
> >     int i, r;
> > @@ -1080,8 +1082,13 @@ int
> amdgpu_multi_ring_reset_helper_end(struct
> > amdgpu_device *adev,
> >             if (!(BIT(ring->funcs->type) & ring_type_mask))
> >                     continue;
> >
> > -           /* Restore contents of the ring */
> > -           r = amdgpu_ring_reset_helper_end(ring, ring-
> > >guilty_fence);
> > +           /*
> > +            * Restore contents of the ring. The backup of the
> > guilty ring
> > +            * may have dropped its guilty fence, eg. after a
> > failed ring
> > +            * reset with the same fence: still complete it.
> > +            */
> > +           r = amdgpu_ring_reset_helper_end(ring, ring ==
> > guilty_ring ?
> > +                                            guilty_fence :
> > ring->guilty_fence);
>
> Can you help me understand what was wrong with the line before?
> I think guilty_fence == ring->guilty_fence when ring == guilty_ring no?

Usually yes, but not when the soft reset follows a failed ring reset of the
same job.
 amdgpu_ring_reset(ring, vmid, job->hw_fence)
   -> amdgpu_ring_reset_helper_begin(ring, F)
   -> amdgpu_ring_backup_unprocessed_commands(ring, F)
      sets ring->guilty_fence = F.

So by the time the end helper runs, ring->guilty_fence no longer equals the
guilty fence: the begin helper cleared it.

Thanks
Jesse

>
>
> >             if (r) {
> >                     dev_err(adev->dev,
> >                             "Failed to recover ring %s after
> > soft reset\n",
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > index 7e6ed43771f2..349c7d503e5f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> > @@ -643,7 +643,8 @@ void amdgpu_multi_ring_reset_helper_begin(struct
> > amdgpu_device *adev,
> >                                       struct amdgpu_fence
> > *guilty_fence);
> >  int amdgpu_multi_ring_reset_helper_end(struct amdgpu_device *adev,
> >                                    const u32 ring_type_mask,
> > -                                  struct amdgpu_ring
> > *guilty_ring, int ret);
> > +                                  struct amdgpu_ring
> > *guilty_ring,
> > +                                  struct amdgpu_fence
> > *guilty_fence, int ret);
> >  bool amdgpu_ring_is_reset_type_supported(struct amdgpu_ring *ring,
> >                                      u32 reset_type);
> >  #endif

RE: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP block soft reset

[Zhang, Jesse(Jie)]

AMD General

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Wednesday, November 4, 2026 4:16 AM
> To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> Subject: Re: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP
> block soft reset
>
> On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> > Some IP blocks have engines that the hardware can reset on their own,
> > eg. the graphics and compute engines of the GFX IP
> > (CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET / CMP_ONLY_SOFT_RESET).
> > Resetting
> > only the engine of a hung ring leaves the work on the other engines
> > alone, which the IP block soft reset can't do, as it resets every ring
> > of the IP block.
> >
> > Add an optional soft_reset_engine() IP block callback, which resets
> > one engine of the IP block, or all of them, and recovers the user
> > queues on it, and amdgpu_device_ip_soft_reset_engine() around it. It
> > only stops, backs up and replays the rings of that engine, and it is
> > also reachable without a guilty kernel ring, for a hung user queue.
> >
> > For a single engine, it only takes the reset domain for read: KFD only
> > try-locks it for read before it adds or removes queues, and would fail
> > those calls with the write lock although the other engines aren't
> > reset.
> >
> > amdgpu_device_ip_soft_reset() uses it when the IP block implements it,
> > for the engine of the guilty ring.
> >
> > v3: replaces the separate engine reset type and helper
> >     (Timur).
> >
> > Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
> > Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   | 116
> > +++++++++++++++++++++++
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h   |   6 ++
> >  drivers/gpu/drm/amd/include/amd_shared.h |  18 ++++
> >  3 files changed, 140 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > index 2eea18965fd1..e95f69dd3142 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > @@ -418,6 +418,111 @@ static u32 amdgpu_ring_mask_from_ip(const
> enum
> > amd_ip_block_type ip_type)
> >     }
> >  }
> >
> > +/**
> > + * amdgpu_ring_mask_from_engine() - Find mask of ring types of an
> > engine.
> > + *
> > + * @ip_type: The IP block type of the engine.
> > + * @engine: The engine whose rings you are looking for.
> > + */
> > +static u32 amdgpu_ring_mask_from_engine(const enum
> amd_ip_block_type
> > ip_type,
> > +                                   const enum amd_ip_engine
> > engine)
> > +{
> > +   switch (engine) {
> > +   case AMD_IP_ENGINE_GFX:
> > +           return BIT(AMDGPU_RING_TYPE_GFX);
> > +
> > +   case AMD_IP_ENGINE_COMPUTE:
> > +           return BIT(AMDGPU_RING_TYPE_COMPUTE);
> > +
> > +   default:
> > +           return amdgpu_ring_mask_from_ip(ip_type);
> > +   }
> > +}
> > +
> > +/**
> > + * amdgpu_engine_from_ring() - Find the engine of a ring type.
> > + *
> > + * @ring_type: The ring type whose engine you are looking for.
> > + */
> > +static enum amd_ip_engine amdgpu_engine_from_ring(const enum
> > amdgpu_ring_type ring_type)
> > +{
> > +   switch (ring_type) {
> > +   case AMDGPU_RING_TYPE_GFX:
> > +           return AMD_IP_ENGINE_GFX;
> > +
> > +   case AMDGPU_RING_TYPE_COMPUTE:
> > +           return AMD_IP_ENGINE_COMPUTE;
> > +
> > +   default:
> > +           return AMD_IP_ENGINE_ALL;
> > +   }
> > +}
> > +
> > +static const char *const amdgpu_ip_engine_names[] = {
> > +   [AMD_IP_ENGINE_ALL] = "all",
> > +   [AMD_IP_ENGINE_GFX] = "gfx",
> > +   [AMD_IP_ENGINE_COMPUTE] = "compute", };
>
> No need to add a new enum.
> This is redundant with amdgpu_ring_type.
Agreed. In v4 the callback takes the ring type of the hung queue.
>
>
> > +
> > +/**
> > + * amdgpu_device_ip_soft_reset_engine() - Soft reset one engine of
> > an IP block.
> > + *
> > + * @adev: amdgpu device
> > + * @ip_type: The IP block type.
> > + * @engine: The engine to reset, or AMD_IP_ENGINE_ALL for the whole
> > IP block.
> > + * @guilty_ring: The ring which is guilty of causing a reset, or
> > NULL.
> > + * @guilty_fence: The fence which didn't signal on @guilty_ring.
> > + * @guilty_queue: The user queue which is guilty of causing a reset,
> > or NULL.
> > + */
> > +int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device *adev,
> > +                                  enum amd_ip_block_type
> > ip_type,
> > +                                  enum amd_ip_engine engine,
> > +                                  struct amdgpu_ring
> > *guilty_ring,
> > +                                  struct amdgpu_fence
> > *guilty_fence,
> > +                                  struct amdgpu_usermode_queue
> > *guilty_queue)
>
> Please don't add a separate function.
> This duplicates a lot of code that shouldn't be duplicated.
>
> Instead, I suggest to just add the new functionality to
> amdgpu_device_ip_soft_reset(), for example like this:
>
> if (ip_block->version->funcs->soft_reset_engine)
>   ip_block->version->funcs->soft_reset_engine(...);
> else if (ip_block->version->funcs->soft_reset)
>   ip_block->version->funcs->soft_reset(...);

OK, I'll fold it into amdgpu_device_ip_soft_reset(). The separate entry
point was only there for the user queue caller (patch 13): a hung gfx
user queue has no kernel ring, and amdgpu_device_ip_soft_reset() gets the
device and the IP block from guilty_ring. So the signature would become:

        int amdgpu_device_ip_soft_reset(struct amdgpu_device *adev,
                                        enum amdgpu_ring_type ring_type,
                                        struct amdgpu_ring *guilty_ring,
                                        struct amdgpu_fence *guilty_fence,
                                        struct amdgpu_usermode_queue *guilty_queue);

amdgpu_job_timedout() passes (ring->adev, ring->funcs->type, ring,
job->hw_fence, NULL), and the user queue reset passes (adev,
AMDGPU_RING_TYPE_GFX, NULL, NULL, queue). The body stays the same as
today. When the IP block has soft_reset_engine, it uses that callback,
only the rings of that type, and the reset domain for read:

        if (funcs->soft_reset_engine) {
                ring_type_mask = BIT(ring_type);
                down_read(&adev->reset_domain->sem);
        } else {
                if (!guilty_ring || !funcs->soft_reset)
                        return -EOPNOTSUPP;
                ring_type_mask = amdgpu_ring_mask_from_ip(ip_type);
                amdgpu_device_lock_reset_domain(adev->reset_domain);
        }

        amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask,
                                             guilty_ring, guilty_fence);
        if (funcs->soft_reset_engine)
                r = funcs->soft_reset_engine(ip_block, ring_type, guilty_queue);
        else
                r = funcs->soft_reset(ip_block);
        r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
                                               guilty_ring, guilty_fence, r);

        if (funcs->soft_reset_engine)
                up_read(&adev->reset_domain->sem);
        else
                amdgpu_device_unlock_reset_domain(adev->reset_domain);

Does the change work for you?

Thanks
Jesse
>
>
> > +{
> > +   struct amdgpu_ip_block *ip_block;
> > +   u32 ring_type_mask;
> > +   int r;
> > +
> > +   ip_block = amdgpu_device_ip_get_ip_block(adev, ip_type);
> > +   if (!ip_block || !ip_block->version->funcs-
> > >soft_reset_engine)
> > +           return -EOPNOTSUPP;
> > +
> > +   dev_err(adev->dev, "Starting %s %s engine soft reset\n",
> > +           ip_block->version->funcs->name,
> > amdgpu_ip_engine_names[engine]);
> > +
> > +   ring_type_mask = amdgpu_ring_mask_from_engine(ip_type,
> > engine);
> > +
> > +   if (engine == AMD_IP_ENGINE_ALL)
> > +           amdgpu_device_lock_reset_domain(adev->reset_domain);
> > +   else
> > +           down_read(&adev->reset_domain->sem);
> > +   amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask,
> > guilty_ring,
> > +                                        guilty_fence);
> > +
> > +   r = ip_block->version->funcs->soft_reset_engine(ip_block,
> > engine,
> > +                                                   guilty_queue
> > );
> > +
> > +   r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> > guilty_ring,
> > +                                          guilty_fence, r);
> > +   if (engine == AMD_IP_ENGINE_ALL)
> > +           amdgpu_device_unlock_reset_domain(adev-
> > >reset_domain);
> > +   else
> > +           up_read(&adev->reset_domain->sem);
> > +
> > +   if (r) {
> > +           dev_err(adev->dev, "Failed %s %s engine soft reset:
> > %d\n",
> > +                   ip_block->version->funcs->name,
> > +                   amdgpu_ip_engine_names[engine], r);
> > +           return r;
> > +   }
> > +
> > +   dev_err(adev->dev, "Successful %s %s engine soft reset\n",
> > +           ip_block->version->funcs->name,
> > amdgpu_ip_engine_names[engine]);
> > +   return 0;
> > +}
> > +
> >  /**
> >   * amdgpu_device_ip_soft_reset() - Perform a graceful soft reset on
> > an IP block.
> >   *
> > @@ -430,6 +535,9 @@ static u32 amdgpu_ring_mask_from_ip(const enum
> > amd_ip_block_type ip_type)
> >   * resets all rings that belong to the same device IP block
> >   * and re-initializes the device IP block.
> >   *
> > + * IP blocks with engines that can be reset separately only reset
> > the engine
> > + * of @guilty_ring, see amdgpu_device_ip_soft_reset_engine().
> > + *
> >   * The reset is handled gracefully, meaning that we try to
> >   * minimize collateral damage (ie. avoid rejecting non-guilty jobs)
> >   * as well as back up and restore the contents of all rings @@ -441,6
> > +549,7 @@ int amdgpu_device_ip_soft_reset(struct
> > amdgpu_ring *guilty_ring,
> >     struct amdgpu_device *adev = guilty_ring->adev;
> >     struct amdgpu_ip_block *ip_block;
> >     enum amd_ip_block_type ip_type;
> > +   enum amd_ip_engine engine;
> >     u32 ring_type_mask;
> >     int r;
> >
> > @@ -453,6 +562,13 @@ int amdgpu_device_ip_soft_reset(struct
> > amdgpu_ring *guilty_ring,
> >             return -EOPNOTSUPP;
> >     }
> >
> > +   if (ip_block->version->funcs->soft_reset_engine) {
> > +           engine = amdgpu_engine_from_ring(guilty_ring->funcs-
> > >type);
> > +           return amdgpu_device_ip_soft_reset_engine(adev,
> > ip_type, engine,
> > +
> > guilty_ring,
> > +
> > guilty_fence, NULL);
> > +   }
> > +
> >     if (!ip_block->version->funcs->soft_reset) {
> >             dev_warn(adev->dev, "IP block soft reset not supported
> on %s\n",
> >                      ip_block->version->funcs->name);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> > index 5dd6273982c4..856297d1c8e3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h
> > @@ -160,5 +160,11 @@ bool amdgpu_device_ip_is_valid(struct
> > amdgpu_device *adev,
> >                            enum amd_ip_block_type block_type);
> >  int amdgpu_device_ip_soft_reset(struct amdgpu_ring *guilty_ring,
> >                             struct amdgpu_fence *guilty_fence);
> > +int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device *adev,
> > +                                  enum amd_ip_block_type
> > ip_type,
> > +                                  enum amd_ip_engine engine,
> > +                                  struct amdgpu_ring
> > *guilty_ring,
> > +                                  struct amdgpu_fence
> > *guilty_fence,
> > +                                  struct amdgpu_usermode_queue
> > *guilty_queue);
> >
> >  #endif /* __AMDGPU_IP_H__ */
> > diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
> > b/drivers/gpu/drm/amd/include/amd_shared.h
> > index e81ce9d79009..95cdd0fc191e 100644
> > --- a/drivers/gpu/drm/amd/include/amd_shared.h
> > +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> > @@ -29,6 +29,7 @@
> >
> >  #define AMD_MAX_USEC_TIMEOUT               1000000  /* 1000 ms */
> >  struct amdgpu_ip_block;
> > +struct amdgpu_usermode_queue;
> >
> >
> >  /*
> > @@ -422,6 +423,18 @@ enum DC_DEBUG_MASK {
> >
> >  enum amd_dpm_forced_level;
> >
> > +/**
> > + * enum amd_ip_engine - the engines of an IP block that can be soft
> > reset
> > + * @AMD_IP_ENGINE_ALL: all the engines of the IP block
> > + * @AMD_IP_ENGINE_GFX: the graphics engine of the GFX IP block
> > + * @AMD_IP_ENGINE_COMPUTE: the compute engine of the GFX IP block
> */
> > +enum amd_ip_engine {
> > +   AMD_IP_ENGINE_ALL,
> > +   AMD_IP_ENGINE_GFX,
> > +   AMD_IP_ENGINE_COMPUTE,
> > +};
> > +
> >  /**
> >   * struct amd_ip_funcs - general hooks for managing amdgpu IP Blocks
> >   * @name: Name of IP block
> > @@ -441,6 +454,8 @@ enum amd_dpm_forced_level;
> >   * @complete: handles IP specific changes after resume
> >   * @wait_for_idle: poll for idle
> >   * @soft_reset: soft reset the IP block
> > + * @soft_reset_engine: soft reset one engine of the IP block, or all
> > of them,
> > + *                     and recover the user queues on it
> >   * @set_clockgating_state: enable/disable cg for the IP block
> >   * @set_powergating_state: enable/disable pg for the IP block
> >   * @get_clockgating_state: get current clockgating status @@ -469,6
> > +484,9 @@ struct amd_ip_funcs {
> >     void (*complete)(struct amdgpu_ip_block *ip_block);
> >     int (*wait_for_idle)(struct amdgpu_ip_block *ip_block);
> >     int (*soft_reset)(struct amdgpu_ip_block *ip_block);
> > +   int (*soft_reset_engine)(struct amdgpu_ip_block *ip_block,
> > +                            enum amd_ip_engine engine,
> > +                            struct amdgpu_usermode_queue
> > *guilty_queue);
> >     int (*set_clockgating_state)(struct amdgpu_ip_block *ip_block,
> >                                  enum amd_clockgating_state
> > state);
> >     int (*set_powergating_state)(struct amdgpu_ip_block *ip_block,

RE: [PATCH v3 15/15] drm/amdgpu/gfx11: enable the gfx engine soft reset

[Zhang, Jesse(Jie)]

AMD General

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Wednesday, November 4, 2026 4:41 AM
> To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> Subject: Re: [PATCH v3 15/15] drm/amdgpu/gfx11: enable the gfx engine soft
> reset
>
> On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> > Report AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET for the gfx rings, with
> > amdgpu_gfx_soft_reset_engine() as the soft_reset_engine() callback,
> > when the reset_engine callback can be used: with MES, async gfx rings
> > and the
> > RS64 firmware. The reset restarts PFP and ME from the start of their
> > RS64 firmware, and remaps the kernel gfx queue through MES from its
> > MQD, which the kernel gfx queue only has with async gfx rings. Not on
> > SR-IOV VFs.
> >
> > The kernel gfx queue and gfx user queue resets then fall back to a
> > soft reset of the gfx engine before a full reset. The IP block soft
> > reset of
> > gfx11 only resets the gfx engine then; the whole IP block
> > soft_reset()
> > isn't used.
> >
> > Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > index a71dae5d0d3a..41ebd2ead99b 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > @@ -363,6 +363,7 @@ 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 bool gfx_v11_0_engine_reset_supported(struct amdgpu_device
> > *adev);
> >  static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
> >                               enum amd_ip_engine engine);
> >  static bool gfx_v11_0_read_gfx_hqd_ptrs(struct amdgpu_device *adev,
> > @@ -1945,6 +1946,10 @@ static int gfx_v11_0_sw_init(struct
> > amdgpu_ip_block *ip_block)
> >             break;
> >     }
> >
> > +   if (gfx_v11_0_engine_reset_supported(adev) &&
> > +       !adev->debug_disable_ip_block_soft_reset)
> > +           adev->gfx.gfx_supported_reset |=
> > AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
> > +
> >     if (!adev->enable_mes_kiq) {
> >             r = amdgpu_gfx_kiq_init(adev, GFX11_MEC_HPD_SIZE, 0);
> >             if (r) {
> > @@ -6927,6 +6932,16 @@ static void gfx_v11_0_emit_mem_sync(struct
> > amdgpu_ring *ring)
> >     amdgpu_ring_write(ring, gcr_cntl); /* GCR_CNTL */
> >  }
> >
> > +/*
> > + * The reset restarts PFP/ME through their RS64 start addresses, and
> > remaps the
> > + * kernel gfx queue from its MQD, which needs async rings.
> > + */
> > +static bool gfx_v11_0_engine_reset_supported(struct amdgpu_device
> > *adev)
> > +{
> > +   return !amdgpu_sriov_vf(adev) && adev->enable_mes &&
> > +          amdgpu_async_gfx_ring && adev->gfx.rs64_enable;
>
> I tried to test this on Strix Halo but it seems it doesn't work, maybe due to
> rs64_enable?
Yes. Strix Halo doesn't use rs64, and the reset isn't enabled for StrixHalo.

>
> Is there any reason to not enable this on GFX11.5 (and GFX11.7)?
No, will try on these devices.
>
> > +}
> > +
> >  /* 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)
> >  {
> > @@ -7370,6 +7385,7 @@ static const struct amd_ip_funcs
> > gfx_v11_0_ip_funcs = {
> >     .resume = gfx_v11_0_resume,
> >     .wait_for_idle = gfx_v11_0_wait_for_idle,
> >     .soft_reset = gfx_v11_0_soft_reset,
> > +   .soft_reset_engine = amdgpu_gfx_soft_reset_engine,
> >     .set_clockgating_state = gfx_v11_0_set_clockgating_state,
> >     .set_powergating_state = gfx_v11_0_set_powergating_state,
> >     .get_clockgating_state = gfx_v11_0_get_clockgating_state,

Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

[Timur Kristóf]

On Wed, 2026-11-04 at 02:46 +0000, Zhang, Jesse(Jie) wrote:
> > > +           r = amdgpu_ring_reset_helper_end(ring, ring ==
> > > guilty_ring ?
> > > +                                            guilty_fence :
> > > ring->guilty_fence);
> > 
> > Can you help me understand what was wrong with the line before?
> > I think guilty_fence == ring->guilty_fence when ring == guilty_ring
> > no?
> 
> Usually yes, but not when the soft reset follows a failed ring reset
> of the
> same job.
>  amdgpu_ring_reset(ring, vmid, job->hw_fence)
>    -> amdgpu_ring_reset_helper_begin(ring, F)
>    -> amdgpu_ring_backup_unprocessed_commands(ring, F)
>       sets ring->guilty_fence = F.
> 
> So by the time the end helper runs, ring->guilty_fence no longer
> equals the
> guilty fence: the begin helper cleared it.

I think I see the problem.

I re-read the queue reset code and the helpers. During a queue reset,
it makes a backup and saves the guilty fence but if the queue reset
fails, it doesn't restore the contents, and on a subsequent call it
clears the guilty fence and the backed up contents.

Unfortunately I think this commit doesn't completely fix that because
the reset begin helper will still clear the backed up contents on the
second call (sets ring_backup_entries_to_copy = 0).

I think the correct fix would be to make sure we don't repeat the ring
reset begin helper:
https://gitlab.freedesktop.org/Venemo/linux/-/commit/86f2bf222caee0da267f9d9e12362513fe4f7f9e
Please take a look and if you agree feel free to take that patch in
your series.

Timur

Re: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP block soft reset

[Timur Kristóf]

On Wed, 2026-11-04 at 03:32 +0000, Zhang, Jesse(Jie) wrote:
> AMD General
> 
> > -----Original Message-----
> > From: Timur Kristóf <timur.kristof@gmail.com>
> > Sent: Wednesday, November 4, 2026 4:16 AM
> > To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> > gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig,
> > Christian
> > <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> > Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> > <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> > Subject: Re: [PATCH v3 03/15] drm/amdgpu: add an engine scope to
> > the IP
> > block soft reset
> > 
> > On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> > > Some IP blocks have engines that the hardware can reset on their
> > > own,
> > > eg. the graphics and compute engines of the GFX IP
> > > (CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET / CMP_ONLY_SOFT_RESET).
> > > Resetting
> > > only the engine of a hung ring leaves the work on the other
> > > engines
> > > alone, which the IP block soft reset can't do, as it resets every
> > > ring
> > > of the IP block.
> > > 
> > > Add an optional soft_reset_engine() IP block callback, which
> > > resets
> > > one engine of the IP block, or all of them, and recovers the user
> > > queues on it, and amdgpu_device_ip_soft_reset_engine() around it.
> > > It
> > > only stops, backs up and replays the rings of that engine, and it
> > > is
> > > also reachable without a guilty kernel ring, for a hung user
> > > queue.
> > > 
> > > For a single engine, it only takes the reset domain for read: KFD
> > > only
> > > try-locks it for read before it adds or removes queues, and would
> > > fail
> > > those calls with the write lock although the other engines aren't
> > > reset.
> > > 
> > > amdgpu_device_ip_soft_reset() uses it when the IP block
> > > implements it,
> > > for the engine of the guilty ring.
> > > 
> > > v3: replaces the separate engine reset type and helper
> > >     (Timur).
> > > 
> > > Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
> > > Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   | 116
> > > +++++++++++++++++++++++
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h   |   6 ++
> > >  drivers/gpu/drm/amd/include/amd_shared.h |  18 ++++
> > >  3 files changed, 140 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > index 2eea18965fd1..e95f69dd3142 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > @@ -418,6 +418,111 @@ static u32 amdgpu_ring_mask_from_ip(const
> > enum
> > > amd_ip_block_type ip_type)
> > >     }
> > >  }
> > > 
> > > +/**
> > > + * amdgpu_ring_mask_from_engine() - Find mask of ring types of
> > > an
> > > engine.
> > > + *
> > > + * @ip_type: The IP block type of the engine.
> > > + * @engine: The engine whose rings you are looking for.
> > > + */
> > > +static u32 amdgpu_ring_mask_from_engine(const enum
> > amd_ip_block_type
> > > ip_type,
> > > +                                   const enum amd_ip_engine
> > > engine)
> > > +{
> > > +   switch (engine) {
> > > +   case AMD_IP_ENGINE_GFX:
> > > +           return BIT(AMDGPU_RING_TYPE_GFX);
> > > +
> > > +   case AMD_IP_ENGINE_COMPUTE:
> > > +           return BIT(AMDGPU_RING_TYPE_COMPUTE);
> > > +
> > > +   default:
> > > +           return amdgpu_ring_mask_from_ip(ip_type);
> > > +   }
> > > +}
> > > +
> > > +/**
> > > + * amdgpu_engine_from_ring() - Find the engine of a ring type.
> > > + *
> > > + * @ring_type: The ring type whose engine you are looking for.
> > > + */
> > > +static enum amd_ip_engine amdgpu_engine_from_ring(const enum
> > > amdgpu_ring_type ring_type)
> > > +{
> > > +   switch (ring_type) {
> > > +   case AMDGPU_RING_TYPE_GFX:
> > > +           return AMD_IP_ENGINE_GFX;
> > > +
> > > +   case AMDGPU_RING_TYPE_COMPUTE:
> > > +           return AMD_IP_ENGINE_COMPUTE;
> > > +
> > > +   default:
> > > +           return AMD_IP_ENGINE_ALL;
> > > +   }
> > > +}
> > > +
> > > +static const char *const amdgpu_ip_engine_names[] = {
> > > +   [AMD_IP_ENGINE_ALL] = "all",
> > > +   [AMD_IP_ENGINE_GFX] = "gfx",
> > > +   [AMD_IP_ENGINE_COMPUTE] = "compute", };
> > 
> > No need to add a new enum.
> > This is redundant with amdgpu_ring_type.
> Agreed. In v4 the callback takes the ring type of the hung queue.

Sounds good, thanks!

> > 
> > 
> > > +
> > > +/**
> > > + * amdgpu_device_ip_soft_reset_engine() - Soft reset one engine
> > > of
> > > an IP block.
> > > + *
> > > + * @adev: amdgpu device
> > > + * @ip_type: The IP block type.
> > > + * @engine: The engine to reset, or AMD_IP_ENGINE_ALL for the
> > > whole
> > > IP block.
> > > + * @guilty_ring: The ring which is guilty of causing a reset, or
> > > NULL.
> > > + * @guilty_fence: The fence which didn't signal on @guilty_ring.
> > > + * @guilty_queue: The user queue which is guilty of causing a
> > > reset,
> > > or NULL.
> > > + */
> > > +int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device
> > > *adev,
> > > +                                  enum amd_ip_block_type
> > > ip_type,
> > > +                                  enum amd_ip_engine engine,
> > > +                                  struct amdgpu_ring
> > > *guilty_ring,
> > > +                                  struct amdgpu_fence
> > > *guilty_fence,
> > > +                                  struct amdgpu_usermode_queue
> > > *guilty_queue)
> > 
> > Please don't add a separate function.
> > This duplicates a lot of code that shouldn't be duplicated.
> > 
> > Instead, I suggest to just add the new functionality to
> > amdgpu_device_ip_soft_reset(), for example like this:
> > 
> > if (ip_block->version->funcs->soft_reset_engine)
> >   ip_block->version->funcs->soft_reset_engine(...);
> > else if (ip_block->version->funcs->soft_reset)
> >   ip_block->version->funcs->soft_reset(...);
> 
> OK, I'll fold it into amdgpu_device_ip_soft_reset(). The separate
> entry
> point was only there for the user queue caller (patch 13): a hung gfx
> user queue has no kernel ring, and amdgpu_device_ip_soft_reset() gets
> the
> device and the IP block from guilty_ring. So the signature would
> become:
> 
>         int amdgpu_device_ip_soft_reset(struct amdgpu_device *adev,
>                                         enum amdgpu_ring_type
> ring_type,
>                                         struct amdgpu_ring
> *guilty_ring,
>                                         struct amdgpu_fence
> *guilty_fence,
>                                         struct amdgpu_usermode_queue
> *guilty_queue);
> 
> amdgpu_job_timedout() passes (ring->adev, ring->funcs->type, ring,
> job->hw_fence, NULL), and the user queue reset passes (adev,
> AMDGPU_RING_TYPE_GFX, NULL, NULL, queue). The body stays the same as
> today. When the IP block has soft_reset_engine, it uses that
> callback,
> only the rings of that type, and the reset domain for read:
> 
>         if (funcs->soft_reset_engine) {
>                 ring_type_mask = BIT(ring_type);
>                 down_read(&adev->reset_domain->sem);
>         } else {
>                 if (!guilty_ring || !funcs->soft_reset)
>                         return -EOPNOTSUPP;
>                 ring_type_mask = amdgpu_ring_mask_from_ip(ip_type);
>                 amdgpu_device_lock_reset_domain(adev->reset_domain);
>         }
> 
>         amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask,
>                                              guilty_ring,
> guilty_fence);
>         if (funcs->soft_reset_engine)
>                 r = funcs->soft_reset_engine(ip_block, ring_type,
> guilty_queue);
>         else
>                 r = funcs->soft_reset(ip_block);
>         r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
>                                                guilty_ring,
> guilty_fence, r);
> 
>         if (funcs->soft_reset_engine)
>                 up_read(&adev->reset_domain->sem);
>         else
>                 amdgpu_device_unlock_reset_domain(adev-
> >reset_domain);
> 
> Does the change work for you?

Yes, that sounds good to me.
I just have one question about it:

> the user queue reset passes (adev,
> AMDGPU_RING_TYPE_GFX, NULL, NULL, queue)

Wouldn't the user queue reset still need to pass the guilty fence from
the user queue? Or is that not possible?

Thanks,
Timur

Re: [PATCH v3 15/15] drm/amdgpu/gfx11: enable the gfx engine soft reset

[Timur Kristóf]

On Wed, 2026-11-04 at 05:54 +0000, Zhang, Jesse(Jie) wrote:
> AMD General
> 
> > -----Original Message-----
> > From: Timur Kristóf <timur.kristof@gmail.com>
> > Sent: Wednesday, November 4, 2026 4:41 AM
> > To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> > gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig,
> > Christian
> > <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> > Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> > <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> > Subject: Re: [PATCH v3 15/15] drm/amdgpu/gfx11: enable the gfx
> > engine soft
> > reset
> > 
> > On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> > > Report AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET for the gfx rings,
> > > with
> > > amdgpu_gfx_soft_reset_engine() as the soft_reset_engine()
> > > callback,
> > > when the reset_engine callback can be used: with MES, async gfx
> > > rings
> > > and the
> > > RS64 firmware. The reset restarts PFP and ME from the start of
> > > their
> > > RS64 firmware, and remaps the kernel gfx queue through MES from
> > > its
> > > MQD, which the kernel gfx queue only has with async gfx rings.
> > > Not on
> > > SR-IOV VFs.
> > > 
> > > The kernel gfx queue and gfx user queue resets then fall back to
> > > a
> > > soft reset of the gfx engine before a full reset. The IP block
> > > soft
> > > reset of
> > > gfx11 only resets the gfx engine then; the whole IP block
> > > soft_reset()
> > > isn't used.
> > > 
> > > Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > index a71dae5d0d3a..41ebd2ead99b 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v11_0.c
> > > @@ -363,6 +363,7 @@ 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 bool gfx_v11_0_engine_reset_supported(struct
> > > amdgpu_device
> > > *adev);
> > >  static int gfx_v11_0_reset_engine(struct amdgpu_device *adev,
> > >                               enum amd_ip_engine engine);
> > >  static bool gfx_v11_0_read_gfx_hqd_ptrs(struct amdgpu_device
> > > *adev,
> > > @@ -1945,6 +1946,10 @@ static int gfx_v11_0_sw_init(struct
> > > amdgpu_ip_block *ip_block)
> > >             break;
> > >     }
> > > 
> > > +   if (gfx_v11_0_engine_reset_supported(adev) &&
> > > +       !adev->debug_disable_ip_block_soft_reset)
> > > +           adev->gfx.gfx_supported_reset |=
> > > AMDGPU_RESET_TYPE_IP_BLOCK_SOFT_RESET;
> > > +
> > >     if (!adev->enable_mes_kiq) {
> > >             r = amdgpu_gfx_kiq_init(adev, GFX11_MEC_HPD_SIZE, 0);
> > >             if (r) {
> > > @@ -6927,6 +6932,16 @@ static void gfx_v11_0_emit_mem_sync(struct
> > > amdgpu_ring *ring)
> > >     amdgpu_ring_write(ring, gcr_cntl); /* GCR_CNTL */
> > >  }
> > > 
> > > +/*
> > > + * The reset restarts PFP/ME through their RS64 start addresses,
> > > and
> > > remaps the
> > > + * kernel gfx queue from its MQD, which needs async rings.
> > > + */
> > > +static bool gfx_v11_0_engine_reset_supported(struct
> > > amdgpu_device
> > > *adev)
> > > +{
> > > +   return !amdgpu_sriov_vf(adev) && adev->enable_mes &&
> > > +          amdgpu_async_gfx_ring && adev->gfx.rs64_enable;
> > 
> > I tried to test this on Strix Halo but it seems it doesn't work,
> > maybe due to
> > rs64_enable?
> Yes. Strix Halo doesn't use rs64, and the reset isn't enabled for
> StrixHalo.
> 
> > 
> > Is there any reason to not enable this on GFX11.5 (and GFX11.7)?
> No, will try on these devices.
> 

Thank you!

I think we definitely need to improve the reset experience on GFX11.5
as well as other GFX11 based APUs. I have seen many failed queue resets
on my Strix Halo GPU.

RE: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

[Zhang, Jesse(Jie)]

AMD General

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Thursday, November 5, 2026 6:47 AM
> To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> Subject: Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the
> end of a multi ring reset
>
> On Wed, 2026-11-04 at 02:46 +0000, Zhang, Jesse(Jie) wrote:
> > > > +           r = amdgpu_ring_reset_helper_end(ring, ring ==
> > > > guilty_ring ?
> > > > +                                            guilty_fence :
> > > > ring->guilty_fence);
> > >
> > > Can you help me understand what was wrong with the line before?
> > > I think guilty_fence == ring->guilty_fence when ring == guilty_ring
> > > no?
> >
> > Usually yes, but not when the soft reset follows a failed ring reset
> > of the same job.
> >  amdgpu_ring_reset(ring, vmid, job->hw_fence)
> >    -> amdgpu_ring_reset_helper_begin(ring, F)
> >    -> amdgpu_ring_backup_unprocessed_commands(ring, F)
> >       sets ring->guilty_fence = F.
> >
> > So by the time the end helper runs, ring->guilty_fence no longer
> > equals the guilty fence: the begin helper cleared it.
>
> I think I see the problem.
>
> I re-read the queue reset code and the helpers. During a queue reset, it makes
> a backup and saves the guilty fence but if the queue reset fails, it doesn't
> restore the contents, and on a subsequent call it clears the guilty fence and the
> backed up contents.
>
> Unfortunately I think this commit doesn't completely fix that because the reset
> begin helper will still clear the backed up contents on the second call (sets
> ring_backup_entries_to_copy = 0).
>
> I think the correct fix would be to make sure we don't repeat the ring reset
> begin helper:
> https://gitlab.freedesktop.org/Venemo/linux/-
> /commit/86f2bf222caee0da267f9d9e12362513fe4f7f9e
> Please take a look and if you agree feel free to take that patch in your series.
Right, the second backup also drops the commands of the other jobs on the
guilty ring, and skipping the begin helper is the better fix. With it,
ring->guilty_fence also stays the guilty fence, so 02/15 isn't needed
anymore.

I think the condition in your patch is inverted though:

        if (guilty_fence && ring->guilty_fence == guilty_fence)
                amdgpu_ring_reset_helper_begin(ring, ring_guilty_fence);

calls the begin helper only for a ring that was already backed up with
this guilty fence. The other rings, and the guilty ring of a soft reset
without a failed ring reset before it, aren't backed up anymore. The
latter would even replay a stale backup. The guilty ring after a failed
ring reset is the one ring that still goes through the begin helper. I
think you meant:

        /* A failed ring reset of the same job already backed it up. */
        if (ring != guilty_ring || ring->guilty_fence != guilty_fence)
                amdgpu_ring_reset_helper_begin(ring, ring_guilty_fence);

If that's what you meant, I'll take your patch with that change in place
of 02 in v4.

Jesse
>
> Timur

RE: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP block soft reset

[Zhang, Jesse(Jie)]

AMD General

> -----Original Message-----
> From: Timur Kristóf <timur.kristof@gmail.com>
> Sent: Thursday, November 5, 2026 6:49 AM
> To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> gfx@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
> <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> Subject: Re: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the IP
> block soft reset
>
> On Wed, 2026-11-04 at 03:32 +0000, Zhang, Jesse(Jie) wrote:
> > AMD General
> >
> > > -----Original Message-----
> > > From: Timur Kristóf <timur.kristof@gmail.com>
> > > Sent: Wednesday, November 4, 2026 4:16 AM
> > > To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> > > gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig,
> > > Christian <Christian.Koenig@amd.com>; Lazar, Lijo
> > > <Lijo.Lazar@amd.com>; Mario Sopena-Novales
> <Mario.Novales@amd.com>;
> > > Rastogi, Manu <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey)
> > > <Jiongyu.Zhu@amd.com>
> > > Subject: Re: [PATCH v3 03/15] drm/amdgpu: add an engine scope to the
> > > IP block soft reset
> > >
> > > On Wed, 2026-10-28 at 16:56 +0800, Jesse Zhang wrote:
> > > > Some IP blocks have engines that the hardware can reset on their
> > > > own, eg. the graphics and compute engines of the GFX IP
> > > > (CP_SOFT_RESET_CNTL.GFX_ONLY_SOFT_RESET /
> CMP_ONLY_SOFT_RESET).
> > > > Resetting
> > > > only the engine of a hung ring leaves the work on the other
> > > > engines alone, which the IP block soft reset can't do, as it
> > > > resets every ring of the IP block.
> > > >
> > > > Add an optional soft_reset_engine() IP block callback, which
> > > > resets one engine of the IP block, or all of them, and recovers
> > > > the user queues on it, and amdgpu_device_ip_soft_reset_engine()
> > > > around it.
> > > > It
> > > > only stops, backs up and replays the rings of that engine, and it
> > > > is also reachable without a guilty kernel ring, for a hung user
> > > > queue.
> > > >
> > > > For a single engine, it only takes the reset domain for read: KFD
> > > > only try-locks it for read before it adds or removes queues, and
> > > > would fail those calls with the write lock although the other
> > > > engines aren't reset.
> > > >
> > > > amdgpu_device_ip_soft_reset() uses it when the IP block implements
> > > > it, for the engine of the guilty ring.
> > > >
> > > > v3: replaces the separate engine reset type and helper
> > > >     (Timur).
> > > >
> > > > Suggested-by: Timur Kristóf <timur.kristof@gmail.com>
> > > > Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> > > > ---
> > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c   | 116
> > > > +++++++++++++++++++++++
> > > >  drivers/gpu/drm/amd/amdgpu/amdgpu_ip.h   |   6 ++
> > > >  drivers/gpu/drm/amd/include/amd_shared.h |  18 ++++
> > > >  3 files changed, 140 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > > index 2eea18965fd1..e95f69dd3142 100644
> > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ip.c
> > > > @@ -418,6 +418,111 @@ static u32 amdgpu_ring_mask_from_ip(const
> > > enum
> > > > amd_ip_block_type ip_type)
> > > >     }
> > > >  }
> > > >
> > > > +/**
> > > > + * amdgpu_ring_mask_from_engine() - Find mask of ring types of
> > > > an
> > > > engine.
> > > > + *
> > > > + * @ip_type: The IP block type of the engine.
> > > > + * @engine: The engine whose rings you are looking for.
> > > > + */
> > > > +static u32 amdgpu_ring_mask_from_engine(const enum
> > > amd_ip_block_type
> > > > ip_type,
> > > > +                                   const enum amd_ip_engine
> > > > engine)
> > > > +{
> > > > +   switch (engine) {
> > > > +   case AMD_IP_ENGINE_GFX:
> > > > +           return BIT(AMDGPU_RING_TYPE_GFX);
> > > > +
> > > > +   case AMD_IP_ENGINE_COMPUTE:
> > > > +           return BIT(AMDGPU_RING_TYPE_COMPUTE);
> > > > +
> > > > +   default:
> > > > +           return amdgpu_ring_mask_from_ip(ip_type);
> > > > +   }
> > > > +}
> > > > +
> > > > +/**
> > > > + * amdgpu_engine_from_ring() - Find the engine of a ring type.
> > > > + *
> > > > + * @ring_type: The ring type whose engine you are looking for.
> > > > + */
> > > > +static enum amd_ip_engine amdgpu_engine_from_ring(const enum
> > > > amdgpu_ring_type ring_type)
> > > > +{
> > > > +   switch (ring_type) {
> > > > +   case AMDGPU_RING_TYPE_GFX:
> > > > +           return AMD_IP_ENGINE_GFX;
> > > > +
> > > > +   case AMDGPU_RING_TYPE_COMPUTE:
> > > > +           return AMD_IP_ENGINE_COMPUTE;
> > > > +
> > > > +   default:
> > > > +           return AMD_IP_ENGINE_ALL;
> > > > +   }
> > > > +}
> > > > +
> > > > +static const char *const amdgpu_ip_engine_names[] = {
> > > > +   [AMD_IP_ENGINE_ALL] = "all",
> > > > +   [AMD_IP_ENGINE_GFX] = "gfx",
> > > > +   [AMD_IP_ENGINE_COMPUTE] = "compute", };
> > >
> > > No need to add a new enum.
> > > This is redundant with amdgpu_ring_type.
> > Agreed. In v4 the callback takes the ring type of the hung queue.
>
> Sounds good, thanks!
>
> > >
> > >
> > > > +
> > > > +/**
> > > > + * amdgpu_device_ip_soft_reset_engine() - Soft reset one engine
> > > > of
> > > > an IP block.
> > > > + *
> > > > + * @adev: amdgpu device
> > > > + * @ip_type: The IP block type.
> > > > + * @engine: The engine to reset, or AMD_IP_ENGINE_ALL for the
> > > > whole
> > > > IP block.
> > > > + * @guilty_ring: The ring which is guilty of causing a reset, or
> > > > NULL.
> > > > + * @guilty_fence: The fence which didn't signal on @guilty_ring.
> > > > + * @guilty_queue: The user queue which is guilty of causing a
> > > > reset,
> > > > or NULL.
> > > > + */
> > > > +int amdgpu_device_ip_soft_reset_engine(struct amdgpu_device
> > > > *adev,
> > > > +                                  enum amd_ip_block_type
> > > > ip_type,
> > > > +                                  enum amd_ip_engine engine,
> > > > +                                  struct amdgpu_ring
> > > > *guilty_ring,
> > > > +                                  struct amdgpu_fence
> > > > *guilty_fence,
> > > > +                                  struct amdgpu_usermode_queue
> > > > *guilty_queue)
> > >
> > > Please don't add a separate function.
> > > This duplicates a lot of code that shouldn't be duplicated.
> > >
> > > Instead, I suggest to just add the new functionality to
> > > amdgpu_device_ip_soft_reset(), for example like this:
> > >
> > > if (ip_block->version->funcs->soft_reset_engine)
> > >   ip_block->version->funcs->soft_reset_engine(...);
> > > else if (ip_block->version->funcs->soft_reset)
> > >   ip_block->version->funcs->soft_reset(...);
> >
> > OK, I'll fold it into amdgpu_device_ip_soft_reset(). The separate
> > entry point was only there for the user queue caller (patch 13): a
> > hung gfx user queue has no kernel ring, and
> > amdgpu_device_ip_soft_reset() gets the device and the IP block from
> > guilty_ring. So the signature would
> > become:
> >
> >         int amdgpu_device_ip_soft_reset(struct amdgpu_device *adev,
> >                                         enum amdgpu_ring_type
> > ring_type,
> >                                         struct amdgpu_ring
> > *guilty_ring,
> >                                         struct amdgpu_fence
> > *guilty_fence,
> >                                         struct amdgpu_usermode_queue
> > *guilty_queue);
> >
> > amdgpu_job_timedout() passes (ring->adev, ring->funcs->type, ring,
> > job->hw_fence, NULL), and the user queue reset passes (adev,
> > AMDGPU_RING_TYPE_GFX, NULL, NULL, queue). The body stays the same
> as
> > today. When the IP block has soft_reset_engine, it uses that callback,
> > only the rings of that type, and the reset domain for read:
> >
> >         if (funcs->soft_reset_engine) {
> >                 ring_type_mask = BIT(ring_type);
> >                 down_read(&adev->reset_domain->sem);
> >         } else {
> >                 if (!guilty_ring || !funcs->soft_reset)
> >                         return -EOPNOTSUPP;
> >                 ring_type_mask = amdgpu_ring_mask_from_ip(ip_type);
> >                 amdgpu_device_lock_reset_domain(adev->reset_domain);
> >         }
> >
> >         amdgpu_multi_ring_reset_helper_begin(adev, ring_type_mask,
> >                                              guilty_ring,
> > guilty_fence);
> >         if (funcs->soft_reset_engine)
> >                 r = funcs->soft_reset_engine(ip_block, ring_type,
> > guilty_queue);
> >         else
> >                 r = funcs->soft_reset(ip_block);
> >         r = amdgpu_multi_ring_reset_helper_end(adev, ring_type_mask,
> >                                                guilty_ring,
> > guilty_fence, r);
> >
> >         if (funcs->soft_reset_engine)
> >                 up_read(&adev->reset_domain->sem);
> >         else
> >                 amdgpu_device_unlock_reset_domain(adev-
> > >reset_domain);
> >
> > Does the change work for you?
>
> Yes, that sounds good to me.
> I just have one question about it:
>
> > the user queue reset passes (adev,
> > AMDGPU_RING_TYPE_GFX, NULL, NULL, queue)
>
> Wouldn't the user queue reset still need to pass the guilty fence from the user
> queue? Or is that not possible?
The guilty fence argument is an amdgpu_fence used to recover a kernel
ring. User queues have their own amdgpu_userq_fence type, which the
multi-ring helpers cannot handle.

The user queue path passes the guilty queue instead. After the engine
reset, amdgpu_userq_restore_hqds() unmaps it, completes its pending
fences with -ECANCELED, and marks it hung. The other user queues are
remapped with their saved state.

Thanks
Jesse
>
> Thanks,
> Timur

Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence at the end of a multi ring reset

[Timur Kristóf]

On Thu, 2026-11-05 at 02:09 +0000, Zhang, Jesse(Jie) wrote:
> AMD General
> 
> > -----Original Message-----
> > From: Timur Kristóf <timur.kristof@gmail.com>
> > Sent: Thursday, November 5, 2026 6:47 AM
> > To: Zhang, Jesse(Jie) <Jesse.Zhang@amd.com>; amd-
> > gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig,
> > Christian
> > <Christian.Koenig@amd.com>; Lazar, Lijo <Lijo.Lazar@amd.com>; Mario
> > Sopena-Novales <Mario.Novales@amd.com>; Rastogi, Manu
> > <Manu.Rastogi@amd.com>; Zhu, Jiongyu (Joey) <Jiongyu.Zhu@amd.com>
> > Subject: Re: [PATCH v3 02/15] drm/amdgpu: complete the guilty fence
> > at the
> > end of a multi ring reset
> > 
> > On Wed, 2026-11-04 at 02:46 +0000, Zhang, Jesse(Jie) wrote:
> > > > > +           r = amdgpu_ring_reset_helper_end(ring, ring ==
> > > > > guilty_ring ?
> > > > > +                                            guilty_fence :
> > > > > ring->guilty_fence);
> > > > 
> > > > Can you help me understand what was wrong with the line before?
> > > > I think guilty_fence == ring->guilty_fence when ring ==
> > > > guilty_ring
> > > > no?
> > > 
> > > Usually yes, but not when the soft reset follows a failed ring
> > > reset
> > > of the same job.
> > >  amdgpu_ring_reset(ring, vmid, job->hw_fence)
> > >    -> amdgpu_ring_reset_helper_begin(ring, F)
> > >    -> amdgpu_ring_backup_unprocessed_commands(ring, F)
> > >       sets ring->guilty_fence = F.
> > > 
> > > So by the time the end helper runs, ring->guilty_fence no longer
> > > equals the guilty fence: the begin helper cleared it.
> > 
> > I think I see the problem.
> > 
> > I re-read the queue reset code and the helpers. During a queue
> > reset, it makes
> > a backup and saves the guilty fence but if the queue reset fails,
> > it doesn't
> > restore the contents, and on a subsequent call it clears the guilty
> > fence and the
> > backed up contents.
> > 
> > Unfortunately I think this commit doesn't completely fix that
> > because the reset
> > begin helper will still clear the backed up contents on the second
> > call (sets
> > ring_backup_entries_to_copy = 0).
> > 
> > I think the correct fix would be to make sure we don't repeat the
> > ring reset
> > begin helper:
> > https://gitlab.freedesktop.org/Venemo/linux/-
> > /commit/86f2bf222caee0da267f9d9e12362513fe4f7f9e
> > Please take a look and if you agree feel free to take that patch in
> > your series.
> Right, the second backup also drops the commands of the other jobs on
> the
> guilty ring, and skipping the begin helper is the better fix. With
> it,
> ring->guilty_fence also stays the guilty fence, so 02/15 isn't needed
> anymore.
> 
> I think the condition in your patch is inverted though:
> 
>         if (guilty_fence && ring->guilty_fence == guilty_fence)
>                 amdgpu_ring_reset_helper_begin(ring,
> ring_guilty_fence);
> 
> calls the begin helper only for a ring that was already backed up
> with
> this guilty fence. The other rings, and the guilty ring of a soft
> reset
> without a failed ring reset before it, aren't backed up anymore. The
> latter would even replay a stale backup. The guilty ring after a
> failed
> ring reset is the one ring that still goes through the begin helper.
> I
> think you meant:
> 
>         /* A failed ring reset of the same job already backed it up.
> */
>         if (ring != guilty_ring || ring->guilty_fence !=
> guilty_fence)
>                 amdgpu_ring_reset_helper_begin(ring,
> ring_guilty_fence);
> 
> If that's what you meant, I'll take your patch with that change in
> place
> of 02 in v4.
> 

Yes, thank you for the correction.