[PATCH v2 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size

[PATCH v2 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size

[Shyam Sundar S K]

Define a new macro BIOS_INPUTS_MAX, to represent the maximum number of
BIOS input values. Replace hardcoded array sizes in relevant structures
with this macro to improve readability and maintainability.

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>
---
v2:
 - New patch spinned from v1
 - Add new BIOS_INPUTS_MAX macro and replace hardcoded values

 drivers/platform/x86/amd/pmf/pmf.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index bd19f2a6bc78..2145df4128cd 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -119,6 +119,7 @@ struct cookie_header {
 
 #define APTS_MAX_STATES		16
 #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
+#define BIOS_INPUTS_MAX		10
 
 typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
 
@@ -204,7 +205,7 @@ struct apmf_sbios_req_v1 {
 	u8 skin_temp_apu;
 	u8 skin_temp_hs2;
 	u8 enable_cnqf;
-	u32 custom_policy[10];
+	u32 custom_policy[BIOS_INPUTS_MAX];
 } __packed;
 
 struct apmf_sbios_req_v2 {
@@ -216,7 +217,7 @@ struct apmf_sbios_req_v2 {
 	u32 stt_min_limit;
 	u8 skin_temp_apu;
 	u8 skin_temp_hs2;
-	u32 custom_policy[10];
+	u32 custom_policy[BIOS_INPUTS_MAX];
 } __packed;
 
 struct apmf_fan_idx {
@@ -355,7 +356,7 @@ enum power_modes_v2 {
 };
 
 struct pmf_bios_inputs_prev {
-	u32 custom_bios_inputs[10];
+	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
 };
 
 struct amd_pmf_dev {
@@ -451,7 +452,7 @@ struct os_power_slider {
 struct amd_pmf_notify_smart_pc_update {
 	u16 size;
 	u32 pending_req;
-	u32 custom_bios[10];
+	u32 custom_bios[BIOS_INPUTS_MAX];
 } __packed;
 
 struct fan_table_control {
-- 
2.34.1

[PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Shyam Sundar S K]

Custom BIOS input values can be updated by multiple sources, such as power
mode changes and sensor events, each triggering a custom BIOS input event.
When these events occur in rapid succession, new data may overwrite
previous values before they are processed, resulting in lost updates.

To address this, introduce a fixed-size, power-of-two ring buffer to
capture every custom BIOS input event, storing both the pending request
and its associated input values. Access to the ring buffer is synchronized
using a mutex.

The previous use of memset() to clear the pending request structure after
each event is removed, as each BIOS input value is now copied into the
buffer as a snapshot. Consumers now process entries directly from the ring
buffer, making explicit clearing of the pending request structure
unnecessary.

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>
---
v2:
 - Add dev_WARN_ONCE()
 - Change variable name rb_mutex to cbi_mutex
 - Move tail increment logic above pending request check

 drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
 drivers/platform/x86/amd/pmf/core.c   |  2 ++
 drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
 drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
 drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
 5 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
index 13c4fec2c7ef..870a56f1fe07 100644
--- a/drivers/platform/x86/amd/pmf/acpi.c
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
 									 req, sizeof(*req));
 }
 
+/* Store custom BIOS inputs data in ring buffer */
+static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
+{
+	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
+	struct bios_input_entry entry = { };
+	int i;
+
+	guard(mutex)(&pmf_dev->cbi_mutex);
+
+	switch (pmf_dev->cpu_id) {
+	case AMD_CPU_ID_PS:
+		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
+			entry.val[i] = pmf_dev->req1.custom_policy[i];
+		entry.preq = pmf_dev->req1.pending_req;
+		break;
+	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
+	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
+		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
+			entry.val[i] = pmf_dev->req.custom_policy[i];
+		entry.preq = pmf_dev->req.pending_req;
+		break;
+	default:
+		return;
+	}
+
+	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
+		rb->data[rb->head] = entry;
+		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
+	} else {
+		/* Rare case: ensures the newest BIOS input value is kept */
+		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
+		rb->data[rb->head] = entry;
+		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
+		rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
+	}
+}
+
 static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
 {
 	if (!pdev->cb_flag)
@@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
 	dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
 
 	amd_pmf_handle_early_preq(pmf_dev);
+
+	amd_pmf_custom_bios_inputs_rb(pmf_dev);
 }
 
 static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
@@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
 	dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
 
 	amd_pmf_handle_early_preq(pmf_dev);
+
+	amd_pmf_custom_bios_inputs_rb(pmf_dev);
 }
 
 static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index bc544a4a5266..8b97eba00dd3 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
 	mutex_init(&dev->lock);
 	mutex_init(&dev->update_mutex);
 	mutex_init(&dev->cb_mutex);
+	mutex_init(&dev->cbi_mutex);
 
 	apmf_acpi_init(dev);
 	platform_set_drvdata(pdev, dev);
@@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
 	mutex_destroy(&dev->lock);
 	mutex_destroy(&dev->update_mutex);
 	mutex_destroy(&dev->cb_mutex);
+	mutex_destroy(&dev->cbi_mutex);
 }
 
 static const struct attribute_group *amd_pmf_driver_groups[] = {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 2145df4128cd..6cb1e228c48e 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -12,6 +12,7 @@
 #define PMF_H
 
 #include <linux/acpi.h>
+#include <linux/circ_buf.h>
 #include <linux/input.h>
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
@@ -120,6 +121,7 @@ struct cookie_header {
 #define APTS_MAX_STATES		16
 #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
 #define BIOS_INPUTS_MAX		10
+#define CUSTOM_BIOS_INPUT_RB_SZ	64	/* Must be power of two for CIRC_* macros */
 
 typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
 
@@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
 	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
 };
 
+/**
+ * struct bios_input_entry - Snapshot of custom BIOS input event
+ * @val: Array of custom BIOS input values
+ * @preq: Pending request value associated with this event
+ */
+struct bios_input_entry {
+	u32 val[BIOS_INPUTS_MAX];
+	u32 preq;
+};
+
+struct cbi_ring_buffer {
+	struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
+	int head;
+	int tail;
+};
+
 struct amd_pmf_dev {
 	void __iomem *regbase;
 	void __iomem *smu_virt_addr;
@@ -407,6 +425,8 @@ struct amd_pmf_dev {
 	struct apmf_sbios_req_v1 req1;
 	struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
 	bool cb_flag;			     /* To handle first custom BIOS input */
+	struct cbi_ring_buffer cbi_buf;
+	struct mutex cbi_mutex;		     /* Protects ring buffer access */
 };
 
 struct apmf_sps_prop_granular_v2 {
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 85192c7536b8..669680ce580a 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
 static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
 					   struct ta_pmf_enact_table *in)
 {
+	struct cbi_ring_buffer *rb = &pdev->cbi_buf;
+	struct bios_input_entry entry = { };
 	unsigned int i;
 
+	guard(mutex)(&pdev->cbi_mutex);
+
 	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
 		amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
 
-	if (!(pdev->req.pending_req || pdev->req1.pending_req))
+	if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
+		return;	/* return if ring buffer is empty */
+
+	entry = rb->data[rb->tail];
+	rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
+
+	/* If no active custom BIOS input pending request, do not consume further work */
+	if (!entry.preq)
 		return;
 
 	if (!pdev->smart_pc_enabled)
@@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
 	case PMF_IF_V1:
 		if (!is_apmf_bios_input_notifications_supported(pdev))
 			return;
-		amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
-					   pdev->req1.custom_policy, in);
+		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
 		break;
 	case PMF_IF_V2:
-		amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
-					   pdev->req.custom_policy, in);
+		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
 		break;
 	default:
 		break;
 	}
-
-	/* Clear pending requests after handling */
-	memset(&pdev->req, 0, sizeof(pdev->req));
-	memset(&pdev->req1, 0, sizeof(pdev->req1));
 }
 
 static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 6e8116bef4f6..add742e33e1e 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
 		status = ret == TA_PMF_TYPE_SUCCESS;
 		if (status) {
 			dev->cb_flag = true;
+			dev->cbi_buf.head = 0;
+			dev->cbi_buf.tail = 0;
 			break;
 		}
 		amd_pmf_tee_deinit(dev);
-- 
2.34.1

Re: [PATCH v2 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size

[Mario Limonciello]

On 10/15/25 9:54 AM, Shyam Sundar S K wrote:
> Define a new macro BIOS_INPUTS_MAX, to represent the maximum number of
> BIOS input values. Replace hardcoded array sizes in relevant structures
> with this macro to improve readability and maintainability.
> 
> 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 (AMD) <superm1@kernel.org>

> ---
> v2:
>   - New patch spinned from v1
>   - Add new BIOS_INPUTS_MAX macro and replace hardcoded values
> 
>   drivers/platform/x86/amd/pmf/pmf.h | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index bd19f2a6bc78..2145df4128cd 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -119,6 +119,7 @@ struct cookie_header {
>   
>   #define APTS_MAX_STATES		16
>   #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
> +#define BIOS_INPUTS_MAX		10
>   
>   typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>   
> @@ -204,7 +205,7 @@ struct apmf_sbios_req_v1 {
>   	u8 skin_temp_apu;
>   	u8 skin_temp_hs2;
>   	u8 enable_cnqf;
> -	u32 custom_policy[10];
> +	u32 custom_policy[BIOS_INPUTS_MAX];
>   } __packed;
>   
>   struct apmf_sbios_req_v2 {
> @@ -216,7 +217,7 @@ struct apmf_sbios_req_v2 {
>   	u32 stt_min_limit;
>   	u8 skin_temp_apu;
>   	u8 skin_temp_hs2;
> -	u32 custom_policy[10];
> +	u32 custom_policy[BIOS_INPUTS_MAX];
>   } __packed;
>   
>   struct apmf_fan_idx {
> @@ -355,7 +356,7 @@ enum power_modes_v2 {
>   };
>   
>   struct pmf_bios_inputs_prev {
> -	u32 custom_bios_inputs[10];
> +	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>   };
>   
>   struct amd_pmf_dev {
> @@ -451,7 +452,7 @@ struct os_power_slider {
>   struct amd_pmf_notify_smart_pc_update {
>   	u16 size;
>   	u32 pending_req;
> -	u32 custom_bios[10];
> +	u32 custom_bios[BIOS_INPUTS_MAX];
>   } __packed;
>   
>   struct fan_table_control {

Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Mario Limonciello]

On 10/15/25 9:54 AM, Shyam Sundar S K wrote:
> Custom BIOS input values can be updated by multiple sources, such as power
> mode changes and sensor events, each triggering a custom BIOS input event.
> When these events occur in rapid succession, new data may overwrite
> previous values before they are processed, resulting in lost updates.
> 
> To address this, introduce a fixed-size, power-of-two ring buffer to
> capture every custom BIOS input event, storing both the pending request
> and its associated input values. Access to the ring buffer is synchronized
> using a mutex.
> 
> The previous use of memset() to clear the pending request structure after
> each event is removed, as each BIOS input value is now copied into the
> buffer as a snapshot. Consumers now process entries directly from the ring
> buffer, making explicit clearing of the pending request structure
> unnecessary.
> 
> 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 (AMD) <superm1@kernel.org>

> ---
> v2:
>   - Add dev_WARN_ONCE()
>   - Change variable name rb_mutex to cbi_mutex
>   - Move tail increment logic above pending request check
> 
>   drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
>   drivers/platform/x86/amd/pmf/core.c   |  2 ++
>   drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
>   drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
>   drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
>   5 files changed, 79 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..870a56f1fe07 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>   									 req, sizeof(*req));
>   }
>   
> +/* Store custom BIOS inputs data in ring buffer */
> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
> +{
> +	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> +	struct bios_input_entry entry = { };
> +	int i;
> +
> +	guard(mutex)(&pmf_dev->cbi_mutex);
> +
> +	switch (pmf_dev->cpu_id) {
> +	case AMD_CPU_ID_PS:
> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> +			entry.val[i] = pmf_dev->req1.custom_policy[i];
> +		entry.preq = pmf_dev->req1.pending_req;
> +		break;
> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> +			entry.val[i] = pmf_dev->req.custom_policy[i];
> +		entry.preq = pmf_dev->req.pending_req;
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
> +		rb->data[rb->head] = entry;
> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +	} else {
> +		/* Rare case: ensures the newest BIOS input value is kept */
> +		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
> +		rb->data[rb->head] = entry;
> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +		rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +	}
> +}
> +
>   static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
>   {
>   	if (!pdev->cb_flag)
> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
>   	dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
>   
>   	amd_pmf_handle_early_preq(pmf_dev);
> +
> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>   }
>   
>   static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>   	dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
>   
>   	amd_pmf_handle_early_preq(pmf_dev);
> +
> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>   }
>   
>   static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..8b97eba00dd3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>   	mutex_init(&dev->lock);
>   	mutex_init(&dev->update_mutex);
>   	mutex_init(&dev->cb_mutex);
> +	mutex_init(&dev->cbi_mutex);
>   
>   	apmf_acpi_init(dev);
>   	platform_set_drvdata(pdev, dev);
> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
>   	mutex_destroy(&dev->lock);
>   	mutex_destroy(&dev->update_mutex);
>   	mutex_destroy(&dev->cb_mutex);
> +	mutex_destroy(&dev->cbi_mutex);
>   }
>   
>   static const struct attribute_group *amd_pmf_driver_groups[] = {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 2145df4128cd..6cb1e228c48e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
>   #define PMF_H
>   
>   #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
>   #include <linux/input.h>
>   #include <linux/platform_device.h>
>   #include <linux/platform_profile.h>
> @@ -120,6 +121,7 @@ struct cookie_header {
>   #define APTS_MAX_STATES		16
>   #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
>   #define BIOS_INPUTS_MAX		10
> +#define CUSTOM_BIOS_INPUT_RB_SZ	64	/* Must be power of two for CIRC_* macros */
>   
>   typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>   
> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
>   	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>   };
>   
> +/**
> + * struct bios_input_entry - Snapshot of custom BIOS input event
> + * @val: Array of custom BIOS input values
> + * @preq: Pending request value associated with this event
> + */
> +struct bios_input_entry {
> +	u32 val[BIOS_INPUTS_MAX];
> +	u32 preq;
> +};
> +
> +struct cbi_ring_buffer {
> +	struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> +	int head;
> +	int tail;
> +};
> +
>   struct amd_pmf_dev {
>   	void __iomem *regbase;
>   	void __iomem *smu_virt_addr;
> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
>   	struct apmf_sbios_req_v1 req1;
>   	struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
>   	bool cb_flag;			     /* To handle first custom BIOS input */
> +	struct cbi_ring_buffer cbi_buf;
> +	struct mutex cbi_mutex;		     /* Protects ring buffer access */
>   };
>   
>   struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..669680ce580a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
>   static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>   					   struct ta_pmf_enact_table *in)
>   {
> +	struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> +	struct bios_input_entry entry = { };
>   	unsigned int i;
>   
> +	guard(mutex)(&pdev->cbi_mutex);
> +
>   	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>   		amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
>   
> -	if (!(pdev->req.pending_req || pdev->req1.pending_req))
> +	if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> +		return;	/* return if ring buffer is empty */
> +
> +	entry = rb->data[rb->tail];
> +	rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +
> +	/* If no active custom BIOS input pending request, do not consume further work */
> +	if (!entry.preq)
>   		return;
>   
>   	if (!pdev->smart_pc_enabled)
> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>   	case PMF_IF_V1:
>   		if (!is_apmf_bios_input_notifications_supported(pdev))
>   			return;
> -		amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
> -					   pdev->req1.custom_policy, in);
> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
>   		break;
>   	case PMF_IF_V2:
> -		amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
> -					   pdev->req.custom_policy, in);
> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
>   		break;
>   	default:
>   		break;
>   	}
> -
> -	/* Clear pending requests after handling */
> -	memset(&pdev->req, 0, sizeof(pdev->req));
> -	memset(&pdev->req1, 0, sizeof(pdev->req1));
>   }
>   
>   static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..add742e33e1e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>   		status = ret == TA_PMF_TYPE_SUCCESS;
>   		if (status) {
>   			dev->cb_flag = true;
> +			dev->cbi_buf.head = 0;
> +			dev->cbi_buf.tail = 0;
>   			break;
>   		}
>   		amd_pmf_tee_deinit(dev);

RE: [PATCH v2 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size

[Shen, Yijun]

> -----Original Message-----
> From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Sent: Wednesday, October 15, 2025 10:55 PM
> To: hdegoede@redhat.com; ilpo.jarvinen@linux.intel.com
> Cc: platform-driver-x86@vger.kernel.org; Patil Rajesh
> <Patil.Reddy@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Shen, Yijun <Yijun.Shen@dell.com>; Shyam
> Sundar S K <Shyam-sundar.S-k@amd.com>
> Subject: [PATCH v2 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX
> macro to replace hardcoded array size
> 
> 
> [EXTERNAL EMAIL]
> 
> Define a new macro BIOS_INPUTS_MAX, to represent the maximum number
> of BIOS input values. Replace hardcoded array sizes in relevant structures
> with this macro to improve readability and maintainability.
> 
> 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>

Verified this patch on the issued system.

Tested-By: Yijun Shen <Yijun.Shen@Dell.com>

> ---
> v2:
>  - New patch spinned from v1
>  - Add new BIOS_INPUTS_MAX macro and replace hardcoded values
> 
>  drivers/platform/x86/amd/pmf/pmf.h | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index bd19f2a6bc78..2145df4128cd 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -119,6 +119,7 @@ struct cookie_header {
> 
>  #define APTS_MAX_STATES		16
>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
> +#define BIOS_INPUTS_MAX		10
> 
>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void
> *data);
> 
> @@ -204,7 +205,7 @@ struct apmf_sbios_req_v1 {
>  	u8 skin_temp_apu;
>  	u8 skin_temp_hs2;
>  	u8 enable_cnqf;
> -	u32 custom_policy[10];
> +	u32 custom_policy[BIOS_INPUTS_MAX];
>  } __packed;
> 
>  struct apmf_sbios_req_v2 {
> @@ -216,7 +217,7 @@ struct apmf_sbios_req_v2 {
>  	u32 stt_min_limit;
>  	u8 skin_temp_apu;
>  	u8 skin_temp_hs2;
> -	u32 custom_policy[10];
> +	u32 custom_policy[BIOS_INPUTS_MAX];
>  } __packed;
> 
>  struct apmf_fan_idx {
> @@ -355,7 +356,7 @@ enum power_modes_v2 {  };
> 
>  struct pmf_bios_inputs_prev {
> -	u32 custom_bios_inputs[10];
> +	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>  };
> 
>  struct amd_pmf_dev {
> @@ -451,7 +452,7 @@ struct os_power_slider {  struct
> amd_pmf_notify_smart_pc_update {
>  	u16 size;
>  	u32 pending_req;
> -	u32 custom_bios[10];
> +	u32 custom_bios[BIOS_INPUTS_MAX];
>  } __packed;
> 
>  struct fan_table_control {
> --
> 2.34.1

RE: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Shen, Yijun]

> -----Original Message-----
> From: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Sent: Wednesday, October 15, 2025 10:55 PM
> To: hdegoede@redhat.com; ilpo.jarvinen@linux.intel.com
> Cc: platform-driver-x86@vger.kernel.org; Patil Rajesh
> <Patil.Reddy@amd.com>; Limonciello, Mario
> <Mario.Limonciello@amd.com>; Shen, Yijun <Yijun.Shen@dell.com>; Shyam
> Sundar S K <Shyam-sundar.S-k@amd.com>
> Subject: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store
> custom BIOS input values
> 
> 
> [EXTERNAL EMAIL]
> 
> Custom BIOS input values can be updated by multiple sources, such as power
> mode changes and sensor events, each triggering a custom BIOS input event.
> When these events occur in rapid succession, new data may overwrite
> previous values before they are processed, resulting in lost updates.
> 
> To address this, introduce a fixed-size, power-of-two ring buffer to capture
> every custom BIOS input event, storing both the pending request and its
> associated input values. Access to the ring buffer is synchronized using a
> mutex.
> 
> The previous use of memset() to clear the pending request structure after
> each event is removed, as each BIOS input value is now copied into the buffer
> as a snapshot. Consumers now process entries directly from the ring buffer,
> making explicit clearing of the pending request structure unnecessary.
> 
> 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>

Verified this patch on the issued system.

Tested-By: Yijun Shen <Yijun.Shen@Dell.com>

> ---
> v2:
>  - Add dev_WARN_ONCE()
>  - Change variable name rb_mutex to cbi_mutex
>  - Move tail increment logic above pending request check
> 
>  drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/core.c   |  2 ++
>  drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
>  drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
>  drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
>  5 files changed, 79 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c
> b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..870a56f1fe07 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev
> *pdev, struct apmf_sbios_req *req
>  									 req,
> sizeof(*req));
>  }
> 
> +/* Store custom BIOS inputs data in ring buffer */ static void
> +amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev) {
> +	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> +	struct bios_input_entry entry = { };
> +	int i;
> +
> +	guard(mutex)(&pmf_dev->cbi_mutex);
> +
> +	switch (pmf_dev->cpu_id) {
> +	case AMD_CPU_ID_PS:
> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> +			entry.val[i] = pmf_dev->req1.custom_policy[i];
> +		entry.preq = pmf_dev->req1.pending_req;
> +		break;
> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> +			entry.val[i] = pmf_dev->req.custom_policy[i];
> +		entry.preq = pmf_dev->req.pending_req;
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0)
> {
> +		rb->data[rb->head] = entry;
> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ -
> 1);
> +	} else {
> +		/* Rare case: ensures the newest BIOS input value is kept */
> +		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input
> value, data may be lost\n");
> +		rb->data[rb->head] = entry;
> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ -
> 1);
> +		rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +	}
> +}
> +
>  static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)  {
>  	if (!pdev->cb_flag)
> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle
> handle, u32 event, void *data)
>  	dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev-
> >req.pending_req);
> 
>  	amd_pmf_handle_early_preq(pmf_dev);
> +
> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>  }
> 
>  static void apmf_event_handler_v1(acpi_handle handle, u32 event, void
> *data) @@ -374,6 +413,8 @@ static void
> apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>  	dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n",
> pmf_dev->req1.pending_req);
> 
>  	amd_pmf_handle_early_preq(pmf_dev);
> +
> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>  }
> 
>  static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> diff --git a/drivers/platform/x86/amd/pmf/core.c
> b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..8b97eba00dd3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device
> *pdev)
>  	mutex_init(&dev->lock);
>  	mutex_init(&dev->update_mutex);
>  	mutex_init(&dev->cb_mutex);
> +	mutex_init(&dev->cbi_mutex);
> 
>  	apmf_acpi_init(dev);
>  	platform_set_drvdata(pdev, dev);
> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device
> *pdev)
>  	mutex_destroy(&dev->lock);
>  	mutex_destroy(&dev->update_mutex);
>  	mutex_destroy(&dev->cb_mutex);
> +	mutex_destroy(&dev->cbi_mutex);
>  }
> 
>  static const struct attribute_group *amd_pmf_driver_groups[] = { diff --git
> a/drivers/platform/x86/amd/pmf/pmf.h
> b/drivers/platform/x86/amd/pmf/pmf.h
> index 2145df4128cd..6cb1e228c48e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
>  #define PMF_H
> 
>  #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
>  #include <linux/input.h>
>  #include <linux/platform_device.h>
>  #include <linux/platform_profile.h>
> @@ -120,6 +121,7 @@ struct cookie_header {
>  #define APTS_MAX_STATES		16
>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
>  #define BIOS_INPUTS_MAX		10
> +#define CUSTOM_BIOS_INPUT_RB_SZ	64	/* Must be power of two for
> CIRC_* macros */
> 
>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void
> *data);
> 
> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
>  	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>  };
> 
> +/**
> + * struct bios_input_entry - Snapshot of custom BIOS input event
> + * @val: Array of custom BIOS input values
> + * @preq: Pending request value associated with this event  */ struct
> +bios_input_entry {
> +	u32 val[BIOS_INPUTS_MAX];
> +	u32 preq;
> +};
> +
> +struct cbi_ring_buffer {
> +	struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> +	int head;
> +	int tail;
> +};
> +
>  struct amd_pmf_dev {
>  	void __iomem *regbase;
>  	void __iomem *smu_virt_addr;
> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
>  	struct apmf_sbios_req_v1 req1;
>  	struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS
> inputs */
>  	bool cb_flag;			     /* To handle first custom BIOS
> input */
> +	struct cbi_ring_buffer cbi_buf;
> +	struct mutex cbi_mutex;		     /* Protects ring buffer
> access */
>  };
> 
>  struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c
> b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..669680ce580a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct
> amd_pmf_dev *pdev, u32 pending_req  static void
> amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>  					   struct ta_pmf_enact_table *in)
>  {
> +	struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> +	struct bios_input_entry entry = { };
>  	unsigned int i;
> 
> +	guard(mutex)(&pdev->cbi_mutex);
> +
>  	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>  		amd_pmf_set_ta_custom_bios_input(in, i, pdev-
> >cb_prev.custom_bios_inputs[i]);
> 
> -	if (!(pdev->req.pending_req || pdev->req1.pending_req))
> +	if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> +		return;	/* return if ring buffer is empty */
> +
> +	entry = rb->data[rb->tail];
> +	rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +
> +	/* If no active custom BIOS input pending request, do not consume
> further work */
> +	if (!entry.preq)
>  		return;
> 
>  	if (!pdev->smart_pc_enabled)
> @@ -165,20 +176,14 @@ static void
> amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>  	case PMF_IF_V1:
>  		if (!is_apmf_bios_input_notifications_supported(pdev))
>  			return;
> -		amd_pmf_update_bios_inputs(pdev, pdev-
> >req1.pending_req, custom_bios_inputs_v1,
> -					   pdev->req1.custom_policy, in);
> +		amd_pmf_update_bios_inputs(pdev, entry.preq,
> custom_bios_inputs_v1,
> +entry.val, in);
>  		break;
>  	case PMF_IF_V2:
> -		amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req,
> custom_bios_inputs,
> -					   pdev->req.custom_policy, in);
> +		amd_pmf_update_bios_inputs(pdev, entry.preq,
> custom_bios_inputs,
> +entry.val, in);
>  		break;
>  	default:
>  		break;
>  	}
> -
> -	/* Clear pending requests after handling */
> -	memset(&pdev->req, 0, sizeof(pdev->req));
> -	memset(&pdev->req1, 0, sizeof(pdev->req1));
>  }
> 
>  static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct
> ta_pmf_enact_table *in) diff --git a/drivers/platform/x86/amd/pmf/tee-if.c
> b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..add742e33e1e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev
> *dev)
>  		status = ret == TA_PMF_TYPE_SUCCESS;
>  		if (status) {
>  			dev->cb_flag = true;
> +			dev->cbi_buf.head = 0;
> +			dev->cbi_buf.tail = 0;
>  			break;
>  		}
>  		amd_pmf_tee_deinit(dev);
> --
> 2.34.1

Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Ilpo Järvinen]

