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;
