[PATCH v2 0/5] PMF NPU metrics cleanup, command flag cleanup, and amdxdna integration

[PATCH v2 0/5] PMF NPU metrics cleanup, command flag cleanup, and amdxdna integration

[Shyam Sundar S K]

This series streamlines NPU-related handling in AMD PMF, adds a
lightweight in-kernel interface for fetching NPU metrics from PMF, and
hooks AMD XDNA to that interface to expose real-time NPU power data to
user space.

Changes include renaming legacy IPU fields to NPU in the metrics structure
replacing ambiguous booleans and magic values with explicit
SET_CMD/GET_CMD and METRICS_TABLE_ID constants, introducing an exported
amd_pmf_get_npu_data() API (guarded by a mutex and platform checks) to
pull NPU metrics from the SMU, and wiring those metrics into the amdxdna
driver.

v2:
----
 - Split the series into multiple patches per v1 feedback
 - Added preliminary PMF driver cleanups as preparatory patches
 - Introduced an initial amd_xdna patch that consumes PMF-provided data to
   establish the plumbing; additional logic can be layered on in future
   patches as more PMF metrics are utilized.

Shyam Sundar S K (4):
  platform/x86/amd/pmf: Rename IPU metrics fields to NPU for consistency
  platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in
    amd_pmf_send_cmd()
  platform/x86/amd/pmf: replace magic table id with METRICS_TABLE_ID
  platform/x86/amd/pmf: Introduce new interface to export NPU metrics

VinitKumar Shukla (1):
  accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

 drivers/accel/amdxdna/aie2_pci.h         |  2 +
 drivers/accel/amdxdna/aie2_smu.c         | 12 ++++
 drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++---
 drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++---
 drivers/platform/x86/amd/pmf/core.c      | 71 +++++++++++++++++++++++-
 drivers/platform/x86/amd/pmf/pmf.h       | 18 ++++--
 drivers/platform/x86/amd/pmf/spc.c       |  2 +-
 drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++-------
 drivers/platform/x86/amd/pmf/tee-if.c    | 20 +++----
 include/linux/amd-pmf-io.h               | 21 +++++++
 10 files changed, 159 insertions(+), 53 deletions(-)

-- 
2.34.1

[PATCH v2 1/5] platform/x86/amd/pmf: Rename IPU metrics fields to NPU for consistency

[Shyam Sundar S K]

Update smu_pmf_metrics_v2 to use NPU terminology instead of IPU. This
aligns the driver with current firmware/hardware naming and fixes the
mismatch. There is no functional change and no impact to the struct layout
and only field names are updated.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/pmf.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index bd19f2a6bc78..6ea5380f3b23 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -243,12 +243,12 @@ struct smu_pmf_metrics_v2 {
 	u16 vclk_freq;			/* MHz */
 	u16 vcn_activity;		/* VCN busy % [0-100] */
 	u16 vpeclk_freq;		/* MHz */
-	u16 ipuclk_freq;		/* MHz */
-	u16 ipu_busy[8];		/* NPU busy % [0-100] */
+	u16 npuclk_freq;		/* MHz */
+	u16 npu_busy[8];		/* NPU busy % [0-100] */
 	u16 dram_reads;			/* MB/sec */
 	u16 dram_writes;		/* MB/sec */
 	u16 core_c0residency[16];	/* C0 residency % [0-100] */
-	u16 ipu_power;			/* mW */
+	u16 npu_power;			/* mW */
 	u32 apu_power;			/* mW */
 	u32 gfx_power;			/* mW */
 	u32 dgpu_power;			/* mW */
@@ -257,9 +257,9 @@ struct smu_pmf_metrics_v2 {
 	u32 filter_alpha_value;		/* time constant [us] */
 	u32 metrics_counter;
 	u16 memclk_freq;		/* MHz */
-	u16 mpipuclk_freq;		/* MHz */
-	u16 ipu_reads;			/* MB/sec */
-	u16 ipu_writes;			/* MB/sec */
+	u16 mpnpuclk_freq;		/* MHz */
+	u16 npu_reads;			/* MB/sec */
+	u16 npu_writes;			/* MB/sec */
 	u32 throttle_residency_prochot;
 	u32 throttle_residency_spl;
 	u32 throttle_residency_fppt;
-- 
2.34.1

Re: [PATCH v2 1/5] platform/x86/amd/pmf: Rename IPU metrics fields to NPU for consistency

[Ilpo Järvinen]

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

On Tue, 11 Nov 2025, Shyam Sundar S K wrote:

> Update smu_pmf_metrics_v2 to use NPU terminology instead of IPU. This
> aligns the driver with current firmware/hardware naming and fixes the
> mismatch. There is no functional change and no impact to the struct layout
> and only field names are updated.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>

You really needed 2 people to "develop" this?? :-/

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.


> ---
>  drivers/platform/x86/amd/pmf/pmf.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index bd19f2a6bc78..6ea5380f3b23 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -243,12 +243,12 @@ struct smu_pmf_metrics_v2 {
>  	u16 vclk_freq;			/* MHz */
>  	u16 vcn_activity;		/* VCN busy % [0-100] */
>  	u16 vpeclk_freq;		/* MHz */
> -	u16 ipuclk_freq;		/* MHz */
> -	u16 ipu_busy[8];		/* NPU busy % [0-100] */
> +	u16 npuclk_freq;		/* MHz */
> +	u16 npu_busy[8];		/* NPU busy % [0-100] */
>  	u16 dram_reads;			/* MB/sec */
>  	u16 dram_writes;		/* MB/sec */
>  	u16 core_c0residency[16];	/* C0 residency % [0-100] */
> -	u16 ipu_power;			/* mW */
> +	u16 npu_power;			/* mW */
>  	u32 apu_power;			/* mW */
>  	u32 gfx_power;			/* mW */
>  	u32 dgpu_power;			/* mW */
> @@ -257,9 +257,9 @@ struct smu_pmf_metrics_v2 {
>  	u32 filter_alpha_value;		/* time constant [us] */
>  	u32 metrics_counter;
>  	u16 memclk_freq;		/* MHz */
> -	u16 mpipuclk_freq;		/* MHz */
> -	u16 ipu_reads;			/* MB/sec */
> -	u16 ipu_writes;			/* MB/sec */
> +	u16 mpnpuclk_freq;		/* MHz */
> +	u16 npu_reads;			/* MB/sec */
> +	u16 npu_writes;			/* MB/sec */
>  	u32 throttle_residency_prochot;
>  	u32 throttle_residency_spl;
>  	u32 throttle_residency_fppt;
> 

[PATCH v2 2/5] platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in amd_pmf_send_cmd()

[Shyam Sundar S K]

Add SET_CMD and GET_CMD constants and replace boolean values passed as the
get/set argument to amd_pmf_send_cmd() with the new explicit flags. This
improves readability, avoids ambiguity around true/false and 0/1 usage.
There is no functional change.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++++-----
 drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++++-----
 drivers/platform/x86/amd/pmf/core.c      |  6 ++--
 drivers/platform/x86/amd/pmf/pmf.h       |  3 ++
 drivers/platform/x86/amd/pmf/spc.c       |  2 +-
 drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++++++++------------
 drivers/platform/x86/amd/pmf/tee-if.c    | 20 ++++++-------
 7 files changed, 50 insertions(+), 47 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
index a184922bba8d..faf15a8f74bb 100644
--- a/drivers/platform/x86/amd/pmf/auto-mode.c
+++ b/drivers/platform/x86/amd/pmf/auto-mode.c
@@ -114,14 +114,14 @@ static void amd_pmf_set_automode(struct amd_pmf_dev *dev, int idx,
 {
 	struct power_table_control *pwr_ctrl = &config_store.mode_set[idx].power_control;
 
-	amd_pmf_send_cmd(dev, SET_SPL, false, pwr_ctrl->spl, NULL);
-	amd_pmf_send_cmd(dev, SET_FPPT, false, pwr_ctrl->fppt, NULL);
-	amd_pmf_send_cmd(dev, SET_SPPT, false, pwr_ctrl->sppt, NULL);
-	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pwr_ctrl->sppt_apu_only, NULL);
-	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pwr_ctrl->stt_min, NULL);
-	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
+	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pwr_ctrl->spl, NULL);
+	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pwr_ctrl->fppt, NULL);
+	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pwr_ctrl->sppt, NULL);
+	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pwr_ctrl->sppt_apu_only, NULL);
+	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pwr_ctrl->stt_min, NULL);
+	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
 			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_APU]), NULL);
-	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
+	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
 			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_HS2]), NULL);
 
 	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
index 207a0b33d8d3..5469fefb6001 100644
--- a/drivers/platform/x86/amd/pmf/cnqf.c
+++ b/drivers/platform/x86/amd/pmf/cnqf.c
@@ -76,14 +76,14 @@ static int amd_pmf_set_cnqf(struct amd_pmf_dev *dev, int src, int idx,
 
 	pc = &config_store.mode_set[src][idx].power_control;
 
-	amd_pmf_send_cmd(dev, SET_SPL, false, pc->spl, NULL);
-	amd_pmf_send_cmd(dev, SET_FPPT, false, pc->fppt, NULL);
-	amd_pmf_send_cmd(dev, SET_SPPT, false, pc->sppt, NULL);
-	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pc->sppt_apu_only, NULL);
-	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pc->stt_min, NULL);
-	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
+	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pc->spl, NULL);
+	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pc->fppt, NULL);
+	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pc->sppt, NULL);
+	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pc->sppt_apu_only, NULL);
+	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pc->stt_min, NULL);
+	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
 			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_APU]), NULL);
-	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
+	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
 			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_HS2]), NULL);
 
 	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index bc544a4a5266..a659cedc4533 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -131,7 +131,7 @@ static void amd_pmf_get_metrics(struct work_struct *work)
 
 	/* Transfer table contents */
 	memset(dev->buf, 0, sizeof(dev->m_table));
-	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
+	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
 	memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
 
 	time_elapsed_ms = ktime_to_ms(ktime_get()) - dev->start_time;
@@ -289,8 +289,8 @@ int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer)
 	hi = phys_addr >> 32;
 	low = phys_addr & GENMASK(31, 0);
 
-	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
-	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
+	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, SET_CMD, hi, NULL);
+	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, SET_CMD, low, NULL);
 
 	return 0;
 }
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 6ea5380f3b23..19e413bd89bc 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -120,6 +120,9 @@ struct cookie_header {
 #define APTS_MAX_STATES		16
 #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
 
+#define SET_CMD		0
+#define GET_CMD		1
+
 typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
 
 /* APTS PMF BIOS Interface */
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index 85192c7536b8..b10229def18a 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -202,7 +202,7 @@ static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
 {
 	/* Get the updated metrics table data */
 	memset(dev->buf, 0, dev->mtable_size);
-	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
+	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
 
 	switch (dev->cpu_id) {
 	case AMD_CPU_ID_PS:
diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
index c28f3c5744c2..0b70a5153f46 100644
--- a/drivers/platform/x86/amd/pmf/sps.c
+++ b/drivers/platform/x86/amd/pmf/sps.c
@@ -192,15 +192,15 @@ static void amd_pmf_load_defaults_sps(struct amd_pmf_dev *dev)
 
 static void amd_pmf_update_slider_v2(struct amd_pmf_dev *dev, int idx)
 {
-	amd_pmf_send_cmd(dev, SET_PMF_PPT, false, apts_config_store.val[idx].pmf_ppt, NULL);
-	amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false,
+	amd_pmf_send_cmd(dev, SET_PMF_PPT, SET_CMD, apts_config_store.val[idx].pmf_ppt, NULL);
+	amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, SET_CMD,
 			 apts_config_store.val[idx].ppt_pmf_apu_only, NULL);
-	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
+	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD,
 			 apts_config_store.val[idx].stt_min_limit, NULL);
-	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
+	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
 			 fixp_q88_fromint(apts_config_store.val[idx].stt_skin_temp_limit_apu),
 			 NULL);
-	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
+	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
 			 fixp_q88_fromint(apts_config_store.val[idx].stt_skin_temp_limit_hs2),
 			 NULL);
 }
@@ -211,30 +211,30 @@ void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
 	int src = amd_pmf_get_power_source();
 
 	if (op == SLIDER_OP_SET) {
-		amd_pmf_send_cmd(dev, SET_SPL, false, config_store.prop[src][idx].spl, NULL);
-		amd_pmf_send_cmd(dev, SET_FPPT, false, config_store.prop[src][idx].fppt, NULL);
-		amd_pmf_send_cmd(dev, SET_SPPT, false, config_store.prop[src][idx].sppt, NULL);
-		amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false,
+		amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, config_store.prop[src][idx].spl, NULL);
+		amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, config_store.prop[src][idx].fppt, NULL);
+		amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, config_store.prop[src][idx].sppt, NULL);
+		amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD,
 				 config_store.prop[src][idx].sppt_apu_only, NULL);
-		amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
+		amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD,
 				 config_store.prop[src][idx].stt_min, NULL);
-		amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
+		amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
 				 fixp_q88_fromint(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_APU]),
 				 NULL);
-		amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
+		amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
 				 fixp_q88_fromint(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_HS2]),
 				 NULL);
 	} else if (op == SLIDER_OP_GET) {
-		amd_pmf_send_cmd(dev, GET_SPL, true, ARG_NONE, &table->prop[src][idx].spl);
-		amd_pmf_send_cmd(dev, GET_FPPT, true, ARG_NONE, &table->prop[src][idx].fppt);
-		amd_pmf_send_cmd(dev, GET_SPPT, true, ARG_NONE, &table->prop[src][idx].sppt);
-		amd_pmf_send_cmd(dev, GET_SPPT_APU_ONLY, true, ARG_NONE,
+		amd_pmf_send_cmd(dev, GET_SPL, GET_CMD, ARG_NONE, &table->prop[src][idx].spl);
+		amd_pmf_send_cmd(dev, GET_FPPT, GET_CMD, ARG_NONE, &table->prop[src][idx].fppt);
+		amd_pmf_send_cmd(dev, GET_SPPT, GET_CMD, ARG_NONE, &table->prop[src][idx].sppt);
+		amd_pmf_send_cmd(dev, GET_SPPT_APU_ONLY, GET_CMD, ARG_NONE,
 				 &table->prop[src][idx].sppt_apu_only);
-		amd_pmf_send_cmd(dev, GET_STT_MIN_LIMIT, true, ARG_NONE,
+		amd_pmf_send_cmd(dev, GET_STT_MIN_LIMIT, GET_CMD, ARG_NONE,
 				 &table->prop[src][idx].stt_min);
-		amd_pmf_send_cmd(dev, GET_STT_LIMIT_APU, true, ARG_NONE,
+		amd_pmf_send_cmd(dev, GET_STT_LIMIT_APU, GET_CMD, ARG_NONE,
 				 (u32 *)&table->prop[src][idx].stt_skin_temp[STT_TEMP_APU]);
-		amd_pmf_send_cmd(dev, GET_STT_LIMIT_HS2, true, ARG_NONE,
+		amd_pmf_send_cmd(dev, GET_STT_LIMIT_HS2, GET_CMD, ARG_NONE,
 				 (u32 *)&table->prop[src][idx].stt_skin_temp[STT_TEMP_HS2]);
 	}
 }
diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
index 6e8116bef4f6..44fc69cfe10e 100644
--- a/drivers/platform/x86/amd/pmf/tee-if.c
+++ b/drivers/platform/x86/amd/pmf/tee-if.c
@@ -83,7 +83,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 		switch (out->actions_list[idx].action_index) {
 		case PMF_POLICY_SPL:
 			if (dev->prev_data->spl != val) {
-				amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update SPL: %u\n", val);
 				dev->prev_data->spl = val;
 			}
@@ -91,7 +91,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_SPPT:
 			if (dev->prev_data->sppt != val) {
-				amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update SPPT: %u\n", val);
 				dev->prev_data->sppt = val;
 			}
@@ -99,7 +99,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_FPPT:
 			if (dev->prev_data->fppt != val) {
-				amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update FPPT: %u\n", val);
 				dev->prev_data->fppt = val;
 			}
@@ -107,7 +107,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_SPPT_APU_ONLY:
 			if (dev->prev_data->sppt_apuonly != val) {
-				amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update SPPT_APU_ONLY: %u\n", val);
 				dev->prev_data->sppt_apuonly = val;
 			}
@@ -115,7 +115,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_STT_MIN:
 			if (dev->prev_data->stt_minlimit != val) {
-				amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update STT_MIN: %u\n", val);
 				dev->prev_data->stt_minlimit = val;
 			}
@@ -123,7 +123,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_STT_SKINTEMP_APU:
 			if (dev->prev_data->stt_skintemp_apu != val) {
-				amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
+				amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
 						 fixp_q88_fromint(val), NULL);
 				dev_dbg(dev->dev, "update STT_SKINTEMP_APU: %u\n", val);
 				dev->prev_data->stt_skintemp_apu = val;
@@ -132,7 +132,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_STT_SKINTEMP_HS2:
 			if (dev->prev_data->stt_skintemp_hs2 != val) {
-				amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
+				amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
 						 fixp_q88_fromint(val), NULL);
 				dev_dbg(dev->dev, "update STT_SKINTEMP_HS2: %u\n", val);
 				dev->prev_data->stt_skintemp_hs2 = val;
@@ -141,7 +141,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_P3T:
 			if (dev->prev_data->p3t_limit != val) {
-				amd_pmf_send_cmd(dev, SET_P3T, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_P3T, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update P3T: %u\n", val);
 				dev->prev_data->p3t_limit = val;
 			}
@@ -149,7 +149,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_PMF_PPT:
 			if (dev->prev_data->pmf_ppt != val) {
-				amd_pmf_send_cmd(dev, SET_PMF_PPT, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_PMF_PPT, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update PMF PPT: %u\n", val);
 				dev->prev_data->pmf_ppt = val;
 			}
@@ -157,7 +157,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
 
 		case PMF_POLICY_PMF_PPT_APU_ONLY:
 			if (dev->prev_data->pmf_ppt_apu_only != val) {
-				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false, val, NULL);
+				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, SET_CMD, val, NULL);
 				dev_dbg(dev->dev, "update PMF PPT APU ONLY: %u\n", val);
 				dev->prev_data->pmf_ppt_apu_only = val;
 			}
-- 
2.34.1

Re: [PATCH v2 2/5] platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in amd_pmf_send_cmd()

[Ilpo Järvinen]

On Tue, 11 Nov 2025, Shyam Sundar S K wrote:

> Add SET_CMD and GET_CMD constants and replace boolean values passed as the
> get/set argument to amd_pmf_send_cmd() with the new explicit flags. This
> improves readability, avoids ambiguity around true/false and 0/1 usage.
> There is no functional change.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++++-----
>  drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++++-----
>  drivers/platform/x86/amd/pmf/core.c      |  6 ++--
>  drivers/platform/x86/amd/pmf/pmf.h       |  3 ++
>  drivers/platform/x86/amd/pmf/spc.c       |  2 +-
>  drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++++++++------------
>  drivers/platform/x86/amd/pmf/tee-if.c    | 20 ++++++-------
>  7 files changed, 50 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
> index a184922bba8d..faf15a8f74bb 100644
> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
> @@ -114,14 +114,14 @@ static void amd_pmf_set_automode(struct amd_pmf_dev *dev, int idx,
>  {
>  	struct power_table_control *pwr_ctrl = &config_store.mode_set[idx].power_control;
>  
> -	amd_pmf_send_cmd(dev, SET_SPL, false, pwr_ctrl->spl, NULL);
> -	amd_pmf_send_cmd(dev, SET_FPPT, false, pwr_ctrl->fppt, NULL);
> -	amd_pmf_send_cmd(dev, SET_SPPT, false, pwr_ctrl->sppt, NULL);
> -	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pwr_ctrl->sppt_apu_only, NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pwr_ctrl->stt_min, NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> +	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pwr_ctrl->spl, NULL);
> +	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pwr_ctrl->fppt, NULL);
> +	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pwr_ctrl->sppt, NULL);
> +	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pwr_ctrl->sppt_apu_only, NULL);
> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pwr_ctrl->stt_min, NULL);
> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>  			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_APU]), NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>  			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_HS2]), NULL);
>  
>  	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> index 207a0b33d8d3..5469fefb6001 100644
> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> @@ -76,14 +76,14 @@ static int amd_pmf_set_cnqf(struct amd_pmf_dev *dev, int src, int idx,
>  
>  	pc = &config_store.mode_set[src][idx].power_control;
>  
> -	amd_pmf_send_cmd(dev, SET_SPL, false, pc->spl, NULL);
> -	amd_pmf_send_cmd(dev, SET_FPPT, false, pc->fppt, NULL);
> -	amd_pmf_send_cmd(dev, SET_SPPT, false, pc->sppt, NULL);
> -	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pc->sppt_apu_only, NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pc->stt_min, NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> +	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pc->spl, NULL);
> +	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pc->fppt, NULL);
> +	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pc->sppt, NULL);
> +	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pc->sppt_apu_only, NULL);
> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pc->stt_min, NULL);
> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>  			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_APU]), NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>  			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_HS2]), NULL);
>  
>  	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index bc544a4a5266..a659cedc4533 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -131,7 +131,7 @@ static void amd_pmf_get_metrics(struct work_struct *work)
>  
>  	/* Transfer table contents */
>  	memset(dev->buf, 0, sizeof(dev->m_table));
> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
>  	memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
>  
>  	time_elapsed_ms = ktime_to_ms(ktime_get()) - dev->start_time;
> @@ -289,8 +289,8 @@ int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer)
>  	hi = phys_addr >> 32;
>  	low = phys_addr & GENMASK(31, 0);
>  
> -	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
> -	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
> +	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, SET_CMD, hi, NULL);
> +	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, SET_CMD, low, NULL);
>  
>  	return 0;
>  }
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 6ea5380f3b23..19e413bd89bc 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -120,6 +120,9 @@ struct cookie_header {
>  #define APTS_MAX_STATES		16
>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
>  
> +#define SET_CMD		0
> +#define GET_CMD		1

amd_pmf_send_cmd() takes bool so I don't know why these are defined as 
numbers?

Other than that, things look much better than before this change.

--
 i.

>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>  
>  /* APTS PMF BIOS Interface */
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index 85192c7536b8..b10229def18a 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -202,7 +202,7 @@ static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
>  {
>  	/* Get the updated metrics table data */
>  	memset(dev->buf, 0, dev->mtable_size);
> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
>  
>  	switch (dev->cpu_id) {
>  	case AMD_CPU_ID_PS:
> diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
> index c28f3c5744c2..0b70a5153f46 100644
> --- a/drivers/platform/x86/amd/pmf/sps.c
> +++ b/drivers/platform/x86/amd/pmf/sps.c
> @@ -192,15 +192,15 @@ static void amd_pmf_load_defaults_sps(struct amd_pmf_dev *dev)
>  
>  static void amd_pmf_update_slider_v2(struct amd_pmf_dev *dev, int idx)
>  {
> -	amd_pmf_send_cmd(dev, SET_PMF_PPT, false, apts_config_store.val[idx].pmf_ppt, NULL);
> -	amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false,
> +	amd_pmf_send_cmd(dev, SET_PMF_PPT, SET_CMD, apts_config_store.val[idx].pmf_ppt, NULL);
> +	amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, SET_CMD,
>  			 apts_config_store.val[idx].ppt_pmf_apu_only, NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD,
>  			 apts_config_store.val[idx].stt_min_limit, NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>  			 fixp_q88_fromint(apts_config_store.val[idx].stt_skin_temp_limit_apu),
>  			 NULL);
> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>  			 fixp_q88_fromint(apts_config_store.val[idx].stt_skin_temp_limit_hs2),
>  			 NULL);
>  }
> @@ -211,30 +211,30 @@ void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
>  	int src = amd_pmf_get_power_source();
>  
>  	if (op == SLIDER_OP_SET) {
> -		amd_pmf_send_cmd(dev, SET_SPL, false, config_store.prop[src][idx].spl, NULL);
> -		amd_pmf_send_cmd(dev, SET_FPPT, false, config_store.prop[src][idx].fppt, NULL);
> -		amd_pmf_send_cmd(dev, SET_SPPT, false, config_store.prop[src][idx].sppt, NULL);
> -		amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false,
> +		amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, config_store.prop[src][idx].spl, NULL);
> +		amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, config_store.prop[src][idx].fppt, NULL);
> +		amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, config_store.prop[src][idx].sppt, NULL);
> +		amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD,
>  				 config_store.prop[src][idx].sppt_apu_only, NULL);
> -		amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
> +		amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD,
>  				 config_store.prop[src][idx].stt_min, NULL);
> -		amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> +		amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>  				 fixp_q88_fromint(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_APU]),
>  				 NULL);
> -		amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> +		amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>  				 fixp_q88_fromint(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_HS2]),
>  				 NULL);
>  	} else if (op == SLIDER_OP_GET) {
> -		amd_pmf_send_cmd(dev, GET_SPL, true, ARG_NONE, &table->prop[src][idx].spl);
> -		amd_pmf_send_cmd(dev, GET_FPPT, true, ARG_NONE, &table->prop[src][idx].fppt);
> -		amd_pmf_send_cmd(dev, GET_SPPT, true, ARG_NONE, &table->prop[src][idx].sppt);
> -		amd_pmf_send_cmd(dev, GET_SPPT_APU_ONLY, true, ARG_NONE,
> +		amd_pmf_send_cmd(dev, GET_SPL, GET_CMD, ARG_NONE, &table->prop[src][idx].spl);
> +		amd_pmf_send_cmd(dev, GET_FPPT, GET_CMD, ARG_NONE, &table->prop[src][idx].fppt);
> +		amd_pmf_send_cmd(dev, GET_SPPT, GET_CMD, ARG_NONE, &table->prop[src][idx].sppt);
> +		amd_pmf_send_cmd(dev, GET_SPPT_APU_ONLY, GET_CMD, ARG_NONE,
>  				 &table->prop[src][idx].sppt_apu_only);
> -		amd_pmf_send_cmd(dev, GET_STT_MIN_LIMIT, true, ARG_NONE,
> +		amd_pmf_send_cmd(dev, GET_STT_MIN_LIMIT, GET_CMD, ARG_NONE,
>  				 &table->prop[src][idx].stt_min);
> -		amd_pmf_send_cmd(dev, GET_STT_LIMIT_APU, true, ARG_NONE,
> +		amd_pmf_send_cmd(dev, GET_STT_LIMIT_APU, GET_CMD, ARG_NONE,
>  				 (u32 *)&table->prop[src][idx].stt_skin_temp[STT_TEMP_APU]);
> -		amd_pmf_send_cmd(dev, GET_STT_LIMIT_HS2, true, ARG_NONE,
> +		amd_pmf_send_cmd(dev, GET_STT_LIMIT_HS2, GET_CMD, ARG_NONE,
>  				 (u32 *)&table->prop[src][idx].stt_skin_temp[STT_TEMP_HS2]);
>  	}
>  }
> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
> index 6e8116bef4f6..44fc69cfe10e 100644
> --- a/drivers/platform/x86/amd/pmf/tee-if.c
> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
> @@ -83,7 +83,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  		switch (out->actions_list[idx].action_index) {
>  		case PMF_POLICY_SPL:
>  			if (dev->prev_data->spl != val) {
> -				amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update SPL: %u\n", val);
>  				dev->prev_data->spl = val;
>  			}
> @@ -91,7 +91,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_SPPT:
>  			if (dev->prev_data->sppt != val) {
> -				amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update SPPT: %u\n", val);
>  				dev->prev_data->sppt = val;
>  			}
> @@ -99,7 +99,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_FPPT:
>  			if (dev->prev_data->fppt != val) {
> -				amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update FPPT: %u\n", val);
>  				dev->prev_data->fppt = val;
>  			}
> @@ -107,7 +107,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_SPPT_APU_ONLY:
>  			if (dev->prev_data->sppt_apuonly != val) {
> -				amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update SPPT_APU_ONLY: %u\n", val);
>  				dev->prev_data->sppt_apuonly = val;
>  			}
> @@ -115,7 +115,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_STT_MIN:
>  			if (dev->prev_data->stt_minlimit != val) {
> -				amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update STT_MIN: %u\n", val);
>  				dev->prev_data->stt_minlimit = val;
>  			}
> @@ -123,7 +123,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_STT_SKINTEMP_APU:
>  			if (dev->prev_data->stt_skintemp_apu != val) {
> -				amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> +				amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>  						 fixp_q88_fromint(val), NULL);
>  				dev_dbg(dev->dev, "update STT_SKINTEMP_APU: %u\n", val);
>  				dev->prev_data->stt_skintemp_apu = val;
> @@ -132,7 +132,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_STT_SKINTEMP_HS2:
>  			if (dev->prev_data->stt_skintemp_hs2 != val) {
> -				amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> +				amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>  						 fixp_q88_fromint(val), NULL);
>  				dev_dbg(dev->dev, "update STT_SKINTEMP_HS2: %u\n", val);
>  				dev->prev_data->stt_skintemp_hs2 = val;
> @@ -141,7 +141,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_P3T:
>  			if (dev->prev_data->p3t_limit != val) {
> -				amd_pmf_send_cmd(dev, SET_P3T, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_P3T, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update P3T: %u\n", val);
>  				dev->prev_data->p3t_limit = val;
>  			}
> @@ -149,7 +149,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_PMF_PPT:
>  			if (dev->prev_data->pmf_ppt != val) {
> -				amd_pmf_send_cmd(dev, SET_PMF_PPT, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_PMF_PPT, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update PMF PPT: %u\n", val);
>  				dev->prev_data->pmf_ppt = val;
>  			}
> @@ -157,7 +157,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>  
>  		case PMF_POLICY_PMF_PPT_APU_ONLY:
>  			if (dev->prev_data->pmf_ppt_apu_only != val) {
> -				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false, val, NULL);
> +				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, SET_CMD, val, NULL);
>  				dev_dbg(dev->dev, "update PMF PPT APU ONLY: %u\n", val);
>  				dev->prev_data->pmf_ppt_apu_only = val;
>  			}
> 

Re: [PATCH v2 2/5] platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in amd_pmf_send_cmd()

[Shyam Sundar S K]

On 11/18/2025 21:11, Ilpo Järvinen wrote:
> On Tue, 11 Nov 2025, Shyam Sundar S K wrote:
> 
>> Add SET_CMD and GET_CMD constants and replace boolean values passed as the
>> get/set argument to amd_pmf_send_cmd() with the new explicit flags. This
>> improves readability, avoids ambiguity around true/false and 0/1 usage.
>> There is no functional change.
>>
>> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>  drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++++-----
>>  drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++++-----
>>  drivers/platform/x86/amd/pmf/core.c      |  6 ++--
>>  drivers/platform/x86/amd/pmf/pmf.h       |  3 ++
>>  drivers/platform/x86/amd/pmf/spc.c       |  2 +-
>>  drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++++++++------------
>>  drivers/platform/x86/amd/pmf/tee-if.c    | 20 ++++++-------
>>  7 files changed, 50 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
>> index a184922bba8d..faf15a8f74bb 100644
>> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
>> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
>> @@ -114,14 +114,14 @@ static void amd_pmf_set_automode(struct amd_pmf_dev *dev, int idx,
>>  {
>>  	struct power_table_control *pwr_ctrl = &config_store.mode_set[idx].power_control;
>>  
>> -	amd_pmf_send_cmd(dev, SET_SPL, false, pwr_ctrl->spl, NULL);
>> -	amd_pmf_send_cmd(dev, SET_FPPT, false, pwr_ctrl->fppt, NULL);
>> -	amd_pmf_send_cmd(dev, SET_SPPT, false, pwr_ctrl->sppt, NULL);
>> -	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pwr_ctrl->sppt_apu_only, NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pwr_ctrl->stt_min, NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
>> +	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pwr_ctrl->spl, NULL);
>> +	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pwr_ctrl->fppt, NULL);
>> +	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pwr_ctrl->sppt, NULL);
>> +	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pwr_ctrl->sppt_apu_only, NULL);
>> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pwr_ctrl->stt_min, NULL);
>> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>>  			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_APU]), NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
>> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>>  			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_HS2]), NULL);
>>  
>>  	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
>> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
>> index 207a0b33d8d3..5469fefb6001 100644
>> --- a/drivers/platform/x86/amd/pmf/cnqf.c
>> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
>> @@ -76,14 +76,14 @@ static int amd_pmf_set_cnqf(struct amd_pmf_dev *dev, int src, int idx,
>>  
>>  	pc = &config_store.mode_set[src][idx].power_control;
>>  
>> -	amd_pmf_send_cmd(dev, SET_SPL, false, pc->spl, NULL);
>> -	amd_pmf_send_cmd(dev, SET_FPPT, false, pc->fppt, NULL);
>> -	amd_pmf_send_cmd(dev, SET_SPPT, false, pc->sppt, NULL);
>> -	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pc->sppt_apu_only, NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pc->stt_min, NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
>> +	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pc->spl, NULL);
>> +	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pc->fppt, NULL);
>> +	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pc->sppt, NULL);
>> +	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pc->sppt_apu_only, NULL);
>> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pc->stt_min, NULL);
>> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>>  			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_APU]), NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
>> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>>  			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_HS2]), NULL);
>>  
>>  	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
>> index bc544a4a5266..a659cedc4533 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -131,7 +131,7 @@ static void amd_pmf_get_metrics(struct work_struct *work)
>>  
>>  	/* Transfer table contents */
>>  	memset(dev->buf, 0, sizeof(dev->m_table));
>> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
>> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
>>  	memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
>>  
>>  	time_elapsed_ms = ktime_to_ms(ktime_get()) - dev->start_time;
>> @@ -289,8 +289,8 @@ int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer)
>>  	hi = phys_addr >> 32;
>>  	low = phys_addr & GENMASK(31, 0);
>>  
>> -	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
>> -	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
>> +	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, SET_CMD, hi, NULL);
>> +	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, SET_CMD, low, NULL);
>>  
>>  	return 0;
>>  }
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
>> index 6ea5380f3b23..19e413bd89bc 100644
>> --- a/drivers/platform/x86/amd/pmf/pmf.h
>> +++ b/drivers/platform/x86/amd/pmf/pmf.h
>> @@ -120,6 +120,9 @@ struct cookie_header {
>>  #define APTS_MAX_STATES		16
>>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
>>  
>> +#define SET_CMD		0
>> +#define GET_CMD		1
> 
> amd_pmf_send_cmd() takes bool so I don't know why these are defined as 
> numbers?

The signature of amd_pmf_send_cmd() is:

amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32
arg, u32 *data);

However, in the code it’s called with mixed values for the get parameter:

amd_pmf_send_cmd(…, …, true, …, …);
amd_pmf_send_cmd(…, …, false, …, …);
amd_pmf_send_cmd(…, …, 0, …, …);

That inconsistency makes it harder to read. So, I think adding those
two macros  would be useful so callers can use explicit names instead
of 0/1/true/false, which should improve clarity.

If you don’t think this is worth doing, I’m fine dropping the change
from this series.

Thanks,
Shyam

> 
> Other than that, things look much better than before this change.
> 
> --
>  i.
> 
>>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>>  
>>  /* APTS PMF BIOS Interface */
>> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
>> index 85192c7536b8..b10229def18a 100644
>> --- a/drivers/platform/x86/amd/pmf/spc.c
>> +++ b/drivers/platform/x86/amd/pmf/spc.c
>> @@ -202,7 +202,7 @@ static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
>>  {
>>  	/* Get the updated metrics table data */
>>  	memset(dev->buf, 0, dev->mtable_size);
>> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
>> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
>>  
>>  	switch (dev->cpu_id) {
>>  	case AMD_CPU_ID_PS:
>> diff --git a/drivers/platform/x86/amd/pmf/sps.c b/drivers/platform/x86/amd/pmf/sps.c
>> index c28f3c5744c2..0b70a5153f46 100644
>> --- a/drivers/platform/x86/amd/pmf/sps.c
>> +++ b/drivers/platform/x86/amd/pmf/sps.c
>> @@ -192,15 +192,15 @@ static void amd_pmf_load_defaults_sps(struct amd_pmf_dev *dev)
>>  
>>  static void amd_pmf_update_slider_v2(struct amd_pmf_dev *dev, int idx)
>>  {
>> -	amd_pmf_send_cmd(dev, SET_PMF_PPT, false, apts_config_store.val[idx].pmf_ppt, NULL);
>> -	amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false,
>> +	amd_pmf_send_cmd(dev, SET_PMF_PPT, SET_CMD, apts_config_store.val[idx].pmf_ppt, NULL);
>> +	amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, SET_CMD,
>>  			 apts_config_store.val[idx].ppt_pmf_apu_only, NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
>> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD,
>>  			 apts_config_store.val[idx].stt_min_limit, NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
>> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>>  			 fixp_q88_fromint(apts_config_store.val[idx].stt_skin_temp_limit_apu),
>>  			 NULL);
>> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
>> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>>  			 fixp_q88_fromint(apts_config_store.val[idx].stt_skin_temp_limit_hs2),
>>  			 NULL);
>>  }
>> @@ -211,30 +211,30 @@ void amd_pmf_update_slider(struct amd_pmf_dev *dev, bool op, int idx,
>>  	int src = amd_pmf_get_power_source();
>>  
>>  	if (op == SLIDER_OP_SET) {
>> -		amd_pmf_send_cmd(dev, SET_SPL, false, config_store.prop[src][idx].spl, NULL);
>> -		amd_pmf_send_cmd(dev, SET_FPPT, false, config_store.prop[src][idx].fppt, NULL);
>> -		amd_pmf_send_cmd(dev, SET_SPPT, false, config_store.prop[src][idx].sppt, NULL);
>> -		amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false,
>> +		amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, config_store.prop[src][idx].spl, NULL);
>> +		amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, config_store.prop[src][idx].fppt, NULL);
>> +		amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, config_store.prop[src][idx].sppt, NULL);
>> +		amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD,
>>  				 config_store.prop[src][idx].sppt_apu_only, NULL);
>> -		amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false,
>> +		amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD,
>>  				 config_store.prop[src][idx].stt_min, NULL);
>> -		amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
>> +		amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>>  				 fixp_q88_fromint(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_APU]),
>>  				 NULL);
>> -		amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
>> +		amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>>  				 fixp_q88_fromint(config_store.prop[src][idx].stt_skin_temp[STT_TEMP_HS2]),
>>  				 NULL);
>>  	} else if (op == SLIDER_OP_GET) {
>> -		amd_pmf_send_cmd(dev, GET_SPL, true, ARG_NONE, &table->prop[src][idx].spl);
>> -		amd_pmf_send_cmd(dev, GET_FPPT, true, ARG_NONE, &table->prop[src][idx].fppt);
>> -		amd_pmf_send_cmd(dev, GET_SPPT, true, ARG_NONE, &table->prop[src][idx].sppt);
>> -		amd_pmf_send_cmd(dev, GET_SPPT_APU_ONLY, true, ARG_NONE,
>> +		amd_pmf_send_cmd(dev, GET_SPL, GET_CMD, ARG_NONE, &table->prop[src][idx].spl);
>> +		amd_pmf_send_cmd(dev, GET_FPPT, GET_CMD, ARG_NONE, &table->prop[src][idx].fppt);
>> +		amd_pmf_send_cmd(dev, GET_SPPT, GET_CMD, ARG_NONE, &table->prop[src][idx].sppt);
>> +		amd_pmf_send_cmd(dev, GET_SPPT_APU_ONLY, GET_CMD, ARG_NONE,
>>  				 &table->prop[src][idx].sppt_apu_only);
>> -		amd_pmf_send_cmd(dev, GET_STT_MIN_LIMIT, true, ARG_NONE,
>> +		amd_pmf_send_cmd(dev, GET_STT_MIN_LIMIT, GET_CMD, ARG_NONE,
>>  				 &table->prop[src][idx].stt_min);
>> -		amd_pmf_send_cmd(dev, GET_STT_LIMIT_APU, true, ARG_NONE,
>> +		amd_pmf_send_cmd(dev, GET_STT_LIMIT_APU, GET_CMD, ARG_NONE,
>>  				 (u32 *)&table->prop[src][idx].stt_skin_temp[STT_TEMP_APU]);
>> -		amd_pmf_send_cmd(dev, GET_STT_LIMIT_HS2, true, ARG_NONE,
>> +		amd_pmf_send_cmd(dev, GET_STT_LIMIT_HS2, GET_CMD, ARG_NONE,
>>  				 (u32 *)&table->prop[src][idx].stt_skin_temp[STT_TEMP_HS2]);
>>  	}
>>  }
>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c
>> index 6e8116bef4f6..44fc69cfe10e 100644
>> --- a/drivers/platform/x86/amd/pmf/tee-if.c
>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c
>> @@ -83,7 +83,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  		switch (out->actions_list[idx].action_index) {
>>  		case PMF_POLICY_SPL:
>>  			if (dev->prev_data->spl != val) {
>> -				amd_pmf_send_cmd(dev, SET_SPL, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update SPL: %u\n", val);
>>  				dev->prev_data->spl = val;
>>  			}
>> @@ -91,7 +91,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_SPPT:
>>  			if (dev->prev_data->sppt != val) {
>> -				amd_pmf_send_cmd(dev, SET_SPPT, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update SPPT: %u\n", val);
>>  				dev->prev_data->sppt = val;
>>  			}
>> @@ -99,7 +99,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_FPPT:
>>  			if (dev->prev_data->fppt != val) {
>> -				amd_pmf_send_cmd(dev, SET_FPPT, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update FPPT: %u\n", val);
>>  				dev->prev_data->fppt = val;
>>  			}
>> @@ -107,7 +107,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_SPPT_APU_ONLY:
>>  			if (dev->prev_data->sppt_apuonly != val) {
>> -				amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update SPPT_APU_ONLY: %u\n", val);
>>  				dev->prev_data->sppt_apuonly = val;
>>  			}
>> @@ -115,7 +115,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_STT_MIN:
>>  			if (dev->prev_data->stt_minlimit != val) {
>> -				amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update STT_MIN: %u\n", val);
>>  				dev->prev_data->stt_minlimit = val;
>>  			}
>> @@ -123,7 +123,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_STT_SKINTEMP_APU:
>>  			if (dev->prev_data->stt_skintemp_apu != val) {
>> -				amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
>> +				amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
>>  						 fixp_q88_fromint(val), NULL);
>>  				dev_dbg(dev->dev, "update STT_SKINTEMP_APU: %u\n", val);
>>  				dev->prev_data->stt_skintemp_apu = val;
>> @@ -132,7 +132,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_STT_SKINTEMP_HS2:
>>  			if (dev->prev_data->stt_skintemp_hs2 != val) {
>> -				amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
>> +				amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
>>  						 fixp_q88_fromint(val), NULL);
>>  				dev_dbg(dev->dev, "update STT_SKINTEMP_HS2: %u\n", val);
>>  				dev->prev_data->stt_skintemp_hs2 = val;
>> @@ -141,7 +141,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_P3T:
>>  			if (dev->prev_data->p3t_limit != val) {
>> -				amd_pmf_send_cmd(dev, SET_P3T, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_P3T, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update P3T: %u\n", val);
>>  				dev->prev_data->p3t_limit = val;
>>  			}
>> @@ -149,7 +149,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_PMF_PPT:
>>  			if (dev->prev_data->pmf_ppt != val) {
>> -				amd_pmf_send_cmd(dev, SET_PMF_PPT, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_PMF_PPT, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update PMF PPT: %u\n", val);
>>  				dev->prev_data->pmf_ppt = val;
>>  			}
>> @@ -157,7 +157,7 @@ static void amd_pmf_apply_policies(struct amd_pmf_dev *dev, struct ta_pmf_enact_
>>  
>>  		case PMF_POLICY_PMF_PPT_APU_ONLY:
>>  			if (dev->prev_data->pmf_ppt_apu_only != val) {
>> -				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, false, val, NULL);
>> +				amd_pmf_send_cmd(dev, SET_PMF_PPT_APU_ONLY, SET_CMD, val, NULL);
>>  				dev_dbg(dev->dev, "update PMF PPT APU ONLY: %u\n", val);
>>  				dev->prev_data->pmf_ppt_apu_only = val;
>>  			}
>>

Re: [PATCH v2 2/5] platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in amd_pmf_send_cmd()

[Ilpo Järvinen]

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

On Tue, 18 Nov 2025, Shyam Sundar S K wrote:

> 
> 
> On 11/18/2025 21:11, Ilpo Järvinen wrote:
> > On Tue, 11 Nov 2025, Shyam Sundar S K wrote:
> > 
> >> Add SET_CMD and GET_CMD constants and replace boolean values passed as the
> >> get/set argument to amd_pmf_send_cmd() with the new explicit flags. This
> >> improves readability, avoids ambiguity around true/false and 0/1 usage.
> >> There is no functional change.
> >>
> >> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> >> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> >> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> >> ---
> >>  drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++++-----
> >>  drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++++-----
> >>  drivers/platform/x86/amd/pmf/core.c      |  6 ++--
> >>  drivers/platform/x86/amd/pmf/pmf.h       |  3 ++
> >>  drivers/platform/x86/amd/pmf/spc.c       |  2 +-
> >>  drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++++++++------------
> >>  drivers/platform/x86/amd/pmf/tee-if.c    | 20 ++++++-------
> >>  7 files changed, 50 insertions(+), 47 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/amd/pmf/auto-mode.c b/drivers/platform/x86/amd/pmf/auto-mode.c
> >> index a184922bba8d..faf15a8f74bb 100644
> >> --- a/drivers/platform/x86/amd/pmf/auto-mode.c
> >> +++ b/drivers/platform/x86/amd/pmf/auto-mode.c
> >> @@ -114,14 +114,14 @@ static void amd_pmf_set_automode(struct amd_pmf_dev *dev, int idx,
> >>  {
> >>  	struct power_table_control *pwr_ctrl = &config_store.mode_set[idx].power_control;
> >>  
> >> -	amd_pmf_send_cmd(dev, SET_SPL, false, pwr_ctrl->spl, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_FPPT, false, pwr_ctrl->fppt, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_SPPT, false, pwr_ctrl->sppt, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pwr_ctrl->sppt_apu_only, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pwr_ctrl->stt_min, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> >> +	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pwr_ctrl->spl, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pwr_ctrl->fppt, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pwr_ctrl->sppt, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pwr_ctrl->sppt_apu_only, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pwr_ctrl->stt_min, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
> >>  			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_APU]), NULL);
> >> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> >> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
> >>  			 fixp_q88_fromint(pwr_ctrl->stt_skin_temp[STT_TEMP_HS2]), NULL);
> >>  
> >>  	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
> >> diff --git a/drivers/platform/x86/amd/pmf/cnqf.c b/drivers/platform/x86/amd/pmf/cnqf.c
> >> index 207a0b33d8d3..5469fefb6001 100644
> >> --- a/drivers/platform/x86/amd/pmf/cnqf.c
> >> +++ b/drivers/platform/x86/amd/pmf/cnqf.c
> >> @@ -76,14 +76,14 @@ static int amd_pmf_set_cnqf(struct amd_pmf_dev *dev, int src, int idx,
> >>  
> >>  	pc = &config_store.mode_set[src][idx].power_control;
> >>  
> >> -	amd_pmf_send_cmd(dev, SET_SPL, false, pc->spl, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_FPPT, false, pc->fppt, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_SPPT, false, pc->sppt, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, false, pc->sppt_apu_only, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, false, pc->stt_min, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, false,
> >> +	amd_pmf_send_cmd(dev, SET_SPL, SET_CMD, pc->spl, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_FPPT, SET_CMD, pc->fppt, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_SPPT, SET_CMD, pc->sppt, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_SPPT_APU_ONLY, SET_CMD, pc->sppt_apu_only, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_STT_MIN_LIMIT, SET_CMD, pc->stt_min, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_APU, SET_CMD,
> >>  			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_APU]), NULL);
> >> -	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, false,
> >> +	amd_pmf_send_cmd(dev, SET_STT_LIMIT_HS2, SET_CMD,
> >>  			 fixp_q88_fromint(pc->stt_skin_temp[STT_TEMP_HS2]), NULL);
> >>  
> >>  	if (is_apmf_func_supported(dev, APMF_FUNC_SET_FAN_IDX))
> >> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> >> index bc544a4a5266..a659cedc4533 100644
> >> --- a/drivers/platform/x86/amd/pmf/core.c
> >> +++ b/drivers/platform/x86/amd/pmf/core.c
> >> @@ -131,7 +131,7 @@ static void amd_pmf_get_metrics(struct work_struct *work)
> >>  
> >>  	/* Transfer table contents */
> >>  	memset(dev->buf, 0, sizeof(dev->m_table));
> >> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, 0, 7, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
> >>  	memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
> >>  
> >>  	time_elapsed_ms = ktime_to_ms(ktime_get()) - dev->start_time;
> >> @@ -289,8 +289,8 @@ int amd_pmf_set_dram_addr(struct amd_pmf_dev *dev, bool alloc_buffer)
> >>  	hi = phys_addr >> 32;
> >>  	low = phys_addr & GENMASK(31, 0);
> >>  
> >> -	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, 0, hi, NULL);
> >> -	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, 0, low, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_HIGH, SET_CMD, hi, NULL);
> >> +	amd_pmf_send_cmd(dev, SET_DRAM_ADDR_LOW, SET_CMD, low, NULL);
> >>  
> >>  	return 0;
> >>  }
> >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> >> index 6ea5380f3b23..19e413bd89bc 100644
> >> --- a/drivers/platform/x86/amd/pmf/pmf.h
> >> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> >> @@ -120,6 +120,9 @@ struct cookie_header {
> >>  #define APTS_MAX_STATES		16
> >>  #define CUSTOM_BIOS_INPUT_BITS	GENMASK(16, 7)
> >>  
> >> +#define SET_CMD		0
> >> +#define GET_CMD		1
> > 
> > amd_pmf_send_cmd() takes bool so I don't know why these are defined as 
> > numbers?
> 
> The signature of amd_pmf_send_cmd() is:
> 
> amd_pmf_send_cmd(struct amd_pmf_dev *dev, u8 message, bool get, u32
> arg, u32 *data);
> 
> However, in the code it’s called with mixed values for the get parameter:
> 
> amd_pmf_send_cmd(…, …, true, …, …);
> amd_pmf_send_cmd(…, …, false, …, …);
> amd_pmf_send_cmd(…, …, 0, …, …);
> 
> That inconsistency makes it harder to read. So, I think adding those
> two macros  would be useful so callers can use explicit names instead
> of 0/1/true/false, which should improve clarity.
> 
> If you don’t think this is worth doing, I’m fine dropping the change
> from this series.

No, no no. This is very good change, please don't drop it. I just didn't 
understand why those defines don't have:

/* amd_pmf_send_cmd() get/set */
#define SET_CMD           false
#define GET_CMD           true


-- 
 i.

[PATCH v2 3/5] platform/x86/amd/pmf: replace magic table id with METRICS_TABLE_ID

[Shyam Sundar S K]

Add METRICS_TABLE_ID (7) and use it when requesting the metrics
transfer table instead of the hard-coded literal. This clarifies the
meaning of the argument to amd_pmf_send_cmd() and centralizes the table
ID definition for easier maintenance.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/core.c | 2 +-
 drivers/platform/x86/amd/pmf/pmf.h  | 1 +
 drivers/platform/x86/amd/pmf/spc.c  | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index a659cedc4533..a6a5d416edf9 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -131,7 +131,7 @@ static void amd_pmf_get_metrics(struct work_struct *work)
 
 	/* Transfer table contents */
 	memset(dev->buf, 0, sizeof(dev->m_table));
-	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
+	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, METRICS_TABLE_ID, NULL);
 	memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
 
 	time_elapsed_ms = ktime_to_ms(ktime_get()) - dev->start_time;
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 19e413bd89bc..561919b9f671 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -122,6 +122,7 @@ struct cookie_header {
 
 #define SET_CMD		0
 #define GET_CMD		1
+#define METRICS_TABLE_ID	7
 
 typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
 
diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
index b10229def18a..0a37dc6a7950 100644
--- a/drivers/platform/x86/amd/pmf/spc.c
+++ b/drivers/platform/x86/amd/pmf/spc.c
@@ -202,7 +202,7 @@ static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
 {
 	/* Get the updated metrics table data */
 	memset(dev->buf, 0, dev->mtable_size);
-	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
+	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, METRICS_TABLE_ID, NULL);
 
 	switch (dev->cpu_id) {
 	case AMD_CPU_ID_PS:
-- 
2.34.1

Re: [PATCH v2 3/5] platform/x86/amd/pmf: replace magic table id with METRICS_TABLE_ID

[Ilpo Järvinen]

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

On Tue, 11 Nov 2025, Shyam Sundar S K wrote:

> Add METRICS_TABLE_ID (7) and use it when requesting the metrics
> transfer table instead of the hard-coded literal. This clarifies the
> meaning of the argument to amd_pmf_send_cmd() and centralizes the table
> ID definition for easier maintenance.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/core.c | 2 +-
>  drivers/platform/x86/amd/pmf/pmf.h  | 1 +
>  drivers/platform/x86/amd/pmf/spc.c  | 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index a659cedc4533..a6a5d416edf9 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -131,7 +131,7 @@ static void amd_pmf_get_metrics(struct work_struct *work)
>  
>  	/* Transfer table contents */
>  	memset(dev->buf, 0, sizeof(dev->m_table));
> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, METRICS_TABLE_ID, NULL);
>  	memcpy(&dev->m_table, dev->buf, sizeof(dev->m_table));
>  
>  	time_elapsed_ms = ktime_to_ms(ktime_get()) - dev->start_time;
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 19e413bd89bc..561919b9f671 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -122,6 +122,7 @@ struct cookie_header {
>  
>  #define SET_CMD		0
>  #define GET_CMD		1
> +#define METRICS_TABLE_ID	7
>  
>  typedef void (*apmf_event_handler_t)(acpi_handle handle, u32 event, void *data);
>  
> diff --git a/drivers/platform/x86/amd/pmf/spc.c b/drivers/platform/x86/amd/pmf/spc.c
> index b10229def18a..0a37dc6a7950 100644
> --- a/drivers/platform/x86/amd/pmf/spc.c
> +++ b/drivers/platform/x86/amd/pmf/spc.c
> @@ -202,7 +202,7 @@ static void amd_pmf_get_smu_info(struct amd_pmf_dev *dev, struct ta_pmf_enact_ta
>  {
>  	/* Get the updated metrics table data */
>  	memset(dev->buf, 0, dev->mtable_size);
> -	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, 7, NULL);
> +	amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, METRICS_TABLE_ID, NULL);
>  
>  	switch (dev->cpu_id) {
>  	case AMD_CPU_ID_PS:
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>


-- 
 i.

[PATCH v2 4/5] platform/x86/amd/pmf: Introduce new interface to export NPU metrics

[Shyam Sundar S K]

The PMF driver retrieves NPU metrics data from the PMFW. Introduce a new
interface to make NPU metrics accessible to other drivers like AMDXDNA
driver, which can access and utilize this information as needed.

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/platform/x86/amd/pmf/core.c | 65 +++++++++++++++++++++++++++++
 drivers/platform/x86/amd/pmf/pmf.h  |  2 +
 include/linux/amd-pmf-io.h          | 21 ++++++++++
 3 files changed, 88 insertions(+)

diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index a6a5d416edf9..c1c317418633 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -53,6 +53,8 @@ static bool force_load;
 module_param(force_load, bool, 0444);
 MODULE_PARM_DESC(force_load, "Force load this driver on supported older platforms (experimental)");
 
+static struct device *pmf;
+
 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);
@@ -314,6 +316,63 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
 	return 0;
 }
 
+static int is_npu_metrics_supported(struct amd_pmf_dev *pdev)
+{
+	switch (pdev->cpu_id) {
+	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
+	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
+		return 0;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int amd_pmf_get_smu_metrics(struct amd_pmf_dev *dev, struct amd_pmf_npu_metrics *data)
+{
+	int ret, i;
+
+	guard(mutex)(&dev->metrics_mutex);
+
+	if (is_npu_metrics_supported(dev))
+		return -EOPNOTSUPP;
+
+	ret = amd_pmf_set_dram_addr(dev, true);
+	if (ret)
+		return ret;
+
+	memset(dev->buf, 0, dev->mtable_size);
+
+	/* Send SMU command to get NPU metrics */
+	ret = amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, METRICS_TABLE_ID, NULL);
+	if (ret) {
+		dev_err(dev->dev, "SMU command failed to get NPU metrics: %d\n", ret);
+		return ret;
+	}
+
+	memcpy(&dev->m_table_v2, dev->buf, dev->mtable_size);
+
+	data->npuclk_freq = dev->m_table_v2.npuclk_freq;
+	for (i = 0; i < ARRAY_SIZE(data->npu_busy); i++)
+		data->npu_busy[i] = dev->m_table_v2.npu_busy[i];
+	data->npu_power = dev->m_table_v2.npu_power;
+	data->mpnpuclk_freq = dev->m_table_v2.mpnpuclk_freq;
+	data->npu_reads = dev->m_table_v2.npu_reads;
+	data->npu_writes = dev->m_table_v2.npu_writes;
+
+	return 0;
+}
+
+int amd_pmf_get_npu_data(struct amd_pmf_npu_metrics *info)
+{
+	struct amd_pmf_dev *pdev = dev_get_drvdata(pmf);
+
+	if (!pdev || !info)
+		return -EINVAL;
+
+	return amd_pmf_get_smu_metrics(pdev, info);
+}
+EXPORT_SYMBOL_GPL(amd_pmf_get_npu_data);
+
 static int amd_pmf_suspend_handler(struct device *dev)
 {
 	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
@@ -468,6 +527,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
 	mutex_init(&dev->lock);
 	mutex_init(&dev->update_mutex);
 	mutex_init(&dev->cb_mutex);
+	mutex_init(&dev->metrics_mutex);
 
 	apmf_acpi_init(dev);
 	platform_set_drvdata(pdev, dev);
@@ -477,6 +537,8 @@ 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);
 
+	pmf = dev->dev;
+
 	dev_info(dev->dev, "registered PMF device successfully\n");
 
 	return 0;
@@ -494,6 +556,9 @@ static void amd_pmf_remove(struct platform_device *pdev)
 	mutex_destroy(&dev->lock);
 	mutex_destroy(&dev->update_mutex);
 	mutex_destroy(&dev->cb_mutex);
+	mutex_destroy(&dev->metrics_mutex);
+	if (pmf)
+		pmf = NULL;
 }
 
 static const struct attribute_group *amd_pmf_driver_groups[] = {
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 561919b9f671..63de4172a383 100644
--- a/drivers/platform/x86/amd/pmf/pmf.h
+++ b/drivers/platform/x86/amd/pmf/pmf.h
@@ -12,6 +12,7 @@
 #define PMF_H
 
 #include <linux/acpi.h>
+#include <linux/amd-pmf-io.h>
 #include <linux/input.h>
 #include <linux/platform_device.h>
 #include <linux/platform_profile.h>
@@ -410,6 +411,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 */
+	struct mutex metrics_mutex;
 };
 
 struct apmf_sps_prop_granular_v2 {
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 6fa510f419c0..55198d2875cc 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -61,5 +61,26 @@ enum laptop_placement {
 	LP_UNDEFINED,
 };
 
+/**
+ * struct amd_pmf_npu_metrics: Get NPU metrics data from PMF driver
+ * @npuclk_freq: NPU clock frequency [MHz]
+ * @npu_busy: NPU busy % [0-100]
+ * @npu_power: NPU power [mW]
+ * @mpnpuclk_freq: MPNPU [MHz]
+ * @npu_reads: NPU read bandwidth [MB/sec]
+ * @npu_writes: NPU write bandwidth [MB/sec]
+ */
+struct amd_pmf_npu_metrics {
+	u16 npuclk_freq;
+	u16 npu_busy[8];
+	u16 npu_power;
+	u16 mpnpuclk_freq;
+	u16 npu_reads;
+	u16 npu_writes;
+};
+
 int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
+
+/* AMD PMF and NPU interface */
+int amd_pmf_get_npu_data(struct amd_pmf_npu_metrics *info);
 #endif
-- 
2.34.1

Re: [PATCH v2 4/5] platform/x86/amd/pmf: Introduce new interface to export NPU metrics

[Ilpo Järvinen]

On Tue, 11 Nov 2025, Shyam Sundar S K wrote:

> The PMF driver retrieves NPU metrics data from the PMFW. Introduce a new
> interface to make NPU metrics accessible to other drivers like AMDXDNA
> driver, which can access and utilize this information as needed.
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>  drivers/platform/x86/amd/pmf/core.c | 65 +++++++++++++++++++++++++++++
>  drivers/platform/x86/amd/pmf/pmf.h  |  2 +
>  include/linux/amd-pmf-io.h          | 21 ++++++++++
>  3 files changed, 88 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index a6a5d416edf9..c1c317418633 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -53,6 +53,8 @@ static bool force_load;
>  module_param(force_load, bool, 0444);
>  MODULE_PARM_DESC(force_load, "Force load this driver on supported older platforms (experimental)");
>  
> +static struct device *pmf;
> +
>  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);
> @@ -314,6 +316,63 @@ int amd_pmf_init_metrics_table(struct amd_pmf_dev *dev)
>  	return 0;
>  }
>  
> +static int is_npu_metrics_supported(struct amd_pmf_dev *pdev)
> +{
> +	switch (pdev->cpu_id) {
> +	case PCI_DEVICE_ID_AMD_1AH_M20H_ROOT:
> +	case PCI_DEVICE_ID_AMD_1AH_M60H_ROOT:
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int amd_pmf_get_smu_metrics(struct amd_pmf_dev *dev, struct amd_pmf_npu_metrics *data)
> +{
> +	int ret, i;
> +
> +	guard(mutex)(&dev->metrics_mutex);
> +
> +	if (is_npu_metrics_supported(dev))
> +		return -EOPNOTSUPP;
> +
> +	ret = amd_pmf_set_dram_addr(dev, true);
> +	if (ret)
> +		return ret;
> +
> +	memset(dev->buf, 0, dev->mtable_size);
> +
> +	/* Send SMU command to get NPU metrics */
> +	ret = amd_pmf_send_cmd(dev, SET_TRANSFER_TABLE, SET_CMD, METRICS_TABLE_ID, NULL);
> +	if (ret) {
> +		dev_err(dev->dev, "SMU command failed to get NPU metrics: %d\n", ret);
> +		return ret;
> +	}
> +
> +	memcpy(&dev->m_table_v2, dev->buf, dev->mtable_size);
> +
> +	data->npuclk_freq = dev->m_table_v2.npuclk_freq;
> +	for (i = 0; i < ARRAY_SIZE(data->npu_busy); i++)
> +		data->npu_busy[i] = dev->m_table_v2.npu_busy[i];
> +	data->npu_power = dev->m_table_v2.npu_power;
> +	data->mpnpuclk_freq = dev->m_table_v2.mpnpuclk_freq;
> +	data->npu_reads = dev->m_table_v2.npu_reads;
> +	data->npu_writes = dev->m_table_v2.npu_writes;
> +
> +	return 0;
> +}
> +
> +int amd_pmf_get_npu_data(struct amd_pmf_npu_metrics *info)
> +{
> +	struct amd_pmf_dev *pdev = dev_get_drvdata(pmf);
> +
> +	if (!pdev || !info)
> +		return -EINVAL;
> +
> +	return amd_pmf_get_smu_metrics(pdev, info);
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_get_npu_data);
> +
>  static int amd_pmf_suspend_handler(struct device *dev)
>  {
>  	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> @@ -468,6 +527,7 @@ static int amd_pmf_probe(struct platform_device *pdev)
>  	mutex_init(&dev->lock);
>  	mutex_init(&dev->update_mutex);
>  	mutex_init(&dev->cb_mutex);
> +	mutex_init(&dev->metrics_mutex);

As mentioned with the other series, you should use devm to init these 
mutexes.

>  
>  	apmf_acpi_init(dev);
>  	platform_set_drvdata(pdev, dev);
> @@ -477,6 +537,8 @@ 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);
>  
> +	pmf = dev->dev;
> +
>  	dev_info(dev->dev, "registered PMF device successfully\n");
>  
>  	return 0;
> @@ -494,6 +556,9 @@ static void amd_pmf_remove(struct platform_device *pdev)
>  	mutex_destroy(&dev->lock);
>  	mutex_destroy(&dev->update_mutex);
>  	mutex_destroy(&dev->cb_mutex);
> +	mutex_destroy(&dev->metrics_mutex);
> +	if (pmf)
> +		pmf = NULL;

Why's this needed?

-- 
 i.

>  }
>  
>  static const struct attribute_group *amd_pmf_driver_groups[] = {
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 561919b9f671..63de4172a383 100644
> --- a/drivers/platform/x86/amd/pmf/pmf.h
> +++ b/drivers/platform/x86/amd/pmf/pmf.h
> @@ -12,6 +12,7 @@
>  #define PMF_H
>  
>  #include <linux/acpi.h>
> +#include <linux/amd-pmf-io.h>
>  #include <linux/input.h>
>  #include <linux/platform_device.h>
>  #include <linux/platform_profile.h>
> @@ -410,6 +411,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 */
> +	struct mutex metrics_mutex;
>  };
>  
>  struct apmf_sps_prop_granular_v2 {
> diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
> index 6fa510f419c0..55198d2875cc 100644
> --- a/include/linux/amd-pmf-io.h
> +++ b/include/linux/amd-pmf-io.h
> @@ -61,5 +61,26 @@ enum laptop_placement {
>  	LP_UNDEFINED,
>  };
>  
> +/**
> + * struct amd_pmf_npu_metrics: Get NPU metrics data from PMF driver
> + * @npuclk_freq: NPU clock frequency [MHz]
> + * @npu_busy: NPU busy % [0-100]
> + * @npu_power: NPU power [mW]
> + * @mpnpuclk_freq: MPNPU [MHz]
> + * @npu_reads: NPU read bandwidth [MB/sec]
> + * @npu_writes: NPU write bandwidth [MB/sec]
> + */
> +struct amd_pmf_npu_metrics {
> +	u16 npuclk_freq;
> +	u16 npu_busy[8];
> +	u16 npu_power;
> +	u16 mpnpuclk_freq;
> +	u16 npu_reads;
> +	u16 npu_writes;
> +};
> +
>  int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
> +
> +/* AMD PMF and NPU interface */
> +int amd_pmf_get_npu_data(struct amd_pmf_npu_metrics *info);
>  #endif
> 

[PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Shyam Sundar S K]

From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>

Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
AMD PMF driver. This xdna interface enables user space to reflect current
workload power consumption in real time.

Signed-off-by: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
---
 drivers/accel/amdxdna/aie2_pci.h |  2 ++
 drivers/accel/amdxdna/aie2_smu.c | 12 ++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
index 91a8e948f82a..5601de841b5b 100644
--- a/drivers/accel/amdxdna/aie2_pci.h
+++ b/drivers/accel/amdxdna/aie2_pci.h
@@ -8,6 +8,7 @@
 
 #include <drm/amdxdna_accel.h>
 #include <linux/semaphore.h>
+#include <linux/amd-pmf-io.h>
 
 #include "amdxdna_mailbox.h"
 
@@ -240,6 +241,7 @@ int aie2_smu_init(struct amdxdna_dev_hdl *ndev);
 void aie2_smu_fini(struct amdxdna_dev_hdl *ndev);
 int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
 int npu4_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
+int aie2_smu_get_power_estimate(void);
 
 /* aie2_pm.c */
 int aie2_pm_init(struct amdxdna_dev_hdl *ndev);
diff --git a/drivers/accel/amdxdna/aie2_smu.c b/drivers/accel/amdxdna/aie2_smu.c
index d303701b0ded..7b5e2c383dd3 100644
--- a/drivers/accel/amdxdna/aie2_smu.c
+++ b/drivers/accel/amdxdna/aie2_smu.c
@@ -54,6 +54,18 @@ static int aie2_smu_exec(struct amdxdna_dev_hdl *ndev, u32 reg_cmd,
 	return 0;
 }
 
+int aie2_smu_get_power_estimate(void)
+{
+	struct amd_pmf_npu_metrics npu_metrics;
+	int ret;
+
+	ret = amd_pmf_get_npu_data(&npu_metrics);
+	if (ret)
+		return ret;
+
+	return npu_metrics.npu_power;
+}
+
 int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
 {
 	u32 freq;
-- 
2.34.1

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Mario Limonciello]

On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
> 
> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
> AMD PMF driver. This xdna interface enables user space to reflect current
> workload power consumption in real time.

But.. it doesn't.  It just adds a new function that could potentially do 
this job.  The actual gluing it to userspace in some way to use this 
function will be another patch.

> 
> Signed-off-by: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> ---
>   drivers/accel/amdxdna/aie2_pci.h |  2 ++
>   drivers/accel/amdxdna/aie2_smu.c | 12 ++++++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/amdxdna/aie2_pci.h
> index 91a8e948f82a..5601de841b5b 100644
> --- a/drivers/accel/amdxdna/aie2_pci.h
> +++ b/drivers/accel/amdxdna/aie2_pci.h
> @@ -8,6 +8,7 @@
>   
>   #include <drm/amdxdna_accel.h>
>   #include <linux/semaphore.h>
> +#include <linux/amd-pmf-io.h>
>   
>   #include "amdxdna_mailbox.h"
>   
> @@ -240,6 +241,7 @@ int aie2_smu_init(struct amdxdna_dev_hdl *ndev);
>   void aie2_smu_fini(struct amdxdna_dev_hdl *ndev);
>   int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
>   int npu4_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
> +int aie2_smu_get_power_estimate(void);
>   
>   /* aie2_pm.c */
>   int aie2_pm_init(struct amdxdna_dev_hdl *ndev);
> diff --git a/drivers/accel/amdxdna/aie2_smu.c b/drivers/accel/amdxdna/aie2_smu.c
> index d303701b0ded..7b5e2c383dd3 100644
> --- a/drivers/accel/amdxdna/aie2_smu.c
> +++ b/drivers/accel/amdxdna/aie2_smu.c
> @@ -54,6 +54,18 @@ static int aie2_smu_exec(struct amdxdna_dev_hdl *ndev, u32 reg_cmd,
>   	return 0;
>   }
>   
> +int aie2_smu_get_power_estimate(void)
> +{
> +	struct amd_pmf_npu_metrics npu_metrics;
> +	int ret;
> +
> +	ret = amd_pmf_get_npu_data(&npu_metrics);
> +	if (ret)
> +		return ret;
> +
> +	return npu_metrics.npu_power;
> +}
> +
>   int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
>   {
>   	u32 freq;

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Shyam Sundar S K]

On 11/12/2025 23:33, Mario Limonciello wrote:
> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
>> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>>
>> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
>> AMD PMF driver. This xdna interface enables user space to reflect
>> current
>> workload power consumption in real time.
> 
> But.. it doesn't.  It just adds a new function that could potentially
> do this job.  The actual gluing it to userspace in some way to use
> this function will be another patch.

For now, yes.. that's right. XDNA team will add more support on it
that goes via the accel tree. But this patch is meant to have a
minimal change on the xdna side so that there is a  consumer of the
pmf symbol that is getting exported to the kernel.

So, there is more support coming on the way. If you think, its worth
to rephrase the commit?

Thanks,
Shyam
> 
>>
>> Signed-off-by: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>> ---
>>   drivers/accel/amdxdna/aie2_pci.h |  2 ++
>>   drivers/accel/amdxdna/aie2_smu.c | 12 ++++++++++++
>>   2 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/
>> amdxdna/aie2_pci.h
>> index 91a8e948f82a..5601de841b5b 100644
>> --- a/drivers/accel/amdxdna/aie2_pci.h
>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>> @@ -8,6 +8,7 @@
>>     #include <drm/amdxdna_accel.h>
>>   #include <linux/semaphore.h>
>> +#include <linux/amd-pmf-io.h>
>>     #include "amdxdna_mailbox.h"
>>   @@ -240,6 +241,7 @@ int aie2_smu_init(struct amdxdna_dev_hdl *ndev);
>>   void aie2_smu_fini(struct amdxdna_dev_hdl *ndev);
>>   int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
>>   int npu4_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
>> +int aie2_smu_get_power_estimate(void);
>>     /* aie2_pm.c */
>>   int aie2_pm_init(struct amdxdna_dev_hdl *ndev);
>> diff --git a/drivers/accel/amdxdna/aie2_smu.c b/drivers/accel/
>> amdxdna/aie2_smu.c
>> index d303701b0ded..7b5e2c383dd3 100644
>> --- a/drivers/accel/amdxdna/aie2_smu.c
>> +++ b/drivers/accel/amdxdna/aie2_smu.c
>> @@ -54,6 +54,18 @@ static int aie2_smu_exec(struct amdxdna_dev_hdl
>> *ndev, u32 reg_cmd,
>>       return 0;
>>   }
>>   +int aie2_smu_get_power_estimate(void)
>> +{
>> +    struct amd_pmf_npu_metrics npu_metrics;
>> +    int ret;
>> +
>> +    ret = amd_pmf_get_npu_data(&npu_metrics);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return npu_metrics.npu_power;
>> +}
>> +
>>   int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
>>   {
>>       u32 freq;
> 

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Mario Limonciello]

On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
> 
> 
> On 11/12/2025 23:33, Mario Limonciello wrote:
>> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
>>> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>>>
>>> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
>>> AMD PMF driver. This xdna interface enables user space to reflect
>>> current
>>> workload power consumption in real time.
>>
>> But.. it doesn't.  It just adds a new function that could potentially
>> do this job.  The actual gluing it to userspace in some way to use
>> this function will be another patch.
> 
> For now, yes.. that's right. XDNA team will add more support on it
> that goes via the accel tree. But this patch is meant to have a
> minimal change on the xdna side so that there is a  consumer of the
> pmf symbol that is getting exported to the kernel.
> 
> So, there is more support coming on the way. If you think, its worth
> to rephrase the commit?

Yes; that other support isn't happening this kernel cycle.  So the 
commit message should be accurate to what it's actually doing (laying 
ground work).

> 
> Thanks,
> Shyam
>>
>>>
>>> Signed-off-by: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>> ---
>>>    drivers/accel/amdxdna/aie2_pci.h |  2 ++
>>>    drivers/accel/amdxdna/aie2_smu.c | 12 ++++++++++++
>>>    2 files changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/
>>> amdxdna/aie2_pci.h
>>> index 91a8e948f82a..5601de841b5b 100644
>>> --- a/drivers/accel/amdxdna/aie2_pci.h
>>> +++ b/drivers/accel/amdxdna/aie2_pci.h
>>> @@ -8,6 +8,7 @@
>>>      #include <drm/amdxdna_accel.h>
>>>    #include <linux/semaphore.h>
>>> +#include <linux/amd-pmf-io.h>
>>>      #include "amdxdna_mailbox.h"
>>>    @@ -240,6 +241,7 @@ int aie2_smu_init(struct amdxdna_dev_hdl *ndev);
>>>    void aie2_smu_fini(struct amdxdna_dev_hdl *ndev);
>>>    int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
>>>    int npu4_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
>>> +int aie2_smu_get_power_estimate(void);
>>>      /* aie2_pm.c */
>>>    int aie2_pm_init(struct amdxdna_dev_hdl *ndev);
>>> diff --git a/drivers/accel/amdxdna/aie2_smu.c b/drivers/accel/
>>> amdxdna/aie2_smu.c
>>> index d303701b0ded..7b5e2c383dd3 100644
>>> --- a/drivers/accel/amdxdna/aie2_smu.c
>>> +++ b/drivers/accel/amdxdna/aie2_smu.c
>>> @@ -54,6 +54,18 @@ static int aie2_smu_exec(struct amdxdna_dev_hdl
>>> *ndev, u32 reg_cmd,
>>>        return 0;
>>>    }
>>>    +int aie2_smu_get_power_estimate(void)
>>> +{
>>> +    struct amd_pmf_npu_metrics npu_metrics;
>>> +    int ret;
>>> +
>>> +    ret = amd_pmf_get_npu_data(&npu_metrics);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return npu_metrics.npu_power;
>>> +}
>>> +
>>>    int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
>>>    {
>>>        u32 freq;
>>
> 

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Ilpo Järvinen]

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

On Fri, 14 Nov 2025, Mario Limonciello wrote:
> On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
> > On 11/12/2025 23:33, Mario Limonciello wrote:
> > > On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
> > > > From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
> > > > 
> > > > Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
> > > > AMD PMF driver. This xdna interface enables user space to reflect
> > > > current
> > > > workload power consumption in real time.
> > > 
> > > But.. it doesn't.  It just adds a new function that could potentially
> > > do this job.  The actual gluing it to userspace in some way to use
> > > this function will be another patch.
> > 
> > For now, yes.. that's right. XDNA team will add more support on it
> > that goes via the accel tree. But this patch is meant to have a
> > minimal change on the xdna side so that there is a  consumer of the
> > pmf symbol that is getting exported to the kernel.
> > 
> > So, there is more support coming on the way. If you think, its worth
> > to rephrase the commit?
> 
> Yes; that other support isn't happening this kernel cycle.  So the commit
> message should be accurate to what it's actually doing (laying ground work).

To me it looks this and patch 4 should be submitted with the actual user 
instead of trying to justify things with far in the future changes. I've 
no problem with creating immutable branch if necessary when multiple trees 
are involved.

-- 
 i.

> > > > Signed-off-by: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
> > > > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> > > > ---
> > > >    drivers/accel/amdxdna/aie2_pci.h |  2 ++
> > > >    drivers/accel/amdxdna/aie2_smu.c | 12 ++++++++++++
> > > >    2 files changed, 14 insertions(+)
> > > > 
> > > > diff --git a/drivers/accel/amdxdna/aie2_pci.h b/drivers/accel/
> > > > amdxdna/aie2_pci.h
> > > > index 91a8e948f82a..5601de841b5b 100644
> > > > --- a/drivers/accel/amdxdna/aie2_pci.h
> > > > +++ b/drivers/accel/amdxdna/aie2_pci.h
> > > > @@ -8,6 +8,7 @@
> > > >      #include <drm/amdxdna_accel.h>
> > > >    #include <linux/semaphore.h>
> > > > +#include <linux/amd-pmf-io.h>
> > > >      #include "amdxdna_mailbox.h"
> > > >    @@ -240,6 +241,7 @@ int aie2_smu_init(struct amdxdna_dev_hdl *ndev);
> > > >    void aie2_smu_fini(struct amdxdna_dev_hdl *ndev);
> > > >    int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
> > > >    int npu4_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level);
> > > > +int aie2_smu_get_power_estimate(void);
> > > >      /* aie2_pm.c */
> > > >    int aie2_pm_init(struct amdxdna_dev_hdl *ndev);
> > > > diff --git a/drivers/accel/amdxdna/aie2_smu.c b/drivers/accel/
> > > > amdxdna/aie2_smu.c
> > > > index d303701b0ded..7b5e2c383dd3 100644
> > > > --- a/drivers/accel/amdxdna/aie2_smu.c
> > > > +++ b/drivers/accel/amdxdna/aie2_smu.c
> > > > @@ -54,6 +54,18 @@ static int aie2_smu_exec(struct amdxdna_dev_hdl
> > > > *ndev, u32 reg_cmd,
> > > >        return 0;
> > > >    }
> > > >    +int aie2_smu_get_power_estimate(void)
> > > > +{
> > > > +    struct amd_pmf_npu_metrics npu_metrics;
> > > > +    int ret;
> > > > +
> > > > +    ret = amd_pmf_get_npu_data(&npu_metrics);
> > > > +    if (ret)
> > > > +        return ret;
> > > > +
> > > > +    return npu_metrics.npu_power;
> > > > +}
> > > > +
> > > >    int npu1_set_dpm(struct amdxdna_dev_hdl *ndev, u32 dpm_level)
> > > >    {
> > > >        u32 freq;
> > > 
> > 
> 

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Ilpo Järvinen]

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

On Tue, 18 Nov 2025, Ilpo Järvinen wrote:

> On Fri, 14 Nov 2025, Mario Limonciello wrote:
> > On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
> > > On 11/12/2025 23:33, Mario Limonciello wrote:
> > > > On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
> > > > > From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
> > > > > 
> > > > > Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
> > > > > AMD PMF driver. This xdna interface enables user space to reflect
> > > > > current
> > > > > workload power consumption in real time.
> > > > 
> > > > But.. it doesn't.  It just adds a new function that could potentially
> > > > do this job.  The actual gluing it to userspace in some way to use
> > > > this function will be another patch.
> > > 
> > > For now, yes.. that's right. XDNA team will add more support on it
> > > that goes via the accel tree. But this patch is meant to have a
> > > minimal change on the xdna side so that there is a  consumer of the
> > > pmf symbol that is getting exported to the kernel.
> > > 
> > > So, there is more support coming on the way. If you think, its worth
> > > to rephrase the commit?
> > 
> > Yes; that other support isn't happening this kernel cycle.  So the commit
> > message should be accurate to what it's actually doing (laying ground work).
> 
> To me it looks this and patch 4 should be submitted with the actual user 
> instead of trying to justify things with far in the future changes. I've 
> no problem with creating immutable branch if necessary when multiple trees 
> are involved.

In case it wasn't obvious, I'm fine with taking patches 1-3 in earlier.

-- 
 i.

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Shyam Sundar S K]

On 11/18/2025 21:31, Ilpo Järvinen wrote:
> On Tue, 18 Nov 2025, Ilpo Järvinen wrote:
> 
>> On Fri, 14 Nov 2025, Mario Limonciello wrote:
>>> On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
>>>> On 11/12/2025 23:33, Mario Limonciello wrote:
>>>>> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
>>>>>> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>>>>>>
>>>>>> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
>>>>>> AMD PMF driver. This xdna interface enables user space to reflect
>>>>>> current
>>>>>> workload power consumption in real time.
>>>>>
>>>>> But.. it doesn't.  It just adds a new function that could potentially
>>>>> do this job.  The actual gluing it to userspace in some way to use
>>>>> this function will be another patch.
>>>>
>>>> For now, yes.. that's right. XDNA team will add more support on it
>>>> that goes via the accel tree. But this patch is meant to have a
>>>> minimal change on the xdna side so that there is a  consumer of the
>>>> pmf symbol that is getting exported to the kernel.
>>>>
>>>> So, there is more support coming on the way. If you think, its worth
>>>> to rephrase the commit?
>>>
>>> Yes; that other support isn't happening this kernel cycle.  So the commit
>>> message should be accurate to what it's actually doing (laying ground work).
>>
>> To me it looks this and patch 4 should be submitted with the actual user 
>> instead of trying to justify things with far in the future changes. I've 
>> no problem with creating immutable branch if necessary when multiple trees 
>> are involved.
> 
> In case it wasn't obvious, I'm fine with taking patches 1-3 in earlier.
> 

The context for this series is that the XDNA driver needs
power-related information, which is only available from the PMF
driver. We’re providing that information via an exported symbol.

Patch 4 exports the data and patch 5 begins consuming it. The goal is
to establish the channel through which power-related information can flow.

Creating an immutable branch is a reasonable idea, but in this case
all further PMF-side work stops after this patch. The subsequent
activity is primarily on the XDNA side — for example, passing the
power data into the XRT environment via IOCTLs. If both trees had
active development or large, simultaneous changes, an immutable branch
would make sense, but given the current situation it doesn’t seem
worth the added complexity.

The XDNA changes are distributed and will be implemented gradually;
they depend on this exported power information from PMF. So my
proposal is for PMF to export the information now with minimal changes
on the XDNA side, and land that upstream. The XDNA team can then build
on top of the exported interface, and their changes can be submitted
via the misc-drm tree.

Ilpo, does this approach work for you?

Thanks,
Shyam

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Ilpo Järvinen]

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

On Tue, 18 Nov 2025, Shyam Sundar S K wrote:
> On 11/18/2025 21:31, Ilpo Järvinen wrote:
> > On Tue, 18 Nov 2025, Ilpo Järvinen wrote:
> > 
> >> On Fri, 14 Nov 2025, Mario Limonciello wrote:
> >>> On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
> >>>> On 11/12/2025 23:33, Mario Limonciello wrote:
> >>>>> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
> >>>>>> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
> >>>>>>
> >>>>>> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
> >>>>>> AMD PMF driver. This xdna interface enables user space to reflect
> >>>>>> current
> >>>>>> workload power consumption in real time.
> >>>>>
> >>>>> But.. it doesn't.  It just adds a new function that could potentially
> >>>>> do this job.  The actual gluing it to userspace in some way to use
> >>>>> this function will be another patch.
> >>>>
> >>>> For now, yes.. that's right. XDNA team will add more support on it
> >>>> that goes via the accel tree. But this patch is meant to have a
> >>>> minimal change on the xdna side so that there is a  consumer of the
> >>>> pmf symbol that is getting exported to the kernel.
> >>>>
> >>>> So, there is more support coming on the way. If you think, its worth
> >>>> to rephrase the commit?
> >>>
> >>> Yes; that other support isn't happening this kernel cycle.  So the commit
> >>> message should be accurate to what it's actually doing (laying ground work).
> >>
> >> To me it looks this and patch 4 should be submitted with the actual user 
> >> instead of trying to justify things with far in the future changes. I've 
> >> no problem with creating immutable branch if necessary when multiple trees 
> >> are involved.
> > 
> > In case it wasn't obvious, I'm fine with taking patches 1-3 in earlier.
> > 
> 
> The context for this series is that the XDNA driver needs
> power-related information, which is only available from the PMF
> driver. We’re providing that information via an exported symbol.
> Patch 4 exports the data and patch 5 begins consuming it. The goal is
> to establish the channel through which power-related information can flow.

It doesn't consume anything, it just adds function without apparent 
purpose (and this will remain so until the other work is submitted).

> Creating an immutable branch is a reasonable idea, but in this case
> all further PMF-side work stops after this patch. The subsequent
> activity is primarily on the XDNA side — for example, passing the
> power data into the XRT environment via IOCTLs. If both trees had
> active development or large, simultaneous changes, an immutable branch
> would make sense, but given the current situation it doesn’t seem
> worth the added complexity.
>
> The XDNA changes are distributed and will be implemented gradually;
> they depend on this exported power information from PMF. So my
> proposal is for PMF to export the information now with minimal changes
> on the XDNA side, and land that upstream. The XDNA team can then build
> on top of the exported interface, and their changes can be submitted
> via the misc-drm tree.
> 
> Ilpo, does this approach work for you?

Lets just wait with these two patches until have real consumer changes 
ready to be submitted, these patches can appear at the head of such a 
series.

-- 
 i.

Re: [PATCH v2 5/5] accel/amdxdna: Provide real-time NPU power estimate via AMD PMF

[Shyam Sundar S K]

On 11/19/2025 18:14, Ilpo Järvinen wrote:
> On Tue, 18 Nov 2025, Shyam Sundar S K wrote:
>> On 11/18/2025 21:31, Ilpo Järvinen wrote:
>>> On Tue, 18 Nov 2025, Ilpo Järvinen wrote:
>>>
>>>> On Fri, 14 Nov 2025, Mario Limonciello wrote:
>>>>> On 11/13/2025 1:33 AM, Shyam Sundar S K wrote:
>>>>>> On 11/12/2025 23:33, Mario Limonciello wrote:
>>>>>>> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
>>>>>>>> From: VinitKumar Shukla <VinitKumar.Shukla@amd.com>
>>>>>>>>
>>>>>>>> Add aie2_smu_get_power_estimate() to obtain NPU power readings from the
>>>>>>>> AMD PMF driver. This xdna interface enables user space to reflect
>>>>>>>> current
>>>>>>>> workload power consumption in real time.
>>>>>>>
>>>>>>> But.. it doesn't.  It just adds a new function that could potentially
>>>>>>> do this job.  The actual gluing it to userspace in some way to use
>>>>>>> this function will be another patch.
>>>>>>
>>>>>> For now, yes.. that's right. XDNA team will add more support on it
>>>>>> that goes via the accel tree. But this patch is meant to have a
>>>>>> minimal change on the xdna side so that there is a  consumer of the
>>>>>> pmf symbol that is getting exported to the kernel.
>>>>>>
>>>>>> So, there is more support coming on the way. If you think, its worth
>>>>>> to rephrase the commit?
>>>>>
>>>>> Yes; that other support isn't happening this kernel cycle.  So the commit
>>>>> message should be accurate to what it's actually doing (laying ground work).
>>>>
>>>> To me it looks this and patch 4 should be submitted with the actual user 
>>>> instead of trying to justify things with far in the future changes. I've 
>>>> no problem with creating immutable branch if necessary when multiple trees 
>>>> are involved.
>>>
>>> In case it wasn't obvious, I'm fine with taking patches 1-3 in earlier.
>>>
>>
>> The context for this series is that the XDNA driver needs
>> power-related information, which is only available from the PMF
>> driver. We’re providing that information via an exported symbol.
>> Patch 4 exports the data and patch 5 begins consuming it. The goal is
>> to establish the channel through which power-related information can flow.
> 
> It doesn't consume anything, it just adds function without apparent 
> purpose (and this will remain so until the other work is submitted).
> 
>> Creating an immutable branch is a reasonable idea, but in this case
>> all further PMF-side work stops after this patch. The subsequent
>> activity is primarily on the XDNA side — for example, passing the
>> power data into the XRT environment via IOCTLs. If both trees had
>> active development or large, simultaneous changes, an immutable branch
>> would make sense, but given the current situation it doesn’t seem
>> worth the added complexity.
>>
>> The XDNA changes are distributed and will be implemented gradually;
>> they depend on this exported power information from PMF. So my
>> proposal is for PMF to export the information now with minimal changes
>> on the XDNA side, and land that upstream. The XDNA team can then build
>> on top of the exported interface, and their changes can be submitted
>> via the misc-drm tree.
>>
>> Ilpo, does this approach work for you?
> 
> Lets just wait with these two patches until have real consumer changes 
> ready to be submitted, these patches can appear at the head of such a 
> series.
> 

OK, I get your point. Thank you. Will have an internal discussion on
this and come back on what could be a best approach for this.

For now, I have dropped the patch 4 and 5 and sent out the rest for
review and they can be applied on mainline with 6fb7433c7b8e
("platform/x86/amd/pmf: Use ring buffer to store custom BIOS input
values") as the git tip.

Thanks,
Shyam

Re: [PATCH v2 0/5] PMF NPU metrics cleanup, command flag cleanup, and amdxdna integration

[Mario Limonciello]

On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
> This series streamlines NPU-related handling in AMD PMF, adds a
> lightweight in-kernel interface for fetching NPU metrics from PMF, and
> hooks AMD XDNA to that interface to expose real-time NPU power data to
> user space.
> 
> Changes include renaming legacy IPU fields to NPU in the metrics structure
> replacing ambiguous booleans and magic values with explicit
> SET_CMD/GET_CMD and METRICS_TABLE_ID constants, introducing an exported
> amd_pmf_get_npu_data() API (guarded by a mutex and platform checks) to
> pull NPU metrics from the SMU, and wiring those metrics into the amdxdna
> driver.
> 
> v2:
> ----
>   - Split the series into multiple patches per v1 feedback
>   - Added preliminary PMF driver cleanups as preparatory patches
>   - Introduced an initial amd_xdna patch that consumes PMF-provided data to
>     establish the plumbing; additional logic can be layered on in future
>     patches as more PMF metrics are utilized.
> 
> Shyam Sundar S K (4):
>    platform/x86/amd/pmf: Rename IPU metrics fields to NPU for consistency
>    platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in
>      amd_pmf_send_cmd()
>    platform/x86/amd/pmf: replace magic table id with METRICS_TABLE_ID
>    platform/x86/amd/pmf: Introduce new interface to export NPU metrics
> 
> VinitKumar Shukla (1):
>    accel/amdxdna: Provide real-time NPU power estimate via AMD PMF
> 
>   drivers/accel/amdxdna/aie2_pci.h         |  2 +
>   drivers/accel/amdxdna/aie2_smu.c         | 12 ++++
>   drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++---
>   drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++---
>   drivers/platform/x86/amd/pmf/core.c      | 71 +++++++++++++++++++++++-
>   drivers/platform/x86/amd/pmf/pmf.h       | 18 ++++--
>   drivers/platform/x86/amd/pmf/spc.c       |  2 +-
>   drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++-------
>   drivers/platform/x86/amd/pmf/tee-if.c    | 20 +++----
>   include/linux/amd-pmf-io.h               | 21 +++++++
>   10 files changed, 159 insertions(+), 53 deletions(-)
> 

Which tree do you think this should merge through?
drm-misc or platform-x86?

I tried and the series cleanly applies to both and compiles on both.  So 
I think it can go either way.

Re: [PATCH v2 0/5] PMF NPU metrics cleanup, command flag cleanup, and amdxdna integration

[Shyam Sundar S K]

On 11/12/2025 22:51, Mario Limonciello wrote:
> On 11/11/25 12:37 AM, Shyam Sundar S K wrote:
>> This series streamlines NPU-related handling in AMD PMF, adds a
>> lightweight in-kernel interface for fetching NPU metrics from PMF, and
>> hooks AMD XDNA to that interface to expose real-time NPU power data to
>> user space.
>>
>> Changes include renaming legacy IPU fields to NPU in the metrics
>> structure
>> replacing ambiguous booleans and magic values with explicit
>> SET_CMD/GET_CMD and METRICS_TABLE_ID constants, introducing an exported
>> amd_pmf_get_npu_data() API (guarded by a mutex and platform checks) to
>> pull NPU metrics from the SMU, and wiring those metrics into the
>> amdxdna
>> driver.
>>
>> v2:
>> ----
>>   - Split the series into multiple patches per v1 feedback
>>   - Added preliminary PMF driver cleanups as preparatory patches
>>   - Introduced an initial amd_xdna patch that consumes PMF-provided
>> data to
>>     establish the plumbing; additional logic can be layered on in
>> future
>>     patches as more PMF metrics are utilized.
>>
>> Shyam Sundar S K (4):
>>    platform/x86/amd/pmf: Rename IPU metrics fields to NPU for
>> consistency
>>    platform/x86/amd/pmf: Use explicit SET_CMD/GET_CMD flags in
>>      amd_pmf_send_cmd()
>>    platform/x86/amd/pmf: replace magic table id with METRICS_TABLE_ID
>>    platform/x86/amd/pmf: Introduce new interface to export NPU metrics
>>
>> VinitKumar Shukla (1):
>>    accel/amdxdna: Provide real-time NPU power estimate via AMD PMF
>>
>>   drivers/accel/amdxdna/aie2_pci.h         |  2 +
>>   drivers/accel/amdxdna/aie2_smu.c         | 12 ++++
>>   drivers/platform/x86/amd/pmf/auto-mode.c | 14 ++---
>>   drivers/platform/x86/amd/pmf/cnqf.c      | 14 ++---
>>   drivers/platform/x86/amd/pmf/core.c      | 71 ++++++++++++++++++++
>> +++-
>>   drivers/platform/x86/amd/pmf/pmf.h       | 18 ++++--
>>   drivers/platform/x86/amd/pmf/spc.c       |  2 +-
>>   drivers/platform/x86/amd/pmf/sps.c       | 38 ++++++-------
>>   drivers/platform/x86/amd/pmf/tee-if.c    | 20 +++----
>>   include/linux/amd-pmf-io.h               | 21 +++++++
>>   10 files changed, 159 insertions(+), 53 deletions(-)
>>
> 
> Which tree do you think this should merge through?
> drm-misc or platform-x86?
> 
> I tried and the series cleanly applies to both and compiles on both. 
> So I think it can go either way.

this series can apply on the latest mainline 6.18-rc5 with
4a0c9b339199 commit as the tip.

there are 3 series out from me on the pdx86 tree, and since I am not
sure which one will go first, all of them are based on the
4a0c9b339199 commit as the tip. So, I can rebase and respin based on
what goes in first.

w.r.t to this series, would like Ilpo's help to take via pdx86 if he
is comfortable (as the last patch in xdna has very minimal changes)

Thanks,
Shyam