On Wed, 15 Oct 2025, Shyam Sundar S K wrote:

> Custom BIOS input values can be updated by multiple sources, such as power
> mode changes and sensor events, each triggering a custom BIOS input event.
> When these events occur in rapid succession, new data may overwrite
> previous values before they are processed, resulting in lost updates.
> 
> To address this, introduce a fixed-size, power-of-two ring buffer to
> capture every custom BIOS input event, storing both the pending request
> and its associated input values. Access to the ring buffer is synchronized
> using a mutex.
> 
> The previous use of memset() to clear the pending request structure after
> each event is removed, as each BIOS input value is now copied into the
> buffer as a snapshot. Consumers now process entries directly from the ring
> buffer, making explicit clearing of the pending request structure
> unnecessary.
> 
> 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>
> ---
> v2:
>  - Add dev_WARN_ONCE()
>  - Change variable name rb_mutex to cbi_mutex
>  - Move tail increment logic above pending request check
> 
>  drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/core.c   |  2 ++
>  drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
>  drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
>  drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
>  5 files changed, 79 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..870a56f1fe07 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>  									 req, sizeof(*req));
>  }
>  
> +/* Store custom BIOS inputs data in ring buffer */
> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
> +{
> +	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> +	struct bios_input_entry entry = { };

Too generic struct name.

> +	int i;
> +
> +	guard(mutex)(&pmf_dev->cbi_mutex);
> +
> +	switch (pmf_dev->cpu_id) {
> +	case AMD_CPU_ID_PS:
> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> +			entry.val[i] = pmf_dev->req1.custom_policy[i];
> +		entry.preq = pmf_dev->req1.pending_req;
> +		break;
> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)

Add include.

> +			entry.val[i] = pmf_dev->req.custom_policy[i];
> +		entry.preq = pmf_dev->req.pending_req;
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
> +		rb->data[rb->head] = entry;
> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +	} else {
> +		/* Rare case: ensures the newest BIOS input value is kept */
> +		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");

Is this really WARN_ONCE thing? That is, does it indicate a bug in the 
kernel code?

Add include.

> +		rb->data[rb->head] = entry;
> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +		rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);

