[PATCH] drm/amdkfd: map sdma queues onto extended engines for navi2x
Felix Kuehling
felix.kuehling at amd.com
Thu Feb 10 00:25:52 UTC 2022
On 2022-02-09 19:18, Kim, Jonathan wrote:
> [AMD Official Use Only]
>
>> -----Original Message-----
>> From: Kuehling, Felix <Felix.Kuehling at amd.com>
>> Sent: February 9, 2022 4:26 PM
>> To: Kim, Jonathan <Jonathan.Kim at amd.com>; amd-gfx at lists.freedesktop.org
>> Subject: Re: [PATCH] drm/amdkfd: map sdma queues onto extended engines for
>> navi2x
>>
>>
>> On 2022-02-09 11:11, Jonathan Kim wrote:
>>> The hardware scheduler requires that all SDMA 5.2.x queues are put on
>>> the RUN_LIST through the extended engines.
>>>
>>> Make extended engine unmap available as well.
>>>
>>> Signed-off-by: Jonathan Kim <jonathan.kim at amd.com>
>>> ---
>>> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 +-
>>> drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c | 5 +++--
>>> drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c | 8 +++++---
>>> drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c | 3 ++-
>>> drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 5 +++--
>>> 5 files changed, 14 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 7f6f1a842b0b..f12e32335eb3 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>>> @@ -1555,7 +1555,7 @@ static int unmap_queues_cpsch(struct
>> device_queue_manager *dqm,
>>> return retval;
>>>
>>> retval = pm_send_unmap_queue(&dqm->packet_mgr,
>> KFD_QUEUE_TYPE_COMPUTE,
>>> - filter, filter_param, reset, 0);
>>> + filter, filter_param, reset, 0, false);
>> Does this still work correctly? We currently rely on HWS unmapping SDMA
>> queues when we request unmapping of compute queues. Is that still the case
>> with extended queue selection in map_queues?
> I wasn't aware of the implicit sdma unmap ...
> That makes much more sense.
>
> I followed up on the FW spec and apparently as long as extended_engine_select=0x1 (sdma0_sdma7),
> a single call to unmap all queues or all dynamic queues will unmap both compute
> queues mapped in legacy mode and sdma queues mapped in extended engine mode.
>
>> How would the caller know to set this to "true"? For mapping, this detail is
>> hidden in the packet-manager implementation. But for unmapping the caller
>> needs to know? That doesn't make sense. But we could probably remove the
>> SDMA filtering functionality from pm_send_unmap_queue completely. I don't
>> see any calls where we try to unmap specific SDMA queues. Since we always
>> have to replace the entire runlist anyway, there is not use case for it.
> Agreed.
> Aside from removing SDMA checks, maybe also pass the device itself through to pm_send_unmap_queue then?
> Or could it be the SDMA ip version?
> That way we can hide the check to toggle between extended_engine_select = 0x0 or 0x1 from the caller.
You can already get that from the struct packet_manager *pm. You can get
the dqm with container_of and get the device with dqm->dev.
Or add a flag for SDMA extended mode in the pm structure itself and set
it in pm_init.
Regards,
Felix
>
> Thanks,
>
> Jon
>
>> Regards,
>> Felix
>>
>>
>>> if (retval)
>>> return retval;
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>> index 1439420925a0..8694cfcd57d1 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
>>> @@ -371,7 +371,8 @@ int pm_send_query_status(struct packet_manager
>> *pm, uint64_t fence_address,
>>> int pm_send_unmap_queue(struct packet_manager *pm, enum
>> kfd_queue_type type,
>>> enum kfd_unmap_queues_filter filter,
>>> uint32_t filter_param, bool reset,
>>> - unsigned int sdma_engine)
>>> + unsigned int sdma_engine,
>>> + bool is_sdma_ext)
>>> {
>>> uint32_t *buffer, size;
>>> int retval = 0;
>>> @@ -387,7 +388,7 @@ int pm_send_unmap_queue(struct packet_manager
>> *pm, enum kfd_queue_type type,
>>> }
>>>
>>> retval = pm->pmf->unmap_queues(pm, buffer, type, filter, filter_param,
>>> - reset, sdma_engine);
>>> + reset, sdma_engine, is_sdma_ext);
>>> if (!retval)
>>> kq_submit_packet(pm->priv_queue);
>>> else
>>> 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 7ea3f671b325..08f736080b7e 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
>>> @@ -183,6 +183,7 @@ static int pm_map_queues_v9(struct packet_manager
>> *pm, uint32_t *buffer,
>>> {
>>> struct pm4_mes_map_queues *packet;
>>> bool use_static = is_static;
>>> + bool is_sdma_ext = q->device->adev->ip_versions[SDMA0_HWIP][0] >=
>>> +IP_VERSION(5, 2, 0);
>>>
>>> packet = (struct pm4_mes_map_queues *)buffer;
>>> memset(buffer, 0, sizeof(struct pm4_mes_map_queues)); @@ -214,7
>>> +215,7 @@ static int pm_map_queues_v9(struct packet_manager *pm,
>> uint32_t *buffer,
>>> case KFD_QUEUE_TYPE_SDMA:
>>> case KFD_QUEUE_TYPE_SDMA_XGMI:
>>> use_static = false; /* no static queues under SDMA */
>>> - if (q->properties.sdma_engine_id < 2)
>>> + if (q->properties.sdma_engine_id < 2 && !is_sdma_ext)
>>> packet->bitfields2.engine_sel = q-
>>> properties.sdma_engine_id +
>>> engine_sel__mes_map_queues__sdma0_vi;
>>> else {
>>> @@ -249,7 +250,8 @@ static int pm_unmap_queues_v9(struct
>> packet_manager *pm, uint32_t *buffer,
>>> enum kfd_queue_type type,
>>> enum kfd_unmap_queues_filter filter,
>>> uint32_t filter_param, bool reset,
>>> - unsigned int sdma_engine)
>>> + unsigned int sdma_engine,
>>> + bool is_sdma_ext)
>>> {
>>> struct pm4_mes_unmap_queues *packet;
>>>
>>> @@ -268,7 +270,7 @@ static int pm_unmap_queues_v9(struct
>> packet_manager *pm, uint32_t *buffer,
>>> break;
>>> case KFD_QUEUE_TYPE_SDMA:
>>> case KFD_QUEUE_TYPE_SDMA_XGMI:
>>> - if (sdma_engine < 2) {
>>> + if (sdma_engine < 2 && !is_sdma_ext) {
>>> packet->bitfields2.extended_engine_sel =
>>>
>> extended_engine_sel__mes_unmap_queues__legacy_engine_sel;
>>> packet->bitfields2.engine_sel =
>>> 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 3c0658e32e93..a83aa94972e7 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_vi.c
>>> @@ -200,7 +200,8 @@ static int pm_unmap_queues_vi(struct
>> packet_manager *pm, uint32_t *buffer,
>>> enum kfd_queue_type type,
>>> enum kfd_unmap_queues_filter filter,
>>> uint32_t filter_param, bool reset,
>>> - unsigned int sdma_engine)
>>> + unsigned int sdma_engine,
>>> + bool is_sdma_ext)
>>> {
>>> struct pm4_mes_unmap_queues *packet;
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> index b6790a637f5c..b157ba0216f0 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>>> @@ -1249,7 +1249,8 @@ struct packet_manager_funcs {
>>> enum kfd_queue_type type,
>>> enum kfd_unmap_queues_filter mode,
>>> uint32_t filter_param, bool reset,
>>> - unsigned int sdma_engine);
>>> + unsigned int sdma_engine,
>>> + bool is_sdma_ext);
>>> int (*query_status)(struct packet_manager *pm, uint32_t *buffer,
>>> uint64_t fence_address, uint64_t fence_value);
>>> int (*release_mem)(uint64_t gpu_addr, uint32_t *buffer); @@ -1279,7
>>> +1280,7 @@ int pm_send_query_status(struct packet_manager *pm, uint64_t
>> fence_address,
>>> int pm_send_unmap_queue(struct packet_manager *pm, enum
>> kfd_queue_type type,
>>> enum kfd_unmap_queues_filter mode,
>>> uint32_t filter_param, bool reset,
>>> - unsigned int sdma_engine);
>>> + unsigned int sdma_engine, bool is_sdma_ext);
>>>
>>> void pm_release_ib(struct packet_manager *pm);
>>>
More information about the amd-gfx
mailing list