[PATCH 0/8] More KFD fixes and cleanups v2

[PATCH 0/8] More KFD fixes and cleanups v2

[Felix Kuehling]

Another pass through a diff between our internal branch and upstream
yielded a few more fixes and cleanups that were previously missed.

v2: Added a fix for a bug found when syncing upstreamed changes back
into our internal branch (Update queue_count before mapping queues).
Remove a redundant condition (Minor cleanups v2).

Ben Goz (1):
  drm/amdkfd: Register/Deregister process on qpd resolution

Felix Kuehling (2):
  drm/amdkfd: Update queue_count before mapping queues
  drm/amdkfd: Minor cleanups v2

Jay Cornwall (1):
  drm/amdkfd: Disable CP/SDMA ring/doorbell in MQD

Yair Shachar (1):
  drm/amdkfd: Fix debug unregister procedure on process termination

Yong Zhao (3):
  drm/amdkfd: Clean up the data structure in kfd_process
  drm/amdkfd: Avoid calling amd_iommu_unbind_pasid() when suspending
  drm/amdkfd: Cleanup DQM ASIC-specific ops

 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c  | 49 +++++++--------
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h  | 11 ++--
 .../drm/amd/amdkfd/kfd_device_queue_manager_cik.c  | 20 +++---
 .../drm/amd/amdkfd/kfd_device_queue_manager_vi.c   | 20 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c   | 34 +++--------
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c    |  7 +--
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h              |  8 +--
 drivers/gpu/drm/amd/amdkfd/kfd_process.c           | 71 ++++++++++------------
 .../gpu/drm/amd/amdkfd/kfd_process_queue_manager.c |  9 ++-
 9 files changed, 94 insertions(+), 135 deletions(-)

-- 
2.7.4

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

[PATCH 1/8] drm/amdkfd: Clean up the data structure in kfd_process

[Felix Kuehling]

From: Yong Zhao <yong.zhao@amd.com>

A list of per-process queues is maintained in the
kfd_process_queue_manager, so the queues array in kfd_process is
redundant and in fact unused.

Signed-off-by: Yong Zhao <yong.zhao@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 ------
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 18 ------------------
 2 files changed, 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index bf29021..3b0cc20 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -527,12 +527,6 @@ struct kfd_process {
 
 	struct process_queue_manager pqm;
 
-	/* The process's queues. */
-	size_t queue_array_size;
-
-	/* Size is queue_array_size, up to MAX_PROCESS_QUEUES. */
-	struct kfd_queue **queues;
-
 	/*Is the user space process 32 bit?*/
 	bool is_32bit_user_mode;
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 21d27e5..f871855 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -35,13 +35,6 @@ struct mm_struct;
 #include "kfd_dbgmgr.h"
 
 /*
- * Initial size for the array of queues.
- * The allocated size is doubled each time
- * it is exceeded up to MAX_PROCESS_QUEUES.
- */
-#define INITIAL_QUEUE_ARRAY_SIZE 16
-
-/*
  * List of struct kfd_process (field kfd_process).
  * Unique/indexed by mm_struct*
  */
@@ -187,8 +180,6 @@ static void kfd_process_wq_release(struct work_struct *work)
 
 	mutex_destroy(&p->mutex);
 
-	kfree(p->queues);
-
 	kfree(p);
 
 	kfree(work);
@@ -275,11 +266,6 @@ static struct kfd_process *create_process(const struct task_struct *thread)
 	if (!process)
 		goto err_alloc_process;
 
-	process->queues = kmalloc_array(INITIAL_QUEUE_ARRAY_SIZE,
-					sizeof(process->queues[0]), GFP_KERNEL);
-	if (!process->queues)
-		goto err_alloc_queues;
-
 	process->pasid = kfd_pasid_alloc();
 	if (process->pasid == 0)
 		goto err_alloc_pasid;
@@ -302,8 +288,6 @@ static struct kfd_process *create_process(const struct task_struct *thread)
 
 	process->lead_thread = thread->group_leader;
 
-	process->queue_array_size = INITIAL_QUEUE_ARRAY_SIZE;
-
 	INIT_LIST_HEAD(&process->per_device_data);
 
 	kfd_event_init_process(process);
@@ -332,8 +316,6 @@ static struct kfd_process *create_process(const struct task_struct *thread)
 err_alloc_doorbells:
 	kfd_pasid_free(process->pasid);
 err_alloc_pasid:
-	kfree(process->queues);
-err_alloc_queues:
 	kfree(process);
 err_alloc_process:
 	return ERR_PTR(err);
-- 
2.7.4

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

[PATCH 2/8] drm/amdkfd: Disable CP/SDMA ring/doorbell in MQD

[Felix Kuehling]

From: Jay Cornwall <Jay.Cornwall@amd.com>

The MQD represents an inactive context and should not have ring or
doorbell enable bits set. Doing so interferes with HWS which streams
the MQD onto the HQD. If enable bits are set this activates the ring
or doorbell before the HQD is fully configured.

Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 34 +++++++-----------------
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c  |  7 ++---
 2 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
index 44ffd23..4859d26 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
@@ -189,12 +189,9 @@ static int update_mqd(struct mqd_manager *mm, void *mqd,
 	if (q->format == KFD_QUEUE_FORMAT_AQL)
 		m->cp_hqd_pq_control |= NO_UPDATE_RPTR;
 
-	q->is_active = false;
-	if (q->queue_size > 0 &&
+	q->is_active = (q->queue_size > 0 &&
 			q->queue_address != 0 &&
-			q->queue_percent > 0) {
-		q->is_active = true;
-	}
+			q->queue_percent > 0);
 
 	return 0;
 }
@@ -215,24 +212,17 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
 	m->sdma_rlc_rb_base_hi = upper_32_bits(q->queue_address >> 8);
 	m->sdma_rlc_rb_rptr_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
 	m->sdma_rlc_rb_rptr_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
-	m->sdma_rlc_doorbell = q->doorbell_off <<
-			SDMA0_RLC0_DOORBELL__OFFSET__SHIFT |
-			1 << SDMA0_RLC0_DOORBELL__ENABLE__SHIFT;
+	m->sdma_rlc_doorbell =
+		q->doorbell_off << SDMA0_RLC0_DOORBELL__OFFSET__SHIFT;
 
 	m->sdma_rlc_virtual_addr = q->sdma_vm_addr;
 
 	m->sdma_engine_id = q->sdma_engine_id;
 	m->sdma_queue_id = q->sdma_queue_id;
 
-	q->is_active = false;
-	if (q->queue_size > 0 &&
+	q->is_active = (q->queue_size > 0 &&
 			q->queue_address != 0 &&
-			q->queue_percent > 0) {
-		m->sdma_rlc_rb_cntl |=
-				1 << SDMA0_RLC0_RB_CNTL__RB_ENABLE__SHIFT;
-
-		q->is_active = true;
-	}
+			q->queue_percent > 0);
 
 	return 0;
 }
@@ -359,19 +349,13 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
 	m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q->queue_address >> 8);
 	m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
 	m->cp_hqd_pq_rptr_report_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
-	m->cp_hqd_pq_doorbell_control = DOORBELL_EN |
-					DOORBELL_OFFSET(q->doorbell_off);
+	m->cp_hqd_pq_doorbell_control = DOORBELL_OFFSET(q->doorbell_off);
 
 	m->cp_hqd_vmid = q->vmid;
 
-	m->cp_hqd_active = 0;
-	q->is_active = false;
-	if (q->queue_size > 0 &&
+	q->is_active = (q->queue_size > 0 &&
 			q->queue_address != 0 &&
-			q->queue_percent > 0) {
-		m->cp_hqd_active = 1;
-		q->is_active = true;
-	}
+			q->queue_percent > 0);
 
 	return 0;
 }
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 73cbfe1..4ea854f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
@@ -163,12 +163,9 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd,
 				2 << CP_HQD_PQ_CONTROL__SLOT_BASED_WPTR__SHIFT;
 	}
 
-	q->is_active = false;
-	if (q->queue_size > 0 &&
+	q->is_active = (q->queue_size > 0 &&
 			q->queue_address != 0 &&
-			q->queue_percent > 0) {
-		q->is_active = true;
-	}
+			q->queue_percent > 0);
 
 	return 0;
 }
-- 
2.7.4

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

[PATCH 3/8] drm/amdkfd: Avoid calling amd_iommu_unbind_pasid() when suspending

[Felix Kuehling]

From: Yong Zhao <yong.zhao@amd.com>

When kfd suspending on APU, we do not need to call
amd_iommu_unbind_pasid(), because pasid will be unbound automatically
when power goes off.

On the other hand, calling amd_iommu_unbind_pasid() will trigger
kfd_process_iommu_unbind_callback() if the process is not terminating.
By design, kfd_process_iommu_unbind_callback() should only be called
for process terminating. So we would rather not to call
amd_iommu_unbind_pasid() when suspending.

Signed-off-by: Yong Zhao <yong.zhao@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index f871855..5d93a5c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -428,29 +428,25 @@ int kfd_bind_processes_to_device(struct kfd_dev *dev)
 }
 
 /*
- * Temporarily unbind currently bound processes from the device and
- * mark them as PDD_BOUND_SUSPENDED. These processes will be restored
- * to PDD_BOUND state in kfd_bind_processes_to_device.
+ * Mark currently bound processes as PDD_BOUND_SUSPENDED. These
+ * processes will be restored to PDD_BOUND state in
+ * kfd_bind_processes_to_device.
  */
 void kfd_unbind_processes_from_device(struct kfd_dev *dev)
 {
 	struct kfd_process_device *pdd;
 	struct kfd_process *p;
-	unsigned int temp, temp_bound, temp_pasid;
+	unsigned int temp;
 
 	int idx = srcu_read_lock(&kfd_processes_srcu);
 
 	hash_for_each_rcu(kfd_processes_table, temp, p, kfd_processes) {
 		mutex_lock(&p->mutex);
 		pdd = kfd_get_process_device_data(dev, p);
-		temp_bound = pdd->bound;
-		temp_pasid = p->pasid;
+
 		if (pdd->bound == PDD_BOUND)
 			pdd->bound = PDD_BOUND_SUSPENDED;
 		mutex_unlock(&p->mutex);
-
-		if (temp_bound == PDD_BOUND)
-			amd_iommu_unbind_pasid(dev->pdev, temp_pasid);
 	}
 
 	srcu_read_unlock(&kfd_processes_srcu, idx);
-- 
2.7.4

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

[PATCH 4/8] drm/amdkfd: Fix debug unregister procedure on process termination

[Felix Kuehling]

From: Yair Shachar <yair.shachar@amd.com>

Take the dbgmgr lock and unregister before destroying the debug manager.
Do this before destroying the queues.

Signed-off-by: Yair Shachar <yair.shachar@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 37 +++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 5d93a5c..6caf6df 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -229,17 +229,26 @@ static void kfd_process_notifier_release(struct mmu_notifier *mn,
 
 	mutex_lock(&p->mutex);
 
+	/* Iterate over all process device data structures and if the
+	 * pdd is in debug mode, we should first force unregistration,
+	 * then we will be able to destroy the queues
+	 */
+	list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
+		struct kfd_dev *dev = pdd->dev;
+
+		mutex_lock(kfd_get_dbgmgr_mutex());
+		if (dev && dev->dbgmgr && dev->dbgmgr->pasid == p->pasid) {
+			if (!kfd_dbgmgr_unregister(dev->dbgmgr, p)) {
+				kfd_dbgmgr_destroy(dev->dbgmgr);
+				dev->dbgmgr = NULL;
+			}
+		}
+		mutex_unlock(kfd_get_dbgmgr_mutex());
+	}
+
 	kfd_process_dequeue_from_all_devices(p);
 	pqm_uninit(&p->pqm);
 
-	/* Iterate over all process device data structure and check
-	 * if we should delete debug managers
-	 */
-	list_for_each_entry(pdd, &p->per_device_data, per_device_list)
-		if ((pdd->dev->dbgmgr) &&
-				(pdd->dev->dbgmgr->pasid == p->pasid))
-			kfd_dbgmgr_destroy(pdd->dev->dbgmgr);
-
 	mutex_unlock(&p->mutex);
 
 	/*
@@ -468,8 +477,16 @@ void kfd_process_iommu_unbind_callback(struct kfd_dev *dev, unsigned int pasid)
 
 	pr_debug("Unbinding process %d from IOMMU\n", pasid);
 
-	if ((dev->dbgmgr) && (dev->dbgmgr->pasid == p->pasid))
-		kfd_dbgmgr_destroy(dev->dbgmgr);
+	mutex_lock(kfd_get_dbgmgr_mutex());
+
+	if (dev->dbgmgr && dev->dbgmgr->pasid == p->pasid) {
+		if (!kfd_dbgmgr_unregister(dev->dbgmgr, p)) {
+			kfd_dbgmgr_destroy(dev->dbgmgr);
+			dev->dbgmgr = NULL;
+		}
+	}
+
+	mutex_unlock(kfd_get_dbgmgr_mutex());
 
 	pdd = kfd_get_process_device_data(dev, p);
 	if (pdd)
-- 
2.7.4

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

[PATCH 5/8] drm/amdkfd: Register/Deregister process on qpd resolution

[Felix Kuehling]

From: Ben Goz <ben.goz@amd.com>

Process registration needs to happen on each device. So use per-device
queue lists to determine when to register/deregister the process.

Signed-off-by: Ben Goz <ben.goz@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
index 5129dc1..2bec902 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
@@ -177,7 +177,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
 	if (retval != 0)
 		return retval;
 
-	if (list_empty(&pqm->queues)) {
+	if (list_empty(&pdd->qpd.queues_list) &&
+	    list_empty(&pdd->qpd.priv_queue_list)) {
 		pdd->qpd.pqm = pqm;
 		dev->dqm->ops.register_process(dev->dqm, &pdd->qpd);
 	}
@@ -248,7 +249,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
 err_allocate_pqn:
 	/* check if queues list is empty unregister process from device */
 	clear_bit(*qid, pqm->queue_slot_bitmap);
-	if (list_empty(&pqm->queues))
+	if (list_empty(&pdd->qpd.queues_list) &&
+	    list_empty(&pdd->qpd.priv_queue_list))
 		dev->dqm->ops.unregister_process(dev->dqm, &pdd->qpd);
 	return retval;
 }
@@ -302,7 +304,8 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
 	kfree(pqn);
 	clear_bit(qid, pqm->queue_slot_bitmap);
 
-	if (list_empty(&pqm->queues))
+	if (list_empty(&pdd->qpd.queues_list) &&
+	    list_empty(&pdd->qpd.priv_queue_list))
 		dqm->ops.unregister_process(dqm, &pdd->qpd);
 
 	return retval;
-- 
2.7.4

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

[PATCH 6/8] drm/amdkfd: Cleanup DQM ASIC-specific ops

[Felix Kuehling]

From: Yong Zhao <yong.zhao@amd.com>

Remove empty initialize function.

Rename register_process to update_qpd to avoid confusion with the
non-ASIC-specific register_process.

Shorten ops_asic_specific to asic_ops.

Signed-off-by: Yong Zhao <yong.zhao@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c    | 19 +++++++------------
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h    | 11 ++++++-----
 .../drm/amd/amdkfd/kfd_device_queue_manager_cik.c    | 20 +++++++-------------
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c | 20 +++++++-------------
 4 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index da3b743..45b98dd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -467,7 +467,7 @@ static int register_process(struct device_queue_manager *dqm,
 	mutex_lock(&dqm->lock);
 	list_add(&n->list, &dqm->queues);
 
-	retval = dqm->ops_asic_specific.register_process(dqm, qpd);
+	retval = dqm->asic_ops.update_qpd(dqm, qpd);
 
 	dqm->processes_count++;
 
@@ -629,7 +629,7 @@ static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
 	pr_debug("SDMA queue id: %d\n", q->properties.sdma_queue_id);
 	pr_debug("SDMA engine id: %d\n", q->properties.sdma_engine_id);
 
-	dqm->ops_asic_specific.init_sdma_vm(dqm, q, qpd);
+	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
 	retval = mqd->init_mqd(mqd, &q->mqd, &q->mqd_mem_obj,
 				&q->gart_mqd_addr, &q->properties);
 	if (retval)
@@ -696,8 +696,6 @@ static int set_sched_resources(struct device_queue_manager *dqm)
 
 static int initialize_cpsch(struct device_queue_manager *dqm)
 {
-	int retval;
-
 	pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
 
 	mutex_init(&dqm->lock);
@@ -706,11 +704,8 @@ static int initialize_cpsch(struct device_queue_manager *dqm)
 	dqm->sdma_queue_count = 0;
 	dqm->active_runlist = false;
 	dqm->sdma_bitmap = (1 << CIK_SDMA_QUEUES) - 1;
-	retval = dqm->ops_asic_specific.initialize(dqm);
-	if (retval)
-		mutex_destroy(&dqm->lock);
 
-	return retval;
+	return 0;
 }
 
 static int start_cpsch(struct device_queue_manager *dqm)
@@ -850,7 +845,7 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 		goto out;
 	}
 
-	dqm->ops_asic_specific.init_sdma_vm(dqm, q, qpd);
+	dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
 	retval = mqd->init_mqd(mqd, &q->mqd, &q->mqd_mem_obj,
 				&q->gart_mqd_addr, &q->properties);
 	if (retval)
@@ -1095,7 +1090,7 @@ static bool set_cache_memory_policy(struct device_queue_manager *dqm,
 		qpd->sh_mem_ape1_limit = limit >> 16;
 	}
 
-	retval = dqm->ops_asic_specific.set_cache_memory_policy(
+	retval = dqm->asic_ops.set_cache_memory_policy(
 			dqm,
 			qpd,
 			default_policy,
@@ -1270,11 +1265,11 @@ struct device_queue_manager *device_queue_manager_init(struct kfd_dev *dev)
 
 	switch (dev->device_info->asic_family) {
 	case CHIP_CARRIZO:
-		device_queue_manager_init_vi(&dqm->ops_asic_specific);
+		device_queue_manager_init_vi(&dqm->asic_ops);
 		break;
 
 	case CHIP_KAVERI:
-		device_queue_manager_init_cik(&dqm->ops_asic_specific);
+		device_queue_manager_init_cik(&dqm->asic_ops);
 		break;
 	default:
 		WARN(1, "Unexpected ASIC family %u",
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
index 31c2b1f..5b77cb6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
@@ -128,9 +128,8 @@ struct device_queue_manager_ops {
 };
 
 struct device_queue_manager_asic_ops {
-	int	(*register_process)(struct device_queue_manager *dqm,
+	int	(*update_qpd)(struct device_queue_manager *dqm,
 					struct qcm_process_device *qpd);
-	int	(*initialize)(struct device_queue_manager *dqm);
 	bool	(*set_cache_memory_policy)(struct device_queue_manager *dqm,
 					   struct qcm_process_device *qpd,
 					   enum cache_policy default_policy,
@@ -156,7 +155,7 @@ struct device_queue_manager_asic_ops {
 
 struct device_queue_manager {
 	struct device_queue_manager_ops ops;
-	struct device_queue_manager_asic_ops ops_asic_specific;
+	struct device_queue_manager_asic_ops asic_ops;
 
 	struct mqd_manager	*mqds[KFD_MQD_TYPE_MAX];
 	struct packet_manager	packets;
@@ -179,8 +178,10 @@ struct device_queue_manager {
 	bool			active_runlist;
 };
 
-void device_queue_manager_init_cik(struct device_queue_manager_asic_ops *ops);
-void device_queue_manager_init_vi(struct device_queue_manager_asic_ops *ops);
+void device_queue_manager_init_cik(
+		struct device_queue_manager_asic_ops *asic_ops);
+void device_queue_manager_init_vi(
+		struct device_queue_manager_asic_ops *asic_ops);
 void program_sh_mem_settings(struct device_queue_manager *dqm,
 					struct qcm_process_device *qpd);
 unsigned int get_queues_num(struct device_queue_manager *dqm);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c
index 72c3cba..28e48c9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c
@@ -32,18 +32,17 @@ static bool set_cache_memory_policy_cik(struct device_queue_manager *dqm,
 				   enum cache_policy alternate_policy,
 				   void __user *alternate_aperture_base,
 				   uint64_t alternate_aperture_size);
-static int register_process_cik(struct device_queue_manager *dqm,
+static int update_qpd_cik(struct device_queue_manager *dqm,
 					struct qcm_process_device *qpd);
-static int initialize_cpsch_cik(struct device_queue_manager *dqm);
 static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
 				struct qcm_process_device *qpd);
 
-void device_queue_manager_init_cik(struct device_queue_manager_asic_ops *ops)
+void device_queue_manager_init_cik(
+		struct device_queue_manager_asic_ops *asic_ops)
 {
-	ops->set_cache_memory_policy = set_cache_memory_policy_cik;
-	ops->register_process = register_process_cik;
-	ops->initialize = initialize_cpsch_cik;
-	ops->init_sdma_vm = init_sdma_vm;
+	asic_ops->set_cache_memory_policy = set_cache_memory_policy_cik;
+	asic_ops->update_qpd = update_qpd_cik;
+	asic_ops->init_sdma_vm = init_sdma_vm;
 }
 
 static uint32_t compute_sh_mem_bases_64bit(unsigned int top_address_nybble)
@@ -99,7 +98,7 @@ static bool set_cache_memory_policy_cik(struct device_queue_manager *dqm,
 	return true;
 }
 
-static int register_process_cik(struct device_queue_manager *dqm,
+static int update_qpd_cik(struct device_queue_manager *dqm,
 		struct qcm_process_device *qpd)
 {
 	struct kfd_process_device *pdd;
@@ -148,8 +147,3 @@ static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
 
 	q->properties.sdma_vm_addr = value;
 }
-
-static int initialize_cpsch_cik(struct device_queue_manager *dqm)
-{
-	return 0;
-}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c
index 40e9ddd..2fbce57 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c
@@ -33,18 +33,17 @@ static bool set_cache_memory_policy_vi(struct device_queue_manager *dqm,
 				   enum cache_policy alternate_policy,
 				   void __user *alternate_aperture_base,
 				   uint64_t alternate_aperture_size);
-static int register_process_vi(struct device_queue_manager *dqm,
+static int update_qpd_vi(struct device_queue_manager *dqm,
 					struct qcm_process_device *qpd);
-static int initialize_cpsch_vi(struct device_queue_manager *dqm);
 static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
 				struct qcm_process_device *qpd);
 
-void device_queue_manager_init_vi(struct device_queue_manager_asic_ops *ops)
+void device_queue_manager_init_vi(
+		struct device_queue_manager_asic_ops *asic_ops)
 {
-	ops->set_cache_memory_policy = set_cache_memory_policy_vi;
-	ops->register_process = register_process_vi;
-	ops->initialize = initialize_cpsch_vi;
-	ops->init_sdma_vm = init_sdma_vm;
+	asic_ops->set_cache_memory_policy = set_cache_memory_policy_vi;
+	asic_ops->update_qpd = update_qpd_vi;
+	asic_ops->init_sdma_vm = init_sdma_vm;
 }
 
 static uint32_t compute_sh_mem_bases_64bit(unsigned int top_address_nybble)
@@ -104,7 +103,7 @@ static bool set_cache_memory_policy_vi(struct device_queue_manager *dqm,
 	return true;
 }
 
-static int register_process_vi(struct device_queue_manager *dqm,
+static int update_qpd_vi(struct device_queue_manager *dqm,
 					struct qcm_process_device *qpd)
 {
 	struct kfd_process_device *pdd;
@@ -160,8 +159,3 @@ static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
 
 	q->properties.sdma_vm_addr = value;
 }
-
-static int initialize_cpsch_vi(struct device_queue_manager *dqm)
-{
-	return 0;
-}
-- 
2.7.4

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

[PATCH 7/8] drm/amdkfd: Update queue_count before mapping queues

[Felix Kuehling]

map_queues_cpsch uses the queue_count to decide whether to upload
a new runlist. So update the counter before calling it.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c    | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 45b98dd..e2fc4c5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -408,6 +408,17 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
 
 	retval = mqd->update_mqd(mqd, q->mqd, &q->properties);
 
+	/*
+	 * check active state vs. the previous state and modify
+	 * counter accordingly. map_queues_cpsch uses the
+	 * dqm->queue_count to determine whether a new runlist must be
+	 * uploaded.
+	 */
+	if (q->properties.is_active && !prev_active)
+		dqm->queue_count++;
+	else if (!q->properties.is_active && prev_active)
+		dqm->queue_count--;
+
 	if (sched_policy != KFD_SCHED_POLICY_NO_HWS)
 		retval = map_queues_cpsch(dqm);
 	else if (sched_policy == KFD_SCHED_POLICY_NO_HWS &&
@@ -417,15 +428,6 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
 		retval = mqd->load_mqd(mqd, q->mqd, q->pipe, q->queue,
 				       &q->properties, q->process->mm);
 
-	/*
-	 * check active state vs. the previous state
-	 * and modify counter accordingly
-	 */
-	if (q->properties.is_active && !prev_active)
-		dqm->queue_count++;
-	else if (!q->properties.is_active && prev_active)
-		dqm->queue_count--;
-
 out_unlock:
 	mutex_unlock(&dqm->lock);
 	return retval;
-- 
2.7.4

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

[PATCH 8/8] drm/amdkfd: Minor cleanups v2

[Felix Kuehling]

These were missed previously when rebasing changes for upstreaming.

v2: Remove redundant sched_policy conditions

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 10 ++++------
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h                 |  2 ++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c              |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e2fc4c5..e202921 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -389,12 +389,11 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
 	if (sched_policy != KFD_SCHED_POLICY_NO_HWS) {
 		retval = unmap_queues_cpsch(dqm,
 				KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
-		if (retval != 0) {
+		if (retval) {
 			pr_err("unmap queue failed\n");
 			goto out_unlock;
 		}
-	} else if (sched_policy == KFD_SCHED_POLICY_NO_HWS &&
-		   prev_active &&
+	} else if (prev_active &&
 		   (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
 		    q->properties.type == KFD_QUEUE_TYPE_SDMA)) {
 		retval = mqd->destroy_mqd(mqd, q->mqd,
@@ -421,8 +420,7 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
 
 	if (sched_policy != KFD_SCHED_POLICY_NO_HWS)
 		retval = map_queues_cpsch(dqm);
-	else if (sched_policy == KFD_SCHED_POLICY_NO_HWS &&
-		 q->properties.is_active &&
+	else if (q->properties.is_active &&
 		 (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
 		  q->properties.type == KFD_QUEUE_TYPE_SDMA))
 		retval = mqd->load_mqd(mqd, q->mqd, q->pipe, q->queue,
@@ -832,7 +830,7 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
 
 	if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
 		retval = allocate_sdma_queue(dqm, &q->sdma_id);
-		if (retval != 0)
+		if (retval)
 			goto out;
 		q->properties.sdma_queue_id =
 			q->sdma_id / CIK_SDMA_QUEUES_PER_ENGINE;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 3b0cc20..9e27c16 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -507,6 +507,8 @@ struct kfd_process {
 	 * In any process, the thread that started main() is the lead
 	 * thread and outlives the rest.
 	 * It is here because amd_iommu_bind_pasid wants a task_struct.
+	 * It can also be used for safely getting a reference to the
+	 * mm_struct of the process.
 	 */
 	struct task_struct *lead_thread;
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 6caf6df..674b788 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -421,7 +421,7 @@ int kfd_bind_processes_to_device(struct kfd_dev *dev)
 		err = amd_iommu_bind_pasid(dev->pdev, p->pasid,
 				p->lead_thread);
 		if (err < 0) {
-			pr_err("unexpected pasid %d binding failure\n",
+			pr_err("Unexpected pasid %d binding failure\n",
 					p->pasid);
 			mutex_unlock(&p->mutex);
 			break;
-- 
2.7.4

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

Re: [PATCH 1/8] drm/amdkfd: Clean up the data structure in kfd_process

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> From: Yong Zhao <yong.zhao@amd.com>
>
> A list of per-process queues is maintained in the
> kfd_process_queue_manager, so the queues array in kfd_process is
> redundant and in fact unused.
>
> Signed-off-by: Yong Zhao <yong.zhao@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 ------
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 18 ------------------
>  2 files changed, 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index bf29021..3b0cc20 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -527,12 +527,6 @@ struct kfd_process {
>
>         struct process_queue_manager pqm;
>
> -       /* The process's queues. */
> -       size_t queue_array_size;
> -
> -       /* Size is queue_array_size, up to MAX_PROCESS_QUEUES. */
> -       struct kfd_queue **queues;
> -
>         /*Is the user space process 32 bit?*/
>         bool is_32bit_user_mode;
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 21d27e5..f871855 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -35,13 +35,6 @@ struct mm_struct;
>  #include "kfd_dbgmgr.h"
>
>  /*
> - * Initial size for the array of queues.
> - * The allocated size is doubled each time
> - * it is exceeded up to MAX_PROCESS_QUEUES.
> - */
> -#define INITIAL_QUEUE_ARRAY_SIZE 16
> -
> -/*
>   * List of struct kfd_process (field kfd_process).
>   * Unique/indexed by mm_struct*
>   */
> @@ -187,8 +180,6 @@ static void kfd_process_wq_release(struct work_struct *work)
>
>         mutex_destroy(&p->mutex);
>
> -       kfree(p->queues);
> -
>         kfree(p);
>
>         kfree(work);
> @@ -275,11 +266,6 @@ static struct kfd_process *create_process(const struct task_struct *thread)
>         if (!process)
>                 goto err_alloc_process;
>
> -       process->queues = kmalloc_array(INITIAL_QUEUE_ARRAY_SIZE,
> -                                       sizeof(process->queues[0]), GFP_KERNEL);
> -       if (!process->queues)
> -               goto err_alloc_queues;
> -
>         process->pasid = kfd_pasid_alloc();
>         if (process->pasid == 0)
>                 goto err_alloc_pasid;
> @@ -302,8 +288,6 @@ static struct kfd_process *create_process(const struct task_struct *thread)
>
>         process->lead_thread = thread->group_leader;
>
> -       process->queue_array_size = INITIAL_QUEUE_ARRAY_SIZE;
> -
>         INIT_LIST_HEAD(&process->per_device_data);
>
>         kfd_event_init_process(process);
> @@ -332,8 +316,6 @@ static struct kfd_process *create_process(const struct task_struct *thread)
>  err_alloc_doorbells:
>         kfd_pasid_free(process->pasid);
>  err_alloc_pasid:
> -       kfree(process->queues);
> -err_alloc_queues:
>         kfree(process);
>  err_alloc_process:
>         return ERR_PTR(err);
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/8] drm/amdkfd: Disable CP/SDMA ring/doorbell in MQD

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> From: Jay Cornwall <Jay.Cornwall@amd.com>
>
> The MQD represents an inactive context and should not have ring or
> doorbell enable bits set. Doing so interferes with HWS which streams
> the MQD onto the HQD. If enable bits are set this activates the ring
> or doorbell before the HQD is fully configured.
>
> Signed-off-by: Jay Cornwall <Jay.Cornwall@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 34 +++++++-----------------
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c  |  7 ++---
>  2 files changed, 11 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> index 44ffd23..4859d26 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> @@ -189,12 +189,9 @@ static int update_mqd(struct mqd_manager *mm, void *mqd,
>         if (q->format == KFD_QUEUE_FORMAT_AQL)
>                 m->cp_hqd_pq_control |= NO_UPDATE_RPTR;
>
> -       q->is_active = false;
> -       if (q->queue_size > 0 &&
> +       q->is_active = (q->queue_size > 0 &&
>                         q->queue_address != 0 &&
> -                       q->queue_percent > 0) {
> -               q->is_active = true;
> -       }
> +                       q->queue_percent > 0);
>
>         return 0;
>  }
> @@ -215,24 +212,17 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>         m->sdma_rlc_rb_base_hi = upper_32_bits(q->queue_address >> 8);
>         m->sdma_rlc_rb_rptr_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
>         m->sdma_rlc_rb_rptr_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
> -       m->sdma_rlc_doorbell = q->doorbell_off <<
> -                       SDMA0_RLC0_DOORBELL__OFFSET__SHIFT |
> -                       1 << SDMA0_RLC0_DOORBELL__ENABLE__SHIFT;
> +       m->sdma_rlc_doorbell =
> +               q->doorbell_off << SDMA0_RLC0_DOORBELL__OFFSET__SHIFT;
>
>         m->sdma_rlc_virtual_addr = q->sdma_vm_addr;
>
>         m->sdma_engine_id = q->sdma_engine_id;
>         m->sdma_queue_id = q->sdma_queue_id;
>
> -       q->is_active = false;
> -       if (q->queue_size > 0 &&
> +       q->is_active = (q->queue_size > 0 &&
>                         q->queue_address != 0 &&
> -                       q->queue_percent > 0) {
> -               m->sdma_rlc_rb_cntl |=
> -                               1 << SDMA0_RLC0_RB_CNTL__RB_ENABLE__SHIFT;
> -
> -               q->is_active = true;
> -       }
> +                       q->queue_percent > 0);
>
>         return 0;
>  }
> @@ -359,19 +349,13 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
>         m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q->queue_address >> 8);
>         m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
>         m->cp_hqd_pq_rptr_report_addr_hi = upper_32_bits((uint64_t)q->read_ptr);
> -       m->cp_hqd_pq_doorbell_control = DOORBELL_EN |
> -                                       DOORBELL_OFFSET(q->doorbell_off);
> +       m->cp_hqd_pq_doorbell_control = DOORBELL_OFFSET(q->doorbell_off);
>
>         m->cp_hqd_vmid = q->vmid;
>
> -       m->cp_hqd_active = 0;
> -       q->is_active = false;
> -       if (q->queue_size > 0 &&
> +       q->is_active = (q->queue_size > 0 &&
>                         q->queue_address != 0 &&
> -                       q->queue_percent > 0) {
> -               m->cp_hqd_active = 1;
> -               q->is_active = true;
> -       }
> +                       q->queue_percent > 0);
>
>         return 0;
>  }
> 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 73cbfe1..4ea854f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> @@ -163,12 +163,9 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd,
>                                 2 << CP_HQD_PQ_CONTROL__SLOT_BASED_WPTR__SHIFT;
>         }
>
> -       q->is_active = false;
> -       if (q->queue_size > 0 &&
> +       q->is_active = (q->queue_size > 0 &&
>                         q->queue_address != 0 &&
> -                       q->queue_percent > 0) {
> -               q->is_active = true;
> -       }
> +                       q->queue_percent > 0);
>
>         return 0;
>  }
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/8] drm/amdkfd: Avoid calling amd_iommu_unbind_pasid() when suspending

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> From: Yong Zhao <yong.zhao@amd.com>
>
> When kfd suspending on APU, we do not need to call
> amd_iommu_unbind_pasid(), because pasid will be unbound automatically
> when power goes off.
>
> On the other hand, calling amd_iommu_unbind_pasid() will trigger
> kfd_process_iommu_unbind_callback() if the process is not terminating.
> By design, kfd_process_iommu_unbind_callback() should only be called
> for process terminating. So we would rather not to call
> amd_iommu_unbind_pasid() when suspending.
>
> Signed-off-by: Yong Zhao <yong.zhao@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index f871855..5d93a5c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -428,29 +428,25 @@ int kfd_bind_processes_to_device(struct kfd_dev *dev)
>  }
>
>  /*
> - * Temporarily unbind currently bound processes from the device and
> - * mark them as PDD_BOUND_SUSPENDED. These processes will be restored
> - * to PDD_BOUND state in kfd_bind_processes_to_device.
> + * Mark currently bound processes as PDD_BOUND_SUSPENDED. These
> + * processes will be restored to PDD_BOUND state in
> + * kfd_bind_processes_to_device.
>   */
>  void kfd_unbind_processes_from_device(struct kfd_dev *dev)
>  {
>         struct kfd_process_device *pdd;
>         struct kfd_process *p;
> -       unsigned int temp, temp_bound, temp_pasid;
> +       unsigned int temp;
>
>         int idx = srcu_read_lock(&kfd_processes_srcu);
>
>         hash_for_each_rcu(kfd_processes_table, temp, p, kfd_processes) {
>                 mutex_lock(&p->mutex);
>                 pdd = kfd_get_process_device_data(dev, p);
> -               temp_bound = pdd->bound;
> -               temp_pasid = p->pasid;
> +
>                 if (pdd->bound == PDD_BOUND)
>                         pdd->bound = PDD_BOUND_SUSPENDED;
>                 mutex_unlock(&p->mutex);
> -
> -               if (temp_bound == PDD_BOUND)
> -                       amd_iommu_unbind_pasid(dev->pdev, temp_pasid);
>         }
>
>         srcu_read_unlock(&kfd_processes_srcu, idx);
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/8] drm/amdkfd: Fix debug unregister procedure on process termination

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> From: Yair Shachar <yair.shachar@amd.com>
>
> Take the dbgmgr lock and unregister before destroying the debug manager.
> Do this before destroying the queues.
>
> Signed-off-by: Yair Shachar <yair.shachar@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 37 +++++++++++++++++++++++---------
>  1 file changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 5d93a5c..6caf6df 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -229,17 +229,26 @@ static void kfd_process_notifier_release(struct mmu_notifier *mn,
>
>         mutex_lock(&p->mutex);
>
> +       /* Iterate over all process device data structures and if the
> +        * pdd is in debug mode, we should first force unregistration,
> +        * then we will be able to destroy the queues
> +        */
> +       list_for_each_entry(pdd, &p->per_device_data, per_device_list) {
> +               struct kfd_dev *dev = pdd->dev;
> +
> +               mutex_lock(kfd_get_dbgmgr_mutex());
> +               if (dev && dev->dbgmgr && dev->dbgmgr->pasid == p->pasid) {
> +                       if (!kfd_dbgmgr_unregister(dev->dbgmgr, p)) {
> +                               kfd_dbgmgr_destroy(dev->dbgmgr);
> +                               dev->dbgmgr = NULL;
> +                       }
> +               }
> +               mutex_unlock(kfd_get_dbgmgr_mutex());
> +       }
> +
>         kfd_process_dequeue_from_all_devices(p);
>         pqm_uninit(&p->pqm);
>
> -       /* Iterate over all process device data structure and check
> -        * if we should delete debug managers
> -        */
> -       list_for_each_entry(pdd, &p->per_device_data, per_device_list)
> -               if ((pdd->dev->dbgmgr) &&
> -                               (pdd->dev->dbgmgr->pasid == p->pasid))
> -                       kfd_dbgmgr_destroy(pdd->dev->dbgmgr);
> -
>         mutex_unlock(&p->mutex);
>
>         /*
> @@ -468,8 +477,16 @@ void kfd_process_iommu_unbind_callback(struct kfd_dev *dev, unsigned int pasid)
>
>         pr_debug("Unbinding process %d from IOMMU\n", pasid);
>
> -       if ((dev->dbgmgr) && (dev->dbgmgr->pasid == p->pasid))
> -               kfd_dbgmgr_destroy(dev->dbgmgr);
> +       mutex_lock(kfd_get_dbgmgr_mutex());
> +
> +       if (dev->dbgmgr && dev->dbgmgr->pasid == p->pasid) {
> +               if (!kfd_dbgmgr_unregister(dev->dbgmgr, p)) {
> +                       kfd_dbgmgr_destroy(dev->dbgmgr);
> +                       dev->dbgmgr = NULL;
> +               }
> +       }
> +
> +       mutex_unlock(kfd_get_dbgmgr_mutex());
>
>         pdd = kfd_get_process_device_data(dev, p);
>         if (pdd)
> --
> 2.7.4
>
The patch is fine but to prevent theoretical deadlocks, I suggest to
add to this patch a change in the order of locks at
kfd_ioctl_dbg_register(), so that p->mutex is taken *before* dbgmgr
mutex

With that change, this patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/8] drm/amdkfd: Register/Deregister process on qpd resolution

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> From: Ben Goz <ben.goz@amd.com>
>
> Process registration needs to happen on each device. So use per-device
> queue lists to determine when to register/deregister the process.
>
> Signed-off-by: Ben Goz <ben.goz@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> index 5129dc1..2bec902 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
> @@ -177,7 +177,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>         if (retval != 0)
>                 return retval;
>
> -       if (list_empty(&pqm->queues)) {
> +       if (list_empty(&pdd->qpd.queues_list) &&
> +           list_empty(&pdd->qpd.priv_queue_list)) {
>                 pdd->qpd.pqm = pqm;
>                 dev->dqm->ops.register_process(dev->dqm, &pdd->qpd);
>         }
> @@ -248,7 +249,8 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>  err_allocate_pqn:
>         /* check if queues list is empty unregister process from device */
>         clear_bit(*qid, pqm->queue_slot_bitmap);
> -       if (list_empty(&pqm->queues))
> +       if (list_empty(&pdd->qpd.queues_list) &&
> +           list_empty(&pdd->qpd.priv_queue_list))
>                 dev->dqm->ops.unregister_process(dev->dqm, &pdd->qpd);
>         return retval;
>  }
> @@ -302,7 +304,8 @@ int pqm_destroy_queue(struct process_queue_manager *pqm, unsigned int qid)
>         kfree(pqn);
>         clear_bit(qid, pqm->queue_slot_bitmap);
>
> -       if (list_empty(&pqm->queues))
> +       if (list_empty(&pdd->qpd.queues_list) &&
> +           list_empty(&pdd->qpd.priv_queue_list))
>                 dqm->ops.unregister_process(dqm, &pdd->qpd);
>
>         return retval;
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 6/8] drm/amdkfd: Cleanup DQM ASIC-specific ops

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> From: Yong Zhao <yong.zhao@amd.com>
>
> Remove empty initialize function.
>
> Rename register_process to update_qpd to avoid confusion with the
> non-ASIC-specific register_process.
>
> Shorten ops_asic_specific to asic_ops.
>
> Signed-off-by: Yong Zhao <yong.zhao@amd.com>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c    | 19 +++++++------------
>  .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.h    | 11 ++++++-----
>  .../drm/amd/amdkfd/kfd_device_queue_manager_cik.c    | 20 +++++++-------------
>  .../gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c | 20 +++++++-------------
>  4 files changed, 27 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index da3b743..45b98dd 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -467,7 +467,7 @@ static int register_process(struct device_queue_manager *dqm,
>         mutex_lock(&dqm->lock);
>         list_add(&n->list, &dqm->queues);
>
> -       retval = dqm->ops_asic_specific.register_process(dqm, qpd);
> +       retval = dqm->asic_ops.update_qpd(dqm, qpd);
>
>         dqm->processes_count++;
>
> @@ -629,7 +629,7 @@ static int create_sdma_queue_nocpsch(struct device_queue_manager *dqm,
>         pr_debug("SDMA queue id: %d\n", q->properties.sdma_queue_id);
>         pr_debug("SDMA engine id: %d\n", q->properties.sdma_engine_id);
>
> -       dqm->ops_asic_specific.init_sdma_vm(dqm, q, qpd);
> +       dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
>         retval = mqd->init_mqd(mqd, &q->mqd, &q->mqd_mem_obj,
>                                 &q->gart_mqd_addr, &q->properties);
>         if (retval)
> @@ -696,8 +696,6 @@ static int set_sched_resources(struct device_queue_manager *dqm)
>
>  static int initialize_cpsch(struct device_queue_manager *dqm)
>  {
> -       int retval;
> -
>         pr_debug("num of pipes: %d\n", get_pipes_per_mec(dqm));
>
>         mutex_init(&dqm->lock);
> @@ -706,11 +704,8 @@ static int initialize_cpsch(struct device_queue_manager *dqm)
>         dqm->sdma_queue_count = 0;
>         dqm->active_runlist = false;
>         dqm->sdma_bitmap = (1 << CIK_SDMA_QUEUES) - 1;
> -       retval = dqm->ops_asic_specific.initialize(dqm);
> -       if (retval)
> -               mutex_destroy(&dqm->lock);
>
> -       return retval;
> +       return 0;
>  }
>
>  static int start_cpsch(struct device_queue_manager *dqm)
> @@ -850,7 +845,7 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
>                 goto out;
>         }
>
> -       dqm->ops_asic_specific.init_sdma_vm(dqm, q, qpd);
> +       dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
>         retval = mqd->init_mqd(mqd, &q->mqd, &q->mqd_mem_obj,
>                                 &q->gart_mqd_addr, &q->properties);
>         if (retval)
> @@ -1095,7 +1090,7 @@ static bool set_cache_memory_policy(struct device_queue_manager *dqm,
>                 qpd->sh_mem_ape1_limit = limit >> 16;
>         }
>
> -       retval = dqm->ops_asic_specific.set_cache_memory_policy(
> +       retval = dqm->asic_ops.set_cache_memory_policy(
>                         dqm,
>                         qpd,
>                         default_policy,
> @@ -1270,11 +1265,11 @@ struct device_queue_manager *device_queue_manager_init(struct kfd_dev *dev)
>
>         switch (dev->device_info->asic_family) {
>         case CHIP_CARRIZO:
> -               device_queue_manager_init_vi(&dqm->ops_asic_specific);
> +               device_queue_manager_init_vi(&dqm->asic_ops);
>                 break;
>
>         case CHIP_KAVERI:
> -               device_queue_manager_init_cik(&dqm->ops_asic_specific);
> +               device_queue_manager_init_cik(&dqm->asic_ops);
>                 break;
>         default:
>                 WARN(1, "Unexpected ASIC family %u",
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> index 31c2b1f..5b77cb6 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
> @@ -128,9 +128,8 @@ struct device_queue_manager_ops {
>  };
>
>  struct device_queue_manager_asic_ops {
> -       int     (*register_process)(struct device_queue_manager *dqm,
> +       int     (*update_qpd)(struct device_queue_manager *dqm,
>                                         struct qcm_process_device *qpd);
> -       int     (*initialize)(struct device_queue_manager *dqm);
>         bool    (*set_cache_memory_policy)(struct device_queue_manager *dqm,
>                                            struct qcm_process_device *qpd,
>                                            enum cache_policy default_policy,
> @@ -156,7 +155,7 @@ struct device_queue_manager_asic_ops {
>
>  struct device_queue_manager {
>         struct device_queue_manager_ops ops;
> -       struct device_queue_manager_asic_ops ops_asic_specific;
> +       struct device_queue_manager_asic_ops asic_ops;
>
>         struct mqd_manager      *mqds[KFD_MQD_TYPE_MAX];
>         struct packet_manager   packets;
> @@ -179,8 +178,10 @@ struct device_queue_manager {
>         bool                    active_runlist;
>  };
>
> -void device_queue_manager_init_cik(struct device_queue_manager_asic_ops *ops);
> -void device_queue_manager_init_vi(struct device_queue_manager_asic_ops *ops);
> +void device_queue_manager_init_cik(
> +               struct device_queue_manager_asic_ops *asic_ops);
> +void device_queue_manager_init_vi(
> +               struct device_queue_manager_asic_ops *asic_ops);
>  void program_sh_mem_settings(struct device_queue_manager *dqm,
>                                         struct qcm_process_device *qpd);
>  unsigned int get_queues_num(struct device_queue_manager *dqm);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c
> index 72c3cba..28e48c9 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_cik.c
> @@ -32,18 +32,17 @@ static bool set_cache_memory_policy_cik(struct device_queue_manager *dqm,
>                                    enum cache_policy alternate_policy,
>                                    void __user *alternate_aperture_base,
>                                    uint64_t alternate_aperture_size);
> -static int register_process_cik(struct device_queue_manager *dqm,
> +static int update_qpd_cik(struct device_queue_manager *dqm,
>                                         struct qcm_process_device *qpd);
> -static int initialize_cpsch_cik(struct device_queue_manager *dqm);
>  static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
>                                 struct qcm_process_device *qpd);
>
> -void device_queue_manager_init_cik(struct device_queue_manager_asic_ops *ops)
> +void device_queue_manager_init_cik(
> +               struct device_queue_manager_asic_ops *asic_ops)
>  {
> -       ops->set_cache_memory_policy = set_cache_memory_policy_cik;
> -       ops->register_process = register_process_cik;
> -       ops->initialize = initialize_cpsch_cik;
> -       ops->init_sdma_vm = init_sdma_vm;
> +       asic_ops->set_cache_memory_policy = set_cache_memory_policy_cik;
> +       asic_ops->update_qpd = update_qpd_cik;
> +       asic_ops->init_sdma_vm = init_sdma_vm;
>  }
>
>  static uint32_t compute_sh_mem_bases_64bit(unsigned int top_address_nybble)
> @@ -99,7 +98,7 @@ static bool set_cache_memory_policy_cik(struct device_queue_manager *dqm,
>         return true;
>  }
>
> -static int register_process_cik(struct device_queue_manager *dqm,
> +static int update_qpd_cik(struct device_queue_manager *dqm,
>                 struct qcm_process_device *qpd)
>  {
>         struct kfd_process_device *pdd;
> @@ -148,8 +147,3 @@ static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
>
>         q->properties.sdma_vm_addr = value;
>  }
> -
> -static int initialize_cpsch_cik(struct device_queue_manager *dqm)
> -{
> -       return 0;
> -}
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c
> index 40e9ddd..2fbce57 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager_vi.c
> @@ -33,18 +33,17 @@ static bool set_cache_memory_policy_vi(struct device_queue_manager *dqm,
>                                    enum cache_policy alternate_policy,
>                                    void __user *alternate_aperture_base,
>                                    uint64_t alternate_aperture_size);
> -static int register_process_vi(struct device_queue_manager *dqm,
> +static int update_qpd_vi(struct device_queue_manager *dqm,
>                                         struct qcm_process_device *qpd);
> -static int initialize_cpsch_vi(struct device_queue_manager *dqm);
>  static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
>                                 struct qcm_process_device *qpd);
>
> -void device_queue_manager_init_vi(struct device_queue_manager_asic_ops *ops)
> +void device_queue_manager_init_vi(
> +               struct device_queue_manager_asic_ops *asic_ops)
>  {
> -       ops->set_cache_memory_policy = set_cache_memory_policy_vi;
> -       ops->register_process = register_process_vi;
> -       ops->initialize = initialize_cpsch_vi;
> -       ops->init_sdma_vm = init_sdma_vm;
> +       asic_ops->set_cache_memory_policy = set_cache_memory_policy_vi;
> +       asic_ops->update_qpd = update_qpd_vi;
> +       asic_ops->init_sdma_vm = init_sdma_vm;
>  }
>
>  static uint32_t compute_sh_mem_bases_64bit(unsigned int top_address_nybble)
> @@ -104,7 +103,7 @@ static bool set_cache_memory_policy_vi(struct device_queue_manager *dqm,
>         return true;
>  }
>
> -static int register_process_vi(struct device_queue_manager *dqm,
> +static int update_qpd_vi(struct device_queue_manager *dqm,
>                                         struct qcm_process_device *qpd)
>  {
>         struct kfd_process_device *pdd;
> @@ -160,8 +159,3 @@ static void init_sdma_vm(struct device_queue_manager *dqm, struct queue *q,
>
>         q->properties.sdma_vm_addr = value;
>  }
> -
> -static int initialize_cpsch_vi(struct device_queue_manager *dqm)
> -{
> -       return 0;
> -}
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 7/8] drm/amdkfd: Update queue_count before mapping queues

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> map_queues_cpsch uses the queue_count to decide whether to upload
> a new runlist. So update the counter before calling it.
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c    | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index 45b98dd..e2fc4c5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -408,6 +408,17 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
>
>         retval = mqd->update_mqd(mqd, q->mqd, &q->properties);
>
> +       /*
> +        * check active state vs. the previous state and modify
> +        * counter accordingly. map_queues_cpsch uses the
> +        * dqm->queue_count to determine whether a new runlist must be
> +        * uploaded.
> +        */
> +       if (q->properties.is_active && !prev_active)
> +               dqm->queue_count++;
> +       else if (!q->properties.is_active && prev_active)
> +               dqm->queue_count--;
> +
>         if (sched_policy != KFD_SCHED_POLICY_NO_HWS)
>                 retval = map_queues_cpsch(dqm);
>         else if (sched_policy == KFD_SCHED_POLICY_NO_HWS &&
> @@ -417,15 +428,6 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
>                 retval = mqd->load_mqd(mqd, q->mqd, q->pipe, q->queue,
>                                        &q->properties, q->process->mm);
>
> -       /*
> -        * check active state vs. the previous state
> -        * and modify counter accordingly
> -        */
> -       if (q->properties.is_active && !prev_active)
> -               dqm->queue_count++;
> -       else if (!q->properties.is_active && prev_active)
> -               dqm->queue_count--;
> -
>  out_unlock:
>         mutex_unlock(&dqm->lock);
>         return retval;
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 8/8] drm/amdkfd: Minor cleanups v2

[Oded Gabbay]

On Thu, Oct 26, 2017 at 1:41 AM, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> These were missed previously when rebasing changes for upstreaming.
>
> v2: Remove redundant sched_policy conditions
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 10 ++++------
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h                 |  2 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c              |  2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index e2fc4c5..e202921 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -389,12 +389,11 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
>         if (sched_policy != KFD_SCHED_POLICY_NO_HWS) {
>                 retval = unmap_queues_cpsch(dqm,
>                                 KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
> -               if (retval != 0) {
> +               if (retval) {
>                         pr_err("unmap queue failed\n");
>                         goto out_unlock;
>                 }
> -       } else if (sched_policy == KFD_SCHED_POLICY_NO_HWS &&
> -                  prev_active &&
> +       } else if (prev_active &&
>                    (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
>                     q->properties.type == KFD_QUEUE_TYPE_SDMA)) {
>                 retval = mqd->destroy_mqd(mqd, q->mqd,
> @@ -421,8 +420,7 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
>
>         if (sched_policy != KFD_SCHED_POLICY_NO_HWS)
>                 retval = map_queues_cpsch(dqm);
> -       else if (sched_policy == KFD_SCHED_POLICY_NO_HWS &&
> -                q->properties.is_active &&
> +       else if (q->properties.is_active &&
>                  (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
>                   q->properties.type == KFD_QUEUE_TYPE_SDMA))
>                 retval = mqd->load_mqd(mqd, q->mqd, q->pipe, q->queue,
> @@ -832,7 +830,7 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
>
>         if (q->properties.type == KFD_QUEUE_TYPE_SDMA) {
>                 retval = allocate_sdma_queue(dqm, &q->sdma_id);
> -               if (retval != 0)
> +               if (retval)
>                         goto out;
>                 q->properties.sdma_queue_id =
>                         q->sdma_id / CIK_SDMA_QUEUES_PER_ENGINE;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 3b0cc20..9e27c16 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -507,6 +507,8 @@ struct kfd_process {
>          * In any process, the thread that started main() is the lead
>          * thread and outlives the rest.
>          * It is here because amd_iommu_bind_pasid wants a task_struct.
> +        * It can also be used for safely getting a reference to the
> +        * mm_struct of the process.
>          */
>         struct task_struct *lead_thread;
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 6caf6df..674b788 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -421,7 +421,7 @@ int kfd_bind_processes_to_device(struct kfd_dev *dev)
>                 err = amd_iommu_bind_pasid(dev->pdev, p->pasid,
>                                 p->lead_thread);
>                 if (err < 0) {
> -                       pr_err("unexpected pasid %d binding failure\n",
> +                       pr_err("Unexpected pasid %d binding failure\n",
>                                         p->pasid);
>                         mutex_unlock(&p->mutex);
>                         break;
> --
> 2.7.4
>
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/8] drm/amdkfd: Avoid calling amd_iommu_unbind_pasid() when suspending

[Felix Kuehling]

From: Yong Zhao <yong.zhao@amd.com>

When kfd suspending on APU, we do not need to call
amd_iommu_unbind_pasid(), because pasid will be unbound automatically
when power goes off.

On the other hand, calling amd_iommu_unbind_pasid() will trigger
kfd_process_iommu_unbind_callback() if the process is not terminating.
By design, kfd_process_iommu_unbind_callback() should only be called
for process terminating. So we would rather not to call
amd_iommu_unbind_pasid() when suspending.

Signed-off-by: Yong Zhao <yong.zhao@amd.com>
Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
Reviewed-by: Oded Gabbay <oded.gabbay@gmail.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 946f4d6..b81ad816 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -423,29 +423,25 @@ int kfd_bind_processes_to_device(struct kfd_dev *dev)
 }
 
 /*
- * Temporarily unbind currently bound processes from the device and
- * mark them as PDD_BOUND_SUSPENDED. These processes will be restored
- * to PDD_BOUND state in kfd_bind_processes_to_device.
+ * Mark currently bound processes as PDD_BOUND_SUSPENDED. These
+ * processes will be restored to PDD_BOUND state in
+ * kfd_bind_processes_to_device.
  */
 void kfd_unbind_processes_from_device(struct kfd_dev *dev)
 {
 	struct kfd_process_device *pdd;
 	struct kfd_process *p;
-	unsigned int temp, temp_bound, temp_pasid;
+	unsigned int temp;
 
 	int idx = srcu_read_lock(&kfd_processes_srcu);
 
 	hash_for_each_rcu(kfd_processes_table, temp, p, kfd_processes) {
 		mutex_lock(&p->mutex);
 		pdd = kfd_get_process_device_data(dev, p);
-		temp_bound = pdd->bound;
-		temp_pasid = p->pasid;
+
 		if (pdd->bound == PDD_BOUND)
 			pdd->bound = PDD_BOUND_SUSPENDED;
 		mutex_unlock(&p->mutex);
-
-		if (temp_bound == PDD_BOUND)
-			amd_iommu_unbind_pasid(dev->pdev, temp_pasid);
 	}
 
 	srcu_read_unlock(&kfd_processes_srcu, idx);
-- 
2.7.4

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