Re: [PATCH 3/4] platform/x86/intel/pmc: Always dump LPM status regs on unsuccessful paths

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

Hi,

My response is inline.

Thanks!

Xi

On 9/22/2025 3:52 PM, Guilherme G. Piccoli wrote:
> Right now, there are 2 fail paths on pmc_core_resume_common(): either
> after (some) package(s) didn't enter Cx state, or after s0ix was
> not successfully entered.
>
> The code has a debug output, dumping LPM registers, but *only*
> on s0ix fail path, not when packages fail to enter some Cx state.
>
> Let's make it output the LPM registers in both fail cases, in order to
> help debugging issues.
>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com>
> ---
>   drivers/platform/x86/intel/pmc/core.c | 22 ++++++++++++++--------
>   1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c
> index c8ce5d6ec30c..aeb5e47cf5bb 100644
> --- a/drivers/platform/x86/intel/pmc/core.c
> +++ b/drivers/platform/x86/intel/pmc/core.c
> @@ -1882,16 +1882,22 @@ int pmc_core_resume_common(struct pmc_dev *pmcdev)
>   					 msr_map[i].name, pc_cnt);
>   			}
>   		}
> -		return 0;
> +	} else {
> +		/* The real interesting case - S0ix failed - lets ask PMC why. */
> +		dev_warn(dev, "CPU did not enter SLP_S0!!! (S0ix cnt=%llu)\n",
> +			 pmcdev->s0ix_counter);
> +
> +		/*
> +		 * Notice that SLP_S0_DBG regs are captured on C10 entry,
> +		 * according to the spec. So if we didn't enter C10 (i.e.,
> +		 * the  above if-block was executed) seems to make no sense
> +		 * in dumping them.
> +		 */
> +		if (pmc->map->slps0_dbg_maps)
> +			pmc_core_slps0_display(pmc, dev, NULL);
> +
>   	}
>   
> -	/* The real interesting case - S0ix failed - lets ask PMC why. */
> -	dev_warn(dev, "CPU did not enter SLP_S0!!! (S0ix cnt=%llu)\n",
> -		 pmcdev->s0ix_counter);
> -
> -	if (pmc->map->slps0_dbg_maps)
> -		pmc_core_slps0_display(pmc, dev, NULL);
> -
>   	for (i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
>   		struct pmc *pmc = pmcdev->pmcs[i];
>   

Entering the S0ix state requires the system to reach the deepest package 
C-state, PC10. If the system fails to achieve PC10 residency during 
suspend, the slps0_dbg_maps data becomes irrelevant and may mislead 
users. For this reason, the driver hides this information when PC10 
residency is not attained.