Re: [PATCH 6/9] drm/amdgpu/gfx7: Clean up gfx ring during reset

On 2026. július 15., szerda 11:18:19 közép-európai nyári idő Tvrtko Ursulin 
wrote:
> On 13/07/2026 13:58, Timur Kristóf wrote:
> > Clear the WPTR and RPTR at ring initialization.
> > Additionally clear the ring contents during reset.
> 
> Please add the why part to your commit messages. ;)

After a reset, the ring contents could be "dirty" and thus need to be cleared 
to prevent the command processor from executing packets left over in the ring 
from before the reset. I think this is obvious, but I'm happy to add some text 
to the commit message to explain it more.

> 
> Cover letter mentions the series is reworking to match gfx7 to gfx8 but
> I looked in drm-tip and amd-staging-drm-next and
> gfx_v8_0_cp_gfx_resume() does not yet have these changes.

The GFX8 code does have this:

For graphics queues:
In gfx_v8_0_cp_gfx_resume() it also sets wptr = 0; and calls 
amdgpu_ring_clear_ring() unconditionally. This is actually not needed during 
first initialization because the amdgpu_ring_init() called from 
gfx_v8_0_sw_init() already clears the ring, which is why I call the clear 
conditionally on GFX6-7. I could of course change the GFX6-7 code to also call 
it unconditionally though for the sake of simplicity and consistency.
(It is missing the atomic things though. I think it should have that too.)

For compute queues:
In gfx_v8_0_kcq_init_queue() it clears the wptr, also the wptr_cpu_addr and 
also calls amdgpu_ring_clear_ring().


> 
> > Signed-off-by: Timur Kristóf <[email protected]>
> > ---
> > 
> >   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 9 ++++++++-
> >   1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c index a93cc02c3400..915612628f9a
> > 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -2546,8 +2546,14 @@ static int gfx_v7_0_cp_gfx_resume(struct
> > amdgpu_device *adev)> 
> >   	WREG32(mmSCRATCH_ADDR, 0);
> >   	
> >   	/* ring 0 - compute and gfx */
> > 
> > -	/* Set ring buffer size */
> > 
> >   	ring = &adev->gfx.gfx_ring[0];
> > 
> > +	atomic64_set((atomic64_t *)ring->wptr_cpu_addr, 0);
> > +	atomic64_set((atomic64_t *)ring->rptr_cpu_addr, 0);
> 
> One day we need to fix this whole atomic64 situation.

What is the situation that needs to be fixed?


> 
> > +
> > +	if (amdgpu_in_reset(adev))
> > +		amdgpu_ring_clear_ring(ring);
> > +
> > +	/* Set ring buffer size */
> > 
> >   	rb_bufsz = order_base_2(ring->ring_size / 8);
> >   	tmp = (order_base_2(AMDGPU_GPU_PAGE_SIZE/8) << 8) | rb_bufsz;
> >   
> >   #ifdef __BIG_ENDIAN
> > 
> > @@ -2559,6 +2565,7 @@ static int gfx_v7_0_cp_gfx_resume(struct
> > amdgpu_device *adev)> 
> >   	WREG32(mmCP_RB0_CNTL, tmp | CP_RB0_CNTL__RB_RPTR_WR_ENA_MASK);
> >   	ring->wptr = 0;
> >   	WREG32(mmCP_RB0_WPTR, lower_32_bits(ring->wptr));
> > 
> > +	WREG32(mmCP_RB0_RPTR, lower_32_bits(ring->wptr));
> > 
> >   	/* set the wb address whether it's enabled or not */
> >   	rptr_addr = ring->rptr_gpu_addr;