>> >> Some people who received this message don't often get email from >> [email protected]. Learn why this is important >> <https://aka.ms/LearnAboutSenderIdentification> >> >> >> Hi Christian, >> >> Thanks, understood. >> >> To make sure I rework this in the right direction: would you expect >> this reset path to do >> >> ring->wptr = 0; >> amdgpu_ring_set_wptr(ring); >> >> instead of writing wptr_cpu_addr directly? >> >> I am asking because amdgpu_ring_set_wptr() also updates the doorbell, >> so I want to confirm that this is the intended sequence for the >> reset/suspend case here. > >Yeah I was wondering the same thing. > >I think the correct approach would be to make both rptr_cpu_addr and >wptr_cpu_addr void* in the amdgpu_ring.h structure instead of u32* and then >cast that to either (u64*) or (u32*) depending on the ring type. > >The atomic64_t hack should really be removed. > >BTW Reading the rptr is wrong on multiple instances as well and should >probably be fixed in the same patch set. > >Regards, >Christian. >
Hi Christian, Thanks for the guidance. I reworked the original idea around changing rptr_cpu_addr and wptr_cpu_addr to void * and using typed 32-bit / 64-bit accesses at the call sites, rather than using the atomic64_t cast. The patchset I have in mind is now roughly: 1. add typed ring writeback helpers 2. convert ring writeback users to those helpers 3. make ring rptr_cpu_addr / wptr_cpu_addr typeless 4. fix the MES queue-init reset path on top of that cleanup The goal is to stop mixing direct u32-typed accesses with ad hoc 64-bit casts in the writeback paths, and then handle the MES 64-bit reset case through the same typed access model. I also plan to include the related rptr read fixes in the same series, as you pointed out. Since this is a bit larger than the original 2-patch fix, I wanted to check before posting v2 whether this overall direction and split look reasonable to you, or whether you would prefer a smaller or differently split series. If you think any part of this cleanup should be handled differently, please let me know and I will adjust before posting. Thanks, Runyu >> >> Thanks, >> Runyu >> >> On Tue, Jun 2, 2026 at 11:49:05AM +0200, Christian König wrote: >>> Clear NAK. >>> >>> The atomic64_t cast hack is just something we did for older >>> generations and is not something which is necessary nor should >>> be done here. >>> >>> What could be possible is that we need to use amdgpu_ring_set_wptr() >>> here to correctly distinguish between queues with 32bit and 64bit >>> wptrs. >> > >
