* [PATCH] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended @ 2024-07-01 22:25 Marek Maslanka 2024-07-02 8:02 ` Hans de Goede 0 siblings, 1 reply; 31+ messages in thread From: Marek Maslanka @ 2024-07-01 22:25 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, David E Box, Hans de Goede, Ilpo Järvinen, Rajneesh Bhardwaj, platform-driver-x86 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. Signed-off-by: Marek Maslanka <mmaslanka@google.com> --- drivers/platform/x86/intel/pmc/Kconfig | 13 ++++++++ drivers/platform/x86/intel/pmc/adl.c | 4 +++ drivers/platform/x86/intel/pmc/cnp.c | 4 +++ drivers/platform/x86/intel/pmc/core.c | 43 ++++++++++++++++++++++++++ drivers/platform/x86/intel/pmc/core.h | 14 +++++++++ drivers/platform/x86/intel/pmc/icl.c | 4 +++ drivers/platform/x86/intel/pmc/mtl.c | 4 +++ drivers/platform/x86/intel/pmc/spt.c | 4 +++ drivers/platform/x86/intel/pmc/tgl.c | 4 +++ 9 files changed, 94 insertions(+) diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig index d2f651fbec2cf..3a563db8eba6a 100644 --- a/drivers/platform/x86/intel/pmc/Kconfig +++ b/drivers/platform/x86/intel/pmc/Kconfig @@ -24,3 +24,16 @@ config INTEL_PMC_CORE - SLPS0 Debug registers (Cannonlake/Icelake PCH) - Low Power Mode registers (Tigerlake and beyond) - PMC quirks as needed to enable SLPS0/S0ix + +config DISABLE_ACPI_PM_TIMER_ON_SUSPEND + bool "Disable ACPI PM Timer on suspend" + default n + depends on INTEL_PMC_CORE + help + Disable ACPI Power Management Timer on entering to suspend and enable it + on resume. This helps optimize energy consumption while the system is + suspend. + + This is only applicable if the ACPI PM timer is enabled by the BIOS. + + Say N if unsure. diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c index e7878558fd909..8859e0d275288 100644 --- a/drivers/platform/x86/intel/pmc/adl.c +++ b/drivers/platform/x86/intel/pmc/adl.c @@ -295,6 +295,10 @@ 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, +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ .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..e92157aa3c9f1 100644 --- a/drivers/platform/x86/intel/pmc/cnp.c +++ b/drivers/platform/x86/intel/pmc/cnp.c @@ -200,6 +200,10 @@ 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, +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ .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..a3e56c524308f 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -1171,6 +1171,37 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) return val == 1; } +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND +/* + * Enable or disable APCI PM Timer + * + * @return: Previous APCI PM Timer enabled state + */ +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) +{ + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; + const struct pmc_reg_map *map = pmc->map; + bool state; + u32 reg; + + if (!map->acpi_pm_tmr_ctl_offset) + return false; + + mutex_lock(&pmcdev->lock); + + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); + state = !(reg & map->acpi_pm_tmr_disable_bit); + if (enable) + 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); + + mutex_unlock(&pmcdev->lock); + + return state; +} +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) { @@ -1446,6 +1477,12 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) if (pmcdev->suspend) pmcdev->suspend(pmcdev); +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + /* Disable APCI PM Timer */ + pmcdev->enable_acpi_pm_timer_on_resume = + pmc_core_enable_apci_pm_timer(pmcdev, false); +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ + /* Check if the syspend will actually use S0ix */ if (pm_suspend_via_firmware()) return 0; @@ -1500,6 +1537,12 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) int offset = pmc->map->lpm_status_offset; int i; +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + /* Enable APCI PM Timer */ + if (pmcdev->enable_acpi_pm_timer_on_resume) + pmc_core_enable_apci_pm_timer(pmcdev, true); +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ + /* Check if the syspend used S0ix */ if (pm_suspend_via_firmware()) return 0; diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h index 83504c49a0e31..4d5983d741433 100644 --- a/drivers/platform/x86/intel/pmc/core.h +++ b/drivers/platform/x86/intel/pmc/core.h @@ -67,6 +67,10 @@ struct telem_endpoint; #define SPT_PMC_LTR_SCC 0x3A0 #define SPT_PMC_LTR_ISH 0x3A4 +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND +#define SPT_PMC_ACPI_PM_TMR_CTL_OFFSET 0x18FC +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ + /* Sunrise Point: PGD PFET Enable Ack Status Registers */ enum ppfear_regs { SPT_PMC_XRAM_PPFEAR0A = 0x590, @@ -147,6 +151,10 @@ enum ppfear_regs { #define SPT_PMC_VRIC1_SLPS0LVEN BIT(13) #define SPT_PMC_VRIC1_XTALSDQDIS BIT(22) +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND +#define SPT_PMC_BIT_ACPI_PM_TMR_DISABLE BIT(1) +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ + /* Cannonlake Power Management Controller register offsets */ #define CNP_PMC_SLPS0_DBG_OFFSET 0x10B4 #define CNP_PMC_PM_CFG_OFFSET 0x1818 @@ -344,6 +352,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 +427,10 @@ struct pmc_dev { u32 die_c6_offset; struct telem_endpoint *punit_ep; struct pmc_info *regmap_list; + +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + bool enable_acpi_pm_timer_on_resume; +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ }; enum pmc_index { diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c index 71b0fd6cb7d84..8b5c782e71ebd 100644 --- a/drivers/platform/x86/intel/pmc/icl.c +++ b/drivers/platform/x86/intel/pmc/icl.c @@ -46,6 +46,10 @@ 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, +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ .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..c726ef8f1d5a9 100644 --- a/drivers/platform/x86/intel/pmc/mtl.c +++ b/drivers/platform/x86/intel/pmc/mtl.c @@ -462,6 +462,10 @@ 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, +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ .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..4832e953d0403 100644 --- a/drivers/platform/x86/intel/pmc/spt.c +++ b/drivers/platform/x86/intel/pmc/spt.c @@ -130,6 +130,10 @@ 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, +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ .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..4742b84fe226e 100644 --- a/drivers/platform/x86/intel/pmc/tgl.c +++ b/drivers/platform/x86/intel/pmc/tgl.c @@ -197,6 +197,10 @@ 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, +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ .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, -- 2.45.2.803.g4e1b14247a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-01 22:25 [PATCH] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended Marek Maslanka @ 2024-07-02 8:02 ` Hans de Goede 2024-07-02 22:41 ` Marek Maślanka 0 siblings, 1 reply; 31+ messages in thread From: Hans de Goede @ 2024-07-02 8:02 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: David E Box, Ilpo Järvinen, Rajneesh Bhardwaj, platform-driver-x86 Hi Marek, On 7/2/24 12:25 AM, 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. > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> Thank you for your patch. I have not looked into it into too much detail (I expect the Intel maintainers of the driver will do that) but why is there a Kconfig option for this ? It seems to me that this is something which we simply always want to do and we don't need all the #ifdef-s ? Regards, Hans > --- > > drivers/platform/x86/intel/pmc/Kconfig | 13 ++++++++ > drivers/platform/x86/intel/pmc/adl.c | 4 +++ > drivers/platform/x86/intel/pmc/cnp.c | 4 +++ > drivers/platform/x86/intel/pmc/core.c | 43 ++++++++++++++++++++++++++ > drivers/platform/x86/intel/pmc/core.h | 14 +++++++++ > drivers/platform/x86/intel/pmc/icl.c | 4 +++ > drivers/platform/x86/intel/pmc/mtl.c | 4 +++ > drivers/platform/x86/intel/pmc/spt.c | 4 +++ > drivers/platform/x86/intel/pmc/tgl.c | 4 +++ > 9 files changed, 94 insertions(+) > > diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig > index d2f651fbec2cf..3a563db8eba6a 100644 > --- a/drivers/platform/x86/intel/pmc/Kconfig > +++ b/drivers/platform/x86/intel/pmc/Kconfig > @@ -24,3 +24,16 @@ config INTEL_PMC_CORE > - SLPS0 Debug registers (Cannonlake/Icelake PCH) > - Low Power Mode registers (Tigerlake and beyond) > - PMC quirks as needed to enable SLPS0/S0ix > + > +config DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + bool "Disable ACPI PM Timer on suspend" > + default n > + depends on INTEL_PMC_CORE > + help > + Disable ACPI Power Management Timer on entering to suspend and enable it > + on resume. This helps optimize energy consumption while the system is > + suspend. > + > + This is only applicable if the ACPI PM timer is enabled by the BIOS. > + > + Say N if unsure. > diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c > index e7878558fd909..8859e0d275288 100644 > --- a/drivers/platform/x86/intel/pmc/adl.c > +++ b/drivers/platform/x86/intel/pmc/adl.c > @@ -295,6 +295,10 @@ 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, > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > .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..e92157aa3c9f1 100644 > --- a/drivers/platform/x86/intel/pmc/cnp.c > +++ b/drivers/platform/x86/intel/pmc/cnp.c > @@ -200,6 +200,10 @@ 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, > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > .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..a3e56c524308f 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -1171,6 +1171,37 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) > return val == 1; > } > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > +/* > + * Enable or disable APCI PM Timer > + * > + * @return: Previous APCI PM Timer enabled state > + */ > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) > +{ > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > + const struct pmc_reg_map *map = pmc->map; > + bool state; > + u32 reg; > + > + if (!map->acpi_pm_tmr_ctl_offset) > + return false; > + > + mutex_lock(&pmcdev->lock); > + > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > + state = !(reg & map->acpi_pm_tmr_disable_bit); > + if (enable) > + 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); > + > + mutex_unlock(&pmcdev->lock); > + > + return state; > +} > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > { > @@ -1446,6 +1477,12 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) > if (pmcdev->suspend) > pmcdev->suspend(pmcdev); > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + /* Disable APCI PM Timer */ > + pmcdev->enable_acpi_pm_timer_on_resume = > + pmc_core_enable_apci_pm_timer(pmcdev, false); > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > + > /* Check if the syspend will actually use S0ix */ > if (pm_suspend_via_firmware()) > return 0; > @@ -1500,6 +1537,12 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) > int offset = pmc->map->lpm_status_offset; > int i; > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + /* Enable APCI PM Timer */ > + if (pmcdev->enable_acpi_pm_timer_on_resume) > + pmc_core_enable_apci_pm_timer(pmcdev, true); > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > + > /* Check if the syspend used S0ix */ > if (pm_suspend_via_firmware()) > return 0; > diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h > index 83504c49a0e31..4d5983d741433 100644 > --- a/drivers/platform/x86/intel/pmc/core.h > +++ b/drivers/platform/x86/intel/pmc/core.h > @@ -67,6 +67,10 @@ struct telem_endpoint; > #define SPT_PMC_LTR_SCC 0x3A0 > #define SPT_PMC_LTR_ISH 0x3A4 > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > +#define SPT_PMC_ACPI_PM_TMR_CTL_OFFSET 0x18FC > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > + > /* Sunrise Point: PGD PFET Enable Ack Status Registers */ > enum ppfear_regs { > SPT_PMC_XRAM_PPFEAR0A = 0x590, > @@ -147,6 +151,10 @@ enum ppfear_regs { > #define SPT_PMC_VRIC1_SLPS0LVEN BIT(13) > #define SPT_PMC_VRIC1_XTALSDQDIS BIT(22) > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > +#define SPT_PMC_BIT_ACPI_PM_TMR_DISABLE BIT(1) > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > + > /* Cannonlake Power Management Controller register offsets */ > #define CNP_PMC_SLPS0_DBG_OFFSET 0x10B4 > #define CNP_PMC_PM_CFG_OFFSET 0x1818 > @@ -344,6 +352,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 +427,10 @@ struct pmc_dev { > u32 die_c6_offset; > struct telem_endpoint *punit_ep; > struct pmc_info *regmap_list; > + > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + bool enable_acpi_pm_timer_on_resume; > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > }; > > enum pmc_index { > diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c > index 71b0fd6cb7d84..8b5c782e71ebd 100644 > --- a/drivers/platform/x86/intel/pmc/icl.c > +++ b/drivers/platform/x86/intel/pmc/icl.c > @@ -46,6 +46,10 @@ 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, > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > .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..c726ef8f1d5a9 100644 > --- a/drivers/platform/x86/intel/pmc/mtl.c > +++ b/drivers/platform/x86/intel/pmc/mtl.c > @@ -462,6 +462,10 @@ 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, > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > .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..4832e953d0403 100644 > --- a/drivers/platform/x86/intel/pmc/spt.c > +++ b/drivers/platform/x86/intel/pmc/spt.c > @@ -130,6 +130,10 @@ 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, > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > .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..4742b84fe226e 100644 > --- a/drivers/platform/x86/intel/pmc/tgl.c > +++ b/drivers/platform/x86/intel/pmc/tgl.c > @@ -197,6 +197,10 @@ 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, > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > .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, ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-02 8:02 ` Hans de Goede @ 2024-07-02 22:41 ` Marek Maślanka 2024-07-03 11:38 ` [PATCH v2] " Marek Maslanka 0 siblings, 1 reply; 31+ messages in thread From: Marek Maślanka @ 2024-07-02 22:41 UTC (permalink / raw) To: Hans de Goede Cc: LKML, David E Box, Ilpo Järvinen, Rajneesh Bhardwaj, platform-driver-x86 Hi Hans, On Tue, Jul 2, 2024 at 10:02 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi Marek, > > On 7/2/24 12:25 AM, 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. > > > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> > > Thank you for your patch. I have not looked into it into too much > detail (I expect the Intel maintainers of the driver will do that) > but why is there a Kconfig option for this ? > > It seems to me that this is something which we simply always want > to do and we don't need all the #ifdef-s ? Yes, you're right, ifdef-s are not needed as there are conditions that check if CPU support disables the timer. I'll remove it in the next patch. Best, Marek > > Regards, > > Hans > > > > > --- > > > > drivers/platform/x86/intel/pmc/Kconfig | 13 ++++++++ > > drivers/platform/x86/intel/pmc/adl.c | 4 +++ > > drivers/platform/x86/intel/pmc/cnp.c | 4 +++ > > drivers/platform/x86/intel/pmc/core.c | 43 ++++++++++++++++++++++++++ > > drivers/platform/x86/intel/pmc/core.h | 14 +++++++++ > > drivers/platform/x86/intel/pmc/icl.c | 4 +++ > > drivers/platform/x86/intel/pmc/mtl.c | 4 +++ > > drivers/platform/x86/intel/pmc/spt.c | 4 +++ > > drivers/platform/x86/intel/pmc/tgl.c | 4 +++ > > 9 files changed, 94 insertions(+) > > > > diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig > > index d2f651fbec2cf..3a563db8eba6a 100644 > > --- a/drivers/platform/x86/intel/pmc/Kconfig > > +++ b/drivers/platform/x86/intel/pmc/Kconfig > > @@ -24,3 +24,16 @@ config INTEL_PMC_CORE > > - SLPS0 Debug registers (Cannonlake/Icelake PCH) > > - Low Power Mode registers (Tigerlake and beyond) > > - PMC quirks as needed to enable SLPS0/S0ix > > + > > +config DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + bool "Disable ACPI PM Timer on suspend" > > + default n > > + depends on INTEL_PMC_CORE > > + help > > + Disable ACPI Power Management Timer on entering to suspend and enable it > > + on resume. This helps optimize energy consumption while the system is > > + suspend. > > + > > + This is only applicable if the ACPI PM timer is enabled by the BIOS. > > + > > + Say N if unsure. > > diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c > > index e7878558fd909..8859e0d275288 100644 > > --- a/drivers/platform/x86/intel/pmc/adl.c > > +++ b/drivers/platform/x86/intel/pmc/adl.c > > @@ -295,6 +295,10 @@ 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, > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > .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..e92157aa3c9f1 100644 > > --- a/drivers/platform/x86/intel/pmc/cnp.c > > +++ b/drivers/platform/x86/intel/pmc/cnp.c > > @@ -200,6 +200,10 @@ 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, > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > .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..a3e56c524308f 100644 > > --- a/drivers/platform/x86/intel/pmc/core.c > > +++ b/drivers/platform/x86/intel/pmc/core.c > > @@ -1171,6 +1171,37 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) > > return val == 1; > > } > > > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > +/* > > + * Enable or disable APCI PM Timer > > + * > > + * @return: Previous APCI PM Timer enabled state > > + */ > > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) > > +{ > > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > > + const struct pmc_reg_map *map = pmc->map; > > + bool state; > > + u32 reg; > > + > > + if (!map->acpi_pm_tmr_ctl_offset) > > + return false; > > + > > + mutex_lock(&pmcdev->lock); > > + > > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > > + state = !(reg & map->acpi_pm_tmr_disable_bit); > > + if (enable) > > + 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); > > + > > + mutex_unlock(&pmcdev->lock); > > + > > + return state; > > +} > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > > { > > @@ -1446,6 +1477,12 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) > > if (pmcdev->suspend) > > pmcdev->suspend(pmcdev); > > > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + /* Disable APCI PM Timer */ > > + pmcdev->enable_acpi_pm_timer_on_resume = > > + pmc_core_enable_apci_pm_timer(pmcdev, false); > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > + > > /* Check if the syspend will actually use S0ix */ > > if (pm_suspend_via_firmware()) > > return 0; > > @@ -1500,6 +1537,12 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) > > int offset = pmc->map->lpm_status_offset; > > int i; > > > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + /* Enable APCI PM Timer */ > > + if (pmcdev->enable_acpi_pm_timer_on_resume) > > + pmc_core_enable_apci_pm_timer(pmcdev, true); > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > + > > /* Check if the syspend used S0ix */ > > if (pm_suspend_via_firmware()) > > return 0; > > diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h > > index 83504c49a0e31..4d5983d741433 100644 > > --- a/drivers/platform/x86/intel/pmc/core.h > > +++ b/drivers/platform/x86/intel/pmc/core.h > > @@ -67,6 +67,10 @@ struct telem_endpoint; > > #define SPT_PMC_LTR_SCC 0x3A0 > > #define SPT_PMC_LTR_ISH 0x3A4 > > > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > +#define SPT_PMC_ACPI_PM_TMR_CTL_OFFSET 0x18FC > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > + > > /* Sunrise Point: PGD PFET Enable Ack Status Registers */ > > enum ppfear_regs { > > SPT_PMC_XRAM_PPFEAR0A = 0x590, > > @@ -147,6 +151,10 @@ enum ppfear_regs { > > #define SPT_PMC_VRIC1_SLPS0LVEN BIT(13) > > #define SPT_PMC_VRIC1_XTALSDQDIS BIT(22) > > > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > +#define SPT_PMC_BIT_ACPI_PM_TMR_DISABLE BIT(1) > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > + > > /* Cannonlake Power Management Controller register offsets */ > > #define CNP_PMC_SLPS0_DBG_OFFSET 0x10B4 > > #define CNP_PMC_PM_CFG_OFFSET 0x1818 > > @@ -344,6 +352,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 +427,10 @@ struct pmc_dev { > > u32 die_c6_offset; > > struct telem_endpoint *punit_ep; > > struct pmc_info *regmap_list; > > + > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + bool enable_acpi_pm_timer_on_resume; > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > }; > > > > enum pmc_index { > > diff --git a/drivers/platform/x86/intel/pmc/icl.c b/drivers/platform/x86/intel/pmc/icl.c > > index 71b0fd6cb7d84..8b5c782e71ebd 100644 > > --- a/drivers/platform/x86/intel/pmc/icl.c > > +++ b/drivers/platform/x86/intel/pmc/icl.c > > @@ -46,6 +46,10 @@ 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, > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > .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..c726ef8f1d5a9 100644 > > --- a/drivers/platform/x86/intel/pmc/mtl.c > > +++ b/drivers/platform/x86/intel/pmc/mtl.c > > @@ -462,6 +462,10 @@ 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, > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > .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..4832e953d0403 100644 > > --- a/drivers/platform/x86/intel/pmc/spt.c > > +++ b/drivers/platform/x86/intel/pmc/spt.c > > @@ -130,6 +130,10 @@ 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, > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > .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..4742b84fe226e 100644 > > --- a/drivers/platform/x86/intel/pmc/tgl.c > > +++ b/drivers/platform/x86/intel/pmc/tgl.c > > @@ -197,6 +197,10 @@ 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, > > +#ifdef CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND > > + .acpi_pm_tmr_ctl_offset = SPT_PMC_ACPI_PM_TMR_CTL_OFFSET, > > + .acpi_pm_tmr_disable_bit = SPT_PMC_BIT_ACPI_PM_TMR_DISABLE, > > +#endif /* CONFIG_DISABLE_ACPI_PM_TIMER_ON_SUSPEND */ > > .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, > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-02 22:41 ` Marek Maślanka @ 2024-07-03 11:38 ` Marek Maslanka 2024-07-03 16:30 ` Rajneesh Bhardwaj 0 siblings, 1 reply; 31+ messages in thread From: Marek Maslanka @ 2024-07-03 11:38 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 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. Signed-off-by: Marek Maslanka <mmaslanka@google.com> --- drivers/platform/x86/intel/pmc/adl.c | 2 ++ drivers/platform/x86/intel/pmc/cnp.c | 2 ++ drivers/platform/x86/intel/pmc/core.c | 37 +++++++++++++++++++++++++++ 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, 57 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..e97ac7a8a18bc 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -1171,6 +1171,35 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) return val == 1; } +/* + * Enable or disable APCI PM Timer + * + * @return: Previous APCI PM Timer enabled state + */ +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) +{ + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; + const struct pmc_reg_map *map = pmc->map; + bool state; + u32 reg; + + if (!map->acpi_pm_tmr_ctl_offset) + return false; + + mutex_lock(&pmcdev->lock); + + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); + state = !(reg & map->acpi_pm_tmr_disable_bit); + if (enable) + 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); + + mutex_unlock(&pmcdev->lock); + + return state; +} static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) { @@ -1446,6 +1475,10 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) if (pmcdev->suspend) pmcdev->suspend(pmcdev); + /* Disable APCI PM Timer */ + pmcdev->enable_acpi_pm_timer_on_resume = + pmc_core_enable_apci_pm_timer(pmcdev, false); + /* Check if the syspend will actually use S0ix */ if (pm_suspend_via_firmware()) return 0; @@ -1500,6 +1533,10 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) int offset = pmc->map->lpm_status_offset; int i; + /* Enable APCI PM Timer */ + if (pmcdev->enable_acpi_pm_timer_on_resume) + pmc_core_enable_apci_pm_timer(pmcdev, true); + /* Check if the syspend used S0ix */ if (pm_suspend_via_firmware()) return 0; 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, -- 2.45.2.803.g4e1b14247a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-03 11:38 ` [PATCH v2] " Marek Maslanka @ 2024-07-03 16:30 ` Rajneesh Bhardwaj 2024-07-11 15:34 ` David E. Box 0 siblings, 1 reply; 31+ messages in thread From: Rajneesh Bhardwaj @ 2024-07-03 16:30 UTC (permalink / raw) To: Marek Maslanka Cc: LKML, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86, David E. Box, Rafael J Wysocki On Wed, Jul 3, 2024 at 7:39 AM Marek Maslanka <mmaslanka@google.com> 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. Back in the days IIRC, it was frowned upon but I am not sure anymore. Maybe Rafael or David will have some opinion on this change. Is this something that could be done in a platform specific manner such as in coreboot? > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> > --- > drivers/platform/x86/intel/pmc/adl.c | 2 ++ > drivers/platform/x86/intel/pmc/cnp.c | 2 ++ > drivers/platform/x86/intel/pmc/core.c | 37 +++++++++++++++++++++++++++ > 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, 57 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..e97ac7a8a18bc 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -1171,6 +1171,35 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) > return val == 1; > } > > +/* > + * Enable or disable APCI PM Timer > + * > + * @return: Previous APCI PM Timer enabled state > + */ > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) > +{ > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > + const struct pmc_reg_map *map = pmc->map; > + bool state; > + u32 reg; > + > + if (!map->acpi_pm_tmr_ctl_offset) > + return false; > + > + mutex_lock(&pmcdev->lock); > + > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > + state = !(reg & map->acpi_pm_tmr_disable_bit); > + if (enable) > + 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); > + > + mutex_unlock(&pmcdev->lock); > + > + return state; > +} > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > { > @@ -1446,6 +1475,10 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) > if (pmcdev->suspend) > pmcdev->suspend(pmcdev); > > + /* Disable APCI PM Timer */ > + pmcdev->enable_acpi_pm_timer_on_resume = > + pmc_core_enable_apci_pm_timer(pmcdev, false); > + > /* Check if the syspend will actually use S0ix */ > if (pm_suspend_via_firmware()) > return 0; > @@ -1500,6 +1533,10 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) > int offset = pmc->map->lpm_status_offset; > int i; > > + /* Enable APCI PM Timer */ > + if (pmcdev->enable_acpi_pm_timer_on_resume) > + pmc_core_enable_apci_pm_timer(pmcdev, true); > + > /* Check if the syspend used S0ix */ > if (pm_suspend_via_firmware()) > return 0; > 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, > -- > 2.45.2.803.g4e1b14247a-goog > -- Thanks, Rajneesh ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-03 16:30 ` Rajneesh Bhardwaj @ 2024-07-11 15:34 ` David E. Box 2024-07-15 12:39 ` Marek Maślanka 0 siblings, 1 reply; 31+ messages in thread From: David E. Box @ 2024-07-11 15:34 UTC (permalink / raw) To: Rajneesh Bhardwaj, Marek Maslanka Cc: LKML, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86, Rafael J Wysocki Hi Marek. Thanks for the patch. On Wed, 2024-07-03 at 12:30 -0400, Rajneesh Bhardwaj wrote: > On Wed, Jul 3, 2024 at 7:39 AM Marek Maslanka <mmaslanka@google.com> 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. > > Back in the days IIRC, it was frowned upon but I am not sure anymore. > Maybe Rafael or David will have some opinion on this change. Is this > something that could be done in a platform specific manner such as in > coreboot? I discussed with Rafael. This is generally a good idea, but need to ensure that the ACPI PM Timer isn't being used as a clock source. This could mess with the timekeeping system. Also, maybe a better idea is to disable it altogether at probe time if it's not being used as a clock source. This should only be the case when TSC is unstable and HPET is unavailable, but need to confirm. David > > > > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> > > --- > > drivers/platform/x86/intel/pmc/adl.c | 2 ++ > > drivers/platform/x86/intel/pmc/cnp.c | 2 ++ > > drivers/platform/x86/intel/pmc/core.c | 37 +++++++++++++++++++++++++++ > > 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, 57 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..e97ac7a8a18bc 100644 > > --- a/drivers/platform/x86/intel/pmc/core.c > > +++ b/drivers/platform/x86/intel/pmc/core.c > > @@ -1171,6 +1171,35 @@ static bool pmc_core_is_pson_residency_enabled(struct > > pmc_dev *pmcdev) > > return val == 1; > > } > > > > +/* > > + * Enable or disable APCI PM Timer > > + * > > + * @return: Previous APCI PM Timer enabled state > > + */ > > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool > > enable) > > +{ > > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > > + const struct pmc_reg_map *map = pmc->map; > > + bool state; > > + u32 reg; > > + > > + if (!map->acpi_pm_tmr_ctl_offset) > > + return false; > > + > > + mutex_lock(&pmcdev->lock); > > + > > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > > + state = !(reg & map->acpi_pm_tmr_disable_bit); > > + if (enable) > > + 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); > > + > > + mutex_unlock(&pmcdev->lock); > > + > > + return state; > > +} > > > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > > { > > @@ -1446,6 +1475,10 @@ static __maybe_unused int pmc_core_suspend(struct > > device *dev) > > if (pmcdev->suspend) > > pmcdev->suspend(pmcdev); > > > > + /* Disable APCI PM Timer */ > > + pmcdev->enable_acpi_pm_timer_on_resume = > > + pmc_core_enable_apci_pm_timer(pmcdev, false); > > + > > /* Check if the syspend will actually use S0ix */ > > if (pm_suspend_via_firmware()) > > return 0; > > @@ -1500,6 +1533,10 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) > > int offset = pmc->map->lpm_status_offset; > > int i; > > > > + /* Enable APCI PM Timer */ > > + if (pmcdev->enable_acpi_pm_timer_on_resume) > > + pmc_core_enable_apci_pm_timer(pmcdev, true); > > + > > /* Check if the syspend used S0ix */ > > if (pm_suspend_via_firmware()) > > return 0; > > 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, > > -- > > 2.45.2.803.g4e1b14247a-goog > > > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-11 15:34 ` David E. Box @ 2024-07-15 12:39 ` Marek Maślanka 2024-07-30 12:05 ` [PATCH v3] " Marek Maslanka 0 siblings, 1 reply; 31+ messages in thread From: Marek Maślanka @ 2024-07-15 12:39 UTC (permalink / raw) To: david.e.box Cc: Rajneesh Bhardwaj, LKML, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86, Rafael J Wysocki Hi David On Thu, Jul 11, 2024 at 5:34 PM David E. Box <david.e.box@linux.intel.com> wrote: > > Hi Marek. Thanks for the patch. > > On Wed, 2024-07-03 at 12:30 -0400, Rajneesh Bhardwaj wrote: > > On Wed, Jul 3, 2024 at 7:39 AM Marek Maslanka <mmaslanka@google.com> 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. > > > > Back in the days IIRC, it was frowned upon but I am not sure anymore. > > Maybe Rafael or David will have some opinion on this change. Is this > > something that could be done in a platform specific manner such as in > > coreboot? > > I discussed with Rafael. This is generally a good idea, but need to ensure that > the ACPI PM Timer isn't being used as a clock source. This could mess with the > timekeeping system. Also, maybe a better idea is to disable it altogether at > probe time if it's not being used as a clock source. This should only be the > case when TSC is unstable and HPET is unavailable, but need to confirm. > > David > From what I can see there is no possibility right now to access the suspend_clocksource variable from outside to check what suspend clocksource is selected. I'll need to add that function. The ACPI PM Timer can't be disabled at probe time as it might be used as a timer for a watchdog. Best, Marek > > > > > > > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> > > > --- > > > drivers/platform/x86/intel/pmc/adl.c | 2 ++ > > > drivers/platform/x86/intel/pmc/cnp.c | 2 ++ > > > drivers/platform/x86/intel/pmc/core.c | 37 +++++++++++++++++++++++++++ > > > 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, 57 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..e97ac7a8a18bc 100644 > > > --- a/drivers/platform/x86/intel/pmc/core.c > > > +++ b/drivers/platform/x86/intel/pmc/core.c > > > @@ -1171,6 +1171,35 @@ static bool pmc_core_is_pson_residency_enabled(struct > > > pmc_dev *pmcdev) > > > return val == 1; > > > } > > > > > > +/* > > > + * Enable or disable APCI PM Timer > > > + * > > > + * @return: Previous APCI PM Timer enabled state > > > + */ > > > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool > > > enable) > > > +{ > > > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > > > + const struct pmc_reg_map *map = pmc->map; > > > + bool state; > > > + u32 reg; > > > + > > > + if (!map->acpi_pm_tmr_ctl_offset) > > > + return false; > > > + > > > + mutex_lock(&pmcdev->lock); > > > + > > > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > > > + state = !(reg & map->acpi_pm_tmr_disable_bit); > > > + if (enable) > > > + 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); > > > + > > > + mutex_unlock(&pmcdev->lock); > > > + > > > + return state; > > > +} > > > > > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > > > { > > > @@ -1446,6 +1475,10 @@ static __maybe_unused int pmc_core_suspend(struct > > > device *dev) > > > if (pmcdev->suspend) > > > pmcdev->suspend(pmcdev); > > > > > > + /* Disable APCI PM Timer */ > > > + pmcdev->enable_acpi_pm_timer_on_resume = > > > + pmc_core_enable_apci_pm_timer(pmcdev, false); > > > + > > > /* Check if the syspend will actually use S0ix */ > > > if (pm_suspend_via_firmware()) > > > return 0; > > > @@ -1500,6 +1533,10 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) > > > int offset = pmc->map->lpm_status_offset; > > > int i; > > > > > > + /* Enable APCI PM Timer */ > > > + if (pmcdev->enable_acpi_pm_timer_on_resume) > > > + pmc_core_enable_apci_pm_timer(pmcdev, true); > > > + > > > /* Check if the syspend used S0ix */ > > > if (pm_suspend_via_firmware()) > > > return 0; > > > 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, > > > -- > > > 2.45.2.803.g4e1b14247a-goog > > > > > > > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-15 12:39 ` Marek Maślanka @ 2024-07-30 12:05 ` Marek Maslanka 2024-07-30 12:57 ` Ilpo Järvinen 2024-07-30 16:08 ` Thomas Gleixner 0 siblings, 2 replies; 31+ messages in thread From: Marek Maslanka @ 2024-07-30 12:05 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, John Stultz, Thomas Gleixner, Stephen Boyd, platform-driver-x86 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. Signed-off-by: Marek Maslanka <mmaslanka@google.com> --- Changes in v3: - Add the clocksource_current_cs_name and clocksource_suspend_cs_name to the clocksource.c - Do not disable the ACPI PM timer if it's selected as the clocksource - Link to v2: https://lore.kernel.org/lkml/20240703113850.2726539-1-mmaslanka@google.com/ --- --- drivers/platform/x86/intel/pmc/adl.c | 2 ++ drivers/platform/x86/intel/pmc/cnp.c | 2 ++ drivers/platform/x86/intel/pmc/core.c | 47 +++++++++++++++++++++++++++ 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 ++ include/linux/clocksource.h | 2 ++ kernel/time/clocksource.c | 22 +++++++++++++ 10 files changed, 91 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..1a435f5ca08c5 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -12,6 +12,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt #include <linux/bitfield.h> +#include <linux/clocksource.h> #include <linux/debugfs.h> #include <linux/delay.h> #include <linux/dmi.h> @@ -1171,6 +1172,44 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) return val == 1; } +/* + * Enable or disable APCI PM Timer + * + * @return: Previous APCI PM Timer enabled state + */ +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) +{ + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; + const struct pmc_reg_map *map = pmc->map; + char cs_name[32]; + bool state; + u32 reg; + + if (!map->acpi_pm_tmr_ctl_offset) + return false; + + clocksource_current_cs_name(cs_name, sizeof(cs_name)); + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) + return false; + + clocksource_suspend_cs_name(cs_name, sizeof(cs_name)); + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) + return false; + + mutex_lock(&pmcdev->lock); + + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); + state = !(reg & map->acpi_pm_tmr_disable_bit); + if (enable) + 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); + + mutex_unlock(&pmcdev->lock); + + return state; +} static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) { @@ -1446,6 +1485,10 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) if (pmcdev->suspend) pmcdev->suspend(pmcdev); + /* Disable APCI PM Timer */ + pmcdev->enable_acpi_pm_timer_on_resume = + pmc_core_enable_apci_pm_timer(pmcdev, false); + /* Check if the syspend will actually use S0ix */ if (pm_suspend_via_firmware()) return 0; @@ -1500,6 +1543,10 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) int offset = pmc->map->lpm_status_offset; int i; + /* Enable APCI PM Timer */ + if (pmcdev->enable_acpi_pm_timer_on_resume) + pmc_core_enable_apci_pm_timer(pmcdev, true); + /* Check if the syspend used S0ix */ if (pm_suspend_via_firmware()) return 0; 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, diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h index 0ad8b550bb4b4..f1953c5687683 100644 --- a/include/linux/clocksource.h +++ b/include/linux/clocksource.h @@ -305,5 +305,7 @@ static inline unsigned int clocksource_get_max_watchdog_retry(void) } void clocksource_verify_percpu(struct clocksource *cs); +void clocksource_current_cs_name(char *buf, size_t buf_size); +void clocksource_suspend_cs_name(char *buf, size_t buf_size); #endif /* _LINUX_CLOCKSOURCE_H */ diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c index e5b260aa0e02c..e2e2609f7f4b2 100644 --- a/kernel/time/clocksource.c +++ b/kernel/time/clocksource.c @@ -1320,6 +1320,28 @@ int clocksource_unregister(struct clocksource *cs) } EXPORT_SYMBOL(clocksource_unregister); +void clocksource_suspend_cs_name(char *buf, size_t buf_size) +{ + mutex_lock(&clocksource_mutex); + + buf[0] = '\0'; + if (suspend_clocksource) + strscpy(buf, suspend_clocksource->name, buf_size); + + mutex_unlock(&clocksource_mutex); +} + +void clocksource_current_cs_name(char *buf, size_t buf_size) +{ + mutex_lock(&clocksource_mutex); + + buf[0] = '\0'; + if (curr_clocksource) + strscpy(buf, curr_clocksource->name, buf_size); + + mutex_unlock(&clocksource_mutex); +} + #ifdef CONFIG_SYSFS /** * current_clocksource_show - sysfs interface for current clocksource -- 2.46.0.rc2.264.g509ed76dc8-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-30 12:05 ` [PATCH v3] " Marek Maslanka @ 2024-07-30 12:57 ` Ilpo Järvinen 2024-07-30 16:08 ` Thomas Gleixner 1 sibling, 0 replies; 31+ messages in thread From: Ilpo Järvinen @ 2024-07-30 12:57 UTC (permalink / raw) To: Marek Maslanka Cc: LKML, Rajneesh Bhardwaj, David E Box, Hans de Goede, John Stultz, Thomas Gleixner, Stephen Boyd, platform-driver-x86 On Tue, 30 Jul 2024, 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. > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> > > --- > Changes in v3: > - Add the clocksource_current_cs_name and clocksource_suspend_cs_name to the clocksource.c > - Do not disable the ACPI PM timer if it's selected as the clocksource > - Link to v2: https://lore.kernel.org/lkml/20240703113850.2726539-1-mmaslanka@google.com/ > --- > --- > drivers/platform/x86/intel/pmc/adl.c | 2 ++ > drivers/platform/x86/intel/pmc/cnp.c | 2 ++ > drivers/platform/x86/intel/pmc/core.c | 47 +++++++++++++++++++++++++++ > 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 ++ > include/linux/clocksource.h | 2 ++ > kernel/time/clocksource.c | 22 +++++++++++++ > 10 files changed, 91 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..1a435f5ca08c5 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -12,6 +12,7 @@ > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > #include <linux/bitfield.h> > +#include <linux/clocksource.h> > #include <linux/debugfs.h> > #include <linux/delay.h> > #include <linux/dmi.h> > @@ -1171,6 +1172,44 @@ static bool pmc_core_is_pson_residency_enabled(struct pmc_dev *pmcdev) > return val == 1; > } > > +/* > + * Enable or disable APCI PM Timer > + * > + * @return: Previous APCI PM Timer enabled state > + */ > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) > +{ > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > + const struct pmc_reg_map *map = pmc->map; > + char cs_name[32]; > + bool state; > + u32 reg; > + > + if (!map->acpi_pm_tmr_ctl_offset) > + return false; > + > + clocksource_current_cs_name(cs_name, sizeof(cs_name)); > + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) If you want to do a prefix check, there's str_has_prefix() for that. Otherwise, I don't understand why you need the n variant? > + return false; > + > + clocksource_suspend_cs_name(cs_name, sizeof(cs_name)); > + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) > + return false; > + > + mutex_lock(&pmcdev->lock); > + > + reg = pmc_core_reg_read(pmc, map->acpi_pm_tmr_ctl_offset); > + state = !(reg & map->acpi_pm_tmr_disable_bit); > + if (enable) > + 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); > + > + mutex_unlock(&pmcdev->lock); > + > + return state; > +} > > static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) > { > @@ -1446,6 +1485,10 @@ static __maybe_unused int pmc_core_suspend(struct device *dev) > if (pmcdev->suspend) > pmcdev->suspend(pmcdev); > > + /* Disable APCI PM Timer */ > + pmcdev->enable_acpi_pm_timer_on_resume = > + pmc_core_enable_apci_pm_timer(pmcdev, false); > + > /* Check if the syspend will actually use S0ix */ > if (pm_suspend_via_firmware()) > return 0; > @@ -1500,6 +1543,10 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev) > int offset = pmc->map->lpm_status_offset; > int i; > > + /* Enable APCI PM Timer */ > + if (pmcdev->enable_acpi_pm_timer_on_resume) > + pmc_core_enable_apci_pm_timer(pmcdev, true); > + > /* Check if the syspend used S0ix */ > if (pm_suspend_via_firmware()) > return 0; > 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, > diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h > index 0ad8b550bb4b4..f1953c5687683 100644 > --- a/include/linux/clocksource.h > +++ b/include/linux/clocksource.h > @@ -305,5 +305,7 @@ static inline unsigned int clocksource_get_max_watchdog_retry(void) > } > > void clocksource_verify_percpu(struct clocksource *cs); > +void clocksource_current_cs_name(char *buf, size_t buf_size); > +void clocksource_suspend_cs_name(char *buf, size_t buf_size); > > #endif /* _LINUX_CLOCKSOURCE_H */ > diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c > index e5b260aa0e02c..e2e2609f7f4b2 100644 > --- a/kernel/time/clocksource.c > +++ b/kernel/time/clocksource.c > @@ -1320,6 +1320,28 @@ int clocksource_unregister(struct clocksource *cs) > } > EXPORT_SYMBOL(clocksource_unregister); > > +void clocksource_suspend_cs_name(char *buf, size_t buf_size) > +{ > + mutex_lock(&clocksource_mutex); > + > + buf[0] = '\0'; > + if (suspend_clocksource) > + strscpy(buf, suspend_clocksource->name, buf_size); > + > + mutex_unlock(&clocksource_mutex); > +} > + > +void clocksource_current_cs_name(char *buf, size_t buf_size) > +{ > + mutex_lock(&clocksource_mutex); > + > + buf[0] = '\0'; > + if (curr_clocksource) > + strscpy(buf, curr_clocksource->name, buf_size); > + > + mutex_unlock(&clocksource_mutex); > +} Have you tested allmodconfig build? These functions are not exported so I'd expect it to fail... One could use guard() for the mutex. -- i. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-30 12:05 ` [PATCH v3] " Marek Maslanka 2024-07-30 12:57 ` Ilpo Järvinen @ 2024-07-30 16:08 ` Thomas Gleixner 2024-07-31 14:44 ` Marek Maślanka 1 sibling, 1 reply; 31+ messages in thread From: Thomas Gleixner @ 2024-07-30 16:08 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Marek Maslanka, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, John Stultz, Stephen Boyd, platform-driver-x86 Marek! On Tue, Jul 30 2024 at 12:05, 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. > > include/linux/clocksource.h | 2 ++ > kernel/time/clocksource.c | 22 +++++++++++++ The changelog is completely silent about the core code change. That's not how it works. Add the core code change as a separate patch with a proper justification and not hide it in the pile of the PMC changes without cc'ing the relevant maintainers. It's documented how this works, no? > +/* > + * Enable or disable APCI PM Timer > + * > + * @return: Previous APCI PM Timer enabled state > + */ > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) > +{ > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > + const struct pmc_reg_map *map = pmc->map; > + char cs_name[32]; > + bool state; > + u32 reg; > + > + if (!map->acpi_pm_tmr_ctl_offset) > + return false; > + > + clocksource_current_cs_name(cs_name, sizeof(cs_name)); > + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) > + return false; > + > + clocksource_suspend_cs_name(cs_name, sizeof(cs_name)); > + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) > + return false; How would ACPI/PM ever be selected as a suspend clocksource? It's not marked CLOCK_SOURCE_SUSPEND_NONSTOP. There is a reason why clocksources have suspend/resume and enable/disable callbacks. The latter allow you to turn it completely off when it is not in use. Something like the below should work. It's uncompiled, but you get the idea. Thanks, tglx --- --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -63,12 +63,40 @@ static u64 acpi_pm_read(struct clocksour return (u64)read_pmtmr(); } +static bool acpi_pm_enabled; + +static void (*enable_callback)(bool enable); + +bool acpi_pm_register_enable_callback(void (*cb)(bool enable)) +{ + enable_callback = cb; + if (cb) + cb(acpi_pm_enabled); +} + +static int acpi_pm_enable(struct clocksource *cs) +{ + acpi_pm_enabled = true; + if (enable_callback) + enable_callback(true); + return 0; +} + +static void acpi_pm_disable(struct clocksource *cs) +{ + acpi_pm_enabled = false; + if (enable_callback) + enable_callback(false); +} + static struct clocksource clocksource_acpi_pm = { .name = "acpi_pm", .rating = 200, .read = acpi_pm_read, .mask = (u64)ACPI_PM_MASK, .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .enable = acpi_pm_enable, + .disable = acpi_pm_disable, }; ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-30 16:08 ` Thomas Gleixner @ 2024-07-31 14:44 ` Marek Maślanka 2024-07-31 16:33 ` Thomas Gleixner 0 siblings, 1 reply; 31+ messages in thread From: Marek Maślanka @ 2024-07-31 14:44 UTC (permalink / raw) To: Thomas Gleixner Cc: LKML, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, John Stultz, Stephen Boyd, platform-driver-x86 Hi Thomas On Tue, Jul 30, 2024 at 6:08 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > Marek! > > On Tue, Jul 30 2024 at 12:05, 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. > > > > include/linux/clocksource.h | 2 ++ > > kernel/time/clocksource.c | 22 +++++++++++++ > > The changelog is completely silent about the core code change. That's > not how it works. > > Add the core code change as a separate patch with a proper justification > and not hide it in the pile of the PMC changes without cc'ing the > relevant maintainers. It's documented how this works, no? Ok > > > +/* > > + * Enable or disable APCI PM Timer > > + * > > + * @return: Previous APCI PM Timer enabled state > > + */ > > +static bool pmc_core_enable_apci_pm_timer(struct pmc_dev *pmcdev, bool enable) > > +{ > > + struct pmc *pmc = pmcdev->pmcs[PMC_IDX_MAIN]; > > + const struct pmc_reg_map *map = pmc->map; > > + char cs_name[32]; > > + bool state; > > + u32 reg; > > + > > + if (!map->acpi_pm_tmr_ctl_offset) > > + return false; > > + > > + clocksource_current_cs_name(cs_name, sizeof(cs_name)); > > + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) > > + return false; > > + > > + clocksource_suspend_cs_name(cs_name, sizeof(cs_name)); > > + if (strncmp(cs_name, "acpi_pm", sizeof(cs_name)) == 0) > > + return false; > > How would ACPI/PM ever be selected as a suspend clocksource? It's not > marked CLOCK_SOURCE_SUSPEND_NONSTOP. > > There is a reason why clocksources have suspend/resume and > enable/disable callbacks. The latter allow you to turn it completely off > when it is not in use. > > Something like the below should work. It's uncompiled, but you get the > idea. > > Thanks, > > tglx > --- > --- a/drivers/clocksource/acpi_pm.c > +++ b/drivers/clocksource/acpi_pm.c > @@ -63,12 +63,40 @@ static u64 acpi_pm_read(struct clocksour > return (u64)read_pmtmr(); > } > > +static bool acpi_pm_enabled; > + > +static void (*enable_callback)(bool enable); > + > +bool acpi_pm_register_enable_callback(void (*cb)(bool enable)) > +{ > + enable_callback = cb; > + if (cb) > + cb(acpi_pm_enabled); > +} > + > +static int acpi_pm_enable(struct clocksource *cs) > +{ > + acpi_pm_enabled = true; > + if (enable_callback) > + enable_callback(true); > + return 0; > +} > + > +static void acpi_pm_disable(struct clocksource *cs) > +{ > + acpi_pm_enabled = false; > + if (enable_callback) > + enable_callback(false); > +} > + > static struct clocksource clocksource_acpi_pm = { > .name = "acpi_pm", > .rating = 200, > .read = acpi_pm_read, > .mask = (u64)ACPI_PM_MASK, > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > + .enable = acpi_pm_enable, > + .disable = acpi_pm_disable, > }; > Thanks. I'll try do this in that way. But I need to disable/enable ACPI PM timer only on suspend/resume, so I'll use suspend/resume callbacks. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-31 14:44 ` Marek Maślanka @ 2024-07-31 16:33 ` Thomas Gleixner 2024-07-31 21:41 ` Marek Maślanka 0 siblings, 1 reply; 31+ messages in thread From: Thomas Gleixner @ 2024-07-31 16:33 UTC (permalink / raw) To: Marek Maślanka Cc: LKML, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, John Stultz, Stephen Boyd, platform-driver-x86 Marek! On Wed, Jul 31 2024 at 16:44, Marek Maślanka wrote: > On Tue, Jul 30, 2024 at 6:08 PM Thomas Gleixner <tglx@linutronix.de> wrote: >> On Tue, Jul 30 2024 at 12:05, Marek Maslanka wrote: >> +static void acpi_pm_disable(struct clocksource *cs) >> +{ >> + acpi_pm_enabled = false; >> + if (enable_callback) >> + enable_callback(false); >> +} >> + >> static struct clocksource clocksource_acpi_pm = { >> .name = "acpi_pm", >> .rating = 200, >> .read = acpi_pm_read, >> .mask = (u64)ACPI_PM_MASK, >> .flags = CLOCK_SOURCE_IS_CONTINUOUS, >> + .enable = acpi_pm_enable, >> + .disable = acpi_pm_disable, >> }; >> > Thanks. I'll try do this in that way. But I need to disable/enable > ACPI PM timer only on suspend/resume, so I'll use suspend/resume > callbacks. Why? What's the point of keeping it running when nothing uses it? Thanks, tglx ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-31 16:33 ` Thomas Gleixner @ 2024-07-31 21:41 ` Marek Maślanka 2024-07-31 21:46 ` Thomas Gleixner 2024-08-06 7:24 ` Ilpo Järvinen 0 siblings, 2 replies; 31+ messages in thread From: Marek Maślanka @ 2024-07-31 21:41 UTC (permalink / raw) To: Thomas Gleixner Cc: LKML, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, John Stultz, Stephen Boyd, platform-driver-x86 On Wed, Jul 31, 2024 at 6:33 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > Marek! > > On Wed, Jul 31 2024 at 16:44, Marek Maślanka wrote: > > On Tue, Jul 30, 2024 at 6:08 PM Thomas Gleixner <tglx@linutronix.de> wrote: > >> On Tue, Jul 30 2024 at 12:05, Marek Maslanka wrote: > >> +static void acpi_pm_disable(struct clocksource *cs) > >> +{ > >> + acpi_pm_enabled = false; > >> + if (enable_callback) > >> + enable_callback(false); > >> +} > >> + > >> static struct clocksource clocksource_acpi_pm = { > >> .name = "acpi_pm", > >> .rating = 200, > >> .read = acpi_pm_read, > >> .mask = (u64)ACPI_PM_MASK, > >> .flags = CLOCK_SOURCE_IS_CONTINUOUS, > >> + .enable = acpi_pm_enable, > >> + .disable = acpi_pm_disable, > >> }; > >> > > Thanks. I'll try do this in that way. But I need to disable/enable > > ACPI PM timer only on suspend/resume, so I'll use suspend/resume > > callbacks. > > Why? What's the point of keeping it running when nothing uses it? > > Thanks, > > tglx In case of Intel CPUs the watchdog (iTCO/wdat_wdt) is driven by ACPI PM Timer. But it may also be used by others that I don't know about, so I don't want to disable it. Best Marek ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-31 21:41 ` Marek Maślanka @ 2024-07-31 21:46 ` Thomas Gleixner 2024-08-06 7:24 ` Ilpo Järvinen 1 sibling, 0 replies; 31+ messages in thread From: Thomas Gleixner @ 2024-07-31 21:46 UTC (permalink / raw) To: Marek Maślanka Cc: LKML, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, John Stultz, Stephen Boyd, platform-driver-x86 On Wed, Jul 31 2024 at 23:41, Marek Maślanka wrote: > On Wed, Jul 31, 2024 at 6:33 PM Thomas Gleixner <tglx@linutronix.de> wrote: >> On Wed, Jul 31 2024 at 16:44, Marek Maślanka wrote: >> > Thanks. I'll try do this in that way. But I need to disable/enable >> > ACPI PM timer only on suspend/resume, so I'll use suspend/resume >> > callbacks. >> >> Why? What's the point of keeping it running when nothing uses it? >> > In case of Intel CPUs the watchdog (iTCO/wdat_wdt) is driven by ACPI PM > Timer. But it may also be used by others that I don't know about, so I don't > want to disable it. Fair enough. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-07-31 21:41 ` Marek Maślanka 2024-07-31 21:46 ` Thomas Gleixner @ 2024-08-06 7:24 ` Ilpo Järvinen 2024-08-09 13:13 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Marek Maslanka 1 sibling, 1 reply; 31+ messages in thread From: Ilpo Järvinen @ 2024-08-06 7:24 UTC (permalink / raw) To: Marek Maślanka Cc: Thomas Gleixner, LKML, Rajneesh Bhardwaj, David E Box, Hans de Goede, John Stultz, Stephen Boyd, platform-driver-x86 [-- Attachment #1: Type: text/plain, Size: 1667 bytes --] On Wed, 31 Jul 2024, Marek Maślanka wrote: > On Wed, Jul 31, 2024 at 6:33 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > > > Marek! > > > > On Wed, Jul 31 2024 at 16:44, Marek Maślanka wrote: > > > On Tue, Jul 30, 2024 at 6:08 PM Thomas Gleixner <tglx@linutronix.de> wrote: > > >> On Tue, Jul 30 2024 at 12:05, Marek Maslanka wrote: > > >> +static void acpi_pm_disable(struct clocksource *cs) > > >> +{ > > >> + acpi_pm_enabled = false; > > >> + if (enable_callback) > > >> + enable_callback(false); > > >> +} > > >> + > > >> static struct clocksource clocksource_acpi_pm = { > > >> .name = "acpi_pm", > > >> .rating = 200, > > >> .read = acpi_pm_read, > > >> .mask = (u64)ACPI_PM_MASK, > > >> .flags = CLOCK_SOURCE_IS_CONTINUOUS, > > >> + .enable = acpi_pm_enable, > > >> + .disable = acpi_pm_disable, > > >> }; > > >> > > > Thanks. I'll try do this in that way. But I need to disable/enable > > > ACPI PM timer only on suspend/resume, so I'll use suspend/resume > > > callbacks. > > > > Why? What's the point of keeping it running when nothing uses it? > > > > Thanks, > > > > tglx > > In case of Intel CPUs the watchdog (iTCO/wdat_wdt) is driven by ACPI PM > Timer. But it may also be used by others that I don't know about, so I don't > want to disable it. Hi Marek, This kind of non-obvious information should be put into the changelog because it helps if after ten years somebody is looking into this change and asks similar why questions. -- i. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-06 7:24 ` Ilpo Järvinen @ 2024-08-09 13:13 ` Marek Maslanka 2024-08-09 13:13 ` [PATCH v4 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended Marek Maslanka 2024-08-09 19:15 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Thomas Gleixner 0 siblings, 2 replies; 31+ messages in thread From: Marek Maslanka @ 2024-08-09 13:13 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 Provides the capability to register an external callback for the ACPI PM timer, which is called during the suspend and resume processes. Signed-off-by: Marek Maslanka <mmaslanka@google.com> --- Changes in v4: - No changes as this was introduced as a separated patch after the v3 review. - Link to v3: https://lore.kernel.org/lkml/20240730120546.1042515-1-mmaslanka@google.com/ --- --- drivers/clocksource/acpi_pm.c | 27 +++++++++++++++++++++++++++ drivers/clocksource/acpi_pm.h | 16 ++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 drivers/clocksource/acpi_pm.h diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c index 82338773602ca..c629f5462bc0f 100644 --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -25,6 +25,12 @@ #include <asm/io.h> #include <asm/time.h> +#include "acpi_pm.h" + +static void *suspend_resume_cb_data; + +static void (*suspend_resume_callback)(void *data, bool suspend); + /* * The I/O port the PMTMR resides at. * The location is detected during setup_arch(), @@ -58,6 +64,25 @@ u32 acpi_pm_read_verified(void) return v2; } +void acpi_pm_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), + void *data) +{ + suspend_resume_callback = cb; + suspend_resume_cb_data = data; +} + +static void acpi_pm_suspend(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, true); +} + +static void acpi_pm_resume(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, false); +} + static u64 acpi_pm_read(struct clocksource *cs) { return (u64)read_pmtmr(); @@ -69,6 +94,8 @@ static struct clocksource clocksource_acpi_pm = { .read = acpi_pm_read, .mask = (u64)ACPI_PM_MASK, .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .suspend = acpi_pm_suspend, + .resume = acpi_pm_resume, }; diff --git a/drivers/clocksource/acpi_pm.h b/drivers/clocksource/acpi_pm.h new file mode 100644 index 0000000000000..c932899f04282 --- /dev/null +++ b/drivers/clocksource/acpi_pm.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ACPI_PM_H__ +#define __ACPI_PM_H__ + +#include <linux/types.h> + +/** + * Register callback for suspend and resume event + * + * @cb Callback triggered on suspend and resume + * @data Data passed with the callback + */ +void acpi_pm_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), + void *data); + +#endif -- 2.46.0.76.ge559c4bf1a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v4 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-09 13:13 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Marek Maslanka @ 2024-08-09 13:13 ` Marek Maslanka 2024-08-09 16:36 ` Thomas Gleixner 2024-08-09 19:15 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Thomas Gleixner 1 sibling, 1 reply; 31+ messages in thread From: Marek Maslanka @ 2024-08-09 13:13 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 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@google.com> --- Changes in v4: - Enable/disable ACPI PM Timer with the clocksource resume/suspend process instead of during pmc_core resume/suspend. Use added callback in acpi_pm to do this. - Do not rely on current set clocksource to check if ACPI PM Timer can be disabled. - Link to v3: https://lore.kernel.org/lkml/20240730120546.1042515-1-mmaslanka@google.com/ --- --- drivers/platform/x86/intel/pmc/adl.c | 2 ++ drivers/platform/x86/intel/pmc/cnp.c | 2 ++ drivers/platform/x86/intel/pmc/core.c | 49 +++++++++++++++++++++++++++ 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, 69 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..f6a29530347bb 100644 --- a/drivers/platform/x86/intel/pmc/core.c +++ b/drivers/platform/x86/intel/pmc/core.c @@ -29,6 +29,7 @@ #include "core.h" #include "../pmt/telemetry.h" +#include "../../../../clocksource/acpi_pm.h" /* Maximum number of modes supported by platfoms that has low power mode capability */ const char *pmc_lpm_modes[] = { @@ -1171,6 +1172,42 @@ 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; + + mutex_lock(&pmcdev->lock); + + if (!suspend && !pmcdev->enable_acpi_pm_timer_on_resume) { + mutex_unlock(&pmcdev->lock); + 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; + + mutex_unlock(&pmcdev->lock); +} static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) { @@ -1362,6 +1399,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 +1458,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_pm_register_suspend_resume_callback(pmc_core_acpi_pm_timer_suspend_resume, + pmcdev); + device_initialized = true; dev_info(&pdev->dev, " initialized\n"); @@ -1429,6 +1472,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_pm_register_suspend_resume_callback(NULL, NULL); + 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, -- 2.46.0.76.ge559c4bf1a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v4 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-09 13:13 ` [PATCH v4 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended Marek Maslanka @ 2024-08-09 16:36 ` Thomas Gleixner 2024-08-12 4:40 ` [PATCH v5 " Marek Maslanka 0 siblings, 1 reply; 31+ messages in thread From: Thomas Gleixner @ 2024-08-09 16:36 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Marek Maslanka, Daniel Lezcano, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 On Fri, Aug 09 2024 at 13:13, Marek Maslanka wrote: > +#include "../../../../clocksource/acpi_pm.h" No. include/linux/acpi_pmtmr.h is the right place for this. ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-09 16:36 ` Thomas Gleixner @ 2024-08-12 4:40 ` Marek Maslanka 2024-08-12 7:49 ` Ilpo Järvinen 0 siblings, 1 reply; 31+ messages in thread From: Marek Maslanka @ 2024-08-12 4:40 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 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@google.com> --- Changes in v5: - Use renamed acpi_pmtmr_register_suspend_resume_callback instead of acpi_pm_register_suspend_resume_callback - Link to v4: https://lore.kernel.org/lkml/20240809131343.1173369-2-mmaslanka@google.com/ --- --- drivers/platform/x86/intel/pmc/adl.c | 2 ++ drivers/platform/x86/intel/pmc/cnp.c | 2 ++ drivers/platform/x86/intel/pmc/core.c | 49 +++++++++++++++++++++++++++ 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, 69 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..d65e3e77ec2ca 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,42 @@ 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; + + mutex_lock(&pmcdev->lock); + + if (!suspend && !pmcdev->enable_acpi_pm_timer_on_resume) { + mutex_unlock(&pmcdev->lock); + 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; + + mutex_unlock(&pmcdev->lock); +} static void pmc_core_dbgfs_unregister(struct pmc_dev *pmcdev) { @@ -1362,6 +1399,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 +1458,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 +1472,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_register_suspend_resume_callback(NULL, NULL); + 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, -- 2.46.0.76.ge559c4bf1a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v5 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-12 4:40 ` [PATCH v5 " Marek Maslanka @ 2024-08-12 7:49 ` Ilpo Järvinen 2024-08-12 18:42 ` [PATCH v6 " Marek Maslanka 0 siblings, 1 reply; 31+ messages in thread From: Ilpo Järvinen @ 2024-08-12 7:49 UTC (permalink / raw) To: Marek Maslanka Cc: LKML, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, platform-driver-x86 On Mon, 12 Aug 2024, 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@google.com> > > --- > Changes in v5: > - Use renamed acpi_pmtmr_register_suspend_resume_callback instead of > acpi_pm_register_suspend_resume_callback > - Link to v4: https://lore.kernel.org/lkml/20240809131343.1173369-2-mmaslanka@google.com/ > --- > --- > drivers/platform/x86/intel/pmc/adl.c | 2 ++ > drivers/platform/x86/intel/pmc/cnp.c | 2 ++ > drivers/platform/x86/intel/pmc/core.c | 49 +++++++++++++++++++++++++++ > 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, 69 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..d65e3e77ec2ca 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,42 @@ 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; > + > + mutex_lock(&pmcdev->lock); > + > + if (!suspend && !pmcdev->enable_acpi_pm_timer_on_resume) { > + mutex_unlock(&pmcdev->lock); Use guard() in this function so you don't need to manually unlock. -- i. > + 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; > + > + mutex_unlock(&pmcdev->lock); > +} ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-12 7:49 ` Ilpo Järvinen @ 2024-08-12 18:42 ` Marek Maslanka 2024-08-19 11:31 ` Hans de Goede 2024-09-06 18:56 ` [tip: timers/core] " tip-bot2 for Marek Maslanka 0 siblings, 2 replies; 31+ messages in thread From: Marek Maslanka @ 2024-08-12 18:42 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 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@google.com> --- 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@google.com/ --- --- 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, -- 2.46.0.76.ge559c4bf1a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-12 18:42 ` [PATCH v6 " Marek Maslanka @ 2024-08-19 11:31 ` Hans de Goede 2024-09-06 18:56 ` [tip: timers/core] " tip-bot2 for Marek Maslanka 1 sibling, 0 replies; 31+ messages in thread From: Hans de Goede @ 2024-08-19 11:31 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Ilpo Järvinen, platform-driver-x86 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@google.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> 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@google.com/ > --- > --- > 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, ^ permalink raw reply [flat|nested] 31+ messages in thread
* [tip: timers/core] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 2024-08-12 18:42 ` [PATCH v6 " Marek Maslanka 2024-08-19 11:31 ` Hans de Goede @ 2024-09-06 18:56 ` tip-bot2 for Marek Maslanka 1 sibling, 0 replies; 31+ messages in thread From: tip-bot2 for Marek Maslanka @ 2024-09-06 18:56 UTC (permalink / raw) To: linux-tip-commits Cc: Marek Maslanka, Hans de Goede, Daniel Lezcano, x86, linux-kernel The following commit has been merged into the timers/core branch of tip: Commit-ID: e86c8186d03a6ba018e881ed45f0962ad553e861 Gitweb: https://git.kernel.org/tip/e86c8186d03a6ba018e881ed45f0962ad553e861 Author: Marek Maslanka <mmaslanka@google.com> AuthorDate: Mon, 12 Aug 2024 18:42:00 Committer: Daniel Lezcano <daniel.lezcano@linaro.org> CommitterDate: Fri, 06 Sep 2024 14:49:20 +02:00 platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended 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@google.com> Reviewed-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20240812184208.1080710-1-mmaslanka@google.com Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- 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 e787855..9d9c07f 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 dd72974..513c026 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 01ae71c..c91c753 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> @@ -1208,6 +1209,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) { @@ -1404,6 +1437,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; @@ -1462,6 +1496,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"); @@ -1471,6 +1510,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 ea04de7..4d37ef7 100644 --- a/drivers/platform/x86/intel/pmc/core.h +++ b/drivers/platform/x86/intel/pmc/core.h @@ -68,6 +68,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, @@ -148,6 +150,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 @@ -351,6 +355,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; }; /** @@ -424,6 +430,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 71b0fd6..cbbd440 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 c7d15d8..91f2fa7 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 ab993a6..2cd2b3c 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 e0580de..371b4e3 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, ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-09 13:13 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Marek Maslanka 2024-08-09 13:13 ` [PATCH v4 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended Marek Maslanka @ 2024-08-09 19:15 ` Thomas Gleixner 2024-08-12 4:37 ` [PATCH v5 " Marek Maslanka 1 sibling, 1 reply; 31+ messages in thread From: Thomas Gleixner @ 2024-08-09 19:15 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Marek Maslanka, Daniel Lezcano, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 On Fri, Aug 09 2024 at 13:13, Marek Maslanka wrote: > --- a/drivers/clocksource/acpi_pm.c > +++ b/drivers/clocksource/acpi_pm.c > @@ -25,6 +25,12 @@ > #include <asm/io.h> > #include <asm/time.h> > > +#include "acpi_pm.h" include/linux/acpi_pmtmr.h please > +static void *suspend_resume_cb_data; > + > +static void (*suspend_resume_callback)(void *data, bool suspend); > + > /* > * The I/O port the PMTMR resides at. > * The location is detected during setup_arch(), > @@ -58,6 +64,25 @@ u32 acpi_pm_read_verified(void) > return v2; > } > > +void acpi_pm_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), > + void *data) No line break required. Also the name wants to be acpi_pmtmr_... for the global visible function so that it can't be confused with the power management related acpi_pm_* functions Thanks, tglx ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v5 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-09 19:15 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Thomas Gleixner @ 2024-08-12 4:37 ` Marek Maslanka 2024-08-12 8:03 ` Hans de Goede 0 siblings, 1 reply; 31+ messages in thread From: Marek Maslanka @ 2024-08-12 4:37 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 Provides the capability to register an external callback for the ACPI PM timer, which is called during the suspend and resume processes. Signed-off-by: Marek Maslanka <mmaslanka@google.com> --- Changes in v5: - Rename acpi_pm_register_suspend_resume_callback to acpi_pmtmr_register_suspend_resume_callback and move prototype to include/linux/acpi_pmtmr.h - Remove the acpi_pm.h header added in the previous patch. - Link to v4: https://lore.kernel.org/lkml/20240809131343.1173369-1-mmaslanka@google.com/ --- --- drivers/clocksource/acpi_pm.c | 24 ++++++++++++++++++++++++ include/linux/acpi_pmtmr.h | 8 ++++++++ 2 files changed, 32 insertions(+) diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c index 82338773602ca..fab19b7de55c1 100644 --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -25,6 +25,10 @@ #include <asm/io.h> #include <asm/time.h> +static void *suspend_resume_cb_data; + +static void (*suspend_resume_callback)(void *data, bool suspend); + /* * The I/O port the PMTMR resides at. * The location is detected during setup_arch(), @@ -58,6 +62,24 @@ u32 acpi_pm_read_verified(void) return v2; } +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data) +{ + suspend_resume_callback = cb; + suspend_resume_cb_data = data; +} + +static void acpi_pm_suspend(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, true); +} + +static void acpi_pm_resume(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, false); +} + static u64 acpi_pm_read(struct clocksource *cs) { return (u64)read_pmtmr(); @@ -69,6 +91,8 @@ static struct clocksource clocksource_acpi_pm = { .read = acpi_pm_read, .mask = (u64)ACPI_PM_MASK, .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .suspend = acpi_pm_suspend, + .resume = acpi_pm_resume, }; diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h index 50d88bf1498d7..a5262d28b97e0 100644 --- a/include/linux/acpi_pmtmr.h +++ b/include/linux/acpi_pmtmr.h @@ -26,6 +26,14 @@ static inline u32 acpi_pm_read_early(void) return acpi_pm_read_verified() & ACPI_PM_MASK; } +/** + * Register callback for suspend and resume event + * + * @cb Callback triggered on suspend and resume + * @data Data passed with the callback + */ +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data); + #else static inline u32 acpi_pm_read_early(void) -- 2.46.0.76.ge559c4bf1a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v5 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-12 4:37 ` [PATCH v5 " Marek Maslanka @ 2024-08-12 8:03 ` Hans de Goede 2024-08-12 18:41 ` [PATCH v6 " Marek Maslanka 0 siblings, 1 reply; 31+ messages in thread From: Hans de Goede @ 2024-08-12 8:03 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Ilpo Järvinen, platform-driver-x86 Hi, Thank you for your patch. On 8/12/24 6:37 AM, Marek Maslanka wrote: > Provides the capability to register an external callback for the ACPI PM > timer, which is called during the suspend and resume processes. > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> > > --- > Changes in v5: > - Rename acpi_pm_register_suspend_resume_callback to > acpi_pmtmr_register_suspend_resume_callback and move prototype to > include/linux/acpi_pmtmr.h > - Remove the acpi_pm.h header added in the previous patch. > - Link to v4: https://lore.kernel.org/lkml/20240809131343.1173369-1-mmaslanka@google.com/ > --- > --- > drivers/clocksource/acpi_pm.c | 24 ++++++++++++++++++++++++ > include/linux/acpi_pmtmr.h | 8 ++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c > index 82338773602ca..fab19b7de55c1 100644 > --- a/drivers/clocksource/acpi_pm.c > +++ b/drivers/clocksource/acpi_pm.c > @@ -25,6 +25,10 @@ > #include <asm/io.h> > #include <asm/time.h> > > +static void *suspend_resume_cb_data; > + > +static void (*suspend_resume_callback)(void *data, bool suspend); > + > /* > * The I/O port the PMTMR resides at. > * The location is detected during setup_arch(), > @@ -58,6 +62,24 @@ u32 acpi_pm_read_verified(void) > return v2; > } > > +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data) > +{ > + suspend_resume_callback = cb; > + suspend_resume_cb_data = data; > +} The intel-pmc driver which is a consumer of this symbol can be build as a module, so this needs a EXPORT_SYMBOL_GPL(). Also the pmc driver can be unbound from its device, or the entire module can be removed, so this also needs an unregister function to match, so that the pmc driver can unregister its callback from pmc_core_remove(). Regards, Hans > + > +static void acpi_pm_suspend(struct clocksource *cs) > +{ > + if (suspend_resume_callback) > + suspend_resume_callback(suspend_resume_cb_data, true); > +} > + > +static void acpi_pm_resume(struct clocksource *cs) > +{ > + if (suspend_resume_callback) > + suspend_resume_callback(suspend_resume_cb_data, false); > +} > + > static u64 acpi_pm_read(struct clocksource *cs) > { > return (u64)read_pmtmr(); > @@ -69,6 +91,8 @@ static struct clocksource clocksource_acpi_pm = { > .read = acpi_pm_read, > .mask = (u64)ACPI_PM_MASK, > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > + .suspend = acpi_pm_suspend, > + .resume = acpi_pm_resume, > }; > > > diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h > index 50d88bf1498d7..a5262d28b97e0 100644 > --- a/include/linux/acpi_pmtmr.h > +++ b/include/linux/acpi_pmtmr.h > @@ -26,6 +26,14 @@ static inline u32 acpi_pm_read_early(void) > return acpi_pm_read_verified() & ACPI_PM_MASK; > } > > +/** > + * Register callback for suspend and resume event > + * > + * @cb Callback triggered on suspend and resume > + * @data Data passed with the callback > + */ > +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data); > + > #else > > static inline u32 acpi_pm_read_early(void) ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-12 8:03 ` Hans de Goede @ 2024-08-12 18:41 ` Marek Maslanka 2024-08-19 11:31 ` Hans de Goede ` (2 more replies) 0 siblings, 3 replies; 31+ messages in thread From: Marek Maslanka @ 2024-08-12 18:41 UTC (permalink / raw) To: LKML Cc: Marek Maslanka, Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Hans de Goede, Ilpo Järvinen, platform-driver-x86 Provides the capability to register an external callback for the ACPI PM timer, which is called during the suspend and resume processes. Signed-off-by: Marek Maslanka <mmaslanka@google.com> --- Changes in v6: - Add the acpi_pmtmr_unregister_suspend_resume_callback function to remove callback - Add EXPORT_SYMBOL_GPL to the added functions - Link to v5: https://lore.kernel.org/lkml/20240812043741.3434744-1-mmaslanka@google.com/ --- --- drivers/clocksource/acpi_pm.c | 32 ++++++++++++++++++++++++++++++++ include/linux/acpi_pmtmr.h | 13 +++++++++++++ 2 files changed, 45 insertions(+) diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c index 82338773602ca..b4330a01a566b 100644 --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -25,6 +25,10 @@ #include <asm/io.h> #include <asm/time.h> +static void *suspend_resume_cb_data; + +static void (*suspend_resume_callback)(void *data, bool suspend); + /* * The I/O port the PMTMR resides at. * The location is detected during setup_arch(), @@ -58,6 +62,32 @@ u32 acpi_pm_read_verified(void) return v2; } +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data) +{ + suspend_resume_callback = cb; + suspend_resume_cb_data = data; +} +EXPORT_SYMBOL_GPL(acpi_pmtmr_register_suspend_resume_callback); + +void acpi_pmtmr_unregister_suspend_resume_callback(void) +{ + suspend_resume_callback = NULL; + suspend_resume_cb_data = NULL; +} +EXPORT_SYMBOL_GPL(acpi_pmtmr_unregister_suspend_resume_callback); + +static void acpi_pm_suspend(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, true); +} + +static void acpi_pm_resume(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, false); +} + static u64 acpi_pm_read(struct clocksource *cs) { return (u64)read_pmtmr(); @@ -69,6 +99,8 @@ static struct clocksource clocksource_acpi_pm = { .read = acpi_pm_read, .mask = (u64)ACPI_PM_MASK, .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .suspend = acpi_pm_suspend, + .resume = acpi_pm_resume, }; diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h index 50d88bf1498d7..0ded9220d379c 100644 --- a/include/linux/acpi_pmtmr.h +++ b/include/linux/acpi_pmtmr.h @@ -26,6 +26,19 @@ static inline u32 acpi_pm_read_early(void) return acpi_pm_read_verified() & ACPI_PM_MASK; } +/** + * Register callback for suspend and resume event + * + * @cb Callback triggered on suspend and resume + * @data Data passed with the callback + */ +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data); + +/** + * Remove registered callback for suspend and resume event + */ +void acpi_pmtmr_unregister_suspend_resume_callback(void); + #else static inline u32 acpi_pm_read_early(void) -- 2.46.0.76.ge559c4bf1a-goog ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-12 18:41 ` [PATCH v6 " Marek Maslanka @ 2024-08-19 11:31 ` Hans de Goede 2024-08-19 11:35 ` Hans de Goede 2024-09-06 18:56 ` [tip: timers/core] " tip-bot2 for Marek Maslanka 2 siblings, 0 replies; 31+ messages in thread From: Hans de Goede @ 2024-08-19 11:31 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Ilpo Järvinen, platform-driver-x86 Hi, On 8/12/24 8:41 PM, Marek Maslanka wrote: > Provides the capability to register an external callback for the ACPI PM > timer, which is called during the suspend and resume processes. > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > > --- > Changes in v6: > - Add the acpi_pmtmr_unregister_suspend_resume_callback function to remove callback > - Add EXPORT_SYMBOL_GPL to the added functions > - Link to v5: https://lore.kernel.org/lkml/20240812043741.3434744-1-mmaslanka@google.com/ > --- > --- > drivers/clocksource/acpi_pm.c | 32 ++++++++++++++++++++++++++++++++ > include/linux/acpi_pmtmr.h | 13 +++++++++++++ > 2 files changed, 45 insertions(+) > > diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c > index 82338773602ca..b4330a01a566b 100644 > --- a/drivers/clocksource/acpi_pm.c > +++ b/drivers/clocksource/acpi_pm.c > @@ -25,6 +25,10 @@ > #include <asm/io.h> > #include <asm/time.h> > > +static void *suspend_resume_cb_data; > + > +static void (*suspend_resume_callback)(void *data, bool suspend); > + > /* > * The I/O port the PMTMR resides at. > * The location is detected during setup_arch(), > @@ -58,6 +62,32 @@ u32 acpi_pm_read_verified(void) > return v2; > } > > +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data) > +{ > + suspend_resume_callback = cb; > + suspend_resume_cb_data = data; > +} > +EXPORT_SYMBOL_GPL(acpi_pmtmr_register_suspend_resume_callback); > + > +void acpi_pmtmr_unregister_suspend_resume_callback(void) > +{ > + suspend_resume_callback = NULL; > + suspend_resume_cb_data = NULL; > +} > +EXPORT_SYMBOL_GPL(acpi_pmtmr_unregister_suspend_resume_callback); > + > +static void acpi_pm_suspend(struct clocksource *cs) > +{ > + if (suspend_resume_callback) > + suspend_resume_callback(suspend_resume_cb_data, true); > +} > + > +static void acpi_pm_resume(struct clocksource *cs) > +{ > + if (suspend_resume_callback) > + suspend_resume_callback(suspend_resume_cb_data, false); > +} > + > static u64 acpi_pm_read(struct clocksource *cs) > { > return (u64)read_pmtmr(); > @@ -69,6 +99,8 @@ static struct clocksource clocksource_acpi_pm = { > .read = acpi_pm_read, > .mask = (u64)ACPI_PM_MASK, > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > + .suspend = acpi_pm_suspend, > + .resume = acpi_pm_resume, > }; > > > diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h > index 50d88bf1498d7..0ded9220d379c 100644 > --- a/include/linux/acpi_pmtmr.h > +++ b/include/linux/acpi_pmtmr.h > @@ -26,6 +26,19 @@ static inline u32 acpi_pm_read_early(void) > return acpi_pm_read_verified() & ACPI_PM_MASK; > } > > +/** > + * Register callback for suspend and resume event > + * > + * @cb Callback triggered on suspend and resume > + * @data Data passed with the callback > + */ > +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data); > + > +/** > + * Remove registered callback for suspend and resume event > + */ > +void acpi_pmtmr_unregister_suspend_resume_callback(void); > + > #else > > static inline u32 acpi_pm_read_early(void) ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-12 18:41 ` [PATCH v6 " Marek Maslanka 2024-08-19 11:31 ` Hans de Goede @ 2024-08-19 11:35 ` Hans de Goede 2024-08-19 18:31 ` Daniel Lezcano 2024-09-06 18:56 ` [tip: timers/core] " tip-bot2 for Marek Maslanka 2 siblings, 1 reply; 31+ messages in thread From: Hans de Goede @ 2024-08-19 11:35 UTC (permalink / raw) To: Marek Maslanka, LKML Cc: Daniel Lezcano, Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Ilpo Järvinen, platform-driver-x86 Hi, On 8/12/24 8:41 PM, Marek Maslanka wrote: > Provides the capability to register an external callback for the ACPI PM > timer, which is called during the suspend and resume processes. > > Signed-off-by: Marek Maslanka <mmaslanka@google.com> Daniel / Thomas I believe this series is ready for merging now, do you want to merge this through the tree for drivers/clocksource ? Or shall I merge this through the platform-drivers-x86 tree ? In case of the latter may I please have your Acked-by for patch 1/2 for ths ? Regards, Hans > > --- > Changes in v6: > - Add the acpi_pmtmr_unregister_suspend_resume_callback function to remove callback > - Add EXPORT_SYMBOL_GPL to the added functions > - Link to v5: https://lore.kernel.org/lkml/20240812043741.3434744-1-mmaslanka@google.com/ > --- > --- > drivers/clocksource/acpi_pm.c | 32 ++++++++++++++++++++++++++++++++ > include/linux/acpi_pmtmr.h | 13 +++++++++++++ > 2 files changed, 45 insertions(+) > > diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c > index 82338773602ca..b4330a01a566b 100644 > --- a/drivers/clocksource/acpi_pm.c > +++ b/drivers/clocksource/acpi_pm.c > @@ -25,6 +25,10 @@ > #include <asm/io.h> > #include <asm/time.h> > > +static void *suspend_resume_cb_data; > + > +static void (*suspend_resume_callback)(void *data, bool suspend); > + > /* > * The I/O port the PMTMR resides at. > * The location is detected during setup_arch(), > @@ -58,6 +62,32 @@ u32 acpi_pm_read_verified(void) > return v2; > } > > +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data) > +{ > + suspend_resume_callback = cb; > + suspend_resume_cb_data = data; > +} > +EXPORT_SYMBOL_GPL(acpi_pmtmr_register_suspend_resume_callback); > + > +void acpi_pmtmr_unregister_suspend_resume_callback(void) > +{ > + suspend_resume_callback = NULL; > + suspend_resume_cb_data = NULL; > +} > +EXPORT_SYMBOL_GPL(acpi_pmtmr_unregister_suspend_resume_callback); > + > +static void acpi_pm_suspend(struct clocksource *cs) > +{ > + if (suspend_resume_callback) > + suspend_resume_callback(suspend_resume_cb_data, true); > +} > + > +static void acpi_pm_resume(struct clocksource *cs) > +{ > + if (suspend_resume_callback) > + suspend_resume_callback(suspend_resume_cb_data, false); > +} > + > static u64 acpi_pm_read(struct clocksource *cs) > { > return (u64)read_pmtmr(); > @@ -69,6 +99,8 @@ static struct clocksource clocksource_acpi_pm = { > .read = acpi_pm_read, > .mask = (u64)ACPI_PM_MASK, > .flags = CLOCK_SOURCE_IS_CONTINUOUS, > + .suspend = acpi_pm_suspend, > + .resume = acpi_pm_resume, > }; > > > diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h > index 50d88bf1498d7..0ded9220d379c 100644 > --- a/include/linux/acpi_pmtmr.h > +++ b/include/linux/acpi_pmtmr.h > @@ -26,6 +26,19 @@ static inline u32 acpi_pm_read_early(void) > return acpi_pm_read_verified() & ACPI_PM_MASK; > } > > +/** > + * Register callback for suspend and resume event > + * > + * @cb Callback triggered on suspend and resume > + * @data Data passed with the callback > + */ > +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data); > + > +/** > + * Remove registered callback for suspend and resume event > + */ > +void acpi_pmtmr_unregister_suspend_resume_callback(void); > + > #else > > static inline u32 acpi_pm_read_early(void) ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/2] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-19 11:35 ` Hans de Goede @ 2024-08-19 18:31 ` Daniel Lezcano 0 siblings, 0 replies; 31+ messages in thread From: Daniel Lezcano @ 2024-08-19 18:31 UTC (permalink / raw) To: Hans de Goede, Marek Maslanka, LKML Cc: Thomas Gleixner, Rajneesh Bhardwaj, David E Box, Ilpo Järvinen, platform-driver-x86 On 19/08/2024 13:35, Hans de Goede wrote: > Hi, > > On 8/12/24 8:41 PM, Marek Maslanka wrote: >> Provides the capability to register an external callback for the ACPI PM >> timer, which is called during the suspend and resume processes. >> >> Signed-off-by: Marek Maslanka <mmaslanka@google.com> > > Daniel / Thomas I believe this series is ready for merging now, > do you want to merge this through the tree for drivers/clocksource ? > > Or shall I merge this through the platform-drivers-x86 tree ? > > In case of the latter may I please have your Acked-by for patch 1/2 for ths ? I'll pick them, thanks -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 31+ messages in thread
* [tip: timers/core] clocksource: acpi_pm: Add external callback for suspend/resume 2024-08-12 18:41 ` [PATCH v6 " Marek Maslanka 2024-08-19 11:31 ` Hans de Goede 2024-08-19 11:35 ` Hans de Goede @ 2024-09-06 18:56 ` tip-bot2 for Marek Maslanka 2 siblings, 0 replies; 31+ messages in thread From: tip-bot2 for Marek Maslanka @ 2024-09-06 18:56 UTC (permalink / raw) To: linux-tip-commits Cc: Marek Maslanka, Hans de Goede, Daniel Lezcano, x86, linux-kernel The following commit has been merged into the timers/core branch of tip: Commit-ID: 56bd72e9cd8272687aa2a8eccddabc5526cd7845 Gitweb: https://git.kernel.org/tip/56bd72e9cd8272687aa2a8eccddabc5526cd7845 Author: Marek Maslanka <mmaslanka@google.com> AuthorDate: Mon, 12 Aug 2024 18:41:42 Committer: Daniel Lezcano <daniel.lezcano@linaro.org> CommitterDate: Fri, 06 Sep 2024 14:49:20 +02:00 clocksource: acpi_pm: Add external callback for suspend/resume Provides the capability to register an external callback for the ACPI PM timer, which is called during the suspend and resume processes. Signed-off-by: Marek Maslanka <mmaslanka@google.com> Reviewed-by: Hans de Goede <hdegoede@redhat.com> Link: https://lore.kernel.org/r/20240812184150.1079924-1-mmaslanka@google.com Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> --- drivers/clocksource/acpi_pm.c | 32 ++++++++++++++++++++++++++++++++ include/linux/acpi_pmtmr.h | 13 +++++++++++++ 2 files changed, 45 insertions(+) diff --git a/drivers/clocksource/acpi_pm.c b/drivers/clocksource/acpi_pm.c index 8233877..b4330a0 100644 --- a/drivers/clocksource/acpi_pm.c +++ b/drivers/clocksource/acpi_pm.c @@ -25,6 +25,10 @@ #include <asm/io.h> #include <asm/time.h> +static void *suspend_resume_cb_data; + +static void (*suspend_resume_callback)(void *data, bool suspend); + /* * The I/O port the PMTMR resides at. * The location is detected during setup_arch(), @@ -58,6 +62,32 @@ u32 acpi_pm_read_verified(void) return v2; } +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data) +{ + suspend_resume_callback = cb; + suspend_resume_cb_data = data; +} +EXPORT_SYMBOL_GPL(acpi_pmtmr_register_suspend_resume_callback); + +void acpi_pmtmr_unregister_suspend_resume_callback(void) +{ + suspend_resume_callback = NULL; + suspend_resume_cb_data = NULL; +} +EXPORT_SYMBOL_GPL(acpi_pmtmr_unregister_suspend_resume_callback); + +static void acpi_pm_suspend(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, true); +} + +static void acpi_pm_resume(struct clocksource *cs) +{ + if (suspend_resume_callback) + suspend_resume_callback(suspend_resume_cb_data, false); +} + static u64 acpi_pm_read(struct clocksource *cs) { return (u64)read_pmtmr(); @@ -69,6 +99,8 @@ static struct clocksource clocksource_acpi_pm = { .read = acpi_pm_read, .mask = (u64)ACPI_PM_MASK, .flags = CLOCK_SOURCE_IS_CONTINUOUS, + .suspend = acpi_pm_suspend, + .resume = acpi_pm_resume, }; diff --git a/include/linux/acpi_pmtmr.h b/include/linux/acpi_pmtmr.h index 50d88bf..0ded922 100644 --- a/include/linux/acpi_pmtmr.h +++ b/include/linux/acpi_pmtmr.h @@ -26,6 +26,19 @@ static inline u32 acpi_pm_read_early(void) return acpi_pm_read_verified() & ACPI_PM_MASK; } +/** + * Register callback for suspend and resume event + * + * @cb Callback triggered on suspend and resume + * @data Data passed with the callback + */ +void acpi_pmtmr_register_suspend_resume_callback(void (*cb)(void *data, bool suspend), void *data); + +/** + * Remove registered callback for suspend and resume event + */ +void acpi_pmtmr_unregister_suspend_resume_callback(void); + #else static inline u32 acpi_pm_read_early(void) ^ permalink raw reply related [flat|nested] 31+ messages in thread
end of thread, other threads:[~2024-09-06 18:56 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-01 22:25 [PATCH] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended Marek Maslanka 2024-07-02 8:02 ` Hans de Goede 2024-07-02 22:41 ` Marek Maślanka 2024-07-03 11:38 ` [PATCH v2] " Marek Maslanka 2024-07-03 16:30 ` Rajneesh Bhardwaj 2024-07-11 15:34 ` David E. Box 2024-07-15 12:39 ` Marek Maślanka 2024-07-30 12:05 ` [PATCH v3] " Marek Maslanka 2024-07-30 12:57 ` Ilpo Järvinen 2024-07-30 16:08 ` Thomas Gleixner 2024-07-31 14:44 ` Marek Maślanka 2024-07-31 16:33 ` Thomas Gleixner 2024-07-31 21:41 ` Marek Maślanka 2024-07-31 21:46 ` Thomas Gleixner 2024-08-06 7:24 ` Ilpo Järvinen 2024-08-09 13:13 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Marek Maslanka 2024-08-09 13:13 ` [PATCH v4 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended Marek Maslanka 2024-08-09 16:36 ` Thomas Gleixner 2024-08-12 4:40 ` [PATCH v5 " Marek Maslanka 2024-08-12 7:49 ` Ilpo Järvinen 2024-08-12 18:42 ` [PATCH v6 " Marek Maslanka 2024-08-19 11:31 ` Hans de Goede 2024-09-06 18:56 ` [tip: timers/core] " tip-bot2 for Marek Maslanka 2024-08-09 19:15 ` [PATCH v4 1/2] clocksource: acpi_pm: Add external callback for suspend/resume Thomas Gleixner 2024-08-12 4:37 ` [PATCH v5 " Marek Maslanka 2024-08-12 8:03 ` Hans de Goede 2024-08-12 18:41 ` [PATCH v6 " Marek Maslanka 2024-08-19 11:31 ` Hans de Goede 2024-08-19 11:35 ` Hans de Goede 2024-08-19 18:31 ` Daniel Lezcano 2024-09-06 18:56 ` [tip: timers/core] " tip-bot2 for Marek Maslanka
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox