[PATCH v5 1/2] drm/amd/pm: Add support to check SMT state periodically

[Limonciello, Mario]

On 4/6/2023 07:45, Wenyou Yang wrote:
> Add a timer to poll the SMT state periodically, if the SMT state
> is changed, invoke the interface to notify the PMFW.
> 
> Signed-off-by: Wenyou Yang <WenYou.Yang at amd.com>
> ---
>   drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h |  8 ++++
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c        | 44 +++++++++++++++++++
>   drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h        |  5 +++
>   3 files changed, 57 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> index 09469c750a96..fc571c122e87 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h
> @@ -566,6 +566,9 @@ struct smu_context
>   
>   	struct firmware pptable_firmware;
>   
> +	bool last_smt_active;
> +	struct timer_list smt_timer;
> +
>   	u32 param_reg;
>   	u32 msg_reg;
>   	u32 resp_reg;
> @@ -1354,6 +1357,11 @@ struct pptable_funcs {
>   	 * @init_pptable_microcode: Prepare the pptable microcode to upload via PSP
>   	 */
>   	int (*init_pptable_microcode)(struct smu_context *smu);
> +
> +	/**
> +	 * @set_cpu_smt_enable: Set the CPU SMT status.
> +	 */
> +	int (*set_cpu_smt_enable)(struct smu_context *smu, bool smt_enable);
>   };
>   
>   typedef enum {
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> index 3ecb900e6ecd..b0e0c6664ac3 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.c
> @@ -26,6 +26,7 @@
>   #include "amdgpu_smu.h"
>   #include "smu_cmn.h"
>   #include "soc15_common.h"
> +#include <linux/sched/smt.h>
>   
>   /*
>    * DO NOT use these for err/warn/info/debug messages.
> @@ -1058,3 +1059,46 @@ bool smu_cmn_is_audio_func_enabled(struct amdgpu_device *adev)
>   
>   	return snd_driver_loaded;
>   }
> +
> +#define TIME_INTERVAL	200
> +
> +static int smu_set_cpu_smt_enable(struct smu_context *smu, bool enable)
> +{
> +	int ret = -EINVAL;
> +
> +	if (smu->ppt_funcs && smu->ppt_funcs->set_cpu_smt_enable)
> +		ret = smu->ppt_funcs->set_cpu_smt_enable(smu, enable);
> +
> +	return ret;
> +}
> +
> +static void smu_smt_timer_callback(struct timer_list *timer)
> +{
> +	struct smu_context *smu = container_of(timer,
> +					       struct smu_context, smt_timer);
> +	bool smt_active;
> +
> +	smt_active = sched_smt_active();
> +	if (smt_active != smu->last_smt_active) {
> +		smu->last_smt_active = smt_active;
> +		smu_set_cpu_smt_enable(smu, smt_active);

You're ignoring the return value for smu_set_cpu_smt_enable.  If the 
message failed to send that means smu->last_smt_active will have the 
wrong value and the message will never attempt to send again while in
this SMT state even though the timer triggered again.

I think you should do it like this:

if (!smu_set_cpu_smt_enable(smu, smt_active))
	smu->last_smt_active = smt_active;

> +	}
> +
> +	mod_timer(timer, jiffies + msecs_to_jiffies(TIME_INTERVAL));
> +}
> +
> +void smu_smt_timer_init(struct smu_context *smu)
> +{
> +	struct timer_list *timer = &smu->smt_timer;
> +
> +	smu->last_smt_active = sched_smt_active();
> +
> +	timer_setup(timer, smu_smt_timer_callback, 0);
> +
> +	mod_timer(timer, jiffies + msecs_to_jiffies(TIME_INTERVAL));
> +}
> +
> +void smu_smt_timer_fini(struct smu_context *smu)
> +{
> +	del_timer(&smu->smt_timer);
> +}
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> index d7cd358a53bd..928dd9e30d83 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu_cmn.h
> @@ -127,5 +127,10 @@ static inline void smu_cmn_get_sysfs_buf(char **buf, int *offset)
>   
>   bool smu_cmn_is_audio_func_enabled(struct amdgpu_device *adev);
>   
> +void smu_smt_timer_init(struct smu_context *smu);
> +
>   #endif
> +
> +void smu_smt_timer_fini(struct smu_context *smu);
> +
>   #endif