* [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size
@ 2025-11-07 11:01 Shyam Sundar S K
2025-11-07 11:01 ` [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
2025-11-18 10:18 ` [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size Ilpo Järvinen
0 siblings, 2 replies; 5+ messages in thread
From: Shyam Sundar S K @ 2025-11-07 11:01 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, Patil.Reddy, Shyam Sundar S K,
Mario Limonciello, Yijun Shen
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.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Yijun Shen <Yijun.Shen@Dell.com>
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>
---
v3:
- No change
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-11-07 11:01 [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size Shyam Sundar S K
@ 2025-11-07 11:01 ` Shyam Sundar S K
2025-11-18 10:15 ` Ilpo Järvinen
2025-11-18 10:18 ` [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size Ilpo Järvinen
1 sibling, 1 reply; 5+ messages in thread
From: Shyam Sundar S K @ 2025-11-07 11:01 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, Patil.Reddy, Shyam Sundar S K,
Mario Limonciello, Yijun Shen
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.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
Tested-by: Yijun Shen <Yijun.Shen@Dell.com>
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>
---
v3:
- include headers wherever missing
- use dev_warn() instead of dev_WARN_ONCE()
- remove generic struct names
- enhance ringbuffer mechanism to handle common path
- other cosmetic remarks
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 | 42 +++++++++++++++++++++++++++
drivers/platform/x86/amd/pmf/core.c | 3 ++
drivers/platform/x86/amd/pmf/pmf.h | 21 ++++++++++++++
drivers/platform/x86/amd/pmf/spc.c | 36 +++++++++++++----------
drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
5 files changed, 89 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
index 13c4fec2c7ef..4750ae6d70b0 100644
--- a/drivers/platform/x86/amd/pmf/acpi.c
+++ b/drivers/platform/x86/amd/pmf/acpi.c
@@ -9,6 +9,9 @@
*/
#include <linux/acpi.h>
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/dev_printk.h>
#include "pmf.h"
#define APMF_CQL_NOTIFICATION 2
@@ -331,6 +334,41 @@ 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 pmf_cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
+ struct pmf_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_RING_ENTRIES) == 0) {
+ /* Rare case: ensures the newest BIOS input value is kept */
+ dev_warn(pmf_dev->dev, "Overwriting BIOS input value, data may be lost\n");
+ rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
+ }
+
+ rb->data[rb->head] = entry;
+ rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
+}
+
static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
{
if (!pdev->cb_flag)
@@ -356,6 +394,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 +414,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..8d5ac84ae025 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -11,6 +11,7 @@
#include <linux/debugfs.h>
#include <linux/iopoll.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
#include <linux/power_supply.h>
@@ -468,6 +469,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 +496,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..5a18b3604b6e 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -12,7 +12,9 @@
#define PMF_H
#include <linux/acpi.h>
+#include <linux/circ_buf.h>
#include <linux/input.h>
+#include <linux/mutex_types.h>
#include <linux/platform_device.h>
#include <linux/platform_profile.h>
@@ -120,6 +122,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_RING_ENTRIES 64 /* Must be power of two for CIRC_* macros */
typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
@@ -359,6 +362,22 @@ struct pmf_bios_inputs_prev {
u32 custom_bios_inputs[BIOS_INPUTS_MAX];
};
+/**
+ * struct pmf_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 pmf_bios_input_entry {
+ u32 val[BIOS_INPUTS_MAX];
+ u32 preq;
+};
+
+struct pmf_cbi_ring_buffer {
+ struct pmf_bios_input_entry data[CUSTOM_BIOS_INPUT_RING_ENTRIES];
+ int head;
+ int tail;
+};
+
struct amd_pmf_dev {
void __iomem *regbase;
void __iomem *smu_virt_addr;
@@ -407,6 +426,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 pmf_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..7c6bbfaa785a 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -11,6 +11,7 @@
#include <acpi/button.h>
#include <linux/amd-pmf-io.h>
+#include <linux/cleanup.h>
#include <linux/power_supply.h>
#include <linux/units.h>
#include "pmf.h"
@@ -132,30 +133,41 @@ static void amd_pmf_set_ta_custom_bios_input(struct ta_pmf_enact_table *in, int
}
}
-static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req,
+static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, struct pmf_bios_input_entry *data,
const struct amd_pmf_pb_bitmap *inputs,
- const u32 *custom_policy, struct ta_pmf_enact_table *in)
+ struct ta_pmf_enact_table *in)
{
unsigned int i;
for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++) {
- if (!(pending_req & inputs[i].bit_mask))
+ if (!(data->preq & inputs[i].bit_mask))
continue;
- amd_pmf_set_ta_custom_bios_input(in, i, custom_policy[i]);
- pdev->cb_prev.custom_bios_inputs[i] = custom_policy[i];
- dev_dbg(pdev->dev, "Custom BIOS Input[%d]: %u\n", i, custom_policy[i]);
+ amd_pmf_set_ta_custom_bios_input(in, i, data->val[i]);
+ pdev->cb_prev.custom_bios_inputs[i] = data->val[i];
+ dev_dbg(pdev->dev, "Custom BIOS Input[%d]: %u\n", i, data->val[i]);
}
}
static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
struct ta_pmf_enact_table *in)
{
+ struct pmf_cbi_ring_buffer *rb = &pdev->cbi_buf;
+ struct pmf_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_RING_ENTRIES) == 0)
+ return; /* return if ring buffer is empty */
+
+ entry = rb->data[rb->tail];
+ rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 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 +177,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, custom_bios_inputs_v1, 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, custom_bios_inputs, 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-11-07 11:01 ` [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
@ 2025-11-18 10:15 ` Ilpo Järvinen
2025-11-19 5:59 ` Shyam Sundar S K
0 siblings, 1 reply; 5+ messages in thread
From: Ilpo Järvinen @ 2025-11-18 10:15 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, platform-driver-x86, Patil.Reddy,
Mario Limonciello, Yijun Shen
On Fri, 7 Nov 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.
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Tested-by: Yijun Shen <Yijun.Shen@Dell.com>
> 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>
> ---
> v3:
> - include headers wherever missing
> - use dev_warn() instead of dev_WARN_ONCE()
> - remove generic struct names
> - enhance ringbuffer mechanism to handle common path
> - other cosmetic remarks
>
> 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 | 42 +++++++++++++++++++++++++++
> drivers/platform/x86/amd/pmf/core.c | 3 ++
> drivers/platform/x86/amd/pmf/pmf.h | 21 ++++++++++++++
> drivers/platform/x86/amd/pmf/spc.c | 36 +++++++++++++----------
> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
> 5 files changed, 89 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
> index 13c4fec2c7ef..4750ae6d70b0 100644
> --- a/drivers/platform/x86/amd/pmf/acpi.c
> +++ b/drivers/platform/x86/amd/pmf/acpi.c
> @@ -9,6 +9,9 @@
> */
>
> #include <linux/acpi.h>
> +#include <linux/array_size.h>
> +#include <linux/cleanup.h>
> +#include <linux/dev_printk.h>
> #include "pmf.h"
>
> #define APMF_CQL_NOTIFICATION 2
> @@ -331,6 +334,41 @@ 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 pmf_cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
> + struct pmf_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_RING_ENTRIES) == 0) {
> + /* Rare case: ensures the newest BIOS input value is kept */
> + dev_warn(pmf_dev->dev, "Overwriting BIOS input value, data may be lost\n");
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
> + }
> +
> + rb->data[rb->head] = entry;
I'd prefer the entry is construct in place.
> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
> +}
> +
> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
> {
> if (!pdev->cb_flag)
> @@ -356,6 +394,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 +414,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..8d5ac84ae025 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -11,6 +11,7 @@
> #include <linux/debugfs.h>
> #include <linux/iopoll.h>
> #include <linux/module.h>
> +#include <linux/mutex.h>
> #include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/power_supply.h>
> @@ -468,6 +469,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);
devm_mutex_init() + add a patch to convert the existing once to use it
too. And don't forget the error handling as devm_*() can fail.
> apmf_acpi_init(dev);
> platform_set_drvdata(pdev, dev);
> @@ -494,6 +496,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..5a18b3604b6e 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,7 +12,9 @@
> #define PMF_H
>
> #include <linux/acpi.h>
> +#include <linux/circ_buf.h>
> #include <linux/input.h>
> +#include <linux/mutex_types.h>
> #include <linux/platform_device.h>
> #include <linux/platform_profile.h>
>
> @@ -120,6 +122,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_RING_ENTRIES 64 /* Must be power of two for CIRC_* macros */
>
> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>
> @@ -359,6 +362,22 @@ struct pmf_bios_inputs_prev {
> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
> };
>
> +/**
> + * struct pmf_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 pmf_bios_input_entry {
> + u32 val[BIOS_INPUTS_MAX];
> + u32 preq;
> +};
> +
> +struct pmf_cbi_ring_buffer {
> + struct pmf_bios_input_entry data[CUSTOM_BIOS_INPUT_RING_ENTRIES];
> + int head;
> + int tail;
> +};
> +
> struct amd_pmf_dev {
> void __iomem *regbase;
> void __iomem *smu_virt_addr;
> @@ -407,6 +426,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 pmf_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..7c6bbfaa785a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -11,6 +11,7 @@
>
> #include <acpi/button.h>
> #include <linux/amd-pmf-io.h>
> +#include <linux/cleanup.h>
> #include <linux/power_supply.h>
> #include <linux/units.h>
> #include "pmf.h"
> @@ -132,30 +133,41 @@ static void amd_pmf_set_ta_custom_bios_input(struct ta_pmf_enact_table *in, int
> }
> }
>
> -static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req,
> +static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, struct pmf_bios_input_entry *data,
> const struct amd_pmf_pb_bitmap *inputs,
> - const u32 *custom_policy, struct ta_pmf_enact_table *in)
> + struct ta_pmf_enact_table *in)
> {
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++) {
> - if (!(pending_req & inputs[i].bit_mask))
> + if (!(data->preq & inputs[i].bit_mask))
> continue;
> - amd_pmf_set_ta_custom_bios_input(in, i, custom_policy[i]);
> - pdev->cb_prev.custom_bios_inputs[i] = custom_policy[i];
> - dev_dbg(pdev->dev, "Custom BIOS Input[%d]: %u\n", i, custom_policy[i]);
> + amd_pmf_set_ta_custom_bios_input(in, i, data->val[i]);
> + pdev->cb_prev.custom_bios_inputs[i] = data->val[i];
> + dev_dbg(pdev->dev, "Custom BIOS Input[%d]: %u\n", i, data->val[i]);
> }
> }
>
> static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
> struct ta_pmf_enact_table *in)
> {
> + struct pmf_cbi_ring_buffer *rb = &pdev->cbi_buf;
> + struct pmf_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_RING_ENTRIES) == 0)
> + return; /* return if ring buffer is empty */
> +
> + entry = rb->data[rb->tail];
> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
> +
> + /* If no active custom BIOS input pending request, do not consume further work */
> + if (!entry.preq)
> return;
Should this function empty the whole ring, not just process one entry?
> if (!pdev->smart_pc_enabled)
> @@ -165,20 +177,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, custom_bios_inputs_v1, 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, custom_bios_inputs, 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);
>
--
i.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size
2025-11-07 11:01 [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size Shyam Sundar S K
2025-11-07 11:01 ` [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
@ 2025-11-18 10:18 ` Ilpo Järvinen
1 sibling, 0 replies; 5+ messages in thread
From: Ilpo Järvinen @ 2025-11-18 10:18 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, platform-driver-x86, Patil.Reddy,
Mario Limonciello, Yijun Shen
[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]
On Fri, 7 Nov 2025, 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.
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
> Tested-by: Yijun Shen <Yijun.Shen@Dell.com>
> 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>
> ---
> v3:
> - No change
>
> 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];
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Unrelated to the patch, I'm not entirely sure though why the naming
differs for custom_bios_inputs vs custom_bios.
--
i.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values
2025-11-18 10:15 ` Ilpo Järvinen
@ 2025-11-19 5:59 ` Shyam Sundar S K
0 siblings, 0 replies; 5+ messages in thread
From: Shyam Sundar S K @ 2025-11-19 5:59 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, Patil.Reddy,
Mario Limonciello, Yijun Shen
On 11/18/2025 15:45, Ilpo Järvinen wrote:
> On Fri, 7 Nov 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.
>>
>> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>> Tested-by: Yijun Shen <Yijun.Shen@Dell.com>
>> 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>
>> ---
>> v3:
>> - include headers wherever missing
>> - use dev_warn() instead of dev_WARN_ONCE()
>> - remove generic struct names
>> - enhance ringbuffer mechanism to handle common path
>> - other cosmetic remarks
>>
>> 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 | 42 +++++++++++++++++++++++++++
>> drivers/platform/x86/amd/pmf/core.c | 3 ++
>> drivers/platform/x86/amd/pmf/pmf.h | 21 ++++++++++++++
>> drivers/platform/x86/amd/pmf/spc.c | 36 +++++++++++++----------
>> drivers/platform/x86/amd/pmf/tee-if.c | 2 ++
>> 5 files changed, 89 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c
>> index 13c4fec2c7ef..4750ae6d70b0 100644
>> --- a/drivers/platform/x86/amd/pmf/acpi.c
>> +++ b/drivers/platform/x86/amd/pmf/acpi.c
>> @@ -9,6 +9,9 @@
>> */
>>
>> #include <linux/acpi.h>
>> +#include <linux/array_size.h>
>> +#include <linux/cleanup.h>
>> +#include <linux/dev_printk.h>
>> #include "pmf.h"
>>
>> #define APMF_CQL_NOTIFICATION 2
>> @@ -331,6 +334,41 @@ 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 pmf_cbi_ring_buffer *rb = &pmf_dev->cbi_buf;
>> + struct pmf_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_RING_ENTRIES) == 0) {
>> + /* Rare case: ensures the newest BIOS input value is kept */
>> + dev_warn(pmf_dev->dev, "Overwriting BIOS input value, data may be lost\n");
>> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
>> + }
>> +
>> + rb->data[rb->head] = entry;
>
> I'd prefer the entry is construct in place.
>
>> + rb->head = (rb->head + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
>> +}
>> +
>> static void amd_pmf_handle_early_preq(struct amd_pmf_dev *pdev)
>> {
>> if (!pdev->cb_flag)
>> @@ -356,6 +394,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 +414,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..8d5ac84ae025 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -11,6 +11,7 @@
>> #include <linux/debugfs.h>
>> #include <linux/iopoll.h>
>> #include <linux/module.h>
>> +#include <linux/mutex.h>
>> #include <linux/pci.h>
>> #include <linux/platform_device.h>
>> #include <linux/power_supply.h>
>> @@ -468,6 +469,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);
>
> devm_mutex_init() + add a patch to convert the existing once to use it
> too. And don't forget the error handling as devm_*() can fail.
>
>> apmf_acpi_init(dev);
>> platform_set_drvdata(pdev, dev);
>> @@ -494,6 +496,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..5a18b3604b6e 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -12,7 +12,9 @@
>> #define PMF_H
>>
>> #include <linux/acpi.h>
>> +#include <linux/circ_buf.h>
>> #include <linux/input.h>
>> +#include <linux/mutex_types.h>
>> #include <linux/platform_device.h>
>> #include <linux/platform_profile.h>
>>
>> @@ -120,6 +122,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_RING_ENTRIES 64 /* Must be power of two for CIRC_* macros */
>>
>> typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>>
>> @@ -359,6 +362,22 @@ struct pmf_bios_inputs_prev {
>> u32 custom_bios_inputs[BIOS_INPUTS_MAX];
>> };
>>
>> +/**
>> + * struct pmf_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 pmf_bios_input_entry {
>> + u32 val[BIOS_INPUTS_MAX];
>> + u32 preq;
>> +};
>> +
>> +struct pmf_cbi_ring_buffer {
>> + struct pmf_bios_input_entry data[CUSTOM_BIOS_INPUT_RING_ENTRIES];
>> + int head;
>> + int tail;
>> +};
>> +
>> struct amd_pmf_dev {
>> void __iomem *regbase;
>> void __iomem *smu_virt_addr;
>> @@ -407,6 +426,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 pmf_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..7c6bbfaa785a 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -11,6 +11,7 @@
>>
>> #include <acpi/button.h>
>> #include <linux/amd-pmf-io.h>
>> +#include <linux/cleanup.h>
>> #include <linux/power_supply.h>
>> #include <linux/units.h>
>> #include "pmf.h"
>> @@ -132,30 +133,41 @@ static void amd_pmf_set_ta_custom_bios_input(struct ta_pmf_enact_table *in, int
>> }
>> }
>>
>> -static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, u32 pending_req,
>> +static void amd_pmf_update_bios_inputs(struct amd_pmf_dev *pdev, struct pmf_bios_input_entry *data,
>> const struct amd_pmf_pb_bitmap *inputs,
>> - const u32 *custom_policy, struct ta_pmf_enact_table *in)
>> + struct ta_pmf_enact_table *in)
>> {
>> unsigned int i;
>>
>> for (i = 0; i < ARRAY_SIZE(custom_bios_inputs); i++) {
>> - if (!(pending_req & inputs[i].bit_mask))
>> + if (!(data->preq & inputs[i].bit_mask))
>> continue;
>> - amd_pmf_set_ta_custom_bios_input(in, i, custom_policy[i]);
>> - pdev->cb_prev.custom_bios_inputs[i] = custom_policy[i];
>> - dev_dbg(pdev->dev, "Custom BIOS Input[%d]: %u\n", i, custom_policy[i]);
>> + amd_pmf_set_ta_custom_bios_input(in, i, data->val[i]);
>> + pdev->cb_prev.custom_bios_inputs[i] = data->val[i];
>> + dev_dbg(pdev->dev, "Custom BIOS Input[%d]: %u\n", i, data->val[i]);
>> }
>> }
>>
>> static void amd_pmf_get_custom_bios_inputs(struct amd_pmf_dev *pdev,
>> struct ta_pmf_enact_table *in)
>> {
>> + struct pmf_cbi_ring_buffer *rb = &pdev->cbi_buf;
>> + struct pmf_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_RING_ENTRIES) == 0)
>> + return; /* return if ring buffer is empty */
>> +
>> + entry = rb->data[rb->tail];
>> + rb->tail = (rb->tail + 1) & (CUSTOM_BIOS_INPUT_RING_ENTRIES - 1);
>> +
>> + /* If no active custom BIOS input pending request, do not consume further work */
>> + if (!entry.preq)
>> return;
>
> Should this function empty the whole ring, not just process one entry?
Yes. This function should drain the entire ring buffer - but not all
at once (i.e. process one entry for each invocation)
Ack for other comments.
Thanks,
Shyam
>
>> if (!pdev->smart_pc_enabled)
>> @@ -165,20 +177,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, custom_bios_inputs_v1, 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, custom_bios_inputs, 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);
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-19 5:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07 11:01 [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size Shyam Sundar S K
2025-11-07 11:01 ` [PATCH v3 2/2] platform/x86/amd/pmf: Use ring buffer to store custom BIOS input values Shyam Sundar S K
2025-11-18 10:15 ` Ilpo Järvinen
2025-11-19 5:59 ` Shyam Sundar S K
2025-11-18 10:18 ` [PATCH v3 1/2] platform/x86/amd/pmf: Add BIOS_INPUTS_MAX macro to replace hardcoded array size Ilpo Järvinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).