* [PATCH v2 1/5] platform/x86/amd/pmf: add util layer and user-space misc device interface
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
@ 2025-11-11 7:10 ` Shyam Sundar S K
2025-11-11 7:10 ` [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL Shyam Sundar S K
` (5 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-11 7:10 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami, Shyam Sundar S K
Add a util layer to AMD PMF that exposes a minimal user-space interface
via a misc character device for metrics monitoring and feature discovery.
This creates /dev/amdpmf_interface with a basic ioctl to report support
for PMF features (Smart PC Builder, Auto Mode, Static Power Slider,
Dynamic Power Slider AC/DC), enabling smoother integration with
user-space tools such as AMD SystemDeck[1].
AMD SystemDeck is a widely used user-space tool for monitoring and
controlling power and thermal behavior. It helps designers keep major
components within their thermal limits to ensure proper operation.
Maintaining temperatures within specification enhances overall system
stability and reliability.
Link: https://docs.amd.com/v/u/en-US/68773_0.50 [1]
Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/Kconfig | 10 +++
drivers/platform/x86/amd/pmf/Makefile | 2 +
drivers/platform/x86/amd/pmf/core.c | 19 ++++++
drivers/platform/x86/amd/pmf/pmf.h | 10 +++
drivers/platform/x86/amd/pmf/util.c | 96 +++++++++++++++++++++++++++
include/uapi/linux/amd-pmf.h | 26 ++++++++
6 files changed, 163 insertions(+)
create mode 100644 drivers/platform/x86/amd/pmf/util.c
create mode 100644 include/uapi/linux/amd-pmf.h
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig
index 25b8f7ae3abd..1e1a76de52b6 100644
--- a/drivers/platform/x86/amd/pmf/Kconfig
+++ b/drivers/platform/x86/amd/pmf/Kconfig
@@ -30,3 +30,13 @@ config AMD_PMF_DEBUG
in the PMF config store.
Say Y here to enable more debug logs and Say N here if you are not sure.
+
+config AMD_PMF_UTIL_SUPPORT
+ bool "AMD PMF Util layer support"
+ depends on AMD_PMF
+ help
+ Enabling this option provides a character device for userspace to capture
+ PMF features (Smart PC Builder, Auto Mode, Static Power Slider, Dynamic
+ Power Slider AC/DC) and PMF metrics from the AMD PMF driver.
+
+ Say Y here to enable it and Say N here if you are not sure.
diff --git a/drivers/platform/x86/amd/pmf/Makefile b/drivers/platform/x86/amd/pmf/Makefile
index 5978464e0eb7..bf7aad80b9e9 100644
--- a/drivers/platform/x86/amd/pmf/Makefile
+++ b/drivers/platform/x86/amd/pmf/Makefile
@@ -8,3 +8,5 @@ obj-$(CONFIG_AMD_PMF) += amd-pmf.o
amd-pmf-y := core.o acpi.o sps.o \
auto-mode.o cnqf.o \
tee-if.o spc.o
+# Build util.c only when AMD_PMF_UTIL_SUPPORT is enabled
+amd-pmf-$(CONFIG_AMD_PMF_UTIL_SUPPORT) += util.o
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index bc544a4a5266..ac2b80db019e 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -53,6 +53,19 @@ static bool force_load;
module_param(force_load, bool, 0444);
MODULE_PARM_DESC(force_load, "Force load this driver on supported older platforms (experimental)");
+#if IS_ENABLED(CONFIG_AMD_PMF_UTIL_SUPPORT)
+struct amd_pmf_dev *pmf_handle;
+
+struct amd_pmf_dev *amd_pmf_get_handle(void)
+{
+ struct amd_pmf_dev *pmf = pmf_handle;
+
+ return pmf;
+}
+#else
+struct amd_pmf_dev *amd_pmf_get_handle(void) { return NULL; }
+#endif
+
static int amd_pmf_pwr_src_notify_call(struct notifier_block *nb, unsigned long event, void *data)
{
struct amd_pmf_dev *pmf = container_of(nb, struct amd_pmf_dev, pwr_src_notifier);
@@ -465,6 +478,10 @@ static int amd_pmf_probe(struct platform_device *pdev)
if (!dev->regbase)
return -ENOMEM;
+#if IS_ENABLED(CONFIG_AMD_PMF_UTIL_SUPPORT)
+ pmf_handle = dev;
+#endif
+
mutex_init(&dev->lock);
mutex_init(&dev->update_mutex);
mutex_init(&dev->cb_mutex);
@@ -477,6 +494,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
if (is_apmf_func_supported(dev, APMF_FUNC_SBIOS_HEARTBEAT_V2))
amd_pmf_notify_sbios_heartbeat_event_v2(dev, ON_LOAD);
+ amd_pmf_cdev_register();
dev_info(dev->dev, "registered PMF device successfully\n");
return 0;
@@ -489,6 +507,7 @@ static void amd_pmf_remove(struct platform_device *pdev)
amd_pmf_deinit_features(dev);
if (is_apmf_func_supported(dev, APMF_FUNC_SBIOS_HEARTBEAT_V2))
amd_pmf_notify_sbios_heartbeat_event_v2(dev, ON_UNLOAD);
+ amd_pmf_cdev_unregister();
apmf_acpi_deinit(dev);
amd_pmf_dbgfs_unregister(dev);
mutex_destroy(&dev->lock);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index bd19f2a6bc78..3b1aae8a60a5 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -823,6 +823,7 @@ struct ta_pmf_shared_memory {
};
/* Core Layer */
+struct amd_pmf_dev *amd_pmf_get_handle(void);
int apmf_acpi_init(struct amd_pmf_dev *pmf_dev);
void apmf_acpi_deinit(struct amd_pmf_dev *pmf_dev);
int is_apmf_func_supported(struct amd_pmf_dev *pdev, unsigned long index);
@@ -888,4 +889,13 @@ void amd_pmf_populate_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_tab
void amd_pmf_dump_ta_inputs(struct amd_pmf_dev *dev, struct ta_pmf_enact_table *in);
int amd_pmf_invoke_cmd_enact(struct amd_pmf_dev *dev);
+/* Util Layer */
+#if IS_ENABLED(CONFIG_AMD_PMF_UTIL_SUPPORT)
+int amd_pmf_cdev_register(void);
+void amd_pmf_cdev_unregister(void);
+#else
+static inline int amd_pmf_cdev_register(void) { return 0; }
+static inline void amd_pmf_cdev_unregister(void) {}
+#endif
+
#endif /* PMF_H */
diff --git a/drivers/platform/x86/amd/pmf/util.c b/drivers/platform/x86/amd/pmf/util.c
new file mode 100644
index 000000000000..97a626bac034
--- /dev/null
+++ b/drivers/platform/x86/amd/pmf/util.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD Platform Management Framework Util Layer
+ *
+ * Copyright (c) 2025, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ * Sanket Goswami <Sanket.Goswami@amd.com>
+ */
+
+#include <linux/amd-pmf.h>
+#include <linux/miscdevice.h>
+#include <linux/uaccess.h>
+
+#include "pmf.h"
+
+#define AMD_PMF_FEATURE_VERSION 0x02
+
+static int amd_pmf_get_feature_status(void __user *argp, unsigned long feat_id)
+{
+ struct amd_pmf_feature_support info = {0};
+ struct amd_pmf_dev *pdev;
+
+ pdev = amd_pmf_get_handle();
+ if (!pdev)
+ return -EINVAL;
+
+ switch (feat_id) {
+ case PMF_FEATURE_AUTO_MODE:
+ info.feature_supported = is_apmf_func_supported(pdev, APMF_FUNC_AUTO_MODE);
+ break;
+ case PMF_FEATURE_STATIC_POWER_SLIDER:
+ info.feature_supported = is_apmf_func_supported(pdev,
+ APMF_FUNC_STATIC_SLIDER_GRANULAR);
+ break;
+ case PMF_FEATURE_POLICY_BUILDER:
+ info.feature_supported = pdev->smart_pc_enabled;
+ break;
+ case PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC:
+ info.feature_supported = is_apmf_func_supported(pdev, APMF_FUNC_DYN_SLIDER_AC);
+ break;
+ case PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC:
+ info.feature_supported = is_apmf_func_supported(pdev, APMF_FUNC_DYN_SLIDER_DC);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ info.feature_version = AMD_PMF_FEATURE_VERSION;
+ if (copy_to_user(argp, &info, sizeof(info)))
+ return -EFAULT;
+
+ return 0;
+}
+
+static long amd_pmf_set_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ struct amd_pmf_feature_support output;
+
+ if (copy_from_user(&output, argp, sizeof(output)))
+ return -EFAULT;
+
+ switch (cmd) {
+ case IOCTL_PMF_QUERY_FEATURE_SUPPORT:
+ return amd_pmf_get_feature_status(argp, output.feature_id);
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct file_operations pmf_if_ops = {
+ .unlocked_ioctl = amd_pmf_set_ioctl,
+};
+
+#if IS_ENABLED(CONFIG_AMD_PMF_UTIL_SUPPORT)
+static struct miscdevice amd_pmf_util_if = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "amdpmf_interface",
+ .fops = &pmf_if_ops,
+};
+
+int amd_pmf_cdev_register(void)
+{
+ return misc_register(&amd_pmf_util_if);
+}
+
+void amd_pmf_cdev_unregister(void)
+{
+ misc_deregister(&amd_pmf_util_if);
+}
+#else
+int amd_pmf_cdev_register(void) { return 0; }
+void amd_pmf_cdev_unregister(void) {}
+#endif
diff --git a/include/uapi/linux/amd-pmf.h b/include/uapi/linux/amd-pmf.h
new file mode 100644
index 000000000000..2147240c2bb1
--- /dev/null
+++ b/include/uapi/linux/amd-pmf.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later WITH Linux-syscall-note */
+
+#ifndef _UAPI_LINUX_AMD_PMF_H
+#define _UAPI_LINUX_AMD_PMF_H
+
+#include <linux/types.h>
+
+#define AMD_PMF_IOC_MAGIC 'p'
+
+#define IOCTL_PMF_QUERY_FEATURE_SUPPORT _IOWR(AMD_PMF_IOC_MAGIC, 0x00, \
+ struct amd_pmf_feature_support)
+
+/* AMD PMF Feature IDs */
+#define PMF_FEATURE_AUTO_MODE 0
+#define PMF_FEATURE_STATIC_POWER_SLIDER 2
+#define PMF_FEATURE_POLICY_BUILDER 4
+#define PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC 5
+#define PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC 6
+
+struct amd_pmf_feature_support {
+ bool feature_supported; /* Out */
+ unsigned long feature_version; /* Out */
+ unsigned long feature_id; /* In */
+};
+
+#endif /* _UAPI_LINUX_AMD_PMF_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
2025-11-11 7:10 ` [PATCH v2 1/5] platform/x86/amd/pmf: add util layer and user-space misc device interface Shyam Sundar S K
@ 2025-11-11 7:10 ` Shyam Sundar S K
2025-11-18 16:18 ` Ilpo Järvinen
2025-11-11 7:10 ` [PATCH v2 3/5] Documentation/ABI: add testing entry for AMD PMF misc device interface Shyam Sundar S K
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-11 7:10 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami, Shyam Sundar S K
Add a bios_output[] cache to amd_pmf_dev and store the latest values for
BIOS output policies when applying PMF policies. This enables the AMD PMF
util layer to expose these BIOS outputs alongside selected thermal and
power telemetry to user space via /dev/amdpmf_interface and a new IOCTL,
supporting real-time monitoring tools such as SystemDeck.
Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 1 +
drivers/platform/x86/amd/pmf/tee-if.c | 10 ++
drivers/platform/x86/amd/pmf/util.c | 140 ++++++++++++++++++++++++++
include/uapi/linux/amd-pmf.h | 48 +++++++++
4 files changed, 199 insertions(+)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 3b1aae8a60a5..5880d6fc3afb 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -406,6 +406,7 @@ 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 */
+ u32 bios_output[10];
};
struct apmf_sps_prop_granular_v2 {
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 6e8116bef4f6..8ea309aade95 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -183,42 +183,52 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
break;
case PMF_POLICY_BIOS_OUTPUT_1:
+ dev->bios_output[0] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(0), 0);
break;
case PMF_POLICY_BIOS_OUTPUT_2:
+ dev->bios_output[1] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(1), 1);
break;
case PMF_POLICY_BIOS_OUTPUT_3:
+ dev->bios_output[2] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(2), 2);
break;
case PMF_POLICY_BIOS_OUTPUT_4:
+ dev->bios_output[3] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(3), 3);
break;
case PMF_POLICY_BIOS_OUTPUT_5:
+ dev->bios_output[4] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(4), 4);
break;
case PMF_POLICY_BIOS_OUTPUT_6:
+ dev->bios_output[5] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(5), 5);
break;
case PMF_POLICY_BIOS_OUTPUT_7:
+ dev->bios_output[6] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(6), 6);
break;
case PMF_POLICY_BIOS_OUTPUT_8:
+ dev->bios_output[7] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(7), 7);
break;
case PMF_POLICY_BIOS_OUTPUT_9:
+ dev->bios_output[8] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(8), 8);
break;
case PMF_POLICY_BIOS_OUTPUT_10:
+ dev->bios_output[9] = val;
amd_pmf_smartpc_apply_bios_output(dev, val, BIT(9), 9);
break;
}
diff --git a/drivers/platform/x86/amd/pmf/util.c b/drivers/platform/x86/amd/pmf/util.c
index 97a626bac034..bfb30117a5af 100644
--- a/drivers/platform/x86/amd/pmf/util.c
+++ b/drivers/platform/x86/amd/pmf/util.c
@@ -17,6 +17,143 @@
#define AMD_PMF_FEATURE_VERSION 0x02
+static int amd_pmf_get_metrics_data(struct device *dev, void __user *argp)
+{
+ struct ta_pmf_shared_memory *ta_sm = NULL;
+ struct ta_pmf_enact_table *in = NULL;
+ struct amd_pmf_metrics_key output;
+ struct amd_pmf_dev *pdev;
+
+ pdev = amd_pmf_get_handle();
+ if (!pdev)
+ return -EINVAL;
+
+ memset(pdev->shbuf, 0, pdev->policy_sz);
+ ta_sm = pdev->shbuf;
+ in = &ta_sm->pmf_input.enact_table;
+
+ if (copy_from_user(&output, argp, sizeof(output)))
+ return -EFAULT;
+
+ switch (output.control_code) {
+ case IOCTL_POWER_SOURCE:
+ output.val = in->ev_info.power_source;
+ break;
+ case IOCTL_POWER_SLIDER_POSITION:
+ output.val = in->ev_info.power_slider;
+ break;
+ case IOCTL_PLATFORM_TYPE:
+ output.val = in->ev_info.platform_type;
+ break;
+ case IOCTL_LAPTOP_PLACEMENT:
+ output.val = in->ev_info.device_state;
+ break;
+ case IOCTL_LID_STATE:
+ output.val = in->ev_info.lid_state;
+ break;
+ case IOCTL_SKIN_TEMP:
+ output.val = in->ev_info.skin_temperature / 100;
+ break;
+ case IOCTL_USER_PRESENCE:
+ output.val = in->ev_info.user_present;
+ break;
+ case IOCTL_BATTERY_PERCENTAGE:
+ output.val = in->ev_info.bat_percentage;
+ break;
+ case IOCTL_BIOS_INPUT_1:
+ output.val = in->ev_info.bios_input_1[0];
+ break;
+ case IOCTL_BIOS_INPUT_2:
+ output.val = in->ev_info.bios_input_1[1];
+ break;
+ case IOCTL_BIOS_INPUT_3:
+ output.val = in->ev_info.bios_input_2[0];
+ break;
+ case IOCTL_BIOS_INPUT_4:
+ output.val = in->ev_info.bios_input_2[1];
+ break;
+ case IOCTL_BIOS_INPUT_5:
+ output.val = in->ev_info.bios_input_2[2];
+ break;
+ case IOCTL_BIOS_INPUT_6:
+ output.val = in->ev_info.bios_input_2[3];
+ break;
+ case IOCTL_BIOS_INPUT_7:
+ output.val = in->ev_info.bios_input_2[4];
+ break;
+ case IOCTL_BIOS_INPUT_8:
+ output.val = in->ev_info.bios_input_2[5];
+ break;
+ case IOCTL_BIOS_INPUT_9:
+ output.val = in->ev_info.bios_input_2[6];
+ break;
+ case IOCTL_BIOS_INPUT_10:
+ output.val = in->ev_info.bios_input_2[7];
+ break;
+ case IOCTL_GFX_WORKLOAD:
+ output.val = in->ev_info.gfx_busy;
+ break;
+ case IOCTL_DESIGNED_BATTERY_CAPACITY:
+ output.val = in->ev_info.bat_design;
+ break;
+ case IOCTL_FULLY_CHARGED_BATTERY_CAPACITY:
+ output.val = in->ev_info.full_charge_capacity;
+ break;
+ case IOCTL_BATTERY_DRAIN_RATE:
+ output.val = in->ev_info.drain_rate;
+ break;
+ case IOCTL_AMBIENT_LIGHT:
+ output.val = in->ev_info.ambient_light;
+ break;
+ case IOCTL_AVG_C0_RES:
+ output.val = in->ev_info.avg_c0residency;
+ break;
+ case IOCTL_MAX_C0_RES:
+ output.val = in->ev_info.max_c0residency;
+ break;
+ case IOCTL_SOCKET_POWER:
+ output.val = in->ev_info.socket_power;
+ break;
+ case IOCTL_BIOS_OUTPUT_1:
+ output.val = pdev->bios_output[0];
+ break;
+ case IOCTL_BIOS_OUTPUT_2:
+ output.val = pdev->bios_output[1];
+ break;
+ case IOCTL_BIOS_OUTPUT_3:
+ output.val = pdev->bios_output[2];
+ break;
+ case IOCTL_BIOS_OUTPUT_4:
+ output.val = pdev->bios_output[3];
+ break;
+ case IOCTL_BIOS_OUTPUT_5:
+ output.val = pdev->bios_output[4];
+ break;
+ case IOCTL_BIOS_OUTPUT_6:
+ output.val = pdev->bios_output[5];
+ break;
+ case IOCTL_BIOS_OUTPUT_7:
+ output.val = pdev->bios_output[6];
+ break;
+ case IOCTL_BIOS_OUTPUT_8:
+ output.val = pdev->bios_output[7];
+ break;
+ case IOCTL_BIOS_OUTPUT_9:
+ output.val = pdev->bios_output[8];
+ break;
+ case IOCTL_BIOS_OUTPUT_10:
+ output.val = pdev->bios_output[9];
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (copy_to_user(argp, &output, sizeof(output)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int amd_pmf_get_feature_status(void __user *argp, unsigned long feat_id)
{
struct amd_pmf_feature_support info = {0};
@@ -56,6 +193,7 @@ static int amd_pmf_get_feature_status(void __user *argp, unsigned long feat_id)
static long amd_pmf_set_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
+ struct device *dev = filp->private_data;
void __user *argp = (void __user *)arg;
struct amd_pmf_feature_support output;
@@ -65,6 +203,8 @@ static long amd_pmf_set_ioctl(struct file *filp, unsigned int cmd, unsigned long
switch (cmd) {
case IOCTL_PMF_QUERY_FEATURE_SUPPORT:
return amd_pmf_get_feature_status(argp, output.feature_id);
+ case IOCTL_PMF_GET_METRICS_DATA:
+ return amd_pmf_get_metrics_data(dev, argp);
default:
return -EINVAL;
}
diff --git a/include/uapi/linux/amd-pmf.h b/include/uapi/linux/amd-pmf.h
index 2147240c2bb1..bbc45c6fc113 100644
--- a/include/uapi/linux/amd-pmf.h
+++ b/include/uapi/linux/amd-pmf.h
@@ -9,6 +9,7 @@
#define IOCTL_PMF_QUERY_FEATURE_SUPPORT _IOWR(AMD_PMF_IOC_MAGIC, 0x00, \
struct amd_pmf_feature_support)
+#define IOCTL_PMF_GET_METRICS_DATA _IOWR(AMD_PMF_IOC_MAGIC, 0x26, struct amd_pmf_metrics_key)
/* AMD PMF Feature IDs */
#define PMF_FEATURE_AUTO_MODE 0
@@ -17,6 +18,53 @@
#define PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC 5
#define PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC 6
+enum pmf_metrics_id {
+ IOCTL_POWER_SOURCE,
+ IOCTL_POWER_SLIDER_POSITION,
+ IOCTL_PLATFORM_TYPE,
+ IOCTL_LAPTOP_PLACEMENT,
+ IOCTL_LID_STATE,
+ IOCTL_HETERO_STATE,
+ IOCTL_SKIN_TEMP,
+ IOCTL_USER_PRESENCE,
+ IOCTL_DISPLAY_NUM,
+ IOCTL_BATTERY_PERCENTAGE,
+ IOCTL_BIOS_INPUT_1,
+ IOCTL_BIOS_INPUT_2,
+ IOCTL_BIOS_INPUT_3,
+ IOCTL_BIOS_INPUT_4,
+ IOCTL_BIOS_INPUT_5,
+ IOCTL_BIOS_INPUT_6,
+ IOCTL_BIOS_INPUT_7,
+ IOCTL_BIOS_INPUT_8,
+ IOCTL_BIOS_INPUT_9,
+ IOCTL_BIOS_INPUT_10,
+ IOCTL_GFX_WORKLOAD,
+ IOCTL_DESIGNED_BATTERY_CAPACITY = 24,
+ IOCTL_FULLY_CHARGED_BATTERY_CAPACITY,
+ IOCTL_BATTERY_DRAIN_RATE,
+ IOCTL_AMBIENT_LIGHT = 29,
+ IOCTL_AVG_C0_RES = 36,
+ IOCTL_MAX_C0_RES,
+ IOCTL_SOCKET_POWER = 50,
+ IOCTL_TA_BIN_VER,
+ IOCTL_BIOS_OUTPUT_1,
+ IOCTL_BIOS_OUTPUT_2,
+ IOCTL_BIOS_OUTPUT_3,
+ IOCTL_BIOS_OUTPUT_4,
+ IOCTL_BIOS_OUTPUT_5,
+ IOCTL_BIOS_OUTPUT_6,
+ IOCTL_BIOS_OUTPUT_7,
+ IOCTL_BIOS_OUTPUT_8,
+ IOCTL_BIOS_OUTPUT_9,
+ IOCTL_BIOS_OUTPUT_10,
+};
+
+struct amd_pmf_metrics_key {
+ enum pmf_metrics_id control_code; /* In */
+ long long val; /* Out */
+};
+
struct amd_pmf_feature_support {
bool feature_supported; /* Out */
unsigned long feature_version; /* Out */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL
2025-11-11 7:10 ` [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL Shyam Sundar S K
@ 2025-11-18 16:18 ` Ilpo Järvinen
2025-11-26 9:36 ` Shyam Sundar S K
0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2025-11-18 16:18 UTC (permalink / raw)
To: Shyam Sundar S K
Cc: Hans de Goede, platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami
On Tue, 11 Nov 2025, Shyam Sundar S K wrote:
> Add a bios_output[] cache to amd_pmf_dev and store the latest values for
> BIOS output policies when applying PMF policies. This enables the AMD PMF
> util layer to expose these BIOS outputs alongside selected thermal and
> power telemetry to user space via /dev/amdpmf_interface and a new IOCTL,
> supporting real-time monitoring tools such as SystemDeck.
>
> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> drivers/platform/x86/amd/pmf/pmf.h | 1 +
> drivers/platform/x86/amd/pmf/tee-if.c | 10 ++
> drivers/platform/x86/amd/pmf/util.c | 140 ++++++++++++++++++++++++++
> include/uapi/linux/amd-pmf.h | 48 +++++++++
> 4 files changed, 199 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 3b1aae8a60a5..5880d6fc3afb 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -406,6 +406,7 @@ 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 */
> + u32 bios_output[10];
> };
>
> struct apmf_sps_prop_granular_v2 {
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..8ea309aade95 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -183,42 +183,52 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_1:
> + dev->bios_output[0] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(0), 0);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_2:
> + dev->bios_output[1] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(1), 1);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_3:
> + dev->bios_output[2] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(2), 2);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_4:
> + dev->bios_output[3] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(3), 3);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_5:
> + dev->bios_output[4] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(4), 4);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_6:
> + dev->bios_output[5] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(5), 5);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_7:
> + dev->bios_output[6] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(6), 6);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_8:
> + dev->bios_output[7] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(7), 7);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_9:
> + dev->bios_output[8] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(8), 8);
> break;
>
> case PMF_POLICY_BIOS_OUTPUT_10:
> + dev->bios_output[9] = val;
> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(9), 9);
> break;
Duplicating that n times doesn't look nice...
> }
> diff --git a/drivers/platform/x86/amd/pmf/util.c b/drivers/platform/x86/amd/pmf/util.c
> index 97a626bac034..bfb30117a5af 100644
> --- a/drivers/platform/x86/amd/pmf/util.c
> +++ b/drivers/platform/x86/amd/pmf/util.c
> @@ -17,6 +17,143 @@
>
> #define AMD_PMF_FEATURE_VERSION 0x02
>
> +static int amd_pmf_get_metrics_data(struct device *dev, void __user *argp)
> +{
> + struct ta_pmf_shared_memory *ta_sm = NULL;
> + struct ta_pmf_enact_table *in = NULL;
> + struct amd_pmf_metrics_key output;
> + struct amd_pmf_dev *pdev;
> +
> + pdev = amd_pmf_get_handle();
> + if (!pdev)
> + return -EINVAL;
> +
> + memset(pdev->shbuf, 0, pdev->policy_sz);
> + ta_sm = pdev->shbuf;
> + in = &ta_sm->pmf_input.enact_table;
> +
> + if (copy_from_user(&output, argp, sizeof(output)))
> + return -EFAULT;
> +
> + switch (output.control_code) {
> + case IOCTL_POWER_SOURCE:
> + output.val = in->ev_info.power_source;
> + break;
> + case IOCTL_POWER_SLIDER_POSITION:
> + output.val = in->ev_info.power_slider;
> + break;
> + case IOCTL_PLATFORM_TYPE:
> + output.val = in->ev_info.platform_type;
> + break;
> + case IOCTL_LAPTOP_PLACEMENT:
> + output.val = in->ev_info.device_state;
> + break;
> + case IOCTL_LID_STATE:
> + output.val = in->ev_info.lid_state;
> + break;
> + case IOCTL_SKIN_TEMP:
> + output.val = in->ev_info.skin_temperature / 100;
> + break;
> + case IOCTL_USER_PRESENCE:
> + output.val = in->ev_info.user_present;
> + break;
> + case IOCTL_BATTERY_PERCENTAGE:
> + output.val = in->ev_info.bat_percentage;
> + break;
> + case IOCTL_BIOS_INPUT_1:
> + output.val = in->ev_info.bios_input_1[0];
> + break;
> + case IOCTL_BIOS_INPUT_2:
> + output.val = in->ev_info.bios_input_1[1];
> + break;
> + case IOCTL_BIOS_INPUT_3:
> + output.val = in->ev_info.bios_input_2[0];
> + break;
> + case IOCTL_BIOS_INPUT_4:
> + output.val = in->ev_info.bios_input_2[1];
> + break;
> + case IOCTL_BIOS_INPUT_5:
> + output.val = in->ev_info.bios_input_2[2];
> + break;
> + case IOCTL_BIOS_INPUT_6:
> + output.val = in->ev_info.bios_input_2[3];
> + break;
> + case IOCTL_BIOS_INPUT_7:
> + output.val = in->ev_info.bios_input_2[4];
> + break;
> + case IOCTL_BIOS_INPUT_8:
> + output.val = in->ev_info.bios_input_2[5];
> + break;
> + case IOCTL_BIOS_INPUT_9:
> + output.val = in->ev_info.bios_input_2[6];
> + break;
> + case IOCTL_BIOS_INPUT_10:
> + output.val = in->ev_info.bios_input_2[7];
> + break;
> + case IOCTL_GFX_WORKLOAD:
> + output.val = in->ev_info.gfx_busy;
> + break;
> + case IOCTL_DESIGNED_BATTERY_CAPACITY:
> + output.val = in->ev_info.bat_design;
> + break;
> + case IOCTL_FULLY_CHARGED_BATTERY_CAPACITY:
> + output.val = in->ev_info.full_charge_capacity;
> + break;
We do have other interfaces to expose e.g., battery information.
I'm far from convinced ioctl is a great idea here anyway.
> + case IOCTL_BATTERY_DRAIN_RATE:
> + output.val = in->ev_info.drain_rate;
> + break;
> + case IOCTL_AMBIENT_LIGHT:
> + output.val = in->ev_info.ambient_light;
> + break;
> + case IOCTL_AVG_C0_RES:
> + output.val = in->ev_info.avg_c0residency;
> + break;
> + case IOCTL_MAX_C0_RES:
> + output.val = in->ev_info.max_c0residency;
> + break;
> + case IOCTL_SOCKET_POWER:
> + output.val = in->ev_info.socket_power;
> + break;
> + case IOCTL_BIOS_OUTPUT_1:
> + output.val = pdev->bios_output[0];
> + break;
> + case IOCTL_BIOS_OUTPUT_2:
> + output.val = pdev->bios_output[1];
> + break;
> + case IOCTL_BIOS_OUTPUT_3:
> + output.val = pdev->bios_output[2];
> + break;
> + case IOCTL_BIOS_OUTPUT_4:
> + output.val = pdev->bios_output[3];
> + break;
> + case IOCTL_BIOS_OUTPUT_5:
> + output.val = pdev->bios_output[4];
> + break;
> + case IOCTL_BIOS_OUTPUT_6:
> + output.val = pdev->bios_output[5];
> + break;
> + case IOCTL_BIOS_OUTPUT_7:
> + output.val = pdev->bios_output[6];
> + break;
> + case IOCTL_BIOS_OUTPUT_8:
> + output.val = pdev->bios_output[7];
> + break;
> + case IOCTL_BIOS_OUTPUT_9:
> + output.val = pdev->bios_output[8];
> + break;
> + case IOCTL_BIOS_OUTPUT_10:
> + output.val = pdev->bios_output[9];
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + if (copy_to_user(argp, &output, sizeof(output)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
> static int amd_pmf_get_feature_status(void __user *argp, unsigned long feat_id)
> {
> struct amd_pmf_feature_support info = {0};
> @@ -56,6 +193,7 @@ static int amd_pmf_get_feature_status(void __user *argp, unsigned long feat_id)
>
> static long amd_pmf_set_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> {
> + struct device *dev = filp->private_data;
> void __user *argp = (void __user *)arg;
> struct amd_pmf_feature_support output;
>
> @@ -65,6 +203,8 @@ static long amd_pmf_set_ioctl(struct file *filp, unsigned int cmd, unsigned long
> switch (cmd) {
> case IOCTL_PMF_QUERY_FEATURE_SUPPORT:
> return amd_pmf_get_feature_status(argp, output.feature_id);
> + case IOCTL_PMF_GET_METRICS_DATA:
> + return amd_pmf_get_metrics_data(dev, argp);
> default:
> return -EINVAL;
> }
> diff --git a/include/uapi/linux/amd-pmf.h b/include/uapi/linux/amd-pmf.h
> index 2147240c2bb1..bbc45c6fc113 100644
> --- a/include/uapi/linux/amd-pmf.h
> +++ b/include/uapi/linux/amd-pmf.h
> @@ -9,6 +9,7 @@
>
> #define IOCTL_PMF_QUERY_FEATURE_SUPPORT _IOWR(AMD_PMF_IOC_MAGIC, 0x00, \
> struct amd_pmf_feature_support)
> +#define IOCTL_PMF_GET_METRICS_DATA _IOWR(AMD_PMF_IOC_MAGIC, 0x26, struct amd_pmf_metrics_key)
>
> /* AMD PMF Feature IDs */
> #define PMF_FEATURE_AUTO_MODE 0
> @@ -17,6 +18,53 @@
> #define PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC 5
> #define PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC 6
>
> +enum pmf_metrics_id {
> + IOCTL_POWER_SOURCE,
> + IOCTL_POWER_SLIDER_POSITION,
> + IOCTL_PLATFORM_TYPE,
> + IOCTL_LAPTOP_PLACEMENT,
> + IOCTL_LID_STATE,
> + IOCTL_HETERO_STATE,
> + IOCTL_SKIN_TEMP,
> + IOCTL_USER_PRESENCE,
> + IOCTL_DISPLAY_NUM,
> + IOCTL_BATTERY_PERCENTAGE,
> + IOCTL_BIOS_INPUT_1,
> + IOCTL_BIOS_INPUT_2,
> + IOCTL_BIOS_INPUT_3,
> + IOCTL_BIOS_INPUT_4,
> + IOCTL_BIOS_INPUT_5,
> + IOCTL_BIOS_INPUT_6,
> + IOCTL_BIOS_INPUT_7,
> + IOCTL_BIOS_INPUT_8,
> + IOCTL_BIOS_INPUT_9,
> + IOCTL_BIOS_INPUT_10,
> + IOCTL_GFX_WORKLOAD,
> + IOCTL_DESIGNED_BATTERY_CAPACITY = 24,
> + IOCTL_FULLY_CHARGED_BATTERY_CAPACITY,
> + IOCTL_BATTERY_DRAIN_RATE,
> + IOCTL_AMBIENT_LIGHT = 29,
> + IOCTL_AVG_C0_RES = 36,
> + IOCTL_MAX_C0_RES,
> + IOCTL_SOCKET_POWER = 50,
> + IOCTL_TA_BIN_VER,
> + IOCTL_BIOS_OUTPUT_1,
> + IOCTL_BIOS_OUTPUT_2,
> + IOCTL_BIOS_OUTPUT_3,
> + IOCTL_BIOS_OUTPUT_4,
> + IOCTL_BIOS_OUTPUT_5,
> + IOCTL_BIOS_OUTPUT_6,
> + IOCTL_BIOS_OUTPUT_7,
> + IOCTL_BIOS_OUTPUT_8,
> + IOCTL_BIOS_OUTPUT_9,
> + IOCTL_BIOS_OUTPUT_10,
> +};
> +
> +struct amd_pmf_metrics_key {
> + enum pmf_metrics_id control_code; /* In */
> + long long val; /* Out */
Comments should be aligned.
> +};
> +
> struct amd_pmf_feature_support {
> bool feature_supported; /* Out */
> unsigned long feature_version; /* Out */
>
--
i.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL
2025-11-18 16:18 ` Ilpo Järvinen
@ 2025-11-26 9:36 ` Shyam Sundar S K
0 siblings, 0 replies; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-26 9:36 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Hans de Goede, platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami
On 11/18/2025 21:48, Ilpo Järvinen wrote:
> On Tue, 11 Nov 2025, Shyam Sundar S K wrote:
>
>> Add a bios_output[] cache to amd_pmf_dev and store the latest values for
>> BIOS output policies when applying PMF policies. This enables the AMD PMF
>> util layer to expose these BIOS outputs alongside selected thermal and
>> power telemetry to user space via /dev/amdpmf_interface and a new IOCTL,
>> supporting real-time monitoring tools such as SystemDeck.
>>
>> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>> drivers/platform/x86/amd/pmf/pmf.h | 1 +
>> drivers/platform/x86/amd/pmf/tee-if.c | 10 ++
>> drivers/platform/x86/amd/pmf/util.c | 140 ++++++++++++++++++++++++++
>> include/uapi/linux/amd-pmf.h | 48 +++++++++
>> 4 files changed, 199 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 3b1aae8a60a5..5880d6fc3afb 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -406,6 +406,7 @@ 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 */
>> + u32 bios_output[10];
>> };
>>
>> struct apmf_sps_prop_granular_v2 {
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 6e8116bef4f6..8ea309aade95 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -183,42 +183,52 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_1:
>> + dev->bios_output[0] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(0), 0);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_2:
>> + dev->bios_output[1] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(1), 1);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_3:
>> + dev->bios_output[2] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(2), 2);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_4:
>> + dev->bios_output[3] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(3), 3);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_5:
>> + dev->bios_output[4] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(4), 4);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_6:
>> + dev->bios_output[5] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(5), 5);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_7:
>> + dev->bios_output[6] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(6), 6);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_8:
>> + dev->bios_output[7] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(7), 7);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_9:
>> + dev->bios_output[8] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(8), 8);
>> break;
>>
>> case PMF_POLICY_BIOS_OUTPUT_10:
>> + dev->bios_output[9] = val;
>> amd_pmf_smartpc_apply_bios_output(dev, val, BIT(9), 9);
>> break;
>
> Duplicating that n times doesn't look nice...
Thank you for the feedback. I have sent a preparatory patch to it
which unifies the handling of PMF_POLICY_BIOS_OUTPUT_*. Once this
change lands, the current patch (bios_output[] caching ) should be
much simpler.
Thanks,
Shyam
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/5] Documentation/ABI: add testing entry for AMD PMF misc device interface
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
2025-11-11 7:10 ` [PATCH v2 1/5] platform/x86/amd/pmf: add util layer and user-space misc device interface Shyam Sundar S K
2025-11-11 7:10 ` [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL Shyam Sundar S K
@ 2025-11-11 7:10 ` Shyam Sundar S K
2025-11-11 7:10 ` [PATCH v2 4/5] platform/x86/amd/pmf: Store commonly used enums in the header file Shyam Sundar S K
` (3 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-11 7:10 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami, Shyam Sundar S K
Add a Documentation/ABI/testing entry describing the AMD PMF util layer
misc device (/dev/amdpmf_interface) and the initial ioctls used to query
feature support and metrics data information. This interface is available
when CONFIG_AMD_PMF_UTIL_SUPPORT=y.
Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
.../ABI/testing/misc-amdpmf_interface | 49 +++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 50 insertions(+)
create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
diff --git a/Documentation/ABI/testing/misc-amdpmf_interface b/Documentation/ABI/testing/misc-amdpmf_interface
new file mode 100644
index 000000000000..2057f8e5bb1d
--- /dev/null
+++ b/Documentation/ABI/testing/misc-amdpmf_interface
@@ -0,0 +1,49 @@
+What: /dev/amdpmf_interface
+Date: November 2025
+KernelVersion: 6.20
+Contact: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+Description:
+ The AMD Platform Management Framework (PMF) util layer exposes a
+ minimal user-space interface via a misc character device for
+ feature discovery and metrics monitoring.
+
+ When CONFIG_AMD_PMF_UTIL_SUPPORT is enabled, the driver creates
+ a misc device:
+
+ ======================
+ /dev/amdpmf_interface
+ ======================
+
+ The interface currently supports following ioctls:
+
+ ========================================== =========================================
+ IOCTL Usage
+ IOCTL_PMF_QUERY_FEATURE_SUPPORT (value: 0) User passes a struct amd_pmf_feature_support with feature_id set.
+ The driver returns whether the feature is supported and a version.
+
+ IOCTL_PMF_GET_METRICS_DATA (value: 38) User passes a struct amd_pmf_feature_support with feature_id set.
+ The driver returns the requested metrics data.
+ ========================================== =========================================
+
+ Following are the IOCTL codes supported currently:
+
+ ============ =========================================
+ Feature ID IOCTL Support
+ 0 PMF_FEATURE_AUTO_MODE
+ 2 PMF_FEATURE_STATIC_POWER_SLIDER
+ 4 PMF_FEATURE_POLICY_BUILDER (Smart PC)
+ 5 PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC
+ 6 PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC
+ ============ =========================================
+
+ Following are the return codes:
+
+ ============= =========================================
+ Return code Description
+ 0 success
+ EINVAL feature_id is not recognized, or handle is not available
+ EFAULT copy_to_user/copy_from_user failures
+ ============= =========================================
+
+ User-space tools integrating with AMD PMF to discover capabilities
+ and monitor metrics data.
diff --git a/MAINTAINERS b/MAINTAINERS
index ddecf1ef3bed..4fd7dfa39e1d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1201,6 +1201,7 @@ AMD PMF DRIVER
M: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
L: platform-driver-x86@vger.kernel.org
S: Supported
+F: Documentation/ABI/testing/misc-amdpmf_interface
F: Documentation/ABI/testing/sysfs-amd-pmf
F: drivers/platform/x86/amd/pmf/
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 4/5] platform/x86/amd/pmf: Store commonly used enums in the header file
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
` (2 preceding siblings ...)
2025-11-11 7:10 ` [PATCH v2 3/5] Documentation/ABI: add testing entry for AMD PMF misc device interface Shyam Sundar S K
@ 2025-11-11 7:10 ` Shyam Sundar S K
2025-11-11 7:10 ` [PATCH v2 5/5] platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver metrics and features Shyam Sundar S K
` (2 subsequent siblings)
6 siblings, 0 replies; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-11 7:10 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami, Shyam Sundar S K
Relocate commonly used enums from multiple source files into a shared
header file to simplify code structure, improve readability, and
enhance maintainability.
Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
drivers/platform/x86/amd/pmf/pmf.h | 22 ----------------------
drivers/platform/x86/amd/pmf/spc.c | 1 +
include/uapi/linux/amd-pmf.h | 22 ++++++++++++++++++++++
3 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 5880d6fc3afb..95121d173b41 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -645,14 +645,6 @@ enum system_state {
SYSTEM_STATE_MAX,
};
-enum ta_slider {
- TA_BEST_BATTERY,
- TA_BETTER_BATTERY,
- TA_BETTER_PERFORMANCE,
- TA_BEST_PERFORMANCE,
- TA_MAX,
-};
-
struct amd_pmf_pb_bitmap {
const char *name;
u32 bit_mask;
@@ -684,20 +676,6 @@ static const struct amd_pmf_pb_bitmap custom_bios_inputs_v1[] __used = {
{"NOTIFY_CUSTOM_BIOS_INPUT10", BIT(16)},
};
-enum platform_type {
- PTYPE_UNKNOWN = 0,
- LID_CLOSE,
- CLAMSHELL,
- FLAT,
- TENT,
- STAND,
- TABLET,
- BOOK,
- PRESENTATION,
- PULL_FWD,
- PTYPE_INVALID = 0xf,
-};
-
/* Command ids for TA communication */
enum ta_pmf_command {
TA_PMF_COMMAND_POLICY_BUILDER_INITIALIZE,
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 85192c7536b8..a2745b4a5717 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -10,6 +10,7 @@
*/
#include <acpi/button.h>
+#include <linux/amd-pmf.h>
#include <linux/amd-pmf-io.h>
#include <linux/power_supply.h>
#include <linux/units.h>
diff --git a/include/uapi/linux/amd-pmf.h b/include/uapi/linux/amd-pmf.h
index bbc45c6fc113..b47eb45196f7 100644
--- a/include/uapi/linux/amd-pmf.h
+++ b/include/uapi/linux/amd-pmf.h
@@ -18,6 +18,28 @@
#define PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC 5
#define PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC 6
+enum ta_slider {
+ TA_BEST_BATTERY,
+ TA_BETTER_BATTERY,
+ TA_BETTER_PERFORMANCE,
+ TA_BEST_PERFORMANCE,
+ TA_MAX,
+};
+
+enum platform_type {
+ PTYPE_UNKNOWN = 0,
+ LID_CLOSE,
+ CLAMSHELL,
+ FLAT,
+ TENT,
+ STAND,
+ TABLET,
+ BOOK,
+ PRESENTATION,
+ PULL_FWD,
+ PTYPE_INVALID = 0xf,
+};
+
enum pmf_metrics_id {
IOCTL_POWER_SOURCE,
IOCTL_POWER_SLIDER_POSITION,
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 5/5] platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver metrics and features
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
` (3 preceding siblings ...)
2025-11-11 7:10 ` [PATCH v2 4/5] platform/x86/amd/pmf: Store commonly used enums in the header file Shyam Sundar S K
@ 2025-11-11 7:10 ` Shyam Sundar S K
2025-11-12 18:07 ` Mario Limonciello
2025-11-12 18:08 ` [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Mario Limonciello
2025-11-19 16:20 ` Hans de Goede
6 siblings, 1 reply; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-11 7:10 UTC (permalink / raw)
To: hansg, ilpo.jarvinen
Cc: platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami, Shyam Sundar S K
This tool development adds selftests for the AMD PMF driver, allowing
validation of its newly integrated util layer and /dev/amdpmf_interface.
It includes a user-space test application, test_pmf, designed to interact
with the PMF driver and retrieve relevant metrics for the testing and
analysis.
It provides definitions for test metrics, feature IDs, and device states,
and includes tests for various AMD PMF metrics such as power source, skin
temperature, battery state, and custom BIOS inputs/outputs. It also
enables the testing of PMF telemetry data and feature support reporting.
Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
tools/testing/selftests/Makefile | 1 +
.../drivers/platform/x86/amd/pmf/Makefile | 8 +
.../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
3 files changed, 397 insertions(+)
create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index c46ebdb9b8ef..6dd2549b5feb 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -25,6 +25,7 @@ TARGETS += drivers/net/bonding
TARGETS += drivers/net/team
TARGETS += drivers/net/virtio_net
TARGETS += drivers/platform/x86/intel/ifs
+TARGETS += drivers/platform/x86/amd/pmf
TARGETS += dt
TARGETS += efivarfs
TARGETS += exec
diff --git a/tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile b/tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
new file mode 100644
index 000000000000..876424941e83
--- /dev/null
+++ b/tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+CFLAGS += $(KHDR_INCLUDES)
+
+TEST_GEN_PROGS := test_pmf
+
+top_srcdir ?=../../../../..
+
+include ../../../../../lib.mk
diff --git a/tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c b/tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
new file mode 100644
index 000000000000..f376bc6f18e1
--- /dev/null
+++ b/tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * AMD Platform Management Framework Test Tool
+ *
+ * Copyright (c) 2025, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ * Sanket Goswami <Sanket.Goswami@amd.com>
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <inttypes.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <unistd.h>
+#include <linux/amd-pmf.h>
+
+#include "../../../../../kselftest.h"
+
+#define DEVICE_NODE "/dev/amdpmf_interface"
+
+#define BIOS_INPUT_START 9
+#define BIOS_OUTPUT_START 51
+
+struct feature_map {
+ unsigned long id;
+ const char *name;
+};
+
+static const struct feature_map feature_table[] = {
+ { PMF_FEATURE_AUTO_MODE, "Auto Mode" },
+ { PMF_FEATURE_STATIC_POWER_SLIDER, "Static Power Slider" },
+ { PMF_FEATURE_POLICY_BUILDER, "Policy Builder" },
+ { PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC, "Dynamic Power Slider AC" },
+ { PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC, "Dynamic Power Slider DC" },
+};
+
+enum laptop_placement {
+ LP_UNKNOWN,
+ ON_TABLE,
+ ON_LAP_MOTION,
+ IN_BAG,
+ OUT_OF_BAG,
+ LP_UNDEFINED,
+};
+
+static const char *platform_type_as_str(unsigned int platform_type)
+{
+ switch (platform_type) {
+ case CLAMSHELL:
+ return "CLAMSHELL";
+ case FLAT:
+ return "FLAT";
+ case TENT:
+ return "TENT";
+ case STAND:
+ return "STAND";
+ case TABLET:
+ return "TABLET";
+ case BOOK:
+ return "BOOK";
+ case PRESENTATION:
+ return "PRESENTATION";
+ case PULL_FWD:
+ return "PULL_FWD";
+ case LID_CLOSE:
+ return "LID_CLOSE";
+ case PTYPE_UNKNOWN:
+ default:
+ return "UNKNOWN";
+ }
+}
+
+static const char *laptop_placement_as_str(unsigned int device_state)
+{
+ switch (device_state) {
+ case ON_TABLE:
+ return "ON_TABLE";
+ case ON_LAP_MOTION:
+ return "ON_LAP_MOTION";
+ case IN_BAG:
+ return "IN_BAG";
+ case OUT_OF_BAG:
+ return "OUT_OF_BAG";
+ default:
+ return "UNKNOWN";
+ }
+}
+
+static const char *ta_slider_as_str(unsigned int state)
+{
+ switch (state) {
+ case TA_BEST_PERFORMANCE:
+ return "PERFORMANCE";
+ case TA_BETTER_PERFORMANCE:
+ return "BALANCED";
+ case TA_BEST_BATTERY:
+ return "POWER_SAVER";
+ case TA_BETTER_BATTERY:
+ return "BETTER_BATTERY";
+ default:
+ return "UNKNOWN";
+ }
+}
+
+static int pmf_open_device(void)
+{
+ int fd;
+
+ fd = open(DEVICE_NODE, O_RDONLY);
+ if (fd < 0)
+ fprintf(stderr, "opening PMF Device Node failed: %s\n", strerror(errno));
+
+ return fd;
+}
+
+/* Helper to run IOCTL_PMF_GET_METRICS_DATA for one control code and return 0 on success. */
+static int pmf_get_metrics_fd(int fd, enum pmf_metrics_id code, struct amd_pmf_metrics_key *out)
+{
+ struct amd_pmf_metrics_key info = {0};
+ int ret;
+
+ if (!out)
+ return -EINVAL;
+
+ info.control_code = code;
+
+ ret = ioctl(fd, IOCTL_PMF_GET_METRICS_DATA, &info);
+ if (ret < 0)
+ return ret;
+
+ if (out)
+ *out = info;
+
+ return 0;
+}
+
+static int pmf_get_metrics(enum pmf_metrics_id code, struct amd_pmf_metrics_key *out)
+{
+ int fd, ret;
+
+ fd = pmf_open_device();
+ if (fd < 0)
+ return fd;
+
+ ret = pmf_get_metrics_fd(fd, code, out);
+
+ close(fd);
+ return ret;
+}
+
+static int pmf_get_feature_status(unsigned long feature_id)
+{
+ struct amd_pmf_feature_support info = {0};
+ const char *name = NULL;
+ static char buf[32];
+ int i, fd, ret;
+
+ fd = pmf_open_device();
+ if (fd < 0)
+ return fd;
+
+ info.feature_id = feature_id;
+
+ ret = ioctl(fd, IOCTL_PMF_QUERY_FEATURE_SUPPORT, &info);
+ if (ret < 0)
+ goto exit_err;
+
+ for (i = 0; i < ARRAY_SIZE(feature_table); i++) {
+ if (feature_table[i].id == feature_id) {
+ name = feature_table[i].name;
+ break;
+ }
+ }
+ if (!name) {
+ snprintf(buf, sizeof(buf), "Feature %lu", feature_id);
+ name = buf;
+ }
+
+ printf("%-24s %s\n", name, info.feature_supported ? "Yes" : "No");
+
+exit_err:
+ close(fd);
+ return ret;
+}
+
+static int pmf_get_device_state(unsigned int code)
+{
+ struct amd_pmf_metrics_key info = {0};
+ int ret;
+
+ ret = pmf_get_metrics(code, &info);
+ if (ret < 0)
+ return ret;
+
+ switch (code) {
+ case IOCTL_PLATFORM_TYPE:
+ printf("Platform Type: %s\n", platform_type_as_str(info.val));
+ break;
+ case IOCTL_LAPTOP_PLACEMENT:
+ printf("Laptop placement: %s\n", laptop_placement_as_str(info.val));
+ break;
+ case IOCTL_LID_STATE:
+ printf("Lid State: %s\n", info.val ? "Close" : "Open");
+ break;
+ case IOCTL_USER_PRESENCE:
+ printf("User Presence: %s\n", info.val ? "Present" : "Away");
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+static int pmf_get_battery_state(unsigned int code)
+{
+ struct amd_pmf_metrics_key info = {0};
+ int ret;
+
+ ret = pmf_get_metrics(code, &info);
+ if (ret < 0)
+ return ret;
+
+ switch (code) {
+ case IOCTL_POWER_SOURCE:
+ printf("Power Source: %s\n", info.val ? "DC" : "AC");
+ break;
+ case IOCTL_BATTERY_PERCENTAGE:
+ printf("Battery Percentage: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_DESIGNED_BATTERY_CAPACITY:
+ printf("Designed Battery Capacity: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_FULLY_CHARGED_BATTERY_CAPACITY:
+ printf("Fully Charged Capacity: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_BATTERY_DRAIN_RATE:
+ printf("Battery Drain Rate: %ld\n", (int64_t)info.val);
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+static int pmf_get_custom_bios_input(unsigned int code)
+{
+ struct amd_pmf_metrics_key info = {0};
+ int ret;
+
+ ret = pmf_get_metrics(code, &info);
+ if (ret < 0)
+ return ret;
+
+ switch (code) {
+ case IOCTL_BIOS_INPUT_1:
+ case IOCTL_BIOS_INPUT_2:
+ case IOCTL_BIOS_INPUT_3:
+ case IOCTL_BIOS_INPUT_4:
+ case IOCTL_BIOS_INPUT_5:
+ case IOCTL_BIOS_INPUT_6:
+ case IOCTL_BIOS_INPUT_7:
+ case IOCTL_BIOS_INPUT_8:
+ case IOCTL_BIOS_INPUT_9:
+ case IOCTL_BIOS_INPUT_10:
+ printf("Custom BIOS input%u: %lu\n", code - BIOS_INPUT_START, (int64_t)info.val);
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+static int pmf_get_bios_output(unsigned int code)
+{
+ struct amd_pmf_metrics_key info = {0};
+ int ret;
+
+ ret = pmf_get_metrics(code, &info);
+ if (ret < 0)
+ return ret;
+
+ switch (code) {
+ case IOCTL_BIOS_OUTPUT_1:
+ case IOCTL_BIOS_OUTPUT_2:
+ case IOCTL_BIOS_OUTPUT_3:
+ case IOCTL_BIOS_OUTPUT_4:
+ case IOCTL_BIOS_OUTPUT_5:
+ case IOCTL_BIOS_OUTPUT_6:
+ case IOCTL_BIOS_OUTPUT_7:
+ case IOCTL_BIOS_OUTPUT_8:
+ case IOCTL_BIOS_OUTPUT_9:
+ case IOCTL_BIOS_OUTPUT_10:
+ printf("BIOS output%u: %lu\n", code - BIOS_OUTPUT_START, (int64_t)info.val);
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+static int pmf_get_misc_info(unsigned int code)
+{
+ struct amd_pmf_metrics_key info = {0};
+ int ret;
+
+ ret = pmf_get_metrics(code, &info);
+ if (ret < 0)
+ return ret;
+
+ switch (code) {
+ case IOCTL_POWER_SLIDER_POSITION:
+ printf("Slider Position: %s\n", ta_slider_as_str(info.val));
+ break;
+ case IOCTL_SKIN_TEMP:
+ printf("Skin Temperature: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_GFX_WORKLOAD:
+ printf("GFX Busy: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_AMBIENT_LIGHT:
+ printf("Ambient Light: %ld\n", (int64_t)info.val);
+ break;
+ case IOCTL_AVG_C0_RES:
+ printf("Avg C0 Residency: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_MAX_C0_RES:
+ printf("Max C0 Residency: %lu\n", (int64_t)info.val);
+ break;
+ case IOCTL_SOCKET_POWER:
+ printf("Socket Power: %lu\n", (int64_t)info.val);
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+int main(void)
+{
+ unsigned int idx;
+
+ printf("Feature Name Supported\n");
+ printf("---------------------------------\n");
+
+ for (idx = 0; idx < ARRAY_SIZE(feature_table); idx++)
+ pmf_get_feature_status(feature_table[idx].id);
+
+ printf("\nDevice State\n---------------\n");
+ for (idx = IOCTL_PLATFORM_TYPE; idx <= IOCTL_USER_PRESENCE; idx++)
+ pmf_get_device_state(idx);
+
+ printf("\nBattery\n--------\n");
+ pmf_get_battery_state(IOCTL_POWER_SOURCE);
+ pmf_get_battery_state(IOCTL_BATTERY_PERCENTAGE);
+ pmf_get_battery_state(IOCTL_DESIGNED_BATTERY_CAPACITY);
+ pmf_get_battery_state(IOCTL_FULLY_CHARGED_BATTERY_CAPACITY);
+ pmf_get_battery_state(IOCTL_BATTERY_DRAIN_RATE);
+
+ printf("\nCustom BIOS Inputs\n-------------------\n");
+ for (idx = IOCTL_BIOS_INPUT_1; idx <= IOCTL_BIOS_INPUT_10; idx++)
+ pmf_get_custom_bios_input(idx);
+
+ printf("\nBIOS Outputs\n--------------\n");
+ for (idx = IOCTL_BIOS_OUTPUT_1; idx <= IOCTL_BIOS_OUTPUT_10; idx++)
+ pmf_get_bios_output(idx);
+
+ printf("\nMisc\n------\n");
+ pmf_get_misc_info(IOCTL_SKIN_TEMP);
+ pmf_get_misc_info(IOCTL_GFX_WORKLOAD);
+ pmf_get_misc_info(IOCTL_AMBIENT_LIGHT);
+ pmf_get_misc_info(IOCTL_AVG_C0_RES);
+ pmf_get_misc_info(IOCTL_MAX_C0_RES);
+ pmf_get_misc_info(IOCTL_SOCKET_POWER);
+ pmf_get_misc_info(IOCTL_POWER_SLIDER_POSITION);
+
+ return 0;
+}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/5] platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver metrics and features
2025-11-11 7:10 ` [PATCH v2 5/5] platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver metrics and features Shyam Sundar S K
@ 2025-11-12 18:07 ` Mario Limonciello
0 siblings, 0 replies; 16+ messages in thread
From: Mario Limonciello @ 2025-11-12 18:07 UTC (permalink / raw)
To: Shyam Sundar S K, hansg, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
On 11/11/25 1:10 AM, Shyam Sundar S K wrote:
> This tool development adds selftests for the AMD PMF driver, allowing
> validation of its newly integrated util layer and /dev/amdpmf_interface.
> It includes a user-space test application, test_pmf, designed to interact
> with the PMF driver and retrieve relevant metrics for the testing and
> analysis.
>
> It provides definitions for test metrics, feature IDs, and device states,
> and includes tests for various AMD PMF metrics such as power source, skin
> temperature, battery state, and custom BIOS inputs/outputs. It also
> enables the testing of PMF telemetry data and feature support reporting.
>
> Co-developed-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Sanket Goswami <Sanket.Goswami@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
> tools/testing/selftests/Makefile | 1 +
> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
> 3 files changed, 397 insertions(+)
> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index c46ebdb9b8ef..6dd2549b5feb 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -25,6 +25,7 @@ TARGETS += drivers/net/bonding
> TARGETS += drivers/net/team
> TARGETS += drivers/net/virtio_net
> TARGETS += drivers/platform/x86/intel/ifs
> +TARGETS += drivers/platform/x86/amd/pmf
> TARGETS += dt
> TARGETS += efivarfs
> TARGETS += exec
> diff --git a/tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile b/tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
> new file mode 100644
> index 000000000000..876424941e83
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
> @@ -0,0 +1,8 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +CFLAGS += $(KHDR_INCLUDES)
> +
> +TEST_GEN_PROGS := test_pmf
> +
> +top_srcdir ?=../../../../..
> +
> +include ../../../../../lib.mk
> diff --git a/tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c b/tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
> new file mode 100644
> index 000000000000..f376bc6f18e1
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
> @@ -0,0 +1,388 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * AMD Platform Management Framework Test Tool
> + *
> + * Copyright (c) 2025, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + * Sanket Goswami <Sanket.Goswami@amd.com>
> + */
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <stdbool.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <unistd.h>
> +#include <linux/amd-pmf.h>
> +
> +#include "../../../../../kselftest.h"
> +
> +#define DEVICE_NODE "/dev/amdpmf_interface"
> +
> +#define BIOS_INPUT_START 9
> +#define BIOS_OUTPUT_START 51
> +
> +struct feature_map {
> + unsigned long id;
> + const char *name;
> +};
> +
> +static const struct feature_map feature_table[] = {
> + { PMF_FEATURE_AUTO_MODE, "Auto Mode" },
> + { PMF_FEATURE_STATIC_POWER_SLIDER, "Static Power Slider" },
> + { PMF_FEATURE_POLICY_BUILDER, "Policy Builder" },
> + { PMF_FEATURE_DYNAMIC_POWER_SLIDER_AC, "Dynamic Power Slider AC" },
> + { PMF_FEATURE_DYNAMIC_POWER_SLIDER_DC, "Dynamic Power Slider DC" },
> +};
> +
> +enum laptop_placement {
> + LP_UNKNOWN,
> + ON_TABLE,
> + ON_LAP_MOTION,
> + IN_BAG,
> + OUT_OF_BAG,
> + LP_UNDEFINED,
> +};
Can't you include amd-pmf-io.h and then use the define from that for
enum laptop_placement?
> +
> +static const char *platform_type_as_str(unsigned int platform_type)
> +{
> + switch (platform_type) {
> + case CLAMSHELL:
> + return "CLAMSHELL";
> + case FLAT:
> + return "FLAT";
> + case TENT:
> + return "TENT";
> + case STAND:
> + return "STAND";
> + case TABLET:
> + return "TABLET";
> + case BOOK:
> + return "BOOK";
> + case PRESENTATION:
> + return "PRESENTATION";
> + case PULL_FWD:
> + return "PULL_FWD";
> + case LID_CLOSE:
> + return "LID_CLOSE";
> + case PTYPE_UNKNOWN:
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> +static const char *laptop_placement_as_str(unsigned int device_state)
> +{
> + switch (device_state) {
> + case ON_TABLE:
> + return "ON_TABLE";
> + case ON_LAP_MOTION:
> + return "ON_LAP_MOTION";
> + case IN_BAG:
> + return "IN_BAG";
> + case OUT_OF_BAG:
> + return "OUT_OF_BAG";
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> +static const char *ta_slider_as_str(unsigned int state)
> +{
> + switch (state) {
> + case TA_BEST_PERFORMANCE:
> + return "PERFORMANCE";
> + case TA_BETTER_PERFORMANCE:
> + return "BALANCED";
> + case TA_BEST_BATTERY:
> + return "POWER_SAVER";
> + case TA_BETTER_BATTERY:
> + return "BETTER_BATTERY";
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> +static int pmf_open_device(void)
> +{
> + int fd;
> +
> + fd = open(DEVICE_NODE, O_RDONLY);
> + if (fd < 0)
> + fprintf(stderr, "opening PMF Device Node failed: %s\n", strerror(errno));
> +
> + return fd;
> +}
> +
> +/* Helper to run IOCTL_PMF_GET_METRICS_DATA for one control code and return 0 on success. */
> +static int pmf_get_metrics_fd(int fd, enum pmf_metrics_id code, struct amd_pmf_metrics_key *out)
> +{
> + struct amd_pmf_metrics_key info = {0};
> + int ret;
> +
> + if (!out)
> + return -EINVAL;
> +
> + info.control_code = code;
> +
> + ret = ioctl(fd, IOCTL_PMF_GET_METRICS_DATA, &info);
> + if (ret < 0)
> + return ret;
> +
> + if (out)
> + *out = info;
> +
> + return 0;
> +}
> +
> +static int pmf_get_metrics(enum pmf_metrics_id code, struct amd_pmf_metrics_key *out)
> +{
> + int fd, ret;
> +
> + fd = pmf_open_device();
> + if (fd < 0)
> + return fd;
> +
> + ret = pmf_get_metrics_fd(fd, code, out);
> +
> + close(fd);
> + return ret;
> +}
> +
> +static int pmf_get_feature_status(unsigned long feature_id)
> +{
> + struct amd_pmf_feature_support info = {0};
> + const char *name = NULL;
> + static char buf[32];
> + int i, fd, ret;
> +
> + fd = pmf_open_device();
> + if (fd < 0)
> + return fd;
> +
> + info.feature_id = feature_id;
> +
> + ret = ioctl(fd, IOCTL_PMF_QUERY_FEATURE_SUPPORT, &info);
> + if (ret < 0)
> + goto exit_err;
> +
> + for (i = 0; i < ARRAY_SIZE(feature_table); i++) {
> + if (feature_table[i].id == feature_id) {
> + name = feature_table[i].name;
> + break;
> + }
> + }
> + if (!name) {
> + snprintf(buf, sizeof(buf), "Feature %lu", feature_id);
> + name = buf;
> + }
> +
> + printf("%-24s %s\n", name, info.feature_supported ? "Yes" : "No");
> +
> +exit_err:
> + close(fd);
> + return ret;
> +}
> +
> +static int pmf_get_device_state(unsigned int code)
> +{
> + struct amd_pmf_metrics_key info = {0};
> + int ret;
> +
> + ret = pmf_get_metrics(code, &info);
> + if (ret < 0)
> + return ret;
> +
> + switch (code) {
> + case IOCTL_PLATFORM_TYPE:
> + printf("Platform Type: %s\n", platform_type_as_str(info.val));
> + break;
> + case IOCTL_LAPTOP_PLACEMENT:
> + printf("Laptop placement: %s\n", laptop_placement_as_str(info.val));
> + break;
> + case IOCTL_LID_STATE:
> + printf("Lid State: %s\n", info.val ? "Close" : "Open");
> + break;
> + case IOCTL_USER_PRESENCE:
> + printf("User Presence: %s\n", info.val ? "Present" : "Away");
> + break;
> + default:
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int pmf_get_battery_state(unsigned int code)
> +{
> + struct amd_pmf_metrics_key info = {0};
> + int ret;
> +
> + ret = pmf_get_metrics(code, &info);
> + if (ret < 0)
> + return ret;
> +
> + switch (code) {
> + case IOCTL_POWER_SOURCE:
> + printf("Power Source: %s\n", info.val ? "DC" : "AC");
> + break;
> + case IOCTL_BATTERY_PERCENTAGE:
> + printf("Battery Percentage: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_DESIGNED_BATTERY_CAPACITY:
> + printf("Designed Battery Capacity: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_FULLY_CHARGED_BATTERY_CAPACITY:
> + printf("Fully Charged Capacity: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_BATTERY_DRAIN_RATE:
> + printf("Battery Drain Rate: %ld\n", (int64_t)info.val);
> + break;
> + default:
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int pmf_get_custom_bios_input(unsigned int code)
> +{
> + struct amd_pmf_metrics_key info = {0};
> + int ret;
> +
> + ret = pmf_get_metrics(code, &info);
> + if (ret < 0)
> + return ret;
> +
> + switch (code) {
> + case IOCTL_BIOS_INPUT_1:
> + case IOCTL_BIOS_INPUT_2:
> + case IOCTL_BIOS_INPUT_3:
> + case IOCTL_BIOS_INPUT_4:
> + case IOCTL_BIOS_INPUT_5:
> + case IOCTL_BIOS_INPUT_6:
> + case IOCTL_BIOS_INPUT_7:
> + case IOCTL_BIOS_INPUT_8:
> + case IOCTL_BIOS_INPUT_9:
> + case IOCTL_BIOS_INPUT_10:
> + printf("Custom BIOS input%u: %lu\n", code - BIOS_INPUT_START, (int64_t)info.val);
> + break;
> + default:
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int pmf_get_bios_output(unsigned int code)
> +{
> + struct amd_pmf_metrics_key info = {0};
> + int ret;
> +
> + ret = pmf_get_metrics(code, &info);
> + if (ret < 0)
> + return ret;
> +
> + switch (code) {
> + case IOCTL_BIOS_OUTPUT_1:
> + case IOCTL_BIOS_OUTPUT_2:
> + case IOCTL_BIOS_OUTPUT_3:
> + case IOCTL_BIOS_OUTPUT_4:
> + case IOCTL_BIOS_OUTPUT_5:
> + case IOCTL_BIOS_OUTPUT_6:
> + case IOCTL_BIOS_OUTPUT_7:
> + case IOCTL_BIOS_OUTPUT_8:
> + case IOCTL_BIOS_OUTPUT_9:
> + case IOCTL_BIOS_OUTPUT_10:
> + printf("BIOS output%u: %lu\n", code - BIOS_OUTPUT_START, (int64_t)info.val);
> + break;
> + default:
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +static int pmf_get_misc_info(unsigned int code)
> +{
> + struct amd_pmf_metrics_key info = {0};
> + int ret;
> +
> + ret = pmf_get_metrics(code, &info);
> + if (ret < 0)
> + return ret;
> +
> + switch (code) {
> + case IOCTL_POWER_SLIDER_POSITION:
> + printf("Slider Position: %s\n", ta_slider_as_str(info.val));
> + break;
> + case IOCTL_SKIN_TEMP:
> + printf("Skin Temperature: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_GFX_WORKLOAD:
> + printf("GFX Busy: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_AMBIENT_LIGHT:
> + printf("Ambient Light: %ld\n", (int64_t)info.val);
> + break;
> + case IOCTL_AVG_C0_RES:
> + printf("Avg C0 Residency: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_MAX_C0_RES:
> + printf("Max C0 Residency: %lu\n", (int64_t)info.val);
> + break;
> + case IOCTL_SOCKET_POWER:
> + printf("Socket Power: %lu\n", (int64_t)info.val);
> + break;
> + default:
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +int main(void)
> +{
> + unsigned int idx;
> +
> + printf("Feature Name Supported\n");
> + printf("---------------------------------\n");
> +
> + for (idx = 0; idx < ARRAY_SIZE(feature_table); idx++)
> + pmf_get_feature_status(feature_table[idx].id);
> +
> + printf("\nDevice State\n---------------\n");
> + for (idx = IOCTL_PLATFORM_TYPE; idx <= IOCTL_USER_PRESENCE; idx++)
> + pmf_get_device_state(idx);
> +
> + printf("\nBattery\n--------\n");
> + pmf_get_battery_state(IOCTL_POWER_SOURCE);
> + pmf_get_battery_state(IOCTL_BATTERY_PERCENTAGE);
> + pmf_get_battery_state(IOCTL_DESIGNED_BATTERY_CAPACITY);
> + pmf_get_battery_state(IOCTL_FULLY_CHARGED_BATTERY_CAPACITY);
> + pmf_get_battery_state(IOCTL_BATTERY_DRAIN_RATE);
> +
> + printf("\nCustom BIOS Inputs\n-------------------\n");
> + for (idx = IOCTL_BIOS_INPUT_1; idx <= IOCTL_BIOS_INPUT_10; idx++)
> + pmf_get_custom_bios_input(idx);
> +
> + printf("\nBIOS Outputs\n--------------\n");
> + for (idx = IOCTL_BIOS_OUTPUT_1; idx <= IOCTL_BIOS_OUTPUT_10; idx++)
> + pmf_get_bios_output(idx);
> +
> + printf("\nMisc\n------\n");
> + pmf_get_misc_info(IOCTL_SKIN_TEMP);
> + pmf_get_misc_info(IOCTL_GFX_WORKLOAD);
> + pmf_get_misc_info(IOCTL_AMBIENT_LIGHT);
> + pmf_get_misc_info(IOCTL_AVG_C0_RES);
> + pmf_get_misc_info(IOCTL_MAX_C0_RES);
> + pmf_get_misc_info(IOCTL_SOCKET_POWER);
> + pmf_get_misc_info(IOCTL_POWER_SLIDER_POSITION);
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
` (4 preceding siblings ...)
2025-11-11 7:10 ` [PATCH v2 5/5] platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver metrics and features Shyam Sundar S K
@ 2025-11-12 18:08 ` Mario Limonciello
2025-11-13 7:30 ` Shyam Sundar S K
2025-11-19 16:20 ` Hans de Goede
6 siblings, 1 reply; 16+ messages in thread
From: Mario Limonciello @ 2025-11-12 18:08 UTC (permalink / raw)
To: Shyam Sundar S K, hansg, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
On 11/11/25 1:10 AM, Shyam Sundar S K wrote:
> This series introduces a util layer to the AMD Platform Management
> Framework (PMF) and a minimal user-space interface via a misc character
> device, enabling feature discovery and smoother integration with
> user-space tools. It also adds caching of BIOS output policy values to
> prepare for user-space telemetry reporting via IOCTLs.
>
> The motivation is to provide a stable interface for user-space tools to
> discover PMF features and consume selected metrics. Enable smoother
> integration with AMD SystemDeck, a widely used tool for monitoring and
> controlling power and thermal behavior, helping designers keep components
> within thermal limits to ensure proper operation and improve system
> stability and reliability.
>
> This series also adds a small, dependable userspace utility that leverages
> the new IOCTLs to query live power and thermal telemetry. Exposing this
> data in a scriptable interface helps users and tooling make informed,
> workload-aware decisions and supports validation and debugging.
>
> v2:
> ----
> - address remarks from v1
> - add a new tool that exercises the IOCTLs from PMF interface
>
> Shyam Sundar S K (5):
> platform/x86/amd/pmf: add util layer and user-space misc device
> interface
> platform/x86/amd/pmf: cache BIOS output values for user-space
> telemetry via util IOCTL
> Documentation/ABI: add testing entry for AMD PMF misc device interface
> platform/x86/amd/pmf: Store commonly used enums in the header file
> platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver
> metrics and features
>
> .../ABI/testing/misc-amdpmf_interface | 49 +++
> MAINTAINERS | 1 +
> drivers/platform/x86/amd/pmf/Kconfig | 10 +
> drivers/platform/x86/amd/pmf/Makefile | 2 +
> drivers/platform/x86/amd/pmf/core.c | 19 +
> drivers/platform/x86/amd/pmf/pmf.h | 33 +-
> drivers/platform/x86/amd/pmf/spc.c | 1 +
> drivers/platform/x86/amd/pmf/tee-if.c | 10 +
> drivers/platform/x86/amd/pmf/util.c | 236 +++++++++++
> include/uapi/linux/amd-pmf.h | 96 +++++
> tools/testing/selftests/Makefile | 1 +
> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
> 13 files changed, 832 insertions(+), 22 deletions(-)
> create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
> create mode 100644 drivers/platform/x86/amd/pmf/util.c
> create mode 100644 include/uapi/linux/amd-pmf.h
> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
>
Series looks good to me now, minus one nit on the last patch.
Thanks for including an open source consumer of the new IOCTL.
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
The documentation (in patch 4/5) says 6.20, but if Ilpo agrees to take
this for 6.19 that may get updated when committing.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-12 18:08 ` [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Mario Limonciello
@ 2025-11-13 7:30 ` Shyam Sundar S K
0 siblings, 0 replies; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-13 7:30 UTC (permalink / raw)
To: Mario Limonciello, hansg, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
On 11/12/2025 23:38, Mario Limonciello wrote:
> On 11/11/25 1:10 AM, Shyam Sundar S K wrote:
>> This series introduces a util layer to the AMD Platform Management
>> Framework (PMF) and a minimal user-space interface via a misc character
>> device, enabling feature discovery and smoother integration with
>> user-space tools. It also adds caching of BIOS output policy values to
>> prepare for user-space telemetry reporting via IOCTLs.
>>
>> The motivation is to provide a stable interface for user-space tools to
>> discover PMF features and consume selected metrics. Enable smoother
>> integration with AMD SystemDeck, a widely used tool for monitoring and
>> controlling power and thermal behavior, helping designers keep
>> components
>> within thermal limits to ensure proper operation and improve system
>> stability and reliability.
>>
>> This series also adds a small, dependable userspace utility that
>> leverages
>> the new IOCTLs to query live power and thermal telemetry. Exposing this
>> data in a scriptable interface helps users and tooling make informed,
>> workload-aware decisions and supports validation and debugging.
>>
>> v2:
>> ----
>> - address remarks from v1
>> - add a new tool that exercises the IOCTLs from PMF interface
>>
>> Shyam Sundar S K (5):
>> platform/x86/amd/pmf: add util layer and user-space misc device
>> interface
>> platform/x86/amd/pmf: cache BIOS output values for user-space
>> telemetry via util IOCTL
>> Documentation/ABI: add testing entry for AMD PMF misc device
>> interface
>> platform/x86/amd/pmf: Store commonly used enums in the header file
>> platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver
>> metrics and features
>>
>> .../ABI/testing/misc-amdpmf_interface | 49 +++
>> MAINTAINERS | 1 +
>> drivers/platform/x86/amd/pmf/Kconfig | 10 +
>> drivers/platform/x86/amd/pmf/Makefile | 2 +
>> drivers/platform/x86/amd/pmf/core.c | 19 +
>> drivers/platform/x86/amd/pmf/pmf.h | 33 +-
>> drivers/platform/x86/amd/pmf/spc.c | 1 +
>> drivers/platform/x86/amd/pmf/tee-if.c | 10 +
>> drivers/platform/x86/amd/pmf/util.c | 236 +++++++++++
>> include/uapi/linux/amd-pmf.h | 96 +++++
>> tools/testing/selftests/Makefile | 1 +
>> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
>> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++
>> ++++
>> 13 files changed, 832 insertions(+), 22 deletions(-)
>> create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
>> create mode 100644 drivers/platform/x86/amd/pmf/util.c
>> create mode 100644 include/uapi/linux/amd-pmf.h
>> create mode 100644 tools/testing/selftests/drivers/platform/x86/
>> amd/pmf/Makefile
>> create mode 100644 tools/testing/selftests/drivers/platform/x86/
>> amd/pmf/test_pmf.c
>>
>
> Series looks good to me now, minus one nit on the last patch.
> Thanks for including an open source consumer of the new IOCTL.
Thanks for the review! Sure. Would like to see some more feedback
coming in before I respin a new one.
>
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>
>
> The documentation (in patch 4/5) says 6.20, but if Ilpo agrees to take
> this for 6.19 that may get updated when committing.
>
Let's hear from Ilpo on his thoughts.
Thanks,
Shyam
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-11 7:10 [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Shyam Sundar S K
` (5 preceding siblings ...)
2025-11-12 18:08 ` [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck Mario Limonciello
@ 2025-11-19 16:20 ` Hans de Goede
2025-11-19 16:32 ` Mario Limonciello
6 siblings, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2025-11-19 16:20 UTC (permalink / raw)
To: Shyam Sundar S K, ilpo.jarvinen
Cc: platform-driver-x86, mario.limonciello, Yijun.Shen,
Sanket.Goswami
Hi Shyam,
On 11-Nov-25 8:10 AM, Shyam Sundar S K wrote:
> This series introduces a util layer to the AMD Platform Management
> Framework (PMF) and a minimal user-space interface via a misc character
> device, enabling feature discovery and smoother integration with
> user-space tools. It also adds caching of BIOS output policy values to
> prepare for user-space telemetry reporting via IOCTLs.
>
> The motivation is to provide a stable interface for user-space tools to
> discover PMF features and consume selected metrics. Enable smoother
> integration with AMD SystemDeck
This does not really explain why you've chosen for a new character-device
with IOCTLs instead of sysfs where as so far (AFAICT) all the AMD PMF code
has been using sysfs APIs.
Is there any specific reason why to switch to IOCTLs all of a sudden?
Note that:
1. sysfs APIs can be (and must be) stable too, sysfs APIs are not allowed
to be changed once shipped in a stable kernel.
2. sysfs attributes can be used with poll() to so if you want to do
notifications of changes that can be done through sysfs too.
Note I'm not saying you must use sysfs, but so far the PMF code has been
using sysfs everywhere and this new IOCTL based API is not really consistent
with this.
So at a minimum you are going to need some pretty good arguments to
deviate from just doing this with sysfs as has been done before.
Regards,
Hans
> , a widely used tool for monitoring and
> controlling power and thermal behavior, helping designers keep components
> within thermal limits to ensure proper operation and improve system
> stability and reliability.
>
> This series also adds a small, dependable userspace utility that leverages
> the new IOCTLs to query live power and thermal telemetry. Exposing this
> data in a scriptable interface helps users and tooling make informed,
> workload-aware decisions and supports validation and debugging.
>
> v2:
> ----
> - address remarks from v1
> - add a new tool that exercises the IOCTLs from PMF interface
>
> Shyam Sundar S K (5):
> platform/x86/amd/pmf: add util layer and user-space misc device
> interface
> platform/x86/amd/pmf: cache BIOS output values for user-space
> telemetry via util IOCTL
> Documentation/ABI: add testing entry for AMD PMF misc device interface
> platform/x86/amd/pmf: Store commonly used enums in the header file
> platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver
> metrics and features
>
> .../ABI/testing/misc-amdpmf_interface | 49 +++
> MAINTAINERS | 1 +
> drivers/platform/x86/amd/pmf/Kconfig | 10 +
> drivers/platform/x86/amd/pmf/Makefile | 2 +
> drivers/platform/x86/amd/pmf/core.c | 19 +
> drivers/platform/x86/amd/pmf/pmf.h | 33 +-
> drivers/platform/x86/amd/pmf/spc.c | 1 +
> drivers/platform/x86/amd/pmf/tee-if.c | 10 +
> drivers/platform/x86/amd/pmf/util.c | 236 +++++++++++
> include/uapi/linux/amd-pmf.h | 96 +++++
> tools/testing/selftests/Makefile | 1 +
> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
> 13 files changed, 832 insertions(+), 22 deletions(-)
> create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
> create mode 100644 drivers/platform/x86/amd/pmf/util.c
> create mode 100644 include/uapi/linux/amd-pmf.h
> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-19 16:20 ` Hans de Goede
@ 2025-11-19 16:32 ` Mario Limonciello
2025-11-19 19:25 ` Hans de Goede
0 siblings, 1 reply; 16+ messages in thread
From: Mario Limonciello @ 2025-11-19 16:32 UTC (permalink / raw)
To: Hans de Goede, Shyam Sundar S K, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
On 11/19/25 10:20 AM, Hans de Goede wrote:
> Hi Shyam,
>
> On 11-Nov-25 8:10 AM, Shyam Sundar S K wrote:
>> This series introduces a util layer to the AMD Platform Management
>> Framework (PMF) and a minimal user-space interface via a misc character
>> device, enabling feature discovery and smoother integration with
>> user-space tools. It also adds caching of BIOS output policy values to
>> prepare for user-space telemetry reporting via IOCTLs.
>>
>> The motivation is to provide a stable interface for user-space tools to
>> discover PMF features and consume selected metrics. Enable smoother
>> integration with AMD SystemDeck
>
> This does not really explain why you've chosen for a new character-device
> with IOCTLs instead of sysfs where as so far (AFAICT) all the AMD PMF code
> has been using sysfs APIs.
>
> Is there any specific reason why to switch to IOCTLs all of a sudden?
>
> Note that:
>
> 1. sysfs APIs can be (and must be) stable too, sysfs APIs are not allowed
> to be changed once shipped in a stable kernel.
> 2. sysfs attributes can be used with poll() to so if you want to do
> notifications of changes that can be done through sysfs too.
>
> Note I'm not saying you must use sysfs, but so far the PMF code has been
> using sysfs everywhere and this new IOCTL based API is not really consistent
> with this.
Isn't there only one sysfs file for turning on/off CNQF?
>
> So at a minimum you are going to need some pretty good arguments to
> deviate from just doing this with sysfs as has been done before.
>
> Regards,
>
> Hans
>
>
>
>
>> , a widely used tool for monitoring and
>> controlling power and thermal behavior, helping designers keep components
>> within thermal limits to ensure proper operation and improve system
>> stability and reliability.
>>
>> This series also adds a small, dependable userspace utility that leverages
>> the new IOCTLs to query live power and thermal telemetry. Exposing this
>> data in a scriptable interface helps users and tooling make informed,
>> workload-aware decisions and supports validation and debugging.
>>
>> v2:
>> ----
>> - address remarks from v1
>> - add a new tool that exercises the IOCTLs from PMF interface
>>
>> Shyam Sundar S K (5):
>> platform/x86/amd/pmf: add util layer and user-space misc device
>> interface
>> platform/x86/amd/pmf: cache BIOS output values for user-space
>> telemetry via util IOCTL
>> Documentation/ABI: add testing entry for AMD PMF misc device interface
>> platform/x86/amd/pmf: Store commonly used enums in the header file
>> platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver
>> metrics and features
>>
>> .../ABI/testing/misc-amdpmf_interface | 49 +++
>> MAINTAINERS | 1 +
>> drivers/platform/x86/amd/pmf/Kconfig | 10 +
>> drivers/platform/x86/amd/pmf/Makefile | 2 +
>> drivers/platform/x86/amd/pmf/core.c | 19 +
>> drivers/platform/x86/amd/pmf/pmf.h | 33 +-
>> drivers/platform/x86/amd/pmf/spc.c | 1 +
>> drivers/platform/x86/amd/pmf/tee-if.c | 10 +
>> drivers/platform/x86/amd/pmf/util.c | 236 +++++++++++
>> include/uapi/linux/amd-pmf.h | 96 +++++
>> tools/testing/selftests/Makefile | 1 +
>> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
>> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
>> 13 files changed, 832 insertions(+), 22 deletions(-)
>> create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
>> create mode 100644 drivers/platform/x86/amd/pmf/util.c
>> create mode 100644 include/uapi/linux/amd-pmf.h
>> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
>> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-19 16:32 ` Mario Limonciello
@ 2025-11-19 19:25 ` Hans de Goede
2025-11-20 7:49 ` Shyam Sundar S K
0 siblings, 1 reply; 16+ messages in thread
From: Hans de Goede @ 2025-11-19 19:25 UTC (permalink / raw)
To: Mario Limonciello, Shyam Sundar S K, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
Hi Mario,
On 19-Nov-25 5:32 PM, Mario Limonciello wrote:
> On 11/19/25 10:20 AM, Hans de Goede wrote:
>> Hi Shyam,
>>
>> On 11-Nov-25 8:10 AM, Shyam Sundar S K wrote:
>>> This series introduces a util layer to the AMD Platform Management
>>> Framework (PMF) and a minimal user-space interface via a misc character
>>> device, enabling feature discovery and smoother integration with
>>> user-space tools. It also adds caching of BIOS output policy values to
>>> prepare for user-space telemetry reporting via IOCTLs.
>>>
>>> The motivation is to provide a stable interface for user-space tools to
>>> discover PMF features and consume selected metrics. Enable smoother
>>> integration with AMD SystemDeck
>>
>> This does not really explain why you've chosen for a new character-device
>> with IOCTLs instead of sysfs where as so far (AFAICT) all the AMD PMF code
>> has been using sysfs APIs.
>>
>> Is there any specific reason why to switch to IOCTLs all of a sudden?
>>
>> Note that:
>>
>> 1. sysfs APIs can be (and must be) stable too, sysfs APIs are not allowed
>> to be changed once shipped in a stable kernel.
>> 2. sysfs attributes can be used with poll() to so if you want to do
>> notifications of changes that can be done through sysfs too.
>>
>> Note I'm not saying you must use sysfs, but so far the PMF code has been
>> using sysfs everywhere and this new IOCTL based API is not really consistent
>> with this.
>
> Isn't there only one sysfs file for turning on/off CNQF?
Ah yes you're right somehow I thought there were more.
Still generally speaking the kernel community is trying to avoid
adding new ioctl based interfaces / adding random new char devices
in preference of using sysfs interface where possible.
So I've taken a better look at the actual ioctl interface
and it seems like a really weird multiplexer interface,
where there is only 2 ioctl commands and then the argument
gets 1 of a ton of possible feature flags resp. info variables.
Where it also seems that none of these variables require
a round-trip to the hardware.
Given the amount of different variables I can see some sense
in having this as an ioctl interface, but why do the whole
thing where userspace has to make ioctl per value it wants
to read. That feels very sysfs-ish if you want that maybe
just use sysfs ?
I would define a uAPI struct like this:
struct foo {
u64 size; /* in + out, all other fields out only */
u64 features_supported; /* bitmask with feature info from patch 1/5 */
u64 feature_version /* from patch 1/5 */
u64 power_source; /* from patch 2/5 */
...
u64 bios_input[10]; /* from patch 2/5 */
...
etc.
};
And have a copy of this struct embedded in the driver
data struct and keep that updated (replacing the cache
stuff) so that you can just copy_to_user that on the ioctl.
Combined with a single get-info ioctl which just fills
the struct, using the min of the size passed in by userspace
+ the size supported by the kernel to determine how much
to copy and set the copied size in the struct passed
back to userspace (to indicate for new userspace on
old kernel that the new fields are not set).
This way for future extensions new fields can be added to
the end of the struct and the size handling will automatically
do the right thing.
As for Ilpo's comment about the battery info being duplicate
with /sys/class/power_supply/BAT*, where is this info coming
from ? Is this a PMF specific view of the battery info,
IOW it might be different then the power_supply calls info?
Note stil not a fan of adding new IOCTLs but given the large
amount of variables I can see how using an IOCTL might be
better then adding a ton of new sysfs attributes.
Regards,
Hans
>>> , a widely used tool for monitoring and
>>> controlling power and thermal behavior, helping designers keep components
>>> within thermal limits to ensure proper operation and improve system
>>> stability and reliability.
>>>
>>> This series also adds a small, dependable userspace utility that leverages
>>> the new IOCTLs to query live power and thermal telemetry. Exposing this
>>> data in a scriptable interface helps users and tooling make informed,
>>> workload-aware decisions and supports validation and debugging.
>>>
>>> v2:
>>> ----
>>> - address remarks from v1
>>> - add a new tool that exercises the IOCTLs from PMF interface
>>>
>>> Shyam Sundar S K (5):
>>> platform/x86/amd/pmf: add util layer and user-space misc device
>>> interface
>>> platform/x86/amd/pmf: cache BIOS output values for user-space
>>> telemetry via util IOCTL
>>> Documentation/ABI: add testing entry for AMD PMF misc device interface
>>> platform/x86/amd/pmf: Store commonly used enums in the header file
>>> platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver
>>> metrics and features
>>>
>>> .../ABI/testing/misc-amdpmf_interface | 49 +++
>>> MAINTAINERS | 1 +
>>> drivers/platform/x86/amd/pmf/Kconfig | 10 +
>>> drivers/platform/x86/amd/pmf/Makefile | 2 +
>>> drivers/platform/x86/amd/pmf/core.c | 19 +
>>> drivers/platform/x86/amd/pmf/pmf.h | 33 +-
>>> drivers/platform/x86/amd/pmf/spc.c | 1 +
>>> drivers/platform/x86/amd/pmf/tee-if.c | 10 +
>>> drivers/platform/x86/amd/pmf/util.c | 236 +++++++++++
>>> include/uapi/linux/amd-pmf.h | 96 +++++
>>> tools/testing/selftests/Makefile | 1 +
>>> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
>>> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
>>> 13 files changed, 832 insertions(+), 22 deletions(-)
>>> create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
>>> create mode 100644 drivers/platform/x86/amd/pmf/util.c
>>> create mode 100644 include/uapi/linux/amd-pmf.h
>>> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
>>> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
>>>
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-19 19:25 ` Hans de Goede
@ 2025-11-20 7:49 ` Shyam Sundar S K
2025-11-24 10:04 ` Hans de Goede
0 siblings, 1 reply; 16+ messages in thread
From: Shyam Sundar S K @ 2025-11-20 7:49 UTC (permalink / raw)
To: Hans de Goede, Mario Limonciello, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
Hi hans,
On 11/20/2025 00:55, Hans de Goede wrote:
> Hi Mario,
>
> On 19-Nov-25 5:32 PM, Mario Limonciello wrote:
>> On 11/19/25 10:20 AM, Hans de Goede wrote:
>>> Hi Shyam,
>>>
>>> On 11-Nov-25 8:10 AM, Shyam Sundar S K wrote:
>>>> This series introduces a util layer to the AMD Platform Management
>>>> Framework (PMF) and a minimal user-space interface via a misc character
>>>> device, enabling feature discovery and smoother integration with
>>>> user-space tools. It also adds caching of BIOS output policy values to
>>>> prepare for user-space telemetry reporting via IOCTLs.
>>>>
>>>> The motivation is to provide a stable interface for user-space tools to
>>>> discover PMF features and consume selected metrics. Enable smoother
>>>> integration with AMD SystemDeck
>>>
>>> This does not really explain why you've chosen for a new character-device
>>> with IOCTLs instead of sysfs where as so far (AFAICT) all the AMD PMF code
>>> has been using sysfs APIs.
>>>
>>> Is there any specific reason why to switch to IOCTLs all of a sudden?
>>>
>>> Note that:
>>>
>>> 1. sysfs APIs can be (and must be) stable too, sysfs APIs are not allowed
>>> to be changed once shipped in a stable kernel.
>>> 2. sysfs attributes can be used with poll() to so if you want to do
>>> notifications of changes that can be done through sysfs too.
>>>
>>> Note I'm not saying you must use sysfs, but so far the PMF code has been
>>> using sysfs everywhere and this new IOCTL based API is not really consistent
>>> with this.
>>
>> Isn't there only one sysfs file for turning on/off CNQF?
>
> Ah yes you're right somehow I thought there were more.
>
> Still generally speaking the kernel community is trying to avoid
> adding new ioctl based interfaces / adding random new char devices
> in preference of using sysfs interface where possible.
>
> So I've taken a better look at the actual ioctl interface
> and it seems like a really weird multiplexer interface,
> where there is only 2 ioctl commands and then the argument
> gets 1 of a ton of possible feature flags resp. info variables.
>
> Where it also seems that none of these variables require
> a round-trip to the hardware.
>
> Given the amount of different variables I can see some sense
> in having this as an ioctl interface, but why do the whole
> thing where userspace has to make ioctl per value it wants
> to read. That feels very sysfs-ish if you want that maybe
> just use sysfs ?
>
> I would define a uAPI struct like this:
>
> struct foo {
> u64 size; /* in + out, all other fields out only */
> u64 features_supported; /* bitmask with feature info from patch 1/5 */
> u64 feature_version /* from patch 1/5 */
> u64 power_source; /* from patch 2/5 */
> ...
> u64 bios_input[10]; /* from patch 2/5 */
> ...
> etc.
> };
>
thank you for your remarks and the UAPI struct suggestion. Let me make
this change and come back with a newer version.
> And have a copy of this struct embedded in the driver
> data struct and keep that updated (replacing the cache
> stuff) so that you can just copy_to_user that on the ioctl.
>
> Combined with a single get-info ioctl which just fills
> the struct, using the min of the size passed in by userspace
> + the size supported by the kernel to determine how much
> to copy and set the copied size in the struct passed
> back to userspace (to indicate for new userspace on
> old kernel that the new fields are not set).
>
> This way for future extensions new fields can be added to
> the end of the struct and the size handling will automatically
> do the right thing.
>
> As for Ilpo's comment about the battery info being duplicate
> with /sys/class/power_supply/BAT*, where is this info coming
> from ? Is this a PMF specific view of the battery info,
> IOW it might be different then the power_supply calls info?
>
with regards to this:
The function amd_pmf_get_battery_info() fills in the data that will be
sent to the TA. Inside this function, it calls
amd_pmf_get_battery_prop() to retrieve each required battery property.
For example:
/* to get the battery percentage */
in->ev_info.bat_percentage =
amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
Because the PMF already obtains battery information from the
power_supply, amd_pmf_get_battery_info() simply transfers those values
into the ioctl structure so they can be returned to user space via
copy_to_user(). To do this we map the ioctl requests to the
corresponding fields, for example:
+ case IOCTL_DESIGNED_BATTERY_CAPACITY:
+ output.val = in->ev_info.bat_design;
+ break;
+ case IOCTL_FULLY_CHARGED_BATTERY_CAPACITY:
+ output.val = in->ev_info.full_charge_capacity;
+ break;
+ case IOCTL_BATTERY_DRAIN_RATE:
+ output.val = in->ev_info.drain_rate;
+ break;
> Note stil not a fan of adding new IOCTLs but given the large
> amount of variables I can see how using an IOCTL might be
> better then adding a ton of new sysfs attributes.
>
Thank you..
Thanks,
Shyam
> Regards,
>
> Hans
>
>
>
>
>>>> , a widely used tool for monitoring and
>>>> controlling power and thermal behavior, helping designers keep components
>>>> within thermal limits to ensure proper operation and improve system
>>>> stability and reliability.
>>>>
>>>> This series also adds a small, dependable userspace utility that leverages
>>>> the new IOCTLs to query live power and thermal telemetry. Exposing this
>>>> data in a scriptable interface helps users and tooling make informed,
>>>> workload-aware decisions and supports validation and debugging.
>>>>
>>>> v2:
>>>> ----
>>>> - address remarks from v1
>>>> - add a new tool that exercises the IOCTLs from PMF interface
>>>>
>>>> Shyam Sundar S K (5):
>>>> platform/x86/amd/pmf: add util layer and user-space misc device
>>>> interface
>>>> platform/x86/amd/pmf: cache BIOS output values for user-space
>>>> telemetry via util IOCTL
>>>> Documentation/ABI: add testing entry for AMD PMF misc device interface
>>>> platform/x86/amd/pmf: Store commonly used enums in the header file
>>>> platform/x86/amd/pmf: Introduce AMD PMF testing tool for driver
>>>> metrics and features
>>>>
>>>> .../ABI/testing/misc-amdpmf_interface | 49 +++
>>>> MAINTAINERS | 1 +
>>>> drivers/platform/x86/amd/pmf/Kconfig | 10 +
>>>> drivers/platform/x86/amd/pmf/Makefile | 2 +
>>>> drivers/platform/x86/amd/pmf/core.c | 19 +
>>>> drivers/platform/x86/amd/pmf/pmf.h | 33 +-
>>>> drivers/platform/x86/amd/pmf/spc.c | 1 +
>>>> drivers/platform/x86/amd/pmf/tee-if.c | 10 +
>>>> drivers/platform/x86/amd/pmf/util.c | 236 +++++++++++
>>>> include/uapi/linux/amd-pmf.h | 96 +++++
>>>> tools/testing/selftests/Makefile | 1 +
>>>> .../drivers/platform/x86/amd/pmf/Makefile | 8 +
>>>> .../drivers/platform/x86/amd/pmf/test_pmf.c | 388 ++++++++++++++++++
>>>> 13 files changed, 832 insertions(+), 22 deletions(-)
>>>> create mode 100644 Documentation/ABI/testing/misc-amdpmf_interface
>>>> create mode 100644 drivers/platform/x86/amd/pmf/util.c
>>>> create mode 100644 include/uapi/linux/amd-pmf.h
>>>> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/Makefile
>>>> create mode 100644 tools/testing/selftests/drivers/platform/x86/amd/pmf/test_pmf.c
>>>>
>>>
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 0/5] Introduce AMD PMF util layer and user-space interface for SystemDeck
2025-11-20 7:49 ` Shyam Sundar S K
@ 2025-11-24 10:04 ` Hans de Goede
0 siblings, 0 replies; 16+ messages in thread
From: Hans de Goede @ 2025-11-24 10:04 UTC (permalink / raw)
To: Shyam Sundar S K, Mario Limonciello, ilpo.jarvinen
Cc: platform-driver-x86, Yijun.Shen, Sanket.Goswami
Hi,
On 20-Nov-25 08:49, Shyam Sundar S K wrote:
> Hi hans,
>
> On 11/20/2025 00:55, Hans de Goede wrote:
>> Hi Mario,
>>
>> On 19-Nov-25 5:32 PM, Mario Limonciello wrote:
>>> On 11/19/25 10:20 AM, Hans de Goede wrote:
>>>> Hi Shyam,
>>>>
>>>> On 11-Nov-25 8:10 AM, Shyam Sundar S K wrote:
>>>>> This series introduces a util layer to the AMD Platform Management
>>>>> Framework (PMF) and a minimal user-space interface via a misc character
>>>>> device, enabling feature discovery and smoother integration with
>>>>> user-space tools. It also adds caching of BIOS output policy values to
>>>>> prepare for user-space telemetry reporting via IOCTLs.
>>>>>
>>>>> The motivation is to provide a stable interface for user-space tools to
>>>>> discover PMF features and consume selected metrics. Enable smoother
>>>>> integration with AMD SystemDeck
>>>>
>>>> This does not really explain why you've chosen for a new character-device
>>>> with IOCTLs instead of sysfs where as so far (AFAICT) all the AMD PMF code
>>>> has been using sysfs APIs.
>>>>
>>>> Is there any specific reason why to switch to IOCTLs all of a sudden?
>>>>
>>>> Note that:
>>>>
>>>> 1. sysfs APIs can be (and must be) stable too, sysfs APIs are not allowed
>>>> to be changed once shipped in a stable kernel.
>>>> 2. sysfs attributes can be used with poll() to so if you want to do
>>>> notifications of changes that can be done through sysfs too.
>>>>
>>>> Note I'm not saying you must use sysfs, but so far the PMF code has been
>>>> using sysfs everywhere and this new IOCTL based API is not really consistent
>>>> with this.
>>>
>>> Isn't there only one sysfs file for turning on/off CNQF?
>>
>> Ah yes you're right somehow I thought there were more.
>>
>> Still generally speaking the kernel community is trying to avoid
>> adding new ioctl based interfaces / adding random new char devices
>> in preference of using sysfs interface where possible.
>>
>> So I've taken a better look at the actual ioctl interface
>> and it seems like a really weird multiplexer interface,
>> where there is only 2 ioctl commands and then the argument
>> gets 1 of a ton of possible feature flags resp. info variables.
>>
>> Where it also seems that none of these variables require
>> a round-trip to the hardware.
>>
>> Given the amount of different variables I can see some sense
>> in having this as an ioctl interface, but why do the whole
>> thing where userspace has to make ioctl per value it wants
>> to read. That feels very sysfs-ish if you want that maybe
>> just use sysfs ?
>>
>> I would define a uAPI struct like this:
>>
>> struct foo {
>> u64 size; /* in + out, all other fields out only */
>> u64 features_supported; /* bitmask with feature info from patch 1/5 */
>> u64 feature_version /* from patch 1/5 */
>> u64 power_source; /* from patch 2/5 */
>> ...
>> u64 bios_input[10]; /* from patch 2/5 */
>> ...
>> etc.
>> };
>>
>
> thank you for your remarks and the UAPI struct suggestion. Let me make
> this change and come back with a newer version.
>
>> And have a copy of this struct embedded in the driver
>> data struct and keep that updated (replacing the cache
>> stuff) so that you can just copy_to_user that on the ioctl.
>>
>> Combined with a single get-info ioctl which just fills
>> the struct, using the min of the size passed in by userspace
>> + the size supported by the kernel to determine how much
>> to copy and set the copied size in the struct passed
>> back to userspace (to indicate for new userspace on
>> old kernel that the new fields are not set).
>>
>> This way for future extensions new fields can be added to
>> the end of the struct and the size handling will automatically
>> do the right thing.
>>
>> As for Ilpo's comment about the battery info being duplicate
>> with /sys/class/power_supply/BAT*, where is this info coming
>> from ? Is this a PMF specific view of the battery info,
>> IOW it might be different then the power_supply calls info?
>>
>
> with regards to this:
>
> The function amd_pmf_get_battery_info() fills in the data that will be
> sent to the TA. Inside this function, it calls
> amd_pmf_get_battery_prop() to retrieve each required battery property.
>
> For example:
>
> /* to get the battery percentage */
> in->ev_info.bat_percentage =
> amd_pmf_get_battery_prop(POWER_SUPPLY_PROP_CAPACITY);
Ok, so this is duplicating the information from /sys/class/power_supply .
Is there a reason why your userspace app cannot just directly get
these values from /sys/class/power_supply ?
I guess it might be convenient to have this all in the same ioctl,
I understand that the PMF code already needs to retrieve it regardless.
Maybe just have an ioctl which just memcpy-s the entire
ta_pmf_condition_info struct to userspace? I think that is already
part of the kernel <-> PMF-firmware API so making that struct uAPI
should be fine since it is already something which cannot be changed
because it is also firmware API ?
It seems just being able to copy the cached ev_info the driver
already has to userspace is better then defining a new struct with
similar fields and needing to manually copy over everything field
by field ?
And then instead of an ioctl it could even just be a binary sysfs
file, avoiding the need to add a somewhat random new chardev
to /dev ?
Regards,
Hans
^ permalink raw reply [flat|nested] 16+ messages in thread