[AMD Official Use Only]
>-----Original Message-----
>From: Kuehling, Felix <Felix.Kuehling@xxxxxxx>
>Sent: Friday, October 22, 2021 12:46 AM
>To: Yu, Lang <Lang.Yu@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
>Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Huang, Ray
><Ray.Huang@xxxxxxx>
>Subject: Re: [PATCH 1/2] drm/amdkfd: Add an optional argument into update
>queue operation
>
>
>Am 2021-10-15 um 4:48 a.m. schrieb Lang Yu:
>> Currently, queue is updated with data stored in queue_properties.
>> And all allocated resource in queue_properties will not be freed until
>> the queue is destroyed.
>>
>> But some properties(e.g., cu mask) bring some memory management
>> headaches(e.g., memory leak) and make code complex. Actually they
>> don't have to persist in queue_properties.
>>
>> So add an argument into update queue to pass such properties and
>> remove them from queue_properties.
>>
>> Signed-off-by: Lang Yu <lang.yu@xxxxxxx>
>> ---
>> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 4 ++--
>> .../drm/amd/amdkfd/kfd_device_queue_manager.h | 2 +-
>> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h | 2 +-
>> .../gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 18 +++++++--------
>> .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c | 8 +++----
>> .../gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c | 8 +++----
>> .../gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c | 22 +++++++++----------
>> .../amd/amdkfd/kfd_process_queue_manager.c | 6 ++---
>> 8 files changed, 35 insertions(+), 35 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 f8fce9d05f50..7f6f4937eedb 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -557,7 +557,7 @@ static int destroy_queue_nocpsch(struct
>device_queue_manager *dqm,
>> return retval;
>> }
>>
>> -static int update_queue(struct device_queue_manager *dqm, struct
>> queue *q)
>> +static int update_queue(struct device_queue_manager *dqm, struct
>> +queue *q, void *args)
>
>Please don't use a void * here. If you don't want to declare the struct
>queue_update_info in this patch, you can just declare it as an abstract
>type:
>
> struct queue_update_info;
>
>You can cast NULL to (struct queue_update_info *) without requiring the
>structure definition.
Got it. Thanks!
Regards,
Lang
>
>Regards,
> Felix
>
>
>> {
>> int retval = 0;
>> struct mqd_manager *mqd_mgr;
>> @@ -605,7 +605,7 @@ static int update_queue(struct device_queue_manager
>*dqm, struct queue *q)
>> }
>> }
>>
>> - mqd_mgr->update_mqd(mqd_mgr, q->mqd, &q->properties);
>> + mqd_mgr->update_mqd(mqd_mgr, q->mqd, &q->properties, args);
>>
>> /*
>> * check active state vs. the previous state and modify 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 c8719682c4da..08cfc2a2fdbb 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.h
>> @@ -93,7 +93,7 @@ struct device_queue_manager_ops {
>> struct queue *q);
>>
>> int (*update_queue)(struct device_queue_manager *dqm,
>> - struct queue *q);
>> + struct queue *q, void *args);
>>
>> int (*register_process)(struct device_queue_manager *dqm,
>> struct qcm_process_device *qpd); diff --
>git
>> a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
>> index 6e6918ccedfd..6ddf93629b8c 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager.h
>> @@ -80,7 +80,7 @@ struct mqd_manager {
>> struct mm_struct *mms);
>>
>> void (*update_mqd)(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q);
>> + struct queue_properties *q, void *args);
>>
>> int (*destroy_mqd)(struct mqd_manager *mm, void *mqd,
>> enum kfd_preempt_type type,
>> 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 064914e1e8d6..8bb2fd4cba41 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
>> @@ -135,7 +135,7 @@ static void init_mqd(struct mqd_manager *mm, void
>**mqd,
>> *mqd = m;
>> if (gart_addr)
>> *gart_addr = addr;
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static void init_mqd_sdma(struct mqd_manager *mm, void **mqd, @@
>> -152,7 +152,7 @@ static void init_mqd_sdma(struct mqd_manager *mm, void
>**mqd,
>> if (gart_addr)
>> *gart_addr = mqd_mem_obj->gpu_addr;
>>
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static void free_mqd(struct mqd_manager *mm, void *mqd, @@ -185,7
>> +185,7 @@ static int load_mqd_sdma(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 queue_properties *q, void *args, unsigned int
>atc_bit)
>> {
>> struct cik_mqd *m;
>>
>> @@ -221,9 +221,9 @@ static void __update_mqd(struct mqd_manager *mm,
>> void *mqd, }
>>
>> static void update_mqd(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> - __update_mqd(mm, mqd, q, 1);
>> + __update_mqd(mm, mqd, q, args, 1);
>> }
>>
>> static uint32_t read_doorbell_id(void *mqd) @@ -234,13 +234,13 @@
>> static uint32_t read_doorbell_id(void *mqd) }
>>
>> static void update_mqd_hawaii(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> - __update_mqd(mm, mqd, q, 0);
>> + __update_mqd(mm, mqd, q, args, 0);
>> }
>>
>> static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct cik_sdma_rlc_registers *m;
>>
>> @@ -318,7 +318,7 @@ static void init_mqd_hiq(struct mqd_manager *mm,
>> void **mqd, }
>>
>> static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct cik_mqd *m;
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
>> index c7fb59ca597f..6d468b6c8d7d 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v10.c
>> @@ -136,7 +136,7 @@ static void init_mqd(struct mqd_manager *mm, void
>**mqd,
>> *mqd = m;
>> if (gart_addr)
>> *gart_addr = addr;
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static int load_mqd(struct mqd_manager *mm, void *mqd, @@ -162,7
>> +162,7 @@ static int hiq_load_mqd_kiq(struct mqd_manager *mm, void
>> *mqd, }
>>
>> static void update_mqd(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct v10_compute_mqd *m;
>>
>> @@ -311,7 +311,7 @@ static void init_mqd_sdma(struct mqd_manager *mm,
>void **mqd,
>> if (gart_addr)
>> *gart_addr = mqd_mem_obj->gpu_addr;
>>
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static int load_mqd_sdma(struct mqd_manager *mm, void *mqd, @@ -326,7
>> +326,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> #define SDMA_RLC_DUMMY_DEFAULT 0xf
>>
>> static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct v10_sdma_mqd *m;
>>
>> 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 7f4e102ff4bd..f6c10b1b5f8b 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v9.c
>> @@ -188,7 +188,7 @@ static void init_mqd(struct mqd_manager *mm, void
>**mqd,
>> *mqd = m;
>> if (gart_addr)
>> *gart_addr = addr;
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static int load_mqd(struct mqd_manager *mm, void *mqd, @@ -212,7
>> +212,7 @@ static int hiq_load_mqd_kiq(struct mqd_manager *mm, void
>> *mqd, }
>>
>> static void update_mqd(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct v9_mqd *m;
>>
>> @@ -366,7 +366,7 @@ static void init_mqd_sdma(struct mqd_manager *mm,
>void **mqd,
>> if (gart_addr)
>> *gart_addr = mqd_mem_obj->gpu_addr;
>>
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static int load_mqd_sdma(struct mqd_manager *mm, void *mqd, @@ -381,7
>> +381,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> #define SDMA_RLC_DUMMY_DEFAULT 0xf
>>
>> static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct v9_sdma_mqd *m;
>>
>> 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 33dbd22d290f..bddd6d8fdf32 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
>> @@ -150,7 +150,7 @@ static void init_mqd(struct mqd_manager *mm, void
>**mqd,
>> *mqd = m;
>> if (gart_addr)
>> *gart_addr = addr;
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static int load_mqd(struct mqd_manager *mm, void *mqd, @@ -167,8
>> +167,8 @@ static int load_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)
>> + struct queue_properties *q, void *args,
>> + unsigned int mtype, unsigned int atc_bit)
>> {
>> struct vi_mqd *m;
>>
>> @@ -238,9 +238,9 @@ static void __update_mqd(struct mqd_manager *mm,
>> void *mqd,
>>
>>
>> static void update_mqd(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> - __update_mqd(mm, mqd, q, MTYPE_CC, 1);
>> + __update_mqd(mm, mqd, q, args, MTYPE_CC, 1);
>> }
>>
>> static uint32_t read_doorbell_id(void *mqd) @@ -251,9 +251,9 @@
>> static uint32_t read_doorbell_id(void *mqd) }
>>
>> static void update_mqd_tonga(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> - __update_mqd(mm, mqd, q, MTYPE_UC, 0);
>> + __update_mqd(mm, mqd, q, args, MTYPE_UC, 0);
>> }
>>
>> static int destroy_mqd(struct mqd_manager *mm, void *mqd, @@ -317,9
>> +317,9 @@ static void init_mqd_hiq(struct mqd_manager *mm, void **mqd,
>> }
>>
>> static void update_mqd_hiq(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> - __update_mqd(mm, mqd, q, MTYPE_UC, 0);
>> + __update_mqd(mm, mqd, q, args, MTYPE_UC, 0);
>> }
>>
>> static void init_mqd_sdma(struct mqd_manager *mm, void **mqd, @@
>> -336,7 +336,7 @@ static void init_mqd_sdma(struct mqd_manager *mm, void
>**mqd,
>> if (gart_addr)
>> *gart_addr = mqd_mem_obj->gpu_addr;
>>
>> - mm->update_mqd(mm, m, q);
>> + mm->update_mqd(mm, m, q, NULL);
>> }
>>
>> static int load_mqd_sdma(struct mqd_manager *mm, void *mqd, @@ -349,7
>> +349,7 @@ static int load_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> }
>>
>> static void update_mqd_sdma(struct mqd_manager *mm, void *mqd,
>> - struct queue_properties *q)
>> + struct queue_properties *q, void *args)
>> {
>> struct vi_sdma_mqd *m;
>>
>> 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 243dd1efcdbf..37529592457d 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> @@ -121,7 +121,7 @@ int pqm_set_gws(struct process_queue_manager *pqm,
>unsigned int qid,
>> pdd->qpd.num_gws = gws ? amdgpu_amdkfd_get_num_gws(dev->kgd) :
>0;
>>
>> return pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
>> - pqn->q);
>> + pqn->q, NULL);
>> }
>>
>> void kfd_process_dequeue_from_all_devices(struct kfd_process *p) @@
>> -429,7 +429,7 @@ int pqm_update_queue(struct process_queue_manager
>*pqm, unsigned int qid,
>> pqn->q->properties.priority = p->priority;
>>
>> retval = pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
>> - pqn->q);
>> + pqn->q, NULL);
>> if (retval != 0)
>> return retval;
>>
>> @@ -457,7 +457,7 @@ int pqm_set_cu_mask(struct process_queue_manager
>*pqm, unsigned int qid,
>> pqn->q->properties.cu_mask = p->cu_mask;
>>
>> retval = pqn->q->device->dqm->ops.update_queue(pqn->q->device->dqm,
>> - pqn->q);
>> + pqn->q, NULL);
>> if (retval != 0)
>> return retval;
>>
