Re: [PATCH v2 2/9] platform/x86/amd/pmf: Fix the custom bios input handling mechanism

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

On Tue, 17 Jun 2025, Shyam Sundar S K wrote:

> Originally, the 'amd_pmf_get_custom_bios_inputs()' function was written
> under the assumption that the BIOS would only send a single pending
> request for the driver to process. However, following OEM enablement, it
> became clear that multiple pending requests for custom BIOS inputs might
> be sent at the same time, a scenario that the current code logic does not
> support when it comes to handling multiple custom BIOS inputs.
> 
> To address this, the code logic needs to be improved to not only manage
> multiple simultaneous custom BIOS inputs but also to ensure it is scalable
> for future additional inputs.
> 
> Cc: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/pmf.h | 12 ++++++------
>  drivers/platform/x86/amd/pmf/spc.c | 15 ++++++---------
>  2 files changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 1a5a8d70c360..696b170255e0 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -623,14 +623,14 @@ enum ta_slider {
>  	TA_MAX,
>  };
>  
> -enum apmf_smartpc_custom_bios_inputs {
> -	APMF_SMARTPC_CUSTOM_BIOS_INPUT1,
> -	APMF_SMARTPC_CUSTOM_BIOS_INPUT2,
> +struct amd_pmf_pb_bitmap {
> +	const char *name;
> +	u32 bit_mask;
>  };
>  
> -enum apmf_preq_smartpc {
> -	NOTIFY_CUSTOM_BIOS_INPUT1 = 5,
> -	NOTIFY_CUSTOM_BIOS_INPUT2,
> +static const struct amd_pmf_pb_bitmap custom_bios_inputs[] __used = {
> +	{"NOTIFY_CUSTOM_BIOS_INPUT1",     BIT(5)},
> +	{"NOTIFY_CUSTOM_BIOS_INPUT2",     BIT(6)},
>  };
>  
>  enum platform_type {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 1d90f9382024..e72c11aba31d 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -101,18 +101,15 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
>  static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>  					   struct ta_pmf_enact_table *in)
>  {
> +	u32 *bios_inputs[] = {&in->ev_info.bios_input1, &in->ev_info.bios_input2};
> +	int i;
> +
>  	if (!pdev->req.pending_req)
>  		return;
>  
> -	switch (pdev->req.pending_req) {
> -	case BIT(NOTIFY_CUSTOM_BIOS_INPUT1):
> -		in->ev_info.bios_input1 = pdev->req.custom_policy[APMF_SMARTPC_CUSTOM_BIOS_INPUT1];
> -		break;
> -	case BIT(NOTIFY_CUSTOM_BIOS_INPUT2):
> -		in->ev_info.bios_input2 = pdev->req.custom_policy[APMF_SMARTPC_CUSTOM_BIOS_INPUT2];
> -		break;
> -	default:
> -		dev_dbg(pdev->dev, "Invalid preq for BIOS input: 0x%x\n", pdev->req.pending_req);
> +	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++) {
> +		if (pdev->req.pending_req & custom_bios_inputs[i].bit_mask)
> +			*bios_inputs[i] = pdev->req.custom_policy[i];

Hi,

After seeing this version, I think there's better way to solve the 
discontinous layout problem.

Could you instead add a helper amd_pmf_set_ta_bios_input() or something 
along those lines. With that, you don't need to add that local array at 
all. I'd also convert bios_input1/2 to array so both are arrays (I 
understand you cannot change the layout and merge the arrays). The 
helper can then pick which of the arrays it should set to based on the 
index.

And briefly mention in the helper's function comment, that we need to do 
this because the layout does not have continuous area for bios inputs.

That seems simpler approach than this local array approach.

-- 
 i.