From: Jani Nikula <jani.nikula@linux.intel.com>
To: Luca Coelho <luciano.coelho@intel.com>, intel-gfx@lists.freedesktop.org
Cc: jani.saarinen@intel.com
Subject: Re: [core-for-CI PATCH] Revert "platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended"
Date: Wed, 09 Oct 2024 16:34:18 +0300 [thread overview]
Message-ID: <87y12x8ko5.fsf@intel.com> (raw)
In-Reply-To: <20241007090739.61293-1-luciano.coelho@intel.com>
On Mon, 07 Oct 2024, Luca Coelho <luciano.coelho@intel.com> wrote:
> This is causing a regression since 6.12-rc1. We are getting the
> following error messages when running flip-vs-suspend* tests:
>
> [222.857770] =============================
> [222.857771] [ BUG: Invalid wait context ]
> [222.857772] 6.12.0-rc1-xe #1 Not tainted
> [222.857773] -----------------------------
> [222.857774] swapper/4/0 is trying to lock:
> [222.857775] ffff8881174c88c8 (&pmcdev->lock){+.+.}-{3:3}, at: pmc_core_acpi_pm_timer_suspend_resume+0x50/0xe0 [intel_pmc_core]
> [222.857782] other info that might help us debug this:
> [222.857783] context-{4:4}
> [222.857784] 1 lock held by swapper/4/0:
> [222.857785] #0: ffffffff83452258 (tick_freeze_lock){....}-{2:2}, at: tick_freeze+0x16/0x110
> [222.857791] stack backtrace:
> [222.857793] CPU: 4 UID: 0 PID: 0 Comm: swapper/4 Not tainted 6.12.0-rc1-xe #1
> [222.857794] Hardware name: Intel Corporation Alder Lake Client Platform/AlderLake-P DDR5 RVP, BIOS RPLPFWI1.R00.4035.A00.2301200723 01/20/2023
> [222.857796] Call Trace:
> [222.857797] <TASK>
> [222.857798] dump_stack_lvl+0x80/0xc0
> [222.857802] dump_stack+0x10/0x20
> [222.857805] __lock_acquire+0x943/0x2800
> [222.857808] ? stack_trace_save+0x4b/0x70
> [222.857812] lock_acquire+0xc5/0x2f0
> [222.857814] ? pmc_core_acpi_pm_timer_suspend_resume+0x50/0xe0 [intel_pmc_core]
> [222.857817] __mutex_lock+0xbe/0xc70
> [222.857819] ? pmc_core_acpi_pm_timer_suspend_resume+0x50/0xe0 [intel_pmc_core]
> [222.857822] ? pmc_core_acpi_pm_timer_suspend_resume+0x50/0xe0 [intel_pmc_core]
> [222.857825] mutex_lock_nested+0x1b/0x30
> [222.857827] ? mutex_lock_nested+0x1b/0x30
> [222.857828] pmc_core_acpi_pm_timer_suspend_resume+0x50/0xe0 [intel_pmc_core]
> [222.857831] acpi_pm_suspend+0x23/0x40
> [222.857834] clocksource_suspend+0x2b/0x50
> [222.857836] timekeeping_suspend+0x22a/0x360
> [222.857839] tick_freeze+0x89/0x110
> [222.857840] enter_s2idle_proper+0x34/0x1d0
> [222.857843] cpuidle_enter_s2idle+0xaa/0x120
> [222.857845] ? tsc_verify_tsc_adjust+0x42/0x100
> [222.857849] do_idle+0x221/0x250
> [222.857852] cpu_startup_entry+0x29/0x30
> [222.857854] start_secondary+0x12e/0x160
> [222.857856] common_startup_64+0x13e/0x141
> [222.857859] </TASK>
>
> This reverts commit e86c8186d03a6ba018e881ed45f0962ad553e861.
>
> Signed-off-by: Luca Coelho <luciano.coelho@intel.com>
References: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12342
Pushed to topic/core-for-CI.
BR,
Jani.
> ---
>
> 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/tgl.c | 2 --
> 7 files changed, 63 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/adl.c b/drivers/platform/x86/intel/pmc/adl.c
> index 9d9c07f44ff6..e7878558fd90 100644
> --- a/drivers/platform/x86/intel/pmc/adl.c
> +++ b/drivers/platform/x86/intel/pmc/adl.c
> @@ -295,8 +295,6 @@ 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 513c02670c5a..dd72974bf71e 100644
> --- a/drivers/platform/x86/intel/pmc/cnp.c
> +++ b/drivers/platform/x86/intel/pmc/cnp.c
> @@ -200,8 +200,6 @@ 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 ecb47f8b4f83..d6c0f88f8c7b 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -11,7 +11,6 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> -#include <linux/acpi_pmtmr.h>
> #include <linux/bitfield.h>
> #include <linux/debugfs.h>
> #include <linux/delay.h>
> @@ -1258,38 +1257,6 @@ 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)
> {
> @@ -1486,7 +1453,6 @@ 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;
>
> @@ -1545,11 +1511,6 @@ 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");
>
> @@ -1559,12 +1520,6 @@ 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 75fd593a7b0f..b9d3291d0bf2 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -68,8 +68,6 @@ 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,
> @@ -150,8 +148,6 @@ 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
> @@ -355,8 +351,6 @@ 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;
> };
>
> /**
> @@ -432,8 +426,6 @@ 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 cbbd44054468..71b0fd6cb7d8 100644
> --- a/drivers/platform/x86/intel/pmc/icl.c
> +++ b/drivers/platform/x86/intel/pmc/icl.c
> @@ -46,8 +46,6 @@ 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 91f2fa728f5c..c7d15d864039 100644
> --- a/drivers/platform/x86/intel/pmc/mtl.c
> +++ b/drivers/platform/x86/intel/pmc/mtl.c
> @@ -462,8 +462,6 @@ 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/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c
> index 371b4e30f142..e0580de18077 100644
> --- a/drivers/platform/x86/intel/pmc/tgl.c
> +++ b/drivers/platform/x86/intel/pmc/tgl.c
> @@ -197,8 +197,6 @@ 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,
--
Jani Nikula, Intel
prev parent reply other threads:[~2024-10-09 13:34 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-07 9:05 [core-for-CI PATCH] Revert "platform/x86:intel/pmc: Enable the ACPI PM Timer to be turned off when suspended" Luca Coelho
2024-10-08 3:20 ` ✓ Fi.CI.BAT: success for " Patchwork
2024-10-09 4:43 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-10-09 13:34 ` Jani Nikula [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y12x8ko5.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.saarinen@intel.com \
--cc=luciano.coelho@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.