[PATCH 1/5] drm/amdgpu: only set cp active field for kiq queue

[PATCH 1/5] drm/amdgpu: only set cp active field for kiq queue

[Huang Rui]

The mec ucode will set the CP_HQD_ACTIVE bit while the queue is mapped by
MAP_QUEUES packet. So we only need set cp active field for kiq queue.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 7 +++++--
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 7 +++++--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 7 +++++--
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 5b05334..22c69d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3323,8 +3323,11 @@ static int gfx_v10_0_compute_mqd_init(struct amdgpu_ring *ring)
 	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
 	mqd->cp_hqd_ib_control = tmp;
 
-	/* activate the queue */
-	mqd->cp_hqd_active = 1;
+	/* map_queues packet doesn't need activate the queue,
+	 * so only kiq need set this field.
+	 */
+	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
+		mqd->cp_hqd_active = 1;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 8b9f440..306ee61 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -4558,8 +4558,11 @@ static int gfx_v8_0_mqd_init(struct amdgpu_ring *ring)
 	mqd->cp_hqd_eop_wptr_mem = RREG32(mmCP_HQD_EOP_WPTR_MEM);
 	mqd->cp_hqd_eop_dones = RREG32(mmCP_HQD_EOP_DONES);
 
-	/* activate the queue */
-	mqd->cp_hqd_active = 1;
+	/* map_queues packet doesn't need activate the queue,
+	 * so only kiq need set this field.
+	 */
+	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
+		mqd->cp_hqd_active = 1;
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 45328f9..e29818b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3313,8 +3313,11 @@ static int gfx_v9_0_mqd_init(struct amdgpu_ring *ring)
 	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
 	mqd->cp_hqd_ib_control = tmp;
 
-	/* activate the queue */
-	mqd->cp_hqd_active = 1;
+	/* map_queues packet doesn't need activate the queue,
+	 * so only kiq need set this field.
+	 */
+	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
+		mqd->cp_hqd_active = 1;
 
 	return 0;
 }
-- 
2.7.4

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

[PATCH 2/5] drm/amdkfd: use kiq to load the mqd of hiq queue for gfx v9 (v5)

[Huang Rui]

From: Aaron Liu <aaron.liu@amd.com>

There is an issue that CP will check the HIQ queue to be configured and mapped
with KIQ ring, otherwise, it will be unable to read back the secure buffer while
the gfxoff is enabled even with trusted IP blocks.

v1 -> v2:
- Fix to remove surplus set_resources packets.
- Fill the whole configuration in MQD.
- Change the author as Aaron because he addressed the key point of this issue.
- Add kiq ring lock.

v2 -> v3:
- Free the lock while in error return case.
- Remove the programming only needed by the queue is unmapped.

v3 -> v4:
- Remove doorbell programming because it's used for restarting queue.
- Remove CP scheduler programming because map_queue packet will handle this.

v4 -> v5:
- Remove cp_hqd_active because mec ucode will enable it while use map_queues.
- Revise goto out_unlock.
- Correct the right doorbell offset for HIQ that kfd driver assigned in the
  packet.

Reported-by: Lisa Saturday <Lisa.Saturday@amd.com>
Signed-off-by: Aaron Liu <aaron.liu@amd.com>
Signed-off-by: Huang Rui <ray.huang@amd.com>
Reviewed-and-Tested-by: Aaron Liu <aaron.liu@amd.com>
Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 79 +++++++++++++++++------
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c   | 10 ++-
 drivers/gpu/drm/amd/include/kgd_kfd_interface.h   |  4 ++
 3 files changed, 72 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
index e7861f0..ab8c23a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
@@ -103,13 +103,13 @@ static void acquire_queue(struct kgd_dev *kgd, uint32_t pipe_id,
 	lock_srbm(kgd, mec, pipe, queue_id, 0);
 }
 
-static uint32_t get_queue_mask(struct amdgpu_device *adev,
+static uint64_t get_queue_mask(struct amdgpu_device *adev,
 			       uint32_t pipe_id, uint32_t queue_id)
 {
-	unsigned int bit = (pipe_id * adev->gfx.mec.num_queue_per_pipe +
-			    queue_id) & 31;
+	unsigned int bit = pipe_id * adev->gfx.mec.num_queue_per_pipe +
+			queue_id;
 
-	return ((uint32_t)1) << bit;
+	return 1ull << bit;
 }
 
 static void release_queue(struct kgd_dev *kgd)
@@ -258,21 +258,6 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 
 	acquire_queue(kgd, pipe_id, queue_id);
 
-	/* HIQ is set during driver init period with vmid set to 0*/
-	if (m->cp_hqd_vmid == 0) {
-		uint32_t value, mec, pipe;
-
-		mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
-		pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
-
-		pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
-			mec, pipe, queue_id);
-		value = RREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS));
-		value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1,
-			((mec << 5) | (pipe << 3) | queue_id | 0x80));
-		WREG32_RLC(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS), value);
-	}
-
 	/* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */
 	mqd_hqd = &m->cp_mqd_base_addr_lo;
 	hqd_base = SOC15_REG_OFFSET(GC, 0, mmCP_MQD_BASE_ADDR);
@@ -323,7 +308,7 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 		WREG32_RLC(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR_HI),
 		       upper_32_bits((uintptr_t)wptr));
 		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_PQ_WPTR_POLL_CNTL1),
-		       get_queue_mask(adev, pipe_id, queue_id));
+		       (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
 	}
 
 	/* Start the EOP fetcher */
@@ -339,6 +324,59 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 	return 0;
 }
 
+static int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
+				   uint32_t pipe_id, uint32_t queue_id,
+				   uint32_t doorbell_off)
+{
+	struct amdgpu_device *adev = get_amdgpu_device(kgd);
+	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
+	struct v9_mqd *m;
+	uint32_t mec, pipe;
+	int r;
+
+	m = get_mqd(mqd);
+
+	acquire_queue(kgd, pipe_id, queue_id);
+
+	mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
+	pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
+
+	pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
+		 mec, pipe, queue_id);
+
+	spin_lock(&adev->gfx.kiq.ring_lock);
+	r = amdgpu_ring_alloc(kiq_ring, 7);
+	if (r) {
+		pr_err("Failed to alloc KIQ (%d).\n", r);
+		goto out_unlock;
+	}
+
+	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_MAP_QUEUES, 5));
+	amdgpu_ring_write(kiq_ring,
+			  PACKET3_MAP_QUEUES_QUEUE_SEL(0) | /* Queue_Sel */
+			  PACKET3_MAP_QUEUES_VMID(m->cp_hqd_vmid) | /* VMID */
+			  PACKET3_MAP_QUEUES_QUEUE(queue_id) |
+			  PACKET3_MAP_QUEUES_PIPE(pipe) |
+			  PACKET3_MAP_QUEUES_ME((mec - 1)) |
+			  PACKET3_MAP_QUEUES_QUEUE_TYPE(0) | /*queue_type: normal compute queue */
+			  PACKET3_MAP_QUEUES_ALLOC_FORMAT(0) | /* alloc format: all_on_one_pipe */
+			  PACKET3_MAP_QUEUES_ENGINE_SEL(1) | /* engine_sel: hiq */
+			  PACKET3_MAP_QUEUES_NUM_QUEUES(1)); /* num_queues: must be 1 */
+	amdgpu_ring_write(kiq_ring,
+			  PACKET3_MAP_QUEUES_DOORBELL_OFFSET(doorbell_off));
+	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_lo);
+	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_hi);
+	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_lo);
+	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_hi);
+	amdgpu_ring_commit(kiq_ring);
+
+out_unlock:
+	spin_unlock(&adev->gfx.kiq.ring_lock);
+	release_queue(kgd);
+
+	return r;
+}
+
 int kgd_gfx_v9_hqd_dump(struct kgd_dev *kgd,
 			uint32_t pipe_id, uint32_t queue_id,
 			uint32_t (**dump)[2], uint32_t *n_regs)
@@ -778,6 +816,7 @@ const struct kfd2kgd_calls gfx_v9_kfd2kgd = {
 	.set_pasid_vmid_mapping = kgd_gfx_v9_set_pasid_vmid_mapping,
 	.init_interrupts = kgd_gfx_v9_init_interrupts,
 	.hqd_load = kgd_gfx_v9_hqd_load,
+	.hiq_mqd_load = kgd_gfx_v9_hiq_mqd_load,
 	.hqd_sdma_load = kgd_hqd_sdma_load,
 	.hqd_dump = kgd_gfx_v9_hqd_dump,
 	.hqd_sdma_dump = kgd_hqd_sdma_dump,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
index aa90109..436b7f5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
@@ -191,6 +191,14 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
 					  wptr_shift, 0, mms);
 }
 
+static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    struct queue_properties *p, struct mm_struct *mms)
+{
+	return mm->dev->kfd2kgd->hiq_mqd_load(mm->dev->kgd, mqd, pipe_id,
+					      queue_id, p->doorbell_off);
+}
+
 static void update_mqd(struct mqd_manager *mm, void *mqd,
 		      struct queue_properties *q)
 {
@@ -449,7 +457,7 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
 		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->free_mqd = free_mqd_hiq_sdma;
-		mqd->load_mqd = load_mqd;
+		mqd->load_mqd = hiq_load_mqd_kiq;
 		mqd->update_mqd = update_mqd;
 		mqd->destroy_mqd = destroy_mqd;
 		mqd->is_occupied = is_occupied;
diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
index 2cd217e..e3831e6 100644
--- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
@@ -256,6 +256,10 @@ struct kfd2kgd_calls {
 			uint32_t wptr_shift, uint32_t wptr_mask,
 			struct mm_struct *mm);
 
+	int (*hiq_mqd_load)(struct kgd_dev *kgd, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    uint32_t doorbell_off);
+
 	int (*hqd_sdma_load)(struct kgd_dev *kgd, void *mqd,
 			     uint32_t __user *wptr, struct mm_struct *mm);
 
-- 
2.7.4

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

[PATCH 3/5] drm/amdkfd: use map_queues for hiq on arcturus as well

[Huang Rui]

Align with gfx v9, use the map_queues packet to load hiq MQD.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c   | 6 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h   | 3 +++
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
index 3c11940..8baad42 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
@@ -281,6 +281,7 @@ const struct kfd2kgd_calls arcturus_kfd2kgd = {
 	.set_pasid_vmid_mapping = kgd_gfx_v9_set_pasid_vmid_mapping,
 	.init_interrupts = kgd_gfx_v9_init_interrupts,
 	.hqd_load = kgd_gfx_v9_hqd_load,
+	.hiq_mqd_load = kgd_gfx_v9_hiq_mqd_load,
 	.hqd_sdma_load = kgd_hqd_sdma_load,
 	.hqd_dump = kgd_gfx_v9_hqd_dump,
 	.hqd_sdma_dump = kgd_hqd_sdma_dump,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
index ab8c23a..d2f9396 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
@@ -324,9 +324,9 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 	return 0;
 }
 
-static int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
-				   uint32_t pipe_id, uint32_t queue_id,
-				   uint32_t doorbell_off)
+int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    uint32_t doorbell_off)
 {
 	struct amdgpu_device *adev = get_amdgpu_device(kgd);
 	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
index 02b1426..32dd1a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
@@ -33,6 +33,9 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 			uint32_t queue_id, uint32_t __user *wptr,
 			uint32_t wptr_shift, uint32_t wptr_mask,
 			struct mm_struct *mm);
+int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    uint32_t doorbell_off);
 int kgd_gfx_v9_hqd_dump(struct kgd_dev *kgd,
 			uint32_t pipe_id, uint32_t queue_id,
 			uint32_t (**dump)[2], uint32_t *n_regs);
-- 
2.7.4

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

[PATCH 4/5] drm/amdkfd: use map_queues for hiq on gfx v10 as well

[Huang Rui]

To align with gfx v9, we use the map_queues packet to load hiq MQD.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c | 82 ++++++++++++++++------
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c   | 10 ++-
 2 files changed, 70 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
index 61cd707..2a60f73 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
@@ -107,13 +107,13 @@ static void acquire_queue(struct kgd_dev *kgd, uint32_t pipe_id,
 	lock_srbm(kgd, mec, pipe, queue_id, 0);
 }
 
-static uint32_t get_queue_mask(struct amdgpu_device *adev,
+static uint64_t get_queue_mask(struct amdgpu_device *adev,
 			       uint32_t pipe_id, uint32_t queue_id)
 {
-	unsigned int bit = (pipe_id * adev->gfx.mec.num_queue_per_pipe +
-			    queue_id) & 31;
+	unsigned int bit = pipe_id * adev->gfx.mec.num_queue_per_pipe +
+			queue_id;
 
-	return ((uint32_t)1) << bit;
+	return 1ull << bit;
 }
 
 static void release_queue(struct kgd_dev *kgd)
@@ -268,21 +268,6 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 	pr_debug("Load hqd of pipe %d queue %d\n", pipe_id, queue_id);
 	acquire_queue(kgd, pipe_id, queue_id);
 
-	/* HIQ is set during driver init period with vmid set to 0*/
-	if (m->cp_hqd_vmid == 0) {
-		uint32_t value, mec, pipe;
-
-		mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
-		pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
-
-		pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
-			mec, pipe, queue_id);
-		value = RREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS));
-		value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1,
-			((mec << 5) | (pipe << 3) | queue_id | 0x80));
-		WREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS), value);
-	}
-
 	/* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */
 	mqd_hqd = &m->cp_mqd_base_addr_lo;
 	hqd_base = SOC15_REG_OFFSET(GC, 0, mmCP_MQD_BASE_ADDR);
@@ -332,9 +317,10 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 		       lower_32_bits((uint64_t)wptr));
 		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR_HI),
 		       upper_32_bits((uint64_t)wptr));
-		pr_debug("%s setting CP_PQ_WPTR_POLL_CNTL1 to %x\n", __func__, get_queue_mask(adev, pipe_id, queue_id));
+		pr_debug("%s setting CP_PQ_WPTR_POLL_CNTL1 to %x\n", __func__,
+			 (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
 		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_PQ_WPTR_POLL_CNTL1),
-		       get_queue_mask(adev, pipe_id, queue_id));
+		       (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
 	}
 
 	/* Start the EOP fetcher */
@@ -350,6 +336,59 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 	return 0;
 }
 
+static int kgd_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    uint32_t doorbell_off)
+{
+	struct amdgpu_device *adev = get_amdgpu_device(kgd);
+	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
+	struct v10_compute_mqd *m;
+	uint32_t mec, pipe;
+	int r;
+
+	m = get_mqd(mqd);
+
+	acquire_queue(kgd, pipe_id, queue_id);
+
+	mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
+	pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
+
+	pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
+		 mec, pipe, queue_id);
+
+	spin_lock(&adev->gfx.kiq.ring_lock);
+	r = amdgpu_ring_alloc(kiq_ring, 7);
+	if (r) {
+		pr_err("Failed to alloc KIQ (%d).\n", r);
+		goto out_unlock;
+	}
+
+	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_MAP_QUEUES, 5));
+	amdgpu_ring_write(kiq_ring,
+			  PACKET3_MAP_QUEUES_QUEUE_SEL(0) | /* Queue_Sel */
+			  PACKET3_MAP_QUEUES_VMID(m->cp_hqd_vmid) | /* VMID */
+			  PACKET3_MAP_QUEUES_QUEUE(queue_id) |
+			  PACKET3_MAP_QUEUES_PIPE(pipe) |
+			  PACKET3_MAP_QUEUES_ME((mec - 1)) |
+			  PACKET3_MAP_QUEUES_QUEUE_TYPE(0) | /*queue_type: normal compute queue */
+			  PACKET3_MAP_QUEUES_ALLOC_FORMAT(0) | /* alloc format: all_on_one_pipe */
+			  PACKET3_MAP_QUEUES_ENGINE_SEL(1) | /* engine_sel: hiq */
+			  PACKET3_MAP_QUEUES_NUM_QUEUES(1)); /* num_queues: must be 1 */
+	amdgpu_ring_write(kiq_ring,
+			  PACKET3_MAP_QUEUES_DOORBELL_OFFSET(doorbell_off));
+	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_lo);
+	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_hi);
+	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_lo);
+	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_hi);
+	amdgpu_ring_commit(kiq_ring);
+
+out_unlock:
+	spin_unlock(&adev->gfx.kiq.ring_lock);
+	release_queue(kgd);
+
+	return r;
+}
+
 static int kgd_hqd_dump(struct kgd_dev *kgd,
 			uint32_t pipe_id, uint32_t queue_id,
 			uint32_t (**dump)[2], uint32_t *n_regs)
@@ -817,6 +856,7 @@ const struct kfd2kgd_calls gfx_v10_kfd2kgd = {
 	.set_pasid_vmid_mapping = kgd_set_pasid_vmid_mapping,
 	.init_interrupts = kgd_init_interrupts,
 	.hqd_load = kgd_hqd_load,
+	.hiq_mqd_load = kgd_hiq_mqd_load,
 	.hqd_sdma_load = kgd_hqd_sdma_load,
 	.hqd_dump = kgd_hqd_dump,
 	.hqd_sdma_dump = kgd_hqd_sdma_dump,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
index 7832ec6..d1d68a5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
@@ -153,6 +153,14 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
 	return r;
 }
 
+static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    struct queue_properties *p, struct mm_struct *mms)
+{
+	return mm->dev->kfd2kgd->hiq_mqd_load(mm->dev->kgd, mqd, pipe_id,
+					      queue_id, p->doorbell_off);
+}
+
 static void update_mqd(struct mqd_manager *mm, void *mqd,
 		      struct queue_properties *q)
 {
@@ -409,7 +417,7 @@ struct mqd_manager *mqd_manager_init_v10(enum KFD_MQD_TYPE type,
 		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->free_mqd = free_mqd_hiq_sdma;
-		mqd->load_mqd = load_mqd;
+		mqd->load_mqd = hiq_load_mqd_kiq;
 		mqd->update_mqd = update_mqd;
 		mqd->destroy_mqd = destroy_mqd;
 		mqd->is_occupied = is_occupied;
-- 
2.7.4

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

[PATCH 5/5] drm/amdkfd: use map_queues for hiq on gfx v8 as well

[Huang Rui]

Align with gfx v9, use map_queues packet to load hiq MQD.

Signed-off-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 69 ++++++++++++++++++-----
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c   | 10 +++-
 2 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
index bfbdded..6adac95 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
@@ -207,21 +207,6 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 
 	acquire_queue(kgd, pipe_id, queue_id);
 
-	/* HIQ is set during driver init period with vmid set to 0*/
-	if (m->cp_hqd_vmid == 0) {
-		uint32_t value, mec, pipe;
-
-		mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
-		pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
-
-		pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
-			mec, pipe, queue_id);
-		value = RREG32(mmRLC_CP_SCHEDULERS);
-		value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1,
-			((mec << 5) | (pipe << 3) | queue_id | 0x80));
-		WREG32(mmRLC_CP_SCHEDULERS, value);
-	}
-
 	/* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */
 	mqd_hqd = &m->cp_mqd_base_addr_lo;
 
@@ -267,6 +252,59 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
 	return 0;
 }
 
+static int kgd_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    uint32_t doorbell_off)
+{
+	struct amdgpu_device *adev = get_amdgpu_device(kgd);
+	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
+	struct vi_mqd *m;
+	uint32_t mec, pipe;
+	int r;
+
+	m = get_mqd(mqd);
+
+	acquire_queue(kgd, pipe_id, queue_id);
+
+	mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
+	pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
+
+	pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
+		 mec, pipe, queue_id);
+
+	spin_lock(&adev->gfx.kiq.ring_lock);
+	r = amdgpu_ring_alloc(kiq_ring, 7);
+	if (r) {
+		pr_err("Failed to alloc KIQ (%d).\n", r);
+		goto out_unlock;
+	}
+
+	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_MAP_QUEUES, 5));
+	amdgpu_ring_write(kiq_ring,
+			  PACKET3_MAP_QUEUES_QUEUE_SEL(0) | /* Queue_Sel */
+			  PACKET3_MAP_QUEUES_VMID(m->cp_hqd_vmid) | /* VMID */
+			  PACKET3_MAP_QUEUES_QUEUE(queue_id) |
+			  PACKET3_MAP_QUEUES_PIPE(pipe) |
+			  PACKET3_MAP_QUEUES_ME((mec - 1)) |
+			  PACKET3_MAP_QUEUES_QUEUE_TYPE(0) | /*queue_type: normal compute queue */
+			  PACKET3_MAP_QUEUES_ALLOC_FORMAT(0) | /* alloc format: all_on_one_pipe */
+			  PACKET3_MAP_QUEUES_ENGINE_SEL(1) | /* engine_sel: hiq */
+			  PACKET3_MAP_QUEUES_NUM_QUEUES(1)); /* num_queues: must be 1 */
+	amdgpu_ring_write(kiq_ring,
+			  PACKET3_MAP_QUEUES_DOORBELL_OFFSET(doorbell_off));
+	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_lo);
+	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_hi);
+	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_lo);
+	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_hi);
+	amdgpu_ring_commit(kiq_ring);
+
+out_unlock:
+	spin_unlock(&adev->gfx.kiq.ring_lock);
+	release_queue(kgd);
+
+	return r;
+}
+
 static int kgd_hqd_dump(struct kgd_dev *kgd,
 			uint32_t pipe_id, uint32_t queue_id,
 			uint32_t (**dump)[2], uint32_t *n_regs)
@@ -701,6 +739,7 @@ const struct kfd2kgd_calls gfx_v8_kfd2kgd = {
 	.set_pasid_vmid_mapping = kgd_set_pasid_vmid_mapping,
 	.init_interrupts = kgd_init_interrupts,
 	.hqd_load = kgd_hqd_load,
+	.hiq_mqd_load = kgd_hiq_mqd_load,
 	.hqd_sdma_load = kgd_hqd_sdma_load,
 	.hqd_dump = kgd_hqd_dump,
 	.hqd_sdma_dump = kgd_hqd_sdma_dump,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
index a5e8ff1..15cbfd4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
@@ -166,6 +166,14 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
 					  wptr_shift, wptr_mask, mms);
 }
 
+static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
+			    uint32_t pipe_id, uint32_t queue_id,
+			    struct queue_properties *p, struct mm_struct *mms)
+{
+	return mm->dev->kfd2kgd->hiq_mqd_load(mm->dev->kgd, mqd, pipe_id,
+					      queue_id, p->doorbell_off);
+}
+
 static void __update_mqd(struct mqd_manager *mm, void *mqd,
 			struct queue_properties *q, unsigned int mtype,
 			unsigned int atc_bit)
@@ -438,7 +446,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
 		mqd->allocate_mqd = allocate_hiq_mqd;
 		mqd->init_mqd = init_mqd_hiq;
 		mqd->free_mqd = free_mqd_hiq_sdma;
-		mqd->load_mqd = load_mqd;
+		mqd->load_mqd = hiq_load_mqd_kiq;
 		mqd->update_mqd = update_mqd_hiq;
 		mqd->destroy_mqd = destroy_mqd;
 		mqd->is_occupied = is_occupied;
-- 
2.7.4

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

Re: [PATCH 3/5] drm/amdkfd: use map_queues for hiq on arcturus as well

[Felix Kuehling]

What happens on Arcturus without this patch? Does it oops with a null 
pointer dereference? If yes, then you should squash this patch into 
patch 2 to avoid a broken intermediate state.

Regards,
   Felix

On 2020-01-10 1:37 a.m., Huang Rui wrote:
> Align with gfx v9, use the map_queues packet to load hiq MQD.
>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c   | 6 +++---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h   | 3 +++
>   3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> index 3c11940..8baad42 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> @@ -281,6 +281,7 @@ const struct kfd2kgd_calls arcturus_kfd2kgd = {
>   	.set_pasid_vmid_mapping = kgd_gfx_v9_set_pasid_vmid_mapping,
>   	.init_interrupts = kgd_gfx_v9_init_interrupts,
>   	.hqd_load = kgd_gfx_v9_hqd_load,
> +	.hiq_mqd_load = kgd_gfx_v9_hiq_mqd_load,
>   	.hqd_sdma_load = kgd_hqd_sdma_load,
>   	.hqd_dump = kgd_gfx_v9_hqd_dump,
>   	.hqd_sdma_dump = kgd_hqd_sdma_dump,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index ab8c23a..d2f9396 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -324,9 +324,9 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   	return 0;
>   }
>   
> -static int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> -				   uint32_t pipe_id, uint32_t queue_id,
> -				   uint32_t doorbell_off)
> +int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> +			    uint32_t pipe_id, uint32_t queue_id,
> +			    uint32_t doorbell_off)
>   {
>   	struct amdgpu_device *adev = get_amdgpu_device(kgd);
>   	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> index 02b1426..32dd1a9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> @@ -33,6 +33,9 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   			uint32_t queue_id, uint32_t __user *wptr,
>   			uint32_t wptr_shift, uint32_t wptr_mask,
>   			struct mm_struct *mm);
> +int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> +			    uint32_t pipe_id, uint32_t queue_id,
> +			    uint32_t doorbell_off);
>   int kgd_gfx_v9_hqd_dump(struct kgd_dev *kgd,
>   			uint32_t pipe_id, uint32_t queue_id,
>   			uint32_t (**dump)[2], uint32_t *n_regs);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] drm/amdkfd: use map_queues for hiq on gfx v10 as well

[Felix Kuehling]

On 2020-01-10 1:37 a.m., Huang Rui wrote:
> To align with gfx v9, we use the map_queues packet to load hiq MQD.
>
> Signed-off-by: Huang Rui <ray.huang@amd.com>

Please make sure you test this on a GFXv10 GPU.

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c | 82 ++++++++++++++++------
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c   | 10 ++-
>   2 files changed, 70 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> index 61cd707..2a60f73 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> @@ -107,13 +107,13 @@ static void acquire_queue(struct kgd_dev *kgd, uint32_t pipe_id,
>   	lock_srbm(kgd, mec, pipe, queue_id, 0);
>   }
>   
> -static uint32_t get_queue_mask(struct amdgpu_device *adev,
> +static uint64_t get_queue_mask(struct amdgpu_device *adev,
>   			       uint32_t pipe_id, uint32_t queue_id)
>   {
> -	unsigned int bit = (pipe_id * adev->gfx.mec.num_queue_per_pipe +
> -			    queue_id) & 31;
> +	unsigned int bit = pipe_id * adev->gfx.mec.num_queue_per_pipe +
> +			queue_id;
>   
> -	return ((uint32_t)1) << bit;
> +	return 1ull << bit;
>   }
>   
>   static void release_queue(struct kgd_dev *kgd)
> @@ -268,21 +268,6 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   	pr_debug("Load hqd of pipe %d queue %d\n", pipe_id, queue_id);
>   	acquire_queue(kgd, pipe_id, queue_id);
>   
> -	/* HIQ is set during driver init period with vmid set to 0*/
> -	if (m->cp_hqd_vmid == 0) {
> -		uint32_t value, mec, pipe;
> -
> -		mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
> -		pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
> -
> -		pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
> -			mec, pipe, queue_id);
> -		value = RREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS));
> -		value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1,
> -			((mec << 5) | (pipe << 3) | queue_id | 0x80));
> -		WREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS), value);
> -	}
> -
>   	/* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */
>   	mqd_hqd = &m->cp_mqd_base_addr_lo;
>   	hqd_base = SOC15_REG_OFFSET(GC, 0, mmCP_MQD_BASE_ADDR);
> @@ -332,9 +317,10 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   		       lower_32_bits((uint64_t)wptr));
>   		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR_HI),
>   		       upper_32_bits((uint64_t)wptr));
> -		pr_debug("%s setting CP_PQ_WPTR_POLL_CNTL1 to %x\n", __func__, get_queue_mask(adev, pipe_id, queue_id));
> +		pr_debug("%s setting CP_PQ_WPTR_POLL_CNTL1 to %x\n", __func__,
> +			 (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
>   		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_PQ_WPTR_POLL_CNTL1),
> -		       get_queue_mask(adev, pipe_id, queue_id));
> +		       (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
>   	}
>   
>   	/* Start the EOP fetcher */
> @@ -350,6 +336,59 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   	return 0;
>   }
>   
> +static int kgd_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> +			    uint32_t pipe_id, uint32_t queue_id,
> +			    uint32_t doorbell_off)
> +{
> +	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> +	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
> +	struct v10_compute_mqd *m;
> +	uint32_t mec, pipe;
> +	int r;
> +
> +	m = get_mqd(mqd);
> +
> +	acquire_queue(kgd, pipe_id, queue_id);
> +
> +	mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
> +	pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
> +
> +	pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
> +		 mec, pipe, queue_id);
> +
> +	spin_lock(&adev->gfx.kiq.ring_lock);
> +	r = amdgpu_ring_alloc(kiq_ring, 7);
> +	if (r) {
> +		pr_err("Failed to alloc KIQ (%d).\n", r);
> +		goto out_unlock;
> +	}
> +
> +	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_MAP_QUEUES, 5));
> +	amdgpu_ring_write(kiq_ring,
> +			  PACKET3_MAP_QUEUES_QUEUE_SEL(0) | /* Queue_Sel */
> +			  PACKET3_MAP_QUEUES_VMID(m->cp_hqd_vmid) | /* VMID */
> +			  PACKET3_MAP_QUEUES_QUEUE(queue_id) |
> +			  PACKET3_MAP_QUEUES_PIPE(pipe) |
> +			  PACKET3_MAP_QUEUES_ME((mec - 1)) |
> +			  PACKET3_MAP_QUEUES_QUEUE_TYPE(0) | /*queue_type: normal compute queue */
> +			  PACKET3_MAP_QUEUES_ALLOC_FORMAT(0) | /* alloc format: all_on_one_pipe */
> +			  PACKET3_MAP_QUEUES_ENGINE_SEL(1) | /* engine_sel: hiq */
> +			  PACKET3_MAP_QUEUES_NUM_QUEUES(1)); /* num_queues: must be 1 */
> +	amdgpu_ring_write(kiq_ring,
> +			  PACKET3_MAP_QUEUES_DOORBELL_OFFSET(doorbell_off));
> +	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_lo);
> +	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_hi);
> +	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_lo);
> +	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_hi);
> +	amdgpu_ring_commit(kiq_ring);
> +
> +out_unlock:
> +	spin_unlock(&adev->gfx.kiq.ring_lock);
> +	release_queue(kgd);
> +
> +	return r;
> +}
> +
>   static int kgd_hqd_dump(struct kgd_dev *kgd,
>   			uint32_t pipe_id, uint32_t queue_id,
>   			uint32_t (**dump)[2], uint32_t *n_regs)
> @@ -817,6 +856,7 @@ const struct kfd2kgd_calls gfx_v10_kfd2kgd = {
>   	.set_pasid_vmid_mapping = kgd_set_pasid_vmid_mapping,
>   	.init_interrupts = kgd_init_interrupts,
>   	.hqd_load = kgd_hqd_load,
> +	.hiq_mqd_load = kgd_hiq_mqd_load,
>   	.hqd_sdma_load = kgd_hqd_sdma_load,
>   	.hqd_dump = kgd_hqd_dump,
>   	.hqd_sdma_dump = kgd_hqd_sdma_dump,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> index 7832ec6..d1d68a5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> @@ -153,6 +153,14 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
>   	return r;
>   }
>   
> +static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
> +			    uint32_t pipe_id, uint32_t queue_id,
> +			    struct queue_properties *p, struct mm_struct *mms)
> +{
> +	return mm->dev->kfd2kgd->hiq_mqd_load(mm->dev->kgd, mqd, pipe_id,
> +					      queue_id, p->doorbell_off);
> +}
> +
>   static void update_mqd(struct mqd_manager *mm, void *mqd,
>   		      struct queue_properties *q)
>   {
> @@ -409,7 +417,7 @@ struct mqd_manager *mqd_manager_init_v10(enum KFD_MQD_TYPE type,
>   		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->free_mqd = free_mqd_hiq_sdma;
> -		mqd->load_mqd = load_mqd;
> +		mqd->load_mqd = hiq_load_mqd_kiq;
>   		mqd->update_mqd = update_mqd;
>   		mqd->destroy_mqd = destroy_mqd;
>   		mqd->is_occupied = is_occupied;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/5] drm/amdkfd: use map_queues for hiq on gfx v8 as well

[Felix Kuehling]

On 2020-01-10 1:37 a.m., Huang Rui wrote:
> Align with gfx v9, use map_queues packet to load hiq MQD.
>
> Signed-off-by: Huang Rui <ray.huang@amd.com>
Please make sure you test this on a GFXv8 GPU.

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 69 ++++++++++++++++++-----
>   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c   | 10 +++-
>   2 files changed, 63 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> index bfbdded..6adac95 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> @@ -207,21 +207,6 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   
>   	acquire_queue(kgd, pipe_id, queue_id);
>   
> -	/* HIQ is set during driver init period with vmid set to 0*/
> -	if (m->cp_hqd_vmid == 0) {
> -		uint32_t value, mec, pipe;
> -
> -		mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
> -		pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
> -
> -		pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
> -			mec, pipe, queue_id);
> -		value = RREG32(mmRLC_CP_SCHEDULERS);
> -		value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1,
> -			((mec << 5) | (pipe << 3) | queue_id | 0x80));
> -		WREG32(mmRLC_CP_SCHEDULERS, value);
> -	}
> -
>   	/* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */
>   	mqd_hqd = &m->cp_mqd_base_addr_lo;
>   
> @@ -267,6 +252,59 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
>   	return 0;
>   }
>   
> +static int kgd_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> +			    uint32_t pipe_id, uint32_t queue_id,
> +			    uint32_t doorbell_off)
> +{
> +	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> +	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
> +	struct vi_mqd *m;
> +	uint32_t mec, pipe;
> +	int r;
> +
> +	m = get_mqd(mqd);
> +
> +	acquire_queue(kgd, pipe_id, queue_id);
> +
> +	mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
> +	pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
> +
> +	pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
> +		 mec, pipe, queue_id);
> +
> +	spin_lock(&adev->gfx.kiq.ring_lock);
> +	r = amdgpu_ring_alloc(kiq_ring, 7);
> +	if (r) {
> +		pr_err("Failed to alloc KIQ (%d).\n", r);
> +		goto out_unlock;
> +	}
> +
> +	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_MAP_QUEUES, 5));
> +	amdgpu_ring_write(kiq_ring,
> +			  PACKET3_MAP_QUEUES_QUEUE_SEL(0) | /* Queue_Sel */
> +			  PACKET3_MAP_QUEUES_VMID(m->cp_hqd_vmid) | /* VMID */
> +			  PACKET3_MAP_QUEUES_QUEUE(queue_id) |
> +			  PACKET3_MAP_QUEUES_PIPE(pipe) |
> +			  PACKET3_MAP_QUEUES_ME((mec - 1)) |
> +			  PACKET3_MAP_QUEUES_QUEUE_TYPE(0) | /*queue_type: normal compute queue */
> +			  PACKET3_MAP_QUEUES_ALLOC_FORMAT(0) | /* alloc format: all_on_one_pipe */
> +			  PACKET3_MAP_QUEUES_ENGINE_SEL(1) | /* engine_sel: hiq */
> +			  PACKET3_MAP_QUEUES_NUM_QUEUES(1)); /* num_queues: must be 1 */
> +	amdgpu_ring_write(kiq_ring,
> +			  PACKET3_MAP_QUEUES_DOORBELL_OFFSET(doorbell_off));
> +	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_lo);
> +	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_hi);
> +	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_lo);
> +	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_hi);
> +	amdgpu_ring_commit(kiq_ring);
> +
> +out_unlock:
> +	spin_unlock(&adev->gfx.kiq.ring_lock);
> +	release_queue(kgd);
> +
> +	return r;
> +}
> +
>   static int kgd_hqd_dump(struct kgd_dev *kgd,
>   			uint32_t pipe_id, uint32_t queue_id,
>   			uint32_t (**dump)[2], uint32_t *n_regs)
> @@ -701,6 +739,7 @@ const struct kfd2kgd_calls gfx_v8_kfd2kgd = {
>   	.set_pasid_vmid_mapping = kgd_set_pasid_vmid_mapping,
>   	.init_interrupts = kgd_init_interrupts,
>   	.hqd_load = kgd_hqd_load,
> +	.hiq_mqd_load = kgd_hiq_mqd_load,
>   	.hqd_sdma_load = kgd_hqd_sdma_load,
>   	.hqd_dump = kgd_hqd_dump,
>   	.hqd_sdma_dump = kgd_hqd_sdma_dump,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> index a5e8ff1..15cbfd4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> @@ -166,6 +166,14 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
>   					  wptr_shift, wptr_mask, mms);
>   }
>   
> +static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
> +			    uint32_t pipe_id, uint32_t queue_id,
> +			    struct queue_properties *p, struct mm_struct *mms)
> +{
> +	return mm->dev->kfd2kgd->hiq_mqd_load(mm->dev->kgd, mqd, pipe_id,
> +					      queue_id, p->doorbell_off);
> +}
> +
>   static void __update_mqd(struct mqd_manager *mm, void *mqd,
>   			struct queue_properties *q, unsigned int mtype,
>   			unsigned int atc_bit)
> @@ -438,7 +446,7 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
>   		mqd->allocate_mqd = allocate_hiq_mqd;
>   		mqd->init_mqd = init_mqd_hiq;
>   		mqd->free_mqd = free_mqd_hiq_sdma;
> -		mqd->load_mqd = load_mqd;
> +		mqd->load_mqd = hiq_load_mqd_kiq;
>   		mqd->update_mqd = update_mqd_hiq;
>   		mqd->destroy_mqd = destroy_mqd;
>   		mqd->is_occupied = is_occupied;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amdkfd: use map_queues for hiq on arcturus as well

