From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Cc: Hans de Goede <hansg@kernel.org>,
platform-driver-x86@vger.kernel.org, mario.limonciello@amd.com,
Yijun.Shen@Dell.com, Sanket.Goswami@amd.com
Subject: Re: [PATCH v2 2/5] platform/x86/amd/pmf: cache BIOS output values for user-space telemetry via util IOCTL
Date: Tue, 18 Nov 2025 18:18:22 +0200 (EET) [thread overview]
Message-ID: <6f4f236e-6dab-46ce-ea3f-fb5805d0116b@linux.intel.com> (raw)
In-Reply-To: <20251111071010.4179492-3-Shyam-sundar.S-k@amd.com>
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.
next prev parent reply other threads:[~2025-11-18 16:18 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
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-18 16:18 ` Ilpo Järvinen [this message]
2025-11-26 9:36 ` Shyam Sundar S K
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 ` [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 ` [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
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
2025-11-19 16:20 ` Hans de Goede
2025-11-19 16:32 ` Mario Limonciello
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6f4f236e-6dab-46ce-ea3f-fb5805d0116b@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=Sanket.Goswami@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=Yijun.Shen@Dell.com \
--cc=hansg@kernel.org \
--cc=mario.limonciello@amd.com \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox