[Public]
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Harish
> Kasiviswanathan
> Sent: Friday, March 14, 2025 12:17 PM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Kasiviswanathan, Harish <Harish.Kasiviswanathan@xxxxxxx>
> Subject: [PATCH] drm/amdkfd: Update return value of config_dequeue_wait_counts
>
> .config_dequeue_wait_counts returns a nop case. Modify return parameter
> to reflect that since the caller also needs to ignore this condition.
>
> Fixes: <98a5af8103f> ("drm/amdkfd: Add pm_config_dequeue_wait_counts API")
>
> Signed-off-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c | 11 ++++++++---
> drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c | 9 ++++++++-
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> index 3f574d82b5fc..47de572741e7 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c
> @@ -436,19 +436,24 @@ int pm_config_dequeue_wait_counts(struct
> packet_manager *pm,
>
> retval = pm->pmf->config_dequeue_wait_counts(pm, buffer,
> cmd, value);
> - if (!retval)
> + if (retval > 0) {
> retval = kq_submit_packet(pm->priv_queue);
> +
> + /* If default value is modified, cache that in dqm->wait_times
> */
> + if (!retval && cmd == KFD_DEQUEUE_WAIT_INIT)
> + update_dqm_wait_times(pm->dqm);
> + }
> else
> kq_rollback_packet(pm->priv_queue);
> }
>
> /* If default value is modified, cache that value in dqm->wait_times */
> - if (!retval && cmd == KFD_DEQUEUE_WAIT_INIT)
> + if (retval > 0 && cmd == KFD_DEQUEUE_WAIT_INIT)
> update_dqm_wait_times(pm->dqm);
Why are we caching this twice?
>
> out:
> mutex_unlock(&pm->lock);
> - return retval;
> + return retval < 0 ? retval : 0;
> }
>
> int pm_send_unmap_queue(struct packet_manager *pm,
> 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 d440df602393..af3a18d81900 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_packet_manager_v9.c
> @@ -310,6 +310,13 @@ static inline void
> pm_build_dequeue_wait_counts_packet_info(struct packet_manage
> reg_data);
> }
>
> +/* pm_config_dequeue_wait_counts_v9: Builds WRITE_DATA packet with
> + * register/value for configuring dequeue wait counts
> + *
> + * @return: -ve for failure, 0 for nop and +ve for success and buffer is
> + * filled in with packet
> + *
> + **/
> static int pm_config_dequeue_wait_counts_v9(struct packet_manager *pm,
> uint32_t *buffer,
> enum kfd_config_dequeue_wait_counts_cmd cmd,
> @@ -377,7 +384,7 @@ static int pm_config_dequeue_wait_counts_v9(struct
> packet_manager *pm,
>
> packet->data = reg_data;
>
> - return 0;
> + return sizeof(struct pm4_mec_write_data_mmio);
The return value handling is getting really complicated here.
Either return a unique error to handle NOP or pull some of the IP checker higher to early return sooner.
We're already overloading v9 with other asics so there's no harm in -> if (WAIT_INIT && <not optimized ip range check>) return 0 in the abtracted wait_counts function that calls this.
You can always set a new dev->has_optimized_wait_count flag in an earlier init if you don't like defining IP checking there.
Jon
> }
>
> static int pm_unmap_queues_v9(struct packet_manager *pm, uint32_t *buffer,
> --
> 2.34.1
