Hi, On 8/12/24 8:42 PM, Marek Maslanka wrote: > Allow to disable ACPI PM Timer on suspend and enable on resume. A > disabled timer helps optimise power consumption when the system is > suspended. On resume the timer is only reactivated if it was activated > prior to suspend, so unless the ACPI PM timer is enabled in the BIOS, > this won't change anything. > > The ACPI PM timer is used by Intel's iTCO/wdat_wdt watchdog to drive the > watchdog, so it doesn't need to run during suspend. > > Signed-off-by: Marek Maslanka <mmaslanka@xxxxxxxxxx> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx> Regards, Hans > > --- > Changes in v6: > - Use guard() to manage the mutex instead of mutex_lock/mutex_unlock > - Use newly added acpi_pmtmr_unregister_suspend_resume_callback to remove callback > - Link to v5: https://lore.kernel.org/lkml/20240812044028.3439329-1-mmaslanka@xxxxxxxxxx/ > --- > --- > drivers/platform/x86/intel/pmc/adl.c | 2 ++ > drivers/platform/x86/intel/pmc/cnp.c | 2 ++ > drivers/platform/x86/intel/pmc/core.c | 45 +++++++++++++++++++++++++++ > drivers/platform/x86/intel/pmc/core.h | 8 +++++ > drivers/platform/x86/intel/pmc/icl.c | 2 ++ > drivers/platform/x86/intel/pmc/mtl.c | 2 ++ > drivers/platform/x86/intel/pmc/spt.c | 2 ++ > drivers/platform/x86/intel/pmc/tgl.c | 2 ++ > 8 files changed, 65 insertions(+) > > diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c > index e7878558fd909..9d9c07f44ff61 100644 > --- a/drivers/platform/x86/intel/pmc/adl.c > +++ b/drivers/platform/x86/intel/pmc/adl.c > @@ -295,6 +295,8 @@ const struct pmc_reg_map adl_reg_map = { > .ppfear_buckets = CNP_PPFEAR_NUM_ENTRIES, > .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT, > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > .ltr_ignore_max = ADL_NUM_IP_IGN_ALLOWED, > .lpm_num_modes = ADL_LPM_NUM_MODES, > .lpm_num_maps = ADL_LPM_NUM_MAPS, > diff --git a/drivers/platform/x86/intel/pmc/cnp.c b/drivers/platform/x86/intel/pmc/cnp.c > index dd72974bf71e2..513c02670c5aa 100644 > --- a/drivers/platform/x86/intel/pmc/cnp.c > +++ b/drivers/platform/x86/intel/pmc/cnp.c > @@ -200,6 +200,8 @@ const struct pmc_reg_map cnp_reg_map = { > .ppfear_buckets = CNP_PPFEAR_NUM_ENTRIES, > .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT, > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > .ltr_ignore_max = CNP_NUM_IP_IGN_ALLOWED, > .etr3_offset = ETR3_OFFSET, > }; > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index 10c96c1a850af..d8fa9bed4cd3c 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -11,6 +11,7 @@ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > +#include <linux/acpi_pmtmr.h> > #include <linux/bitfield.h> > #include <linux/debugfs.h> > #include <linux/delay.h> > @@ -1171,6 +1172,38 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) > return val == 1; > } > > +/** > + * Enable or disable ACPI PM Timer > + * > + * This function is intended to be a callback for ACPI PM suspend/resume event. > + * The ACPI PM Timer is enabled on resume only if it was enabled during suspend. > + */ > +static void pmc_core_acpi_pm_timer_suspend_resume(void *data, bool suspend) > +{ > + struct pmc_dev *pmcdev = data; > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > + const struct pmc_reg_map *map = pmc->map; > + bool enabled; > + u32 reg; > + > + if (!map->acpi_pm_tmr_ctl_offset) > + return; > + > + guard(mutex)(&pmcdev->lock); > + > + if (!suspend && !pmcdev->enable_acpi_pm_timer_on_resume) > + return; > + > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > + enabled = !(reg & map->acpi_pm_tmr_disable_bit); > + if (suspend) > + reg |= map->acpi_pm_tmr_disable_bit; > + else > + reg &= ~map->acpi_pm_tmr_disable_bit; > + pmc_core_reg_write(pmc, map->acpi_pm_tmr_ctl_offset, reg); > + > + pmcdev->enable_acpi_pm_timer_on_resume = suspend && enabled; > +} > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > { > @@ -1362,6 +1395,7 @@ static int pmc_core_probe(struct platform_device *pdev) > struct pmc_dev *pmcdev; > const struct x86_cpu_id *cpu_id; > int (*core_init)(struct pmc_dev *pmcdev); > + const struct pmc_reg_map *map; > struct pmc *primary_pmc; > int ret; > > @@ -1420,6 +1454,11 @@ static int pmc_core_probe(struct platform_device *pdev) > pm_report_max_hw_sleep(FIELD_MAX(SLP_S0_RES_COUNTER_MASK) * > pmc_core_adjust_slp_s0_step(primary_pmc, 1)); > > + map = primary_pmc->map; > + if (map->acpi_pm_tmr_ctl_offset) > + acpi_pmtmr_register_suspend_resume_callback(pmc_core_acpi_pm_timer_suspend_resume, > + pmcdev); > + > device_initialized = true; > dev_info(&pdev->dev, " initialized\n"); > > @@ -1429,6 +1468,12 @@ static int pmc_core_probe(struct platform_device *pdev) > static void pmc_core_remove(struct platform_device *pdev) > { > struct pmc_dev *pmcdev = platform_get_drvdata(pdev); > + const struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > + const struct pmc_reg_map *map = pmc->map; > + > + if (map->acpi_pm_tmr_ctl_offset) > + acpi_pmtmr_unregister_suspend_resume_callback(); > + > pmc_core_dbgfs_unregister(pmcdev); > pmc_core_clean_structure(pdev); > } > diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h > index 83504c49a0e31..fe1a94f693b63 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/drivers/platform/x86/intel/pmc/core.h > @@ -67,6 +67,8 @@ struct telem_endpoint; > #define SPT_PMC_LTR_SCC 0x3A0 > #define SPT_PMC_LTR_ISH 0x3A4 > > +#define SPT_PMC_ACPI_PM_TMR_CTL_OFFSET 0x18FC > + > /* Sunrise Point: PGD PFET Enable Ack Status Registers */ > enum ppfear_regs { > SPT_PMC_XRAM_PPFEAR0A = 0x590, > @@ -147,6 +149,8 @@ enum ppfear_regs { > #define SPT_PMC_VRIC1_SLPS0LVEN BIT(13) > #define SPT_PMC_VRIC1_XTALSDQDIS BIT(22) > > +#define SPT_PMC_BIT_ACPI_PM_TMR_DISABLE BIT(1) > + > /* Cannonlake Power Management Controller register offsets */ > #define CNP_PMC_SLPS0_DBG_OFFSET 0x10B4 > #define CNP_PMC_PM_CFG_OFFSET 0x1818 > @@ -344,6 +348,8 @@ struct pmc_reg_map { > const u8 *lpm_reg_index; > const u32 pson_residency_offset; > const u32 pson_residency_counter_step; > + const u32 acpi_pm_tmr_ctl_offset; > + const u32 acpi_pm_tmr_disable_bit; > }; > > /** > @@ -417,6 +423,8 @@ struct pmc_dev { > u32 die_c6_offset; > struct telem_endpoint *punit_ep; > struct pmc_info *regmap_list; > + > + bool enable_acpi_pm_timer_on_resume; > }; > > enum pmc_index { > diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c > index 71b0fd6cb7d84..cbbd440544688 100644 > --- a/drivers/platform/x86/intel/pmc/icl.c > +++ b/drivers/platform/x86/intel/pmc/icl.c > @@ -46,6 +46,8 @@ const struct pmc_reg_map icl_reg_map = { > .ppfear_buckets = ICL_PPFEAR_NUM_ENTRIES, > .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT, > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > .ltr_ignore_max = ICL_NUM_IP_IGN_ALLOWED, > .etr3_offset = ETR3_OFFSET, > }; > diff --git a/drivers/platform/x86/intel/pmc/mtl.c b/drivers/platform/x86/intel/pmc/mtl.c > index c7d15d864039d..91f2fa728f5c8 100644 > --- a/drivers/platform/x86/intel/pmc/mtl.c > +++ b/drivers/platform/x86/intel/pmc/mtl.c > @@ -462,6 +462,8 @@ const struct pmc_reg_map mtl_socm_reg_map = { > .ppfear_buckets = MTL_SOCM_PPFEAR_NUM_ENTRIES, > .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT, > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > .lpm_num_maps = ADL_LPM_NUM_MAPS, > .ltr_ignore_max = MTL_SOCM_NUM_IP_IGN_ALLOWED, > .lpm_res_counter_step_x2 = TGL_PMC_LPM_RES_COUNTER_STEP_X2, > diff --git a/drivers/platform/x86/intel/pmc/spt.c b/drivers/platform/x86/intel/pmc/spt.c > index ab993a69e33ee..2cd2b3c68e468 100644 > --- a/drivers/platform/x86/intel/pmc/spt.c > +++ b/drivers/platform/x86/intel/pmc/spt.c > @@ -130,6 +130,8 @@ const struct pmc_reg_map spt_reg_map = { > .ppfear_buckets = SPT_PPFEAR_NUM_ENTRIES, > .pm_cfg_offset = SPT_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = SPT_PMC_READ_DISABLE_BIT, > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > .ltr_ignore_max = SPT_NUM_IP_IGN_ALLOWED, > .pm_vric1_offset = SPT_PMC_VRIC1_OFFSET, > }; > diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c > index e0580de180773..371b4e30f1426 100644 > --- a/drivers/platform/x86/intel/pmc/tgl.c > +++ b/drivers/platform/x86/intel/pmc/tgl.c > @@ -197,6 +197,8 @@ const struct pmc_reg_map tgl_reg_map = { > .ppfear_buckets = ICL_PPFEAR_NUM_ENTRIES, > .pm_cfg_offset = CNP_PMC_PM_CFG_OFFSET, > .pm_read_disable_bit = CNP_PMC_READ_DISABLE_BIT, > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > .ltr_ignore_max = TGL_NUM_IP_IGN_ALLOWED, > .lpm_num_maps = TGL_LPM_NUM_MAPS, > .lpm_res_counter_step_x2 = TGL_PMC_LPM_RES_COUNTER_STEP_X2,