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

[Runyu Xiao <runyu.xiao@seu.edu.cn>]

[-- Attachment #1: Type: text/plain, Size: 2610 bytes --]

>>
>> Some people who received this message don't often get email from 220255722@seu.edu.cn. 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.
>> 
>
>


[-- Attachment #2: Type: text/html, Size: 6331 bytes --]