Re: [PATCH 1/6] drm/amdgpu: Add SDMA ring init helper

Thu, 10 Sep 2026 08:28:09 -0700 


On 09/09/2026 19:27, Timur Kristóf wrote:

On 2026. szeptember 9., szerda 12:52:10 közép-európai nyári idő Tvrtko Ursulin
wrote:

Consolidate one part of the SDMA ring initialization with a new
amdgpu_sdma_ring_init() helper.

Signed-off-by: Tvrtko Ursulin <[email protected]>
Cc: Timur Kristóf <[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c | 19 +++++++++++++++++++
  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  3 +++
  drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  7 +------
  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  7 +------
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   | 13 ++-----------
  drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 19 ++++++-------------
  drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   |  7 +------
  drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c   |  6 +-----
  drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c   |  8 ++------
  drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c   |  7 +------
  drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c   | 13 +++++--------
  11 files changed, 42 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c index fbac732f3e01..cca8b3a98f6a
100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.c
@@ -37,6 +37,25 @@
   * GPU SDMA IP block helpers function.
   */

+int amdgpu_sdma_ring_init(struct amdgpu_device *adev, struct amdgpu_ring
*ring, +                          const char *prefix, unsigned int index)


Instead of "index", I suggest "instance_id" to make it clear what it is.
Additionally please add an xcc_id argument because some SDMA versions seem to
need that too.



I can use instance_id although then it will be incosistent with the two 
helpers just below it which call it index.



Regarding xcc_id I didn't to pull in thing which are IP version 
specific. With the expection of the name, will comment on that a bit lower.





+{
+       int r;
+
+       ring->ring_obj = NULL;


Maybe also consider moving here the following lines:
ring->me = instance_id;


I considered this but noticed ring->me assignment is a bit messy.


Basically it is done it early init via *_set_ring_funcs(). And I think 
this is required for ordering.



Some IP versions then set in again *_sw_init() but I don't see how that 
is required. Shall I add a patch to remove those instead of initializing 
ring->me in this new helper?



ring->use_doorbell = ... // based on SDMA IP version



I didn't want to pull in anything which would require branching on IP 
version.



ring->no_user_submission = adev->sdma.no_user_submission;



ring->no_user_submission I haven't looked at. If the default would be 
the same on IPs which currently leave it at default of false.. Let me 
see.. It looks it would be safe becuase both adev and sdma instance 
struct allocations are zeroed. Don't know somehow I would be nervous of 
baking this assumption in.





+
+       if (prefix)
+               sprintf(ring->name, "sdma%u", index);


Seems like this wouldn't name the page queues correctly.
Suggestion:

sprintf(ring->name, "%s%u.%u", prefix ?: "sdma", xcc_id, instance_id);



This is the exception to my rule in that I left those callers override 
it. For 7.1:


                r = amdgpu_sdma_ring_init(adev, ring, NULL, i);
                if (r)
                        return r;

                sprintf(ring->name, "sdma%d.%d", xcc_id,
                        GET_INST(SDMA0, i) % adev->sdma.num_inst_per_xcc);
        }

And 4.4.2:

                ring = &adev->sdma.instance[i].ring;
...
                r = amdgpu_sdma_ring_init(adev, ring, NULL, i);
...
                sprintf(ring->name, "sdma%d.%d", aid_id,
                                i % adev->sdma.num_inst_per_aid);


                        ring = &adev->sdma.instance[i].page;
...
                        r = amdgpu_sdma_ring_init(adev, ring, NULL, i);
...
                        sprintf(ring->name, "page%d.%d", aid_id,
                                        i % adev->sdma.num_inst_per_aid);


Maybe I try to make amdgpu_sdma_ring_init variadic and so the format and 
args can be passed in?


Regards,

Tvrtko





+
+       r = amdgpu_ring_init(adev, ring, 1024, &adev->sdma.trap_irq,
+                            AMDGPU_SDMA_IRQ_INSTANCE0 + index,
+                            AMDGPU_RING_PRIO_DEFAULT, NULL);
+       if (r)
+               return r;
+
+       return 0;
+}
+
  struct amdgpu_sdma_instance *amdgpu_sdma_get_instance_from_ring(struct
amdgpu_ring *ring) {
        struct amdgpu_device *adev = ring->adev;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h index 671cfbb67b7a..2f1edef97c2f
100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
@@ -158,6 +158,9 @@ struct amdgpu_buffer_funcs {
                                 uint32_t byte_count);
  };

