Skip to contentSkip to conversation

Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

PATCH In review Emily Deng 3 participants 6 messages Mar 2, 2018 – Mar 6, 2018 3 messages not in the archive

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

Emily Deng

author
#0 lore raw

PATCH 1/1 Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

This reverts commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145. The sdma wptr poll memomy doesn't have the same efficiency as doorbell, and it will make sdma hang when running tests.

Change-Id I6e334430b309b0c21aa18a08764320c7ff51e353
Signed-off-by Emily Deng <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 -
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -1
index 102dad3..5dcf98b 100644
@@ -186,7 +186,6 @@ struct amdgpu_ring {
 	uint64_t                eop_gpu_addr;
 	u32			doorbell_index;
 	bool			use_doorbell;
-	bool			use_pollmem;
 	unsigned		wptr_offs;
 	unsigned		fence_offs;
 	uint64_t		current_ctx;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +8 -19
index 521978c..d3fb3ca 100644
@@ -355,7 +355,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct amdgpu_ring *ring)
 	struct amdgpu_device *adev = ring->adev;
 	u32 wptr;
 
-	if (ring->use_doorbell || ring->use_pollmem) {
+	if (ring->use_doorbell) {
 		/* XXX check if swapping is necessary on BE */
 		wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
 	} else {

@@ -380,13 +380,10 @@ static void sdma_v3_0_ring_set_wptr(struct amdgpu_ring *ring)
 
 	if (ring->use_doorbell) {
 		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
+
 		/* XXX check if swapping is necessary on BE */
 		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring->wptr) << 2);
-	} else if (ring->use_pollmem) {
-		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
-
-		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 	} else {
 		int me = (ring == &ring->adev->sdma.instance[0].ring) ? 0 : 1;
 

@@ -719,14 +716,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
 		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]);
-		if (ring->use_pollmem)
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-						       SDMA0_GFX_RB_WPTR_POLL_CNTL,
-						       ENABLE, 1);
+		if (amdgpu_sriov_vf(adev))
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
 		else
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-						       SDMA0_GFX_RB_WPTR_POLL_CNTL,
-						       ENABLE, 0);
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i], wptr_poll_cntl);
 
 		/* enable DMA RB */

@@ -1208,13 +1201,9 @@ static int sdma_v3_0_sw_init(void *handle)
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
-		if (!amdgpu_sriov_vf(adev)) {
-			ring->use_doorbell = true;
-			ring->doorbell_index = (i == 0) ?
-				AMDGPU_DOORBELL_sDMA_ENGINE0 : AMDGPU_DOORBELL_sDMA_ENGINE1;
-		} else {
-			ring->use_pollmem = true;
-		}
+		ring->use_doorbell = true;
+		ring->doorbell_index = (i == 0) ?
+			AMDGPU_DOORBELL_sDMA_ENGINE0 : AMDGPU_DOORBELL_sDMA_ENGINE1;
 
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,
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): 1519961490-2813-1-git-send-email-Emily.Deng-5C7GfCeVMHo@public.gmane.org

author
#2 lore raw

Ping....
Hi all,
Please help review this, the regression introduced by the commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145 is gating the sriov sanity test.

Best Wishes,
Emily Deng

Quoted 110 lines

-----Original Message-----
From: Emily Deng [mailto:[email protected]]
Sent: Friday, March 02, 2018 11:32 AM
To: [email protected]
Cc: Deng, Emily <[email protected]>
Subject: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

This reverts commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145. The sdma wptr poll memomy doesn't have the same efficiency as doorbell, and it will make sdma hang when running tests.

Change-Id I6e334430b309b0c21aa18a08764320c7ff51e353
Signed-off-by Emily Deng <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 -
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -1
index 102dad3..5dcf98b 100644
@@ -186,7 +186,6 @@ struct amdgpu_ring {
 	uint64_t                eop_gpu_addr;
 	u32			doorbell_index;
 	bool			use_doorbell;
-	bool			use_pollmem;
 	unsigned		wptr_offs;
 	unsigned		fence_offs;
 	uint64_t		current_ctx;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 521978c..d3fb3ca 100644
@@ -355,7 +355,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct
 amdgpu_ring *ring)
 	struct amdgpu_device *adev = ring->adev;
 	u32 wptr;
 
-	if (ring->use_doorbell || ring->use_pollmem) {
+	if (ring->use_doorbell) {
 		/* XXX check if swapping is necessary on BE */
 		wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -4
 	} else {

@@ -380,13 +380,10 @@ static void sdma_v3_0_ring_set_wptr(struct
 amdgpu_ring *ring)
 
 	if (ring->use_doorbell) {
 		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
+
 		/* XXX check if swapping is necessary on BE */
 		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring-
 >wptr) << 2);
-	} else if (ring->use_pollmem) {
-		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
-
-		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 	} else {
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -6
 		int me = (ring == &ring->adev->sdma.instance[0].ring) ? 0 : 1;

@@ -719,14 +716,10 @@ static int sdma_v3_0_gfx_resume(struct
 amdgpu_device *adev)
 		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]);
-		if (ring->use_pollmem)
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-
 SDMA0_GFX_RB_WPTR_POLL_CNTL,
-						       ENABLE, 1);
+		if (amdgpu_sriov_vf(adev))
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
 		else
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-

SDMA0_GFX_RB_WPTR_POLL_CNTL,

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +2 -1
-						       ENABLE, 0);
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL +

sdma_offsets[i], wptr_poll_cntl);

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -7
 		/* enable DMA RB */

@@ -1208,13 +1201,9 @@ static int sdma_v3_0_sw_init(void *handle)
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
-		if (!amdgpu_sriov_vf(adev)) {
-			ring->use_doorbell = true;
-			ring->doorbell_index = (i == 0) ?
-				AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
-		} else {
-			ring->use_pollmem = true;
-		}
+		ring->use_doorbell = true;
+		ring->doorbell_index = (i == 0) ?
+			AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +0 -1
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,
--
Quoted 1 line

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): CY4PR12MB11253B1EA35C42340A0B1D118FDA0-rpdhrqHFk07v2MZdTKcfDgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org

#4 lore raw

You can tag my RB

-----Original Message-----
From: amd-gfx [mailto:[email protected]] On Behalf Of Deng, Emily
Sent: 2018年3月5日 18:20
To: Deng, Emily <[email protected]>; [email protected]
Subject: RE: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

Ping....
Hi all,
Please help review this, the regression introduced by the commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145 is gating the sriov sanity test.

Best Wishes,
Emily Deng

Quoted 109 lines

-----Original Message-----
From: Emily Deng [mailto:[email protected]]
Sent: Friday, March 02, 2018 11:32 AM
To: [email protected]
Cc: Deng, Emily <[email protected]>
Subject: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

This reverts commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145. The sdma wptr poll memomy doesn't have the same efficiency as doorbell, and it will make sdma hang when running tests.

Change-Id I6e334430b309b0c21aa18a08764320c7ff51e353
Signed-off-by Emily Deng <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 -
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -1
index 102dad3..5dcf98b 100644
@@ -186,7 +186,6 @@ struct amdgpu_ring {
 	uint64_t                eop_gpu_addr;
 	u32			doorbell_index;
 	bool			use_doorbell;
-	bool			use_pollmem;
 	unsigned		wptr_offs;
 	unsigned		fence_offs;
 	uint64_t		current_ctx;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 521978c..d3fb3ca 100644
@@ -355,7 +355,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct 
 amdgpu_ring *ring)
 	struct amdgpu_device *adev = ring->adev;
 	u32 wptr;
 
-	if (ring->use_doorbell || ring->use_pollmem) {
+	if (ring->use_doorbell) {
 		/* XXX check if swapping is necessary on BE */
 		wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -4
 	} else {

@@ -380,13 +380,10 @@ static void sdma_v3_0_ring_set_wptr(struct 
 amdgpu_ring *ring)
 
 	if (ring->use_doorbell) {
 		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
+
 		/* XXX check if swapping is necessary on BE */
 		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring-
 >wptr) << 2);
-	} else if (ring->use_pollmem) {
-		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
-
-		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 	} else {
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -6
 		int me = (ring == &ring->adev->sdma.instance[0].ring) ? 0 : 1;

@@ -719,14 +716,10 @@ static int sdma_v3_0_gfx_resume(struct 
 amdgpu_device *adev)
 		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]);
-		if (ring->use_pollmem)
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-
 SDMA0_GFX_RB_WPTR_POLL_CNTL,
-						       ENABLE, 1);
+		if (amdgpu_sriov_vf(adev))
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, 
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
 		else
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-

SDMA0_GFX_RB_WPTR_POLL_CNTL,

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +2 -1
-						       ENABLE, 0);
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, 
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i],

wptr_poll_cntl);

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -7
 		/* enable DMA RB */

@@ -1208,13 +1201,9 @@ static int sdma_v3_0_sw_init(void *handle)
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
-		if (!amdgpu_sriov_vf(adev)) {
-			ring->use_doorbell = true;
-			ring->doorbell_index = (i == 0) ?
-				AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
-		} else {
-			ring->use_pollmem = true;
-		}
+		ring->use_doorbell = true;
+		ring->doorbell_index = (i == 0) ?
+			AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +0 -1
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,
--
Quoted 1 line

2.7.4

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

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

Ping....
Hi all,
Please help review this, the regression introduced by the commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145 is gating the sriov sanity test.

Best Wishes,
Emily Deng

Quoted 110 lines

-----Original Message-----
From: Emily Deng [mailto:[email protected]]
Sent: Friday, March 02, 2018 11:32 AM
To: [email protected]
Cc: Deng, Emily <[email protected]>
Subject: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

This reverts commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145. The sdma wptr poll memomy doesn't have the same efficiency as doorbell, and it will make sdma hang when running tests.

Change-Id I6e334430b309b0c21aa18a08764320c7ff51e353
Signed-off-by Emily Deng <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 -
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -1
index 102dad3..5dcf98b 100644
@@ -186,7 +186,6 @@ struct amdgpu_ring {
 	uint64_t                eop_gpu_addr;
 	u32			doorbell_index;
 	bool			use_doorbell;
-	bool			use_pollmem;
 	unsigned		wptr_offs;
 	unsigned		fence_offs;
 	uint64_t		current_ctx;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 521978c..d3fb3ca 100644
@@ -355,7 +355,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct
 amdgpu_ring *ring)
 	struct amdgpu_device *adev = ring->adev;
 	u32 wptr;
 
-	if (ring->use_doorbell || ring->use_pollmem) {
+	if (ring->use_doorbell) {
 		/* XXX check if swapping is necessary on BE */
 		wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -4
 	} else {

@@ -380,13 +380,10 @@ static void sdma_v3_0_ring_set_wptr(struct
 amdgpu_ring *ring)
 
 	if (ring->use_doorbell) {
 		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
+
 		/* XXX check if swapping is necessary on BE */
 		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring-
 >wptr) << 2);
-	} else if (ring->use_pollmem) {
-		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
-
-		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 	} else {
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -6
 		int me = (ring == &ring->adev->sdma.instance[0].ring) ? 0 : 1;

@@ -719,14 +716,10 @@ static int sdma_v3_0_gfx_resume(struct
 amdgpu_device *adev)
 		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]);
-		if (ring->use_pollmem)
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-
 SDMA0_GFX_RB_WPTR_POLL_CNTL,
-						       ENABLE, 1);
+		if (amdgpu_sriov_vf(adev))
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
 		else
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-

SDMA0_GFX_RB_WPTR_POLL_CNTL,

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +2 -1
-						       ENABLE, 0);
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL +

sdma_offsets[i], wptr_poll_cntl);

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -7
 		/* enable DMA RB */

@@ -1208,13 +1201,9 @@ static int sdma_v3_0_sw_init(void *handle)
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
-		if (!amdgpu_sriov_vf(adev)) {
-			ring->use_doorbell = true;
-			ring->doorbell_index = (i == 0) ?
-				AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
-		} else {
-			ring->use_pollmem = true;
-		}
+		ring->use_doorbell = true;
+		ring->doorbell_index = (i == 0) ?
+			AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +0 -1
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,
--
Quoted 1 line

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): CY4PR12MB11256AD429333AD64D632D3D8FD90-rpdhrqHFk07v2MZdTKcfDgdYzm3356FpvxpqHgZTriW3zl9H0oFU5g@public.gmane.org

author
#7 lore raw

