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

["Brandt, Todd E" <todd.e.brandt@intel.com>]

On Fri, 2025-05-16 at 14:40 +0300, Ilpo Järvinen wrote:
> 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?

Yes, you're right, let me re-send it. 

> 
> > 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,
> >  };
> > 
>