Re: [PATCH] drm/amdkfd: map sdma queues onto extended engines for navi2x

[Felix Kuehling <felix.kuehling@xxxxxxx>]

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@xxxxxxx>
> ---
>   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?

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.

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);
>