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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

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@intel.com>
> ---
>  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.