[PATCH 1/2] platform/x86/amd/pmf: Rename ipu fields to npu in smu_pmf_metrics_v2 structure

[PATCH 1/2] platform/x86/amd/pmf: Rename ipu fields to npu in smu_pmf_metrics_v2 structure

[Shyam Sundar S K]

Rename multiple fields in the struct smu_pmf_metrics_v2 from "ipu" to "npu"
to align with updated hardware terminology and enhance code clarity.

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>
---
Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip commit
"8236b4667aca"

 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

[PATCH 2/2] 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. This commit
introduces a new interface to make NPU metrics accessible to other
drivers like AMDXDNA driver, which can access and utilize this
information as needed.

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>
---
Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip commit
"8236b4667aca"

 drivers/platform/x86/amd/pmf/core.c | 60 +++++++++++++++++++++++++++++
 drivers/platform/x86/amd/pmf/pmf.h  |  1 +
 include/linux/amd-pmf-io.h          | 21 ++++++++++
 3 files changed, 82 insertions(+)

diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
index ef988605c4da..75529047377c 100644
--- a/drivers/platform/x86/amd/pmf/core.c
+++ b/drivers/platform/x86/amd/pmf/core.c
@@ -8,6 +8,7 @@
  * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
  */
 
+#include <linux/amd-pmf-io.h>
 #include <linux/debugfs.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
@@ -53,6 +54,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 +317,61 @@ 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;
+
+	if (is_npu_metrics_supported(dev))
+		return -EINVAL;
+
+	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, 0, 7, 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);
@@ -431,6 +489,8 @@ static int amd_pmf_probe(struct platform_device *pdev)
 
 	dev->dev = &pdev->dev;
 
+	pmf = dev->dev;
+
 	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
 	if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
 		pci_dev_put(rdev);
diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
index 6ea5380f3b23..622404e71136 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>
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 1/2] platform/x86/amd/pmf: Rename ipu fields to npu in smu_pmf_metrics_v2 structure

[Mario Limonciello (AMD) (kernel.org)]

On 9/19/2025 12:54 AM, Shyam Sundar S K wrote:
> Rename multiple fields in the struct smu_pmf_metrics_v2 from "ipu" to "npu"
> to align with updated hardware terminology and enhance code clarity.
> 
> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com>
> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>> ---
> Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip commit
> "8236b4667aca"
> 
>   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;

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

[Mario Limonciello]

On 9/19/2025 12:54 AM, Shyam Sundar S K wrote:
> The PMF driver retrieves NPU metrics data from the PMFW. This commit
> introduces a new interface to make NPU metrics accessible to other
> drivers like AMDXDNA driver, which can access and utilize this
> information as needed.
> 
> 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>

+Lizhi for comments.

As this interface will be something amdxdna uses I think we should make 
sure there is a R-b from Lizhi.

I have a few comments below as well.

> ---
> Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip commit
> "8236b4667aca"
> 
>   drivers/platform/x86/amd/pmf/core.c | 60 +++++++++++++++++++++++++++++
>   drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>   include/linux/amd-pmf-io.h          | 21 ++++++++++
>   3 files changed, 82 insertions(+)
> 
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index ef988605c4da..75529047377c 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -8,6 +8,7 @@
>    * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>    */
>   
> +#include <linux/amd-pmf-io.h>
>   #include <linux/debugfs.h>
>   #include <linux/iopoll.h>
>   #include <linux/module.h>
> @@ -53,6 +54,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 +317,61 @@ 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:

I'm a bit surprised AMD_CPU_ID_PS isn't in this list.  Does it not have 
the same interface?

> +		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;
> +
> +	if (is_npu_metrics_supported(dev))
> +		return -EINVAL;
> +
> +	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, 0, 7, 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);
> @@ -431,6 +489,8 @@ static int amd_pmf_probe(struct platform_device *pdev)
>   
>   	dev->dev = &pdev->dev;
>   
> +	pmf = dev->dev;
> +
>   	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>   	if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
>   		pci_dev_put(rdev);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 6ea5380f3b23..622404e71136 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>
> 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];

This 8 comes from there being 8 columns, right?  What happens when we 
have a product with more?  It seems like this doesn't scale well outside 
of that design.

I know it is a simple memcpy() right now and the structure resembles the 
firmware structure, but would it make sense for inter-kernel 
commnication to define a different structure that would remain stable no 
matter how many columns?

Something like this:

struct amd_pmf_npu_metrics {
	u16 npuclk_freq;
	u16 npu_power;
	u16 mpnpuclk_freq;
	u16 npu_reads;
	u16 npu_writes;
	u8 npu_columns;
	u16 *npu_busy;
};

The size of npu_busy could be allocated dynamically, but this would also 
mean putting the responsibility of freeing it on the caller.

The alternative is of course a amd_pmf_npu_metrics_v2 if/when that 
happens.

I just thought it was worth considering right now.
> +	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

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

[Lizhi Hou]

On 9/18/25 22:54, Shyam Sundar S K wrote:
> The PMF driver retrieves NPU metrics data from the PMFW. This commit
> introduces a new interface to make NPU metrics accessible to other
> drivers like AMDXDNA driver, which can access and utilize this
> information as needed.
>
> 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>
> ---
> Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip commit
> "8236b4667aca"
>
>   drivers/platform/x86/amd/pmf/core.c | 60 +++++++++++++++++++++++++++++
>   drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>   include/linux/amd-pmf-io.h          | 21 ++++++++++
>   3 files changed, 82 insertions(+)
>
> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/x86/amd/pmf/core.c
> index ef988605c4da..75529047377c 100644
> --- a/drivers/platform/x86/amd/pmf/core.c
> +++ b/drivers/platform/x86/amd/pmf/core.c
> @@ -8,6 +8,7 @@
>    * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>    */
>   
> +#include <linux/amd-pmf-io.h>
>   #include <linux/debugfs.h>
>   #include <linux/iopoll.h>
>   #include <linux/module.h>
> @@ -53,6 +54,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;
So there will never be more than one pmf device in the future?
> +
>   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 +317,61 @@ 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;
> +
> +	if (is_npu_metrics_supported(dev))
> +		return -EINVAL;
> +
> +	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, 0, 7, NULL);
s/0/false/ ?

What does the 7 mean? Is defining a macro better?

> +	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);
Could 'pmf' be invalid pointer if the driver is unbound from the device?
> +
> +	if (!pdev || !info)
> +		return -EINVAL;
> +
> +	return amd_pmf_get_smu_metrics(pdev, info);
> +}
> +EXPORT_SYMBOL_GPL(amd_pmf_get_npu_data);
Should the interface be thread safe? I do not see any lock to protect 
dev->m_table_v2 or dev->buf.
> +
>   static int amd_pmf_suspend_handler(struct device *dev)
>   {
>   	struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
> @@ -431,6 +489,8 @@ static int amd_pmf_probe(struct platform_device *pdev)
>   
>   	dev->dev = &pdev->dev;
>   
> +	pmf = dev->dev;

Maybe set pmf after probe is success? And clear it in remove?

Because driver could be bound/unbound, simply using global pointer might 
be unreliable.


Thanks,

Lizhi

> +
>   	rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>   	if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
>   		pci_dev_put(rdev);
> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h
> index 6ea5380f3b23..622404e71136 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>
> 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

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

[Shyam Sundar S K]

On 9/19/2025 19:51, Mario Limonciello wrote:
> On 9/19/2025 12:54 AM, Shyam Sundar S K wrote:
>> The PMF driver retrieves NPU metrics data from the PMFW. This commit
>> introduces a new interface to make NPU metrics accessible to other
>> drivers like AMDXDNA driver, which can access and utilize this
>> information as needed.
>>
>> 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>
> 
> +Lizhi for comments.
> 
> As this interface will be something amdxdna uses I think we should
> make sure there is a R-b from Lizhi.

Sure.

> 
> I have a few comments below as well.
> 
>> ---
>> Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip
>> commit
>> "8236b4667aca"
>>
>>   drivers/platform/x86/amd/pmf/core.c | 60 +++++++++++++++++++++++++
>> ++++
>>   drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>   include/linux/amd-pmf-io.h          | 21 ++++++++++
>>   3 files changed, 82 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/
>> x86/amd/pmf/core.c
>> index ef988605c4da..75529047377c 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -8,6 +8,7 @@
>>    * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>    */
>>   +#include <linux/amd-pmf-io.h>
>>   #include <linux/debugfs.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/module.h>
>> @@ -53,6 +54,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 +317,61 @@ 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:
> 
> I'm a bit surprised AMD_CPU_ID_PS isn't in this list.  Does it not
> have the same interface?

Intial thoughts is to have support starting from 1Ah. Based on futher
tests we can extend this to older platforms as well, albeit I am not
sure if PMFW provides this information for older ones. So it would be
on the wishlist.


> 
>> +        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;
>> +
>> +    if (is_npu_metrics_supported(dev))
>> +        return -EINVAL;
>> +
>> +    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, 0, 7, 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);
>> @@ -431,6 +489,8 @@ static int amd_pmf_probe(struct platform_device
>> *pdev)
>>         dev->dev = &pdev->dev;
>>   +    pmf = dev->dev;
>> +
>>       rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>>       if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
>>           pci_dev_put(rdev);
>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/
>> x86/amd/pmf/pmf.h
>> index 6ea5380f3b23..622404e71136 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>
>> 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];
> 
> This 8 comes from there being 8 columns, right?  What happens when we
> have a product with more?  It seems like this doesn't scale well
> outside of that design.
> 
> I know it is a simple memcpy() right now and the structure resembles
> the firmware structure, but would it make sense for inter-kernel
> commnication to define a different structure that would remain stable
> no matter how many columns?
> 
> Something like this:
> 
> struct amd_pmf_npu_metrics {
>     u16 npuclk_freq;
>     u16 npu_power;
>     u16 mpnpuclk_freq;
>     u16 npu_reads;
>     u16 npu_writes;
>     u8 npu_columns;
>     u16 *npu_busy;
> };
> 
> The size of npu_busy could be allocated dynamically, but this would
> also mean putting the responsibility of freeing it on the caller.
> 
> The alternative is of course a amd_pmf_npu_metrics_v2 if/when that
> happens.
> 

I can give it a shot. But I presume structure changes would not be
more frequent. So, I would still lean to having a _v2/_v3 whenever
that happens in the future.

Thanks,
Shyam


> I just thought it was worth considering right now.
>> +    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
> 

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

[Shyam Sundar S K]

On 9/20/2025 01:56, Lizhi Hou wrote:
> 
> On 9/18/25 22:54, Shyam Sundar S K wrote:
>> The PMF driver retrieves NPU metrics data from the PMFW. This commit
>> introduces a new interface to make NPU metrics accessible to other
>> drivers like AMDXDNA driver, which can access and utilize this
>> information as needed.
>>
>> 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>
>> ---
>> Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip
>> commit
>> "8236b4667aca"
>>
>>   drivers/platform/x86/amd/pmf/core.c | 60 +++++++++++++++++++++++++
>> ++++
>>   drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>   include/linux/amd-pmf-io.h          | 21 ++++++++++
>>   3 files changed, 82 insertions(+)
>>
>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/
>> x86/amd/pmf/core.c
>> index ef988605c4da..75529047377c 100644
>> --- a/drivers/platform/x86/amd/pmf/core.c
>> +++ b/drivers/platform/x86/amd/pmf/core.c
>> @@ -8,6 +8,7 @@
>>    * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>    */
>>   +#include <linux/amd-pmf-io.h>
>>   #include <linux/debugfs.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/module.h>
>> @@ -53,6 +54,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;
> So there will never be more than one pmf device in the future?
>> +
>>   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 +317,61 @@ 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;
>> +
>> +    if (is_npu_metrics_supported(dev))
>> +        return -EINVAL;
>> +
>> +    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, 0, 7, NULL);
> s/0/false/ ?
> 
> What does the 7 mean? Is defining a macro better?
> 
>> +    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);
> Could 'pmf' be invalid pointer if the driver is unbound from the device?
>> +
>> +    if (!pdev || !info)
>> +        return -EINVAL;
>> +
>> +    return amd_pmf_get_smu_metrics(pdev, info);
>> +}
>> +EXPORT_SYMBOL_GPL(amd_pmf_get_npu_data);
> Should the interface be thread safe? I do not see any lock to protect
> dev->m_table_v2 or dev->buf.
>> +
>>   static int amd_pmf_suspend_handler(struct device *dev)
>>   {
>>       struct amd_pmf_dev *pdev = dev_get_drvdata(dev);
>> @@ -431,6 +489,8 @@ static int amd_pmf_probe(struct platform_device
>> *pdev)
>>         dev->dev = &pdev->dev;
>>   +    pmf = dev->dev;
> 
> Maybe set pmf after probe is success? And clear it in remove?
> 
> Because driver could be bound/unbound, simply using global pointer
> might be unreliable.
> 

Ack for all your comments. I will address them in v2.

Thanks,
Shyam

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

[Mario Limonciello (AMD) (kernel.org)]

On 9/22/2025 11:07 AM, Shyam Sundar S K wrote:
> 
> 
> On 9/19/2025 19:51, Mario Limonciello wrote:
>> On 9/19/2025 12:54 AM, Shyam Sundar S K wrote:
>>> The PMF driver retrieves NPU metrics data from the PMFW. This commit
>>> introduces a new interface to make NPU metrics accessible to other
>>> drivers like AMDXDNA driver, which can access and utilize this
>>> information as needed.
>>>
>>> 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>
>>
>> +Lizhi for comments.
>>
>> As this interface will be something amdxdna uses I think we should
>> make sure there is a R-b from Lizhi.
> 
> Sure.

With the comments you shared I personally have no concerns that need 
addressing.

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

> 
>>
>> I have a few comments below as well.
>>
>>> ---
>>> Hi Ilpo, this can be applied on origin/review-ilpo-next with git tip
>>> commit
>>> "8236b4667aca"
>>>
>>>    drivers/platform/x86/amd/pmf/core.c | 60 +++++++++++++++++++++++++
>>> ++++
>>>    drivers/platform/x86/amd/pmf/pmf.h  |  1 +
>>>    include/linux/amd-pmf-io.h          | 21 ++++++++++
>>>    3 files changed, 82 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/amd/pmf/core.c b/drivers/platform/
>>> x86/amd/pmf/core.c
>>> index ef988605c4da..75529047377c 100644
>>> --- a/drivers/platform/x86/amd/pmf/core.c
>>> +++ b/drivers/platform/x86/amd/pmf/core.c
>>> @@ -8,6 +8,7 @@
>>>     * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
>>>     */
>>>    +#include <linux/amd-pmf-io.h>
>>>    #include <linux/debugfs.h>
>>>    #include <linux/iopoll.h>
>>>    #include <linux/module.h>
>>> @@ -53,6 +54,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 +317,61 @@ 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:
>>
>> I'm a bit surprised AMD_CPU_ID_PS isn't in this list.  Does it not
>> have the same interface?
> 
> Intial thoughts is to have support starting from 1Ah. Based on futher
> tests we can extend this to older platforms as well, albeit I am not
> sure if PMFW provides this information for older ones. So it would be
> on the wishlist.

Got it, thanks for clarification.

> 
> 
>>
>>> +        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;
>>> +
>>> +    if (is_npu_metrics_supported(dev))
>>> +        return -EINVAL;
>>> +
>>> +    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, 0, 7, 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);
>>> @@ -431,6 +489,8 @@ static int amd_pmf_probe(struct platform_device
>>> *pdev)
>>>          dev->dev = &pdev->dev;
>>>    +    pmf = dev->dev;
>>> +
>>>        rdev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(0, 0));
>>>        if (!rdev || !pci_match_id(pmf_pci_ids, rdev)) {
>>>            pci_dev_put(rdev);
>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/
>>> x86/amd/pmf/pmf.h
>>> index 6ea5380f3b23..622404e71136 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>
>>> 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];
>>
>> This 8 comes from there being 8 columns, right?  What happens when we
>> have a product with more?  It seems like this doesn't scale well
>> outside of that design.
>>
>> I know it is a simple memcpy() right now and the structure resembles
>> the firmware structure, but would it make sense for inter-kernel
>> commnication to define a different structure that would remain stable
>> no matter how many columns?
>>
>> Something like this:
>>
>> struct amd_pmf_npu_metrics {
>>      u16 npuclk_freq;
>>      u16 npu_power;
>>      u16 mpnpuclk_freq;
>>      u16 npu_reads;
>>      u16 npu_writes;
>>      u8 npu_columns;
>>      u16 *npu_busy;
>> };
>>
>> The size of npu_busy could be allocated dynamically, but this would
>> also mean putting the responsibility of freeing it on the caller.
>>
>> The alternative is of course a amd_pmf_npu_metrics_v2 if/when that
>> happens.
>>
> 
> I can give it a shot. But I presume structure changes would not be
> more frequent. So, I would still lean to having a _v2/_v3 whenever
> that happens in the future.
> 

Yeah if it's not going to be a frequent update, it might not be worth 
the extra complexity.  You can make a decision here, and if you want to 
stick with what you've got that is fine.

> Thanks,
> Shyam
> 
> 
>> I just thought it was worth considering right now.
>>> +    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
>>
>