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 6/6] drm/amdgpu: Use memset32 for SDMA padding
Date: Wed, 09 Sep 2026 20:37:53 +0200 [thread overview]
Message-ID: <ax2H_UmvS4-HpuCRpSm7EA@gmail.com> (raw)
In-Reply-To: <20260909105215.88242-7-tvrtko.ursulin@igalia.com>
On 2026. szeptember 9., szerda 12:52:15 közép-európai nyári idő Tvrtko Ursulin
wrote:
> Instead of open coding it via the inefficient amdgpu_ring_write(), which
> which the compiler is not able to optimise much, we can add a new
> amdgpu_ring_fill() helper which pads using memset32.
Can you elaborate more on that? It seems to me that amdgpu_ring_insert_nop()
already uses memset32() so I don't see how the commit improves it.
>
> We convert the amdgpu_ring_insert_nop() used by the GFX rings and also
> the SDMA ones.
As far as I see amdgpu_ring_insert_nop() is used by all rings not just GFX and
SDMA, isn't it?
> Although with SDMA this should have much less benefit than
> with GFX (only SDMA v4.0 uses the 256 byte ring padding while the rest use
> 16), but on the other hand it should not harm and is at least more
> consistent.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
> Cc: Timur Kristóf <timur.kristof@gmail.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 17 +---------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 26 ++++++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c | 15 +++++++-------
> drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c | 15 +++++++-------
> 11 files changed, 90 insertions(+), 88 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c index 686c92e96025..38434a4c3566
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
> @@ -117,22 +117,7 @@ int amdgpu_ring_alloc(struct amdgpu_ring *ring,
> unsigned int ndw) */
> void amdgpu_ring_insert_nop(struct amdgpu_ring *ring, uint32_t count)
> {
> - uint32_t occupied, chunk1, chunk2;
> -
> - occupied = ring->wptr & ring->buf_mask;
> - chunk1 = ring->buf_mask + 1 - occupied;
> - chunk1 = (chunk1 >= count) ? count : chunk1;
> - chunk2 = count - chunk1;
> -
> - if (chunk1)
> - memset32(&ring->ring[occupied], ring->funcs->nop,
chunk1);
> -
> - if (chunk2)
> - memset32(ring->ring, ring->funcs->nop, chunk2);
> -
> - ring->wptr += count;
> - ring->wptr &= ring->ptr_mask;
> - ring->count_dw -= count;
> + amdgpu_ring_fill(ring, ring->funcs->nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h index 6b6ee4083c8d..2b1d3956cdd3
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
> @@ -522,6 +522,32 @@ static inline void amdgpu_ring_write_multiple(struct
> amdgpu_ring *ring, ring->count_dw -= count_dw;
> }
>
> +static inline void amdgpu_ring_fill(struct amdgpu_ring *ring,
> + u32 val, u32 count)
> +{
> + const u32 buf_mask = ring->buf_mask;
> + u32 occupied, chunk1, chunk2;
> + u64 wptr = ring->wptr;
> +
> + if (count == 0)
> + return;
> +
> + occupied = wptr & buf_mask;
> + chunk1 = buf_mask + 1 - occupied;
> + chunk1 = (chunk1 >= count) ? count : chunk1;
> + chunk2 = count - chunk1;
> +
> + if (chunk1)
> + memset32(&ring->ring[occupied], val, chunk1);
> +
> + if (chunk2)
> + memset32(ring->ring, val, chunk2);
> +
> + wptr += count;
> + ring->wptr = wptr & ring->ptr_mask;
> + ring->count_dw -= count;
> +}
> +
> static inline unsigned int amdgpu_ring_get_dw_distance(struct amdgpu_ring
> *ring, u64 start_wptr, u64 end_wptr)
> {
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c index 006f3fd3464a..e461e0236b5f
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
> @@ -224,15 +224,14 @@ static void sdma_v2_4_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v2_4_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c index 3fb15032e1ef..5cca7f715c6a
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
> @@ -401,15 +401,14 @@ static void sdma_v3_0_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v3_0_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c index dfb0ea709bad..63bdf8d4e8d5
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -784,15 +784,14 @@ static void sdma_v4_0_page_ring_set_wptr(struct
> amdgpu_ring *ring) static void sdma_v4_0_ring_insert_nop(struct amdgpu_ring
> *ring, uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> 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 f2e6abe44a43..145f862cbcc6
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_4_2.c
> @@ -350,15 +350,14 @@ static void sdma_v4_4_2_page_ring_set_wptr(struct
> amdgpu_ring *ring) static void sdma_v4_4_2_ring_insert_nop(struct
> amdgpu_ring *ring, uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c index cb36b38582c5..124bc5768983
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -407,15 +407,14 @@ static void sdma_v5_0_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v5_0_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c index 2858820bb864..242586ea951a
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
> @@ -255,15 +255,14 @@ static void sdma_v5_2_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v5_2_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c index d3504606bee7..ae063ac3bd74
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v6_0.c
> @@ -243,15 +243,14 @@ static void sdma_v6_0_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v6_0_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c index 1760f03db9e7..fd92830d3324
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_0.c
> @@ -245,15 +245,14 @@ static void sdma_v7_0_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v7_0_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c
> b/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c index b9f11f2f7e5c..c0824c83ace9
> 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v7_1.c
> @@ -239,15 +239,14 @@ static void sdma_v7_1_ring_set_wptr(struct amdgpu_ring
> *ring) static void sdma_v7_1_ring_insert_nop(struct amdgpu_ring *ring,
> uint32_t count) {
> struct amdgpu_sdma_instance *sdma =
> amdgpu_sdma_get_instance_from_ring(ring); - const bool burst_nop =
> sdma->burst_nop;
> - int i;
> + const u32 nop = ring->funcs->nop;
>
> - for (i = 0; i < count; i++)
> - if (i == 0 && burst_nop)
> - amdgpu_ring_write(ring, ring->funcs->nop |
> - SDMA_PKT_NOP_HEADER_COUNT(count -
1));
> - else
> - amdgpu_ring_write(ring, ring->funcs->nop);
> + if (sdma->burst_nop) {
> + --count;
> + amdgpu_ring_write(ring, nop |
SDMA_PKT_NOP_HEADER_COUNT(count));
> + }
> +
> + amdgpu_ring_fill(ring, nop, count);
> }
>
> /**
next prev parent reply other threads:[~2026-09-10 13:32 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
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 [this message]
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=ax2H_UmvS4-HpuCRpSm7EA@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.