Re: [PATCH 4/4][RFC] platform/x86/intel/pmc: Re-add SLP_S0_DBG register dump on Tiger Lake

[Xi Pardee <xi.pardee@linux.intel.com>]

Hi,

My response is in line.

Thanks!
Xi

On 9/22/2025 3:52 PM, Guilherme G. Piccoli wrote:
> Commit a018e28f0880 ("platform/x86: intel_pmc_core: Remove slp_s0 attributes from tgl_reg_map")
> removed the SLP_Sx_DBG register dump on suspend/resume s0ix-related failures
> on Tiger Lake. The mentioned reason was related to potential sub-states.
>
> Let's re-enable the SLP_Sx_DBG register dumping on failures, also fixing
> the register mapping (according to the spec[0]) and adding it also to
> Tiger Lake H, as a means to improve debug of suspend/resume failures .
>
> If we do have the sub-states, but not in all cases, better to have some
> platforms with more debug information than entirely suppress this info.
>
> [0] Refer to: "Intel 500 Series Chipset Family PCH datasheet - Vol 2"
> (Doc ID: 636174). Link (from Sep/2025):
> www.intel.com/content/www/us/en/content-details/636174/intel-500-series-chipset-family-platform-controller-hub-pch-datasheet-volume-2-of-2.html
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>

Starting with Tiger Lake platforms, the slp_s0_dbg register maps are 
deprecated. The data provided by these registers may no longer be valid.

On Tiger Lake and newer platforms, the intel_pmc_core driver introduces 
substate_status_registers and other updated attributes that offer 
information useful for debugging S0ix-related issues.

> ---
>   drivers/platform/x86/intel/pmc/tgl.c | 48 ++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
>
> diff --git a/drivers/platform/x86/intel/pmc/tgl.c b/drivers/platform/x86/intel/pmc/tgl.c
> index 02e731ed3391..cdabe9b5c20b 100644
> --- a/drivers/platform/x86/intel/pmc/tgl.c
> +++ b/drivers/platform/x86/intel/pmc/tgl.c
> @@ -185,12 +185,58 @@ static const struct pmc_bit_map *tgl_lpm_maps[] = {
>   	NULL
>   };
>   
> +/*
> + * The following SLP_S0_DBG register mappings are based on the
> + * "Intel 500 Series Chipset Family PCH datasheet - Vol 2"
> + * specification (Doc ID: 636174).
> + */
> +
> +static const struct pmc_bit_map tgl_slps0_dbg0_map[] = {
> +	{"AUDIO_D3",		BIT(0)},
> +	{"OTG_D3",		BIT(1)},
> +	{"XHCI_D3",		BIT(2)},
> +	{"LPIO_D3",		BIT(3)},
> +	{"SATA_D3",		BIT(5)},
> +	{}
> +};
> +
> +static const struct pmc_bit_map tgl_slps0_dbg1_map[] = {
> +	{"USB2_PLL_OFF",	BIT(1)},
> +	{"AUDIO_PLL_OFF",	BIT(2)},
> +	{"MAIN_PLL_OFF",	BIT(4)},
> +	{"XOSC_OFF",		BIT(5)},
> +	{"PCIE_CLKREQS_OFF",	BIT(7)},
> +	{"AUDIO_ROSC_OFF",	BIT(8)},
> +	{}
> +};
> +
> +static const struct pmc_bit_map tgl_slps0_dbg2_map[] = {
> +	{"HSIO_CORE_GATED",	BIT(0)},
> +	{"CSME_GATED",		BIT(1)},
> +	{"GBE_NO_LINK",		BIT(4)},
> +	{"PCIE_LOW_POWER",	BIT(6)},
> +	{"ISH_VNN_REQ_ACT",	BIT(8)},
> +	{"CNV_VNN_REQ_ACT",	BIT(10)},
> +	{"PMSYNC_STATE_IDLE",	BIT(12)},
> +	{"ASLT_GT_THRES",	BIT(13)},
> +	{}
> +};
> +
> +const struct pmc_bit_map *tgl_slps0_dbg_maps[] = {
> +	tgl_slps0_dbg0_map,
> +	tgl_slps0_dbg1_map,
> +	tgl_slps0_dbg2_map,
> +	NULL
> +};
> +
>   static const struct pmc_reg_map tgl_reg_map = {
>   	.pfear_sts = ext_tgl_pfear_map,
>   	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
> +	.slps0_dbg_maps = tgl_slps0_dbg_maps,
>   	.slp_s0_res_counter_step = TGL_PMC_SLP_S0_RES_COUNTER_STEP,
>   	.ltr_show_sts = cnp_ltr_show_map,
>   	.msr_sts = msr_map,
> +	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
>   	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
>   	.regmap_length = CNP_PMC_MMIO_REG_LEN,
>   	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,
> @@ -213,9 +259,11 @@ static const struct pmc_reg_map tgl_reg_map = {
>   static const struct pmc_reg_map tgl_h_reg_map = {
>   	.pfear_sts = ext_tgl_pfear_map,
>   	.slp_s0_offset = CNP_PMC_SLP_S0_RES_COUNTER_OFFSET,
> +	.slps0_dbg_maps = tgl_slps0_dbg_maps,
>   	.slp_s0_res_counter_step = TGL_PMC_SLP_S0_RES_COUNTER_STEP,
>   	.ltr_show_sts = cnp_ltr_show_map,
>   	.msr_sts = msr_map,
> +	.slps0_dbg_offset = CNP_PMC_SLPS0_DBG_OFFSET,
>   	.ltr_ignore_offset = CNP_PMC_LTR_IGNORE_OFFSET,
>   	.regmap_length = CNP_PMC_MMIO_REG_LEN,
>   	.ppfear0_offset = CNP_PMC_HOST_PPFEAR0A,