Skip to contentSkip to conversation

drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

PATCH In review Pixel Ding 3 participants 5 messages Sep 25, 2017 3 messages not in the archive

Subsystems (2)
DRM drivers Radeon and amdgpu DRM drivers
lore
Share & tools
lore

Pixel Ding

author
#0 lore raw

PATCH 1/1 drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Both Tonga and Vega register SPECs indicate that this registers only use 31:2 bits in DW. SRIOV test case immediately fails withtout this shift.

Signed-off-by Pixel Ding <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 72f31cc..947f019 100644
@@ -714,7 +714,7 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -1
index c26d205..26d7f03 100644
@@ -665,7 +665,7 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
Signature · 6 lines
2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 1506320151-7386-1-git-send-email-Pixel.Ding-5C7GfCeVMHo@public.gmane.org

#2 lore raw

Hold on,

We didn't hit test fail without your patch, actually at least VEGA10 doesn't have the issue you mentioned, Can you elaborate what issue or test case you can fix with this patch ?
Besides, please don't change anything on vega10 before you verified it

BR Monk

-----Original Message-----
From: Pixel Ding [mailto:[email protected]]
Sent: Monday, September 25, 2017 2:16 PM
To: [email protected]; Ding, Pixel <[email protected]>; Min, Frank <[email protected]>; Liu, Monk <[email protected]>; Deucher, Alexander <[email protected]>
Subject: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Both Tonga and Vega register SPECs indicate that this registers only use 31:2 bits in DW. SRIOV test case immediately fails withtout this shift.

Signed-off-by Pixel Ding <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 2 +-  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 72f31cc..947f019 100644
@@ -714,7 +714,7 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]); diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index c26d205..26d7f03 100644

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -1
@@ -665,7 +665,7 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));

--
2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): BLUPR12MB0449E6B2520C186E65DBE477847A0-7LeqcoF/[email protected]

author
#4 lore raw

Hi Monk,

The world switch gets immediately fail. According to Xiangliang’s comment, I think 17.50 also has this issue. Other G branch uses original patch from Frank, that doesn’t have this issue. Please confirm this.

Also refer to http://adcweb02.amd.com/orlvalid/regspec/web_regspec/vega11/regspec/vega11_chip/public/index.html

SDMA0_GFX_RB_WPTR_POLL_ADDR_LO
ADDR 31:2

I update a v2 patch for this. I think it must fail if you really overwrite the address to 31:0.

—
Sincerely Yours,
Pixel

Quoted 47 lines · Monk

Hold on,

We didn't hit test fail without your patch, actually at least VEGA10 doesn't have the issue you mentioned, Can you elaborate what issue or test case you can fix with this patch ?
Besides, please don't change anything on vega10 before you verified it

BR Monk

-----Original Message-----
From: Pixel Ding [mailto:[email protected]]
Sent: Monday, September 25, 2017 2:16 PM
To: [email protected]; Ding, Pixel <[email protected]>; Min, Frank <[email protected]>; Liu, Monk <[email protected]>; Deucher, Alexander <[email protected]>
Subject: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Both Tonga and Vega register SPECs indicate that this registers only use 31:2 bits in DW. SRIOV test case immediately fails withtout this shift.

Signed-off-by Pixel Ding <[email protected]>

---
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-

2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 72f31cc..947f019 100644
@@ -714,7 +714,7 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]); diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index c26d205..26d7f03 100644

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -2
@@ -665,7 +665,7 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
--
Quoted 2 lines

2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): D1BCE7C1-EC7F-4DF3-98EF-AEEB376C5A29-5C7GfCeVMHo@public.gmane.org

#6 lore raw

RB: Xiangliang Yu <[email protected]>

-----Original Message-----
From: Ding, Pixel
Sent: Monday, September 25, 2017 2:42 PM
To: Liu, Monk <[email protected]>; [email protected]; Min, Frank <[email protected]>; Deucher, Alexander <[email protected]>; Yu, Xiangliang <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Hi Monk,

The world switch gets immediately fail. According to Xiangliang’s comment, I think 17.50 also has this issue. Other G branch uses original patch from Frank, that doesn’t have this issue. Please confirm this.

Also refer to http://adcweb02.amd.com/orlvalid/regspec/web_regspec/vega11/regspec/vega11_chip/public/index.html

SDMA0_GFX_RB_WPTR_POLL_ADDR_LO
ADDR 31:2

I update a v2 patch for this. I think it must fail if you really overwrite the address to 31:0.

—
Sincerely Yours,
Pixel

Quoted 47 lines · Monk

Hold on,

We didn't hit test fail without your patch, actually at least VEGA10 doesn't have the issue you mentioned, Can you elaborate what issue or test case you can fix with this patch ?
Besides, please don't change anything on vega10 before you verified it

BR Monk

-----Original Message-----
From: Pixel Ding [mailto:[email protected]]
Sent: Monday, September 25, 2017 2:16 PM
To: [email protected]; Ding, Pixel <[email protected]>; Min, Frank <[email protected]>; Liu, Monk <[email protected]>; Deucher, Alexander <[email protected]>
Subject: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Both Tonga and Vega register SPECs indicate that this registers only use 31:2 bits in DW. SRIOV test case immediately fails withtout this shift.

Signed-off-by Pixel Ding <[email protected]>

---
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-

2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 72f31cc..947f019 100644
@@ -714,7 +714,7 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]); diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index c26d205..26d7f03 100644

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -2
@@ -665,7 +665,7 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
--
Quoted 2 lines

2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

#7 lore raw
In reply to a message not in the archive

See the code in drm-next:

		/* setup the wptr shadow polling */
		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
		       lower_32_bits(wptr_gpu_addr));
		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
		       upper_32_bits(wptr_gpu_addr));

the wptr_gpu_addr is gpu_addr + wptr_offs * 4, so it is a value with low two bits all zero, e.g. 0xffff000C

this is your new change (v2):

Review

Quoted 9 lines
		/* setup the wptr shadow polling */
		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
(quoted diff) +4 -3
-
-		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		wptr_poll_addr_lo = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i]);
+		wptr_poll_addr_lo = REG_SET_FIELD(wptr_poll_addr_lo, SDMA0_GFX_RB_WPTR_POLL_ADDR_LO,
+						  ADDR, lower_32_bits(wptr_gpu_addr) >> 2);
+		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i], wptr_poll_addr_lo;

You right shift " wptr_gpu_addr" two bits and later write it into 31:2 bits of SDMA0_GFX_RB_WPTR_POLL_ADDR_LO registers, that way The value finally written to the register will be totally the same with or without you patch

BR Monk

-----Original Message-----
From: Ding, Pixel
Sent: Monday, September 25, 2017 2:42 PM
To: Liu, Monk <[email protected]>; [email protected]; Min, Frank <[email protected]>; Deucher, Alexander <[email protected]>; Yu, Xiangliang <[email protected]>
Subject: Re: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Hi Monk,

The world switch gets immediately fail. According to Xiangliang’s comment, I think 17.50 also has this issue. Other G branch uses original patch from Frank, that doesn’t have this issue. Please confirm this.

Also refer to http://adcweb02.amd.com/orlvalid/regspec/web_regspec/vega11/regspec/vega11_chip/public/index.html

SDMA0_GFX_RB_WPTR_POLL_ADDR_LO
ADDR 31:2

I update a v2 patch for this. I think it must fail if you really overwrite the address to 31:0.

—
Sincerely Yours,
Pixel

Quoted 47 lines · Monk

Hold on,

We didn't hit test fail without your patch, actually at least VEGA10 doesn't have the issue you mentioned, Can you elaborate what issue or test case you can fix with this patch ?
Besides, please don't change anything on vega10 before you verified it

BR Monk

-----Original Message-----
From: Pixel Ding [mailto:[email protected]]
Sent: Monday, September 25, 2017 2:16 PM
To: [email protected]; Ding, Pixel <[email protected]>; Min, Frank <[email protected]>; Liu, Monk <[email protected]>; Deucher, Alexander <[email protected]>
Subject: [PATCH] drm/amdgpu: right shift 2 bits for SDMA_GFX_RB_WPTR_POLL_ADDR_LO

Both Tonga and Vega register SPECs indicate that this registers only use 31:2 bits in DW. SRIOV test case immediately fails withtout this shift.

Signed-off-by Pixel Ding <[email protected]>

---
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c | 2 +- drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 2 +-

2 files changed, 2 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 72f31cc..947f019 100644
@@ -714,7 +714,7 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO + sdma_offsets[i],
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI + sdma_offsets[i],
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]); diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c

index c26d205..26d7f03 100644

drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c +1 -2
@@ -665,7 +665,7 @@ static int sdma_v4_0_gfx_resume(struct amdgpu_device *adev)
 		/* setup the wptr shadow polling */
 		wptr_gpu_addr = adev->wb.gpu_addr + (ring->wptr_offs * 4);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_LO),
-		       lower_32_bits(wptr_gpu_addr));
+		       lower_32_bits(wptr_gpu_addr) >> 2);
 		WREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI),
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl = RREG32(sdma_v4_0_get_reg_offset(i, mmSDMA0_GFX_RB_WPTR_POLL_CNTL));
--
Quoted 2 lines

2.7.4

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx