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

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

On Tue, 20 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.
> 
> [v2 changes]
> 
> Added Fixes line
> 
> [v3 changes]
> 
> Xi asked that I see if adding the arl_h_suspend call actually made any
> difference in fixing the issue of S0iX not being achieved, as there are
> 4 other platforms that use this same fix in just the init and resume
> callbacks. Upon further testing I discovered it didn't matter. So I just
> simplified the D3 change and left the fixup call to init and resume.

Okay, I've replaced the commit in the review-ilpo-fixes branch with this 
v3 patch. Thanks.

In future, put that patch vX history under --- line so I don't have to 
manually remove it (our tools cut the text below that triple dash line
automatically).

-- 
 i.

> 
> Fixes: bd820906ea9d ("platform/x86/intel/pmc: Add Arrow Lake U/H support to intel_pmc_core driver")
> Signed-off-by: Todd Brandt <todd.e.brandt@intel.com>
> ---
>  drivers/platform/x86/intel/pmc/arl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/arl.c b/drivers/platform/x86/intel/pmc/arl.c
> index 320993bd6d31..f9c48738b853 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);
>  }