[PATCH 0/5] drm/amdgpu/sdma: Improve existing SDMA queue resets

[PATCH 0/5] drm/amdgpu/sdma: Improve existing SDMA queue resets

[Timur Kristóf]

Improve SDMA queue reset for SDMA v4.4.2, v5.0, v5.2
and also moves some functionality from backend-specific
code to common code in the amdgpu_sdma.c file.

This prepares for implementing SDMA queue resets
on more generations, as the same logic can be shared
with all SDMA HW generations that don't use the MES.
The actual queue reset implementations for various
older SDMA versions will come in subsequent patch series
after this one is accepted.

Timur Kristóf (5):
  drm/amdgpu/sdma: Clear SDMA rings after reset before starting them
  drm/amdgpu/sdma: Remove unimplemented soft_reset() for SDMA and SI DMA
  drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code
  drm/amdgpu/sdma: Use common SDMA legacy queue reset on SDMA v4.4.2
  drm/amdgpu/sdma: In legacy queue reset function, check if KFD is
    initialized

 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 54 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  4 ++
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   |  8 ----
 drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 26 +-----------
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 33 +--------------
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   | 25 +----------
 drivers/gpu/drm/amd/amdgpu/si_dma.c      |  7 ---
 7 files changed, 62 insertions(+), 95 deletions(-)

-- 
2.55.0

[PATCH 1/5] drm/amdgpu/sdma: Clear SDMA rings after reset before starting them

[Timur Kristóf]

The ring contains commands that were emitted before the reset.
These need to be cleared to make sure the HW doesn't execute
them, because they are garbage at this point.

Note that the ring reset helpers will re-emit the commands
that are necessary after the reset.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index fbac732f3e01..66f278f77f71 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -593,6 +593,18 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id,
 		goto exit;
 	}
 
