From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Xi Pardee <xi.pardee@linux.intel.com>
Cc: irenic.rajneesh@gmail.com, david.e.box@linux.intel.com,
hdegoede@redhat.com, platform-driver-x86@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>,
linux-pm@vger.kernel.org
Subject: Re: [PATCH 2/3] platform/x86/intel/pmc: Move LPM mode attributes to PMC
Date: Tue, 23 Dec 2025 14:34:51 +0200 (EET) [thread overview]
Message-ID: <d9439785-e9d8-ec6b-740a-382efe2eaf22@linux.intel.com> (raw)
In-Reply-To: <20251217002343.2289577-3-xi.pardee@linux.intel.com>
On Tue, 16 Dec 2025, Xi Pardee wrote:
> Move LPM modes attributes from the pmc_dev to the pmc
> structure. LPM modes are PMC-specific and should be
> stored within the pmc structure.
This looks very short. Please reflow your commit message paragraphs to 72
chars.
> After the change, LPM mode information will be retrieved
> and stored per PMC. The substate_requirements attribute
> in debugfs will display the requirements for each enabled
> LPM substate.
>
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> ---
> drivers/platform/x86/intel/pmc/core.c | 39 ++++++++++++++++++---------
> drivers/platform/x86/intel/pmc/core.h | 15 +++++------
> 2 files changed, 34 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index 3e916228e7ed2..25f77a9dc42c5 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -783,7 +783,7 @@ static int pmc_core_substate_res_show(struct seq_file *s, void *unused)
>
> seq_printf(s, "%-10s %-15s\n", "Substate", "Residency");
>
> - pmc_for_each_mode(mode, pmcdev) {
> + pmc_for_each_mode(mode, pmc) {
> seq_printf(s, "%-10s %-15llu\n", pmc_lpm_modes[mode],
> adjust_lpm_residency(pmc, offset + (4 * mode), lpm_adj_x2));
> }
> @@ -838,10 +838,11 @@ 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;
> + struct pmc *pmc = pmcdev->pmcs[pmc_index];
> u8 mode;
>
> seq_printf(s, "%40s |", "Element");
> - pmc_for_each_mode(mode, pmcdev)
> + pmc_for_each_mode(mode, pmc)
> seq_printf(s, " %9s |", pmc_lpm_modes[mode]);
>
> if (type == HEADER_STATUS) {
> @@ -887,7 +888,7 @@ static int pmc_core_substate_blk_req_show(struct seq_file *s, void *unused)
>
> counter = pmc_core_reg_read(pmc, offset);
> seq_printf(s, "pmc%u: %34s |", pmc_idx, map->name);
> - pmc_for_each_mode(mode, pmcdev) {
> + pmc_for_each_mode(mode, pmc) {
> bool required = *lpm_req_regs & BIT(mode);
>
> seq_printf(s, " %9s |", required ? "Required" : " ");
> @@ -961,7 +962,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> * show an element if it's required for at least one of the
> * enabled low power modes
> */
> - pmc_for_each_mode(mode, pmcdev)
> + pmc_for_each_mode(mode, pmc)
> req_mask |= lpm_req_regs[mp + (mode * num_maps)];
>
> /* Get the last latched status for this map */
> @@ -987,7 +988,7 @@ static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused)
> seq_printf(s, "pmc%d: %34s |", pmc_idx, map[i].name);
>
> /* Loop over the enabled states and display if required */
> - pmc_for_each_mode(mode, pmcdev) {
> + pmc_for_each_mode(mode, pmc) {
> bool required = lpm_req_regs[mp + (mode * num_maps)] &
> bit_mask;
> seq_printf(s, " %9s |", required ? "Required" : " ");
> @@ -1077,7 +1078,7 @@ static int pmc_core_lpm_latch_mode_show(struct seq_file *s, void *unused)
> c10 = true;
> }
>
> - pmc_for_each_mode(mode, pmcdev) {
> + pmc_for_each_mode(mode, pmc) {
> if ((BIT(mode) & reg) && !c10)
> seq_printf(s, " [%s]", pmc_lpm_modes[mode]);
> else
> @@ -1117,7 +1118,7 @@ static ssize_t pmc_core_lpm_latch_mode_write(struct file *file,
> mode = sysfs_match_string(pmc_lpm_modes, buf);
>
> /* Check string matches enabled mode */
> - pmc_for_each_mode(m, pmcdev)
> + pmc_for_each_mode(m, pmc)
> if (mode == m)
> break;
>
> @@ -1213,9 +1214,8 @@ static bool pmc_core_pri_verify(u32 lpm_pri, u8 *mode_order)
> return true;
> }
>
> -void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
> +static void pmc_core_pmc_get_low_power_modes(struct pmc_dev *pmcdev, struct pmc *pmc)
> {
> - struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN];
> u8 pri_order[LPM_MAX_NUM_MODES] = LPM_DEFAULT_PRI;
> u8 mode_order[LPM_MAX_NUM_MODES];
> u32 lpm_pri;
> @@ -1232,7 +1232,7 @@ void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
> * Lower byte is enough to cover the number of lpm modes for all
> * platforms and hence mask the upper 3 bytes.
> */
> - pmcdev->num_lpm_modes = hweight32(lpm_en & 0xFF);
> + pmc->num_lpm_modes = hweight32(lpm_en & 0xFF);
>
> /* Read 32 bit LPM_PRI register */
> lpm_pri = pmc_core_reg_read(pmc, pmc->map->lpm_priority_offset);
> @@ -1261,7 +1261,22 @@ void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
> if (!(BIT(mode) & lpm_en))
> continue;
>
> - pmcdev->lpm_en_modes[i++] = mode;
> + pmc->lpm_en_modes[i++] = mode;
> + }
> +}
> +
> +static void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev)
> +{
> + unsigned int pmc_idx;
> +
> + for (pmc_idx = 0; pmc_idx < ARRAY_SIZE(pmcdev->pmcs); ++pmc_idx) {
Please always use post-increment when you are free to choose which one to
use.
> + struct pmc *pmc;
> +
> + pmc = pmcdev->pmcs[pmc_idx];
> + if (!pmc)
> + continue;
> +
> + pmc_core_pmc_get_low_power_modes(pmcdev, pmc);
> }
> }
>
> @@ -1506,7 +1521,7 @@ int pmc_core_pmt_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct tel
> return -ENOMEM;
>
> mode_offset = LPM_HEADER_OFFSET + LPM_MODE_OFFSET;
> - pmc_for_each_mode(mode, pmcdev) {
> + pmc_for_each_mode(mode, pmc) {
> u32 *req_offset = pmc->lpm_req_regs + (mode * num_maps);
> int m;
>
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index ead2f33ed3ed5..118c8740ad3aa 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -423,6 +423,8 @@ struct pmc_info {
> * specific attributes
> * @lpm_req_regs: List of substate requirements
> * @ltr_ign: Holds LTR ignore data while suspended
> + * @num_lpm_modes: Count of enabled modes
> + * @lpm_en_modes: Array of enabled modes from lowest to highest priority
> *
> * pmc contains info about one power management controller device.
> */
> @@ -432,6 +434,8 @@ struct pmc {
> const struct pmc_reg_map *map;
> u32 *lpm_req_regs;
> u32 ltr_ign;
> + u8 num_lpm_modes;
> + u8 lpm_en_modes[LPM_MAX_NUM_MODES];
> };
>
> /**
> @@ -446,8 +450,6 @@ struct pmc {
> * @pkgc_res_cnt: Array of PKGC residency counters
> * @num_of_pkgc: Number of PKGC
> * @s0ix_counter: S0ix residency (step adjusted)
> - * @num_lpm_modes: Count of enabled modes
> - * @lpm_en_modes: Array of enabled modes from lowest to highest priority
> * @suspend: Function to perform platform specific suspend
> * @resume: Function to perform platform specific resume
> *
> @@ -462,8 +464,6 @@ struct pmc_dev {
> struct mutex lock; /* generic mutex lock for PMC Core */
>
> u64 s0ix_counter;
> - u8 num_lpm_modes;
> - u8 lpm_en_modes[LPM_MAX_NUM_MODES];
> void (*suspend)(struct pmc_dev *pmcdev);
> int (*resume)(struct pmc_dev *pmcdev);
>
> @@ -535,7 +535,6 @@ int pmc_core_send_ltr_ignore(struct pmc_dev *pmcdev, u32 value, int ignore);
>
> int pmc_core_resume_common(struct pmc_dev *pmcdev);
> int get_primary_reg_base(struct pmc *pmc);
> -void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev);
> void pmc_core_punit_pmt_init(struct pmc_dev *pmcdev, u32 *guids);
> void pmc_core_set_device_d3(unsigned int device);
>
> @@ -563,10 +562,10 @@ int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
> 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) \
> +#define pmc_for_each_mode(mode, pmc) \
> for (unsigned int __i = 0, __cond; \
> - __cond = __i < (pmcdev)->num_lpm_modes, \
> - __cond && ((mode) = (pmcdev)->lpm_en_modes[__i]), \
> + __cond = __i < (pmc)->num_lpm_modes, \
> + __cond && ((mode) = (pmc)->lpm_en_modes[__i]), \
> __cond; \
> __i++)
>
>
--
i.
next prev parent reply other threads:[~2025-12-23 12:34 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-17 0:23 [PATCH 0/3] Enable substate residencies for multiple PMCs Xi Pardee
2025-12-17 0:23 ` [PATCH 1/3] platform/x86/intel/pmc: Change LPM mode fields to u8 Xi Pardee
2025-12-23 12:29 ` Ilpo Järvinen
2026-01-06 23:45 ` Xi Pardee
2025-12-17 0:23 ` [PATCH 2/3] platform/x86/intel/pmc: Move LPM mode attributes to PMC Xi Pardee
2025-12-23 12:34 ` Ilpo Järvinen [this message]
2026-01-07 0:07 ` Xi Pardee
2025-12-17 0:23 ` [PATCH 3/3] platform/x86/intel/pmc: Enable substate residencies for multiple PMCs Xi Pardee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d9439785-e9d8-ec6b-740a-382efe2eaf22@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=david.e.box@linux.intel.com \
--cc=hdegoede@redhat.com \
--cc=irenic.rajneesh@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
--cc=xi.pardee@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).