Am 22.04.21 um 12:49 schrieb Victor Zhao:
Add sdma single packet invalidation

Well NAK on many aspects of this.


Signed-off-by: Victor Zhao <victor.z...@amd.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c  |  8 ++++----
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  4 ++++
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c   | 14 +++++++++-----
  drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c   | 12 ++++++++++++
  4 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index c39ed9eb0987..b2cf0e00ce38 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -435,13 +435,13 @@ void amdgpu_gmc_ras_fini(struct amdgpu_device *adev)
/*
         * The latest engine allocation on gfx9/10 is:
-        * Engine 2, 3: firmware
-        * Engine 0, 1, 4~16: amdgpu ring,
+        * Engine 1, 2, 3, 4: firmware
+        * Engine 0, 5~16: amdgpu ring,
         *                    subject to change when ring number changes
         * Engine 17: Gart flushes
         */
-#define GFXHUB_FREE_VM_INV_ENGS_BITMAP         0x1FFF3
-#define MMHUB_FREE_VM_INV_ENGS_BITMAP          0x1FFF3
+#define GFXHUB_FREE_VM_INV_ENGS_BITMAP         0x1FFE1
+#define MMHUB_FREE_VM_INV_ENGS_BITMAP          0x1FFE1
int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev)
  {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index ca1622835296..fb3de16321ff 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -192,6 +192,9 @@ struct amdgpu_ring_funcs {
        void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
                                        uint32_t reg0, uint32_t reg1,
                                        uint32_t ref, uint32_t mask);
+       void (*vm_invalidate)(struct amdgpu_ring *ring,
+                               uint32_t reg0, uint32_t reg1,
+                               uint32_t req, uint32_t mask, uint32_t hub);

What is that good for? You need a much wider explanation if you want to change the design here.

Please rather implement the emit_reg_write_reg_wait() callback properly.

        void (*emit_frame_cntl)(struct amdgpu_ring *ring, bool start,
                                bool secure);
        /* Try to soft recover the ring to make the fence signal */
@@ -270,6 +273,7 @@ struct amdgpu_ring {
  #define amdgpu_ring_emit_wreg(r, d, v) (r)->funcs->emit_wreg((r), (d), (v))
  #define amdgpu_ring_emit_reg_wait(r, d, v, m) (r)->funcs->emit_reg_wait((r), 
(d), (v), (m))
  #define amdgpu_ring_emit_reg_write_reg_wait(r, d0, d1, v, m) 
(r)->funcs->emit_reg_write_reg_wait((r), (d0), (d1), (v), (m))
+#define amdgpu_ring_vm_invalidate(r, d0, d1, v, m, h) 
((r)->funcs->vm_invalidate((r), (d0), (d1), (v), (m), (h)))
  #define amdgpu_ring_emit_frame_cntl(r, b, s) (r)->funcs->emit_frame_cntl((r), 
(b), (s))
  #define amdgpu_ring_pad_ib(r, ib) ((r)->funcs->pad_ib((r), (ib)))
  #define amdgpu_ring_init_cond_exec(r) (r)->funcs->init_cond_exec((r))
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 498b28a35f5b..0fdf60a7c53d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -481,11 +481,15 @@ static uint64_t gmc_v10_0_emit_flush_gpu_tlb(struct 
amdgpu_ring *ring,
                              (hub->ctx_addr_distance * vmid),
                              upper_32_bits(pd_addr));
- amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
-                                           hub->eng_distance * eng,
-                                           hub->vm_inv_eng0_ack +
-                                           hub->eng_distance * eng,
-                                           req, 1 << vmid);
+       if ((!strcmp("sdma0", ring->name)) || (!strcmp("sdma1", ring->name)))

Seriously?

+               amdgpu_ring_vm_invalidate(ring, 0xffffffff, 0x1f,
+                                         req, 1 << vmid, 1 << 
ring->funcs->vmhub);
+       else
+               amdgpu_ring_emit_reg_write_reg_wait(ring, hub->vm_inv_eng0_req +
+                                                   hub->eng_distance * eng,
+                                                   hub->vm_inv_eng0_ack +
+                                                   hub->eng_distance * eng,
+                                                   req, 1 << vmid);
/* TODO: It needs to continue working on debugging with semaphore for GFXHUB as well. */
        if (use_semaphore)
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 920fc6d4a127..fd7df9194f05 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1219,6 +1219,17 @@ static void 
sdma_v5_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
        amdgpu_ring_emit_reg_wait(ring, reg1, mask, mask);
  }
+static void sdma_v5_0_ring_vm_invalidate(struct amdgpu_ring *ring,
+               uint32_t reg0, uint32_t reg1,
+               uint32_t req, uint32_t mask, uint32_t hub)
+{
+       amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_POLL_REGMEM) |
+                       SDMA_PKT_POLL_REGMEM_HEADER_SUB_OP(4));
+       amdgpu_ring_write(ring, req);
+       amdgpu_ring_write(ring, reg0);
+       amdgpu_ring_write(ring, ((reg1 & 0x1f) << 16) | (mask & 0xffff) | (hub 
<< 21)); /* reference */

That's not sufficient. That you use the single packet doesn't protects you from concurrent access of the invalidation control registers.

Christian.

+}
+
  static int sdma_v5_0_early_init(void *handle)
  {
        struct amdgpu_device *adev = (struct amdgpu_device *)handle;
@@ -1654,6 +1665,7 @@ static const struct amdgpu_ring_funcs 
sdma_v5_0_ring_funcs = {
        .emit_wreg = sdma_v5_0_ring_emit_wreg,
        .emit_reg_wait = sdma_v5_0_ring_emit_reg_wait,
        .emit_reg_write_reg_wait = sdma_v5_0_ring_emit_reg_write_reg_wait,
+       .vm_invalidate = sdma_v5_0_ring_vm_invalidate,
        .init_cond_exec = sdma_v5_0_ring_init_cond_exec,
        .patch_cond_exec = sdma_v5_0_ring_patch_cond_exec,
        .preempt_ib = sdma_v5_0_ring_preempt_ib,

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to