Hi, On 4/1/21 5:05 AM, David E. Box wrote: > From: Gayatri Kammela <gayatri.kammela@xxxxxxxxx> > > The current implementation of pmc_core_substate_res_show() is written > specifically for Tiger Lake. However, new platforms will also have > sub-states and may support different modes. Therefore rewrite the code to > handle sub-states generically. > > Read the number and type of enabled states from the PMC. Use the Low > Power Mode (LPM) priority register to store the states in order from > shallowest to deepest for displaying. Add a for_each macro to simplify > this. While changing the sub-state display it makes sense to show only the > "enabled" sub-states instead of showing all possible ones. After this > patch, the debugfs file looks like this: > > Substate Residency > S0i2.0 0 > S0i3.0 0 > S0i2.1 9329279 > S0i3.1 0 > S0i3.2 0 > > Suggested-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx> > Signed-off-by: Gayatri Kammela <gayatri.kammela@xxxxxxxxx> > Signed-off-by: David E. Box <david.e.box@xxxxxxxxxxxxxxx> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> Regards, Hans > --- > drivers/platform/x86/intel_pmc_core.c | 59 ++++++++++++++++++++++----- > drivers/platform/x86/intel_pmc_core.h | 18 +++++++- > 2 files changed, 64 insertions(+), 13 deletions(-) > > diff --git a/drivers/platform/x86/intel_pmc_core.c b/drivers/platform/x86/intel_pmc_core.c > index 5ca40fe3da59..ce300c2942d0 100644 > --- a/drivers/platform/x86/intel_pmc_core.c > +++ b/drivers/platform/x86/intel_pmc_core.c > @@ -577,8 +577,9 @@ static const struct pmc_reg_map tgl_reg_map = { > .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT, > .ltr_ignore_max = TGL_NUM_IP_IGN_ALLOWED, > - .lpm_modes = tgl_lpm_modes, > + .lpm_num_maps = TGL_LPM_NUM_MAPS, > .lpm_en_offset = TGL_LPM_EN_OFFSET, > + .lpm_priority_offset = TGL_LPM_PRI_OFFSET, > .lpm_residency_offset = TGL_LPM_RESIDENCY_OFFSET, > .lpm_sts = tgl_lpm_maps, > .lpm_status_offset = TGL_LPM_STATUS_OFFSET, > @@ -1028,18 +1029,14 @@ DEFINE_SHOW_ATTRIBUTE(pmc_core_ltr); > static int pmc_core_substate_res_show(struct seq_file *s, void *unused) > { > struct pmc_dev *pmcdev = s->private; > - const char **lpm_modes = pmcdev->map->lpm_modes; > u32 offset = pmcdev->map->lpm_residency_offset; > - u32 lpm_en; > - int index; > + int i, mode; > > - lpm_en = pmc_core_reg_read(pmcdev, pmcdev->map->lpm_en_offset); > - seq_printf(s, "status substate residency\n"); > - for (index = 0; lpm_modes[index]; index++) { > - seq_printf(s, "%7s %7s %-15u\n", > - BIT(index) & lpm_en ? "Enabled" : " ", > - lpm_modes[index], pmc_core_reg_read(pmcdev, offset)); > - offset += 4; > + seq_printf(s, "%-10s %-15s\n", "Substate", "Residency"); > + > + pmc_for_each_mode(i, mode, pmcdev) { > + seq_printf(s, "%-10s %-15u\n", pmc_lpm_modes[mode], > + pmc_core_reg_read(pmcdev, offset + (4 * mode))); > } > > return 0; > @@ -1091,6 +1088,45 @@ static int pmc_core_pkgc_show(struct seq_file *s, void *unused) > } > DEFINE_SHOW_ATTRIBUTE(pmc_core_pkgc); > > +static void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev) > +{ > + u8 lpm_priority[LPM_MAX_NUM_MODES]; > + u32 lpm_en; > + int mode, i, p; > + > + /* Use LPM Maps to indicate support for substates */ > + if (!pmcdev->map->lpm_num_maps) > + return; > + > + lpm_en = pmc_core_reg_read(pmcdev, pmcdev->map->lpm_en_offset); > + pmcdev->num_modes = hweight32(lpm_en); > + > + /* Each byte contains information for 2 modes (7:4 and 3:0) */ > + for (mode = 0; mode < LPM_MAX_NUM_MODES; mode += 2) { > + u8 priority = pmc_core_reg_read_byte(pmcdev, > + pmcdev->map->lpm_priority_offset + (mode / 2)); > + int pri0 = GENMASK(3, 0) & priority; > + int pri1 = (GENMASK(7, 4) & priority) >> 4; > + > + lpm_priority[pri0] = mode; > + lpm_priority[pri1] = mode + 1; > + } > + > + /* > + * Loop though all modes from lowest to highest priority, > + * and capture all enabled modes in order > + */ > + i = 0; > + for (p = LPM_MAX_NUM_MODES - 1; p >= 0; p--) { > + int mode = lpm_priority[p]; > + > + if (!(BIT(mode) & lpm_en)) > + continue; > + > + pmcdev->lpm_en_modes[i++] = mode; > + } > +} > + > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > { > debugfs_remove_recursive(pmcdev->dbgfs_dir); > @@ -1267,6 +1303,7 @@ static int pmc_core_probe(struct platform_device *pdev) > > mutex_init(&pmcdev->lock); > pmcdev->pmc_xram_read_bit = pmc_core_check_read_lock_bit(pmcdev); > + pmc_core_get_low_power_modes(pmcdev); > pmc_core_do_dmi_quirks(pmcdev); > > /* > diff --git a/drivers/platform/x86/intel_pmc_core.h b/drivers/platform/x86/intel_pmc_core.h > index f33cd2c34835..5a4e3a49f5b1 100644 > --- a/drivers/platform/x86/intel_pmc_core.h > +++ b/drivers/platform/x86/intel_pmc_core.h > @@ -187,6 +187,8 @@ enum ppfear_regs { > #define ICL_PMC_LTR_WIGIG 0x1BFC > #define ICL_PMC_SLP_S0_RES_COUNTER_STEP 0x64 > > +#define LPM_MAX_NUM_MODES 8 > + > #define TGL_NUM_IP_IGN_ALLOWED 22 > #define TGL_PMC_SLP_S0_RES_COUNTER_STEP 0x7A > > @@ -199,8 +201,10 @@ enum ppfear_regs { > /* Tigerlake Low Power Mode debug registers */ > #define TGL_LPM_STATUS_OFFSET 0x1C3C > #define TGL_LPM_LIVE_STATUS_OFFSET 0x1C5C > +#define TGL_LPM_PRI_OFFSET 0x1C7C > +#define TGL_LPM_NUM_MAPS 6 > > -const char *tgl_lpm_modes[] = { > +const char *pmc_lpm_modes[] = { > "S0i2.0", > "S0i2.1", > "S0i2.2", > @@ -258,8 +262,9 @@ struct pmc_reg_map { > const u32 ltr_ignore_max; > const u32 pm_vric1_offset; > /* Low Power Mode registers */ > - const char **lpm_modes; > + const int lpm_num_maps; > const u32 lpm_en_offset; > + const u32 lpm_priority_offset; > const u32 lpm_residency_offset; > const u32 lpm_status_offset; > const u32 lpm_live_status_offset; > @@ -278,6 +283,8 @@ struct pmc_reg_map { > * @check_counters: On resume, check if counters are getting incremented > * @pc10_counter: PC10 residency counter > * @s0ix_counter: S0ix residency (step adjusted) > + * @num_modes: Count of enabled modes > + * @lpm_en_modes: Array of enabled modes from lowest to highest priority > * > * pmc_dev contains info about power management controller device. > */ > @@ -292,6 +299,13 @@ struct pmc_dev { > bool check_counters; /* Check for counter increments on resume */ > u64 pc10_counter; > u64 s0ix_counter; > + int num_modes; > + int lpm_en_modes[LPM_MAX_NUM_MODES]; > }; > > +#define pmc_for_each_mode(i, mode, pmcdev) \ > + for (i = 0, mode = pmcdev->lpm_en_modes[i]; \ > + i < pmcdev->num_modes; \ > + i++, mode = pmcdev->lpm_en_modes[i]) > + > #endif /* PMC_CORE_H */ >