Since you're under lock, can you make room separately first and then add
the entry on a common path?

> +	}
> +}
> +
>  static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
>  {
>  	if (!pdev->cb_flag)
> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
>  	dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
>  
>  	amd_pmf_handle_early_preq(pmf_dev);
> +
> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>  }
>  
>  static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>  	dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
>  
>  	amd_pmf_handle_early_preq(pmf_dev);
> +
> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>  }
>  
>  static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..8b97eba00dd3 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>  	mutex_init(&dev->lock);
>  	mutex_init(&dev->update_mutex);
>  	mutex_init(&dev->cb_mutex);
> +	mutex_init(&dev->cbi_mutex);
>  
>  	apmf_acpi_init(dev);
>  	platform_set_drvdata(pdev, dev);
> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
>  	mutex_destroy(&dev->lock);
>  	mutex_destroy(&dev->update_mutex);
>  	mutex_destroy(&dev->cb_mutex);
> +	mutex_destroy(&dev->cbi_mutex);
>  }
>  
>  static const struct attribute_group *amd_pmf_driver_groups[] = {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 2145df4128cd..6cb1e228c48e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
>  #define PMF_H
>  
>  #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
>  #include <linux/input.h>
>  #include <linux/platform_device.h>
>  #include <linux/platform_profile.h>
> @@ -120,6 +121,7 @@ struct cookie_header {
>  #define APTS_MAX_STATES		16
>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
>  #define BIOS_INPUTS_MAX		10
> +#define CUSTOM_BIOS_INPUT_RB_SZ	64	/* Must be power of two for CIRC_* macros */

This is not really size but there are n entries. The total size will be 
something else. I'd call it NUM or ENTRIES, or something like that. 
Perhaps CUSTOM_BIOS_INPUT_RING_ENTRIES but that is a bit to the long side 
of things?

>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>  
> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
>  	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>  };
>  
> +/**
> + * struct bios_input_entry - Snapshot of custom BIOS input event
> + * @val: Array of custom BIOS input values
> + * @preq: Pending request value associated with this event
> + */
> +struct bios_input_entry {
> +	u32 val[BIOS_INPUTS_MAX];
> +	u32 preq;
> +};
> +
> +struct cbi_ring_buffer {
> +	struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> +	int head;
> +	int tail;
> +};
> +
>  struct amd_pmf_dev {
>  	void __iomem *regbase;
>  	void __iomem *smu_virt_addr;
> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
>  	struct apmf_sbios_req_v1 req1;
>  	struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
>  	bool cb_flag;			     /* To handle first custom BIOS input */
> +	struct cbi_ring_buffer cbi_buf;
> +	struct mutex cbi_mutex;		     /* Protects ring buffer access */

Include missing?

>  };
>  
>  struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..669680ce580a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
>  static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>  					   struct ta_pmf_enact_table *in)
>  {
> +	struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> +	struct bios_input_entry entry = { };
>  	unsigned int i;
>  
> +	guard(mutex)(&pdev->cbi_mutex);
> +
>  	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>  		amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
>  
> -	if (!(pdev->req.pending_req || pdev->req1.pending_req))
> +	if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)