+	amdgpu_ring_clear_ring(gfx_ring);
+	gfx_ring->wptr = 0;
+	atomic64_set((atomic64_t *)gfx_ring->wptr_cpu_addr, 0);
+	atomic64_set((atomic64_t *)gfx_ring->rptr_cpu_addr, 0);
+
+	if (adev->sdma.has_page_queue) {
+		amdgpu_ring_clear_ring(page_ring);
+		page_ring->wptr = 0;
+		atomic64_set((atomic64_t *)page_ring->wptr_cpu_addr, 0);
+		atomic64_set((atomic64_t *)page_ring->rptr_cpu_addr, 0);
+	}
+
 	if (sdma_instance->funcs->start_kernel_queue) {
 		sdma_instance->funcs->start_kernel_queue(gfx_ring);
 		if (adev->sdma.has_page_queue)
-- 
2.55.0

[PATCH 2/5] drm/amdgpu/sdma: Remove unimplemented soft_reset() for SDMA and SI DMA

[Timur Kristóf]

These functions are not called from anywhere
and don't do anything. Let's delete them.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   | 8 --------
 drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 8 --------
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 8 --------
 drivers/gpu/drm/amd/amdgpu/si_dma.c      | 7 -------
 4 files changed, 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index a0f19f7b39e6..bf71f99f7307 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -2046,13 +2046,6 @@ static int sdma_v4_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
-static int sdma_v4_0_soft_reset(struct amdgpu_ip_block *ip_block)
-{
-	/* todo */
-
-	return 0;
-}
-
 static int sdma_v4_0_set_trap_irq_state(struct amdgpu_device *adev,
 					struct amdgpu_irq_src *source,
 					unsigned type,
@@ -2398,7 +2391,6 @@ const struct amd_ip_funcs sdma_v4_0_ip_funcs = {
 	.suspend = sdma_v4_0_suspend,
 	.resume = sdma_v4_0_resume,
 	.wait_for_idle = sdma_v4_0_wait_for_idle,
-	.soft_reset = sdma_v4_0_soft_reset,
 	.set_clockgating_state = sdma_v4_0_set_clockgating_state,
 	.set_powergating_state = sdma_v4_0_set_powergating_state,
 	.get_clockgating_state = sdma_v4_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
index 1a2810b6f27c..72ea9db939b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
@@ -1646,13 +1646,6 @@ static int sdma_v4_4_2_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
-static int sdma_v4_4_2_soft_reset(struct amdgpu_ip_block *ip_block)
-{
-	/* todo */
-
-	return 0;
-}
-
 static bool sdma_v4_4_2_is_queue_selected(struct amdgpu_device *adev, uint32_t instance_id, bool is_page_queue)
 {
 	uint32_t reg_offset = is_page_queue ? regSDMA_PAGE_CONTEXT_STATUS : regSDMA_GFX_CONTEXT_STATUS;
@@ -2104,7 +2097,6 @@ const struct amd_ip_funcs sdma_v4_4_2_ip_funcs = {
 	.suspend = sdma_v4_4_2_suspend,
 	.resume = sdma_v4_4_2_resume,
 	.wait_for_idle = sdma_v4_4_2_wait_for_idle,
-	.soft_reset = sdma_v4_4_2_soft_reset,
 	.set_clockgating_state = sdma_v4_4_2_set_clockgating_state,
 	.set_powergating_state = sdma_v4_4_2_set_powergating_state,
 	.get_clockgating_state = sdma_v4_4_2_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 1a022a251011..0da54c335822 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1518,13 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
-static int sdma_v5_0_soft_reset(struct amdgpu_ip_block *ip_block)
-{
-	/* todo */
-
-	return 0;
-}
-
 static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
 				 unsigned int vmid,
 				 struct amdgpu_fence *timedout_fence)
@@ -1902,7 +1895,6 @@ static const struct amd_ip_funcs sdma_v5_0_ip_funcs = {
 	.suspend = sdma_v5_0_suspend,
 	.resume = sdma_v5_0_resume,
 	.wait_for_idle = sdma_v5_0_wait_for_idle,
-	.soft_reset = sdma_v5_0_soft_reset,
 	.set_clockgating_state = sdma_v5_0_set_clockgating_state,
 	.set_powergating_state = sdma_v5_0_set_powergating_state,
 	.get_clockgating_state = sdma_v5_0_get_clockgating_state,
diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
index edebd9109fd4..1df30779dcbe 100644
--- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
+++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
@@ -594,12 +594,6 @@ static int si_dma_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
-static int si_dma_soft_reset(struct amdgpu_ip_block *ip_block)
-{
-	drm_info(adev_to_drm(ip_block->adev), "si_dma_soft_reset --- not implemented !!!!!!!\n");
-	return 0;
-}
-
 static int si_dma_set_trap_irq_state(struct amdgpu_device *adev,
 					struct amdgpu_irq_src *src,
 					unsigned type,
@@ -726,7 +720,6 @@ static const struct amd_ip_funcs si_dma_ip_funcs = {
 	.suspend = si_dma_suspend,
 	.resume = si_dma_resume,
 	.wait_for_idle = si_dma_wait_for_idle,
-	.soft_reset = si_dma_soft_reset,
 	.set_clockgating_state = si_dma_set_clockgating_state,
 	.set_powergating_state = si_dma_set_powergating_state,
 };
-- 
2.55.0

[PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Timur Kristóf]

The code was exactly the same between SDMA v5.0 and v5.2
furthermore the exact same implementation can be shared
between all SDMA versions that don't use MES.

Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 37 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  4 +++
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 25 +---------------
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   | 25 +---------------
 4 files changed, 43 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index 66f278f77f71..9eebd8380834 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -635,3 +635,40 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id,
 
 	return ret;
 }
+
+/**
+ * amdgpu_sdma_reset_queue_legacy() - Reset legacy SDMA queue after timeout (without MES)
+ *
+ * @ring: Pointer to the ring of the SDMA queue
+ * @vmid: VMID of the timed out job
+ * @timedout_fence: Fence of the timed out job
+ *
+ * Common implementation for resetting SDMA queues without MES (legacy).
+ * This relies on the proper amdgpu_sdma_funcs to be set up
+ * for the given ring.
+ *
+ * Applicable to SDMA versions that don't rely on the MES yet,
+ * that is all versions up to SDMA v5.x and older.
+ */
+int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
+				   unsigned int vmid,
+				   struct amdgpu_fence *timedout_fence)
+{
+	struct amdgpu_device *adev = ring->adev;
+	int r;
+
+	if (ring->me >= adev->sdma.num_instances) {
+		dev_err(adev->dev, "sdma instance not found\n");
+		return -EINVAL;
+	}
+
+	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
+
+	amdgpu_amdkfd_suspend(adev, true);
+	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
+	amdgpu_amdkfd_resume(adev, true);
+	if (r)
+		return r;
+
+	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
index 4f4e56022c97..7c4e145ca0c1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
@@ -160,6 +160,10 @@ struct amdgpu_buffer_funcs {
 int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id,
 			     bool caller_handles_kernel_queues);
 
+int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
+				   unsigned int vmid,
+				   struct amdgpu_fence *timedout_fence);
+
 #define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t) (adev)->mman.buffer_funcs->emit_copy_buffer((ib),  (s), (d), (b), (t))
 #define amdgpu_emit_fill_buffer(adev, ib, s, d, b) (adev)->mman.buffer_funcs->emit_fill_buffer((ib), (s), (d), (b))
 
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 0da54c335822..76f8765fb175 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1518,29 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
-static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
-				 unsigned int vmid,
-				 struct amdgpu_fence *timedout_fence)
-{
-	struct amdgpu_device *adev = ring->adev;
-	int r;
-
-	if (ring->me >= adev->sdma.num_instances) {
-		dev_err(adev->dev, "sdma instance not found\n");
-		return -EINVAL;
-	}
-
-	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
-
-	amdgpu_amdkfd_suspend(adev, true);
-	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
-	amdgpu_amdkfd_resume(adev, true);
-	if (r)
-		return r;
-
-	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
-}
-
 static int sdma_v5_0_stop_queue(struct amdgpu_ring *ring)
 {
 	u32 f32_cntl, freeze, cntl, stat1_reg;
@@ -1936,7 +1913,7 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
 	.emit_reg_write_reg_wait = sdma_v5_0_ring_emit_reg_write_reg_wait,
 	.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
 	.preempt_ib = sdma_v5_0_ring_preempt_ib,
-	.reset = sdma_v5_0_reset_queue,
+	.reset = amdgpu_sdma_reset_queue_legacy,
 };
 
 static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index 5543e381dcca..e7f4b74f27b4 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -1436,29 +1436,6 @@ static int sdma_v5_2_wait_for_idle(struct amdgpu_ip_block *ip_block)
 	return -ETIMEDOUT;
 }
 
-static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring,
-				 unsigned int vmid,
-				 struct amdgpu_fence *timedout_fence)
-{
-	struct amdgpu_device *adev = ring->adev;
-	int r;
-
-	if (ring->me >= adev->sdma.num_instances) {
-		dev_err(adev->dev, "sdma instance not found\n");
-		return -EINVAL;
-	}
-
-	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
-
-	amdgpu_amdkfd_suspend(adev, true);
-	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
-	amdgpu_amdkfd_resume(adev, true);
-	if (r)
-		return r;
-
-	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
-}
-
 static int sdma_v5_2_stop_queue(struct amdgpu_ring *ring)
 {
 	u32 f32_cntl, freeze, cntl, stat1_reg;
@@ -1951,7 +1928,7 @@ static const struct amdgpu_ring_funcs sdma_v5_2_ring_funcs = {
 	.emit_reg_write_reg_wait = sdma_v5_2_ring_emit_reg_write_reg_wait,
 	.init_cond_exec = sdma_v5_2_ring_init_cond_exec,
 	.preempt_ib = sdma_v5_2_ring_preempt_ib,
-	.reset = sdma_v5_2_reset_queue,
+	.reset = amdgpu_sdma_reset_queue_legacy,
 };
 
 static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)
-- 
2.55.0

[PATCH 4/5] drm/amdgpu/sdma: Use common SDMA legacy queue reset on SDMA v4.4.2

[Timur Kristóf]

Besides sharing code, this additionally adds support for
restoring queue contents after the reset.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
index 72ea9db939b4..023821848531 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
@@ -1655,20 +1655,6 @@ static bool sdma_v4_4_2_is_queue_selected(struct amdgpu_device *adev, uint32_t i
 	return (context_status & SDMA_GFX_CONTEXT_STATUS__SELECTED_MASK) != 0;
 }
 
-static int sdma_v4_4_2_reset_queue(struct amdgpu_ring *ring,
-				   unsigned int vmid,
-				   struct amdgpu_fence *timedout_fence)
-{
-	struct amdgpu_device *adev = ring->adev;
-	u32 id = ring->me;
-	int r;
-
-	amdgpu_amdkfd_suspend(adev, true);
-	r = amdgpu_sdma_reset_engine(adev, id, false);
-	amdgpu_amdkfd_resume(adev, true);
-	return r;
-}
-
 static int sdma_v4_4_2_stop_queue(struct amdgpu_ring *ring)
 {
 	struct amdgpu_device *adev = ring->adev;
@@ -2133,7 +2119,7 @@ static const struct amdgpu_ring_funcs sdma_v4_4_2_ring_funcs = {
 	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
 	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
 	.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
-	.reset = sdma_v4_4_2_reset_queue,
+	.reset = amdgpu_sdma_reset_queue_legacy,
 };
 
 static const struct amdgpu_ring_funcs sdma_v4_4_2_page_ring_funcs = {
@@ -2165,7 +2151,7 @@ static const struct amdgpu_ring_funcs sdma_v4_4_2_page_ring_funcs = {
 	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
 	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
 	.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
-	.reset = sdma_v4_4_2_reset_queue,
+	.reset = amdgpu_sdma_reset_queue_legacy,
 };
 
 static void sdma_v4_4_2_set_ring_funcs(struct amdgpu_device *adev)
-- 
2.55.0

[PATCH 5/5] drm/amdgpu/sdma: In legacy queue reset function, check if KFD is initialized

[Timur Kristóf]

Avoid calling amdgpu_amdkfd_suspend() and amdgpu_amdkfd_resume()
when the KFD device is not initialized.

This allows the function to be used on GPUs where KFD is
not supported or the support is hidden behind an experimental
flag, such as CIK, in the future.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index 9eebd8380834..8798caed1ff1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -664,9 +664,14 @@ int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
 
 	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
 
-	amdgpu_amdkfd_suspend(adev, true);
+	if (adev->kfd.dev)
+		amdgpu_amdkfd_suspend(adev, true);
+
 	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
-	amdgpu_amdkfd_resume(adev, true);
+
+	if (adev->kfd.dev)
+		amdgpu_amdkfd_resume(adev, true);
+
 	if (r)
 		return r;
 
-- 
2.55.0

Re: [PATCH 5/5] drm/amdgpu/sdma: In legacy queue reset function, check if KFD is initialized

[Lazar, Lijo]

On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> Avoid calling amdgpu_amdkfd_suspend() and amdgpu_amdkfd_resume()
> when the KFD device is not initialized.
> 
> This allows the function to be used on GPUs where KFD is
> not supported or the support is hidden behind an experimental
> flag, such as CIK, in the future.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 9eebd8380834..8798caed1ff1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -664,9 +664,14 @@ int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
>   
>   	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
>   
> -	amdgpu_amdkfd_suspend(adev, true);
> +	if (adev->kfd.dev)
> +		amdgpu_amdkfd_suspend(adev, true);

The right place would have been suspend/resume functions and that check 
is already there.

Thanks,
Lijo

> +
>   	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> -	amdgpu_amdkfd_resume(adev, true);
> +
> +	if (adev->kfd.dev)
> +		amdgpu_amdkfd_resume(adev, true);
> +
>   	if (r)
>   		return r;
>   

Re: [PATCH 4/5] drm/amdgpu/sdma: Use common SDMA legacy queue reset on SDMA v4.4.2

[Lazar, Lijo]

On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> Besides sharing code, this additionally adds support for
> restoring queue contents after the reset.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 18 ++----------------
>   1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> index 72ea9db939b4..023821848531 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> @@ -1655,20 +1655,6 @@ static bool sdma_v4_4_2_is_queue_selected(struct amdgpu_device *adev, uint32_t i
>   	return (context_status & SDMA_GFX_CONTEXT_STATUS__SELECTED_MASK) != 0;
>   }
>   
> -static int sdma_v4_4_2_reset_queue(struct amdgpu_ring *ring,
> -				   unsigned int vmid,
> -				   struct amdgpu_fence *timedout_fence)
> -{
> -	struct amdgpu_device *adev = ring->adev;
> -	u32 id = ring->me;
> -	int r;
> -
> -	amdgpu_amdkfd_suspend(adev, true);
> -	r = amdgpu_sdma_reset_engine(adev, id, false);

The common one passes true, so this doesn't look right.
  	amdgpu_sdma_reset_engine(adev, ring->me, true);


Thanks,
Lijo

> -	amdgpu_amdkfd_resume(adev, true);
> -	return r;
> -}
> -
>   static int sdma_v4_4_2_stop_queue(struct amdgpu_ring *ring)
>   {
>   	struct amdgpu_device *adev = ring->adev;
> @@ -2133,7 +2119,7 @@ static const struct amdgpu_ring_funcs sdma_v4_4_2_ring_funcs = {
>   	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
>   	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
>   	.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> -	.reset = sdma_v4_4_2_reset_queue,
> +	.reset = amdgpu_sdma_reset_queue_legacy,
>   };
>   
>   static const struct amdgpu_ring_funcs sdma_v4_4_2_page_ring_funcs = {
> @@ -2165,7 +2151,7 @@ static const struct amdgpu_ring_funcs sdma_v4_4_2_page_ring_funcs = {
>   	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
>   	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
>   	.emit_reg_write_reg_wait = amdgpu_ring_emit_reg_write_reg_wait_helper,
> -	.reset = sdma_v4_4_2_reset_queue,
> +	.reset = amdgpu_sdma_reset_queue_legacy,
>   };
>   
>   static void sdma_v4_4_2_set_ring_funcs(struct amdgpu_device *adev)

Re: [PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Lazar, Lijo]

On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> The code was exactly the same between SDMA v5.0 and v5.2
> furthermore the exact same implementation can be shared
> between all SDMA versions that don't use MES.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 37 ++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  4 +++
>   drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 25 +---------------
>   drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   | 25 +---------------
>   4 files changed, 43 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 66f278f77f71..9eebd8380834 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -635,3 +635,40 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id,
>   
>   	return ret;
>   }
> +
> +/**
> + * amdgpu_sdma_reset_queue_legacy() - Reset legacy SDMA queue after timeout (without MES)
> + *
> + * @ring: Pointer to the ring of the SDMA queue
> + * @vmid: VMID of the timed out job
> + * @timedout_fence: Fence of the timed out job
> + *
> + * Common implementation for resetting SDMA queues without MES (legacy).
> + * This relies on the proper amdgpu_sdma_funcs to be set up
> + * for the given ring.
> + *
> + * Applicable to SDMA versions that don't rely on the MES yet,
> + * that is all versions up to SDMA v5.x and older.
> + */
> +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> +				   unsigned int vmid,
> +				   struct amdgpu_fence *timedout_fence)
> +{
> +	struct amdgpu_device *adev = ring->adev;
> +	int r;
> +
> +	if (ring->me >= adev->sdma.num_instances) {
> +		dev_err(adev->dev, "sdma instance not found\n");
> +		return -EINVAL;
> +	}
> +
> +	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> +
> +	amdgpu_amdkfd_suspend(adev, true);
> +	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> +	amdgpu_amdkfd_resume(adev, true);
> +	if (r)
> +		return r;
> +
> +	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> +}

Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2 
is better. This may not work in the same way for all legacy queues.

Thanks,
Lijo

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> index 4f4e56022c97..7c4e145ca0c1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> @@ -160,6 +160,10 @@ struct amdgpu_buffer_funcs {
>   int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id,
>   			     bool caller_handles_kernel_queues);
>   
> +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> +				   unsigned int vmid,
> +				   struct amdgpu_fence *timedout_fence);
> +
>   #define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t) (adev)->mman.buffer_funcs->emit_copy_buffer((ib),  (s), (d), (b), (t))
>   #define amdgpu_emit_fill_buffer(adev, ib, s, d, b) (adev)->mman.buffer_funcs->emit_fill_buffer((ib), (s), (d), (b))
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 0da54c335822..76f8765fb175 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -1518,29 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   	return -ETIMEDOUT;
>   }
>   
> -static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
> -				 unsigned int vmid,
> -				 struct amdgpu_fence *timedout_fence)
> -{
> -	struct amdgpu_device *adev = ring->adev;
> -	int r;
> -
> -	if (ring->me >= adev->sdma.num_instances) {
> -		dev_err(adev->dev, "sdma instance not found\n");
> -		return -EINVAL;
> -	}
> -
> -	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> -
> -	amdgpu_amdkfd_suspend(adev, true);
> -	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> -	amdgpu_amdkfd_resume(adev, true);
> -	if (r)
> -		return r;
> -
> -	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> -}
> -
>   static int sdma_v5_0_stop_queue(struct amdgpu_ring *ring)
>   {
>   	u32 f32_cntl, freeze, cntl, stat1_reg;
> @@ -1936,7 +1913,7 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
>   	.emit_reg_write_reg_wait = sdma_v5_0_ring_emit_reg_write_reg_wait,
>   	.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
>   	.preempt_ib = sdma_v5_0_ring_preempt_ib,
> -	.reset = sdma_v5_0_reset_queue,
> +	.reset = amdgpu_sdma_reset_queue_legacy,
>   };
>   
>   static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> index 5543e381dcca..e7f4b74f27b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -1436,29 +1436,6 @@ static int sdma_v5_2_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   	return -ETIMEDOUT;
>   }
>   
> -static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring,
> -				 unsigned int vmid,
> -				 struct amdgpu_fence *timedout_fence)
> -{
> -	struct amdgpu_device *adev = ring->adev;
> -	int r;
> -
> -	if (ring->me >= adev->sdma.num_instances) {
> -		dev_err(adev->dev, "sdma instance not found\n");
> -		return -EINVAL;
> -	}
> -
> -	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> -
> -	amdgpu_amdkfd_suspend(adev, true);
> -	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> -	amdgpu_amdkfd_resume(adev, true);
> -	if (r)
> -		return r;
> -
> -	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> -}
> -
>   static int sdma_v5_2_stop_queue(struct amdgpu_ring *ring)
>   {
>   	u32 f32_cntl, freeze, cntl, stat1_reg;
> @@ -1951,7 +1928,7 @@ static const struct amdgpu_ring_funcs sdma_v5_2_ring_funcs = {
>   	.emit_reg_write_reg_wait = sdma_v5_2_ring_emit_reg_write_reg_wait,
>   	.init_cond_exec = sdma_v5_2_ring_init_cond_exec,
>   	.preempt_ib = sdma_v5_2_ring_preempt_ib,
> -	.reset = sdma_v5_2_reset_queue,
> +	.reset = amdgpu_sdma_reset_queue_legacy,
>   };
>   
>   static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)

