Re: [PATCH 2/4] drm/amdgpu: Add software ring callbacks for gfx9 (v6)

2022-09-26 Thread Christian König

Am 23.09.22 um 15:16 schrieb jiadong@amd.com:

From: "Jiadong.Zhu" 

Set ring functions with software ring callbacks on gfx9.

The software ring could be tested by debugfs_test_ib case.

v2: Set sw_ring 2 to enable software ring by default.
v3: Remove the parameter for software ring enablement.
v4: Use amdgpu_ring_init/fini for software rings.
v5: Update for code format. Fix conflict.
v6: Remove unnecessary checks and enable software ring on gfx9 by default.

Acked-by: Luben Tuikov 
Cc: Christian Koenig 
Cc: Luben Tuikov 
Cc: Andrey Grodzovsky 
Signed-off-by: Jiadong.Zhu 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  |   1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |   1 +
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 108 ++-
  3 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 9996dadb39f7..4fdfc3ec134a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -348,6 +348,7 @@ struct amdgpu_gfx {
  
  	boolis_poweron;
  
+	struct amdgpu_ring		sw_gfx_ring[AMDGPU_MAX_SW_GFX_RINGS];

struct amdgpu_ring_mux  muxer;
  };
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h

index 40b1277b4f0c..f08ee1ac281c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -39,6 +39,7 @@ struct amdgpu_vm;
  #define AMDGPU_MAX_RINGS  28
  #define AMDGPU_MAX_HWIP_RINGS 8
  #define AMDGPU_MAX_GFX_RINGS  2
+#define AMDGPU_MAX_SW_GFX_RINGS 2
  #define AMDGPU_MAX_COMPUTE_RINGS  8
  #define AMDGPU_MAX_VCE_RINGS  3
  #define AMDGPU_MAX_UVD_ENC_RINGS  2
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 5349ca4d19e3..e688665cd1e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -47,6 +47,7 @@
  
  #include "amdgpu_ras.h"
  
+#include "amdgpu_sw_ring.h"

  #include "gfx_v9_4.h"
  #include "gfx_v9_0.h"
  #include "gfx_v9_4_2.h"
@@ -56,6 +57,7 @@
  #include "asic_reg/gc/gc_9_0_default.h"
  
  #define GFX9_NUM_GFX_RINGS 1

+#define GFX9_NUM_SW_GFX_RINGS  2
  #define GFX9_MEC_HPD_SIZE 4096
  #define RLCG_UCODE_LOADING_START_ADDRESS 0x2000L
  #define RLC_SAVE_RESTORE_ADDR_STARTING_OFFSET 0xL
@@ -2273,6 +2275,7 @@ static int gfx_v9_0_sw_init(void *handle)
struct amdgpu_ring *ring;
struct amdgpu_kiq *kiq;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   unsigned int hw_prio;
  
  	switch (adev->ip_versions[GC_HWIP][0]) {

case IP_VERSION(9, 0, 1):
@@ -2356,6 +2359,9 @@ static int gfx_v9_0_sw_init(void *handle)
sprintf(ring->name, "gfx_%d", i);
ring->use_doorbell = true;
ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
+
+   /* disable scheduler on the real ring */
+   ring->no_scheduler = true;
r = amdgpu_ring_init(adev, ring, 1024, >gfx.eop_irq,
 AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP,
 AMDGPU_RING_PRIO_DEFAULT, NULL);
@@ -2363,6 +2369,42 @@ static int gfx_v9_0_sw_init(void *handle)
return r;
}
  
+	/* set up the software rings */

+   for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++) {
+   ring = >gfx.sw_gfx_ring[i];
+   ring->ring_obj = NULL;
+   if (!i)
+   sprintf(ring->name, "gfx_sw");
+   else
+   sprintf(ring->name, "gfx_sw_%d", i);


I think we should use something like gfx_low/gfx_high for the ring name 
here.


That this is implemented by a sw muxer is pretty much irrelevant for 
overspace.


Maybe use a static array for the names or something like this.

Apart from that looks good to me.

Regards,
Christian.


+   ring->use_doorbell = true;
+   ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
+   ring->is_sw_ring = true;
+   hw_prio = (i == 1) ? AMDGPU_RING_PRIO_2 :
+   AMDGPU_RING_PRIO_DEFAULT;
+   r = amdgpu_ring_init(adev, ring, 1024, >gfx.eop_irq,
+AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP, hw_prio,
+NULL);
+   if (r)
+   return r;
+   ring->wptr = 0;
+   }
+
+   /* init the muxer and add software rings */
+   r = amdgpu_ring_mux_init(>gfx.muxer, >gfx.gfx_ring[0],
+GFX9_NUM_SW_GFX_RINGS);
+   if (r) {
+   DRM_ERROR("amdgpu_ring_mux_init failed(%d)\n", r);
+   return r;
+   }
+   for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++) {
+   r = 

[PATCH 2/4] drm/amdgpu: Add software ring callbacks for gfx9 (v6)

2022-09-23 Thread jiadong.zhu
From: "Jiadong.Zhu" 

Set ring functions with software ring callbacks on gfx9.

The software ring could be tested by debugfs_test_ib case.

v2: Set sw_ring 2 to enable software ring by default.
v3: Remove the parameter for software ring enablement.
v4: Use amdgpu_ring_init/fini for software rings.
v5: Update for code format. Fix conflict.
v6: Remove unnecessary checks and enable software ring on gfx9 by default.

Acked-by: Luben Tuikov 
Cc: Christian Koenig 
Cc: Luben Tuikov 
Cc: Andrey Grodzovsky 
Signed-off-by: Jiadong.Zhu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h  |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |   1 +
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 108 ++-
 3 files changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 9996dadb39f7..4fdfc3ec134a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -348,6 +348,7 @@ struct amdgpu_gfx {
 
boolis_poweron;
 
+   struct amdgpu_ring  sw_gfx_ring[AMDGPU_MAX_SW_GFX_RINGS];
struct amdgpu_ring_mux  muxer;
 };
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 40b1277b4f0c..f08ee1ac281c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -39,6 +39,7 @@ struct amdgpu_vm;
 #define AMDGPU_MAX_RINGS   28
 #define AMDGPU_MAX_HWIP_RINGS  8
 #define AMDGPU_MAX_GFX_RINGS   2
+#define AMDGPU_MAX_SW_GFX_RINGS 2
 #define AMDGPU_MAX_COMPUTE_RINGS   8
 #define AMDGPU_MAX_VCE_RINGS   3
 #define AMDGPU_MAX_UVD_ENC_RINGS   2
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 5349ca4d19e3..e688665cd1e0 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -47,6 +47,7 @@
 
 #include "amdgpu_ras.h"
 
+#include "amdgpu_sw_ring.h"
 #include "gfx_v9_4.h"
 #include "gfx_v9_0.h"
 #include "gfx_v9_4_2.h"
@@ -56,6 +57,7 @@
 #include "asic_reg/gc/gc_9_0_default.h"
 
 #define GFX9_NUM_GFX_RINGS 1
+#define GFX9_NUM_SW_GFX_RINGS  2
 #define GFX9_MEC_HPD_SIZE 4096
 #define RLCG_UCODE_LOADING_START_ADDRESS 0x2000L
 #define RLC_SAVE_RESTORE_ADDR_STARTING_OFFSET 0xL
@@ -2273,6 +2275,7 @@ static int gfx_v9_0_sw_init(void *handle)
struct amdgpu_ring *ring;
struct amdgpu_kiq *kiq;
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   unsigned int hw_prio;
 
switch (adev->ip_versions[GC_HWIP][0]) {
case IP_VERSION(9, 0, 1):
@@ -2356,6 +2359,9 @@ static int gfx_v9_0_sw_init(void *handle)
sprintf(ring->name, "gfx_%d", i);
ring->use_doorbell = true;
ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
+
+   /* disable scheduler on the real ring */
+   ring->no_scheduler = true;
r = amdgpu_ring_init(adev, ring, 1024, >gfx.eop_irq,
 AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP,
 AMDGPU_RING_PRIO_DEFAULT, NULL);
@@ -2363,6 +2369,42 @@ static int gfx_v9_0_sw_init(void *handle)
return r;
}
 
+   /* set up the software rings */
+   for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++) {
+   ring = >gfx.sw_gfx_ring[i];
+   ring->ring_obj = NULL;
+   if (!i)
+   sprintf(ring->name, "gfx_sw");
+   else
+   sprintf(ring->name, "gfx_sw_%d", i);
+   ring->use_doorbell = true;
+   ring->doorbell_index = adev->doorbell_index.gfx_ring0 << 1;
+   ring->is_sw_ring = true;
+   hw_prio = (i == 1) ? AMDGPU_RING_PRIO_2 :
+   AMDGPU_RING_PRIO_DEFAULT;
+   r = amdgpu_ring_init(adev, ring, 1024, >gfx.eop_irq,
+AMDGPU_CP_IRQ_GFX_ME0_PIPE0_EOP, hw_prio,
+NULL);
+   if (r)
+   return r;
+   ring->wptr = 0;
+   }
+
+   /* init the muxer and add software rings */
+   r = amdgpu_ring_mux_init(>gfx.muxer, >gfx.gfx_ring[0],
+GFX9_NUM_SW_GFX_RINGS);
+   if (r) {
+   DRM_ERROR("amdgpu_ring_mux_init failed(%d)\n", r);
+   return r;
+   }
+   for (i = 0; i < GFX9_NUM_SW_GFX_RINGS; i++) {
+   r = amdgpu_ring_mux_add_sw_ring(>gfx.muxer, 
>gfx.sw_gfx_ring[i]);
+   if (r) {
+   DRM_ERROR("amdgpu_ring_mux_add_sw_ring failed(%d)\n", 
r);
+   return r;
+   }
+   }
+
/* set up the compute queues - allocate horizontally across pipes */
ring_id = 0;