Include missing.

As a general comment unrelated to this patch, these files to be missing 
other headers too.

> +		return;	/* return if ring buffer is empty */
> +
> +	entry = rb->data[rb->tail];
> +	rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> +
> +	/* If no active custom BIOS input pending request, do not consume further work */
> +	if (!entry.preq)
>  		return;
>  
>  	if (!pdev->smart_pc_enabled)
> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>  	case PMF_IF_V1:
>  		if (!is_apmf_bios_input_notifications_supported(pdev))
>  			return;
> -		amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
> -					   pdev->req1.custom_policy, in);
> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
>  		break;
>  	case PMF_IF_V2:
> -		amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
> -					   pdev->req.custom_policy, in);
> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);

Could we pass the entry instead now?

>  		break;
>  	default:
>  		break;
>  	}
> -
> -	/* Clear pending requests after handling */
> -	memset(&pdev->req, 0, sizeof(pdev->req));
> -	memset(&pdev->req1, 0, sizeof(pdev->req1));
>  }
>  
>  static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..add742e33e1e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>  		status = ret == TA_PMF_TYPE_SUCCESS;
>  		if (status) {
>  			dev->cb_flag = true;
> +			dev->cbi_buf.head = 0;
> +			dev->cbi_buf.tail = 0;
>  			break;
>  		}
>  		amd_pmf_tee_deinit(dev);
> 

-- 
 i.

Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Shyam Sundar S K]

On 11/5/2025 22:16, Ilpo Järvinen wrote:
> On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
> 
>> Custom BIOS input values can be updated by multiple sources, such as power
>> mode changes and sensor events, each triggering a custom BIOS input event.
>> When these events occur in rapid succession, new data may overwrite
>> previous values before they are processed, resulting in lost updates.
>>
>> To address this, introduce a fixed-size, power-of-two ring buffer to
>> capture every custom BIOS input event, storing both the pending request
>> and its associated input values. Access to the ring buffer is synchronized
>> using a mutex.
>>
>> The previous use of memset() to clear the pending request structure after
>> each event is removed, as each BIOS input value is now copied into the
>> buffer as a snapshot. Consumers now process entries directly from the ring
>> buffer, making explicit clearing of the pending request structure
>> unnecessary.
>>
>> 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>
>> ---
>> v2:
>>  - Add dev_WARN_ONCE()
>>  - Change variable name rb_mutex to cbi_mutex
>>  - Move tail increment logic above pending request check
>>
>>  drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
>>  drivers/platform/x86/amd/pmf/core.c   |  2 ++
>>  drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
>>  drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
>>  drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
>>  5 files changed, 79 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
>> index 13c4fec2c7ef..870a56f1fe07 100644
>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>>  									 req, sizeof(*req));
>>  }
>>  
>> +/* Store custom BIOS inputs data in ring buffer */
>> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
>> +{
>> +	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
>> +	struct bios_input_entry entry = { };
> 
> Too generic struct name.
> 
>> +	int i;
>> +
>> +	guard(mutex)(&pmf_dev->cbi_mutex);
>> +
>> +	switch (pmf_dev->cpu_id) {
>> +	case AMD_CPU_ID_PS:
>> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
>> +			entry.val[i] = pmf_dev->req1.custom_policy[i];
>> +		entry.preq = pmf_dev->req1.pending_req;
>> +		break;
>> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
>> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
>> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> 
> Add include.
> 
>> +			entry.val[i] = pmf_dev->req.custom_policy[i];
>> +		entry.preq = pmf_dev->req.pending_req;
>> +		break;
>> +	default:
>> +		return;
>> +	}
>> +
>> +	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
>> +		rb->data[rb->head] = entry;
>> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>> +	} else {
>> +		/* Rare case: ensures the newest BIOS input value is kept */
>> +		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
> 
> Is this really WARN_ONCE thing? That is, does it indicate a bug in the 
> kernel code?

If we hit this case, we might end up in a debug spew and hence put it
under WARN_ONCE (that way we are not lost in debugging the odd cases,
if we are under this scenario).

Not a bug in the kernel code..

> 
> Add include.
> 
>> +		rb->data[rb->head] = entry;
>> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>> +		rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> 
> Since you're under lock, can you make room separately first and then add
> the entry on a common path?
> 
>> +	}
>> +}
>> +
>>  static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
>>  {
>>  	if (!pdev->cb_flag)
>> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
>>  	dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
>>  
>>  	amd_pmf_handle_early_preq(pmf_dev);
>> +
>> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>>  }
>>  
>>  static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
>>  	dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
>>  
>>  	amd_pmf_handle_early_preq(pmf_dev);
>> +
>> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
>>  }
>>  
>>  static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>> index bc544a4a5266..8b97eba00dd3 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>>  	mutex_init(&dev->lock);
>>  	mutex_init(&dev->update_mutex);
>>  	mutex_init(&dev->cb_mutex);
>> +	mutex_init(&dev->cbi_mutex);
>>  
>>  	apmf_acpi_init(dev);
>>  	platform_set_drvdata(pdev, dev);
>> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
>>  	mutex_destroy(&dev->lock);
>>  	mutex_destroy(&dev->update_mutex);
>>  	mutex_destroy(&dev->cb_mutex);
>> +	mutex_destroy(&dev->cbi_mutex);
>>  }
>>  
>>  static const struct attribute_group *amd_pmf_driver_groups[] = {
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 2145df4128cd..6cb1e228c48e 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -12,6 +12,7 @@
>>  #define PMF_H
>>  
>>  #include <linux/acpi.h>
>> +#include <linux/circ_buf.h>
>>  #include <linux/input.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/platform_profile.h>
>> @@ -120,6 +121,7 @@ struct cookie_header {
>>  #define APTS_MAX_STATES		16
>>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
>>  #define BIOS_INPUTS_MAX		10
>> +#define CUSTOM_BIOS_INPUT_RB_SZ	64	/* Must be power of two for CIRC_* macros */
> 
> This is not really size but there are n entries. The total size will be 
> something else. I'd call it NUM or ENTRIES, or something like that. 
> Perhaps CUSTOM_BIOS_INPUT_RING_ENTRIES but that is a bit to the long side 
> of things?
> 
>>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>>  
>> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
>>  	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>>  };
>>  
>> +/**
>> + * struct bios_input_entry - Snapshot of custom BIOS input event
>> + * @val: Array of custom BIOS input values
>> + * @preq: Pending request value associated with this event
>> + */
>> +struct bios_input_entry {
>> +	u32 val[BIOS_INPUTS_MAX];
>> +	u32 preq;
>> +};
>> +
>> +struct cbi_ring_buffer {
>> +	struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
>> +	int head;
>> +	int tail;
>> +};
>> +
>>  struct amd_pmf_dev {
>>  	void __iomem *regbase;
>>  	void __iomem *smu_virt_addr;
>> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
>>  	struct apmf_sbios_req_v1 req1;
>>  	struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
>>  	bool cb_flag;			     /* To handle first custom BIOS input */
>> +	struct cbi_ring_buffer cbi_buf;
>> +	struct mutex cbi_mutex;		     /* Protects ring buffer access */
> 
> Include missing?

yes..

> 
>>  };
>>  
>>  struct apmf_sps_prop_granular_v2 {
>> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
>> index 85192c7536b8..669680ce580a 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
>>  static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>>  					   struct ta_pmf_enact_table *in)
>>  {
>> +	struct cbi_ring_buffer *rb = &pdev->cbi_buf;
>> +	struct bios_input_entry entry = { };
>>  	unsigned int i;
>>  
>> +	guard(mutex)(&pdev->cbi_mutex);
>> +
>>  	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>>  		amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
>>  
>> -	if (!(pdev->req.pending_req || pdev->req1.pending_req))
>> +	if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> 
> Include missing.

circ_buf.h is included in pmf.h since acpi.c and spc.c are using it.

> 
> As a general comment unrelated to this patch, these files to be missing 
> other headers too.

Yeah. I get your point. Let me add all the relavent missing headers
and respin a new version.

ack to other comments in the patch.

Thanks,
Shyam
> 
>> +		return;	/* return if ring buffer is empty */
>> +
>> +	entry = rb->data[rb->tail];
>> +	rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>> +
>> +	/* If no active custom BIOS input pending request, do not consume further work */
>> +	if (!entry.preq)
>>  		return;
>>  
>>  	if (!pdev->smart_pc_enabled)
>> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>>  	case PMF_IF_V1:
>>  		if (!is_apmf_bios_input_notifications_supported(pdev))
>>  			return;
>> -		amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
>> -					   pdev->req1.custom_policy, in);
>> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
>>  		break;
>>  	case PMF_IF_V2:
>> -		amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
>> -					   pdev->req.custom_policy, in);
>> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
> 
> Could we pass the entry instead now?
> 
>>  		break;
>>  	default:
>>  		break;
>>  	}
>> -
>> -	/* Clear pending requests after handling */
>> -	memset(&pdev->req, 0, sizeof(pdev->req));
>> -	memset(&pdev->req1, 0, sizeof(pdev->req1));
>>  }
>>  
>>  static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 6e8116bef4f6..add742e33e1e 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
>>  		status = ret == TA_PMF_TYPE_SUCCESS;
>>  		if (status) {
>>  			dev->cb_flag = true;
>> +			dev->cbi_buf.head = 0;
>> +			dev->cbi_buf.tail = 0;
>>  			break;
>>  		}
>>  		amd_pmf_tee_deinit(dev);
>>
> 

Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Ilpo Järvinen]

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