+int amdgpu_sdma_ring_init(struct amdgpu_device *adev, struct amdgpu_ring
*ring, +                          const char *prefix, unsigned int index);
+
  int amdgpu_sdma_reset_engine(struct amdgpu_device *adev, uint32_t
instance_id, bool caller_handles_kernel_queues);

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c index 657ef6c93c61..fb2047d8e25a
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -865,13 +865,8 @@ static int sdma_v2_4_sw_init(struct amdgpu_ip_block
*ip_block)

        for (i = 0; i < adev->sdma.num_instances; i++) {
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = false;
-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024, &adev-
sdma.trap_irq,
-                                    (i == 0) ?

AMDGPU_SDMA_IRQ_INSTANCE0 :

-                                    AMDGPU_SDMA_IRQ_INSTANCE1,
-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c index 9478dd034aff..656f66527999
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -1146,7 +1146,6 @@ static int sdma_v3_0_sw_init(struct amdgpu_ip_block
*ip_block)

        for (i = 0; i < adev->sdma.num_instances; i++) {
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                if (!amdgpu_sriov_vf(adev)) {
                        ring->use_doorbell = true;
                        ring->doorbell_index = adev-
doorbell_index.sdma_engine[i];
@@ -1154,11 +1153,7 @@ static int sdma_v3_0_sw_init(struct amdgpu_ip_block
*ip_block) ring->use_pollmem = true;
                }

-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024, &adev-
sdma.trap_irq,
-                                    (i == 0) ?

AMDGPU_SDMA_IRQ_INSTANCE0 :

-                                    AMDGPU_SDMA_IRQ_INSTANCE1,
-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index 9d7d919a5aa1..e135dfb1c3e2
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1858,7 +1858,6 @@ static int sdma_v4_0_sw_init(struct amdgpu_ip_block
*ip_block)

        for (i = 0; i < adev->sdma.num_instances; i++) {
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;

                DRM_DEBUG("SDMA %d use_doorbell being set to: [%s]\n",

i,

@@ -1878,16 +1877,12 @@ static int sdma_v4_0_sw_init(struct amdgpu_ip_block
*ip_block) else
                        ring->vm_hub = AMDGPU_MMHUB0(0);

-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024, &adev-
sdma.trap_irq,
-                                    AMDGPU_SDMA_IRQ_INSTANCE0 +

i,

-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;

                if (adev->sdma.has_page_queue) {
                        ring = &adev->sdma.instance[i].page;
-                       ring->ring_obj = NULL;
                        ring->use_doorbell = true;

                        /* paging queue use same doorbell index/

routing as gfx queue

@@ -1915,11 +1910,7 @@ static int sdma_v4_0_sw_init(struct amdgpu_ip_block
*ip_block) else
                                ring->vm_hub = AMDGPU_MMHUB0(0);

-                       sprintf(ring->name, "page%d", i);
-                       r = amdgpu_ring_init(adev, ring, 1024,
-                                            &adev-
sdma.trap_irq,
-                                       

AMDGPU_SDMA_IRQ_INSTANCE0 + i,

-                                       

AMDGPU_RING_PRIO_DEFAULT, NULL);

+                       r = amdgpu_sdma_ring_init(adev, ring, "page",

i);

                        if (r)
                                return r;
                }
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 461f8b220a3e..2e46e63a6dbf
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
@@ -1484,7 +1484,6 @@ static int sdma_v4_4_2_sw_init(struct amdgpu_ip_block
*ip_block) adev->sdma.instance[i].funcs = &sdma_v4_4_2_sdma_funcs;

                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;
                aid_id = adev->sdma.instance[i].aid_id;

@@ -1495,18 +1494,15 @@ static int sdma_v4_4_2_sw_init(struct
amdgpu_ip_block *ip_block) ring->doorbell_index =
adev->doorbell_index.sdma_engine[i] << 1; ring->vm_hub =
AMDGPU_MMHUB0(aid_id);
                ring->no_user_submission = adev-
sdma.no_user_submission;
+               r = amdgpu_sdma_ring_init(adev, ring, NULL, i);
+               if (r)
+                       return r;

                sprintf(ring->name, "sdma%d.%d", aid_id,
                                i % adev->sdma.num_inst_per_aid);
-               r = amdgpu_ring_init(adev, ring, 1024, &adev-
sdma.trap_irq,
-                                    AMDGPU_SDMA_IRQ_INSTANCE0 +

i,

-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

-               if (r)
-                       return r;

                if (adev->sdma.has_page_queue) {
                        ring = &adev->sdma.instance[i].page;
-                       ring->ring_obj = NULL;
                        ring->use_doorbell = true;

                        /* doorbell index of page queue is assigned

right after

@@ -1515,15 +1511,12 @@ static int sdma_v4_4_2_sw_init(struct
amdgpu_ip_block *ip_block) ring->doorbell_index =
                                (adev-
doorbell_index.sdma_engine[i] + 1) << 1;
                        ring->vm_hub = AMDGPU_MMHUB0(aid_id);
+                       r = amdgpu_sdma_ring_init(adev, ring, NULL,

i);

+                       if (r)
+                               return r;

                        sprintf(ring->name, "page%d.%d", aid_id,
                                        i % adev-
sdma.num_inst_per_aid);
-                       r = amdgpu_ring_init(adev, ring, 1024,
-                                            &adev-
sdma.trap_irq,
-                                       

AMDGPU_SDMA_IRQ_INSTANCE0 + i,

-                                       

AMDGPU_RING_PRIO_DEFAULT, NULL);

-                       if (r)
-                               return r;
                }
        }

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c index 97fee70dc2f6..a0614fa9ffa6
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1411,7 +1411,6 @@ static int sdma_v5_0_sw_init(struct amdgpu_ip_block
*ip_block) mutex_init(&adev->sdma.instance[i].engine_reset_mutex);
                adev->sdma.instance[i].funcs = &sdma_v5_0_sdma_funcs;
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;

                DRM_DEBUG("SDMA %d use_doorbell being set to: [%s]\n",

i,

@@ -1422,11 +1421,7 @@ static int sdma_v5_0_sw_init(struct amdgpu_ip_block
*ip_block)
                        : (adev->doorbell_index.sdma_engine[1] <<

1); // get DWORD offset


                ring->vm_hub = AMDGPU_GFXHUB(0);
-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024, &adev-
sdma.trap_irq,
-                                    (i == 0) ?

AMDGPU_SDMA_IRQ_INSTANCE0 :

-                                    AMDGPU_SDMA_IRQ_INSTANCE1,
-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c index 35cdf6c149f8..5b3dafc194d7
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -1331,7 +1331,6 @@ static int sdma_v5_2_sw_init(struct amdgpu_ip_block
*ip_block) mutex_init(&adev->sdma.instance[i].engine_reset_mutex);
                adev->sdma.instance[i].funcs = &sdma_v5_2_sdma_funcs;
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;
                ring->me = i;

@@ -1342,10 +1341,7 @@ static int sdma_v5_2_sw_init(struct amdgpu_ip_block
*ip_block) (adev->doorbell_index.sdma_engine[i] << 1); //get DWORD offset

                ring->vm_hub = AMDGPU_GFXHUB(0);
-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024, &adev-
sdma.trap_irq,
-                                    AMDGPU_SDMA_IRQ_INSTANCE0 +

i,

-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c index 303fd7d1b7c8..845e622d3c1a
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
@@ -1335,7 +1335,6 @@ static int sdma_v6_0_sw_init(struct amdgpu_ip_block
*ip_block)

        for (i = 0; i < adev->sdma.num_instances; i++) {
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;
                ring->me = i;
                ring->no_user_submission = adev-
sdma.no_user_submission;
@@ -1347,11 +1346,8 @@ static int sdma_v6_0_sw_init(struct amdgpu_ip_block
*ip_block) (adev->doorbell_index.sdma_engine[i] << 1); // get DWORD offset

                ring->vm_hub = AMDGPU_GFXHUB(0);
-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024,
-                                    &adev->sdma.trap_irq,
-                                    AMDGPU_SDMA_IRQ_INSTANCE0 +

i,

-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+
+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c index d5552f206e4d..ea460a19b89d
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
@@ -1319,7 +1319,6 @@ static int sdma_v7_0_sw_init(struct amdgpu_ip_block
*ip_block)

        for (i = 0; i < adev->sdma.num_instances; i++) {
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;
                ring->me = i;
                ring->no_user_submission = adev-
sdma.no_user_submission;
@@ -1331,11 +1330,7 @@ static int sdma_v7_0_sw_init(struct amdgpu_ip_block
*ip_block) (adev->doorbell_index.sdma_engine[i] << 1); // get DWORD offset

                ring->vm_hub = AMDGPU_GFXHUB(0);
-               sprintf(ring->name, "sdma%d", i);
-               r = amdgpu_ring_init(adev, ring, 1024,
-                                    &adev->sdma.trap_irq,
-                                    AMDGPU_SDMA_IRQ_INSTANCE0 +

i,

-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

+               r = amdgpu_sdma_ring_init(adev, ring, "sdma", i);
                if (r)
                        return r;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c
b/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c index 0f30eb503c2a..1704d406c34a
100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c
@@ -1305,7 +1305,6 @@ static int sdma_v7_1_sw_init(struct amdgpu_ip_block
*ip_block)

        for (i = 0; i < adev->sdma.num_instances; i++) {
                ring = &adev->sdma.instance[i].ring;
-               ring->ring_obj = NULL;
                ring->use_doorbell = true;
                ring->me = i;
                ring->no_user_submission = adev-
sdma.no_user_submission;
@@ -1323,14 +1322,12 @@ static int sdma_v7_1_sw_init(struct amdgpu_ip_block
*ip_block) (adev->doorbell_index.sdma_engine[i] << 1); // get DWORD offset

                ring->vm_hub = AMDGPU_GFXHUB(xcc_id);
+               r = amdgpu_sdma_ring_init(adev, ring, NULL, i);
+               if (r)
+                       return r;
+
                sprintf(ring->name, "sdma%d.%d", xcc_id,
-                               GET_INST(SDMA0, i) % adev-
sdma.num_inst_per_xcc);
-               r = amdgpu_ring_init(adev, ring, 1024,
-                                    &adev->sdma.trap_irq,
-                                    AMDGPU_SDMA_IRQ_INSTANCE0 +

i,

-                                    AMDGPU_RING_PRIO_DEFAULT,

NULL);

-               if (r)
-                       return r;
+                       GET_INST(SDMA0, i) % adev-
sdma.num_inst_per_xcc);
        }

        adev->sdma.supported_reset =




























					Reply via email to