On 2019-06-06 2:25 p.m., Zeng, Oak wrote:
> Introduce a new mqd allocation interface and split the original
> init_mqd function into two functions: allocate_mqd and init_mqd.
> Also renamed uninit_mqd to free_mqd. This is preparation work to
> fix a circular lock dependency.
>
> Change-Id: I26e53ee1abcdd688ad11d35b433da77e3fa1bee7
> Signed-off-by: Oak Zeng <Oak.Zeng@xxxxxxx>
> ---
> .../gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 34 ++++-----
> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 16 ++--
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c | 4 +-
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 18 +++--
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 78 +++++++------------
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 78 +++++++------------
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c | 88 ++++++++--------------
> 7 files changed, 124 insertions(+), 192 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 3c042eb..10d4f4f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -319,11 +319,11 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
> if (retval)
> goto out_deallocate_hqd;
>
> - retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> - &q->gart_mqd_addr, &q->properties);
> - if (retval)
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
> + if (!q->mqd_mem_obj)
> goto out_deallocate_doorbell;
You need to set retval = -ENOMEM here.
> -
> + mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
> + &q->gart_mqd_addr, &q->properties);
> if (q->properties.is_active) {
>
> if (WARN(q->process->mm != current->mm,
> @@ -333,7 +333,7 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
> retval = mqd_mgr->load_mqd(mqd_mgr, q->mqd, q->pipe,
> q->queue, &q->properties, current->mm);
> if (retval)
> - goto out_uninit_mqd;
> + goto out_free_mqd;
> }
>
> list_add(&q->list, &qpd->queues_list);
> @@ -355,8 +355,8 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
> dqm->total_queue_count);
> goto out_unlock;
>
> -out_uninit_mqd:
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> +out_free_mqd:
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> out_deallocate_doorbell:
> deallocate_doorbell(qpd, q);
> out_deallocate_hqd:
> @@ -450,7 +450,7 @@ static int destroy_queue_nocpsch_locked(struct device_queue_manager *dqm,
> if (retval == -ETIME)
> qpd->reset_wavefronts = true;
>
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>
> list_del(&q->list);
> if (list_empty(&qpd->queues_list)) {
> @@ -527,7 +527,7 @@ static int update_queue(struct device_queue_manager *dqm, struct queue *q)
> }
> }
>
> - retval = mqd_mgr->update_mqd(mqd_mgr, q->mqd, &q->properties);
> + mqd_mgr->update_mqd(mqd_mgr, q->mqd, &q->properties);
I think there is a chance now that retval will remain uninitialized in
this function (in the non-HWS case if a queue is inactive). You should
initialized it to 0 at the start of the function.
Check the other functions as well. Check for compiler warnings. Most of
these types of problems should be caught by the compiler.
>
> /*
> * check active state vs. the previous state and modify
> @@ -1160,11 +1160,11 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
> dqm->asic_ops.init_sdma_vm(dqm, q, qpd);
> q->properties.tba_addr = qpd->tba_addr;
> q->properties.tma_addr = qpd->tma_addr;
> - retval = mqd_mgr->init_mqd(mqd_mgr, &q->mqd, &q->mqd_mem_obj,
> - &q->gart_mqd_addr, &q->properties);
> - if (retval)
> + q->mqd_mem_obj = mqd_mgr->allocate_mqd(mqd_mgr->dev, &q->properties);
> + if (!q->mqd_mem_obj)
> goto out_deallocate_doorbell;
retval = -ENOMEM;
> -
> + mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
> + &q->gart_mqd_addr, &q->properties);
> dqm_lock(dqm);
>
> list_add(&q->list, &qpd->queues_list);
> @@ -1373,8 +1373,8 @@ static int destroy_queue_cpsch(struct device_queue_manager *dqm,
>
> dqm_unlock(dqm);
>
> - /* Do uninit_mqd after dqm_unlock(dqm) to avoid circular locking */
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + /* Do free_mqd after dqm_unlock(dqm) to avoid circular locking */
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
>
> return retval;
>
> @@ -1615,14 +1615,14 @@ static int process_termination_cpsch(struct device_queue_manager *dqm,
> kfd_dec_compute_active(dqm->dev);
>
> /* Lastly, free mqd resources.
> - * Do uninit_mqd() after dqm_unlock to avoid circular locking.
> + * Do free_mqd() after dqm_unlock to avoid circular locking.
> */
> list_for_each_entry_safe(q, next, &qpd->queues_list, list) {
> mqd_mgr = dqm->mqd_mgrs[get_mqd_type_from_queue_type(
> q->properties.type)];
> list_del(&q->list);
> qpd->queue_count--;
> - mqd_mgr->uninit_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> + mqd_mgr->free_mqd(mqd_mgr, q->mqd, q->mqd_mem_obj);
> }
>
> return retval;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> index 1cc03b3..229500c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
> @@ -132,13 +132,14 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
> kq->queue->device = dev;
> kq->queue->process = kfd_get_process(current);
>
> - retval = kq->mqd_mgr->init_mqd(kq->mqd_mgr, &kq->queue->mqd,
> - &kq->queue->mqd_mem_obj,
> + kq->queue->mqd_mem_obj = kq->mqd_mgr->allocate_mqd(kq->mqd_mgr->dev,
> + &kq->queue->properties);
> + if (!kq->queue->mqd_mem_obj)
> + goto err_allocate_mqd;
retval = -ENOMEM;
Regards,
Felix
> + kq->mqd_mgr->init_mqd(kq->mqd_mgr, &kq->queue->mqd,
> + kq->queue->mqd_mem_obj,
> &kq->queue->gart_mqd_addr,
> &kq->queue->properties);
> - if (retval != 0)
> - goto err_init_mqd;
> -
> /* assign HIQ to HQD */
> if (type == KFD_QUEUE_TYPE_HIQ) {
> pr_debug("Assigning hiq to hqd\n");
> @@ -164,7 +165,8 @@ static bool initialize(struct kernel_queue *kq, struct kfd_dev *dev,
>
> return true;
> err_alloc_fence:
> -err_init_mqd:
> + kq->mqd_mgr->free_mqd(kq->mqd_mgr, kq->queue->mqd, kq->queue->mqd_mem_obj);
> +err_allocate_mqd:
> uninit_queue(kq->queue);
> err_init_queue:
> kfd_gtt_sa_free(dev, kq->wptr_mem);
> @@ -193,7 +195,7 @@ static void uninitialize(struct kernel_queue *kq)
> else if (kq->queue->properties.type == KFD_QUEUE_TYPE_DIQ)
> kfd_gtt_sa_free(kq->dev, kq->fence_mem_obj);
>
> - kq->mqd_mgr->uninit_mqd(kq->mqd_mgr, kq->queue->mqd,
> + kq->mqd_mgr->free_mqd(kq->mqd_mgr, kq->queue->mqd,
> kq->queue->mqd_mem_obj);
>
> kfd_gtt_sa_free(kq->dev, kq->rptr_mem);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> index cc04b362..d6cf391 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.c
> @@ -45,7 +45,7 @@ int pipe_priority_map[] = {
> KFD_PIPE_PRIORITY_CS_HIGH
> };
>
> -struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev)
> +struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev, struct queue_properties *q)
> {
> struct kfd_mem_obj *mqd_mem_obj = NULL;
>
> @@ -86,7 +86,7 @@ struct kfd_mem_obj *allocate_sdma_mqd(struct kfd_dev *dev,
> return mqd_mem_obj;
> }
>
> -void uninit_mqd_hiq_sdma(struct mqd_manager *mm, void *mqd,
> +void free_mqd_hiq_sdma(struct mqd_manager *mm, void *mqd,
> struct kfd_mem_obj *mqd_mem_obj)
> {
> WARN_ON(!mqd_mem_obj->gtt_mem);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> index 66b8c67..550b61e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
> @@ -39,7 +39,7 @@
> * @destroy_mqd: Destroys the HQD slot and by that preempt the relevant queue.
> * Used only for no cp scheduling.
> *
> - * @uninit_mqd: Releases the mqd buffer from local gpu memory.
> + * @free_mqd: Releases the mqd buffer from local gpu memory.
> *
> * @is_occupied: Checks if the relevant HQD slot is occupied.
> *
> @@ -64,8 +64,11 @@
> */
> extern int pipe_priority_map[];
> struct mqd_manager {
> - int (*init_mqd)(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> + struct kfd_mem_obj* (*allocate_mqd)(struct kfd_dev *kfd,
> + struct queue_properties *q);
> +
> + void (*init_mqd)(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q);
>
> int (*load_mqd)(struct mqd_manager *mm, void *mqd,
> @@ -73,7 +76,7 @@ struct mqd_manager {
> struct queue_properties *p,
> struct mm_struct *mms);
>
> - int (*update_mqd)(struct mqd_manager *mm, void *mqd,
> + void (*update_mqd)(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q);
>
> int (*destroy_mqd)(struct mqd_manager *mm, void *mqd,
> @@ -81,7 +84,7 @@ struct mqd_manager {
> unsigned int timeout, uint32_t pipe_id,
> uint32_t queue_id);
>
> - void (*uninit_mqd)(struct mqd_manager *mm, void *mqd,
> + void (*free_mqd)(struct mqd_manager *mm, void *mqd,
> struct kfd_mem_obj *mqd_mem_obj);
>
> bool (*is_occupied)(struct mqd_manager *mm, void *mqd,
> @@ -102,11 +105,12 @@ struct mqd_manager {
> uint32_t mqd_size;
> };
>
> -struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev);
> +struct kfd_mem_obj *allocate_hiq_mqd(struct kfd_dev *dev,
> + struct queue_properties *q);
>
> struct kfd_mem_obj *allocate_sdma_mqd(struct kfd_dev *dev,
> struct queue_properties *q);
> -void uninit_mqd_hiq_sdma(struct mqd_manager *mm, void *mqd,
> +void free_mqd_hiq_sdma(struct mqd_manager *mm, void *mqd,
> struct kfd_mem_obj *mqd_mem_obj);
>
> void mqd_symmetrically_map_cu_mask(struct mqd_manager *mm,
> 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 e911438..28876ac 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> @@ -77,9 +77,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
> {
> struct kfd_mem_obj *mqd_mem_obj;
>
> - if (q->type == KFD_QUEUE_TYPE_HIQ)
> - return allocate_hiq_mqd(kfd);
> -
> if (kfd_gtt_sa_allocate(kfd, sizeof(struct cik_mqd),
> &mqd_mem_obj))
> return NULL;
> @@ -87,21 +84,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
> return mqd_mem_obj;
> }
>
> -static int init_mqd(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> uint64_t addr;
> struct cik_mqd *m;
> - int retval;
> - struct kfd_dev *kfd = mm->dev;
> -
> - *mqd_mem_obj = allocate_mqd(kfd, q);
> - if (!*mqd_mem_obj)
> - return -ENOMEM;
>
> - m = (struct cik_mqd *) (*mqd_mem_obj)->cpu_ptr;
> - addr = (*mqd_mem_obj)->gpu_addr;
> + m = (struct cik_mqd *) mqd_mem_obj->cpu_ptr;
> + addr = mqd_mem_obj->gpu_addr;
>
> memset(m, 0, ALIGN(sizeof(struct cik_mqd), 256));
>
> @@ -144,37 +135,27 @@ static int init_mqd(struct mqd_manager *mm, void **mqd,
> *mqd = m;
> if (gart_addr)
> *gart_addr = addr;
> - retval = mm->update_mqd(mm, m, q);
> -
> - return retval;
> + mm->update_mqd(mm, m, q);
> }
>
> -static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> - int retval;
> struct cik_sdma_rlc_registers *m;
> - struct kfd_dev *dev = mm->dev;
>
> - *mqd_mem_obj = allocate_sdma_mqd(dev, q);
> - if (!*mqd_mem_obj)
> - return -ENOMEM;
> -
> - m = (struct cik_sdma_rlc_registers *) (*mqd_mem_obj)->cpu_ptr;
> + m = (struct cik_sdma_rlc_registers *) mqd_mem_obj->cpu_ptr;
>
> memset(m, 0, sizeof(struct cik_sdma_rlc_registers));
>
> *mqd = m;
> if (gart_addr)
> - *gart_addr = (*mqd_mem_obj)->gpu_addr;
> -
> - retval = mm->update_mqd(mm, m, q);
> + *gart_addr = mqd_mem_obj->gpu_addr;
>
> - return retval;
> + mm->update_mqd(mm, m, q);
> }
>
> -static void uninit_mqd(struct mqd_manager *mm, void *mqd,
> +static void free_mqd(struct mqd_manager *mm, void *mqd,
> struct kfd_mem_obj *mqd_mem_obj)
> {
> kfd_gtt_sa_free(mm->dev, mqd_mem_obj);
> @@ -203,7 +184,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> mms);
> }
>
> -static int __update_mqd(struct mqd_manager *mm, void *mqd,
> +static void __update_mqd(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q, unsigned int atc_bit)
> {
> struct cik_mqd *m;
> @@ -237,23 +218,21 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd,
> set_priority(m, q);
>
> q->is_active = QUEUE_IS_ACTIVE(*q);
> -
> - return 0;
> }
>
> -static int update_mqd(struct mqd_manager *mm, void *mqd,
> +static void update_mqd(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> - return __update_mqd(mm, mqd, q, 1);
> + __update_mqd(mm, mqd, q, 1);
> }
>
> -static int update_mqd_hawaii(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_hawaii(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> - return __update_mqd(mm, mqd, q, 0);
> + __update_mqd(mm, mqd, q, 0);
> }
>
> -static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct cik_sdma_rlc_registers *m;
> @@ -278,8 +257,6 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> m->sdma_queue_id = q->sdma_queue_id;
>
> q->is_active = QUEUE_IS_ACTIVE(*q);
> -
> - return 0;
> }
>
> static int destroy_mqd(struct mqd_manager *mm, void *mqd,
> @@ -326,14 +303,14 @@ static bool is_occupied_sdma(struct mqd_manager *mm, void *mqd,
> * queues but with different initial values.
> */
>
> -static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> - return init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
> + init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
> }
>
> -static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct cik_mqd *m;
> @@ -360,7 +337,6 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> q->is_active = QUEUE_IS_ACTIVE(*q);
>
> set_priority(m, q);
> - return 0;
> }
>
> #if defined(CONFIG_DEBUG_FS)
> @@ -399,8 +375,9 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
> switch (type) {
> case KFD_MQD_TYPE_CP:
> case KFD_MQD_TYPE_COMPUTE:
> + mqd->allocate_mqd = allocate_mqd;
> mqd->init_mqd = init_mqd;
> - mqd->uninit_mqd = uninit_mqd;
> + mqd->free_mqd = free_mqd;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd;
> mqd->destroy_mqd = destroy_mqd;
> @@ -411,8 +388,9 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_HIQ:
> + mqd->allocate_mqd = allocate_hiq_mqd;
> mqd->init_mqd = init_mqd_hiq;
> - mqd->uninit_mqd = uninit_mqd_hiq_sdma;
> + mqd->free_mqd = free_mqd_hiq_sdma;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd_hiq;
> mqd->destroy_mqd = destroy_mqd;
> @@ -423,8 +401,9 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_DIQ:
> + mqd->allocate_mqd = allocate_hiq_mqd;
> mqd->init_mqd = init_mqd_hiq;
> - mqd->uninit_mqd = uninit_mqd;
> + mqd->free_mqd = free_mqd;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd_hiq;
> mqd->destroy_mqd = destroy_mqd;
> @@ -435,8 +414,9 @@ struct mqd_manager *mqd_manager_init_cik(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_SDMA:
> + mqd->allocate_mqd = allocate_sdma_mqd;
> mqd->init_mqd = init_mqd_sdma;
> - mqd->uninit_mqd = uninit_mqd_hiq_sdma;
> + mqd->free_mqd = free_mqd_hiq_sdma;
> mqd->load_mqd = load_mqd_sdma;
> mqd->update_mqd = update_mqd_sdma;
> mqd->destroy_mqd = destroy_mqd_sdma;
> 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 818944b..0c58f91 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
> @@ -79,9 +79,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
> int retval;
> struct kfd_mem_obj *mqd_mem_obj = NULL;
>
> - if (q->type == KFD_QUEUE_TYPE_HIQ)
> - return allocate_hiq_mqd(kfd);
> -
> /* From V9, for CWSR, the control stack is located on the next page
> * boundary after the mqd, we will use the gtt allocation function
> * instead of sub-allocation function.
> @@ -110,21 +107,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
>
> }
>
> -static int init_mqd(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> - int retval;
> uint64_t addr;
> struct v9_mqd *m;
> - struct kfd_dev *kfd = mm->dev;
> -
> - *mqd_mem_obj = allocate_mqd(kfd, q);
> - if (!*mqd_mem_obj)
> - return -ENOMEM;
>
> - m = (struct v9_mqd *) (*mqd_mem_obj)->cpu_ptr;
> - addr = (*mqd_mem_obj)->gpu_addr;
> + m = (struct v9_mqd *) mqd_mem_obj->cpu_ptr;
> + addr = mqd_mem_obj->gpu_addr;
>
> memset(m, 0, sizeof(struct v9_mqd));
>
> @@ -173,9 +164,7 @@ static int init_mqd(struct mqd_manager *mm, void **mqd,
> *mqd = m;
> if (gart_addr)
> *gart_addr = addr;
> - retval = mm->update_mqd(mm, m, q);
> -
> - return retval;
> + mm->update_mqd(mm, m, q);
> }
>
> static int load_mqd(struct mqd_manager *mm, void *mqd,
> @@ -190,7 +179,7 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
> wptr_shift, 0, mms);
> }
>
> -static int update_mqd(struct mqd_manager *mm, void *mqd,
> +static void update_mqd(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct v9_mqd *m;
> @@ -252,8 +241,6 @@ static int update_mqd(struct mqd_manager *mm, void *mqd,
> set_priority(m, q);
>
> q->is_active = QUEUE_IS_ACTIVE(*q);
> -
> - return 0;
> }
>
>
> @@ -267,7 +254,7 @@ static int destroy_mqd(struct mqd_manager *mm, void *mqd,
> pipe_id, queue_id);
> }
>
> -static void uninit_mqd(struct mqd_manager *mm, void *mqd,
> +static void free_mqd(struct mqd_manager *mm, void *mqd,
> struct kfd_mem_obj *mqd_mem_obj)
> {
> struct kfd_dev *kfd = mm->dev;
> @@ -311,62 +298,47 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
> return 0;
> }
>
> -static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> struct v9_mqd *m;
> - int retval = init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
>
> - if (retval != 0)
> - return retval;
> + init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
>
> m = get_mqd(*mqd);
>
> m->cp_hqd_pq_control |= 1 << CP_HQD_PQ_CONTROL__PRIV_STATE__SHIFT |
> 1 << CP_HQD_PQ_CONTROL__KMD_QUEUE__SHIFT;
> -
> - return retval;
> }
>
> -static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct v9_mqd *m;
> - int retval = update_mqd(mm, mqd, q);
>
> - if (retval != 0)
> - return retval;
> + update_mqd(mm, mqd, q);
>
> /* TODO: what's the point? update_mqd already does this. */
> m = get_mqd(mqd);
> m->cp_hqd_vmid = q->vmid;
> - return retval;
> }
>
> -static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> - int retval;
> struct v9_sdma_mqd *m;
> - struct kfd_dev *dev = mm->dev;
> -
> - *mqd_mem_obj = allocate_sdma_mqd(dev, q);
> - if (!*mqd_mem_obj)
> - return -ENOMEM;
>
> - m = (struct v9_sdma_mqd *) (*mqd_mem_obj)->cpu_ptr;
> + m = (struct v9_sdma_mqd *) mqd_mem_obj->cpu_ptr;
>
> memset(m, 0, sizeof(struct v9_sdma_mqd));
>
> *mqd = m;
> if (gart_addr)
> - *gart_addr = (*mqd_mem_obj)->gpu_addr;
> + *gart_addr = mqd_mem_obj->gpu_addr;
>
> - retval = mm->update_mqd(mm, m, q);
> -
> - return retval;
> + mm->update_mqd(mm, m, q);
> }
>
> static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> @@ -380,7 +352,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>
> #define SDMA_RLC_DUMMY_DEFAULT 0xf
>
> -static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct v9_sdma_mqd *m;
> @@ -404,8 +376,6 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> m->sdmax_rlcx_dummy_reg = SDMA_RLC_DUMMY_DEFAULT;
>
> q->is_active = QUEUE_IS_ACTIVE(*q);
> -
> - return 0;
> }
>
> /*
> @@ -462,8 +432,9 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
> switch (type) {
> case KFD_MQD_TYPE_CP:
> case KFD_MQD_TYPE_COMPUTE:
> + mqd->allocate_mqd = allocate_mqd;
> mqd->init_mqd = init_mqd;
> - mqd->uninit_mqd = uninit_mqd;
> + mqd->free_mqd = free_mqd;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd;
> mqd->destroy_mqd = destroy_mqd;
> @@ -475,8 +446,9 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_HIQ:
> + mqd->allocate_mqd = allocate_hiq_mqd;
> mqd->init_mqd = init_mqd_hiq;
> - mqd->uninit_mqd = uninit_mqd_hiq_sdma;
> + mqd->free_mqd = free_mqd_hiq_sdma;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd_hiq;
> mqd->destroy_mqd = destroy_mqd;
> @@ -487,8 +459,9 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_DIQ:
> + mqd->allocate_mqd = allocate_hiq_mqd;
> mqd->init_mqd = init_mqd_hiq;
> - mqd->uninit_mqd = uninit_mqd;
> + mqd->free_mqd = free_mqd;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd_hiq;
> mqd->destroy_mqd = destroy_mqd;
> @@ -499,8 +472,9 @@ struct mqd_manager *mqd_manager_init_v9(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_SDMA:
> + mqd->allocate_mqd = allocate_sdma_mqd;
> mqd->init_mqd = init_mqd_sdma;
> - mqd->uninit_mqd = uninit_mqd_hiq_sdma;
> + mqd->free_mqd = free_mqd_hiq_sdma;
> mqd->load_mqd = load_mqd_sdma;
> mqd->update_mqd = update_mqd_sdma;
> mqd->destroy_mqd = destroy_mqd_sdma;
> 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 00e6a59..7d144f5 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> @@ -80,9 +80,6 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
> {
> struct kfd_mem_obj *mqd_mem_obj;
>
> - if (q->type == KFD_QUEUE_TYPE_HIQ)
> - return allocate_hiq_mqd(kfd);
> -
> if (kfd_gtt_sa_allocate(kfd, sizeof(struct vi_mqd),
> &mqd_mem_obj))
> return NULL;
> @@ -90,21 +87,15 @@ static struct kfd_mem_obj *allocate_mqd(struct kfd_dev *kfd,
> return mqd_mem_obj;
> }
>
> -static int init_mqd(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> - int retval;
> uint64_t addr;
> struct vi_mqd *m;
> - struct kfd_dev *kfd = mm->dev;
> -
> - *mqd_mem_obj = allocate_mqd(kfd, q);
> - if (!*mqd_mem_obj)
> - return -ENOMEM;
>
> - m = (struct vi_mqd *) (*mqd_mem_obj)->cpu_ptr;
> - addr = (*mqd_mem_obj)->gpu_addr;
> + m = (struct vi_mqd *) mqd_mem_obj->cpu_ptr;
> + addr = mqd_mem_obj->gpu_addr;
>
> memset(m, 0, sizeof(struct vi_mqd));
>
> @@ -159,9 +150,7 @@ static int init_mqd(struct mqd_manager *mm, void **mqd,
> *mqd = m;
> if (gart_addr)
> *gart_addr = addr;
> - retval = mm->update_mqd(mm, m, q);
> -
> - return retval;
> + mm->update_mqd(mm, m, q);
> }
>
> static int load_mqd(struct mqd_manager *mm, void *mqd,
> @@ -177,7 +166,7 @@ static int load_mqd(struct mqd_manager *mm, void *mqd,
> wptr_shift, wptr_mask, mms);
> }
>
> -static int __update_mqd(struct mqd_manager *mm, void *mqd,
> +static void __update_mqd(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q, unsigned int mtype,
> unsigned int atc_bit)
> {
> @@ -245,21 +234,19 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd,
> set_priority(m, q);
>
> q->is_active = QUEUE_IS_ACTIVE(*q);
> -
> - return 0;
> }
>
>
> -static int update_mqd(struct mqd_manager *mm, void *mqd,
> +static void update_mqd(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> - return __update_mqd(mm, mqd, q, MTYPE_CC, 1);
> + __update_mqd(mm, mqd, q, MTYPE_CC, 1);
> }
>
> -static int update_mqd_tonga(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_tonga(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> - return __update_mqd(mm, mqd, q, MTYPE_UC, 0);
> + __update_mqd(mm, mqd, q, MTYPE_UC, 0);
> }
>
> static int destroy_mqd(struct mqd_manager *mm, void *mqd,
> @@ -272,7 +259,7 @@ static int destroy_mqd(struct mqd_manager *mm, void *mqd,
> pipe_id, queue_id);
> }
>
> -static void uninit_mqd(struct mqd_manager *mm, void *mqd,
> +static void free_mqd(struct mqd_manager *mm, void *mqd,
> struct kfd_mem_obj *mqd_mem_obj)
> {
> kfd_gtt_sa_free(mm->dev, mqd_mem_obj);
> @@ -309,61 +296,44 @@ static int get_wave_state(struct mqd_manager *mm, void *mqd,
> return 0;
> }
>
> -static int init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> struct vi_mqd *m;
> - int retval = init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
> -
> - if (retval != 0)
> - return retval;
> + init_mqd(mm, mqd, mqd_mem_obj, gart_addr, q);
>
> m = get_mqd(*mqd);
>
> m->cp_hqd_pq_control |= 1 << CP_HQD_PQ_CONTROL__PRIV_STATE__SHIFT |
> 1 << CP_HQD_PQ_CONTROL__KMD_QUEUE__SHIFT;
> -
> - return retval;
> }
>
> -static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct vi_mqd *m;
> - int retval = __update_mqd(mm, mqd, q, MTYPE_UC, 0);
> -
> - if (retval != 0)
> - return retval;
> + __update_mqd(mm, mqd, q, MTYPE_UC, 0);
>
> m = get_mqd(mqd);
> m->cp_hqd_vmid = q->vmid;
> - return retval;
> }
>
> -static int init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> - struct kfd_mem_obj **mqd_mem_obj, uint64_t *gart_addr,
> +static void init_mqd_sdma(struct mqd_manager *mm, void **mqd,
> + struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
> struct queue_properties *q)
> {
> - int retval;
> struct vi_sdma_mqd *m;
> - struct kfd_dev *dev = mm->dev;
> -
> - *mqd_mem_obj = allocate_sdma_mqd(dev, q);
> - if (!*mqd_mem_obj)
> - return -ENOMEM;
>
> - m = (struct vi_sdma_mqd *) (*mqd_mem_obj)->cpu_ptr;
> + m = (struct vi_sdma_mqd *) mqd_mem_obj->cpu_ptr;
>
> memset(m, 0, sizeof(struct vi_sdma_mqd));
>
> *mqd = m;
> if (gart_addr)
> - *gart_addr = (*mqd_mem_obj)->gpu_addr;
> + *gart_addr = mqd_mem_obj->gpu_addr;
>
> - retval = mm->update_mqd(mm, m, q);
> -
> - return retval;
> + mm->update_mqd(mm, m, q);
> }
>
> static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> @@ -375,7 +345,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
> mms);
> }
>
> -static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> +static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> struct queue_properties *q)
> {
> struct vi_sdma_mqd *m;
> @@ -400,8 +370,6 @@ static int update_mqd_sdma(struct mqd_manager *mm, void *mqd,
> m->sdma_queue_id = q->sdma_queue_id;
>
> q->is_active = QUEUE_IS_ACTIVE(*q);
> -
> - return 0;
> }
>
> /*
> @@ -458,8 +426,9 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
> switch (type) {
> case KFD_MQD_TYPE_CP:
> case KFD_MQD_TYPE_COMPUTE:
> + mqd->allocate_mqd = allocate_mqd;
> mqd->init_mqd = init_mqd;
> - mqd->uninit_mqd = uninit_mqd;
> + mqd->free_mqd = free_mqd;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd;
> mqd->destroy_mqd = destroy_mqd;
> @@ -471,8 +440,9 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_HIQ:
> + mqd->allocate_mqd = allocate_hiq_mqd;
> mqd->init_mqd = init_mqd_hiq;
> - mqd->uninit_mqd = uninit_mqd_hiq_sdma;
> + mqd->free_mqd = free_mqd_hiq_sdma;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd_hiq;
> mqd->destroy_mqd = destroy_mqd;
> @@ -483,8 +453,9 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_DIQ:
> + mqd->allocate_mqd = allocate_hiq_mqd;
> mqd->init_mqd = init_mqd_hiq;
> - mqd->uninit_mqd = uninit_mqd;
> + mqd->free_mqd = free_mqd;
> mqd->load_mqd = load_mqd;
> mqd->update_mqd = update_mqd_hiq;
> mqd->destroy_mqd = destroy_mqd;
> @@ -495,8 +466,9 @@ struct mqd_manager *mqd_manager_init_vi(enum KFD_MQD_TYPE type,
> #endif
> break;
> case KFD_MQD_TYPE_SDMA:
> + mqd->allocate_mqd = allocate_sdma_mqd;
> mqd->init_mqd = init_mqd_sdma;
> - mqd->uninit_mqd = uninit_mqd_hiq_sdma;
> + mqd->free_mqd = free_mqd_hiq_sdma;
> mqd->load_mqd = load_mqd_sdma;
> mqd->update_mqd = update_mqd_sdma;
> mqd->destroy_mqd = destroy_mqd_sdma;
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