On Fri, 7 Nov 2025, Shyam Sundar S K wrote:
> On 11/5/2025 22:16, Ilpo Järvinen wrote:
> > On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
> > 
> >> Custom BIOS input values can be updated by multiple sources, such as power
> >> mode changes and sensor events, each triggering a custom BIOS input event.
> >> When these events occur in rapid succession, new data may overwrite
> >> previous values before they are processed, resulting in lost updates.
> >>
> >> To address this, introduce a fixed-size, power-of-two ring buffer to
> >> capture every custom BIOS input event, storing both the pending request
> >> and its associated input values. Access to the ring buffer is synchronized
> >> using a mutex.
> >>
> >> The previous use of memset() to clear the pending request structure after
> >> each event is removed, as each BIOS input value is now copied into the
> >> buffer as a snapshot. Consumers now process entries directly from the ring
> >> buffer, making explicit clearing of the pending request structure
> >> unnecessary.
> >>
> >> 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>
> >> ---
> >> v2:
> >>  - Add dev_WARN_ONCE()
> >>  - Change variable name rb_mutex to cbi_mutex
> >>  - Move tail increment logic above pending request check
> >>
> >>  drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
> >>  drivers/platform/x86/amd/pmf/core.c   |  2 ++
> >>  drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
> >>  drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
> >>  drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
> >>  5 files changed, 79 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> >> index 13c4fec2c7ef..870a56f1fe07 100644
> >> --- a/drivers/platform/x86/amd/pmf/acpi.c
> >> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> >> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
> >>  									 req, sizeof(*req));
> >>  }
> >>  
> >> +/* Store custom BIOS inputs data in ring buffer */
> >> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
> >> +{
> >> +	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> >> +	struct bios_input_entry entry = { };
> > 
> > Too generic struct name.
> > 
> >> +	int i;
> >> +
> >> +	guard(mutex)(&pmf_dev->cbi_mutex);
> >> +
> >> +	switch (pmf_dev->cpu_id) {
> >> +	case AMD_CPU_ID_PS:
> >> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
> >> +			entry.val[i] = pmf_dev->req1.custom_policy[i];
> >> +		entry.preq = pmf_dev->req1.pending_req;
> >> +		break;
> >> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> >> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> >> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> > 
> > Add include.
> > 
> >> +			entry.val[i] = pmf_dev->req.custom_policy[i];
> >> +		entry.preq = pmf_dev->req.pending_req;
> >> +		break;
> >> +	default:
> >> +		return;
> >> +	}
> >> +
> >> +	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
> >> +		rb->data[rb->head] = entry;
> >> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >> +	} else {
> >> +		/* Rare case: ensures the newest BIOS input value is kept */
> >> +		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
> > 
> > Is this really WARN_ONCE thing? That is, does it indicate a bug in the 
> > kernel code?
> 
> If we hit this case, we might end up in a debug spew and hence put it
> under WARN_ONCE (that way we are not lost in debugging the odd cases,
> if we are under this scenario).

Why doesn't printing just a dev_warn() suffice (it should be just as 
visible in the log provided as the stacktrace, etc. is)? Why you need 
stacktrace, etc. to differentiate?

