From: "Timur Kristóf" <timur.kristof@gmail.com>
To: amd-gfx@lists.freedesktop.org,
Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: kernel-dev@igalia.com, Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Subject: Re: [PATCH 1/6] drm/amdgpu: Add SDMA ring init helper
Date: Wed, 09 Sep 2026 20:27:49 +0200 [thread overview]
Message-ID: <s5-Dk6INQlqq93576HqTlw@gmail.com> (raw)
In-Reply-To: <20260909105215.88242-2-tvrtko.ursulin@igalia.com>
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 <tvrtko.ursulin@igalia.com>
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> ---
> 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.
> +{
> + int r;
> +
> + ring->ring_obj = NULL;
Maybe also consider moving here the following lines:
ring->me = instance_id;
ring->use_doorbell = ... // based on SDMA IP version
ring->no_user_submission = adev->sdma.no_user_submission;
> +
> + 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);
> +
> + 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 =
next prev parent reply other threads:[~2026-09-10 13:25 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-09 10:52 [PATCH 0/6] A bit of SDMA (mostly) streamlining Tvrtko Ursulin
2026-09-09 10:52 ` [PATCH 1/6] drm/amdgpu: Add SDMA ring init helper Tvrtko Ursulin
2026-09-09 18:27 ` Timur Kristóf [this message]
2026-09-10 14:38 ` Tvrtko Ursulin
2026-09-09 10:52 ` [PATCH 2/6] drm/amdgpu: Add amdgpu_sdma_types.h header Tvrtko Ursulin
2026-09-09 18:28 ` Timur Kristóf
2026-09-10 14:39 ` Tvrtko Ursulin
2026-09-09 10:52 ` [PATCH 3/6] drm/amdgpu: Convert SDMA instance and index to direct lookup Tvrtko Ursulin
2026-09-09 10:52 ` [PATCH 4/6] drm/amdgpu: Cache the SDMA CSA address Tvrtko Ursulin
2026-09-09 10:52 ` [PATCH 5/6] drm/amdgpu: Extend logical to device instance lookup to all devices Tvrtko Ursulin
2026-09-09 10:52 ` [PATCH 6/6] drm/amdgpu: Use memset32 for SDMA padding Tvrtko Ursulin
2026-09-09 18:37 ` Timur Kristóf
2026-09-10 14:46 ` Tvrtko Ursulin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=s5-Dk6INQlqq93576HqTlw@gmail.com \
--to=timur.kristof@gmail.com \
--cc=amd-gfx@lists.freedesktop.org \
--cc=kernel-dev@igalia.com \
--cc=tvrtko.ursulin@igalia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.