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 <[email protected]>
> > ---
> > 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.
@Christian - does that sound alright to you?
>
> commit 11b0b5d942fe46bfb01f021cdb0616c8385d5ea8
> Author: Tvrtko Ursulin <[email protected]>
> 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 <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Sunil Khatri <[email protected]>
>
> 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)
>
