| Message ID | 20210401030558.2301621-7-david.e.box@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | intel_pmc_core: Add sub-state requirements and mode latching support | expand |
Hi, On 4/1/21 5:05 AM, David E. Box wrote: > From: Gayatri Kammela <gayatri.kammela@intel.com> > > Add the debugfs file, substate_requirements, to view the low power mode > (LPM) requirements for each enabled mode alongside the last latched status > of the condition. > > After this patch, the new file will look like this: > > Element | S0i2.0 | S0i3.0 | S0i2.1 | S0i3.1 | S0i3.2 | Status | > USB2PLL_OFF_STS | Required | Required | Required | Required | Required | | > PCIe/USB3.1_Gen2PLL_OFF_STS | Required | Required | Required | Required | Required | | > PCIe_Gen3PLL_OFF_STS | Required | Required | Required | Required | Required | Yes | > OPIOPLL_OFF_STS | Required | Required | Required | Required | Required | Yes | > OCPLL_OFF_STS | Required | Required | Required | Required | Required | Yes | > MainPLL_OFF_STS | | Required | | Required | Required | | > > Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com> > Co-developed-by: David E. Box <david.e.box@linux.intel.com> > Signed-off-by: David E. Box <david.e.box@linux.intel.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/platform/x86/intel_pmc_core.c | 86 +++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c > index 0ec26a4c715e..0b47a1da5f49 100644 > --- a/drivers/platform/x86/intel_pmc_core.c > +++ b/drivers/platform/x86/intel_pmc_core.c > @@ -1122,6 +1122,86 @@ 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) > +{ > + struct pmc_dev *pmcdev = s->private; > + int i, mode; > + > + seq_printf(s, "%30s |", "Element"); > + pmc_for_each_mode(i, mode, pmcdev) > + seq_printf(s, " %9s |", pmc_lpm_modes[mode]); > + > + seq_printf(s, " %9s |\n", "Status"); > +} > + > +static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused) > +{ > + struct pmc_dev *pmcdev = s->private; > + const struct pmc_bit_map **maps = pmcdev->map->lpm_sts; > + const struct pmc_bit_map *map; > + const int num_maps = pmcdev->map->lpm_num_maps; > + u32 sts_offset = pmcdev->map->lpm_status_offset; > + u32 *lpm_req_regs = pmcdev->lpm_req_regs; > + int mp; > + > + /* Display the header */ > + pmc_core_substate_req_header_show(s); > + > + /* Loop over maps */ > + for (mp = 0; mp < num_maps; mp++) { > + u32 req_mask = 0; > + u32 lpm_status; > + int mode, idx, i, len = 32; > + > + /* > + * Capture the requirements and create a mask so that we only > + * show an element if it's required for at least one of the > + * enabled low power modes > + */ > + pmc_for_each_mode(idx, mode, pmcdev) > + req_mask |= lpm_req_regs[mp + (mode * num_maps)]; > + > + /* Get the last latched status for this map */ > + lpm_status = pmc_core_reg_read(pmcdev, sts_offset + (mp * 4)); > + > + /* Loop over elements in this map */ > + map = maps[mp]; > + for (i = 0; map[i].name && i < len; i++) { > + u32 bit_mask = map[i].bit_mask; > + > + if (!(bit_mask & req_mask)) > + /* > + * Not required for any enabled states > + * so don't display > + */ > + continue; > + > + /* Display the element name in the first column */ > + seq_printf(s, "%30s |", map[i].name); > + > + /* Loop over the enabled states and display if required */ > + pmc_for_each_mode(idx, mode, pmcdev) { > + if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask) > + seq_printf(s, " %9s |", > + "Required"); > + else > + seq_printf(s, " %9s |", " "); > + } > + > + /* In Status column, show the last captured state of this agent */ > + if (lpm_status & bit_mask) > + seq_printf(s, " %9s |", "Yes"); > + else > + seq_printf(s, " %9s |", " "); > + > + seq_puts(s, "\n"); > + } > + } > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs); > + > static int pmc_core_pkgc_show(struct seq_file *s, void *unused) > { > struct pmc_dev *pmcdev = s->private; > @@ -1241,6 +1321,12 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev) > pmcdev->dbgfs_dir, pmcdev, > &pmc_core_substate_l_sts_regs_fops); > } > + > + if (pmcdev->lpm_req_regs) { > + debugfs_create_file("substate_requirements", 0444, > + pmcdev->dbgfs_dir, pmcdev, > + &pmc_core_substate_req_regs_fops); > + } > } > > static const struct x86_cpu_id intel_pmc_core_ids[] = { >
On Wed, Mar 31, 2021 at 11:06 PM David E. Box <david.e.box@linux.intel.com> wrote: > > From: Gayatri Kammela <gayatri.kammela@intel.com> > > Add the debugfs file, substate_requirements, to view the low power mode > (LPM) requirements for each enabled mode alongside the last latched status > of the condition. > > After this patch, the new file will look like this: > > Element | S0i2.0 | S0i3.0 | S0i2.1 | S0i3.1 | S0i3.2 | Status | > USB2PLL_OFF_STS | Required | Required | Required | Required | Required | | > PCIe/USB3.1_Gen2PLL_OFF_STS | Required | Required | Required | Required | Required | | > PCIe_Gen3PLL_OFF_STS | Required | Required | Required | Required | Required | Yes | > OPIOPLL_OFF_STS | Required | Required | Required | Required | Required | Yes | > OCPLL_OFF_STS | Required | Required | Required | Required | Required | Yes | > MainPLL_OFF_STS | | Required | | Required | Required | | > > Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com> > Co-developed-by: David E. Box <david.e.box@linux.intel.com> > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > --- > drivers/platform/x86/intel_pmc_core.c | 86 +++++++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c > index 0ec26a4c715e..0b47a1da5f49 100644 > --- a/drivers/platform/x86/intel_pmc_core.c > +++ b/drivers/platform/x86/intel_pmc_core.c > @@ -1122,6 +1122,86 @@ 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) > +{ > + struct pmc_dev *pmcdev = s->private; > + int i, mode; > + > + seq_printf(s, "%30s |", "Element"); > + pmc_for_each_mode(i, mode, pmcdev) > + seq_printf(s, " %9s |", pmc_lpm_modes[mode]); > + > + seq_printf(s, " %9s |\n", "Status"); > +} > + > +static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused) > +{ > + struct pmc_dev *pmcdev = s->private; > + const struct pmc_bit_map **maps = pmcdev->map->lpm_sts; > + const struct pmc_bit_map *map; > + const int num_maps = pmcdev->map->lpm_num_maps; > + u32 sts_offset = pmcdev->map->lpm_status_offset; > + u32 *lpm_req_regs = pmcdev->lpm_req_regs; > + int mp; > + > + /* Display the header */ > + pmc_core_substate_req_header_show(s); > + > + /* Loop over maps */ > + for (mp = 0; mp < num_maps; mp++) { > + u32 req_mask = 0; > + u32 lpm_status; > + int mode, idx, i, len = 32; > + > + /* > + * Capture the requirements and create a mask so that we only > + * show an element if it's required for at least one of the > + * enabled low power modes > + */ > + pmc_for_each_mode(idx, mode, pmcdev) > + req_mask |= lpm_req_regs[mp + (mode * num_maps)]; > + > + /* Get the last latched status for this map */ > + lpm_status = pmc_core_reg_read(pmcdev, sts_offset + (mp * 4)); > + > + /* Loop over elements in this map */ > + map = maps[mp]; > + for (i = 0; map[i].name && i < len; i++) { > + u32 bit_mask = map[i].bit_mask; > + > + if (!(bit_mask & req_mask)) > + /* > + * Not required for any enabled states > + * so don't display > + */ > + continue; > + > + /* Display the element name in the first column */ > + seq_printf(s, "%30s |", map[i].name); > + > + /* Loop over the enabled states and display if required */ > + pmc_for_each_mode(idx, mode, pmcdev) { > + if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask) > + seq_printf(s, " %9s |", > + "Required"); > + else > + seq_printf(s, " %9s |", " "); > + } > + > + /* In Status column, show the last captured state of this agent */ > + if (lpm_status & bit_mask) > + seq_printf(s, " %9s |", "Yes"); > + else > + seq_printf(s, " %9s |", " "); Why is this left blank, maybe NA (Not Available)? > + > + seq_puts(s, "\n"); > + } > + } > + > + return 0; > +} > +DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs); > + > static int pmc_core_pkgc_show(struct seq_file *s, void *unused) > { > struct pmc_dev *pmcdev = s->private; > @@ -1241,6 +1321,12 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev) > pmcdev->dbgfs_dir, pmcdev, > &pmc_core_substate_l_sts_regs_fops); > } > + > + if (pmcdev->lpm_req_regs) { > + debugfs_create_file("substate_requirements", 0444, > + pmcdev->dbgfs_dir, pmcdev, > + &pmc_core_substate_req_regs_fops); > + } > } > > static const struct x86_cpu_id intel_pmc_core_ids[] = { > -- > 2.25.1 >
On Wed, 2021-04-07 at 11:45 -0400, Rajneesh Bhardwaj wrote: > On Wed, Mar 31, 2021 at 11:06 PM David E. Box > <david.e.box@linux.intel.com> wrote: > > > > From: Gayatri Kammela <gayatri.kammela@intel.com> > > > > Add the debugfs file, substate_requirements, to view the low power > > mode > > (LPM) requirements for each enabled mode alongside the last latched > > status > > of the condition. > > > > After this patch, the new file will look like this: > > > > Element | S0i2.0 | S0i3.0 | S0i2.1 > > | S0i3.1 | S0i3.2 | Status | > > USB2PLL_OFF_STS | Required | Required | Required | > > Required | Required | | > > PCIe/USB3.1_Gen2PLL_OFF_STS | Required | Required | Required | > > Required | Required | | > > PCIe_Gen3PLL_OFF_STS | Required | Required | Required | > > Required | Required | Yes | > > OPIOPLL_OFF_STS | Required | Required | Required | > > Required | Required | Yes | > > OCPLL_OFF_STS | Required | Required | Required | > > Required | Required | Yes | > > MainPLL_OFF_STS | | Required | | > > Required | Required | | > > > > Signed-off-by: Gayatri Kammela <gayatri.kammela@intel.com> > > Co-developed-by: David E. Box <david.e.box@linux.intel.com> > > Signed-off-by: David E. Box <david.e.box@linux.intel.com> > > --- > > drivers/platform/x86/intel_pmc_core.c | 86 > > +++++++++++++++++++++++++++ > > 1 file changed, 86 insertions(+) > > > > diff --git a/drivers/platform/x86/intel_pmc_core.c > > b/drivers/platform/x86/intel_pmc_core.c > > index 0ec26a4c715e..0b47a1da5f49 100644 > > --- a/drivers/platform/x86/intel_pmc_core.c > > +++ b/drivers/platform/x86/intel_pmc_core.c > > @@ -1122,6 +1122,86 @@ 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) > > +{ > > + struct pmc_dev *pmcdev = s->private; > > + int i, mode; > > + > > + seq_printf(s, "%30s |", "Element"); > > + pmc_for_each_mode(i, mode, pmcdev) > > + seq_printf(s, " %9s |", pmc_lpm_modes[mode]); > > + > > + seq_printf(s, " %9s |\n", "Status"); > > +} > > + > > +static int pmc_core_substate_req_regs_show(struct seq_file *s, > > void *unused) > > +{ > > + struct pmc_dev *pmcdev = s->private; > > + const struct pmc_bit_map **maps = pmcdev->map->lpm_sts; > > + const struct pmc_bit_map *map; > > + const int num_maps = pmcdev->map->lpm_num_maps; > > + u32 sts_offset = pmcdev->map->lpm_status_offset; > > + u32 *lpm_req_regs = pmcdev->lpm_req_regs; > > + int mp; > > + > > + /* Display the header */ > > + pmc_core_substate_req_header_show(s); > > + > > + /* Loop over maps */ > > + for (mp = 0; mp < num_maps; mp++) { > > + u32 req_mask = 0; > > + u32 lpm_status; > > + int mode, idx, i, len = 32; > > + > > + /* > > + * Capture the requirements and create a mask so > > that we only > > + * show an element if it's required for at least > > one of the > > + * enabled low power modes > > + */ > > + pmc_for_each_mode(idx, mode, pmcdev) > > + req_mask |= lpm_req_regs[mp + (mode * > > num_maps)]; > > + > > + /* Get the last latched status for this map */ > > + lpm_status = pmc_core_reg_read(pmcdev, sts_offset + > > (mp * 4)); > > + > > + /* Loop over elements in this map */ > > + map = maps[mp]; > > + for (i = 0; map[i].name && i < len; i++) { > > + u32 bit_mask = map[i].bit_mask; > > + > > + if (!(bit_mask & req_mask)) > > + /* > > + * Not required for any enabled > > states > > + * so don't display > > + */ > > + continue; > > + > > + /* Display the element name in the first > > column */ > > + seq_printf(s, "%30s |", map[i].name); > > + > > + /* Loop over the enabled states and display > > if required */ > > + pmc_for_each_mode(idx, mode, pmcdev) { > > + if (lpm_req_regs[mp + (mode * > > num_maps)] & bit_mask) > > + seq_printf(s, " %9s |", > > + "Required"); > > + else > > + seq_printf(s, " %9s |", " > > "); > > + } > > + > > + /* In Status column, show the last captured > > state of this agent */ > > + if (lpm_status & bit_mask) > > + seq_printf(s, " %9s |", "Yes"); > > + else > > + seq_printf(s, " %9s |", " "); > > Why is this left blank, maybe NA (Not Available)? The last column shows that last latched state of that agent. So if anything it would be "Not Seen". But a blank space makes it visually easier to parse. David > > > + > > + seq_puts(s, "\n"); > > + } > > + } > > + > > + return 0; > > +} > > +DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs); > > + > > static int pmc_core_pkgc_show(struct seq_file *s, void *unused) > > { > > struct pmc_dev *pmcdev = s->private; > > @@ -1241,6 +1321,12 @@ static void pmc_core_dbgfs_register(struct > > pmc_dev *pmcdev) > > pmcdev->dbgfs_dir, pmcdev, > > > > &pmc_core_substate_l_sts_regs_fops); > > } > > + > > + if (pmcdev->lpm_req_regs) { > > + debugfs_create_file("substate_requirements", 0444, > > + pmcdev->dbgfs_dir, pmcdev, > > + > > &pmc_core_substate_req_regs_fops); > > + } > > } > > > > static const struct x86_cpu_id intel_pmc_core_ids[] = { > > -- > > 2.25.1 > > > >
diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c index 0ec26a4c715e..0b47a1da5f49 100644 --- a/drivers/platform/x86/intel_pmc_core.c +++ b/drivers/platform/x86/intel_pmc_core.c @@ -1122,6 +1122,86 @@ 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) +{ + struct pmc_dev *pmcdev = s->private; + int i, mode; + + seq_printf(s, "%30s |", "Element"); + pmc_for_each_mode(i, mode, pmcdev) + seq_printf(s, " %9s |", pmc_lpm_modes[mode]); + + seq_printf(s, " %9s |\n", "Status"); +} + +static int pmc_core_substate_req_regs_show(struct seq_file *s, void *unused) +{ + struct pmc_dev *pmcdev = s->private; + const struct pmc_bit_map **maps = pmcdev->map->lpm_sts; + const struct pmc_bit_map *map; + const int num_maps = pmcdev->map->lpm_num_maps; + u32 sts_offset = pmcdev->map->lpm_status_offset; + u32 *lpm_req_regs = pmcdev->lpm_req_regs; + int mp; + + /* Display the header */ + pmc_core_substate_req_header_show(s); + + /* Loop over maps */ + for (mp = 0; mp < num_maps; mp++) { + u32 req_mask = 0; + u32 lpm_status; + int mode, idx, i, len = 32; + + /* + * Capture the requirements and create a mask so that we only + * show an element if it's required for at least one of the + * enabled low power modes + */ + pmc_for_each_mode(idx, mode, pmcdev) + req_mask |= lpm_req_regs[mp + (mode * num_maps)]; + + /* Get the last latched status for this map */ + lpm_status = pmc_core_reg_read(pmcdev, sts_offset + (mp * 4)); + + /* Loop over elements in this map */ + map = maps[mp]; + for (i = 0; map[i].name && i < len; i++) { + u32 bit_mask = map[i].bit_mask; + + if (!(bit_mask & req_mask)) + /* + * Not required for any enabled states + * so don't display + */ + continue; + + /* Display the element name in the first column */ + seq_printf(s, "%30s |", map[i].name); + + /* Loop over the enabled states and display if required */ + pmc_for_each_mode(idx, mode, pmcdev) { + if (lpm_req_regs[mp + (mode * num_maps)] & bit_mask) + seq_printf(s, " %9s |", + "Required"); + else + seq_printf(s, " %9s |", " "); + } + + /* In Status column, show the last captured state of this agent */ + if (lpm_status & bit_mask) + seq_printf(s, " %9s |", "Yes"); + else + seq_printf(s, " %9s |", " "); + + seq_puts(s, "\n"); + } + } + + return 0; +} +DEFINE_SHOW_ATTRIBUTE(pmc_core_substate_req_regs); + static int pmc_core_pkgc_show(struct seq_file *s, void *unused) { struct pmc_dev *pmcdev = s->private; @@ -1241,6 +1321,12 @@ static void pmc_core_dbgfs_register(struct pmc_dev *pmcdev) pmcdev->dbgfs_dir, pmcdev, &pmc_core_substate_l_sts_regs_fops); } + + if (pmcdev->lpm_req_regs) { + debugfs_create_file("substate_requirements", 0444, + pmcdev->dbgfs_dir, pmcdev, + &pmc_core_substate_req_regs_fops); + } } static const struct x86_cpu_id intel_pmc_core_ids[] = {