On 02/09/2025 12:30, Timur Kristóf wrote:
On Mon, 2025-09-01 at 11:13 +0100, Tvrtko Ursulin wrote:

Hi,

On 01/09/2025 11:00, Timur Kristóf wrote:
Technically not necessary, but clear the extra dwords too,
so that the command processors don't read uninitialized memory.

Fixes: c8c1a1d2ef04 ("drm/amdgpu: define and add extra dword for
jpeg ring")
Signed-off-by: Timur Kristóf <timur.kris...@gmail.com>
---
   drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h | 5 +++++
   1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 7670f5d82b9e..6a55a85744a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -474,6 +474,11 @@ static inline void
amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
        while (i <= ring->buf_mask)
                ring->ring[i++] = ring->funcs->nop;
+ /* Technically not necessary, but clear the extra dwords
too,
+        * so that the command processors don't read uninitialized
memory.
+        */
+       for (i = 0; i < ring->funcs->extra_dw; i++)
+               ring->ring[ring->ring_size / 4 + i] = ring->funcs-
nop;

Should I resend this maybe?


Hi Tvrtko,

The patch you commented on is going to be dropped.

However, your patch makes good sense, so I can include it in the next
version of this series if that's OK.

Yes, (more than) fine by me. Thanks!

Tvrtko


@Christian - does that sound alright to you?



commit 11b0b5d942fe46bfb01f021cdb0616c8385d5ea8
Author: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
Date:   Thu Dec 26 16:12:37 2024 +0000

      drm/amdgpu: Use memset32 for ring clearing

      Use memset32 instead of open coding it, just because it is
      a tiny bit nicer.

      Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
      Cc: Christian König <christian.koe...@amd.com>
      Cc: Sunil Khatri <sunil.kha...@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index dee5a1b4e572..96bfc0c23413 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -369,10 +369,7 @@ static inline void
amdgpu_ring_set_preempt_cond_exec(struct amdgpu_ring *ring,

   static inline void amdgpu_ring_clear_ring(struct amdgpu_ring *ring)
   {
-       int i = 0;
-       while (i <= ring->buf_mask)
-               ring->ring[i++] = ring->funcs->nop;
-
+       memset32(ring->ring, ring->funcs->nop, ring->buf_mask + 1);
   }

   static inline void amdgpu_ring_write(struct amdgpu_ring *ring,
uint32_t v)

Looks like with two loops it would made even more sense to
consolidate.

Regards,

Tvrtko

   }
  static inline void amdgpu_ring_write(struct amdgpu_ring *ring,
uint32_t v)


Reply via email to