One minor style nit pick:
+               } else
+                       ring->use_pollmem = true;
When the true part of an "if/else" uses {} the false part should use it as well.

With that fixed and a Monks comment addressed the patch is Reviewed-by: Christian König <[email protected]>.

Christian.

Am 12.12.2017 um 08:33 schrieb Liu, Monk:
Please make description in your comments more detail, e.g. :

We have two sources to update wptr registers for sdma3: 1) wptr_poll and
2) is doorbell. When doorbell and wptr_poll are both enabled on sdma3,
there will be collision hit in occasion between those two sources when
ucode and h/w are doing the updating on wptr register in parallel.


With that addressed:
Reviewed-by: Monk Liu <[email protected]>

-----Original Message-----
From: Pixel Ding [mailto:[email protected]]
Sent: 2017年12月12日 15:26
To: [email protected]; Liu, Monk <[email protected]>
Cc: Ding, Pixel <[email protected]>
Subject: [PATCH] drm/amdgpu: always use polling mem to update wptr v2

Both doorbell and polling mem are working on Tonga VF. SDMA issue happens 
because SDMA engine accepts doorbell writes even if it's inactive, that 
introduces conflict when world switch routine update wptr though polling 
memory. Use polling mem in driver too.

v2:
  - standalone pollmem code path

Signed-off-by: Pixel Ding <[email protected]>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  1 +
  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   | 26 ++++++++++++++++++--------
  2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index a405022..dcc7cf1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -188,6 +188,7 @@ 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;
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index c8c93f9..c506e2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -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) {
+       if (ring->use_doorbell || ring->use_pollmem) {
                /* XXX check if swapping is necessary on BE */
                wptr = ring->adev->wb.wb[ring->wptr_offs] >> 2;
        } else {
@@ -380,10 +380,13 @@ 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;
@@ -718,10 +721,14 @@ 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 (amdgpu_sriov_vf(adev))
-                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, 
SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 1);
+               if (ring->use_pollmem)
+                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+                                                      
SDMA0_GFX_RB_WPTR_POLL_CNTL,
+                                                      ENABLE, 1);
                else
-                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl, 
SDMA0_GFX_RB_WPTR_POLL_CNTL, F32_POLL_ENABLE, 0);
+                       wptr_poll_cntl = REG_SET_FIELD(wptr_poll_cntl,
+                                                      
SDMA0_GFX_RB_WPTR_POLL_CNTL,
+                                                      ENABLE, 0);
                WREG32(mmSDMA0_GFX_RB_WPTR_POLL_CNTL + sdma_offsets[i], 
wptr_poll_cntl);
/* enable DMA RB */
@@ -1203,9 +1210,12 @@ 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;
-               ring->use_doorbell = true;
-               ring->doorbell_index = (i == 0) ?
-                       AMDGPU_DOORBELL_sDMA_ENGINE0 : 
AMDGPU_DOORBELL_sDMA_ENGINE1;
+               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;
sprintf(ring->name, "sdma%d", i);
                r = amdgpu_ring_init(adev, ring, 1024,
--
2.9.5

_______________________________________________
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

Reply via email to