-- 
 i.

> Not a bug in the kernel code..
> 
> > 
> > Add include.
> > 
> >> +		rb->data[rb->head] = entry;
> >> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >> +		rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> > 
> > Since you're under lock, can you make room separately first and then add
> > the entry on a common path?
> > 
> >> +	}
> >> +}
> >> +
> >>  static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
> >>  {
> >>  	if (!pdev->cb_flag)
> >> @@ -356,6 +393,8 @@ static void apmf_event_handler_v2(acpi_handle handle, u32 event, void *data)
> >>  	dev_dbg(pmf_dev->dev, "Pending request (preq): 0x%x\n", pmf_dev->req.pending_req);
> >>  
> >>  	amd_pmf_handle_early_preq(pmf_dev);
> >> +
> >> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
> >>  }
> >>  
> >>  static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> >> @@ -374,6 +413,8 @@ static void apmf_event_handler_v1(acpi_handle handle, u32 event, void *data)
> >>  	dev_dbg(pmf_dev->dev, "Pending request (preq1): 0x%x\n", pmf_dev->req1.pending_req);
> >>  
> >>  	amd_pmf_handle_early_preq(pmf_dev);
> >> +
> >> +	amd_pmf_custom_bios_inputs_rb(pmf_dev);
> >>  }
> >>  
> >>  static void apmf_event_handler(acpi_handle handle, u32 event, void *data)
> >> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> >> index bc544a4a5266..8b97eba00dd3 100644
> >> --- a/drivers/platform/x86/amd/pmf/core.c
> >> +++ b/drivers/platform/x86/amd/pmf/core.c
> >> @@ -468,6 +468,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
> >>  	mutex_init(&dev->lock);
> >>  	mutex_init(&dev->update_mutex);
> >>  	mutex_init(&dev->cb_mutex);
> >> +	mutex_init(&dev->cbi_mutex);
> >>  
> >>  	apmf_acpi_init(dev);
> >>  	platform_set_drvdata(pdev, dev);
> >> @@ -494,6 +495,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
> >>  	mutex_destroy(&dev->lock);
> >>  	mutex_destroy(&dev->update_mutex);
> >>  	mutex_destroy(&dev->cb_mutex);
> >> +	mutex_destroy(&dev->cbi_mutex);
> >>  }
> >>  
> >>  static const struct attribute_group *amd_pmf_driver_groups[] = {
> >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> >> index 2145df4128cd..6cb1e228c48e 100644
> >> --- a/drivers/platform/x86/amd/pmf/pmf.h
> >> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> >> @@ -12,6 +12,7 @@
> >>  #define PMF_H
> >>  
> >>  #include <linux/acpi.h>
> >> +#include <linux/circ_buf.h>
> >>  #include <linux/input.h>
> >>  #include <linux/platform_device.h>
> >>  #include <linux/platform_profile.h>
> >> @@ -120,6 +121,7 @@ struct cookie_header {
> >>  #define APTS_MAX_STATES		16
> >>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
> >>  #define BIOS_INPUTS_MAX		10
> >> +#define CUSTOM_BIOS_INPUT_RB_SZ	64	/* Must be power of two for CIRC_* macros */
> > 
> > This is not really size but there are n entries. The total size will be 
> > something else. I'd call it NUM or ENTRIES, or something like that. 
> > Perhaps CUSTOM_BIOS_INPUT_RING_ENTRIES but that is a bit to the long side 
> > of things?
> > 
> >>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
> >>  
> >> @@ -359,6 +361,22 @@ struct pmf_bios_inputs_prev {
> >>  	u32 custom_bios_inputs[BIOS_INPUTS_MAX];
> >>  };
> >>  
> >> +/**
> >> + * struct bios_input_entry - Snapshot of custom BIOS input event
> >> + * @val: Array of custom BIOS input values
> >> + * @preq: Pending request value associated with this event
> >> + */
> >> +struct bios_input_entry {
> >> +	u32 val[BIOS_INPUTS_MAX];
> >> +	u32 preq;
> >> +};
> >> +
> >> +struct cbi_ring_buffer {
> >> +	struct bios_input_entry data[CUSTOM_BIOS_INPUT_RB_SZ];
> >> +	int head;
> >> +	int tail;
> >> +};
> >> +
> >>  struct amd_pmf_dev {
> >>  	void __iomem *regbase;
> >>  	void __iomem *smu_virt_addr;
> >> @@ -407,6 +425,8 @@ struct amd_pmf_dev {
> >>  	struct apmf_sbios_req_v1 req1;
> >>  	struct pmf_bios_inputs_prev cb_prev; /* To preserve custom BIOS inputs */
> >>  	bool cb_flag;			     /* To handle first custom BIOS input */
> >> +	struct cbi_ring_buffer cbi_buf;
> >> +	struct mutex cbi_mutex;		     /* Protects ring buffer access */
> > 
> > Include missing?
> 
> yes..
> 
> > 
> >>  };
> >>  
> >>  struct apmf_sps_prop_granular_v2 {
> >> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> >> index 85192c7536b8..669680ce580a 100644
> >> --- a/drivers/platform/x86/amd/pmf/spc.c
> >> +++ b/drivers/platform/x86/amd/pmf/spc.c
> >> @@ -150,12 +150,23 @@ static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req
> >>  static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> >>  					   struct ta_pmf_enact_table *in)
> >>  {
> >> +	struct cbi_ring_buffer *rb = &pdev->cbi_buf;
> >> +	struct bios_input_entry entry = { };
> >>  	unsigned int i;
> >>  
> >> +	guard(mutex)(&pdev->cbi_mutex);
> >> +
> >>  	for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
> >>  		amd_pmf_set_ta_custom_bios_input(in, i, pdev->cb_prev.custom_bios_inputs[i]);
> >>  
> >> -	if (!(pdev->req.pending_req || pdev->req1.pending_req))
> >> +	if (CIRC_CNT(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) == 0)
> > 
> > Include missing.
> 
> circ_buf.h is included in pmf.h since acpi.c and spc.c are using it.
> 
> > 
> > As a general comment unrelated to this patch, these files to be missing 
> > other headers too.
> 
> Yeah. I get your point. Let me add all the relavent missing headers
> and respin a new version.
> 
> ack to other comments in the patch.
> 
> Thanks,
> Shyam
> > 
> >> +		return;	/* return if ring buffer is empty */
> >> +
> >> +	entry = rb->data[rb->tail];
> >> +	rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
> >> +
> >> +	/* If no active custom BIOS input pending request, do not consume further work */
> >> +	if (!entry.preq)
> >>  		return;
> >>  
> >>  	if (!pdev->smart_pc_enabled)
> >> @@ -165,20 +176,14 @@ static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> >>  	case PMF_IF_V1:
> >>  		if (!is_apmf_bios_input_notifications_supported(pdev))
> >>  			return;
> >> -		amd_pmf_update_bios_inputs(pdev, pdev->req1.pending_req, custom_bios_inputs_v1,
> >> -					   pdev->req1.custom_policy, in);
> >> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs_v1, entry.val, in);
> >>  		break;
> >>  	case PMF_IF_V2:
> >> -		amd_pmf_update_bios_inputs(pdev, pdev->req.pending_req, custom_bios_inputs,
> >> -					   pdev->req.custom_policy, in);
> >> +		amd_pmf_update_bios_inputs(pdev, entry.preq, custom_bios_inputs, entry.val, in);
> > 
> > Could we pass the entry instead now?
> > 
> >>  		break;
> >>  	default:
> >>  		break;
> >>  	}
> >> -
> >> -	/* Clear pending requests after handling */
> >> -	memset(&pdev->req, 0, sizeof(pdev->req));
> >> -	memset(&pdev->req1, 0, sizeof(pdev->req1));
> >>  }
> >>  
> >>  static void amd_pmf_get_c0_residency(u16 *core_res, size_t size, struct ta_pmf_enact_table *in)
> >> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> >> index 6e8116bef4f6..add742e33e1e 100644
> >> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> >> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> >> @@ -579,6 +579,8 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev)
> >>  		status = ret == TA_PMF_TYPE_SUCCESS;
> >>  		if (status) {
> >>  			dev->cb_flag = true;
> >> +			dev->cbi_buf.head = 0;
> >> +			dev->cbi_buf.tail = 0;
> >>  			break;
> >>  		}
> >>  		amd_pmf_tee_deinit(dev);
> >>
> > 
> 