Re: [PATCH 2/5] drm/amdgpu/sdma: Remove unimplemented soft_reset() for SDMA and SI DMA

[Lazar, Lijo]

On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> These functions are not called from anywhere
> and don't do anything. Let's delete them.
> 
> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>

Reviewed-by: Lijo Lazar <lijo.lazar@amd.com>

Thanks,
Lijo

> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   | 8 --------
>   drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 8 --------
>   drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 8 --------
>   drivers/gpu/drm/amd/amdgpu/si_dma.c      | 7 -------
>   4 files changed, 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index a0f19f7b39e6..bf71f99f7307 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -2046,13 +2046,6 @@ static int sdma_v4_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   	return -ETIMEDOUT;
>   }
>   
> -static int sdma_v4_0_soft_reset(struct amdgpu_ip_block *ip_block)
> -{
> -	/* todo */
> -
> -	return 0;
> -}
> -
>   static int sdma_v4_0_set_trap_irq_state(struct amdgpu_device *adev,
>   					struct amdgpu_irq_src *source,
>   					unsigned type,
> @@ -2398,7 +2391,6 @@ const struct amd_ip_funcs sdma_v4_0_ip_funcs = {
>   	.suspend = sdma_v4_0_suspend,
>   	.resume = sdma_v4_0_resume,
>   	.wait_for_idle = sdma_v4_0_wait_for_idle,
> -	.soft_reset = sdma_v4_0_soft_reset,
>   	.set_clockgating_state = sdma_v4_0_set_clockgating_state,
>   	.set_powergating_state = sdma_v4_0_set_powergating_state,
>   	.get_clockgating_state = sdma_v4_0_get_clockgating_state,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> index 1a2810b6f27c..72ea9db939b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> @@ -1646,13 +1646,6 @@ static int sdma_v4_4_2_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   	return -ETIMEDOUT;
>   }
>   
> -static int sdma_v4_4_2_soft_reset(struct amdgpu_ip_block *ip_block)
> -{
> -	/* todo */
> -
> -	return 0;
> -}
> -
>   static bool sdma_v4_4_2_is_queue_selected(struct amdgpu_device *adev, uint32_t instance_id, bool is_page_queue)
>   {
>   	uint32_t reg_offset = is_page_queue ? regSDMA_PAGE_CONTEXT_STATUS : regSDMA_GFX_CONTEXT_STATUS;
> @@ -2104,7 +2097,6 @@ const struct amd_ip_funcs sdma_v4_4_2_ip_funcs = {
>   	.suspend = sdma_v4_4_2_suspend,
>   	.resume = sdma_v4_4_2_resume,
>   	.wait_for_idle = sdma_v4_4_2_wait_for_idle,
> -	.soft_reset = sdma_v4_4_2_soft_reset,
>   	.set_clockgating_state = sdma_v4_4_2_set_clockgating_state,
>   	.set_powergating_state = sdma_v4_4_2_set_powergating_state,
>   	.get_clockgating_state = sdma_v4_4_2_get_clockgating_state,
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 1a022a251011..0da54c335822 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -1518,13 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   	return -ETIMEDOUT;
>   }
>   
> -static int sdma_v5_0_soft_reset(struct amdgpu_ip_block *ip_block)
> -{
> -	/* todo */
> -
> -	return 0;
> -}
> -
>   static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
>   				 unsigned int vmid,
>   				 struct amdgpu_fence *timedout_fence)
> @@ -1902,7 +1895,6 @@ static const struct amd_ip_funcs sdma_v5_0_ip_funcs = {
>   	.suspend = sdma_v5_0_suspend,
>   	.resume = sdma_v5_0_resume,
>   	.wait_for_idle = sdma_v5_0_wait_for_idle,
> -	.soft_reset = sdma_v5_0_soft_reset,
>   	.set_clockgating_state = sdma_v5_0_set_clockgating_state,
>   	.set_powergating_state = sdma_v5_0_set_powergating_state,
>   	.get_clockgating_state = sdma_v5_0_get_clockgating_state,
> diff --git a/drivers/gpu/drm/amd/amdgpu/si_dma.c b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> index edebd9109fd4..1df30779dcbe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/si_dma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/si_dma.c
> @@ -594,12 +594,6 @@ static int si_dma_wait_for_idle(struct amdgpu_ip_block *ip_block)
>   	return -ETIMEDOUT;
>   }
>   
> -static int si_dma_soft_reset(struct amdgpu_ip_block *ip_block)
> -{
> -	drm_info(adev_to_drm(ip_block->adev), "si_dma_soft_reset --- not implemented !!!!!!!\n");
> -	return 0;
> -}
> -
>   static int si_dma_set_trap_irq_state(struct amdgpu_device *adev,
>   					struct amdgpu_irq_src *src,
>   					unsigned type,
> @@ -726,7 +720,6 @@ static const struct amd_ip_funcs si_dma_ip_funcs = {
>   	.suspend = si_dma_suspend,
>   	.resume = si_dma_resume,
>   	.wait_for_idle = si_dma_wait_for_idle,
> -	.soft_reset = si_dma_soft_reset,
>   	.set_clockgating_state = si_dma_set_clockgating_state,
>   	.set_powergating_state = si_dma_set_powergating_state,
>   };

Re: [PATCH 5/5] drm/amdgpu/sdma: In legacy queue reset function, check if KFD is initialized

[Timur Kristóf]

On Friday, August 28, 2026 4:11:47 PM Central European Summer Time Lazar, Lijo 
wrote:
> On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> > Avoid calling amdgpu_amdkfd_suspend() and amdgpu_amdkfd_resume()
> > when the KFD device is not initialized.
> > 
> > This allows the function to be used on GPUs where KFD is
> > not supported or the support is hidden behind an experimental
> > flag, such as CIK, in the future.
> > 
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> > 
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 9 +++++++--
> >   1 file changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c index
> > 9eebd8380834..8798caed1ff1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > @@ -664,9 +664,14 @@ int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring
> > *ring,> 
> >   	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > 
> > -	amdgpu_amdkfd_suspend(adev, true);
> > +	if (adev->kfd.dev)
> > +		amdgpu_amdkfd_suspend(adev, true);
> 
> The right place would have been suspend/resume functions and that check
> is already there.
> 
> Thanks,
> Lijo

I see. In that case we can just drop this patch.

> 
> > +
> > 
> >   	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > 
> > -	amdgpu_amdkfd_resume(adev, true);
> > +
> > +	if (adev->kfd.dev)
> > +		amdgpu_amdkfd_resume(adev, true);
> > +
> > 
> >   	if (r)
> >   	
> >   		return r;

Re: [PATCH 4/5] drm/amdgpu/sdma: Use common SDMA legacy queue reset on SDMA v4.4.2

[Timur Kristóf]

On Friday, August 28, 2026 4:16:15 PM Central European Summer Time Lazar, Lijo 
wrote:
> On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> > Besides sharing code, this additionally adds support for
> > restoring queue contents after the reset.
> > 
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> > 
> >   drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 18 ++----------------
> >   1 file changed, 2 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c index
> > 72ea9db939b4..023821848531 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> > @@ -1655,20 +1655,6 @@ static bool sdma_v4_4_2_is_queue_selected(struct
> > amdgpu_device *adev, uint32_t i> 
> >   	return (context_status & SDMA_GFX_CONTEXT_STATUS__SELECTED_MASK) 
!= 0;
> >   
> >   }
> > 
> > -static int sdma_v4_4_2_reset_queue(struct amdgpu_ring *ring,
> > -				   unsigned int vmid,
> > -				   struct amdgpu_fence 
*timedout_fence)
> > -{
> > -	struct amdgpu_device *adev = ring->adev;
> > -	u32 id = ring->me;
> > -	int r;
> > -
> > -	amdgpu_amdkfd_suspend(adev, true);
> > -	r = amdgpu_sdma_reset_engine(adev, id, false);
> 
> The common one passes true, so this doesn't look right.
>   	amdgpu_sdma_reset_engine(adev, ring->me, true);
> 
> 
> Thanks,
> Lijo

Can you please elaborate more on what you are suggesting here?
The caller of the queue reset is amdgpu_job_timedout() and that function 
already handles kernel queues.


> 
> > -	amdgpu_amdkfd_resume(adev, true);
> > -	return r;
> > -}
> > -
> > 
> >   static int sdma_v4_4_2_stop_queue(struct amdgpu_ring *ring)
> >   {
> >   
> >   	struct amdgpu_device *adev = ring->adev;
> > 
> > @@ -2133,7 +2119,7 @@ static const struct amdgpu_ring_funcs
> > sdma_v4_4_2_ring_funcs = {> 
> >   	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
> >   	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
> >   	.emit_reg_write_reg_wait = 
amdgpu_ring_emit_reg_write_reg_wait_helper,
> > 
> > -	.reset = sdma_v4_4_2_reset_queue,
> > +	.reset = amdgpu_sdma_reset_queue_legacy,
> > 
> >   };
> >   
> >   static const struct amdgpu_ring_funcs sdma_v4_4_2_page_ring_funcs = {
> > 
> > @@ -2165,7 +2151,7 @@ static const struct amdgpu_ring_funcs
> > sdma_v4_4_2_page_ring_funcs = {> 
> >   	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
> >   	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
> >   	.emit_reg_write_reg_wait = 
amdgpu_ring_emit_reg_write_reg_wait_helper,
> > 
> > -	.reset = sdma_v4_4_2_reset_queue,
> > +	.reset = amdgpu_sdma_reset_queue_legacy,
> > 
> >   };
> >   
> >   static void sdma_v4_4_2_set_ring_funcs(struct amdgpu_device *adev)

Re: [PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Timur Kristóf]

On Friday, August 28, 2026 4:20:19 PM Central European Summer Time Lazar, Lijo 
wrote:
> On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> > The code was exactly the same between SDMA v5.0 and v5.2
> > furthermore the exact same implementation can be shared
> > between all SDMA versions that don't use MES.
> > 
> > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > ---
> > 
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 37 ++++++++++++++++++++++++
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  4 +++
> >   drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 25 +---------------
> >   drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   | 25 +---------------
> >   4 files changed, 43 insertions(+), 48 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c index
> > 66f278f77f71..9eebd8380834 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > @@ -635,3 +635,40 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device
> > *adev, uint32_t instance_id,> 
> >   	return ret;
> >   
> >   }
> > 
> > +
> > +/**
> > + * amdgpu_sdma_reset_queue_legacy() - Reset legacy SDMA queue after
> > timeout (without MES) + *
> > + * @ring: Pointer to the ring of the SDMA queue
> > + * @vmid: VMID of the timed out job
> > + * @timedout_fence: Fence of the timed out job
> > + *
> > + * Common implementation for resetting SDMA queues without MES (legacy).
> > + * This relies on the proper amdgpu_sdma_funcs to be set up
> > + * for the given ring.
> > + *
> > + * Applicable to SDMA versions that don't rely on the MES yet,
> > + * that is all versions up to SDMA v5.x and older.
> > + */
> > +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> > +				   unsigned int vmid,
> > +				   struct amdgpu_fence 
*timedout_fence)
> > +{
> > +	struct amdgpu_device *adev = ring->adev;
> > +	int r;
> > +
> > +	if (ring->me >= adev->sdma.num_instances) {
> > +		dev_err(adev->dev, "sdma instance not found\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > +
> > +	amdgpu_amdkfd_suspend(adev, true);
> > +	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > +	amdgpu_amdkfd_resume(adev, true);
> > +	if (r)
> > +		return r;
> > +
> > +	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > +}
> 
> Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2
> is better. This may not work in the same way for all legacy queues.
> 
> Thanks,
> Lijo

Hi,

As far as I see, this function can be reused for all generations that don't 
use MES. If you don't think so, please explain why not.

Thanks,
Timur

> 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h index
> > 4f4e56022c97..7c4e145ca0c1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > @@ -160,6 +160,10 @@ struct amdgpu_buffer_funcs {
> > 
> >   int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t
> >   instance_id,>   
> >   			     bool caller_handles_kernel_queues);
> > 
> > +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> > +				   unsigned int vmid,
> > +				   struct amdgpu_fence 
*timedout_fence);
> > +
> > 
> >   #define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t)
> >   (adev)->mman.buffer_funcs->emit_copy_buffer((ib),  (s), (d), (b), (t))
> >   #define amdgpu_emit_fill_buffer(adev, ib, s, d, b)
> >   (adev)->mman.buffer_funcs->emit_fill_buffer((ib), (s), (d), (b))> 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c index 0da54c335822..76f8765fb175
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > @@ -1518,29 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct
> > amdgpu_ip_block *ip_block)> 
> >   	return -ETIMEDOUT;
> >   
> >   }
> > 
> > -static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
> > -				 unsigned int vmid,
> > -				 struct amdgpu_fence 
*timedout_fence)
> > -{
> > -	struct amdgpu_device *adev = ring->adev;
> > -	int r;
> > -
> > -	if (ring->me >= adev->sdma.num_instances) {
> > -		dev_err(adev->dev, "sdma instance not found\n");
> > -		return -EINVAL;
> > -	}
> > -
> > -	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > -
> > -	amdgpu_amdkfd_suspend(adev, true);
> > -	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > -	amdgpu_amdkfd_resume(adev, true);
> > -	if (r)
> > -		return r;
> > -
> > -	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > -}
> > -
> > 
> >   static int sdma_v5_0_stop_queue(struct amdgpu_ring *ring)
> >   {
> >   
> >   	u32 f32_cntl, freeze, cntl, stat1_reg;
> > 
> > @@ -1936,7 +1913,7 @@ static const struct amdgpu_ring_funcs
> > sdma_v5_0_ring_funcs = {> 
> >   	.emit_reg_write_reg_wait = 
sdma_v5_0_ring_emit_reg_write_reg_wait,
> >   	.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
> >   	.preempt_ib = sdma_v5_0_ring_preempt_ib,
> > 
> > -	.reset = sdma_v5_0_reset_queue,
> > +	.reset = amdgpu_sdma_reset_queue_legacy,
> > 
> >   };
> >   
> >   static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c index 5543e381dcca..e7f4b74f27b4
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > @@ -1436,29 +1436,6 @@ static int sdma_v5_2_wait_for_idle(struct
> > amdgpu_ip_block *ip_block)> 
> >   	return -ETIMEDOUT;
> >   
> >   }
> > 
> > -static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring,
> > -				 unsigned int vmid,
> > -				 struct amdgpu_fence 
*timedout_fence)
> > -{
> > -	struct amdgpu_device *adev = ring->adev;
> > -	int r;
> > -
> > -	if (ring->me >= adev->sdma.num_instances) {
> > -		dev_err(adev->dev, "sdma instance not found\n");
> > -		return -EINVAL;
> > -	}
> > -
> > -	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > -
> > -	amdgpu_amdkfd_suspend(adev, true);
> > -	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > -	amdgpu_amdkfd_resume(adev, true);
> > -	if (r)
> > -		return r;
> > -
> > -	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > -}
> > -
> > 
> >   static int sdma_v5_2_stop_queue(struct amdgpu_ring *ring)
> >   {
> >   
> >   	u32 f32_cntl, freeze, cntl, stat1_reg;
> > 
> > @@ -1951,7 +1928,7 @@ static const struct amdgpu_ring_funcs
> > sdma_v5_2_ring_funcs = {> 
> >   	.emit_reg_write_reg_wait = 
sdma_v5_2_ring_emit_reg_write_reg_wait,
> >   	.init_cond_exec = sdma_v5_2_ring_init_cond_exec,
> >   	.preempt_ib = sdma_v5_2_ring_preempt_ib,
> > 
> > -	.reset = sdma_v5_2_reset_queue,
> > +	.reset = amdgpu_sdma_reset_queue_legacy,
> > 
> >   };
> >   
> >   static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)