[Huang Rui]

On Sat, Jan 11, 2020 at 07:05:40AM +0800, Kuehling, Felix wrote:
> What happens on Arcturus without this patch? Does it oops with a null 
> pointer dereference? If yes, then you should squash this patch into 
> patch 2 to avoid a broken intermediate state.

Yes, Arcturus will get a null pointer panic without this patch becasue
hiq_mqd_load is not inited in arcturus_kfd2kgd. I will squash this patch
into the patch 2.

Thanks,
Ray

> 
> Regards,
>    Felix
> 
> On 2020-01-10 1:37 a.m., Huang Rui wrote:
> > Align with gfx v9, use the map_queues packet to load hiq MQD.
> >
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 1 +
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c   | 6 +++---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h   | 3 +++
> >   3 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> > index 3c11940..8baad42 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c
> > @@ -281,6 +281,7 @@ const struct kfd2kgd_calls arcturus_kfd2kgd = {
> >   	.set_pasid_vmid_mapping = kgd_gfx_v9_set_pasid_vmid_mapping,
> >   	.init_interrupts = kgd_gfx_v9_init_interrupts,
> >   	.hqd_load = kgd_gfx_v9_hqd_load,
> > +	.hiq_mqd_load = kgd_gfx_v9_hiq_mqd_load,
> >   	.hqd_sdma_load = kgd_hqd_sdma_load,
> >   	.hqd_dump = kgd_gfx_v9_hqd_dump,
> >   	.hqd_sdma_dump = kgd_hqd_sdma_dump,
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> > index ab8c23a..d2f9396 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> > @@ -324,9 +324,9 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
> >   	return 0;
> >   }
> >   
> > -static int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> > -				   uint32_t pipe_id, uint32_t queue_id,
> > -				   uint32_t doorbell_off)
> > +int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> > +			    uint32_t pipe_id, uint32_t queue_id,
> > +			    uint32_t doorbell_off)
> >   {
> >   	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> >   	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> > index 02b1426..32dd1a9 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h
> > @@ -33,6 +33,9 @@ int kgd_gfx_v9_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
> >   			uint32_t queue_id, uint32_t __user *wptr,
> >   			uint32_t wptr_shift, uint32_t wptr_mask,
> >   			struct mm_struct *mm);
> > +int kgd_gfx_v9_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> > +			    uint32_t pipe_id, uint32_t queue_id,
> > +			    uint32_t doorbell_off);
> >   int kgd_gfx_v9_hqd_dump(struct kgd_dev *kgd,
> >   			uint32_t pipe_id, uint32_t queue_id,
> >   			uint32_t (**dump)[2], uint32_t *n_regs);
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] drm/amdkfd: use map_queues for hiq on gfx v10 as well

[Huang Rui]

On Sat, Jan 11, 2020 at 07:08:42AM +0800, Kuehling, Felix wrote:
> On 2020-01-10 1:37 a.m., Huang Rui wrote:
> > To align with gfx v9, we use the map_queues packet to load hiq MQD.
> >
> > Signed-off-by: Huang Rui <ray.huang@amd.com>
> 
> Please make sure you test this on a GFXv10 GPU.
> 
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> 

I tested both navi14 and fiji card, gfx10 works well. But gfx8 failed with
page fault...

Thanks,
Ray

> 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c | 82 ++++++++++++++++------
> >   drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c   | 10 ++-
> >   2 files changed, 70 insertions(+), 22 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> > index 61cd707..2a60f73 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c
> > @@ -107,13 +107,13 @@ static void acquire_queue(struct kgd_dev *kgd, uint32_t pipe_id,
> >   	lock_srbm(kgd, mec, pipe, queue_id, 0);
> >   }
> >   
> > -static uint32_t get_queue_mask(struct amdgpu_device *adev,
> > +static uint64_t get_queue_mask(struct amdgpu_device *adev,
> >   			       uint32_t pipe_id, uint32_t queue_id)
> >   {
> > -	unsigned int bit = (pipe_id * adev->gfx.mec.num_queue_per_pipe +
> > -			    queue_id) & 31;
> > +	unsigned int bit = pipe_id * adev->gfx.mec.num_queue_per_pipe +
> > +			queue_id;
> >   
> > -	return ((uint32_t)1) << bit;
> > +	return 1ull << bit;
> >   }
> >   
> >   static void release_queue(struct kgd_dev *kgd)
> > @@ -268,21 +268,6 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
> >   	pr_debug("Load hqd of pipe %d queue %d\n", pipe_id, queue_id);
> >   	acquire_queue(kgd, pipe_id, queue_id);
> >   
> > -	/* HIQ is set during driver init period with vmid set to 0*/
> > -	if (m->cp_hqd_vmid == 0) {
> > -		uint32_t value, mec, pipe;
> > -
> > -		mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
> > -		pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
> > -
> > -		pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
> > -			mec, pipe, queue_id);
> > -		value = RREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS));
> > -		value = REG_SET_FIELD(value, RLC_CP_SCHEDULERS, scheduler1,
> > -			((mec << 5) | (pipe << 3) | queue_id | 0x80));
> > -		WREG32(SOC15_REG_OFFSET(GC, 0, mmRLC_CP_SCHEDULERS), value);
> > -	}
> > -
> >   	/* HQD registers extend from CP_MQD_BASE_ADDR to CP_HQD_EOP_WPTR_MEM. */
> >   	mqd_hqd = &m->cp_mqd_base_addr_lo;
> >   	hqd_base = SOC15_REG_OFFSET(GC, 0, mmCP_MQD_BASE_ADDR);
> > @@ -332,9 +317,10 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
> >   		       lower_32_bits((uint64_t)wptr));
> >   		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_HQD_PQ_WPTR_POLL_ADDR_HI),
> >   		       upper_32_bits((uint64_t)wptr));
> > -		pr_debug("%s setting CP_PQ_WPTR_POLL_CNTL1 to %x\n", __func__, get_queue_mask(adev, pipe_id, queue_id));
> > +		pr_debug("%s setting CP_PQ_WPTR_POLL_CNTL1 to %x\n", __func__,
> > +			 (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
> >   		WREG32(SOC15_REG_OFFSET(GC, 0, mmCP_PQ_WPTR_POLL_CNTL1),
> > -		       get_queue_mask(adev, pipe_id, queue_id));
> > +		       (uint32_t)get_queue_mask(adev, pipe_id, queue_id));
> >   	}
> >   
> >   	/* Start the EOP fetcher */
> > @@ -350,6 +336,59 @@ static int kgd_hqd_load(struct kgd_dev *kgd, void *mqd, uint32_t pipe_id,
> >   	return 0;
> >   }
> >   
> > +static int kgd_hiq_mqd_load(struct kgd_dev *kgd, void *mqd,
> > +			    uint32_t pipe_id, uint32_t queue_id,
> > +			    uint32_t doorbell_off)
> > +{
> > +	struct amdgpu_device *adev = get_amdgpu_device(kgd);
> > +	struct amdgpu_ring *kiq_ring = &adev->gfx.kiq.ring;
> > +	struct v10_compute_mqd *m;
> > +	uint32_t mec, pipe;
> > +	int r;
> > +
> > +	m = get_mqd(mqd);
> > +
> > +	acquire_queue(kgd, pipe_id, queue_id);
> > +
> > +	mec = (pipe_id / adev->gfx.mec.num_pipe_per_mec) + 1;
> > +	pipe = (pipe_id % adev->gfx.mec.num_pipe_per_mec);
> > +
> > +	pr_debug("kfd: set HIQ, mec:%d, pipe:%d, queue:%d.\n",
> > +		 mec, pipe, queue_id);
> > +
> > +	spin_lock(&adev->gfx.kiq.ring_lock);
> > +	r = amdgpu_ring_alloc(kiq_ring, 7);
> > +	if (r) {
> > +		pr_err("Failed to alloc KIQ (%d).\n", r);
> > +		goto out_unlock;
> > +	}
> > +
> > +	amdgpu_ring_write(kiq_ring, PACKET3(PACKET3_MAP_QUEUES, 5));
> > +	amdgpu_ring_write(kiq_ring,
> > +			  PACKET3_MAP_QUEUES_QUEUE_SEL(0) | /* Queue_Sel */
> > +			  PACKET3_MAP_QUEUES_VMID(m->cp_hqd_vmid) | /* VMID */
> > +			  PACKET3_MAP_QUEUES_QUEUE(queue_id) |
> > +			  PACKET3_MAP_QUEUES_PIPE(pipe) |
> > +			  PACKET3_MAP_QUEUES_ME((mec - 1)) |
> > +			  PACKET3_MAP_QUEUES_QUEUE_TYPE(0) | /*queue_type: normal compute queue */
> > +			  PACKET3_MAP_QUEUES_ALLOC_FORMAT(0) | /* alloc format: all_on_one_pipe */
> > +			  PACKET3_MAP_QUEUES_ENGINE_SEL(1) | /* engine_sel: hiq */
> > +			  PACKET3_MAP_QUEUES_NUM_QUEUES(1)); /* num_queues: must be 1 */
> > +	amdgpu_ring_write(kiq_ring,
> > +			  PACKET3_MAP_QUEUES_DOORBELL_OFFSET(doorbell_off));
> > +	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_lo);
> > +	amdgpu_ring_write(kiq_ring, m->cp_mqd_base_addr_hi);
> > +	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_lo);
> > +	amdgpu_ring_write(kiq_ring, m->cp_hqd_pq_wptr_poll_addr_hi);
> > +	amdgpu_ring_commit(kiq_ring);
> > +
> > +out_unlock:
> > +	spin_unlock(&adev->gfx.kiq.ring_lock);
> > +	release_queue(kgd);
> > +
> > +	return r;
> > +}
> > +
> >   static int kgd_hqd_dump(struct kgd_dev *kgd,
> >   			uint32_t pipe_id, uint32_t queue_id,
> >   			uint32_t (**dump)[2], uint32_t *n_regs)
> > @@ -817,6 +856,7 @@ const struct kfd2kgd_calls gfx_v10_kfd2kgd = {
> >   	.set_pasid_vmid_mapping = kgd_set_pasid_vmid_mapping,
> >   	.init_interrupts = kgd_init_interrupts,
> >   	.hqd_load = kgd_hqd_load,
> > +	.hiq_mqd_load = kgd_hiq_mqd_load,
> >   	.hqd_sdma_load = kgd_hqd_sdma_load,
> >   	.hqd_dump = kgd_hqd_dump,
> >   	.hqd_sdma_dump = kgd_hqd_sdma_dump,
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> > index 7832ec6..d1d68a5 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
> > @@ -153,6 +153,14 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
> >   	return r;
> >   }
> >   
> > +static int hiq_load_mqd_kiq(struct mqd_manager *mm, void *mqd,
> > +			    uint32_t pipe_id, uint32_t queue_id,
> > +			    struct queue_properties *p, struct mm_struct *mms)
> > +{
> > +	return mm->dev->kfd2kgd->hiq_mqd_load(mm->dev->kgd, mqd, pipe_id,
> > +					      queue_id, p->doorbell_off);
> > +}
> > +
> >   static void update_mqd(struct mqd_manager *mm, void *mqd,
> >   		      struct queue_properties *q)
> >   {
> > @@ -409,7 +417,7 @@ struct mqd_manager *mqd_manager_init_v10(enum KFD_MQD_TYPE type,
> >   		mqd->allocate_mqd = allocate_hiq_mqd;
> >   		mqd->init_mqd = init_mqd_hiq;
> >   		mqd->free_mqd = free_mqd_hiq_sdma;
> > -		mqd->load_mqd = load_mqd;
> > +		mqd->load_mqd = hiq_load_mqd_kiq;
> >   		mqd->update_mqd = update_mqd;
> >   		mqd->destroy_mqd = destroy_mqd;
> >   		mqd->is_occupied = is_occupied;
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: only set cp active field for kiq queue

[Huang Rui]

Ping~

On Fri, Jan 10, 2020 at 02:37:09PM +0800, Huang, Ray wrote:
> The mec ucode will set the CP_HQD_ACTIVE bit while the queue is mapped by
> MAP_QUEUES packet. So we only need set cp active field for kiq queue.
> 
> Signed-off-by: Huang Rui <ray.huang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 7 +++++--
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 7 +++++--
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 7 +++++--
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 5b05334..22c69d8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -3323,8 +3323,11 @@ static int gfx_v10_0_compute_mqd_init(struct amdgpu_ring *ring)
>  	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
>  	mqd->cp_hqd_ib_control = tmp;
>  
> -	/* activate the queue */
> -	mqd->cp_hqd_active = 1;
> +	/* map_queues packet doesn't need activate the queue,
> +	 * so only kiq need set this field.
> +	 */
> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> +		mqd->cp_hqd_active = 1;
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 8b9f440..306ee61 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -4558,8 +4558,11 @@ static int gfx_v8_0_mqd_init(struct amdgpu_ring *ring)
>  	mqd->cp_hqd_eop_wptr_mem = RREG32(mmCP_HQD_EOP_WPTR_MEM);
>  	mqd->cp_hqd_eop_dones = RREG32(mmCP_HQD_EOP_DONES);
>  
> -	/* activate the queue */
> -	mqd->cp_hqd_active = 1;
> +	/* map_queues packet doesn't need activate the queue,
> +	 * so only kiq need set this field.
> +	 */
> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> +		mqd->cp_hqd_active = 1;
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 45328f9..e29818b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -3313,8 +3313,11 @@ static int gfx_v9_0_mqd_init(struct amdgpu_ring *ring)
>  	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
>  	mqd->cp_hqd_ib_control = tmp;
>  
> -	/* activate the queue */
> -	mqd->cp_hqd_active = 1;
> +	/* map_queues packet doesn't need activate the queue,
> +	 * so only kiq need set this field.
> +	 */
> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> +		mqd->cp_hqd_active = 1;
>  
>  	return 0;
>  }
> -- 
> 2.7.4
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: only set cp active field for kiq queue

[Felix Kuehling]

Patch 1 is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

Patches 2 and 3 should be squashed. In that case they are

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

Patches 4 and 5 need to be tested. I think you mentioned GFX10 worked 
(patch 4) but GFX8 was failing (patch 5). Have you found the problem? I 
haven't seen any update to the patch.

Regards,
   Felix

On 2020-01-14 4:36, Huang Rui wrote:
> Ping~
>
> On Fri, Jan 10, 2020 at 02:37:09PM +0800, Huang, Ray wrote:
>> The mec ucode will set the CP_HQD_ACTIVE bit while the queue is mapped by
>> MAP_QUEUES packet. So we only need set cp active field for kiq queue.
>>
>> Signed-off-by: Huang Rui <ray.huang@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 7 +++++--
>>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 7 +++++--
>>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 7 +++++--
>>   3 files changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> index 5b05334..22c69d8 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
>> @@ -3323,8 +3323,11 @@ static int gfx_v10_0_compute_mqd_init(struct amdgpu_ring *ring)
>>   	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
>>   	mqd->cp_hqd_ib_control = tmp;
>>   
>> -	/* activate the queue */
>> -	mqd->cp_hqd_active = 1;
>> +	/* map_queues packet doesn't need activate the queue,
>> +	 * so only kiq need set this field.
>> +	 */
>> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
>> +		mqd->cp_hqd_active = 1;
>>   
>>   	return 0;
>>   }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> index 8b9f440..306ee61 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
>> @@ -4558,8 +4558,11 @@ static int gfx_v8_0_mqd_init(struct amdgpu_ring *ring)
>>   	mqd->cp_hqd_eop_wptr_mem = RREG32(mmCP_HQD_EOP_WPTR_MEM);
>>   	mqd->cp_hqd_eop_dones = RREG32(mmCP_HQD_EOP_DONES);
>>   
>> -	/* activate the queue */
>> -	mqd->cp_hqd_active = 1;
>> +	/* map_queues packet doesn't need activate the queue,
>> +	 * so only kiq need set this field.
>> +	 */
>> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
>> +		mqd->cp_hqd_active = 1;
>>   
>>   	return 0;
>>   }
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 45328f9..e29818b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -3313,8 +3313,11 @@ static int gfx_v9_0_mqd_init(struct amdgpu_ring *ring)
>>   	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
>>   	mqd->cp_hqd_ib_control = tmp;
>>   
>> -	/* activate the queue */
>> -	mqd->cp_hqd_active = 1;
>> +	/* map_queues packet doesn't need activate the queue,
>> +	 * so only kiq need set this field.
>> +	 */
>> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
>> +		mqd->cp_hqd_active = 1;
>>   
>>   	return 0;
>>   }
>> -- 
>> 2.7.4
>>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/5] drm/amdgpu: only set cp active field for kiq queue

[Huang Rui]

On Tue, Jan 14, 2020 at 11:30:15PM +0800, Kuehling, Felix wrote:
> Patch 1 is
> 
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> 
> Patches 2 and 3 should be squashed. In that case they are
> 
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
> 
> Patches 4 and 5 need to be tested. I think you mentioned GFX10 worked 
> (patch 4) but GFX8 was failing (patch 5). Have you found the problem? I 
> haven't seen any update to the patch.

Yes, GFX8 (patch5) failed with the page fault, I am still looking at this
issue. For other patches (verified good), I have pushed into drm-next.

Thanks,
Ray

> 
> Regards,
>    Felix
> 
> On 2020-01-14 4:36, Huang Rui wrote:
> > Ping~
> >
> > On Fri, Jan 10, 2020 at 02:37:09PM +0800, Huang, Ray wrote:
> >> The mec ucode will set the CP_HQD_ACTIVE bit while the queue is mapped by
> >> MAP_QUEUES packet. So we only need set cp active field for kiq queue.
> >>
> >> Signed-off-by: Huang Rui <ray.huang@amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 7 +++++--
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c  | 7 +++++--
> >>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 7 +++++--
> >>   3 files changed, 15 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> index 5b05334..22c69d8 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> >> @@ -3323,8 +3323,11 @@ static int gfx_v10_0_compute_mqd_init(struct amdgpu_ring *ring)
> >>   	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
> >>   	mqd->cp_hqd_ib_control = tmp;
> >>   
> >> -	/* activate the queue */
> >> -	mqd->cp_hqd_active = 1;
> >> +	/* map_queues packet doesn't need activate the queue,
> >> +	 * so only kiq need set this field.
> >> +	 */
> >> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> >> +		mqd->cp_hqd_active = 1;
> >>   
> >>   	return 0;
> >>   }
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> index 8b9f440..306ee61 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> >> @@ -4558,8 +4558,11 @@ static int gfx_v8_0_mqd_init(struct amdgpu_ring *ring)
> >>   	mqd->cp_hqd_eop_wptr_mem = RREG32(mmCP_HQD_EOP_WPTR_MEM);
> >>   	mqd->cp_hqd_eop_dones = RREG32(mmCP_HQD_EOP_DONES);
> >>   
> >> -	/* activate the queue */
> >> -	mqd->cp_hqd_active = 1;
> >> +	/* map_queues packet doesn't need activate the queue,
> >> +	 * so only kiq need set this field.
> >> +	 */
> >> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> >> +		mqd->cp_hqd_active = 1;
> >>   
> >>   	return 0;
> >>   }
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> index 45328f9..e29818b 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> >> @@ -3313,8 +3313,11 @@ static int gfx_v9_0_mqd_init(struct amdgpu_ring *ring)
> >>   	tmp = REG_SET_FIELD(tmp, CP_HQD_IB_CONTROL, MIN_IB_AVAIL_SIZE, 3);
> >>   	mqd->cp_hqd_ib_control = tmp;
> >>   
> >> -	/* activate the queue */
> >> -	mqd->cp_hqd_active = 1;
> >> +	/* map_queues packet doesn't need activate the queue,
> >> +	 * so only kiq need set this field.
> >> +	 */
> >> +	if (ring->funcs->type == AMDGPU_RING_TYPE_KIQ)
> >> +		mqd->cp_hqd_active = 1;
> >>   
> >>   	return 0;
> >>   }
> >> -- 
> >> 2.7.4
> >>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx