[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f5f758f9-bf6f-57dd-179e-475a6b65e903@linux.intel.com>
Date: Mon, 12 Aug 2024 10:49:13 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Marek Maslanka <mmaslanka@...gle.com>
cc: LKML <linux-kernel@...r.kernel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Thomas Gleixner <tglx@...utronix.de>,
Rajneesh Bhardwaj <irenic.rajneesh@...il.com>,
David E Box <david.e.box@...el.com>, Hans de Goede <hdegoede@...hat.com>,
platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v5 2/2] platform/x86:intel/pmc: Enable the ACPI PM Timer
to be turned off when suspended
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@...gle.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);
> +}
Powered by blists - more mailing lists