Re: [PATCH 4/5] drm/amdgpu/sdma: Use common SDMA legacy queue reset on SDMA v4.4.2

[Lazar, Lijo]

On 28-Aug-26 9:06 PM, Timur Kristóf wrote:
> On Friday, August 28, 2026 4:16:15 PM Central European Summer Time Lazar, Lijo
> wrote:
>> On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
>>> Besides sharing code, this additionally adds support for
>>> restoring queue contents after the reset.
>>>
>>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
>>> ---
>>>
>>>    drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 18 ++----------------
>>>    1 file changed, 2 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c index
>>> 72ea9db939b4..023821848531 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
>>> @@ -1655,20 +1655,6 @@ static bool sdma_v4_4_2_is_queue_selected(struct
>>> amdgpu_device *adev, uint32_t i>
>>>    	return (context_status & SDMA_GFX_CONTEXT_STATUS__SELECTED_MASK)
> != 0;
>>>    
>>>    }
>>>
>>> -static int sdma_v4_4_2_reset_queue(struct amdgpu_ring *ring,
>>> -				   unsigned int vmid,
>>> -				   struct amdgpu_fence
> *timedout_fence)
>>> -{
>>> -	struct amdgpu_device *adev = ring->adev;
>>> -	u32 id = ring->me;
>>> -	int r;
>>> -
>>> -	amdgpu_amdkfd_suspend(adev, true);
>>> -	r = amdgpu_sdma_reset_engine(adev, id, false);
>>
>> The common one passes true, so this doesn't look right.
>>    	amdgpu_sdma_reset_engine(adev, ring->me, true);
>>
>>
>> Thanks,
>> Lijo
> 
> Can you please elaborate more on what you are suggesting here?
> The caller of the queue reset is amdgpu_job_timedout() and that function
> already handles kernel queues.
> 
> 

It passes a false here which takes care of both paging and gfx rings. 
The one in job timeout is a generic one which only looks at the hung ring.

Thanks,
Lijo

>>
>>> -	amdgpu_amdkfd_resume(adev, true);
>>> -	return r;
>>> -}
>>> -
>>>
>>>    static int sdma_v4_4_2_stop_queue(struct amdgpu_ring *ring)
>>>    {
>>>    
>>>    	struct amdgpu_device *adev = ring->adev;
>>>
>>> @@ -2133,7 +2119,7 @@ static const struct amdgpu_ring_funcs
>>> sdma_v4_4_2_ring_funcs = {>
>>>    	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
>>>    	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
>>>    	.emit_reg_write_reg_wait =
> amdgpu_ring_emit_reg_write_reg_wait_helper,
>>>
>>> -	.reset = sdma_v4_4_2_reset_queue,
>>> +	.reset = amdgpu_sdma_reset_queue_legacy,
>>>
>>>    };
>>>    
>>>    static const struct amdgpu_ring_funcs sdma_v4_4_2_page_ring_funcs = {
>>>
>>> @@ -2165,7 +2151,7 @@ static const struct amdgpu_ring_funcs
>>> sdma_v4_4_2_page_ring_funcs = {>
>>>    	.emit_wreg = sdma_v4_4_2_ring_emit_wreg,
>>>    	.emit_reg_wait = sdma_v4_4_2_ring_emit_reg_wait,
>>>    	.emit_reg_write_reg_wait =
> amdgpu_ring_emit_reg_write_reg_wait_helper,
>>>
>>> -	.reset = sdma_v4_4_2_reset_queue,
>>> +	.reset = amdgpu_sdma_reset_queue_legacy,
>>>
>>>    };
>>>    
>>>    static void sdma_v4_4_2_set_ring_funcs(struct amdgpu_device *adev)
> 
> 
> 
> 

Re: [PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Alex Deucher]

On Fri, Aug 28, 2026 at 11:45 AM Timur Kristóf <timur.kristof@gmail.com> wrote:
>
> On Friday, August 28, 2026 4:20:19 PM Central European Summer Time Lazar, Lijo
> wrote:
> > On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
> > > The code was exactly the same between SDMA v5.0 and v5.2
> > > furthermore the exact same implementation can be shared
> > > between all SDMA versions that don't use MES.
> > >
> > > Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
> > > ---
> > >
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 37 ++++++++++++++++++++++++
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  4 +++
> > >   drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 25 +---------------
> > >   drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   | 25 +---------------
> > >   4 files changed, 43 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c index
> > > 66f278f77f71..9eebd8380834 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> > > @@ -635,3 +635,40 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device
> > > *adev, uint32_t instance_id,>
> > >     return ret;
> > >
> > >   }
> > >
> > > +
> > > +/**
> > > + * amdgpu_sdma_reset_queue_legacy() - Reset legacy SDMA queue after
> > > timeout (without MES) + *
> > > + * @ring: Pointer to the ring of the SDMA queue
> > > + * @vmid: VMID of the timed out job
> > > + * @timedout_fence: Fence of the timed out job
> > > + *
> > > + * Common implementation for resetting SDMA queues without MES (legacy).
> > > + * This relies on the proper amdgpu_sdma_funcs to be set up
> > > + * for the given ring.
> > > + *
> > > + * Applicable to SDMA versions that don't rely on the MES yet,
> > > + * that is all versions up to SDMA v5.x and older.
> > > + */
> > > +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> > > +                              unsigned int vmid,
> > > +                              struct amdgpu_fence
> *timedout_fence)
> > > +{
> > > +   struct amdgpu_device *adev = ring->adev;
> > > +   int r;
> > > +
> > > +   if (ring->me >= adev->sdma.num_instances) {
> > > +           dev_err(adev->dev, "sdma instance not found\n");
> > > +           return -EINVAL;
> > > +   }
> > > +
> > > +   amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > > +
> > > +   amdgpu_amdkfd_suspend(adev, true);
> > > +   r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > > +   amdgpu_amdkfd_resume(adev, true);
> > > +   if (r)
> > > +           return r;
> > > +
> > > +   return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > > +}
> >
> > Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2
> > is better. This may not work in the same way for all legacy queues.
> >
> > Thanks,
> > Lijo
>
> Hi,
>
> As far as I see, this function can be reused for all generations that don't
> use MES. If you don't think so, please explain why not.

I think the context is missing that you want to extend this to older
generations as well and it's still applicable there.

Alex

>
> Thanks,
> Timur
>
> >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h index
> > > 4f4e56022c97..7c4e145ca0c1 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
> > > @@ -160,6 +160,10 @@ struct amdgpu_buffer_funcs {
> > >
> > >   int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t
> > >   instance_id,>
> > >                          bool caller_handles_kernel_queues);
> > >
> > > +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
> > > +                              unsigned int vmid,
> > > +                              struct amdgpu_fence
> *timedout_fence);
> > > +
> > >
> > >   #define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t)
> > >   (adev)->mman.buffer_funcs->emit_copy_buffer((ib),  (s), (d), (b), (t))
> > >   #define amdgpu_emit_fill_buffer(adev, ib, s, d, b)
> > >   (adev)->mman.buffer_funcs->emit_fill_buffer((ib), (s), (d), (b))>
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c index 0da54c335822..76f8765fb175
> > > 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> > > @@ -1518,29 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct
> > > amdgpu_ip_block *ip_block)>
> > >     return -ETIMEDOUT;
> > >
> > >   }
> > >
> > > -static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
> > > -                            unsigned int vmid,
> > > -                            struct amdgpu_fence
> *timedout_fence)
> > > -{
> > > -   struct amdgpu_device *adev = ring->adev;
> > > -   int r;
> > > -
> > > -   if (ring->me >= adev->sdma.num_instances) {
> > > -           dev_err(adev->dev, "sdma instance not found\n");
> > > -           return -EINVAL;
> > > -   }
> > > -
> > > -   amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > > -
> > > -   amdgpu_amdkfd_suspend(adev, true);
> > > -   r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > > -   amdgpu_amdkfd_resume(adev, true);
> > > -   if (r)
> > > -           return r;
> > > -
> > > -   return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > > -}
> > > -
> > >
> > >   static int sdma_v5_0_stop_queue(struct amdgpu_ring *ring)
> > >   {
> > >
> > >     u32 f32_cntl, freeze, cntl, stat1_reg;
> > >
> > > @@ -1936,7 +1913,7 @@ static const struct amdgpu_ring_funcs
> > > sdma_v5_0_ring_funcs = {>
> > >     .emit_reg_write_reg_wait =
> sdma_v5_0_ring_emit_reg_write_reg_wait,
> > >     .init_cond_exec = sdma_v5_0_ring_init_cond_exec,
> > >     .preempt_ib = sdma_v5_0_ring_preempt_ib,
> > >
> > > -   .reset = sdma_v5_0_reset_queue,
> > > +   .reset = amdgpu_sdma_reset_queue_legacy,
> > >
> > >   };
> > >
> > >   static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c index 5543e381dcca..e7f4b74f27b4
> > > 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> > > @@ -1436,29 +1436,6 @@ static int sdma_v5_2_wait_for_idle(struct
> > > amdgpu_ip_block *ip_block)>
> > >     return -ETIMEDOUT;
> > >
> > >   }
> > >
> > > -static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring,
> > > -                            unsigned int vmid,
> > > -                            struct amdgpu_fence
> *timedout_fence)
> > > -{
> > > -   struct amdgpu_device *adev = ring->adev;
> > > -   int r;
> > > -
> > > -   if (ring->me >= adev->sdma.num_instances) {
> > > -           dev_err(adev->dev, "sdma instance not found\n");
> > > -           return -EINVAL;
> > > -   }
> > > -
> > > -   amdgpu_ring_reset_helper_begin(ring, timedout_fence);
> > > -
> > > -   amdgpu_amdkfd_suspend(adev, true);
> > > -   r = amdgpu_sdma_reset_engine(adev, ring->me, true);
> > > -   amdgpu_amdkfd_resume(adev, true);
> > > -   if (r)
> > > -           return r;
> > > -
> > > -   return amdgpu_ring_reset_helper_end(ring, timedout_fence);
> > > -}
> > > -
> > >
> > >   static int sdma_v5_2_stop_queue(struct amdgpu_ring *ring)
> > >   {
> > >
> > >     u32 f32_cntl, freeze, cntl, stat1_reg;
> > >
> > > @@ -1951,7 +1928,7 @@ static const struct amdgpu_ring_funcs
> > > sdma_v5_2_ring_funcs = {>
> > >     .emit_reg_write_reg_wait =
> sdma_v5_2_ring_emit_reg_write_reg_wait,
> > >     .init_cond_exec = sdma_v5_2_ring_init_cond_exec,
> > >     .preempt_ib = sdma_v5_2_ring_preempt_ib,
> > >
> > > -   .reset = sdma_v5_2_reset_queue,
> > > +   .reset = amdgpu_sdma_reset_queue_legacy,
> > >
> > >   };
> > >
> > >   static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)
>
>
>
>

Re: [PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Lazar, Lijo]

On 28-Aug-26 9:07 PM, Timur Kristóf wrote:
> On Friday, August 28, 2026 4:20:19 PM Central European Summer Time Lazar, Lijo
> wrote:
>> On 28-Aug-26 5:37 PM, Timur Kristóf wrote:
>>> The code was exactly the same between SDMA v5.0 and v5.2
>>> furthermore the exact same implementation can be shared
>>> between all SDMA versions that don't use MES.
>>>
>>> Signed-off-by: Timur Kristóf <timur.kristof@gmail.com>
>>> ---
>>>
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 37 ++++++++++++++++++++++++
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  4 +++
>>>    drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 25 +---------------
>>>    drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   | 25 +---------------
>>>    4 files changed, 43 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c index
>>> 66f278f77f71..9eebd8380834 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
>>> @@ -635,3 +635,40 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device
>>> *adev, uint32_t instance_id,>
>>>    	return ret;
>>>    
>>>    }
>>>
>>> +
>>> +/**
>>> + * amdgpu_sdma_reset_queue_legacy() - Reset legacy SDMA queue after
>>> timeout (without MES) + *
>>> + * @ring: Pointer to the ring of the SDMA queue
>>> + * @vmid: VMID of the timed out job
>>> + * @timedout_fence: Fence of the timed out job
>>> + *
>>> + * Common implementation for resetting SDMA queues without MES (legacy).
>>> + * This relies on the proper amdgpu_sdma_funcs to be set up
>>> + * for the given ring.
>>> + *
>>> + * Applicable to SDMA versions that don't rely on the MES yet,
>>> + * that is all versions up to SDMA v5.x and older.
>>> + */
>>> +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
>>> +				   unsigned int vmid,
>>> +				   struct amdgpu_fence
> *timedout_fence)
>>> +{
>>> +	struct amdgpu_device *adev = ring->adev;
>>> +	int r;
>>> +
>>> +	if (ring->me >= adev->sdma.num_instances) {
>>> +		dev_err(adev->dev, "sdma instance not found\n");
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
>>> +
>>> +	amdgpu_amdkfd_suspend(adev, true);
>>> +	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
>>> +	amdgpu_amdkfd_resume(adev, true);
>>> +	if (r)
>>> +		return r;
>>> +
>>> +	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
>>> +}
>>
>> Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2
>> is better. This may not work in the same way for all legacy queues.
>>
>> Thanks,
>> Lijo
> 
> Hi,
> 
> As far as I see, this function can be reused for all generations that don't
> use MES. If you don't think so, please explain why not.
> 

This function doesn't take care of paging ring (if enabled). Or, this 
could be made simlar to the one in sdma 4.4.2. It also makes use of the 
helpers, but not sure if the sequence reversal (save content -> kfd 
suspend vs kfd suspend -> save content) has other side effects.

Thanks,
Lijo

> Thanks,
> Timur
> 
>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h index
>>> 4f4e56022c97..7c4e145ca0c1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
>>> @@ -160,6 +160,10 @@ struct amdgpu_buffer_funcs {
>>>
>>>    int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t
>>>    instance_id,>
>>>    			     bool caller_handles_kernel_queues);
>>>
>>> +int amdgpu_sdma_reset_queue_legacy(struct amdgpu_ring *ring,
>>> +				   unsigned int vmid,
>>> +				   struct amdgpu_fence
> *timedout_fence);
>>> +
>>>
>>>    #define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t)
>>>    (adev)->mman.buffer_funcs->emit_copy_buffer((ib),  (s), (d), (b), (t))
>>>    #define amdgpu_emit_fill_buffer(adev, ib, s, d, b)
>>>    (adev)->mman.buffer_funcs->emit_fill_buffer((ib), (s), (d), (b))>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c index 0da54c335822..76f8765fb175
>>> 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>> @@ -1518,29 +1518,6 @@ static int sdma_v5_0_wait_for_idle(struct
>>> amdgpu_ip_block *ip_block)>
>>>    	return -ETIMEDOUT;
>>>    
>>>    }
>>>
>>> -static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring,
>>> -				 unsigned int vmid,
>>> -				 struct amdgpu_fence
> *timedout_fence)
>>> -{
>>> -	struct amdgpu_device *adev = ring->adev;
>>> -	int r;
>>> -
>>> -	if (ring->me >= adev->sdma.num_instances) {
>>> -		dev_err(adev->dev, "sdma instance not found\n");
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
>>> -
>>> -	amdgpu_amdkfd_suspend(adev, true);
>>> -	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
>>> -	amdgpu_amdkfd_resume(adev, true);
>>> -	if (r)
>>> -		return r;
>>> -
>>> -	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
>>> -}
>>> -
>>>
>>>    static int sdma_v5_0_stop_queue(struct amdgpu_ring *ring)
>>>    {
>>>    
>>>    	u32 f32_cntl, freeze, cntl, stat1_reg;
>>>
>>> @@ -1936,7 +1913,7 @@ static const struct amdgpu_ring_funcs
>>> sdma_v5_0_ring_funcs = {>
>>>    	.emit_reg_write_reg_wait =
> sdma_v5_0_ring_emit_reg_write_reg_wait,
>>>    	.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
>>>    	.preempt_ib = sdma_v5_0_ring_preempt_ib,
>>>
>>> -	.reset = sdma_v5_0_reset_queue,
>>> +	.reset = amdgpu_sdma_reset_queue_legacy,
>>>
>>>    };
>>>    
>>>    static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c index 5543e381dcca..e7f4b74f27b4
>>> 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
>>> @@ -1436,29 +1436,6 @@ static int sdma_v5_2_wait_for_idle(struct
>>> amdgpu_ip_block *ip_block)>
>>>    	return -ETIMEDOUT;
>>>    
>>>    }
>>>
>>> -static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring,
>>> -				 unsigned int vmid,
>>> -				 struct amdgpu_fence
> *timedout_fence)
>>> -{
>>> -	struct amdgpu_device *adev = ring->adev;
>>> -	int r;
>>> -
>>> -	if (ring->me >= adev->sdma.num_instances) {
>>> -		dev_err(adev->dev, "sdma instance not found\n");
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	amdgpu_ring_reset_helper_begin(ring, timedout_fence);
>>> -
>>> -	amdgpu_amdkfd_suspend(adev, true);
>>> -	r = amdgpu_sdma_reset_engine(adev, ring->me, true);
>>> -	amdgpu_amdkfd_resume(adev, true);
>>> -	if (r)
>>> -		return r;
>>> -
>>> -	return amdgpu_ring_reset_helper_end(ring, timedout_fence);
>>> -}
>>> -
>>>
>>>    static int sdma_v5_2_stop_queue(struct amdgpu_ring *ring)
>>>    {
>>>    
>>>    	u32 f32_cntl, freeze, cntl, stat1_reg;
>>>
>>> @@ -1951,7 +1928,7 @@ static const struct amdgpu_ring_funcs
>>> sdma_v5_2_ring_funcs = {>
>>>    	.emit_reg_write_reg_wait =
> sdma_v5_2_ring_emit_reg_write_reg_wait,
>>>    	.init_cond_exec = sdma_v5_2_ring_init_cond_exec,
>>>    	.preempt_ib = sdma_v5_2_ring_preempt_ib,
>>>
>>> -	.reset = sdma_v5_2_reset_queue,
>>> +	.reset = amdgpu_sdma_reset_queue_legacy,
>>>
>>>    };
>>>    
>>>    static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)
> 
> 
> 
> 

Re: [PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Timur Kristóf]

On 2026. augusztus 28., péntek 17:53:54 közép-európai nyári idő Lazar, Lijo 
wrote:
> >> 
> >> Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2
> >> is better. This may not work in the same way for all legacy queues.
> >> 
> >> Thanks,
> >> Lijo
> > 
> > Hi,
> > 
> > As far as I see, this function can be reused for all generations that
> > don't
> > use MES. If you don't think so, please explain why not.
> 
> This function doesn't take care of paging ring (if enabled). Or, this
> could be made simlar to the one in sdma 4.4.2.

Hi Lijo,

I took another look at this, and I now see what you mean.

I think the solution is to remove the "caller_handles_kernel_queues" argument 
and just check whether the workqueue on the SDMA gfx_ring is already stopped.
This would be similar to how amdgpu_multi_ring_reset_helper_begin() works, in 
fact (with a few changes) that function could be adopted to work for this use 
case as well.

> not sure if the sequence reversal (save content -> kfd
> suspend vs kfd suspend -> save content) has other side effects.

Good point, thanks for noticing that.

I think the upstream SDMA v5 function is wrong and the correct sequence is the 
SDMA v4.4.2 code: suspend the KFD first and then save the ring contents. I will 
adjust my code to match the v4.4.2 behaviour in the next version of this 
series.

@Alex - does that sound reasonable to you?

Thanks & best regards,
Timur

Re: [PATCH 3/5] drm/amdgpu/sdma: Move SDMA v5.x queue reset to common code

[Timur Kristóf]

On 2026. augusztus 28., péntek 17:52:22 közép-európai nyári idő Alex Deucher 
wrote:
> > > 
> > > Instead of moving it here - sdma_v5_x_reset_queue and using it for 5.2
> > > is better. This may not work in the same way for all legacy queues.
> > > 
> > > Thanks,
> > > Lijo
> > 
> > Hi,
> > 
> > As far as I see, this function can be reused for all generations that
> > don't
> > use MES. If you don't think so, please explain why not.
> 
> I think the context is missing that you want to extend this to older
> generations as well and it's still applicable there.
> 
> Alex

Yes, I would like to implement queue reset for all SDMA versions and I would 
like to extend this function to do that.

My main motivation for this is that we want to enable SDMA for older GPU 
generations in RADV. The SDMA will see more use, and therefore the kernel 
needs to handle when the SDMA hangs.

I already have a prototype implementation of the queue reset for SDMA v2 (CIK) 
which makes is much easier to develop the SDMA code in RADV because it can 
recover from hangs without needing me to reboot my computer as I am developing 
and testing the code in RADV.

Thanks & best regards,
Timur