Hi all,
Please ignore this patch, the root cause about the sdma hang introduced by sdma wptr polling
has been found, will submit another patch.

Best Wishes,
Emily Deng

Quoted 125 lines

-----Original Message-----
From: Deng, Emily
Sent: Tuesday, March 06, 2018 9:50 AM
To: Deng, Emily <[email protected]>; [email protected]
Subject: RE: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

Ping....
Hi all,
Please help review this, the regression introduced by the commit
2ffe31deb27579e2f2c9444e01f4d8abf385d145 is gating the sriov sanity test.

Best Wishes,
Emily Deng

Quoted 12 lines

-----Original Message-----
From: Emily Deng [mailto:[email protected]]
Sent: Friday, March 02, 2018 11:32 AM
To: [email protected]
Cc: Deng, Emily <[email protected]>
Subject: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

This reverts commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145. The sdma wptr poll memomy doesn't have the same efficiency as doorbell, and it will make sdma hang when running tests.

Quoted 5 lines
Change-Id I6e334430b309b0c21aa18a08764320c7ff51e353
Signed-off-by Emily Deng <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 -
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 27 ++++++++----------------

---

Quoted 48 lines
 2 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -1
index 102dad3..5dcf98b 100644
@@ -186,7 +186,6 @@ struct amdgpu_ring {
 	uint64_t                eop_gpu_addr;
 	u32			doorbell_index;
 	bool			use_doorbell;
-	bool			use_pollmem;
 	unsigned		wptr_offs;
 	unsigned		fence_offs;
 	uint64_t		current_ctx;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -1
index 521978c..d3fb3ca 100644
@@ -355,7 +355,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct
 amdgpu_ring *ring)
 	struct amdgpu_device *adev = ring->adev;
 	u32 wptr;
 
-	if (ring->use_doorbell || ring->use_pollmem) {
+	if (ring->use_doorbell) {
 		/* XXX check if swapping is necessary on BE */
 		wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +1 -4
 	} else {

@@ -380,13 +380,10 @@ static void sdma_v3_0_ring_set_wptr(struct
 amdgpu_ring *ring)
 
 	if (ring->use_doorbell) {
 		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
+
 		/* XXX check if swapping is necessary on BE */
 		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 		WDOORBELL32(ring->doorbell_index, lower_32_bits(ring-
 >wptr) << 2);
-	} else if (ring->use_pollmem) {
-		u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
-
-		WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
 	} else {
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +0 -0
 		int me = (ring == &ring->adev->sdma.instance[0].ring) ? 0 : 1;

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c static int sdma_v3_0_gfx_resume(struct · line 717

Quoted 2 lines
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +0 -0
@@ -719,14 +716,10 @@ static int sdma_v3_0_gfx_resume(struct
 amdgpu_device *adev)
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_ADDR_HI +

sdma_offsets[i],

Quoted 15 lines
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -6
 		       upper_32_bits(wptr_gpu_addr));
 		wptr_poll_cntl =
 RREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i]);
-		if (ring->use_pollmem)
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-
 SDMA0_GFX_RB_WPTR_POLL_CNTL,
-						       ENABLE, 1);
+		if (amdgpu_sriov_vf(adev))
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
 		else
-			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-

SDMA0_GFX_RB_WPTR_POLL_CNTL,

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c

Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +2 -1
-						       ENABLE, 0);
+			wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
 		WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL +

sdma_offsets[i],

Quoted 20 lines

wptr_poll_cntl);

drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +3 -7
 		/* enable DMA RB */

@@ -1208,13 +1201,9 @@ static int sdma_v3_0_sw_init(void *handle)
 	for (i = 0; i < adev->sdma.num_instances; i++) {
 		ring = &adev->sdma.instance[i].ring;
 		ring->ring_obj = NULL;
-		if (!amdgpu_sriov_vf(adev)) {
-			ring->use_doorbell = true;
-			ring->doorbell_index = (i == 0) ?
-				AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
-		} else {
-			ring->use_pollmem = true;
-		}
+		ring->use_doorbell = true;
+		ring->doorbell_index = (i == 0) ?
+			AMDGPU_DOORBELL_sDMA_ENGINE0 :
 AMDGPU_DOORBELL_sDMA_ENGINE1;
Quoted 4 lines
Quoted 4 lines
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +0 -1
 		sprintf(ring->name, "sdma%d", i);
 		r = amdgpu_ring_init(adev, ring, 1024,
--

2.7.4

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

Acked-by
#8 lore raw
In reply to a message not in the archive
Acked-by Alex Deucher <[email protected]>

________________________________
From: amd-gfx <amd-gfx-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org> on behalf of Emily Deng <[email protected]>
Sent: Thursday, March 1, 2018 10:31 PM
To: [email protected]
Cc: Deng, Emily
Subject: [PATCH] Revert "drm/amdgpu: use polling mem to set SDMA3 wptr for VF"

This reverts commit 2ffe31deb27579e2f2c9444e01f4d8abf385d145. The sdma wptr poll memomy doesn't have the same efficiency as doorbell, and it will make sdma hang when running tests.

Change-Id I6e334430b309b0c21aa18a08764320c7ff51e353
Signed-off-by Emily Deng <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 -
 drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 27 ++++++++-------------------
 2 files changed, 8 insertions(+), 20 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h +0 -1
index 102dad3..5dcf98b 100644
@@ -186,7 +186,6 @@ struct amdgpu_ring {
         uint64_t                eop_gpu_addr;
         u32                     doorbell_index;
         bool                    use_doorbell;
-       bool                    use_pollmem;
         unsigned                wptr_offs;
         unsigned                fence_offs;
         uint64_t                current_ctx;
drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c +8 -19
index 521978c..d3fb3ca 100644
@@ -355,7 +355,7 @@ static uint64_t sdma_v3_0_ring_get_wptr(struct amdgpu_ring *ring)
         struct amdgpu_device *adev = ring->adev;
         u32 wptr;
 
-       if (ring->use_doorbell || ring->use_pollmem) {
+       if (ring->use_doorbell) {
                 /* XXX check if swapping is necessary on BE */
                 wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
         } else {

@@ -380,13 +380,10 @@ static void sdma_v3_0_ring_set_wptr(struct amdgpu_ring *ring)
 
         if (ring->use_doorbell) {
                 u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
+
                 /* XXX check if swapping is necessary on BE */
                 WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
                 WDOORBELL32(ring->doorbell_index, lower_32_bits(ring->wptr) << 2);
-       } else if (ring->use_pollmem) {
-               u32 *wb = (u32 *)&adev->wb.wb[ring->wptr_offs];
-
-               WRITE_ONCE(*wb, (lower_32_bits(ring->wptr) << 2));
         } else {
                 int me = (ring == &ring->adev->sdma.instance[0].ring) ? 0 : 1;
 

@@ -719,14 +716,10 @@ static int sdma_v3_0_gfx_resume(struct amdgpu_device *adev)
                 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]);
-               if (ring->use_pollmem)
-                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-                                                      SDMA0_GFX_RB_WPTR_POLL_CNTL,
-                                                      ENABLE, 1);
+               if (amdgpu_sriov_vf(adev))
+                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
                 else
-                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
-                                                      SDMA0_GFX_RB_WPTR_POLL_CNTL,
-                                                      ENABLE, 0);
+                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
                 WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i], wptr_poll_cntl);
 
                 /* enable DMA RB */

@@ -1208,13 +1201,9 @@ static int sdma_v3_0_sw_init(void *handle)
         for (i = 0; i < adev->sdma.num_instances; i++) {
                 ring = &adev->sdma.instance[i].ring;
                 ring->ring_obj = NULL;
-               if (!amdgpu_sriov_vf(adev)) {
-                       ring->use_doorbell = true;
-                       ring->doorbell_index = (i == 0) ?
-                               AMDGPU_DOORBELL_sDMA_ENGINE0 : AMDGPU_DOORBELL_sDMA_ENGINE1;
-               } else {
-                       ring->use_pollmem = true;
-               }
+               ring->use_doorbell = true;
+               ring->doorbell_index = (i == 0) ?
+                       AMDGPU_DOORBELL_sDMA_ENGINE0 : AMDGPU_DOORBELL_sDMA_ENGINE1;
 
                 sprintf(ring->name, "sdma%d", i);
                 r = amdgpu_ring_init(adev, ring, 1024,

--
2.7.4

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

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