Re: [PATCH v2 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values

[Shyam Sundar S K]

On 11/7/2025 15:52, Ilpo Järvinen wrote:
> On Fri, 7 Nov 2025, Shyam Sundar S K wrote:
>> On 11/5/2025 22:16, Ilpo Järvinen wrote:
>>> On Wed, 15 Oct 2025, Shyam Sundar S K wrote:
>>>
>>>> Custom BIOS input values can be updated by multiple sources, such as power
>>>> mode changes and sensor events, each triggering a custom BIOS input event.
>>>> When these events occur in rapid succession, new data may overwrite
>>>> previous values before they are processed, resulting in lost updates.
>>>>
>>>> To address this, introduce a fixed-size, power-of-two ring buffer to
>>>> capture every custom BIOS input event, storing both the pending request
>>>> and its associated input values. Access to the ring buffer is synchronized
>>>> using a mutex.
>>>>
>>>> The previous use of memset() to clear the pending request structure after
>>>> each event is removed, as each BIOS input value is now copied into the
>>>> buffer as a snapshot. Consumers now process entries directly from the ring
>>>> buffer, making explicit clearing of the pending request structure
>>>> unnecessary.
>>>>
>>>> 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>
>>>> ---
>>>> v2:
>>>>  - Add dev_WARN_ONCE()
>>>>  - Change variable name rb_mutex to cbi_mutex
>>>>  - Move tail increment logic above pending request check
>>>>
>>>>  drivers/platform/x86/amd/pmf/acpi.c   | 41 +++++++++++++++++++++++++++
>>>>  drivers/platform/x86/amd/pmf/core.c   |  2 ++
>>>>  drivers/platform/x86/amd/pmf/pmf.h    | 20 +++++++++++++
>>>>  drivers/platform/x86/amd/pmf/spc.c    | 23 +++++++++------
>>>>  drivers/platform/x86/amd/pmf/tee-if.c |  2 ++
>>>>  5 files changed, 79 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
>>>> index 13c4fec2c7ef..870a56f1fe07 100644
>>>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>>>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>>>> @@ -331,6 +331,43 @@ int apmf_get_sbios_requests(struct amd_pmf_dev *pdev, struct apmf_sbios_req *req
>>>>  									 req, sizeof(*req));
>>>>  }
>>>>  
>>>> +/* Store custom BIOS inputs data in ring buffer */
>>>> +static void amd_pmf_custom_bios_inputs_rb(struct amd_pmf_dev *pmf_dev)
>>>> +{
>>>> +	struct cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
>>>> +	struct bios_input_entry entry = { };
>>>
>>> Too generic struct name.
>>>
>>>> +	int i;
>>>> +
>>>> +	guard(mutex)(&pmf_dev->cbi_mutex);
>>>> +
>>>> +	switch (pmf_dev->cpu_id) {
>>>> +	case AMD_CPU_ID_PS:
>>>> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs_v1); i++)
>>>> +			entry.val[i] = pmf_dev->req1.custom_policy[i];
>>>> +		entry.preq = pmf_dev->req1.pending_req;
>>>> +		break;
>>>> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
>>>> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
>>>> +		for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++)
>>>
>>> Add include.
>>>
>>>> +			entry.val[i] = pmf_dev->req.custom_policy[i];
>>>> +		entry.preq = pmf_dev->req.pending_req;
>>>> +		break;
>>>> +	default:
>>>> +		return;
>>>> +	}
>>>> +
>>>> +	if (CIRC_SPACE(rb->head, rb->tail, CUSTOM_BIOS_INPUT_RB_SZ) > 0) {
>>>> +		rb->data[rb->head] = entry;
>>>> +		rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RB_SZ - 1);
>>>> +	} else {
>>>> +		/* Rare case: ensures the newest BIOS input value is kept */
>>>> +		dev_WARN_ONCE(pmf_dev->dev, 1, "Overwriting BIOS input value, data may be lost\n");
>>>
>>> Is this really WARN_ONCE thing? That is, does it indicate a bug in the 
>>> kernel code?
>>
>> If we hit this case, we might end up in a debug spew and hence put it
>> under WARN_ONCE (that way we are not lost in debugging the odd cases,
>> if we are under this scenario).
> 
> Why doesn't printing just a dev_warn() suffice (it should be just as 
> visible in the log provided as the stacktrace, etc. is)? Why you need 
> stacktrace, etc. to differentiate?
> 

Yes, that was the intent. But let me try with dev_warn() again..

Thanks,
Shyam