RE: [PATCH] platform/x86/intel/pmc Fix Arrow Lake U/H support to intel_pmc_core driver

[Brandt, Todd E]

Let me resend with a fixes tag (sorry about that), however this might be too late to be included in the upstream 6.15.0-rc7 kernel. So maybe we can slate this for 6.16.0-c1. I also want to get Xi's feedback and she's out this week.

-----Original Message-----
From: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> 
Sent: Friday, May 16, 2025 4:41 AM
To: Brandt, Todd E <todd.e.brandt@xxxxxxxxx>
Cc: platform-driver-x86@xxxxxxxxxxxxxxx; xi.pardee@xxxxxxxxxxxxxxx; LKML <linux-kernel@xxxxxxxxxxxxxxx>; todd.e.brandt@xxxxxxxxxxxxxxx
Subject: Re: [PATCH] platform/x86/intel/pmc Fix Arrow Lake U/H support to intel_pmc_core driver

On Thu, 15 May 2025, Todd Brandt wrote:

> The ARL requires that the GMA and NPU devices both be in D3Hot in 
> order for PC10 and S0iX to be achieved in S2idle. The original ARL-H/U 
> addition to the intel_pmc_core driver attempted to do this by 
> switching them to D3 in the init and resume calls of the intel_pmc_core driver.
> 
> The problem is the ARL-H/U have a different NPU device and thus are 
> not being properly set and thus S0iX does not work properly in 
> ARL-H/U. This patch creates a new ARL-H specific device id that is 
> correct and also adds the D3 fixup to the suspend callback. This way 
> if the PCI devies drop from D3 to D0 after resume they can be 
> corrected for the next suspend. Thus there is no dropout in S0iX.
> 

Shouldn't this have a Fixes tag?

> Signed-off-by: Todd Brandt <todd.e.brandt@xxxxxxxxx>
> ---
>  drivers/platform/x86/intel/pmc/arl.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/arl.c 
> b/drivers/platform/x86/intel/pmc/arl.c
> index 320993bd6d31..5053e3dd8f5e 100644
> --- a/drivers/platform/x86/intel/pmc/arl.c
> +++ b/drivers/platform/x86/intel/pmc/arl.c
> @@ -681,6 +681,7 @@ static struct pmc_info arl_pmc_info_list[] = {
>  
>  #define ARL_NPU_PCI_DEV			0xad1d
>  #define ARL_GNA_PCI_DEV			0xae4c
> +#define ARL_H_NPU_PCI_DEV		0x7d1d
>  #define ARL_H_GNA_PCI_DEV		0x774c
>  /*
>   * Set power state of select devices that do not have drivers to D3 
> @@ -694,7 +695,7 @@ static void arl_d3_fixup(void)
>  
>  static void arl_h_d3_fixup(void)
>  {
> -	pmc_core_set_device_d3(ARL_NPU_PCI_DEV);
> +	pmc_core_set_device_d3(ARL_H_NPU_PCI_DEV);
>  	pmc_core_set_device_d3(ARL_H_GNA_PCI_DEV);
>  }
>  
> @@ -705,6 +706,13 @@ static int arl_resume(struct pmc_dev *pmcdev)
>  	return cnl_resume(pmcdev);
>  }
>  
> +static void arl_h_suspend(struct pmc_dev *pmcdev) {
> +	arl_h_d3_fixup();
> +
> +	cnl_suspend(pmcdev);
> +}
> +
>  static int arl_h_resume(struct pmc_dev *pmcdev)  {
>  	arl_h_d3_fixup();
> @@ -739,7 +747,7 @@ struct pmc_dev_info arl_h_pmc_dev = {
>  	.dmu_guid = ARL_PMT_DMU_GUID,
>  	.regmap_list = arl_pmc_info_list,
>  	.map = &mtl_socm_reg_map,
> -	.suspend = cnl_suspend,
> +	.suspend = arl_h_suspend,
>  	.resume = arl_h_resume,
>  	.init = arl_h_core_init,
>  };
> 

--
 i.