RE: [PATCH 5/6] drm/amdgpu: Disable RPTR write back for navi12
[AMD Official Use Only - Internal Distribution Only] >-Original Message- >From: Christian König >Sent: Tuesday, March 30, 2021 3:24 PM >To: Deng, Emily ; amd-gfx@lists.freedesktop.org; >Deucher, Alexander >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 >>> Sent: Tuesday, March 30, 2021 3:13 PM >>> To: Deng, Emily ; amd-gfx@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 >>>> --- >>>>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 >>>> *)>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) & >>> 0xFFFC); >>>> -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); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
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 Sent: Tuesday, March 30, 2021 3:13 PM To: Deng, Emily ; amd-gfx@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 --- 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. -/* XXX check if swapping is necessary on BE */ -rptr = ((u64 *)>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) & 0xFFFC); -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); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
RE: [PATCH 5/6] drm/amdgpu: Disable RPTR write back for navi12
[AMD Official Use Only - Internal Distribution Only] >-Original Message- >From: Christian König >Sent: Tuesday, March 30, 2021 3:13 PM >To: Deng, Emily ; amd-gfx@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 >> --- >> 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. > >> >> -/* XXX check if swapping is necessary on BE */ >> -rptr = ((u64 *)>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) & >0xFFFC); >> >> -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); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
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 --- 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. - /* XXX check if swapping is necessary on BE */ - rptr = ((u64 *)>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) & 0xFFFC); - 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); ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx