Re:Re: [PATCH 1/2] drm/amdgpu/mes11: fix queue init wptr reset

[Runyu Xiao]

Hi Christian,


Thanks, that makes sense.


I will rework this around typed rptr/wptr_cpu_addr accesses instead of
the atomic64_t cast, and I will include the related rptr fixes in the
same patch set.


Thanks,
Runyu
>> 	
>> 
>> 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.
>
>> 
>> 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.
>> 
>
>

>> 	
>> 
>> 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.
>
>> 
>> 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.
>> 
>
>