Re: [PATCH] platform/x86/amd/pmf: Enable Custom BIOS Inputs for PMF-TA

[Mario Limonciello <mario.limonciello@amd.com>]

On 12/5/2024 04:19, Shyam Sundar S K wrote:
> Introduce support for passing custom BIOS inputs to the PMF-TA to assess
> BIOS input policy conditions. The PMF driver will adjust system settings
> based on these BIOS input conditions and their corresponding output
> actions.
> 
> 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>

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>

I do have one suggestion for a follow up to consider.

> ---
>   drivers/platform/x86/amd/pmf/acpi.c | 24 ++++++++++++++++++++++++
>   drivers/platform/x86/amd/pmf/pmf.h  | 15 ++++++++++++++-
>   drivers/platform/x86/amd/pmf/spc.c  | 24 ++++++++++++++++++++++++
>   3 files changed, 62 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 1b9c7acf0ddf..21ccd677ffa4 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -321,6 +321,18 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>   									 req, sizeof(*req));
>   }
>   
> +static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
> +{
> +	struct amd_pmf_dev *pmf_dev = data;
> +	int ret;
> +
> +	guard(mutex)(&pmf_dev->cb_mutex);
> +
> +	ret = apmf_get_sbios_requests_v2(pmf_dev, &pmf_dev->req);
> +	if (ret)
> +		dev_err(pmf_dev->dev, "Failed to get v2 SBIOS requests: %d\n", ret);
> +}
> +
>   static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
>   {
>   	struct amd_pmf_dev *pmf_dev = data;
> @@ -430,6 +442,15 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev)
>   		apmf_event_handler(ahandle, 0, pmf_dev);
>   	}
>   
> +	if (pmf_dev->smart_pc_enabled && pmf_dev->pmf_if_version == PMF_IF_V2) {
> +		status = acpi_install_notify_handler(ahandle, ACPI_ALL_NOTIFY,
> +						     apmf_event_handler_v2, pmf_dev);
> +		if (ACPI_FAILURE(status)) {
> +			dev_err(pmf_dev->dev, "failed to install notify handler for custom BIOS inputs\n");
> +			return -ENODEV;
> +		}
> +	}
> +
>   	return 0;
>   }
>   
> @@ -480,6 +501,9 @@ void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev)
>   	if (is_apmf_func_supported(pmf_dev, APMF_FUNC_AUTO_MODE) &&
>   	    is_apmf_func_supported(pmf_dev, APMF_FUNC_SBIOS_REQUESTS))
>   		acpi_remove_notify_handler(ahandle, ACPI_ALL_NOTIFY, apmf_event_handler);
> +
> +	if (pmf_dev->smart_pc_enabled && pmf_dev->pmf_if_version == PMF_IF_V2)
> +		acpi_remove_notify_handler(ahandle, ACPI_ALL_NOTIFY, apmf_event_handler_v2);
>   }
>   
>   int apmf_acpi_init(struct amd_pmf_dev *pmf_dev)
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index a79808fda1d8..255c5ad2300e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -370,6 +370,8 @@ struct amd_pmf_dev {
>   	struct input_dev *pmf_idev;
>   	size_t mtable_size;
>   	struct resource *res;
> +	struct apmf_sbios_req_v2 req; /* To get custom bios pending request */
> +	struct mutex cb_mutex;
>   };
>   
>   struct apmf_sps_prop_granular_v2 {
> @@ -616,6 +618,16 @@ enum ta_slider {
>   	TA_MAX,
>   };
>   
> +enum apmf_smartpc_custom_bios_inputs {
> +	APMF_SMARTPC_CUSTOM_BIOS_INPUT1,
> +	APMF_SMARTPC_CUSTOM_BIOS_INPUT2,
> +};
> +
> +enum apmf_preq_smartpc {
> +	NOTIFY_CUSTOM_BIOS_INPUT1 = 5,
> +	NOTIFY_CUSTOM_BIOS_INPUT2,
> +};

Maybe as a follow up you should combine these all together in a common 
header:

APMF_AMT_NOTIFICATION
APMF_CQL_NOTIFICATION

> +
>   /* Command ids for TA communication */
>   enum ta_pmf_command {
>   	TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
> @@ -657,7 +669,8 @@ struct ta_pmf_condition_info {
>   	u32 power_slider;
>   	u32 lid_state;
>   	bool user_present;
> -	u32 rsvd1[2];
> +	u32 bios_input1;
> +	u32 bios_input2;
>   	u32 monitor_count;
>   	u32 rsvd2[2];
>   	u32 bat_design;
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 06226eb0eab3..05bed8206365 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -47,12 +47,35 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *
>   	dev_dbg(dev->dev, "LID State: %s\n", in->ev_info.lid_state ? "close" : "open");
>   	dev_dbg(dev->dev, "User Presence: %s\n", in->ev_info.user_present ? "Present" : "Away");
>   	dev_dbg(dev->dev, "Ambient Light: %d\n", in->ev_info.ambient_light);
> +	dev_dbg(dev->dev, "Custom BIOS input1: %u\n", in->ev_info.bios_input1);
> +	dev_dbg(dev->dev, "Custom BIOS input2: %u\n", in->ev_info.bios_input2);
>   	dev_dbg(dev->dev, "==== TA inputs END ====\n");
>   }
>   #else
>   void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) {}
>   #endif
>   
> +static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> +					   struct ta_pmf_enact_table *in)
> +{
> +	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);
> +	}
> +
> +	/* Clear pending requests after handling */
> +	memset(&pdev->req, 0, sizeof(pdev->req));
> +}
> +
>   static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
>   {
>   	u16 max, avg = 0;
> @@ -201,4 +224,5 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
>   	amd_pmf_get_battery_info(dev, in);
>   	amd_pmf_get_slider_info(dev, in);
>   	amd_pmf_get_sensor_info(dev, in);
> +	amd_pmf_get_custom_bios_inputs(dev, in);
>   }