[AMD Official Use Only - Internal Distribution Only]
The logic in sdma2.4, sdma3 and sdma4 look fine.
I sent v2 to add fix for sdma5.2 as well.
BR,
Xiaojie
________________________________________
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Tuesday, July 14, 2020 4:47 PM
To: Yuan, Xiaojie; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Ma, Le
Subject: Re: [PATCH] drm/amdgpu/sdma5: fix wptr overwritten in ->get_wptr()
Am 14.07.20 um 10:09 schrieb Xiaojie Yuan:
> "u64 *wptr" points to the the wptr value in write back buffer and
> "*wptr = (*wptr) >> 2;" results in the value being overwritten each time
> when ->get_wptr() is called.
>
> umr uses /sys/kernel/debug/dri/0/amdgpu_ring_sdma0 to get rptr/wptr and
> decode ring content and it is affected by this issue.
>
> fix and simplify the logic similar as sdma_v4_0_ring_get_wptr().
>
> Suggested-by: Le Ma <le.ma@xxxxxxx>
> Signed-off-by: Xiaojie Yuan <xiaojie.yuan@xxxxxxx>
Nice, catch. I'm wondering how this code ever came to be.
Patch is Reviewed-by: Christian König <christian.koenig@xxxxxxx>
Can you please double check that we don't have that nonsense in
sdma_v4_0 or even older as well?
Thanks,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 26 ++++++++------------------
> 1 file changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index abb0ab653b10..e2232dd12d8e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -315,30 +315,20 @@ static uint64_t sdma_v5_0_ring_get_rptr(struct amdgpu_ring *ring)
> static uint64_t sdma_v5_0_ring_get_wptr(struct amdgpu_ring *ring)
> {
> struct amdgpu_device *adev = ring->adev;
> - u64 *wptr = NULL;
> - uint64_t local_wptr = 0;
> + u64 wptr;
>
> if (ring->use_doorbell) {
> /* XXX check if swapping is necessary on BE */
> - wptr = ((u64 *)&adev->wb.wb[ring->wptr_offs]);
> - DRM_DEBUG("wptr/doorbell before shift == 0x%016llx\n", *wptr);
> - *wptr = (*wptr) >> 2;
> - DRM_DEBUG("wptr/doorbell after shift == 0x%016llx\n", *wptr);
> + wptr = READ_ONCE(*((u64 *)&adev->wb.wb[ring->wptr_offs]));
> + DRM_DEBUG("wptr/doorbell before shift == 0x%016llx\n", wptr);
> } else {
> - u32 lowbit, highbit;
> -
> - wptr = &local_wptr;
> - lowbit = RREG32(sdma_v5_0_get_reg_offset(adev, ring->me, mmSDMA0_GFX_RB_WPTR)) >> 2;
> - highbit = RREG32(sdma_v5_0_get_reg_offset(adev, ring->me, mmSDMA0_GFX_RB_WPTR_HI)) >> 2;
> -
> - DRM_DEBUG("wptr [%i]high== 0x%08x low==0x%08x\n",
> - ring->me, highbit, lowbit);
> - *wptr = highbit;
> - *wptr = (*wptr) << 32;
> - *wptr |= lowbit;
> + wptr = RREG32(sdma_v5_0_get_reg_offset(adev, ring->me, mmSDMA0_GFX_RB_WPTR_HI));
> + wptr = wptr << 32;
> + wptr |= RREG32(sdma_v5_0_get_reg_offset(adev, ring->me, mmSDMA0_GFX_RB_WPTR));
> + DRM_DEBUG("wptr before shift [%i] wptr == 0x%016llx\n", ring->me, wptr);
> }
>
> - return *wptr;
> + return wptr >> 2;
> }
>
> /**
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
