amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 1/2] drm/amdgpu: implement more ib pools
@ 2020-03-26  9:47 xinhui pan
  2020-03-26  9:47 ` [RFC PATCH 2/2] drm/amdgpu: use new job alloc variation if possible xinhui pan
  2020-03-26 11:02 ` [RFC PATCH 1/2] drm/amdgpu: implement more ib pools Christian König
  0 siblings, 2 replies; 3+ messages in thread
From: xinhui pan @ 2020-03-26  9:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Felix.Kuehling, xinhui pan, christian.koenig

We have tree ib pools, they are normal, VM, direct pools.

Any jobs which schedule IBs without dependence on gpu scheduler should
use DIRECT pool.

Any jobs schedule direct VM update IBs should use VM pool.

Any other jobs use NORMAL pool.

Signed-off-by: xinhui pan <xinhui.pan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 11 ++++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  4 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 41 ++++++++++++++++++-------
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 ++++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  4 ++-
 5 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7dd74253e7b6..3d70c113c205 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -390,6 +390,13 @@ struct amdgpu_sa_bo {
 int amdgpu_fence_slab_init(void);
 void amdgpu_fence_slab_fini(void);
 
+enum amd_ib_pool_type {
+	AMD_IB_POOL_NORMAL = 0,
+	AMD_IB_POOL_VM,
+	AMD_IB_POOL_DIRECT,
+
+	AMD_MAX_IB_POOL
+};
 /*
  * IRQS.
  */
@@ -442,6 +449,8 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
 
 int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		  unsigned size, struct amdgpu_ib *ib);
+int amdgpu_ib_get_with_pool(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+		  unsigned size, struct amdgpu_ib *ib, enum amd_ib_pool_type pool);
 void amdgpu_ib_free(struct amdgpu_device *adev, struct amdgpu_ib *ib,
 		    struct dma_fence *f);
 int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
@@ -848,7 +857,7 @@ struct amdgpu_device {
 	unsigned			num_rings;
 	struct amdgpu_ring		*rings[AMDGPU_MAX_RINGS];
 	bool				ib_pool_ready;
-	struct amdgpu_sa_manager	ring_tmp_bo;
+	struct amdgpu_sa_manager	ring_tmp_bo[AMD_MAX_IB_POOL];
 
 	/* interrupts */
 	struct amdgpu_irq		irq;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 59ec5e2be211..182c7ee439cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -920,8 +920,8 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
 		parser->entity = entity;
 
 		ring = to_amdgpu_ring(entity->rq->sched);
-		r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
-				   chunk_ib->ib_bytes : 0, ib);
+		r =  amdgpu_ib_get_with_pool(adev, vm, ring->funcs->parse_cs ?
+				   chunk_ib->ib_bytes : 0, ib, AMD_IB_POOL_NORMAL);
 		if (r) {
 			DRM_ERROR("Failed to get ib !\n");
 			return r;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index bece01f1cf09..460b50a5f875 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -62,11 +62,17 @@
  */
 int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
 		  unsigned size, struct amdgpu_ib *ib)
+{
+	return amdgpu_ib_get_with_pool(adev, vm, size, ib, AMD_IB_POOL_DIRECT);
+}
+
+int amdgpu_ib_get_with_pool(struct amdgpu_device *adev, struct amdgpu_vm *vm,
+		  unsigned size, struct amdgpu_ib *ib, enum amd_ib_pool_type pool_type)
 {
 	int r;
 
 	if (size) {
-		r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
+		r = amdgpu_sa_bo_new(&adev->ring_tmp_bo[pool_type],
 				      &ib->sa_bo, size, 256);
 		if (r) {
 			dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
@@ -297,19 +303,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
  */
 int amdgpu_ib_pool_init(struct amdgpu_device *adev)
 {
-	int r;
+	int r, i;
+	unsigned size;
 
 	if (adev->ib_pool_ready) {
 		return 0;
 	}
-	r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo,
-				      AMDGPU_IB_POOL_SIZE*64*1024,
-				      AMDGPU_GPU_PAGE_SIZE,
-				      AMDGPU_GEM_DOMAIN_GTT);
-	if (r) {
-		return r;
+	for (i = 0; i < AMD_MAX_IB_POOL; i++) {
+		if (i == AMD_IB_POOL_DIRECT)
+			size = PAGE_SIZE * 2;
+		else
+			size = AMDGPU_IB_POOL_SIZE*64*1024;
+		r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo[i],
+				size,
+				AMDGPU_GPU_PAGE_SIZE,
+				AMDGPU_GEM_DOMAIN_GTT);
+		if (r) {
+			for (i--; i >= 0; i--)
+				amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo[i]);
+			return r;
+		}
 	}
-
 	adev->ib_pool_ready = true;
 
 	return 0;
@@ -325,8 +339,11 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
  */
 void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
 {
+	int i;
+
 	if (adev->ib_pool_ready) {
-		amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
+		for (i = 0; i < AMD_MAX_IB_POOL; i++)
+			amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo[i]);
 		adev->ib_pool_ready = false;
 	}
 }
@@ -423,7 +440,9 @@ static int amdgpu_debugfs_sa_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct amdgpu_device *adev = dev->dev_private;
 
-	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo, m);
+	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo[AMD_IB_POOL_NORMAL], m);
+	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo[AMD_IB_POOL_VM], m);
+	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo[AMD_IB_POOL_DIRECT], m);
 
 	return 0;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 4981e443a884..2e98ce568a3f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 
 int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 			     struct amdgpu_job **job)
+{
+	return amdgpu_job_alloc_with_ib_pool(adev, size, job, AMD_IB_POOL_NORMAL);
+}
+
+int amdgpu_job_alloc_with_ib_pool(struct amdgpu_device *adev, unsigned size,
+			     struct amdgpu_job **job, enum amd_ib_pool_type pool_type)
 {
 	int r;
 
@@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 	if (r)
 		return r;
 
-	r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
+	r = amdgpu_ib_get_with_pool(adev, NULL, size, &(*job)->ibs[0], pool_type);
 	if (r)
 		kfree(*job);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 2e2110dddb76..c34516bf9278 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -38,6 +38,7 @@
 #define AMDGPU_JOB_GET_VMID(job) ((job) ? (job)->vmid : 0)
 
 struct amdgpu_fence;
+enum amd_ib_pool_type;
 
 struct amdgpu_job {
 	struct drm_sched_job    base;
@@ -67,7 +68,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
 		     struct amdgpu_job **job, struct amdgpu_vm *vm);
 int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
 			     struct amdgpu_job **job);
-
+int amdgpu_job_alloc_with_ib_pool(struct amdgpu_device *adev, unsigned size,
+			     struct amdgpu_job **job, enum amd_ib_pool_type pool);
 void amdgpu_job_free_resources(struct amdgpu_job *job);
 void amdgpu_job_free(struct amdgpu_job *job);
 int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [RFC PATCH 2/2] drm/amdgpu: use new job alloc variation if possible
  2020-03-26  9:47 [RFC PATCH 1/2] drm/amdgpu: implement more ib pools xinhui pan
@ 2020-03-26  9:47 ` xinhui pan
  2020-03-26 11:02 ` [RFC PATCH 1/2] drm/amdgpu: implement more ib pools Christian König
  1 sibling, 0 replies; 3+ messages in thread
From: xinhui pan @ 2020-03-26  9:47 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, Felix.Kuehling, xinhui pan, christian.koenig

use corresponding ib pool for each job

Signed-off-by: xinhui pan <xinhui.pan@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c    | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c     | 3 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c     | 5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c     | 6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c      | 2 +-
 drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c       | 4 ++--
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c       | 4 ++--
 9 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
