On 8/8/2025 5:03 AM, Felix Kuehling wrote:
> On 2025-08-04 7:05, Zhu Lingshan wrote:
>> This commit remove DIQ support because it has been
>> marked as DEPRECATED since 2022 in commit 5bdd3eb2
> I think you can also remove KFD_QUEUE_TYPE_DIQ from enum kfd_queue_type in
> kfd_priv.h.
I was thinking about whether there can be any customer maintained out-of-tree
modules rely on hard code offset in this enum. Maybe I am overthinking because
it is named priv.h, I will remove DIQ in the enmu in the next version.
>
> See two more comments inline ...
>
>> Signed-off-by: Zhu Lingshan <lingshan....@amd.com>
>> ---
>> .../drm/amd/amdkfd/kfd_device_queue_manager.c | 6 +--
>> drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c | 26 ++-----------
>> .../drm/amd/amdkfd/kfd_packet_manager_v9.c | 4 --
>> .../drm/amd/amdkfd/kfd_packet_manager_vi.c | 4 --
>> .../amd/amdkfd/kfd_process_queue_manager.c | 39 +------------------
>> 5 files changed, 7 insertions(+), 72 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 2d91027e2a74..58e47e14cf07 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>> @@ -399,8 +399,7 @@ static void increment_queue_count(struct
>> device_queue_manager *dqm,
>> struct queue *q)
>> {
>> dqm->active_queue_count++;
>> - if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
>> - q->properties.type == KFD_QUEUE_TYPE_DIQ)
>> + if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
>> dqm->active_cp_queue_count++;
>>
>> if (q->properties.is_gws) {
>> @@ -414,8 +413,7 @@ static void decrement_queue_count(struct
>> device_queue_manager *dqm,
>> struct queue *q)
>> {
>> dqm->active_queue_count--;
>> - if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE ||
>> - q->properties.type == KFD_QUEUE_TYPE_DIQ)
>> + if (q->properties.type == KFD_QUEUE_TYPE_COMPUTE)
>> dqm->active_cp_queue_count--;
>>
>> if (q->properties.is_gws) {
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> index ebee37937783..f676b7419984 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
>> @@ -46,7 +46,7 @@ static bool kq_initialize(struct kernel_queue *kq, struct
>> kfd_node *dev,
>> int retval;
>> union PM4_MES_TYPE_3_HEADER nop;
>>
>> - if (WARN_ON(type != KFD_QUEUE_TYPE_DIQ && type != KFD_QUEUE_TYPE_HIQ))
>> + if (WARN_ON(type != KFD_QUEUE_TYPE_HIQ))
>> return false;
> You're returning early here for invalid queue types. That's fine, but see
> below ...
>
>
>>
>> pr_debug("Initializing queue type %d size %d\n", KFD_QUEUE_TYPE_HIQ,
>> @@ -61,14 +61,9 @@ static bool kq_initialize(struct kernel_queue *kq, struct
>> kfd_node *dev,
>>
>> kq->dev = dev;
>> kq->nop_packet = nop.u32all;
>> - switch (type) {
>> - case KFD_QUEUE_TYPE_DIQ:
>> - kq->mqd_mgr = dev->dqm->mqd_mgrs[KFD_MQD_TYPE_DIQ];
>> - break;
>> - case KFD_QUEUE_TYPE_HIQ:
>> + if (type == KFD_QUEUE_TYPE_HIQ)
>> kq->mqd_mgr = dev->dqm->mqd_mgrs[KFD_MQD_TYPE_HIQ];
>> - break;
>> - default:
>> + else {
>> dev_err(dev->adev->dev, "Invalid queue type %d\n", type);
>> return false;
> I think this code is unreachable because you returned above for any invalid
> queue type. You can just remove both the if and the else branch. Just assign
> the kq->mqd_mgr unconditionally.
Yes, will improve.
Thanks
Lingshan
>
> Regards,
> Felix
>
>
>> }
>> @@ -163,24 +158,11 @@ static bool kq_initialize(struct kernel_queue *kq,
>> struct kfd_node *dev,
>> kq->mqd_mgr->load_mqd(kq->mqd_mgr, kq->queue->mqd,
>> kq->queue->pipe, kq->queue->queue,
>> &kq->queue->properties, NULL);
>> - } else {
>> - /* allocate fence for DIQ */
>> -
>> - retval = kfd_gtt_sa_allocate(dev, sizeof(uint32_t),
>> - &kq->fence_mem_obj);
>> -
>> - if (retval != 0)
>> - goto err_alloc_fence;
>> -
>> - kq->fence_kernel_address = kq->fence_mem_obj->cpu_ptr;
>> - kq->fence_gpu_addr = kq->fence_mem_obj->gpu_addr;
>> }
>>
>> print_queue(kq->queue);
>>
>> return true;
>> -err_alloc_fence:
>> - 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:
>> @@ -210,8 +192,6 @@ static void kq_uninitialize(struct kernel_queue *kq)
>> kq->queue->queue);
>> up_read(&kq->dev->adev->reset_domain->sem);
>> }
>> - else if (kq->queue->properties.type == KFD_QUEUE_TYPE_DIQ)
>> - kfd_gtt_sa_free(kq->dev, kq->fence_mem_obj);
>>
>> kq->mqd_mgr->free_mqd(kq->mqd_mgr, kq->queue->mqd,
>> kq->queue->mqd_mem_obj);
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> index 505036968a77..3d2375817c3e 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>> @@ -252,10 +252,6 @@ static int pm_map_queues_v9(struct packet_manager *pm,
>> uint32_t *buffer,
>> packet->bitfields2.queue_type =
>> queue_type__mes_map_queues__normal_latency_static_queue_vi;
>> break;
>> - case KFD_QUEUE_TYPE_DIQ:
>> - packet->bitfields2.queue_type =
>> - queue_type__mes_map_queues__debug_interface_queue_vi;
>> - break;
>> case KFD_QUEUE_TYPE_SDMA:
>> case KFD_QUEUE_TYPE_SDMA_XGMI:
>> if (q->properties.sdma_engine_id < 2 &&
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
>> index a1de5d7e173a..60086e7cc258 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
>> @@ -166,10 +166,6 @@ static int pm_map_queues_vi(struct packet_manager *pm,
>> uint32_t *buffer,
>> packet->bitfields2.queue_type =
>> queue_type__mes_map_queues__normal_latency_static_queue_vi;
>> break;
>> - case KFD_QUEUE_TYPE_DIQ:
>> - packet->bitfields2.queue_type =
>> - queue_type__mes_map_queues__debug_interface_queue_vi;
>> - break;
>> case KFD_QUEUE_TYPE_SDMA:
>> case KFD_QUEUE_TYPE_SDMA_XGMI:
>> packet->bitfields2.engine_sel = q->properties.sdma_engine_id +
>> 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 c643e0ccec52..e36950e7e14f 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process_queue_manager.c
>> @@ -345,7 +345,7 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> * If we are just about to create DIQ, the is_debug flag is not set yet
>> * Hence we also check the type as well
>> */
>> - if ((pdd->qpd.is_debug) || (type == KFD_QUEUE_TYPE_DIQ))
>> + if ((pdd->qpd.is_debug))
>> max_queues = dev->kfd->device_info.max_no_of_hqd/2;
>>
>> if (pdd->qpd.queue_count >= max_queues)
>> @@ -426,22 +426,6 @@ int pqm_create_queue(struct process_queue_manager *pqm,
>> restore_mqd,
>> restore_ctl_stack);
>> print_queue(q);
>> break;
>> - case KFD_QUEUE_TYPE_DIQ:
>> - kq = kernel_queue_init(dev, KFD_QUEUE_TYPE_DIQ);
>> - if (!kq) {
>> - retval = -ENOMEM;
>> - goto err_create_queue;
>> - }
>> - kq->queue->properties.queue_id = *qid;
>> - pqn->kq = kq;
>> - pqn->q = NULL;
>> - retval = kfd_process_drain_interrupts(pdd);
>> - if (retval)
>> - break;
>> -
>> - retval = dev->dqm->ops.create_kernel_queue(dev->dqm,
>> - kq, &pdd->qpd);
>> - break;
>> default:
>> WARN(1, "Invalid queue type %d", type);
>> retval = -EINVAL;
>> @@ -1121,32 +1105,13 @@ int pqm_debugfs_mqds(struct seq_file *m, void *data)
>> break;
>> default:
>> seq_printf(m,
>> - " Bad user queue type %d on device %x\n",
>> + " Qeueu node with bad user queue type %d on
>> device %x\n",
>> q->properties.type, q->device->id);
>> continue;
>> }
>> mqd_mgr = q->device->dqm->mqd_mgrs[mqd_type];
>> size = mqd_mgr->mqd_stride(mqd_mgr,
>> &q->properties);
>> - } else if (pqn->kq) {
>> - q = pqn->kq->queue;
>> - mqd_mgr = pqn->kq->mqd_mgr;
>> - switch (q->properties.type) {
>> - case KFD_QUEUE_TYPE_DIQ:
>> - seq_printf(m, " DIQ on device %x\n",
>> - pqn->kq->dev->id);
>> - break;
>> - default:
>> - seq_printf(m,
>> - " Bad kernel queue type %d on device %x\n",
>> - q->properties.type,
>> - pqn->kq->dev->id);
>> - continue;
>> - }
>> - } else {
>> - seq_printf(m,
>> - " Weird: Queue node with neither kernel nor user queue\n");
>> - continue;
>> }
>>
>> for (xcc = 0; xcc < num_xccs; xcc++) {
