[PATCH 5/6] drm/amdgpu: Disable RPTR write back for navi12

[Deng, Emily]

[AMD Official Use Only - Internal Distribution Only]

>-----Original Message-----
>From: Christian König <ckoenig.leichtzumerken at gmail.com>
>Sent: Tuesday, March 30, 2021 3:24 PM
>To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org;
>Deucher, Alexander <Alexander.Deucher at amd.com>
>Subject: Re: [PATCH 5/6] drm/amdgpu: Disable RPTR write back for navi12
>
>Am 30.03.21 um 09:20 schrieb Deng, Emily:
>> [AMD Official Use Only - Internal Distribution Only]
>>
>>> -----Original Message-----
>>> From: Christian König <ckoenig.leichtzumerken at gmail.com>
>>> Sent: Tuesday, March 30, 2021 3:13 PM
>>> To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org
>>> Subject: Re: [PATCH 5/6] drm/amdgpu: Disable RPTR write back for
>>> navi12
>>>
>>>
>>>
>>> Am 30.03.21 um 06:41 schrieb Emily Deng:
>>>> It will hit ramdomly sdma hang, and pending on utcl2 address
>>>> translation when access the RPTR polling address.
>>>>
>>>> According sdma firmware team mentioned, the RPTR writeback is done
>>>> by hardware automatically, and will hit issue when clock gating occurs.
>>>> So stop using the rptr write back for sdma5.0.
>>>>
>>>> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
>>>> ---
>>>>    drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 18 ++++++++++++------
>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>>> b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>>> index 920fc6d4a127..63e4a78181b8 100644
>>>> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>>> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
>>>> @@ -298,13 +298,19 @@ static void
>>> sdma_v5_0_ring_patch_cond_exec(struct amdgpu_ring *ring,
>>>>     */
>>>>    static uint64_t sdma_v5_0_ring_get_rptr(struct amdgpu_ring *ring)
>>>>    {
>>>> -u64 *rptr;
>>>> +struct amdgpu_device *adev = ring->adev;
>>>> +u64 rptr;
>>>> +u32 lowbit, highbit;
>>>> +
>>>> +lowbit = RREG32(sdma_v5_0_get_reg_offset(adev, ring->me,
>>> mmSDMA0_GFX_RB_RPTR));
>>>> +highbit = RREG32(sdma_v5_0_get_reg_offset(adev, ring->me,
>>>> +mmSDMA0_GFX_RB_RPTR_HI));
>>> That won't work like this.
>>>
>>> We have the readpointer writeback because we otherwise can't
>>> guarantee that the two 32bit values read from the registers are coherent.
>>>
>>> In other words it can be that the hi rptr is already wrapped around
>>> while the lo is still the old value.
>>>
>>> Why exactly doesn't the writeback work?
>>>
>>> Christian.
>> Issue occurs, when occurs clockgating, at the same time, the rptr write back
>occurs. At this time, the utcl2 translation will hang.
>
>Mhm, crap. Alex are you up to date on this bug?
>
>I'm not an expert on the SDMA, but my last status is that writeback is
>mandatory when we use 64bit rptr/wptr.
>
>Otherwise we need a workaround how to read a consistent 64bit rptr from
>two 32bit registers.
>
>Can you check the register documentation if there is any double buffering or
>stuff like that?
>
>Christian.
Hi Christian,
     Thanks to point out the inconsistent issue for 64 bit register. Please ignore this patch. Will try to fix the issue in sdma firmware.

Best wishes
Emily Deng


>
>>>> -/* XXX check if swapping is necessary on BE */ -rptr = ((u64
>>>> *)&ring->adev->wb.wb[ring->rptr_offs]);
>>>> +rptr = highbit;
>>>> +rptr = rptr << 32;
>>>> +rptr |= lowbit;
>>>>
>>>> -DRM_DEBUG("rptr before shift == 0x%016llx\n", *rptr); -return
>>>> ((*rptr) >> 2);
>>>> +DRM_DEBUG("rptr before shift == 0x%016llx\n", rptr); return (rptr
>>>> +>> 2);
>>>>    }
>>>>
>>>>    /**
>>>> @@ -702,7 +708,7 @@ static int sdma_v5_0_gfx_resume(struct
>>> amdgpu_device *adev)
>>>>    WREG32(sdma_v5_0_get_reg_offset(adev, i,
>>> mmSDMA0_GFX_RB_RPTR_ADDR_LO),
>>>>           lower_32_bits(adev->wb.gpu_addr + wb_offset) &
>>> 0xFFFFFFFC);
>>>> -rb_cntl = REG_SET_FIELD(rb_cntl, SDMA0_GFX_RB_CNTL,
>>> RPTR_WRITEBACK_ENABLE, 1);
>>>> +rb_cntl = REG_SET_FIELD(rb_cntl, SDMA0_GFX_RB_CNTL,
>>>> +RPTR_WRITEBACK_ENABLE, 0);
>>>>
>>>>    WREG32(sdma_v5_0_get_reg_offset(adev, i,
>>> mmSDMA0_GFX_RB_BASE), ring->gpu_addr >> 8);
>>>>    WREG32(sdma_v5_0_get_reg_offset(adev, i,
>>> mmSDMA0_GFX_RB_BASE_HI),
>>>> ring->gpu_addr >> 40);