index 5727f00afc8e..d129da9236cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
@@ -144,7 +144,7 @@ static int amdgpu_jpeg_dec_set_reg(struct amdgpu_ring *ring, uint32_t handle,
 	const unsigned ib_size_dw = 16;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index e192557db421..fc9ab3f61575 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2124,7 +2124,8 @@ int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
 	num_loops = DIV_ROUND_UP(byte_count, max_bytes);
 	num_dw = ALIGN(num_loops * adev->mman.buffer_funcs->copy_num_dw, 8);
 
-	r = amdgpu_job_alloc_with_ib(adev, num_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(adev, num_dw * 4, &job,
+			direct_submit ? AMD_IB_POOL_DIRECT : AMD_IB_POOL_NORMAL);
 	if (r)
 		return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
index 5fd32ad1c575..1e9a6aa697ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
@@ -1056,7 +1056,8 @@ static int amdgpu_uvd_send_msg(struct amdgpu_ring *ring, struct amdgpu_bo *bo,
 			goto err;
 	}
 
-	r = amdgpu_job_alloc_with_ib(adev, 64, &job);
+	r = amdgpu_job_alloc_with_ib_pool(adev, 64, &job,
+			direct ? AMD_IB_POOL_DIRECT : AMD_IB_POOL_NORMAL);
 	if (r)
 		goto err;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
index 59ddba137946..feab5392144a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c
@@ -446,7 +446,7 @@ static int amdgpu_vce_get_create_msg(struct amdgpu_ring *ring, uint32_t handle,
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
@@ -524,7 +524,8 @@ static int amdgpu_vce_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handle,
 	struct dma_fence *f = NULL;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job,
+			direct ? AMD_IB_POOL_DIRECT : AMD_IB_POOL_NORMAL);
 	if (r)
 		return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index a41272fbcba2..be11599b1a50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -390,7 +390,7 @@ static int amdgpu_vcn_dec_send_msg(struct amdgpu_ring *ring,
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(adev, 64, &job);
+	r = amdgpu_job_alloc_with_ib_pool(adev, 64, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		goto err;
 
@@ -557,7 +557,7 @@ static int amdgpu_vcn_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t hand
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
@@ -610,7 +610,7 @@ static int amdgpu_vcn_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t han
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index d30d103e48a2..663ee44e7187 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -64,7 +64,8 @@ static int amdgpu_vm_sdma_prepare(struct amdgpu_vm_update_params *p,
 	unsigned int ndw = AMDGPU_VM_SDMA_MIN_NUM_DW;
 	int r;
 
-	r = amdgpu_job_alloc_with_ib(p->adev, ndw * 4, &p->job);
+	r = amdgpu_job_alloc_with_ib_pool(p->adev, ndw * 4, &p->job,
+			p->direct ? AMD_IB_POOL_VM : AMD_IB_POOL_NORMAL);
 	if (r)
 		return r;
 
@@ -225,7 +226,8 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p,
 			ndw = max(ndw, AMDGPU_VM_SDMA_MIN_NUM_DW);
 			ndw = min(ndw, AMDGPU_VM_SDMA_MAX_NUM_DW);
 
-			r = amdgpu_job_alloc_with_ib(p->adev, ndw * 4, &p->job);
+			r = amdgpu_job_alloc_with_ib_pool(p->adev, ndw * 4, &p->job,
+					p->direct ? AMD_IB_POOL_VM : AMD_IB_POOL_NORMAL);
 			if (r)
 				return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 9775eca6fe43..aeeb529499e2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -369,7 +369,7 @@ static void gmc_v10_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	 * translation. Avoid this by doing the invalidation from the SDMA
 	 * itself.
 	 */
-	r = amdgpu_job_alloc_with_ib(adev, 16 * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(adev, 16 * 4, &job, AMD_IB_POOL_VM);
 	if (r)
 		goto error_alloc;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
index e0aadcaf6c8b..7db7a62ccc7d 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v6_0.c
@@ -216,7 +216,7 @@ static int uvd_v6_0_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t handle
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
@@ -279,7 +279,7 @@ static int uvd_v6_0_enc_get_destroy_msg(struct amdgpu_ring *ring,
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
index 20f10a5617ca..e9e7ab78c356 100644
--- a/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c
@@ -224,7 +224,7 @@ static int uvd_v7_0_enc_get_create_msg(struct amdgpu_ring *ring, uint32_t handle
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
@@ -286,7 +286,7 @@ static int uvd_v7_0_enc_get_destroy_msg(struct amdgpu_ring *ring, uint32_t handl
 	uint64_t addr;
 	int i, r;
 
-	r = amdgpu_job_alloc_with_ib(ring->adev, ib_size_dw * 4, &job);
+	r = amdgpu_job_alloc_with_ib_pool(ring->adev, ib_size_dw * 4, &job, AMD_IB_POOL_DIRECT);
 	if (r)
 		return r;
 
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [RFC PATCH 1/2] drm/amdgpu: implement more ib pools
  2020-03-26  9:47 [RFC PATCH 1/2] drm/amdgpu: implement more ib pools xinhui pan
  2020-03-26  9:47 ` [RFC PATCH 2/2] drm/amdgpu: use new job alloc variation if possible xinhui pan
@ 2020-03-26 11:02 ` Christian König
  1 sibling, 0 replies; 3+ messages in thread
From: Christian König @ 2020-03-26 11:02 UTC (permalink / raw)
  To: xinhui pan, amd-gfx; +Cc: alexander.deucher, Felix.Kuehling

Am 26.03.20 um 10:47 schrieb xinhui pan:
> We have tree ib pools, they are normal, VM, direct pools.
>
> Any jobs which schedule IBs without dependence on gpu scheduler should
> use DIRECT pool.
>
> Any jobs schedule direct VM update IBs should use VM pool.
>
> Any other jobs use NORMAL pool.
>
> Signed-off-by: xinhui pan <xinhui.pan@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 11 ++++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  4 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  | 41 ++++++++++++++++++-------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  8 ++++-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.h |  4 ++-
>   5 files changed, 52 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 7dd74253e7b6..3d70c113c205 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -390,6 +390,13 @@ struct amdgpu_sa_bo {
>   int amdgpu_fence_slab_init(void);
>   void amdgpu_fence_slab_fini(void);
>   
> +enum amd_ib_pool_type {
> +	AMD_IB_POOL_NORMAL = 0,
> +	AMD_IB_POOL_VM,
> +	AMD_IB_POOL_DIRECT,
> +
> +	AMD_MAX_IB_POOL
> +};

Please use the prefix amdgpu_ib_pool_* here.

>   /*
>    * IRQS.
>    */
> @@ -442,6 +449,8 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
>   
>   int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		  unsigned size, struct amdgpu_ib *ib);
> +int amdgpu_ib_get_with_pool(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +		  unsigned size, struct amdgpu_ib *ib, enum amd_ib_pool_type pool);

Don't add a new function, just change the existing amdgpu_ib_get() and 
all its callers.

>   void amdgpu_ib_free(struct amdgpu_device *adev, struct amdgpu_ib *ib,
>   		    struct dma_fence *f);
>   int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
> @@ -848,7 +857,7 @@ struct amdgpu_device {
>   	unsigned			num_rings;
>   	struct amdgpu_ring		*rings[AMDGPU_MAX_RINGS];
>   	bool				ib_pool_ready;
> -	struct amdgpu_sa_manager	ring_tmp_bo;
> +	struct amdgpu_sa_manager	ring_tmp_bo[AMD_MAX_IB_POOL];
>   
>   	/* interrupts */
>   	struct amdgpu_irq		irq;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 59ec5e2be211..182c7ee439cc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -920,8 +920,8 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
>   		parser->entity = entity;
>   
>   		ring = to_amdgpu_ring(entity->rq->sched);
> -		r =  amdgpu_ib_get(adev, vm, ring->funcs->parse_cs ?
> -				   chunk_ib->ib_bytes : 0, ib);
> +		r =  amdgpu_ib_get_with_pool(adev, vm, ring->funcs->parse_cs ?
> +				   chunk_ib->ib_bytes : 0, ib, AMD_IB_POOL_NORMAL);
>   		if (r) {
>   			DRM_ERROR("Failed to get ib !\n");
>   			return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index bece01f1cf09..460b50a5f875 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -62,11 +62,17 @@
>    */
>   int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>   		  unsigned size, struct amdgpu_ib *ib)
> +{
> +	return amdgpu_ib_get_with_pool(adev, vm, size, ib, AMD_IB_POOL_DIRECT);
> +}
> +
> +int amdgpu_ib_get_with_pool(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> +		  unsigned size, struct amdgpu_ib *ib, enum amd_ib_pool_type pool_type)
>   {
>   	int r;
>   
>   	if (size) {
> -		r = amdgpu_sa_bo_new(&adev->ring_tmp_bo,
> +		r = amdgpu_sa_bo_new(&adev->ring_tmp_bo[pool_type],
>   				      &ib->sa_bo, size, 256);
>   		if (r) {
>   			dev_err(adev->dev, "failed to get a new IB (%d)\n", r);
> @@ -297,19 +303,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>    */
>   int amdgpu_ib_pool_init(struct amdgpu_device *adev)
>   {
> -	int r;
> +	int r, i;
> +	unsigned size;
>   
>   	if (adev->ib_pool_ready) {
>   		return 0;
>   	}
> -	r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo,
> -				      AMDGPU_IB_POOL_SIZE*64*1024,
> -				      AMDGPU_GPU_PAGE_SIZE,
> -				      AMDGPU_GEM_DOMAIN_GTT);
> -	if (r) {
> -		return r;
> +	for (i = 0; i < AMD_MAX_IB_POOL; i++) {
> +		if (i == AMD_IB_POOL_DIRECT)
> +			size = PAGE_SIZE * 2;
> +		else
> +			size = AMDGPU_IB_POOL_SIZE*64*1024;
> +		r = amdgpu_sa_bo_manager_init(adev, &adev->ring_tmp_bo[i],
> +				size,
> +				AMDGPU_GPU_PAGE_SIZE,
> +				AMDGPU_GEM_DOMAIN_GTT);
> +		if (r) {
> +			for (i--; i >= 0; i--)
> +				amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo[i]);
> +			return r;
> +		}
>   	}
> -
>   	adev->ib_pool_ready = true;
>   
>   	return 0;
> @@ -325,8 +339,11 @@ int amdgpu_ib_pool_init(struct amdgpu_device *adev)
>    */
>   void amdgpu_ib_pool_fini(struct amdgpu_device *adev)
>   {
> +	int i;
> +
>   	if (adev->ib_pool_ready) {
> -		amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo);
> +		for (i = 0; i < AMD_MAX_IB_POOL; i++)
> +			amdgpu_sa_bo_manager_fini(adev, &adev->ring_tmp_bo[i]);
>   		adev->ib_pool_ready = false;
>   	}
>   }
> @@ -423,7 +440,9 @@ static int amdgpu_debugfs_sa_info(struct seq_file *m, void *data)
>   	struct drm_device *dev = node->minor->dev;
>   	struct amdgpu_device *adev = dev->dev_private;
>   
> -	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo, m);
> +	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo[AMD_IB_POOL_NORMAL], m);
> +	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo[AMD_IB_POOL_VM], m);
> +	amdgpu_sa_bo_dump_debug_info(&adev->ring_tmp_bo[AMD_IB_POOL_DIRECT], m);

Maybe add something like seq_printf(m, "-------------------- name of 
pool ----------------"); between each call.

>   
>   	return 0;
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index 4981e443a884..2e98ce568a3f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -88,6 +88,12 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>   
>   int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>   			     struct amdgpu_job **job)
> +{
> +	return amdgpu_job_alloc_with_ib_pool(adev, size, job, AMD_IB_POOL_NORMAL);
> +}
> +
> +int amdgpu_job_alloc_with_ib_pool(struct amdgpu_device *adev, unsigned size,
> +			     struct amdgpu_job **job, enum amd_ib_pool_type pool_type)
>   {
>   	int r;
>   
> @@ -95,7 +101,7 @@ int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>   	if (r)
>   		return r;
>   
> -	r = amdgpu_ib_get(adev, NULL, size, &(*job)->ibs[0]);
> +	r = amdgpu_ib_get_with_pool(adev, NULL, size, &(*job)->ibs[0], pool_type);
>   	if (r)
>   		kfree(*job);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> index 2e2110dddb76..c34516bf9278 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
> @@ -38,6 +38,7 @@
>   #define AMDGPU_JOB_GET_VMID(job) ((job) ? (job)->vmid : 0)
>   
>   struct amdgpu_fence;
> +enum amd_ib_pool_type;
>   
>   struct amdgpu_job {
>   	struct drm_sched_job    base;
> @@ -67,7 +68,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
>   		     struct amdgpu_job **job, struct amdgpu_vm *vm);
>   int amdgpu_job_alloc_with_ib(struct amdgpu_device *adev, unsigned size,
>   			     struct amdgpu_job **job);
> -
> +int amdgpu_job_alloc_with_ib_pool(struct amdgpu_device *adev, unsigned size,
> +			     struct amdgpu_job **job, enum amd_ib_pool_type pool);

Again just add the new parameter to amdgpu_job_alloc_with_ib() instead 
of adding a new function.

Apart from that looks good to me,
Christian.

>   void amdgpu_job_free_resources(struct amdgpu_job *job);
>   void amdgpu_job_free(struct amdgpu_job *job);
>   int amdgpu_job_submit(struct amdgpu_job *job, struct drm_sched_entity *entity,

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-03-26 11:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  9:47 [RFC PATCH 1/2] drm/amdgpu: implement more ib pools xinhui pan
2020-03-26  9:47 ` [RFC PATCH 2/2] drm/amdgpu: use new job alloc variation if possible xinhui pan
2020-03-26 11:02 ` [RFC PATCH 1/2] drm/amdgpu: implement more ib pools Christian König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).