On 15/07/2026 11:16, Timur Kristóf wrote:
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.
Yes please, it's easier to review if commit messages always explain the
what, how and why. It does not have to be long when it is simple like in
this patch.
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.)
Ack. Then just improve the commit message please - instead "Additionally
clear the ring contents during reset", have something like "Move the
existing clearing of ring content to happen only during reset because it
is otherwise already done during ring init".
Presumably this is also true during resume?
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().
Specifically about wptr_cpu_addr and rptr_cpu_addr.
1)
Does gfx7 have 64-bit support or normal write would do? Ie. I don't see
gfx7 code using the atomic64. 64-bit support seems to start in gfx9.
2)
Does rptr_cpu_addr need touching at all? Out of all other files only MES
does it. At least if my quick grep is to be trusted.
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?
It is defined as normal u32 * and then, AFAIU, different ip version use
32- or 64-bit access depending on whether the respective ip version has
support for 64-bit ring pointers. The ugly casting should be hidden in
wrappers as minimum. Another task for a rainy day.
Regards,
Tvrtko
+
+ 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;
