Re: [PATCH 01/10] platform/x86/amd/pmf: Add support for adjusting PMF PPT and PPT APU thresholds

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2952 bytes --]

On Fri, 9 May 2025, Shyam Sundar S K wrote:

> The most recent PMF Trusted Application includes enhanced features that
> allow for modifications to PMF thermal parameters such as PPT and PPT APU.
> 
> This update introduces the necessary driver support to utilize these
> capabilities.
> 
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/pmf.h    |  4 ++++
>  drivers/platform/x86/amd/pmf/tee-if.c | 16 ++++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index e6bdee68ccf3..4665e68e5c07 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -93,6 +93,8 @@ struct cookie_header {
>  #define PMF_POLICY_BIOS_OUTPUT_1				10
>  #define PMF_POLICY_BIOS_OUTPUT_2				11
>  #define PMF_POLICY_P3T						38
> +#define PMF_POLICY_PMF_PPT					54
> +#define PMF_POLICY_PMF_PPT_APU_ONLY				55
>  #define PMF_POLICY_BIOS_OUTPUT_3				57
>  #define PMF_POLICY_BIOS_OUTPUT_4				58
>  #define PMF_POLICY_BIOS_OUTPUT_5				59
> @@ -677,6 +679,8 @@ struct pmf_action_table {
>  	u32 stt_skintemp_apu;	/* in C */
>  	u32 stt_skintemp_hs2;	/* in C */
>  	u32 p3t_limit;		/* in mW */
> +	u32 pmf_ppt;		/* in mW */
> +	u32 pmf_ppt_apu_only;	/* in mW */
>  };
>  
>  /* Input conditions */
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index a1e43873a07b..679c5bf5faca 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -145,6 +145,22 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  			}
>  			break;
>  
> +		case PMF_POLICY_PMF_PPT:
> +			if (dev->prev_data->pmf_ppt != val) {
> +				amd_pmf_send_cmd(dev, SET_PMF_PPT, false, val, NULL);
> +				dev_dbg(dev->dev, "update PMF PPT: %u\n", val);
> +				dev->prev_data->pmf_ppt = val;
> +			}
> +			break;
> +
> +		case PMF_POLICY_PMF_PPT_APU_ONLY:
> +			if (dev->prev_data->pmf_ppt_apu_only != val) {
> +				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false, val, NULL);
> +				dev_dbg(dev->dev, "update PMF PPT APU ONLY: %u\n", val);
> +				dev->prev_data->pmf_ppt_apu_only = val;
> +			}
> +			break;
> +

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

...But with the caveat that you should start to consider something more 
clever here as most of these cases are essentially just copy-paste code 
with very minor variations in a single parameters and offset in 
dev->prev_data so it would be nice if the similar cases could be handled 
with once generic code fragment (I don't count dev_dbg variations).

>  		case PMF_POLICY_SYSTEM_STATE:
>  			switch (val) {
>  			case 0:
> 

-- 
 i.