* [PATCH 2/7 V2] drm/amd/amdgpu: Implement SDMA soft reset directly for sdma v5
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
@ 2025-04-02 3:02 ` jesse.zhang
2025-04-02 3:02 ` [PATCH 3/7 V2] drm/amdgpu: Optimize SDMA v5.0 queue reset and stop logic jesse.zhang
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jesse.zhang @ 2025-04-02 3:02 UTC (permalink / raw)
To: amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu,
Jesse.zhang@amd.com, Alex Deucher, Jesse Zhang
From: "Jesse.zhang@amd.com" <Jesse.zhang@amd.com>
This patch introduces a new function `amdgpu_sdma_soft_reset` to handle SDMA soft resets directly,
rather than relying on the DPM interface.
1. **New `amdgpu_sdma_soft_reset` Function**:
- Implements a soft reset for SDMA engines by directly writing to the hardware registers.
- Handles SDMA versions 4.x and 5.x separately:
- For SDMA 4.x, the existing `amdgpu_dpm_reset_sdma` function is used for backward compatibility.
- For SDMA 5.x, the driver directly manipulates the `GRBM_SOFT_RESET` register to reset the specified SDMA instance.
2. **Integration into `amdgpu_sdma_reset_engine`**:
- The `amdgpu_sdma_soft_reset` function is called during the SDMA reset process, replacing the previous call to `amdgpu_dpm_reset_sdma`.
Suggested-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 46 +++++++++++++++++++++++-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index 7d862c887a1a..dbc7c7cfee01 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -26,6 +26,8 @@
#include "amdgpu_sdma.h"
#include "amdgpu_ras.h"
#include "amdgpu_reset.h"
+#include "gc/gc_10_1_0_offset.h"
+#include "gc/gc_10_3_0_sh_mask.h"
#define AMDGPU_CSA_SDMA_SIZE 64
/* SDMA CSA reside in the 3rd page of CSA */
@@ -553,6 +555,48 @@ void amdgpu_sdma_register_on_reset_callbacks(struct amdgpu_device *adev, struct
list_add_tail(&funcs->list, &adev->sdma.reset_callback_list);
}
+static int amdgpu_sdma_soft_reset(struct amdgpu_device *adev, u32 instance_id)
+{
+ u32 soft_reset;
+ int r = 0;
+
+ switch (amdgpu_ip_version(adev, SDMA0_HWIP, 0)) {
+ case IP_VERSION(4, 4, 2):
+ case IP_VERSION(4, 4, 4):
+ case IP_VERSION(4, 4, 5):
+ /* For SDMA 4.x, use the existing DPM interface for backward compatibility */
+ r = amdgpu_dpm_reset_sdma(adev, 1 << instance_id);
+ break;
+ case IP_VERSION(5, 0, 0):
+ case IP_VERSION(5, 0, 1):
+ case IP_VERSION(5, 0, 2):
+ case IP_VERSION(5, 0, 5):
+ case IP_VERSION(5, 2, 0):
+ case IP_VERSION(5, 2, 2):
+ case IP_VERSION(5, 2, 4):
+ case IP_VERSION(5, 2, 5):
+ case IP_VERSION(5, 2, 6):
+ case IP_VERSION(5, 2, 3):
+ case IP_VERSION(5, 2, 1):
+ case IP_VERSION(5, 2, 7):
+ /* For SDMA 5.x, directly manipulate the GRBM_SOFT_RESET register */
+ soft_reset = RREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET);
+ soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << instance_id;
+ /* Issue the soft reset */
+ WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
+
+ udelay(50);
+ /* Clear the soft reset bit */
+ soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << instance_id);
+ WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
+ break;
+ default:
+ break;
+ }
+
+ return r;
+}
+
/**
* amdgpu_sdma_reset_engine - Reset a specific SDMA engine
* @adev: Pointer to the AMDGPU device
@@ -587,7 +631,7 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id)
gfx_ring->funcs->stop_queue(adev, instance_id);
/* Perform the SDMA reset for the specified instance */
- ret = amdgpu_dpm_reset_sdma(adev, 1 << instance_id);
+ ret = amdgpu_sdma_soft_reset(adev, instance_id);
if (ret) {
dev_err(adev->dev, "Failed to reset SDMA instance %u\n", instance_id);
goto exit;
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/7 V2] drm/amdgpu: Optimize SDMA v5.0 queue reset and stop logic
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
2025-04-02 3:02 ` [PATCH 2/7 V2] drm/amd/amdgpu: Implement SDMA soft reset directly for sdma v5 jesse.zhang
@ 2025-04-02 3:02 ` jesse.zhang
2025-04-02 3:02 ` [PATCH 4/7 V2] drm/amd/amdgpu: Refactor SDMA v5.0 reset logic into stop_queue and restore_queue functions jesse.zhang
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jesse.zhang @ 2025-04-02 3:02 UTC (permalink / raw)
To: amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu,
Jesse.zhang@amd.com, Jesse Zhang
From: "Jesse.zhang@amd.com" <Jesse.zhang@amd.com>
This patch refactors the SDMA v5.0 queue reset and stop logic to improve
code readability, maintainability, and performance. The key changes include:
1. **Generalized `sdma_v5_0_gfx_stop` Function**:
- Added an `inst_mask` parameter to allow stopping specific SDMA instances
instead of all instances. This is useful for resetting individual queues.
2. **Simplified `sdma_v5_0_reset_queue` Function**:
- Removed redundant loops and checks by directly using the `ring->me` field
to identify the SDMA instance.
- Reused the `sdma_v5_0_gfx_stop` function to stop the queue, reducing code
duplication.
Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 65 +++++++++++---------------
1 file changed, 26 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index e1348b6d9c6a..9501652f903d 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -555,15 +555,15 @@ static void sdma_v5_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 se
* sdma_v5_0_gfx_stop - stop the gfx async dma engines
*
* @adev: amdgpu_device pointer
- *
+ * @inst_mask: mask of dma engine instances to be disabled
* Stop the gfx async dma ring buffers (NAVI10).
*/
-static void sdma_v5_0_gfx_stop(struct amdgpu_device *adev)
+static void sdma_v5_0_gfx_stop(struct amdgpu_device *adev, uint32_t inst_mask)
{
u32 rb_cntl, ib_cntl;
int i;
- for (i = 0; i < adev->sdma.num_instances; i++) {
+ for_each_inst(i, inst_mask) {
rb_cntl = RREG32_SOC15_IP(GC, sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL));
rb_cntl = REG_SET_FIELD(rb_cntl, SDMA0_GFX_RB_CNTL, RB_ENABLE, 0);
WREG32_SOC15_IP(GC, sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL), rb_cntl);
@@ -655,9 +655,11 @@ static void sdma_v5_0_enable(struct amdgpu_device *adev, bool enable)
{
u32 f32_cntl;
int i;
+ uint32_t inst_mask;
+ inst_mask = GENMASK(adev->sdma.num_instances - 1, 0);
if (!enable) {
- sdma_v5_0_gfx_stop(adev);
+ sdma_v5_0_gfx_stop(adev, 1 << inst_mask);
sdma_v5_0_rlc_stop(adev);
}
@@ -1506,40 +1508,25 @@ static int sdma_v5_0_soft_reset(struct amdgpu_ip_block *ip_block)
static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
{
struct amdgpu_device *adev = ring->adev;
- int i, j, r;
- u32 rb_cntl, ib_cntl, f32_cntl, freeze, cntl, preempt, soft_reset, stat1_reg;
+ int j, r;
+ u32 f32_cntl, freeze, cntl, preempt, soft_reset, stat1_reg;
+ u32 inst_id;
if (amdgpu_sriov_vf(adev))
return -EINVAL;
-
- for (i = 0; i < adev->sdma.num_instances; i++) {
- if (ring == &adev->sdma.instance[i].ring)
- break;
- }
-
- if (i == adev->sdma.num_instances) {
- DRM_ERROR("sdma instance not found\n");
- return -EINVAL;
- }
-
+ inst_id = ring->me;
amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
/* stop queue */
- ib_cntl = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_IB_CNTL));
- ib_cntl = REG_SET_FIELD(ib_cntl, SDMA0_GFX_IB_CNTL, IB_ENABLE, 0);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_IB_CNTL), ib_cntl);
-
- rb_cntl = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL));
- rb_cntl = REG_SET_FIELD(rb_cntl, SDMA0_GFX_RB_CNTL, RB_ENABLE, 0);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL), rb_cntl);
+ sdma_v5_0_gfx_stop(adev, 1 << ring->me);
/* engine stop SDMA1_F32_CNTL.HALT to 1 and SDMAx_FREEZE freeze bit to 1 */
- freeze = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_FREEZE));
+ freeze = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
freeze = REG_SET_FIELD(freeze, SDMA0_FREEZE, FREEZE, 1);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_FREEZE), freeze);
+ WREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE), freeze);
for (j = 0; j < adev->usec_timeout; j++) {
- freeze = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_FREEZE));
+ freeze = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
if (REG_GET_FIELD(freeze, SDMA0_FREEZE, FROZEN) & 1)
break;
udelay(1);
@@ -1547,7 +1534,7 @@ static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
/* check sdma copy engine all idle if frozen not received*/
if (j == adev->usec_timeout) {
- stat1_reg = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_STATUS1_REG));
+ stat1_reg = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_STATUS1_REG));
if ((stat1_reg & 0x3FF) != 0x3FF) {
DRM_ERROR("cannot soft reset as sdma not idle\n");
r = -ETIMEDOUT;
@@ -1555,35 +1542,35 @@ static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
}
}
- f32_cntl = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_F32_CNTL));
+ f32_cntl = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_F32_CNTL));
f32_cntl = REG_SET_FIELD(f32_cntl, SDMA0_F32_CNTL, HALT, 1);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_F32_CNTL), f32_cntl);
+ WREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_F32_CNTL), f32_cntl);
- cntl = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_CNTL));
+ cntl = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_CNTL));
cntl = REG_SET_FIELD(cntl, SDMA0_CNTL, UTC_L1_ENABLE, 0);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_CNTL), cntl);
+ WREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_CNTL), cntl);
/* soft reset SDMA_GFX_PREEMPT.IB_PREEMPT = 0 mmGRBM_SOFT_RESET.SOFT_RESET_SDMA0/1 = 1 */
- preempt = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_PREEMPT));
+ preempt = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT));
preempt = REG_SET_FIELD(preempt, SDMA0_GFX_PREEMPT, IB_PREEMPT, 0);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_GFX_PREEMPT), preempt);
+ WREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT), preempt);
soft_reset = RREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET);
- soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << i;
+ soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id;
WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
udelay(50);
- soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << i);
+ soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id);
WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
/* unfreeze*/
- freeze = RREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_FREEZE));
+ freeze = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
freeze = REG_SET_FIELD(freeze, SDMA0_FREEZE, FREEZE, 0);
- WREG32(sdma_v5_0_get_reg_offset(adev, i, mmSDMA0_FREEZE), freeze);
+ WREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE), freeze);
- r = sdma_v5_0_gfx_resume_instance(adev, i, true);
+ r = sdma_v5_0_gfx_resume_instance(adev, inst_id, true);
err0:
amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/7 V2] drm/amd/amdgpu: Refactor SDMA v5.0 reset logic into stop_queue and restore_queue functions
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
2025-04-02 3:02 ` [PATCH 2/7 V2] drm/amd/amdgpu: Implement SDMA soft reset directly for sdma v5 jesse.zhang
2025-04-02 3:02 ` [PATCH 3/7 V2] drm/amdgpu: Optimize SDMA v5.0 queue reset and stop logic jesse.zhang
@ 2025-04-02 3:02 ` jesse.zhang
2025-04-02 3:02 ` [PATCH 5/7 V2] drm/amdgpu: Optimize SDMA v5.2 queue reset and stop logic jesse.zhang
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jesse.zhang @ 2025-04-02 3:02 UTC (permalink / raw)
To: amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu,
Jesse.zhang@amd.com, Jesse Zhang
From: "Jesse.zhang@amd.com" <Jesse.zhang@amd.com>
This patch refactors the SDMA v5.0 reset logic by splitting the `sdma_v5_0_reset_queue` function into two separate functions: `sdma_v5_0_stop_queue` and `sdma_v5_0_restore_queue`.
This change aligns with the new SDMA reset mechanism, where the reset process is divided into stopping the queue, performing the reset, and restoring the queue.
1. **Split `sdma_v5_0_reset_queue`**:
- Extracted the queue stopping logic into `sdma_v5_0_stop_queue`.
- Extracted the queue restoration logic into `sdma_v5_0_restore_queue`.
- The soft reset step is now handled by the caller (`amdgpu_sdma_reset_engine`).
2. **Update Ring Functions**:
- Added `stop_queue` and `start_queue` to the `sdma_v5_0_ring_funcs` structure to support the new reset mechanism.
v2: remove the suspend_user_queues param when calling amdgpu_sdma_reset_engine()
Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 37 +++++++++++++++-----------
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 9501652f903d..df77bf2639cf 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1508,17 +1508,23 @@ static int sdma_v5_0_soft_reset(struct amdgpu_ip_block *ip_block)
static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
{
struct amdgpu_device *adev = ring->adev;
- int j, r;
- u32 f32_cntl, freeze, cntl, preempt, soft_reset, stat1_reg;
- u32 inst_id;
+ u32 inst_id = ring->me;
+
+ return amdgpu_sdma_reset_engine(adev, inst_id);
+}
+
+static int sdma_v5_0_stop_queue(struct amdgpu_device *adev, uint32_t inst_id)
+{
+ int j, r = 0;
+ u32 f32_cntl, freeze, cntl, preempt, stat1_reg;
if (amdgpu_sriov_vf(adev))
return -EINVAL;
- inst_id = ring->me;
+
amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
/* stop queue */
- sdma_v5_0_gfx_stop(adev, 1 << ring->me);
+ sdma_v5_0_gfx_stop(adev, inst_id);
/* engine stop SDMA1_F32_CNTL.HALT to 1 and SDMAx_FREEZE freeze bit to 1 */
freeze = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
@@ -1554,17 +1560,17 @@ static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
preempt = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT));
preempt = REG_SET_FIELD(preempt, SDMA0_GFX_PREEMPT, IB_PREEMPT, 0);
WREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT), preempt);
+err0:
+ amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
+ return r;
+}
- soft_reset = RREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET);
- soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id;
-
- WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
-
- udelay(50);
-
- soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id);
- WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
+static int sdma_v5_0_restore_queue(struct amdgpu_device *adev, uint32_t inst_id)
+{
+ int r;
+ u32 freeze;
+ amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
/* unfreeze*/
freeze = RREG32(sdma_v5_0_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
freeze = REG_SET_FIELD(freeze, SDMA0_FREEZE, FREEZE, 0);
@@ -1572,7 +1578,6 @@ static int sdma_v5_0_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
r = sdma_v5_0_gfx_resume_instance(adev, inst_id, true);
-err0:
amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
return r;
}
@@ -1919,6 +1924,8 @@ static const struct amdgpu_ring_funcs sdma_v5_0_ring_funcs = {
.init_cond_exec = sdma_v5_0_ring_init_cond_exec,
.preempt_ib = sdma_v5_0_ring_preempt_ib,
.reset = sdma_v5_0_reset_queue,
+ .stop_queue = sdma_v5_0_stop_queue,
+ .start_queue = sdma_v5_0_restore_queue,
};
static void sdma_v5_0_set_ring_funcs(struct amdgpu_device *adev)
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 5/7 V2] drm/amdgpu: Optimize SDMA v5.2 queue reset and stop logic
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
` (2 preceding siblings ...)
2025-04-02 3:02 ` [PATCH 4/7 V2] drm/amd/amdgpu: Refactor SDMA v5.0 reset logic into stop_queue and restore_queue functions jesse.zhang
@ 2025-04-02 3:02 ` jesse.zhang
2025-04-02 3:02 ` [PATCH 6/7 V2] drm/amd/amdgpu: Refactor SDMA v5.2 reset logic into stop_queue and restore_queue functions jesse.zhang
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: jesse.zhang @ 2025-04-02 3:02 UTC (permalink / raw)
To: amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu,
Jesse.zhang@amd.com, Jesse Zhang
From: "Jesse.zhang@amd.com" <Jesse.zhang@amd.com>
This patch refactors the SDMA v5.2 queue reset and stop logic to improve
code readability, maintainability, and performance. The key changes include:
1. **Generalized `sdma_v5_2_gfx_stop` Function**:
- Added an `inst_mask` parameter to allow stopping specific SDMA instances
instead of all instances. This is useful for resetting individual queues.
2. **Simplified `sdma_v5_2_reset_queue` Function**:
- Removed redundant loops and checks by directly using the `ring->me` field
to identify the SDMA instance.
- Reused the `sdma_v5_2_gfx_stop` function to stop the queue, reducing code
duplication.
Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 64 +++++++++++---------------
1 file changed, 26 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index 964f12afac9e..96b02c3e4993 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -405,15 +405,15 @@ static void sdma_v5_2_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 se
* sdma_v5_2_gfx_stop - stop the gfx async dma engines
*
* @adev: amdgpu_device pointer
- *
+ * @inst_mask: mask of dma engine instances to be disabled
* Stop the gfx async dma ring buffers.
*/
-static void sdma_v5_2_gfx_stop(struct amdgpu_device *adev)
+static void sdma_v5_2_gfx_stop(struct amdgpu_device *adev, uint32_t inst_mask)
{
u32 rb_cntl, ib_cntl;
int i;
- for (i = 0; i < adev->sdma.num_instances; i++) {
+ for_each_inst(i, inst_mask) {
rb_cntl = RREG32_SOC15_IP(GC, sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL));
rb_cntl = REG_SET_FIELD(rb_cntl, SDMA0_GFX_RB_CNTL, RB_ENABLE, 0);
WREG32_SOC15_IP(GC, sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL), rb_cntl);
@@ -504,9 +504,11 @@ static void sdma_v5_2_enable(struct amdgpu_device *adev, bool enable)
{
u32 f32_cntl;
int i;
+ uint32_t inst_mask;
+ inst_mask = GENMASK(adev->sdma.num_instances - 1, 0);
if (!enable) {
- sdma_v5_2_gfx_stop(adev);
+ sdma_v5_2_gfx_stop(adev, inst_mask);
sdma_v5_2_rlc_stop(adev);
}
@@ -1437,40 +1439,26 @@ static int sdma_v5_2_wait_for_idle(struct amdgpu_ip_block *ip_block)
static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
{
struct amdgpu_device *adev = ring->adev;
- int i, j, r;
- u32 rb_cntl, ib_cntl, f32_cntl, freeze, cntl, preempt, soft_reset, stat1_reg;
+ int j, r;
+ u32 f32_cntl, freeze, cntl, preempt, soft_reset, stat1_reg;
+ u32 inst_id;
if (amdgpu_sriov_vf(adev))
return -EINVAL;
- for (i = 0; i < adev->sdma.num_instances; i++) {
- if (ring == &adev->sdma.instance[i].ring)
- break;
- }
-
- if (i == adev->sdma.num_instances) {
- DRM_ERROR("sdma instance not found\n");
- return -EINVAL;
- }
-
+ inst_id = ring->me;
amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
/* stop queue */
- ib_cntl = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_IB_CNTL));
- ib_cntl = REG_SET_FIELD(ib_cntl, SDMA0_GFX_IB_CNTL, IB_ENABLE, 0);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_IB_CNTL), ib_cntl);
-
- rb_cntl = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL));
- rb_cntl = REG_SET_FIELD(rb_cntl, SDMA0_GFX_RB_CNTL, RB_ENABLE, 0);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_RB_CNTL), rb_cntl);
+ sdma_v5_2_gfx_stop(adev, 1 << ring->me);
/*engine stop SDMA1_F32_CNTL.HALT to 1 and SDMAx_FREEZE freeze bit to 1 */
- freeze = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_FREEZE));
+ freeze = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
freeze = REG_SET_FIELD(freeze, SDMA0_FREEZE, FREEZE, 1);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_FREEZE), freeze);
+ WREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE), freeze);
for (j = 0; j < adev->usec_timeout; j++) {
- freeze = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_FREEZE));
+ freeze = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
if (REG_GET_FIELD(freeze, SDMA0_FREEZE, FROZEN) & 1)
break;
@@ -1479,7 +1467,7 @@ static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
if (j == adev->usec_timeout) {
- stat1_reg = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_STATUS1_REG));
+ stat1_reg = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_STATUS1_REG));
if ((stat1_reg & 0x3FF) != 0x3FF) {
DRM_ERROR("cannot soft reset as sdma not idle\n");
r = -ETIMEDOUT;
@@ -1487,37 +1475,37 @@ static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
}
}
- f32_cntl = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_F32_CNTL));
+ f32_cntl = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_F32_CNTL));
f32_cntl = REG_SET_FIELD(f32_cntl, SDMA0_F32_CNTL, HALT, 1);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_F32_CNTL), f32_cntl);
+ WREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_F32_CNTL), f32_cntl);
- cntl = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_CNTL));
+ cntl = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_CNTL));
cntl = REG_SET_FIELD(cntl, SDMA0_CNTL, UTC_L1_ENABLE, 0);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_CNTL), cntl);
+ WREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_CNTL), cntl);
/* soft reset SDMA_GFX_PREEMPT.IB_PREEMPT = 0 mmGRBM_SOFT_RESET.SOFT_RESET_SDMA0/1 = 1 */
- preempt = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_PREEMPT));
+ preempt = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT));
preempt = REG_SET_FIELD(preempt, SDMA0_GFX_PREEMPT, IB_PREEMPT, 0);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_GFX_PREEMPT), preempt);
+ WREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT), preempt);
soft_reset = RREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET);
- soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << i;
+ soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id;
WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
udelay(50);
- soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << i);
+ soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id);
WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
/* unfreeze and unhalt */
- freeze = RREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_FREEZE));
+ freeze = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
freeze = REG_SET_FIELD(freeze, SDMA0_FREEZE, FREEZE, 0);
- WREG32(sdma_v5_2_get_reg_offset(adev, i, mmSDMA0_FREEZE), freeze);
+ WREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE), freeze);
- r = sdma_v5_2_gfx_resume_instance(adev, i, true);
+ r = sdma_v5_2_gfx_resume_instance(adev, inst_id, true);
err0:
amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 6/7 V2] drm/amd/amdgpu: Refactor SDMA v5.2 reset logic into stop_queue and restore_queue functions
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
` (3 preceding siblings ...)
2025-04-02 3:02 ` [PATCH 5/7 V2] drm/amdgpu: Optimize SDMA v5.2 queue reset and stop logic jesse.zhang
@ 2025-04-02 3:02 ` jesse.zhang
2025-04-02 3:02 ` [PATCH 7/7 V2] drm/amd/amdgpu: Remove deprecated SDMA reset callback mechanism jesse.zhang
2025-04-02 7:38 ` [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks Christian König
6 siblings, 0 replies; 8+ messages in thread
From: jesse.zhang @ 2025-04-02 3:02 UTC (permalink / raw)
To: amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu,
Jesse.zhang@amd.com, Jesse Zhang
From: "Jesse.zhang@amd.com" <Jesse.zhang@amd.com>
This patch refactors the SDMA v5.2 reset logic by splitting the `sdma_v5_2_reset_queue` function into two separate functions: `sdma_v5_2_stop_queue` and `sdma_v5_2_restore_queue`.
This change aligns with the new SDMA reset mechanism, where the reset process is divided into stopping the queue, performing the reset, and restoring the queue.
1. **Split `sdma_v5_2_reset_queue`**:
- Extracted the queue stopping logic into `sdma_v5_2_stop_queue`.
- Extracted the queue restoration logic into `sdma_v5_2_restore_queue`.
- The soft reset step is now handled by the caller (`amdgpu_sdma_reset_engine`).
2. **Update Ring Functions**:
- Added `stop_queue` and `start_queue` to the `sdma_v5_2_ring_funcs` structure to support the new reset mechanism.
v2: remove the suspend_user_queues param when calling amdgpu_sdma_reset_engine()
Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
---
drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 38 ++++++++++++++------------
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index 96b02c3e4993..67b7d84c15dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -1439,18 +1439,22 @@ static int sdma_v5_2_wait_for_idle(struct amdgpu_ip_block *ip_block)
static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
{
struct amdgpu_device *adev = ring->adev;
- int j, r;
- u32 f32_cntl, freeze, cntl, preempt, soft_reset, stat1_reg;
- u32 inst_id;
+ u32 inst_id = ring->me;
+
+ return amdgpu_sdma_reset_engine(adev, inst_id);
+}
+
+static int sdma_v5_2_stop_queue(struct amdgpu_device *adev, uint32_t inst_id)
+{
+ int j, r = 0;
+ u32 f32_cntl, freeze, cntl, preempt, stat1_reg;
if (amdgpu_sriov_vf(adev))
return -EINVAL;
- inst_id = ring->me;
amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
-
/* stop queue */
- sdma_v5_2_gfx_stop(adev, 1 << ring->me);
+ sdma_v5_2_gfx_stop(adev, 1 << inst_id);
/*engine stop SDMA1_F32_CNTL.HALT to 1 and SDMAx_FREEZE freeze bit to 1 */
freeze = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
@@ -1488,18 +1492,17 @@ static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
preempt = REG_SET_FIELD(preempt, SDMA0_GFX_PREEMPT, IB_PREEMPT, 0);
WREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_GFX_PREEMPT), preempt);
- soft_reset = RREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET);
- soft_reset |= 1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id;
-
-
- WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
-
- udelay(50);
-
- soft_reset &= ~(1 << GRBM_SOFT_RESET__SOFT_RESET_SDMA0__SHIFT << inst_id);
+err0:
+ amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
+ return r;
+}
- WREG32_SOC15(GC, 0, mmGRBM_SOFT_RESET, soft_reset);
+static int sdma_v5_2_restore_queue(struct amdgpu_device *adev, uint32_t inst_id)
+{
+ u32 freeze;
+ int r;
+ amdgpu_gfx_rlc_enter_safe_mode(adev, 0);
/* unfreeze and unhalt */
freeze = RREG32(sdma_v5_2_get_reg_offset(adev, inst_id, mmSDMA0_FREEZE));
freeze = REG_SET_FIELD(freeze, SDMA0_FREEZE, FREEZE, 0);
@@ -1507,7 +1510,6 @@ static int sdma_v5_2_reset_queue(struct amdgpu_ring *ring, unsigned int vmid)
r = sdma_v5_2_gfx_resume_instance(adev, inst_id, true);
-err0:
amdgpu_gfx_rlc_exit_safe_mode(adev, 0);
return r;
}
@@ -1947,6 +1949,8 @@ static const struct amdgpu_ring_funcs sdma_v5_2_ring_funcs = {
.init_cond_exec = sdma_v5_2_ring_init_cond_exec,
.preempt_ib = sdma_v5_2_ring_preempt_ib,
.reset = sdma_v5_2_reset_queue,
+ .stop_queue = sdma_v5_2_stop_queue,
+ .start_queue = sdma_v5_2_restore_queue,
};
static void sdma_v5_2_set_ring_funcs(struct amdgpu_device *adev)
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 7/7 V2] drm/amd/amdgpu: Remove deprecated SDMA reset callback mechanism
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
` (4 preceding siblings ...)
2025-04-02 3:02 ` [PATCH 6/7 V2] drm/amd/amdgpu: Refactor SDMA v5.2 reset logic into stop_queue and restore_queue functions jesse.zhang
@ 2025-04-02 3:02 ` jesse.zhang
2025-04-02 7:38 ` [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks Christian König
6 siblings, 0 replies; 8+ messages in thread
From: jesse.zhang @ 2025-04-02 3:02 UTC (permalink / raw)
To: amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu,
Jesse.zhang@amd.com, Jesse Zhang
From: "Jesse.zhang@amd.com" <Jesse.zhang@amd.com>
This patch removes the deprecated SDMA reset callback mechanism, which was previously used to register pre-reset and post-reset callbacks for SDMA engine resets.
The SDMA reset callback mechanism allowed KFD and AMDGPU to register pre-reset and post-reset functions for handling SDMA engine resets.
The callback mechanism has been replaced with a more direct and efficient approach using `stop_queue` and `start_queue` functions in the ring's function table.
1. **Remove Callback Mechanism**:
- Removed the `amdgpu_sdma_register_on_reset_callbacks` function and its associated data structures (`sdma_on_reset_funcs`).
- Removed the callback registration logic from the SDMA v4.4.2 initialization code.
2. **Clean Up Related Code**:
- Removed the `sdma_v4_4_2_set_engine_reset_funcs` function, which was used to register the callbacks.
- Removed the `sdma_v4_4_2_engine_reset_funcs` structure, which contained the pre-reset and post-reset callback functions.
Signed-off-by: Jesse Zhang <jesse.zhang@amd.com>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 24 ------------------------
drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h | 8 --------
drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 12 ------------
3 files changed, 44 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
index dbc7c7cfee01..2d61f25528dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -531,30 +531,6 @@ bool amdgpu_sdma_is_shared_inv_eng(struct amdgpu_device *adev, struct amdgpu_rin
return false;
}
-/**
- * amdgpu_sdma_register_on_reset_callbacks - Register SDMA reset callbacks
- * @funcs: Pointer to the callback structure containing pre_reset and post_reset functions
- *
- * This function allows KFD and AMDGPU to register their own callbacks for handling
- * pre-reset and post-reset operations for engine reset. These are needed because engine
- * reset will stop all queues on that engine.
- */
-void amdgpu_sdma_register_on_reset_callbacks(struct amdgpu_device *adev, struct sdma_on_reset_funcs *funcs)
-{
- if (!funcs)
- return;
-
- /* Ensure the reset_callback_list is initialized */
- if (!adev->sdma.reset_callback_list.next) {
- INIT_LIST_HEAD(&adev->sdma.reset_callback_list);
- }
- /* Initialize the list node in the callback structure */
- INIT_LIST_HEAD(&funcs->list);
-
- /* Add the callback structure to the global list */
- list_add_tail(&funcs->list, &adev->sdma.reset_callback_list);
-}
-
static int amdgpu_sdma_soft_reset(struct amdgpu_device *adev, u32 instance_id)
{
u32 soft_reset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
index 47d56fd0589f..419531cc8207 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
@@ -103,13 +103,6 @@ struct amdgpu_sdma_ras {
struct amdgpu_ras_block_object ras_block;
};
-struct sdma_on_reset_funcs {
- int (*pre_reset)(struct amdgpu_device *adev, uint32_t instance_id);
- int (*post_reset)(struct amdgpu_device *adev, uint32_t instance_id);
- /* Linked list node to store this structure in a list; */
- struct list_head list;
-};
-
struct amdgpu_sdma {
struct amdgpu_sdma_instance instance[AMDGPU_MAX_SDMA_INSTANCES];
struct amdgpu_irq_src trap_irq;
@@ -170,7 +163,6 @@ struct amdgpu_buffer_funcs {
uint32_t byte_count);
};
-void amdgpu_sdma_register_on_reset_callbacks(struct amdgpu_device *adev, struct sdma_on_reset_funcs *funcs);
int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id);
#define amdgpu_emit_copy_buffer(adev, ib, s, d, b, t) (adev)->mman.buffer_funcs->emit_copy_buffer((ib), (s), (d), (b), (t))
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 df82a97a5388..29dbee7302c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
@@ -106,7 +106,6 @@ static void sdma_v4_4_2_set_buffer_funcs(struct amdgpu_device *adev);
static void sdma_v4_4_2_set_vm_pte_funcs(struct amdgpu_device *adev);
static void sdma_v4_4_2_set_irq_funcs(struct amdgpu_device *adev);
static void sdma_v4_4_2_set_ras_funcs(struct amdgpu_device *adev);
-static void sdma_v4_4_2_set_engine_reset_funcs(struct amdgpu_device *adev);
static void sdma_v4_4_2_update_reset_mask(struct amdgpu_device *adev);
static u32 sdma_v4_4_2_get_reg_offset(struct amdgpu_device *adev,
@@ -1351,7 +1350,6 @@ static int sdma_v4_4_2_early_init(struct amdgpu_ip_block *ip_block)
sdma_v4_4_2_set_vm_pte_funcs(adev);
sdma_v4_4_2_set_irq_funcs(adev);
sdma_v4_4_2_set_ras_funcs(adev);
- sdma_v4_4_2_set_engine_reset_funcs(adev);
return 0;
}
@@ -1739,16 +1737,6 @@ static int sdma_v4_4_2_restore_queue(struct amdgpu_device *adev, uint32_t instan
return sdma_v4_4_2_inst_start(adev, inst_mask, true);
}
-static struct sdma_on_reset_funcs sdma_v4_4_2_engine_reset_funcs = {
- .pre_reset = sdma_v4_4_2_stop_queue,
- .post_reset = sdma_v4_4_2_restore_queue,
-};
-
-static void sdma_v4_4_2_set_engine_reset_funcs(struct amdgpu_device *adev)
-{
- amdgpu_sdma_register_on_reset_callbacks(adev, &sdma_v4_4_2_engine_reset_funcs);
-}
-
static int sdma_v4_4_2_set_trap_irq_state(struct amdgpu_device *adev,
struct amdgpu_irq_src *source,
unsigned type,
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks
2025-04-02 3:02 [PATCH 1/7 V2] drm/amd/amdgpu: Simplify SDMA reset mechanism by removing dynamic callbacks jesse.zhang
` (5 preceding siblings ...)
2025-04-02 3:02 ` [PATCH 7/7 V2] drm/amd/amdgpu: Remove deprecated SDMA reset callback mechanism jesse.zhang
@ 2025-04-02 7:38 ` Christian König
6 siblings, 0 replies; 8+ messages in thread
From: Christian König @ 2025-04-02 7:38 UTC (permalink / raw)
To: jesse.zhang, amd-gfx
Cc: Alexander.Deucher, christian.koenig, jonathan.kim, jiadong.zhu
Am 02.04.25 um 05:02 schrieb jesse.zhang@amd.com:
> From: "Jesse.zhang@amd.com" <jesse.zhang@amd.com>
>
> Since KFD no longer registers its own callbacks for SDMA resets, and only KGD uses the reset mechanism,
> we can simplify the SDMA reset flow by directly calling the ring's `stop_queue` and `start_queue` functions.
> This patch removes the dynamic callback mechanism and prepares for its eventual deprecation.
>
> 1. **Remove Dynamic Callbacks**:
> - The `pre_reset` and `post_reset` callback invocations in `amdgpu_sdma_reset_engine` are removed.
> - Instead, the ring's `stop_queue` and `start_queue` functions are called directly during the reset process.
>
> 2. **Prepare for Deprecation of Dynamic Mechanism**:
> - By removing the callback invocations, this patch prepares the codebase for the eventual removal of the dynamic callback registration mechanism.
>
> Signed-off-by: Jesse Zhang <Jesse.Zhang@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 2 ++
> drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 34 +++---------------------
> drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 2 ++
> 3 files changed, 8 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> index 615c3d5c5a8d..1b66be2b49dc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -237,6 +237,8 @@ struct amdgpu_ring_funcs {
> void (*patch_ce)(struct amdgpu_ring *ring, unsigned offset);
> void (*patch_de)(struct amdgpu_ring *ring, unsigned offset);
> int (*reset)(struct amdgpu_ring *ring, unsigned int vmid);
> + int (*stop_queue)(struct amdgpu_device *adev, uint32_t instance_id);
> + int (*start_queue)(struct amdgpu_device *adev, uint32_t instance_id);
The those parameters doesn't seem to make sense here.
A specific ring is always associated with a certain instance and that here are the per ring operations.
Instead you should give the ring as parameters to the callback.
Regards,
Christian.
> void (*emit_cleaner_shader)(struct amdgpu_ring *ring);
> bool (*is_guilty)(struct amdgpu_ring *ring);
> };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> index 0a9893fee828..7d862c887a1a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
> @@ -558,16 +558,10 @@ void amdgpu_sdma_register_on_reset_callbacks(struct amdgpu_device *adev, struct
> * @adev: Pointer to the AMDGPU device
> * @instance_id: ID of the SDMA engine instance to reset
> *
> - * This function performs the following steps:
> - * 1. Calls all registered pre_reset callbacks to allow KFD and AMDGPU to save their state.
> - * 2. Resets the specified SDMA engine instance.
> - * 3. Calls all registered post_reset callbacks to allow KFD and AMDGPU to restore their state.
> - *
> * Returns: 0 on success, or a negative error code on failure.
> */
> int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id)
> {
> - struct sdma_on_reset_funcs *funcs;
> int ret = 0;
> struct amdgpu_sdma_instance *sdma_instance = &adev->sdma.instance[instance_id];
> struct amdgpu_ring *gfx_ring = &sdma_instance->ring;
> @@ -589,18 +583,8 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id)
> page_sched_stopped = true;
> }
>
> - /* Invoke all registered pre_reset callbacks */
> - list_for_each_entry(funcs, &adev->sdma.reset_callback_list, list) {
> - if (funcs->pre_reset) {
> - ret = funcs->pre_reset(adev, instance_id);
> - if (ret) {
> - dev_err(adev->dev,
> - "beforeReset callback failed for instance %u: %d\n",
> - instance_id, ret);
> - goto exit;
> - }
> - }
> - }
> + if (gfx_ring->funcs->stop_queue)
> + gfx_ring->funcs->stop_queue(adev, instance_id);
>
> /* Perform the SDMA reset for the specified instance */
> ret = amdgpu_dpm_reset_sdma(adev, 1 << instance_id);
> @@ -609,18 +593,8 @@ int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t instance_id)
> goto exit;
> }
>
> - /* Invoke all registered post_reset callbacks */
> - list_for_each_entry(funcs, &adev->sdma.reset_callback_list, list) {
> - if (funcs->post_reset) {
> - ret = funcs->post_reset(adev, instance_id);
> - if (ret) {
> - dev_err(adev->dev,
> - "afterReset callback failed for instance %u: %d\n",
> - instance_id, ret);
> - goto exit;
> - }
> - }
> - }
> + if (gfx_ring->funcs->start_queue)
> + gfx_ring->funcs->start_queue(adev, instance_id);
>
> exit:
> /* Restart the scheduler's work queue for the GFX and page rings
> 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 688a720bbbbd..df82a97a5388 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> @@ -2143,6 +2143,8 @@ static const struct amdgpu_ring_funcs sdma_v4_4_2_ring_funcs = {
> .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,
> + .stop_queue = sdma_v4_4_2_stop_queue,
> + .start_queue = sdma_v4_4_2_restore_queue,
> .is_guilty = sdma_v4_4_2_ring_is_guilty,
> };
>
^ permalink raw reply [flat|nested] 8+ messages in thread