From: Shyam Sundar S K Shyam-sundar.S-k@amd.com
[ Upstream commit d82e3d2dd0ba019ac6cdd81e47bf4c8ac895cfa0 ]
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.
Co-developed-by: Patil Rajesh Reddy Patil.Reddy@amd.com Signed-off-by: Patil Rajesh Reddy Patil.Reddy@amd.com Tested-by: Yijun Shen Yijun.Shen@Dell.com Signed-off-by: Shyam Sundar S K Shyam-sundar.S-k@amd.com Link: https://patch.msgid.link/20250901110140.2519072-3-Shyam-sundar.S-k@amd.com Reviewed-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
YES
- Bug fixed: The original code assumed only one pending bit would ever be set for custom BIOS inputs, so multiple simultaneous notifications from firmware were mishandled or ignored. This is a real-world OEM- triggered bug that affects policy evaluation and thus system behavior (performance/thermal) for users. - Before: A single-bit switch on `pending_req` handled only exactly one notification and treated others as “invalid.” - After: Iterates over a bitmask and applies all pending custom BIOS inputs, addressing concurrent notifications.
- Scope and risk: Small, localized to AMD PMF Smart PC input plumbing, no UAPI changes, and no architectural rework. It mainly: - Introduces a small static mapping of notification bits to input indices. - Switches to a loop over the bitmask to set multiple inputs. - Renames the TA input fields to an array to make handling scalable.
- Concrete code changes - Introduces a bitmask table and removes rigid enums: - Added per-input bit mapping: `drivers/platform/x86/amd/pmf/pmf.h:660` - `static const struct amd_pmf_pb_bitmap custom_bios_inputs[] __used = { {"NOTIFY_CUSTOM_BIOS_INPUT1", BIT(5)}, {"NOTIFY_CUSTOM_BIOS_INPUT2", BIT(6)}, ... }` - Defines the simple bitmap struct: `drivers/platform/x86/amd/pmf/pmf.h:655` - `struct amd_pmf_pb_bitmap { const char *name; u32 bit_mask; };` - This replaces fixed enum dispatch and makes the logic extensible and correct for multiple bits. - Makes TA inputs scalable but layout-compatible: - Replaces two discrete fields with an array of two: `drivers/platform/x86/amd/pmf/pmf.h:743` - `u32 bios_input_1[2];` - This preserves total size/ordering for the two inputs currently used and enables indexing (scalable, no user-visible ABI). - Correctly handles multiple pending requests: - New helper to set the proper field by index (handles non- contiguous layout): `drivers/platform/x86/amd/pmf/spc.c:121` - `amd_pmf_set_ta_custom_bios_input(in, index, value);` - Iterates all pending bits and applies each matching custom BIOS input: `drivers/platform/x86/amd/pmf/spc.c:150` - Loops over `custom_bios_inputs`, checks `pdev->req.pending_req & bit_mask`, and assigns from `pdev->req.custom_policy[i]`. - Debug dump now iterates all defined custom inputs instead of only two hardcoded fields: `drivers/platform/x86/amd/pmf/spc.c:107`
- Stable backport criteria - Fixes a real bug that affects end users (policy decisions based on multiple BIOS flags). - Small and self-contained to AMD PMF Smart PC path (files: `pmf.h`, `spc.c`). - Minimal regression risk: logic simply adds proper handling for multiple bits; if only one bit is set, behavior remains as before. The field change is internal to the driver/TA IPC and not a kernel ABI. - No architectural overhaul; it’s a straightforward correctness and scalability improvement. - The commit message clearly explains the OEM-found issue; the patch is tested and reviewed.
- Notes - Backport where AMD PMF custom BIOS input handling exists. On branches without that feature, this patch is not applicable. - Later mainline commits add support for more inputs and versions, but this change alone fixes the core bug (multiple simultaneous inputs) without pulling in larger reworks.
drivers/platform/x86/amd/pmf/pmf.h | 15 +++++----- drivers/platform/x86/amd/pmf/spc.c | 48 +++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 19 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h index 45b60238d5277..df1b4a4f9586b 100644 --- a/drivers/platform/x86/amd/pmf/pmf.h +++ b/drivers/platform/x86/amd/pmf/pmf.h @@ -621,14 +621,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 { @@ -686,8 +686,7 @@ struct ta_pmf_condition_info { u32 power_slider; u32 lid_state; bool user_present; - u32 bios_input1; - u32 bios_input2; + u32 bios_input_1[2]; 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 1d90f9382024b..869b4134513f3 100644 --- a/drivers/platform/x86/amd/pmf/spc.c +++ b/drivers/platform/x86/amd/pmf/spc.c @@ -70,8 +70,20 @@ static const char *ta_slider_as_str(unsigned int state) } }
+static u32 amd_pmf_get_ta_custom_bios_inputs(struct ta_pmf_enact_table *in, int index) +{ + switch (index) { + case 0 ... 1: + return in->ev_info.bios_input_1[index]; + default: + return 0; + } +} + void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in) { + int i; + dev_dbg(dev->dev, "==== TA inputs START ====\n"); dev_dbg(dev->dev, "Slider State: %s\n", ta_slider_as_str(in->ev_info.power_slider)); dev_dbg(dev->dev, "Power Source: %s\n", amd_pmf_source_as_str(in->ev_info.power_source)); @@ -90,29 +102,43 @@ void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table * dev_dbg(dev->dev, "Platform type: %s\n", platform_type_as_str(in->ev_info.platform_type)); dev_dbg(dev->dev, "Laptop placement: %s\n", laptop_placement_as_str(in->ev_info.device_state)); - 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); + for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++) + dev_dbg(dev->dev, "Custom BIOS input%d: %u\n", i + 1, + amd_pmf_get_ta_custom_bios_inputs(in, i)); 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
+/* + * This helper function sets the appropriate BIOS input value in the TA enact + * table based on the provided index. We need this approach because the custom + * BIOS input array is not continuous, due to the existing TA structure layout. + */ +static void amd_pmf_set_ta_custom_bios_input(struct ta_pmf_enact_table *in, int index, u32 value) +{ + switch (index) { + case 0 ... 1: + in->ev_info.bios_input_1[index] = value; + break; + default: + return; + } +} + static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev, struct ta_pmf_enact_table *in) { + unsigned 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)) + continue; + amd_pmf_set_ta_custom_bios_input(in, i, pdev->req.custom_policy[i]); }
/* Clear pending requests after handling */