* [PATCH v3 1/6] platform/x86:intel/pmc: Enable SSRAM support for Lunar Lake
2025-08-15 22:45 [PATCH v3 0/6] Enable SSRAM support in PTL and LNL Xi Pardee
@ 2025-08-15 22:45 ` Xi Pardee
2025-08-15 22:46 ` [PATCH v3 2/6] platform/x86:intel/pmc: Move telemetry endpoint register handling Xi Pardee
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Xi Pardee @ 2025-08-15 22:45 UTC (permalink / raw)
To: xi.pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
Enable Lunar Lake platforms to achieve PMC information from
Intel PMC SSRAM Telemetry driver and substate requirements data
from telemetry region.
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/platform/x86/intel/pmc/lnl.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/lnl.c b/drivers/platform/x86/intel/pmc/lnl.c
index da513c234714b..e08a77c778c2c 100644
--- a/drivers/platform/x86/intel/pmc/lnl.c
+++ b/drivers/platform/x86/intel/pmc/lnl.c
@@ -13,6 +13,10 @@
#include "core.h"
+#define SOCM_LPM_REQ_GUID 0x15099748
+
+static const u8 LNL_LPM_REG_INDEX[] = {0, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 20};
+
static const struct pmc_bit_map lnl_ltr_show_map[] = {
{"SOUTHPORT_A", CNP_PMC_LTR_SPA},
{"SOUTHPORT_B", CNP_PMC_LTR_SPB},
@@ -528,6 +532,16 @@ static const struct pmc_reg_map lnl_socm_reg_map = {
.lpm_live_status_offset = MTL_LPM_LIVE_STATUS_OFFSET,
.s0ix_blocker_maps = lnl_blk_maps,
.s0ix_blocker_offset = LNL_S0IX_BLOCKER_OFFSET,
+ .lpm_reg_index = LNL_LPM_REG_INDEX,
+};
+
+static struct pmc_info lnl_pmc_info_list[] = {
+ {
+ .guid = SOCM_LPM_REQ_GUID,
+ .devid = PMC_DEVID_LNL_SOCM,
+ .map = &lnl_socm_reg_map,
+ },
+ {}
};
#define LNL_NPU_PCI_DEV 0x643e
@@ -557,6 +571,8 @@ static int lnl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
}
struct pmc_dev_info lnl_pmc_dev = {
+ .pci_func = 2,
+ .regmap_list = lnl_pmc_info_list,
.map = &lnl_socm_reg_map,
.suspend = cnl_suspend,
.resume = lnl_resume,
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/6] platform/x86:intel/pmc: Move telemetry endpoint register handling
2025-08-15 22:45 [PATCH v3 0/6] Enable SSRAM support in PTL and LNL Xi Pardee
2025-08-15 22:45 ` [PATCH v3 1/6] platform/x86:intel/pmc: Enable SSRAM support for Lunar Lake Xi Pardee
@ 2025-08-15 22:46 ` Xi Pardee
2025-08-15 22:46 ` [PATCH v3 3/6] platform/x86:intel/pmc: Improve function to show substate header Xi Pardee
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Xi Pardee @ 2025-08-15 22:46 UTC (permalink / raw)
To: xi.pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
Move telemetry endpoint handling to pmc_core_get_telem_info(). This
is a preparation patch to introduce a new table to obtain Low Power
Mode substate requirement data for platforms starting from Panther
Lake.
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/platform/x86/intel/pmc/core.c | 51 +++++++++++++--------------
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index 540cd2fb0673b..a1dd80bdbd413 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -1399,36 +1399,23 @@ static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *m
* +----+---------------------------------------------------------+
*
*/
-static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct pci_dev *pcidev)
+static int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc,
+ struct telem_endpoint *ep)
{
- struct telem_endpoint *ep;
const u8 *lpm_indices;
int num_maps, mode_offset = 0;
int ret, mode;
int lpm_size;
- u32 guid;
lpm_indices = pmc->map->lpm_reg_index;
num_maps = pmc->map->lpm_num_maps;
lpm_size = LPM_MAX_NUM_MODES * num_maps;
- guid = pmc_core_find_guid(pmcdev->regmap_list, pmc->map);
- if (!guid)
- return -ENXIO;
-
- ep = pmt_telem_find_and_register_endpoint(pcidev, guid, 0);
- if (IS_ERR(ep)) {
- dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %pe", ep);
- return -EPROBE_DEFER;
- }
-
pmc->lpm_req_regs = devm_kzalloc(&pmcdev->pdev->dev,
lpm_size * sizeof(u32),
GFP_KERNEL);
- if (!pmc->lpm_req_regs) {
- ret = -ENOMEM;
- goto unregister_ep;
- }
+ if (!pmc->lpm_req_regs)
+ return -ENOMEM;
mode_offset = LPM_HEADER_OFFSET + LPM_MODE_OFFSET;
pmc_for_each_mode(mode, pmcdev) {
@@ -1442,23 +1429,21 @@ static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct
if (ret) {
dev_err(&pmcdev->pdev->dev,
"couldn't read Low Power Mode requirements: %d\n", ret);
- goto unregister_ep;
+ return ret;
}
++req_offset;
}
mode_offset += LPM_REG_COUNT + LPM_MODE_OFFSET;
}
-
-unregister_ep:
- pmt_telem_unregister_endpoint(ep);
-
return ret;
}
-static int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev, int func)
+static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
{
struct pci_dev *pcidev __free(pci_dev_put) = NULL;
+ struct telem_endpoint *ep;
unsigned int i;
+ u32 guid;
int ret;
pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, func));
@@ -1466,10 +1451,24 @@ static int pmc_core_ssram_get_lpm_reqs(struct pmc_dev *pmcdev, int func)
return -ENODEV;
for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
- if (!pmcdev->pmcs[i])
+ struct pmc *pmc;
+
+ pmc = pmcdev->pmcs[i];
+ if (!pmc)
continue;
- ret = pmc_core_get_lpm_req(pmcdev, pmcdev->pmcs[i], pcidev);
+ guid = pmc_core_find_guid(pmcdev->regmap_list, pmc->map);
+ if (!guid)
+ return -ENXIO;
+
+ ep = pmt_telem_find_and_register_endpoint(pcidev, guid, 0);
+ if (IS_ERR(ep)) {
+ dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %pe", ep);
+ return -EPROBE_DEFER;
+ }
+
+ ret = pmc_core_pmt_get_lpm_req(pmcdev, pmc, ep);
+ pmt_telem_unregister_endpoint(ep);
if (ret)
return ret;
}
@@ -1583,7 +1582,7 @@ int generic_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
pmc_core_punit_pmt_init(pmcdev, pmc_dev_info->dmu_guid);
if (ssram) {
- ret = pmc_core_ssram_get_lpm_reqs(pmcdev, pmc_dev_info->pci_func);
+ ret = pmc_core_get_telem_info(pmcdev, pmc_dev_info->pci_func);
if (ret)
goto unmap_regbase;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/6] platform/x86:intel/pmc: Improve function to show substate header
2025-08-15 22:45 [PATCH v3 0/6] Enable SSRAM support in PTL and LNL Xi Pardee
2025-08-15 22:45 ` [PATCH v3 1/6] platform/x86:intel/pmc: Enable SSRAM support for Lunar Lake Xi Pardee
2025-08-15 22:46 ` [PATCH v3 2/6] platform/x86:intel/pmc: Move telemetry endpoint register handling Xi Pardee
@ 2025-08-15 22:46 ` Xi Pardee
2025-08-28 13:09 ` Ilpo Järvinen
2025-08-15 22:46 ` [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers Xi Pardee
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Xi Pardee @ 2025-08-15 22:46 UTC (permalink / raw)
To: xi.pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
Refactor pmc_core_substate_req_header_show() to accept a new argument.
This is a preparation patch to introduce a new way to show Low Power
Mode substate requirement data for platforms starting from Panther
Lake. Increased the size for the name column as the Low Power Mode
requirement register name is longer in newer platforms.
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
drivers/platform/x86/intel/pmc/core.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index a1dd80bdbd413..cbfdcdc50ad21 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -11,6 +11,11 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+enum header_type {
+ HEADER_STATUS,
+ HEADER_VALUE
+};
+
#include <linux/bitfield.h>
#include <linux/debugfs.h>
#include <linux/delay.h>
@@ -828,17 +833,22 @@ static int pmc_core_substate_l_sts_regs_show(struct seq_file *s, void *unused)
}
DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_l_sts_regs);
-static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index)
+static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index,
+ enum header_type type)
{
struct pmc_dev *pmcdev = s->private;
int mode;
- seq_printf(s, "%30s |", "Element");
+ seq_printf(s, "%40s |", "Element");
pmc_for_each_mode(mode, pmcdev)
seq_printf(s, " %9s |", pmc_lpm_modes[mode]);
- seq_printf(s, " %9s |", "Status");
- seq_printf(s, " %11s |\n", "Live Status");
+ if (type == HEADER_STATUS) {
+ seq_printf(s, " %9s |", "Status");
+ seq_printf(s, " %11s |\n", "Live Status");
+ } else {
+ seq_printf(s, " %9s |\n", "Value");
+ }
}
static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
@@ -872,7 +882,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
continue;
/* Display the header */
- pmc_core_substate_req_header_show(s, pmc_index);
+ pmc_core_substate_req_header_show(s, pmc_index, HEADER_STATUS);
/* Loop over maps */
for (mp = 0; mp < num_maps; mp++) {
@@ -910,7 +920,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
}
/* Display the element name in the first column */
- seq_printf(s, "pmc%d: %26s |", pmc_index, map[i].name);
+ seq_printf(s, "pmc%d: %34s |", pmc_index, map[i].name);
/* Loop over the enabled states and display if required */
pmc_for_each_mode(mode, pmcdev) {
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 3/6] platform/x86:intel/pmc: Improve function to show substate header
2025-08-15 22:46 ` [PATCH v3 3/6] platform/x86:intel/pmc: Improve function to show substate header Xi Pardee
@ 2025-08-28 13:09 ` Ilpo Järvinen
0 siblings, 0 replies; 11+ messages in thread
From: Ilpo Järvinen @ 2025-08-28 13:09 UTC (permalink / raw)
To: Xi Pardee
Cc: irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
On Fri, 15 Aug 2025, Xi Pardee wrote:
> Refactor pmc_core_substate_req_header_show() to accept a new argument.
> This is a preparation patch to introduce a new way to show Low Power
> Mode substate requirement data for platforms starting from Panther
> Lake. Increased the size for the name column as the Low Power Mode
> requirement register name is longer in newer platforms.
>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
> drivers/platform/x86/intel/pmc/core.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index a1dd80bdbd413..cbfdcdc50ad21 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -11,6 +11,11 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +enum header_type {
> + HEADER_STATUS,
> + HEADER_VALUE
Please have the comma in any non-terminating entry.
> +};
> +
> #include <linux/bitfield.h>
> #include <linux/debugfs.h>
> #include <linux/delay.h>
> @@ -828,17 +833,22 @@ static int pmc_core_substate_l_sts_regs_show(struct seq_file *s, void *unused)
> }
> DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_l_sts_regs);
>
> -static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index)
> +static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index,
> + enum header_type type)
> {
> struct pmc_dev *pmcdev = s->private;
> int mode;
>
> - seq_printf(s, "%30s |", "Element");
> + seq_printf(s, "%40s |", "Element");
> pmc_for_each_mode(mode, pmcdev)
> seq_printf(s, " %9s |", pmc_lpm_modes[mode]);
>
> - seq_printf(s, " %9s |", "Status");
> - seq_printf(s, " %11s |\n", "Live Status");
> + if (type == HEADER_STATUS) {
> + seq_printf(s, " %9s |", "Status");
> + seq_printf(s, " %11s |\n", "Live Status");
> + } else {
> + seq_printf(s, " %9s |\n", "Value");
> + }
> }
>
> static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> @@ -872,7 +882,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> continue;
>
> /* Display the header */
> - pmc_core_substate_req_header_show(s, pmc_index);
> + pmc_core_substate_req_header_show(s, pmc_index, HEADER_STATUS);
>
> /* Loop over maps */
> for (mp = 0; mp < num_maps; mp++) {
> @@ -910,7 +920,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> }
>
> /* Display the element name in the first column */
> - seq_printf(s, "pmc%d: %26s |", pmc_index, map[i].name);
> + seq_printf(s, "pmc%d: %34s |", pmc_index, map[i].name);
>
> /* Loop over the enabled states and display if required */
> pmc_for_each_mode(mode, pmcdev) {
>
--
i.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers
2025-08-15 22:45 [PATCH v3 0/6] Enable SSRAM support in PTL and LNL Xi Pardee
` (2 preceding siblings ...)
2025-08-15 22:46 ` [PATCH v3 3/6] platform/x86:intel/pmc: Improve function to show substate header Xi Pardee
@ 2025-08-15 22:46 ` Xi Pardee
2025-08-28 13:06 ` Ilpo Järvinen
2025-09-01 23:52 ` Russell Haley
2025-08-15 22:46 ` [PATCH v3 5/6] platform/x86:intel/pmc: Show device and function number Xi Pardee
2025-08-15 22:46 ` [PATCH v3 6/6] platform/x86:intel/pmc: Enable SSRAM support for Panther Lake Xi Pardee
5 siblings, 2 replies; 11+ messages in thread
From: Xi Pardee @ 2025-08-15 22:46 UTC (permalink / raw)
To: xi.pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
Add support to read and show S0ix blocker substate requirements.
Starting from Panther Lake, substate requirement data is provided
based on S0ix blockers instead of all low power mode requirements.
For platforms that support this new feature, add support to display
substate requirements based on S0ix blockers.
Change the "substate_requirements" attribute of Intel PMC Core
driver to show the substate requirements for each S0ix blocker
and the corresponding S0ix blocker value.
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
drivers/platform/x86/intel/pmc/arl.c | 4 +
drivers/platform/x86/intel/pmc/core.c | 126 ++++++++++++++++++++++++--
drivers/platform/x86/intel/pmc/core.h | 14 +++
drivers/platform/x86/intel/pmc/lnl.c | 2 +
drivers/platform/x86/intel/pmc/mtl.c | 2 +
5 files changed, 138 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
index 9d66d65e75779..17ad87b392abe 100644
--- a/drivers/platform/x86/intel/pmc/arl.c
+++ b/drivers/platform/x86/intel/pmc/arl.c
@@ -725,9 +725,11 @@ struct pmc_dev_info arl_pmc_dev = {
.dmu_guid = ARL_PMT_DMU_GUID,
.regmap_list = arl_pmc_info_list,
.map = &arl_socs_reg_map,
+ .sub_req_show = &pmc_core_substate_req_regs_fops,
.suspend = cnl_suspend,
.resume = arl_resume,
.init = arl_core_init,
+ .sub_req = pmc_core_pmt_get_lpm_req,
};
struct pmc_dev_info arl_h_pmc_dev = {
@@ -735,7 +737,9 @@ struct pmc_dev_info arl_h_pmc_dev = {
.dmu_guid = ARL_PMT_DMU_GUID,
.regmap_list = arl_pmc_info_list,
.map = &mtl_socm_reg_map,
+ .sub_req_show = &pmc_core_substate_req_regs_fops,
.suspend = cnl_suspend,
.resume = arl_h_resume,
.init = arl_h_core_init,
+ .sub_req = pmc_core_pmt_get_lpm_req,
};
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index cbfdcdc50ad21..a0b948a875a5a 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -851,6 +851,68 @@ static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index,
}
}
+static int pmc_core_substate_blk_req_show(struct seq_file *s, void *unused)
+{
+ struct pmc_dev *pmcdev = s->private;
+ unsigned int pmc_index;
+
+ for (pmc_index = 0; pmc_index < ARRAY_SIZE(pmcdev->pmcs); pmc_index++) {
+ const struct pmc_bit_map **maps;
+ unsigned int arr_size, r_idx;
+ u32 offset, counter;
+ u32 *lpm_req_regs;
+ struct pmc *pmc;
+
+ pmc = pmcdev->pmcs[pmc_index];
+ if (!pmc || !pmc->lpm_req_regs)
+ continue;
+
+ lpm_req_regs = pmc->lpm_req_regs;
+ maps = pmc->map->s0ix_blocker_maps;
+ offset = pmc->map->s0ix_blocker_offset;
+ arr_size = pmc_core_lpm_get_arr_size(maps);
+
+ /* Display the header */
+ pmc_core_substate_req_header_show(s, pmc_index, HEADER_VALUE);
+
+ for (r_idx = 0; r_idx < arr_size; r_idx++) {
+ const struct pmc_bit_map *map;
+
+ for (map = maps[r_idx]; map->name; map++) {
+ int mode;
+
+ if (!map->blk)
+ continue;
+
+ counter = pmc_core_reg_read(pmc, offset);
+ seq_printf(s, "pmc%d: %34s |", pmc_index, map->name);
+ pmc_for_each_mode(mode, pmcdev) {
+ bool required = *lpm_req_regs & BIT(mode);
+
+ seq_printf(s, " %9s |", required ? "Required" : " ");
+ }
+ seq_printf(s, " %9d |\n", counter);
+ offset += map->blk * S0IX_BLK_SIZE;
+ lpm_req_regs++;
+ }
+ }
+ }
+ return 0;
+}
+
+static int pmc_core_substate_blk_req_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, pmc_core_substate_blk_req_show, inode->i_private);
+}
+
+const struct file_operations pmc_core_substate_blk_req_fops = {
+ .owner = THIS_MODULE,
+ .open = pmc_core_substate_blk_req_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
{
struct pmc_dev *pmcdev = s->private;
@@ -941,7 +1003,19 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
}
return 0;
}
-DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
+
+static int pmc_core_substate_req_regs_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, pmc_core_substate_req_regs_show, inode->i_private);
+}
+
+const struct file_operations pmc_core_substate_req_regs_fops = {
+ .owner = THIS_MODULE,
+ .open = pmc_core_substate_req_regs_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
static unsigned int pmc_core_get_crystal_freq(void)
{
@@ -1274,7 +1348,7 @@ static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
debugfs_remove_recursive(pmcdev->dbgfs_dir);
}
-static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
+static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
{
struct pmc *primary_pmc = pmcdev->pmcs[PMC_IDX_MAIN];
struct dentry *dir;
@@ -1341,7 +1415,7 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
if (primary_pmc->lpm_req_regs) {
debugfs_create_file("substate_requirements", 0444,
pmcdev->dbgfs_dir, pmcdev,
- &pmc_core_substate_req_regs_fops);
+ pmc_dev_info->sub_req_show);
}
if (primary_pmc->map->pson_residency_offset && pmc_core_is_pson_residency_enabled(pmcdev)) {
@@ -1409,8 +1483,7 @@ static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *m
* +----+---------------------------------------------------------+
*
*/
-static int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc,
- struct telem_endpoint *ep)
+int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep)
{
const u8 *lpm_indices;
int num_maps, mode_offset = 0;
@@ -1448,7 +1521,37 @@ static int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc,
return ret;
}
-static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
+int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
+ struct telem_endpoint *ep)
+{
+ u32 num_blocker, sample_id;
+ unsigned int index;
+ u32 *req_offset;
+ int ret;
+
+ num_blocker = pmc->map->num_s0ix_blocker;
+ sample_id = pmc->map->blocker_req_offset;
+
+ pmc->lpm_req_regs = devm_kcalloc(&pmcdev->pdev->dev, num_blocker,
+ sizeof(u32), GFP_KERNEL);
+ if (!pmc->lpm_req_regs)
+ return -ENOMEM;
+
+ req_offset = pmc->lpm_req_regs;
+ for (index = 0; index < num_blocker; index++) {
+ ret = pmt_telem_read32(ep, sample_id, req_offset, 1);
+ if (ret) {
+ dev_err(&pmcdev->pdev->dev,
+ "couldn't read Low Power Mode requirements: %d\n", ret);
+ return ret;
+ }
+ sample_id++;
+ req_offset++;
+ }
+ return 0;
+}
+
+static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
{
struct pci_dev *pcidev __free(pci_dev_put) = NULL;
struct telem_endpoint *ep;
@@ -1456,11 +1559,12 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
u32 guid;
int ret;
- pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, func));
+ pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, pmc_dev_info->pci_func));
if (!pcidev)
return -ENODEV;
for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
+ int (*sub_req)(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
struct pmc *pmc;
pmc = pmcdev->pmcs[i];
@@ -1477,7 +1581,9 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
return -EPROBE_DEFER;
}
- ret = pmc_core_pmt_get_lpm_req(pmcdev, pmc, ep);
+ sub_req = pmc_dev_info->sub_req;
+ ret = sub_req(pmcdev, pmc, ep);
+
pmt_telem_unregister_endpoint(ep);
if (ret)
return ret;
@@ -1592,7 +1698,7 @@ int generic_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
pmc_core_punit_pmt_init(pmcdev, pmc_dev_info->dmu_guid);
if (ssram) {
- ret = pmc_core_get_telem_info(pmcdev, pmc_dev_info->pci_func);
+ ret = pmc_core_get_telem_info(pmcdev, pmc_dev_info);
if (ret)
goto unmap_regbase;
}
@@ -1766,7 +1872,7 @@ static int pmc_core_probe(struct platform_device *pdev)
pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(primary_pmc);
pmc_core_do_dmi_quirks(primary_pmc);
- pmc_core_dbgfs_register(pmcdev);
+ pmc_core_dbgfs_register(pmcdev, pmc_dev_info);
pm_report_max_hw_sleep(FIELD_MAX(SLP_S0_RES_COUNTER_MASK) *
pmc_core_adjust_slp_s0_step(primary_pmc, 1));
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 4a94a4ee031e6..bfe8fba808063 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -344,6 +344,8 @@ struct pmc_bit_map {
* @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
* @slps0_dbg_offset: PWRMBASE offset to SLP_S0_DEBUG_REG*
* @s0ix_blocker_offset PWRMBASE offset to S0ix blocker counter
+ * @num_s0ix_blocker: Number of S0ix blockers
+ * @blocker_req_offset: Telemetry offset to S0ix blocker low power mode substate requirement table
*
* Each PCH has unique set of register offsets and bit indexes. This structure
* captures them to have a common implementation.
@@ -369,6 +371,8 @@ struct pmc_reg_map {
const u32 ltr_ignore_max;
const u32 pm_vric1_offset;
const u32 s0ix_blocker_offset;
+ const u32 num_s0ix_blocker;
+ const u32 blocker_req_offset;
/* Low Power Mode registers */
const int lpm_num_maps;
const int lpm_num_modes;
@@ -474,18 +478,22 @@ enum pmc_index {
* SSRAM support.
* @map: Pointer to a pmc_reg_map struct that contains platform
* specific attributes of the primary PMC
+ * @sub_req_show: File operations to show substate requirements
* @suspend: Function to perform platform specific suspend
* @resume: Function to perform platform specific resume
* @init: Function to perform platform specific init action
+ * @sub_req: Function to achieve low power mode substate requirements
*/
struct pmc_dev_info {
u8 pci_func;
u32 dmu_guid;
struct pmc_info *regmap_list;
const struct pmc_reg_map *map;
+ const struct file_operations *sub_req_show;
void (*suspend)(struct pmc_dev *pmcdev);
int (*resume)(struct pmc_dev *pmcdev);
int (*init)(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
+ int (*sub_req)(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
};
extern const struct pmc_bit_map msr_map[];
@@ -531,6 +539,12 @@ extern struct pmc_dev_info ptl_pmc_dev;
void cnl_suspend(struct pmc_dev *pmcdev);
int cnl_resume(struct pmc_dev *pmcdev);
+int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
+int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
+ struct telem_endpoint *ep);
+
+extern const struct file_operations pmc_core_substate_req_regs_fops;
+extern const struct file_operations pmc_core_substate_blk_req_fops;
#define pmc_for_each_mode(mode, pmcdev) \
for (unsigned int __i = 0, __cond; \
diff --git a/drivers/platform/x86/intel/pmc/lnl.c b/drivers/platform/x86/intel/pmc/lnl.c
index e08a77c778c2c..6fa027e7071f4 100644
--- a/drivers/platform/x86/intel/pmc/lnl.c
+++ b/drivers/platform/x86/intel/pmc/lnl.c
@@ -574,7 +574,9 @@ struct pmc_dev_info lnl_pmc_dev = {
.pci_func = 2,
.regmap_list = lnl_pmc_info_list,
.map = &lnl_socm_reg_map,
+ .sub_req_show = &pmc_core_substate_req_regs_fops,
.suspend = cnl_suspend,
.resume = lnl_resume,
.init = lnl_core_init,
+ .sub_req = pmc_core_pmt_get_lpm_req,
};
diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
index faa13a7ee688f..0b87e10f864e6 100644
--- a/drivers/platform/x86/intel/pmc/mtl.c
+++ b/drivers/platform/x86/intel/pmc/mtl.c
@@ -997,7 +997,9 @@ struct pmc_dev_info mtl_pmc_dev = {
.dmu_guid = MTL_PMT_DMU_GUID,
.regmap_list = mtl_pmc_info_list,
.map = &mtl_socm_reg_map,
+ .sub_req_show = &pmc_core_substate_req_regs_fops,
.suspend = cnl_suspend,
.resume = mtl_resume,
.init = mtl_core_init,
+ .sub_req = pmc_core_pmt_get_lpm_req,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers
2025-08-15 22:46 ` [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers Xi Pardee
@ 2025-08-28 13:06 ` Ilpo Järvinen
2025-09-01 23:52 ` Russell Haley
1 sibling, 0 replies; 11+ messages in thread
From: Ilpo Järvinen @ 2025-08-28 13:06 UTC (permalink / raw)
To: Xi Pardee
Cc: irenic.rajneesh, david.e.box, Hans de Goede, platform-driver-x86,
LKML, linux-pm
On Fri, 15 Aug 2025, Xi Pardee wrote:
> Add support to read and show S0ix blocker substate requirements.
> Starting from Panther Lake, substate requirement data is provided
> based on S0ix blockers instead of all low power mode requirements.
> For platforms that support this new feature, add support to display
> substate requirements based on S0ix blockers.
>
> Change the "substate_requirements" attribute of Intel PMC Core
> driver to show the substate requirements for each S0ix blocker
> and the corresponding S0ix blocker value.
>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
> drivers/platform/x86/intel/pmc/arl.c | 4 +
> drivers/platform/x86/intel/pmc/core.c | 126 ++++++++++++++++++++++++--
> drivers/platform/x86/intel/pmc/core.h | 14 +++
> drivers/platform/x86/intel/pmc/lnl.c | 2 +
> drivers/platform/x86/intel/pmc/mtl.c | 2 +
> 5 files changed, 138 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
> index 9d66d65e75779..17ad87b392abe 100644
> --- a/drivers/platform/x86/intel/pmc/arl.c
> +++ b/drivers/platform/x86/intel/pmc/arl.c
> @@ -725,9 +725,11 @@ struct pmc_dev_info arl_pmc_dev = {
> .dmu_guid = ARL_PMT_DMU_GUID,
> .regmap_list = arl_pmc_info_list,
> .map = &arl_socs_reg_map,
> + .sub_req_show = &pmc_core_substate_req_regs_fops,
> .suspend = cnl_suspend,
> .resume = arl_resume,
> .init = arl_core_init,
> + .sub_req = pmc_core_pmt_get_lpm_req,
> };
>
> struct pmc_dev_info arl_h_pmc_dev = {
> @@ -735,7 +737,9 @@ struct pmc_dev_info arl_h_pmc_dev = {
> .dmu_guid = ARL_PMT_DMU_GUID,
> .regmap_list = arl_pmc_info_list,
> .map = &mtl_socm_reg_map,
> + .sub_req_show = &pmc_core_substate_req_regs_fops,
> .suspend = cnl_suspend,
> .resume = arl_h_resume,
> .init = arl_h_core_init,
> + .sub_req = pmc_core_pmt_get_lpm_req,
> };
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index cbfdcdc50ad21..a0b948a875a5a 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -851,6 +851,68 @@ static void pmc_core_substate_req_header_show(struct seq_file *s, int pmc_index,
> }
> }
>
> +static int pmc_core_substate_blk_req_show(struct seq_file *s, void *unused)
> +{
> + struct pmc_dev *pmcdev = s->private;
> + unsigned int pmc_index;
> +
> + for (pmc_index = 0; pmc_index < ARRAY_SIZE(pmcdev->pmcs); pmc_index++) {
> + const struct pmc_bit_map **maps;
> + unsigned int arr_size, r_idx;
> + u32 offset, counter;
> + u32 *lpm_req_regs;
> + struct pmc *pmc;
> +
> + pmc = pmcdev->pmcs[pmc_index];
> + if (!pmc || !pmc->lpm_req_regs)
> + continue;
> +
> + lpm_req_regs = pmc->lpm_req_regs;
> + maps = pmc->map->s0ix_blocker_maps;
> + offset = pmc->map->s0ix_blocker_offset;
> + arr_size = pmc_core_lpm_get_arr_size(maps);
> +
> + /* Display the header */
> + pmc_core_substate_req_header_show(s, pmc_index, HEADER_VALUE);
> +
> + for (r_idx = 0; r_idx < arr_size; r_idx++) {
> + const struct pmc_bit_map *map;
> +
> + for (map = maps[r_idx]; map->name; map++) {
> + int mode;
> +
> + if (!map->blk)
> + continue;
> +
> + counter = pmc_core_reg_read(pmc, offset);
> + seq_printf(s, "pmc%d: %34s |", pmc_index, map->name);
For printing unsigned int, use %u.
> + pmc_for_each_mode(mode, pmcdev) {
> + bool required = *lpm_req_regs & BIT(mode);
> +
> + seq_printf(s, " %9s |", required ? "Required" : " ");
> + }
> + seq_printf(s, " %9d |\n", counter);
Use %u for u32.
> + offset += map->blk * S0IX_BLK_SIZE;
> + lpm_req_regs++;
> + }
> + }
> + }
> + return 0;
> +}
> +
> +static int pmc_core_substate_blk_req_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, pmc_core_substate_blk_req_show, inode->i_private);
> +}
> +
> +const struct file_operations pmc_core_substate_blk_req_fops = {
> + .owner = THIS_MODULE,
> + .open = pmc_core_substate_blk_req_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
> +
> static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> {
> struct pmc_dev *pmcdev = s->private;
> @@ -941,7 +1003,19 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> }
> return 0;
> }
> -DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs);
> +
> +static int pmc_core_substate_req_regs_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, pmc_core_substate_req_regs_show, inode->i_private);
> +}
> +
> +const struct file_operations pmc_core_substate_req_regs_fops = {
> + .owner = THIS_MODULE,
> + .open = pmc_core_substate_req_regs_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
>
> static unsigned int pmc_core_get_crystal_freq(void)
> {
> @@ -1274,7 +1348,7 @@ static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev)
> debugfs_remove_recursive(pmcdev->dbgfs_dir);
> }
>
> -static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
> +static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
> {
> struct pmc *primary_pmc = pmcdev->pmcs[PMC_IDX_MAIN];
> struct dentry *dir;
> @@ -1341,7 +1415,7 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev)
> if (primary_pmc->lpm_req_regs) {
> debugfs_create_file("substate_requirements", 0444,
> pmcdev->dbgfs_dir, pmcdev,
> - &pmc_core_substate_req_regs_fops);
> + pmc_dev_info->sub_req_show);
> }
>
> if (primary_pmc->map->pson_residency_offset && pmc_core_is_pson_residency_enabled(pmcdev)) {
> @@ -1409,8 +1483,7 @@ static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *m
> * +----+---------------------------------------------------------+
> *
> */
> -static int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> - struct telem_endpoint *ep)
> +int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep)
> {
> const u8 *lpm_indices;
> int num_maps, mode_offset = 0;
> @@ -1448,7 +1521,37 @@ static int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> return ret;
> }
>
> -static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
> +int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> + struct telem_endpoint *ep)
> +{
> + u32 num_blocker, sample_id;
> + unsigned int index;
> + u32 *req_offset;
> + int ret;
> +
> + num_blocker = pmc->map->num_s0ix_blocker;
> + sample_id = pmc->map->blocker_req_offset;
> +
> + pmc->lpm_req_regs = devm_kcalloc(&pmcdev->pdev->dev, num_blocker,
> + sizeof(u32), GFP_KERNEL);
> + if (!pmc->lpm_req_regs)
> + return -ENOMEM;
> +
> + req_offset = pmc->lpm_req_regs;
> + for (index = 0; index < num_blocker; index++) {
> + ret = pmt_telem_read32(ep, sample_id, req_offset, 1);
> + if (ret) {
> + dev_err(&pmcdev->pdev->dev,
> + "couldn't read Low Power Mode requirements: %d\n", ret);
> + return ret;
> + }
> + sample_id++;
> + req_offset++;
These two look superfluous variables as they move in-sync with index and
xx + index or xx[index] could be used instead? I can see the pointer one
might make things a bit nicer looking, but if you preserve it, it should
be incremented inside the for loop's increment, not here.
> + }
> + return 0;
> +}
> +
> +static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
> {
> struct pci_dev *pcidev __free(pci_dev_put) = NULL;
> struct telem_endpoint *ep;
> @@ -1456,11 +1559,12 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
> u32 guid;
> int ret;
>
> - pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, func));
> + pcidev = pci_get_domain_bus_and_slot(0, 0, PCI_DEVFN(20, pmc_dev_info->pci_func));
> if (!pcidev)
> return -ENODEV;
>
> for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
> + int (*sub_req)(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
> struct pmc *pmc;
>
> pmc = pmcdev->pmcs[i];
> @@ -1477,7 +1581,9 @@ static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, int func)
> return -EPROBE_DEFER;
> }
>
> - ret = pmc_core_pmt_get_lpm_req(pmcdev, pmc, ep);
> + sub_req = pmc_dev_info->sub_req;
> + ret = sub_req(pmcdev, pmc, ep);
> +
Please don't separate a call and its error handling with empty lines.
Why is the sub_req variable necessary?
> pmt_telem_unregister_endpoint(ep);
> if (ret)
> return ret;
> @@ -1592,7 +1698,7 @@ int generic_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
> pmc_core_punit_pmt_init(pmcdev, pmc_dev_info->dmu_guid);
>
> if (ssram) {
> - ret = pmc_core_get_telem_info(pmcdev, pmc_dev_info->pci_func);
> + ret = pmc_core_get_telem_info(pmcdev, pmc_dev_info);
> if (ret)
> goto unmap_regbase;
> }
> @@ -1766,7 +1872,7 @@ static int pmc_core_probe(struct platform_device *pdev)
> pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(primary_pmc);
> pmc_core_do_dmi_quirks(primary_pmc);
>
> - pmc_core_dbgfs_register(pmcdev);
> + pmc_core_dbgfs_register(pmcdev, pmc_dev_info);
> pm_report_max_hw_sleep(FIELD_MAX(SLP_S0_RES_COUNTER_MASK) *
> pmc_core_adjust_slp_s0_step(primary_pmc, 1));
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index 4a94a4ee031e6..bfe8fba808063 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -344,6 +344,8 @@ struct pmc_bit_map {
> * @pm_read_disable_bit: Bit index to read PMC_READ_DISABLE
> * @slps0_dbg_offset: PWRMBASE offset to SLP_S0_DEBUG_REG*
> * @s0ix_blocker_offset PWRMBASE offset to S0ix blocker counter
> + * @num_s0ix_blocker: Number of S0ix blockers
> + * @blocker_req_offset: Telemetry offset to S0ix blocker low power mode substate requirement table
> *
> * Each PCH has unique set of register offsets and bit indexes. This structure
> * captures them to have a common implementation.
> @@ -369,6 +371,8 @@ struct pmc_reg_map {
> const u32 ltr_ignore_max;
> const u32 pm_vric1_offset;
> const u32 s0ix_blocker_offset;
> + const u32 num_s0ix_blocker;
> + const u32 blocker_req_offset;
> /* Low Power Mode registers */
> const int lpm_num_maps;
> const int lpm_num_modes;
> @@ -474,18 +478,22 @@ enum pmc_index {
> * SSRAM support.
> * @map: Pointer to a pmc_reg_map struct that contains platform
> * specific attributes of the primary PMC
> + * @sub_req_show: File operations to show substate requirements
> * @suspend: Function to perform platform specific suspend
> * @resume: Function to perform platform specific resume
> * @init: Function to perform platform specific init action
> + * @sub_req: Function to achieve low power mode substate requirements
> */
> struct pmc_dev_info {
> u8 pci_func;
> u32 dmu_guid;
> struct pmc_info *regmap_list;
> const struct pmc_reg_map *map;
> + const struct file_operations *sub_req_show;
> void (*suspend)(struct pmc_dev *pmcdev);
> int (*resume)(struct pmc_dev *pmcdev);
> int (*init)(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
> + int (*sub_req)(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
> };
>
> extern const struct pmc_bit_map msr_map[];
> @@ -531,6 +539,12 @@ extern struct pmc_dev_info ptl_pmc_dev;
>
> void cnl_suspend(struct pmc_dev *pmcdev);
> int cnl_resume(struct pmc_dev *pmcdev);
> +int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
> +int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> + struct telem_endpoint *ep);
> +
> +extern const struct file_operations pmc_core_substate_req_regs_fops;
> +extern const struct file_operations pmc_core_substate_blk_req_fops;
>
> #define pmc_for_each_mode(mode, pmcdev) \
> for (unsigned int __i = 0, __cond; \
> diff --git a/drivers/platform/x86/intel/pmc/lnl.c b/drivers/platform/x86/intel/pmc/lnl.c
> index e08a77c778c2c..6fa027e7071f4 100644
> --- a/drivers/platform/x86/intel/pmc/lnl.c
> +++ b/drivers/platform/x86/intel/pmc/lnl.c
> @@ -574,7 +574,9 @@ struct pmc_dev_info lnl_pmc_dev = {
> .pci_func = 2,
> .regmap_list = lnl_pmc_info_list,
> .map = &lnl_socm_reg_map,
> + .sub_req_show = &pmc_core_substate_req_regs_fops,
> .suspend = cnl_suspend,
> .resume = lnl_resume,
> .init = lnl_core_init,
> + .sub_req = pmc_core_pmt_get_lpm_req,
> };
> diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c
> index faa13a7ee688f..0b87e10f864e6 100644
> --- a/drivers/platform/x86/intel/pmc/mtl.c
> +++ b/drivers/platform/x86/intel/pmc/mtl.c
> @@ -997,7 +997,9 @@ struct pmc_dev_info mtl_pmc_dev = {
> .dmu_guid = MTL_PMT_DMU_GUID,
> .regmap_list = mtl_pmc_info_list,
> .map = &mtl_socm_reg_map,
> + .sub_req_show = &pmc_core_substate_req_regs_fops,
> .suspend = cnl_suspend,
> .resume = mtl_resume,
> .init = mtl_core_init,
> + .sub_req = pmc_core_pmt_get_lpm_req,
> };
>
--
i.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers
2025-08-15 22:46 ` [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers Xi Pardee
2025-08-28 13:06 ` Ilpo Järvinen
@ 2025-09-01 23:52 ` Russell Haley
1 sibling, 0 replies; 11+ messages in thread
From: Russell Haley @ 2025-09-01 23:52 UTC (permalink / raw)
To: Xi Pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
On 8/15/25 5:46 PM, Xi Pardee wrote:
> Add support to read and show S0ix blocker substate requirements.
> Starting from Panther Lake, substate requirement data is provided
> based on S0ix blockers instead of all low power mode requirements.
> For platforms that support this new feature, add support to display
> substate requirements based on S0ix blockers.
>
> Change the "substate_requirements" attribute of Intel PMC Core
> driver to show the substate requirements for each S0ix blocker
> and the corresponding S0ix blocker value.
>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
> drivers/platform/x86/intel/pmc/arl.c | 4 +
> drivers/platform/x86/intel/pmc/core.c | 126 ++++++++++++++++++++++++--
> drivers/platform/x86/intel/pmc/core.h | 14 +++
> drivers/platform/x86/intel/pmc/lnl.c | 2 +
> drivers/platform/x86/intel/pmc/mtl.c | 2 +
> 5 files changed, 138 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
> index 9d66d65e75779..17ad87b392abe 100644
> --- a/drivers/platform/x86/intel/pmc/arl.c
> +++ b/drivers/platform/x86/intel/pmc/arl.c
> @@ -725,9 +725,11 @@ struct pmc_dev_info arl_pmc_dev = {
> .dmu_guid = ARL_PMT_DMU_GUID,
> .regmap_list = arl_pmc_info_list,
> .map = &arl_socs_reg_map,
> + .sub_req_show = &pmc_core_substate_req_regs_fops,
> .suspend = cnl_suspend,
> .resume = arl_resume,
> .init = arl_core_init,
> + .sub_req = pmc_core_pmt_get_lpm_req,
> };
>
> struct pmc_dev_info arl_h_pmc_dev = {
> @@ -735,7 +737,9 @@ struct pmc_dev_info arl_h_pmc_dev = {
> .dmu_guid = ARL_PMT_DMU_GUID,
> .regmap_list = arl_pmc_info_list,
> .map = &mtl_socm_reg_map,
> + .sub_req_show = &pmc_core_substate_req_regs_fops,
> .suspend = cnl_suspend,
> .resume = arl_h_resume,
> .init = arl_h_core_init,
> + .sub_req = pmc_core_pmt_get_lpm_req,
> };
The cover letter and commit messages say Panther Lake, but I see a bunch
of "arl" here. Is this just confusing file and struct names, or is my
Arrow Lake 265K getting idle-blocker-cause debug info? If so it should
probably be mentioned in the changelog.
Same question about patch 5.
- Russell Haley
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 5/6] platform/x86:intel/pmc: Show device and function number
2025-08-15 22:45 [PATCH v3 0/6] Enable SSRAM support in PTL and LNL Xi Pardee
` (3 preceding siblings ...)
2025-08-15 22:46 ` [PATCH v3 4/6] platform/x86:intel/pmc: Show substate requirement for S0ix blockers Xi Pardee
@ 2025-08-15 22:46 ` Xi Pardee
2025-08-28 13:56 ` Ilpo Järvinen
2025-08-15 22:46 ` [PATCH v3 6/6] platform/x86:intel/pmc: Enable SSRAM support for Panther Lake Xi Pardee
5 siblings, 1 reply; 11+ messages in thread
From: Xi Pardee @ 2025-08-15 22:46 UTC (permalink / raw)
To: xi.pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
Add support to show device and function number for S0ix blockers. This
feature depends on S0ix blocker substate requirement table and BDF
association table. This feature is available for platforms starting from
Pather Lake.
Only a subset of S0ix blockers has device and function number associated
to it. Get the availability information from the substate requirement
table. Get the device and function number mapping information for each
S0ix blocker from the BDF association table.
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
drivers/platform/x86/intel/pmc/core.c | 182 +++++++++++++++++++++++++-
drivers/platform/x86/intel/pmc/core.h | 23 +++-
2 files changed, 203 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
index a0b948a875a5a..69ee40cbb8b8a 100644
--- a/drivers/platform/x86/intel/pmc/core.c
+++ b/drivers/platform/x86/intel/pmc/core.c
@@ -1017,6 +1017,38 @@ const struct file_operations pmc_core_substate_req_regs_fops = {
.release = single_release,
};
+static int pmc_core_bdf_show(struct seq_file *s, void *unused)
+{
+ struct pmc_dev *pmcdev = s->private;
+ unsigned int pmcidx;
+
+ seq_printf(s, "%36s | %15s | %15s |\n", "Element", "Device Number", "Function Number");
+ for (pmcidx = 0; pmcidx < ARRAY_SIZE(pmcdev->pmcs); pmcidx++) {
+ const char *name = NULL;
+ struct list_head *cur;
+ struct bdf_entry *bdf;
+ struct pmc *pmc;
+
+ pmc = pmcdev->pmcs[pmcidx];
+ if (!pmc)
+ continue;
+
+ list_for_each(cur, pmc->bdf_list) {
+ bdf = list_entry(cur, struct bdf_entry, node);
+ if (bdf->name != name) {
+ seq_printf(s, "pmc%d: %30s | %15x | %15x |\n", pmcidx,
+ bdf->name, bdf->dev_num, bdf->fun_num);
+ name = bdf->name;
+ } else {
+ seq_printf(s, "%54x | %15x |\n",
+ bdf->dev_num, bdf->fun_num);
+ }
+ }
+ }
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(pmc_core_bdf);
+
static unsigned int pmc_core_get_crystal_freq(void)
{
unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
@@ -1418,6 +1450,10 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev, struct pmc_dev_info
pmc_dev_info->sub_req_show);
}
+ if (primary_pmc->bdf_list) {
+ debugfs_create_file("bdf", 0444, pmcdev->dbgfs_dir, pmcdev, &pmc_core_bdf_fops);
+ }
+
if (primary_pmc->map->pson_residency_offset && pmc_core_is_pson_residency_enabled(pmcdev)) {
debugfs_create_file("pson_residency_usec", 0444,
pmcdev->dbgfs_dir, primary_pmc, &pmc_core_pson_residency);
@@ -1521,7 +1557,7 @@ int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct tel
return ret;
}
-int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
+static int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
struct telem_endpoint *ep)
{
u32 num_blocker, sample_id;
@@ -1551,6 +1587,150 @@ int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
return 0;
}
+static const char *pmc_core_get_next_bdf_ip_name(struct pmc *pmc, unsigned int *r_idx,
+ unsigned int *i_idx, u32 **lpm_req_regs)
+{
+ const struct pmc_bit_map **maps;
+ unsigned int arr_size;
+ bool reset = FALSE;
+
+ maps = pmc->map->s0ix_blocker_maps;
+ arr_size = pmc_core_lpm_get_arr_size(maps);
+
+ // Iteration reaches the end of the bitmap array
+ if (!maps[*r_idx][*i_idx].name)
+ (*r_idx)++;
+
+ // Iteration reaches the end of the maps
+ if (*r_idx >= arr_size)
+ return NULL;
+
+ for (; *r_idx < arr_size; (*r_idx)++) {
+ const char *ip_name;
+
+ if (reset)
+ *i_idx = 0;
+
+ for (; maps[*r_idx][*i_idx].name; reset = TRUE, (*i_idx)++) {
+ if (!maps[*r_idx][*i_idx].blk)
+ continue;
+
+ bool exist = **lpm_req_regs & BIT(BDF_EXIST_BIT);
+ (*lpm_req_regs)++;
+ if (exist) {
+ ip_name = maps[*r_idx][*i_idx].name;
+ (*i_idx)++;
+ return ip_name;
+ }
+ }
+ }
+ return NULL;
+}
+
+static int pmc_core_process_bdf(struct pmc_dev *pmcdev, struct pmc *pmc, u32 data,
+ unsigned int *r_idx, unsigned int *i_idx, u32 **lpm_req_regs,
+ const char **name)
+{
+ unsigned int i;
+
+ if (!data)
+ return 0;
+
+ if (!*name)
+ return -EINVAL;
+
+ for (i = BDF_FUN_LOW_BIT; i <= BDF_FUN_HIGH_BIT; i++) {
+ struct bdf_entry *b_entry;
+ u32 function_data;
+
+ function_data = (data & BIT(i));
+ if (function_data) {
+ b_entry = devm_kzalloc(&pmcdev->pdev->dev, sizeof(*b_entry), GFP_KERNEL);
+ if (!b_entry)
+ return -ENOMEM;
+ b_entry->dev_num = data & GENMASK(BDF_DEV_HIGH_BIT, BDF_DEV_LOW_BIT);
+ b_entry->fun_num = i - BDF_FUN_LOW_BIT;
+ b_entry->name = *name;
+ list_add_tail(&b_entry->node, pmc->bdf_list);
+ }
+ }
+
+ if (!(data & BIT(BDF_REQ_BIT)))
+ *name = pmc_core_get_next_bdf_ip_name(pmc, r_idx, i_idx, lpm_req_regs);
+
+ return 0;
+}
+
+static int pmc_core_pmt_get_bdf(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep)
+{
+ unsigned int sample_id, max_sample_id, header_id, size, r_idx, i_idx;
+ struct bdf_entry *entry;
+ u32 *lpm_reg_regs;
+ const char *name;
+ int ret;
+
+ header_id = pmc->map->bdf_offset;
+ sample_id = header_id;
+ max_sample_id = sample_id + pmc->map->bdf_table_size;
+ lpm_reg_regs = pmc->lpm_req_regs;
+ r_idx = 0;
+ i_idx = 0;
+
+ name = pmc_core_get_next_bdf_ip_name(pmc, &r_idx, &i_idx, &lpm_reg_regs);
+ if (!name)
+ return -EINVAL;
+
+ pmc->bdf_list = devm_kzalloc(&pmcdev->pdev->dev, sizeof(struct list_head), GFP_KERNEL);
+ if (!pmc->bdf_list)
+ return -ENOMEM;
+
+ INIT_LIST_HEAD(pmc->bdf_list);
+
+ for (; sample_id < max_sample_id; sample_id++) {
+ u32 data;
+
+ ret = pmt_telem_read32(ep, sample_id, &data, 1);
+ if (ret) {
+ dev_err(&pmcdev->pdev->dev,
+ "couldn't read bdf: %d\n", ret);
+ return ret;
+ }
+
+ if (sample_id == header_id) {
+ size = (data & GENMASK(BDF_SIZE_HIGH_BIT, BDF_SIZE_LOW_BIT))
+ >> BDF_SIZE_LOW_BIT;
+ header_id += size + 1;
+ continue;
+ }
+
+ ret = pmc_core_process_bdf(pmcdev, pmc, data, &r_idx, &i_idx, &lpm_reg_regs, &name);
+ if (ret)
+ return ret;
+ data = data >> BDF_SIZE;
+ ret = pmc_core_process_bdf(pmcdev, pmc, data, &r_idx, &i_idx, &lpm_reg_regs, &name);
+ if (ret)
+ return ret;
+ }
+
+ list_for_each_entry(entry, pmc->bdf_list, node) {
+ dev_dbg(&pmcdev->pdev->dev, "bdf info: name %s, dev_num %x, fun_num %x",
+ entry->name, entry->dev_num, entry->fun_num);
+ }
+ return 0;
+}
+
+int pmc_core_pmt_get_sub_req_bdf(struct pmc_dev *pmcdev, struct pmc *pmc,
+ struct telem_endpoint *ep)
+{
+ int ret;
+
+ ret = pmc_core_pmt_get_blk_sub_req(pmcdev, pmc, ep);
+ if (ret)
+ return ret;
+
+ return pmc_core_pmt_get_bdf(pmcdev, pmc, ep);
+}
+
static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
{
struct pci_dev *pcidev __free(pci_dev_put) = NULL;
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index bfe8fba808063..6ff2d171dc2ba 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -317,6 +317,24 @@ enum ppfear_regs {
#define PMC_DEVID_MTL_IOEP 0x7ecf
#define PMC_DEVID_MTL_IOEM 0x7ebf
+/* BDF offset */
+#define BDF_EXIST_BIT 3
+#define BDF_SIZE_HIGH_BIT 23
+#define BDF_SIZE_LOW_BIT 16
+#define BDF_DEV_HIGH_BIT 4
+#define BDF_DEV_LOW_BIT 0
+#define BDF_FUN_HIGH_BIT 12
+#define BDF_FUN_LOW_BIT 5
+#define BDF_REQ_BIT 15
+#define BDF_SIZE 16
+
+struct bdf_entry {
+ struct list_head node;
+ const char *name;
+ u32 dev_num;
+ u32 fun_num;
+};
+
extern const char *pmc_lpm_modes[];
struct pmc_bit_map {
@@ -373,6 +391,8 @@ struct pmc_reg_map {
const u32 s0ix_blocker_offset;
const u32 num_s0ix_blocker;
const u32 blocker_req_offset;
+ const u32 bdf_offset;
+ const u32 bdf_table_size;
/* Low Power Mode registers */
const int lpm_num_maps;
const int lpm_num_modes;
@@ -418,6 +438,7 @@ struct pmc {
const struct pmc_reg_map *map;
u32 *lpm_req_regs;
u32 ltr_ign;
+ struct list_head *bdf_list;
};
/**
@@ -540,7 +561,7 @@ extern struct pmc_dev_info ptl_pmc_dev;
void cnl_suspend(struct pmc_dev *pmcdev);
int cnl_resume(struct pmc_dev *pmcdev);
int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
-int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
+int pmc_core_pmt_get_sub_req_bdf(struct pmc_dev *pmcdev, struct pmc *pmc,
struct telem_endpoint *ep);
extern const struct file_operations pmc_core_substate_req_regs_fops;
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 5/6] platform/x86:intel/pmc: Show device and function number
2025-08-15 22:46 ` [PATCH v3 5/6] platform/x86:intel/pmc: Show device and function number Xi Pardee
@ 2025-08-28 13:56 ` Ilpo Järvinen
0 siblings, 0 replies; 11+ messages in thread
From: Ilpo Järvinen @ 2025-08-28 13:56 UTC (permalink / raw)
To: Xi Pardee
Cc: irenic.rajneesh, david.e.box, Hans de Goede, platform-driver-x86,
LKML, linux-pm
On Fri, 15 Aug 2025, Xi Pardee wrote:
> Add support to show device and function number for S0ix blockers. This
> feature depends on S0ix blocker substate requirement table and BDF
> association table. This feature is available for platforms starting from
> Pather Lake.
>
> Only a subset of S0ix blockers has device and function number associated
> to it. Get the availability information from the substate requirement
> table. Get the device and function number mapping information for each
> S0ix blocker from the BDF association table.
>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
> drivers/platform/x86/intel/pmc/core.c | 182 +++++++++++++++++++++++++-
> drivers/platform/x86/intel/pmc/core.h | 23 +++-
> 2 files changed, 203 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index a0b948a875a5a..69ee40cbb8b8a 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1017,6 +1017,38 @@ const struct file_operations pmc_core_substate_req_regs_fops = {
> .release = single_release,
> };
>
> +static int pmc_core_bdf_show(struct seq_file *s, void *unused)
> +{
> + struct pmc_dev *pmcdev = s->private;
> + unsigned int pmcidx;
> +
> + seq_printf(s, "%36s | %15s | %15s |\n", "Element", "Device Number", "Function Number");
> + for (pmcidx = 0; pmcidx < ARRAY_SIZE(pmcdev->pmcs); pmcidx++) {
Change this to pmc_idx and lets make it the only form that are added from
this point on.
The other ones should be converted to it eventually, I once had a cleanup
patch to rename them but IIRC I dropped it to not conflict with some
feature worked. Maybe you can fit a rename change into some series so I
won't end up conflicting your feature work :-).
> + const char *name = NULL;
> + struct list_head *cur;
> + struct bdf_entry *bdf;
> + struct pmc *pmc;
> +
> + pmc = pmcdev->pmcs[pmcidx];
> + if (!pmc)
> + continue;
> +
> + list_for_each(cur, pmc->bdf_list) {
> + bdf = list_entry(cur, struct bdf_entry, node);
> + if (bdf->name != name) {
> + seq_printf(s, "pmc%d: %30s | %15x | %15x |\n", pmcidx,
%u
> + bdf->name, bdf->dev_num, bdf->fun_num);
> + name = bdf->name;
> + } else {
> + seq_printf(s, "%54x | %15x |\n",
> + bdf->dev_num, bdf->fun_num);
> + }
> + }
> + }
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(pmc_core_bdf);
> +
> static unsigned int pmc_core_get_crystal_freq(void)
> {
> unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
> @@ -1418,6 +1450,10 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev, struct pmc_dev_info
> pmc_dev_info->sub_req_show);
> }
>
> + if (primary_pmc->bdf_list) {
> + debugfs_create_file("bdf", 0444, pmcdev->dbgfs_dir, pmcdev, &pmc_core_bdf_fops);
> + }
Unnecessary braces.
> +
> if (primary_pmc->map->pson_residency_offset && pmc_core_is_pson_residency_enabled(pmcdev)) {
> debugfs_create_file("pson_residency_usec", 0444,
> pmcdev->dbgfs_dir, primary_pmc, &pmc_core_pson_residency);
> @@ -1521,7 +1557,7 @@ int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct tel
> return ret;
> }
>
> -int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> +static int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> struct telem_endpoint *ep)
> {
> u32 num_blocker, sample_id;
> @@ -1551,6 +1587,150 @@ int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> return 0;
> }
>
> +static const char *pmc_core_get_next_bdf_ip_name(struct pmc *pmc, unsigned int *r_idx,
> + unsigned int *i_idx, u32 **lpm_req_regs)
> +{
> + const struct pmc_bit_map **maps;
> + unsigned int arr_size;
> + bool reset = FALSE;
FALSE is a define in some obscure header (which you probably didn't
include intentionally anyway ;-)).
Please use false.
> +
> + maps = pmc->map->s0ix_blocker_maps;
> + arr_size = pmc_core_lpm_get_arr_size(maps);
> +
> + // Iteration reaches the end of the bitmap array
This driver has used exclusively /* */ comments.
> + if (!maps[*r_idx][*i_idx].name)
> + (*r_idx)++;
> +
> + // Iteration reaches the end of the maps
> + if (*r_idx >= arr_size)
> + return NULL;
> +
> + for (; *r_idx < arr_size; (*r_idx)++) {
> + const char *ip_name;
Can't you put this to the innermost block?
> + if (reset)
Why you need this?
> + *i_idx = 0;
> +
> + for (; maps[*r_idx][*i_idx].name; reset = TRUE, (*i_idx)++) {
true
This is hard enough to understand even without that "for (;". Would
probably be better to use while () instead.
> + if (!maps[*r_idx][*i_idx].blk)
> + continue;
> +
> + bool exist = **lpm_req_regs & BIT(BDF_EXIST_BIT);
> + (*lpm_req_regs)++;
> + if (exist) {
> + ip_name = maps[*r_idx][*i_idx].name;
> + (*i_idx)++;
> + return ip_name;
> + }
> + }
> + }
> + return NULL;
> +}
TBH, this entire function is horrible mess, two nested iterators as
pointers, etc.
I'm very far from following all what going on here.
I suppose I've not seen this patch previously?
> +static int pmc_core_process_bdf(struct pmc_dev *pmcdev, struct pmc *pmc, u32 data,
> + unsigned int *r_idx, unsigned int *i_idx, u32 **lpm_req_regs,
> + const char **name)
> +{
> + unsigned int i;
> +
> + if (!data)
> + return 0;
> +
> + if (!*name)
> + return -EINVAL;
> +
> + for (i = BDF_FUN_LOW_BIT; i <= BDF_FUN_HIGH_BIT; i++) {
I think you can iterate 0 ... __fls(FIELD_MAX()).
> + struct bdf_entry *b_entry;
> + u32 function_data;
> +
> + function_data = (data & BIT(i));
> + if (function_data) {
Why the extra variable???
> + b_entry = devm_kzalloc(&pmcdev->pdev->dev, sizeof(*b_entry), GFP_KERNEL);
> + if (!b_entry)
> + return -ENOMEM;
> + b_entry->dev_num = data & GENMASK(BDF_DEV_HIGH_BIT, BDF_DEV_LOW_BIT);
> + b_entry->fun_num = i - BDF_FUN_LOW_BIT;
What "fun" stands for? Should it be "func" as is the typical short for
"function" in BDF?
> + b_entry->name = *name;
> + list_add_tail(&b_entry->node, pmc->bdf_list);
> + }
> + }
> +
> + if (!(data & BIT(BDF_REQ_BIT)))
> + *name = pmc_core_get_next_bdf_ip_name(pmc, r_idx, i_idx, lpm_req_regs);
> +
> + return 0;
> +}
> +
> +static int pmc_core_pmt_get_bdf(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep)
> +{
> + unsigned int sample_id, max_sample_id, header_id, size, r_idx, i_idx;
> + struct bdf_entry *entry;
> + u32 *lpm_reg_regs;
> + const char *name;
> + int ret;
> +
> + header_id = pmc->map->bdf_offset;
> + sample_id = header_id;
> + max_sample_id = sample_id + pmc->map->bdf_table_size;
> + lpm_reg_regs = pmc->lpm_req_regs;
> + r_idx = 0;
> + i_idx = 0;
> +
> + name = pmc_core_get_next_bdf_ip_name(pmc, &r_idx, &i_idx, &lpm_reg_regs);
> + if (!name)
> + return -EINVAL;
> +
> + pmc->bdf_list = devm_kzalloc(&pmcdev->pdev->dev, sizeof(struct list_head), GFP_KERNEL);
Should use sizeof(*xx).
But why you need to allocate the list head and not have it in place
within the pmc's struct?
> + if (!pmc->bdf_list)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(pmc->bdf_list);
> +
> + for (; sample_id < max_sample_id; sample_id++) {
> + u32 data;
> +
> + ret = pmt_telem_read32(ep, sample_id, &data, 1);
> + if (ret) {
> + dev_err(&pmcdev->pdev->dev,
> + "couldn't read bdf: %d\n", ret);
One line.
> + return ret;
> + }
> +
> + if (sample_id == header_id) {
> + size = (data & GENMASK(BDF_SIZE_HIGH_BIT, BDF_SIZE_LOW_BIT))
> + >> BDF_SIZE_LOW_BIT;
Define the field and use FIELD_GET().
> + header_id += size + 1;
No, I just cannot understand what's going on here, it's hopeless. Always
when I think I've finally understood what its all about you throw a curve
ball like this.
In case this series is in any kind of hurry. I suggest you send the series
without this patch and we work out this patch separately on top of the
applied patches (I expect the patch 1-5 to be fine on next iteration).
> + continue;
> + }
> +
> + ret = pmc_core_process_bdf(pmcdev, pmc, data, &r_idx, &i_idx, &lpm_reg_regs, &name);
> + if (ret)
> + return ret;
> + data = data >> BDF_SIZE;
> + ret = pmc_core_process_bdf(pmcdev, pmc, data, &r_idx, &i_idx, &lpm_reg_regs, &name);
> + if (ret)
> + return ret;
> + }
> +
> + list_for_each_entry(entry, pmc->bdf_list, node) {
> + dev_dbg(&pmcdev->pdev->dev, "bdf info: name %s, dev_num %x, fun_num %x",
> + entry->name, entry->dev_num, entry->fun_num);
> + }
> + return 0;
> +}
> +
> +int pmc_core_pmt_get_sub_req_bdf(struct pmc_dev *pmcdev, struct pmc *pmc,
> + struct telem_endpoint *ep)
> +{
> + int ret;
> +
> + ret = pmc_core_pmt_get_blk_sub_req(pmcdev, pmc, ep);
> + if (ret)
> + return ret;
> +
> + return pmc_core_pmt_get_bdf(pmcdev, pmc, ep);
> +}
> +
> static int pmc_core_get_telem_info(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
> {
> struct pci_dev *pcidev __free(pci_dev_put) = NULL;
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index bfe8fba808063..6ff2d171dc2ba 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -317,6 +317,24 @@ enum ppfear_regs {
> #define PMC_DEVID_MTL_IOEP 0x7ecf
> #define PMC_DEVID_MTL_IOEM 0x7ebf
>
> +/* BDF offset */
> +#define BDF_EXIST_BIT 3
> +#define BDF_SIZE_HIGH_BIT 23
> +#define BDF_SIZE_LOW_BIT 16
> +#define BDF_DEV_HIGH_BIT 4
> +#define BDF_DEV_LOW_BIT 0
> +#define BDF_FUN_HIGH_BIT 12
> +#define BDF_FUN_LOW_BIT 5
> +#define BDF_REQ_BIT 15
> +#define BDF_SIZE 16
Use BIT(), GENMASK() for most right here. All?
> +
> +struct bdf_entry {
> + struct list_head node;
> + const char *name;
> + u32 dev_num;
> + u32 fun_num;
> +};
> +
> extern const char *pmc_lpm_modes[];
>
> struct pmc_bit_map {
> @@ -373,6 +391,8 @@ struct pmc_reg_map {
> const u32 s0ix_blocker_offset;
> const u32 num_s0ix_blocker;
> const u32 blocker_req_offset;
> + const u32 bdf_offset;
> + const u32 bdf_table_size;
> /* Low Power Mode registers */
> const int lpm_num_maps;
> const int lpm_num_modes;
> @@ -418,6 +438,7 @@ struct pmc {
> const struct pmc_reg_map *map;
> u32 *lpm_req_regs;
> u32 ltr_ign;
> + struct list_head *bdf_list;
> };
>
> /**
> @@ -540,7 +561,7 @@ extern struct pmc_dev_info ptl_pmc_dev;
> void cnl_suspend(struct pmc_dev *pmcdev);
> int cnl_resume(struct pmc_dev *pmcdev);
> int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct telem_endpoint *ep);
> -int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> +int pmc_core_pmt_get_sub_req_bdf(struct pmc_dev *pmcdev, struct pmc *pmc,
> struct telem_endpoint *ep);
>
> extern const struct file_operations pmc_core_substate_req_regs_fops;
>
--
i.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 6/6] platform/x86:intel/pmc: Enable SSRAM support for Panther Lake
2025-08-15 22:45 [PATCH v3 0/6] Enable SSRAM support in PTL and LNL Xi Pardee
` (4 preceding siblings ...)
2025-08-15 22:46 ` [PATCH v3 5/6] platform/x86:intel/pmc: Show device and function number Xi Pardee
@ 2025-08-15 22:46 ` Xi Pardee
5 siblings, 0 replies; 11+ messages in thread
From: Xi Pardee @ 2025-08-15 22:46 UTC (permalink / raw)
To: xi.pardee, irenic.rajneesh, david.e.box, hdegoede, ilpo.jarvinen,
platform-driver-x86, linux-kernel, linux-pm
Enable Panther Lake platforms to achieve PMC information from
Intel PMC SSRAM Telemetry driver and substate requirements data
from telemetry region.
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
---
drivers/platform/x86/intel/pmc/core.h | 4 ++++
drivers/platform/x86/intel/pmc/ptl.c | 33 +++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 6ff2d171dc2ba..381755aaeafaf 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -297,6 +297,10 @@ enum ppfear_regs {
#define PTL_PMC_LTR_CUR_ASLT 0x1C28
#define PTL_PMC_LTR_CUR_PLT 0x1C2C
#define PTL_PCD_PMC_MMIO_REG_LEN 0x31A8
+#define PTL_NUM_S0IX_BLOCKER 106
+#define PTL_BLK_REQ_OFFSET 55
+#define PTL_BDF_OFFSET 0x0
+#define PTL_BDF_TABLE_SIZE 54
/* SSRAM PMC Device ID */
/* LNL */
diff --git a/drivers/platform/x86/intel/pmc/ptl.c b/drivers/platform/x86/intel/pmc/ptl.c
index 394515af60d60..20c6dac7a6729 100644
--- a/drivers/platform/x86/intel/pmc/ptl.c
+++ b/drivers/platform/x86/intel/pmc/ptl.c
@@ -10,6 +10,17 @@
#include "core.h"
+/* PMC SSRAM PMT Telemetry GUIDS */
+#define PCDP_LPM_REQ_GUID 0x47179370
+
+/*
+ * Die Mapping to Product.
+ * Product PCDDie
+ * PTL-H PCD-H
+ * PTL-P PCD-P
+ * PTL-U PCD-P
+ */
+
static const struct pmc_bit_map ptl_pcdp_pfear_map[] = {
{"PMC_0", BIT(0)},
{"FUSE_OSSE", BIT(1)},
@@ -515,6 +526,24 @@ static const struct pmc_reg_map ptl_pcdp_reg_map = {
.lpm_live_status_offset = MTL_LPM_LIVE_STATUS_OFFSET,
.s0ix_blocker_maps = ptl_pcdp_blk_maps,
.s0ix_blocker_offset = LNL_S0IX_BLOCKER_OFFSET,
+ .num_s0ix_blocker = PTL_NUM_S0IX_BLOCKER,
+ .blocker_req_offset = PTL_BLK_REQ_OFFSET,
+ .bdf_offset = PTL_BDF_OFFSET,
+ .bdf_table_size = PTL_BDF_TABLE_SIZE,
+};
+
+static struct pmc_info ptl_pmc_info_list[] = {
+ {
+ .guid = PCDP_LPM_REQ_GUID,
+ .devid = PMC_DEVID_PTL_PCDH,
+ .map = &ptl_pcdp_reg_map,
+ },
+ {
+ .guid = PCDP_LPM_REQ_GUID,
+ .devid = PMC_DEVID_PTL_PCDP,
+ .map = &ptl_pcdp_reg_map,
+ },
+ {}
};
#define PTL_NPU_PCI_DEV 0xb03e
@@ -543,8 +572,12 @@ static int ptl_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_in
}
struct pmc_dev_info ptl_pmc_dev = {
+ .pci_func = 2,
+ .regmap_list = ptl_pmc_info_list,
.map = &ptl_pcdp_reg_map,
+ .sub_req_show = &pmc_core_substate_blk_req_fops,
.suspend = cnl_suspend,
.resume = ptl_resume,
.init = ptl_core_init,
+ .sub_req = pmc_core_pmt_get_sub_req_bdf,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread