| Message ID | 20231009225653.36030-5-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Add support for drivers to decide bridge D3 policy | expand |
Hi Mario, On Tue, Oct 10, 2023 at 6:57 AM Mario Limonciello <mario.limonciello@amd.com> wrote: > > The default kernel policy will allow modern machines to effectively put > all PCIe bridges into PCI D3. This policy doesn't match what Windows uses. > > In Windows the driver stack includes a "Power Engine Plugin" (uPEP driver) > to decide the policy for integrated devices using PEP device constraints. > > Device constraints are expressed as a number in the _DSM of the PNP0D80 > device and exported by the kernel in acpi_get_lps0_constraint(). > > Add support for SoCs to use constraints on Linux as well for deciding > target state for integrated PCI bridges. > > No SoCs are introduced by default with this change, they will be added > later on a case by case basis. > > Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/platform/x86/amd/pmc/pmc.c | 59 ++++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c > index c1e788b67a74..34e76c6b3fb2 100644 > --- a/drivers/platform/x86/amd/pmc/pmc.c > +++ b/drivers/platform/x86/amd/pmc/pmc.c > @@ -570,6 +570,14 @@ static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev) > debugfs_remove_recursive(dev->dbgfs_dir); > } > > +static bool amd_pmc_use_acpi_constraints(struct amd_pmc_dev *dev) > +{ > + switch (dev->cpu_id) { > + default: > + return false; > + } > +} > + > static bool amd_pmc_is_stb_supported(struct amd_pmc_dev *dev) > { > switch (dev->cpu_id) { > @@ -741,6 +749,41 @@ static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) > return 0; > } > > +/* > + * Constraints are specified in the ACPI LPS0 device and specify what the > + * platform intended for the device. > + * > + * If a constraint is present and >= to ACPI_STATE_S3, then enable D3 for the > + * device. > + * If a constraint is not present or < ACPI_STATE_S3, then disable D3 for the > + * device. > + */ > +static enum bridge_d3_pref amd_pmc_d3_check(struct pci_dev *pci_dev) > +{ > + struct acpi_device *adev = ACPI_COMPANION(&pci_dev->dev); > + int constraint; > + > + if (!adev) > + return BRIDGE_PREF_UNSET; > + > + constraint = acpi_get_lps0_constraint(adev); > + dev_dbg(&pci_dev->dev, "constraint is %d\n", constraint); > + > + switch (constraint) { > + case ACPI_STATE_UNKNOWN: > + case ACPI_STATE_S0: > + case ACPI_STATE_S1: > + case ACPI_STATE_S2: I believe it's a typo? I think ACPI_STATE_Dx should be used for device state. > + return BRIDGE_PREF_DRIVER_D0; > + case ACPI_STATE_S3: > + return BRIDGE_PREF_DRIVER_D3; I've seen both 3 (i.e. ACPI_STATE_D3_HOT) and 4 (i.e. ACPI_STATE_D3_COLD) defined in LPI constraint table. Should those two be treated differently? > + default: > + break; > + } > + > + return BRIDGE_PREF_UNSET; > +} > + > static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) > { > struct rtc_device *rtc_device; > @@ -881,6 +924,11 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { > .restore = amd_pmc_s2idle_restore, > }; > > +static struct pci_d3_driver_ops amd_pmc_d3_ops = { > + .check = amd_pmc_d3_check, > + .priority = 50, > +}; > + > static int amd_pmc_suspend_handler(struct device *dev) > { > struct amd_pmc_dev *pdev = dev_get_drvdata(dev); > @@ -1074,10 +1122,19 @@ static int amd_pmc_probe(struct platform_device *pdev) > amd_pmc_quirks_init(dev); > } > > + if (amd_pmc_use_acpi_constraints(dev)) { > + err = pci_register_driver_d3_policy_cb(&amd_pmc_d3_ops); > + if (err) > + goto err_register_lps0; > + } Does this only apply to PCI? USB can have ACPI companion too. Kai-Heng > + > amd_pmc_dbgfs_register(dev); > pm_report_max_hw_sleep(U64_MAX); > return 0; > > +err_register_lps0: > + if (IS_ENABLED(CONFIG_SUSPEND)) > + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); > err_pci_dev_put: > pci_dev_put(rdev); > return err; > @@ -1089,6 +1146,8 @@ static void amd_pmc_remove(struct platform_device *pdev) > > if (IS_ENABLED(CONFIG_SUSPEND)) > acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); > + if (amd_pmc_use_acpi_constraints(dev)) > + pci_unregister_driver_d3_policy_cb(&amd_pmc_d3_ops); > amd_pmc_dbgfs_unregister(dev); > pci_dev_put(dev->rdev); > mutex_destroy(&dev->lock); > -- > 2.34.1 >
On 10/15/2023 21:11, Kai-Heng Feng wrote: > Hi Mario, > > On Tue, Oct 10, 2023 at 6:57 AM Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> The default kernel policy will allow modern machines to effectively put >> all PCIe bridges into PCI D3. This policy doesn't match what Windows uses. >> >> In Windows the driver stack includes a "Power Engine Plugin" (uPEP driver) >> to decide the policy for integrated devices using PEP device constraints. >> >> Device constraints are expressed as a number in the _DSM of the PNP0D80 >> device and exported by the kernel in acpi_get_lps0_constraint(). >> >> Add support for SoCs to use constraints on Linux as well for deciding >> target state for integrated PCI bridges. >> >> No SoCs are introduced by default with this change, they will be added >> later on a case by case basis. >> >> Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> drivers/platform/x86/amd/pmc/pmc.c | 59 ++++++++++++++++++++++++++++++ >> 1 file changed, 59 insertions(+) >> >> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c >> index c1e788b67a74..34e76c6b3fb2 100644 >> --- a/drivers/platform/x86/amd/pmc/pmc.c >> +++ b/drivers/platform/x86/amd/pmc/pmc.c >> @@ -570,6 +570,14 @@ static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev) >> debugfs_remove_recursive(dev->dbgfs_dir); >> } >> >> +static bool amd_pmc_use_acpi_constraints(struct amd_pmc_dev *dev) >> +{ >> + switch (dev->cpu_id) { >> + default: >> + return false; >> + } >> +} >> + >> static bool amd_pmc_is_stb_supported(struct amd_pmc_dev *dev) >> { >> switch (dev->cpu_id) { >> @@ -741,6 +749,41 @@ static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) >> return 0; >> } >> >> +/* >> + * Constraints are specified in the ACPI LPS0 device and specify what the >> + * platform intended for the device. >> + * >> + * If a constraint is present and >= to ACPI_STATE_S3, then enable D3 for the >> + * device. >> + * If a constraint is not present or < ACPI_STATE_S3, then disable D3 for the >> + * device. >> + */ >> +static enum bridge_d3_pref amd_pmc_d3_check(struct pci_dev *pci_dev) >> +{ >> + struct acpi_device *adev = ACPI_COMPANION(&pci_dev->dev); >> + int constraint; >> + >> + if (!adev) >> + return BRIDGE_PREF_UNSET; >> + >> + constraint = acpi_get_lps0_constraint(adev); >> + dev_dbg(&pci_dev->dev, "constraint is %d\n", constraint); >> + >> + switch (constraint) { >> + case ACPI_STATE_UNKNOWN: >> + case ACPI_STATE_S0: >> + case ACPI_STATE_S1: >> + case ACPI_STATE_S2: > > I believe it's a typo? > I think ACPI_STATE_Dx should be used for device state. Yes; typo thanks. > >> + return BRIDGE_PREF_DRIVER_D0; >> + case ACPI_STATE_S3: >> + return BRIDGE_PREF_DRIVER_D3; > > I've seen both 3 (i.e. ACPI_STATE_D3_HOT) and 4 (i.e. > ACPI_STATE_D3_COLD) defined in LPI constraint table. > Should those two be treated differently? Was this AMD system or Intel system? AFAIK AMD doesn't use D3cold in constraints, they are collectively "D3". > >> + default: >> + break; >> + } >> + >> + return BRIDGE_PREF_UNSET; >> +} >> + >> static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) >> { >> struct rtc_device *rtc_device; >> @@ -881,6 +924,11 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { >> .restore = amd_pmc_s2idle_restore, >> }; >> >> +static struct pci_d3_driver_ops amd_pmc_d3_ops = { >> + .check = amd_pmc_d3_check, >> + .priority = 50, >> +}; >> + >> static int amd_pmc_suspend_handler(struct device *dev) >> { >> struct amd_pmc_dev *pdev = dev_get_drvdata(dev); >> @@ -1074,10 +1122,19 @@ static int amd_pmc_probe(struct platform_device *pdev) >> amd_pmc_quirks_init(dev); >> } >> >> + if (amd_pmc_use_acpi_constraints(dev)) { >> + err = pci_register_driver_d3_policy_cb(&amd_pmc_d3_ops); >> + if (err) >> + goto err_register_lps0; >> + } > > Does this only apply to PCI? USB can have ACPI companion too. It only applies to things in the constraints, which is only "SOC internal" devices. No internal USB devices. > > Kai-Heng > >> + >> amd_pmc_dbgfs_register(dev); >> pm_report_max_hw_sleep(U64_MAX); >> return 0; >> >> +err_register_lps0: >> + if (IS_ENABLED(CONFIG_SUSPEND)) >> + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); >> err_pci_dev_put: >> pci_dev_put(rdev); >> return err; >> @@ -1089,6 +1146,8 @@ static void amd_pmc_remove(struct platform_device *pdev) >> >> if (IS_ENABLED(CONFIG_SUSPEND)) >> acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); >> + if (amd_pmc_use_acpi_constraints(dev)) >> + pci_unregister_driver_d3_policy_cb(&amd_pmc_d3_ops); >> amd_pmc_dbgfs_unregister(dev); >> pci_dev_put(dev->rdev); >> mutex_destroy(&dev->lock); >> -- >> 2.34.1 >>
On Tue, Oct 17, 2023 at 5:34 AM Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 10/15/2023 21:11, Kai-Heng Feng wrote: > > Hi Mario, > > > > On Tue, Oct 10, 2023 at 6:57 AM Mario Limonciello > > <mario.limonciello@amd.com> wrote: > >> > >> The default kernel policy will allow modern machines to effectively put > >> all PCIe bridges into PCI D3. This policy doesn't match what Windows uses. > >> > >> In Windows the driver stack includes a "Power Engine Plugin" (uPEP driver) > >> to decide the policy for integrated devices using PEP device constraints. > >> > >> Device constraints are expressed as a number in the _DSM of the PNP0D80 > >> device and exported by the kernel in acpi_get_lps0_constraint(). > >> > >> Add support for SoCs to use constraints on Linux as well for deciding > >> target state for integrated PCI bridges. > >> > >> No SoCs are introduced by default with this change, they will be added > >> later on a case by case basis. > >> > >> Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > >> --- > >> drivers/platform/x86/amd/pmc/pmc.c | 59 ++++++++++++++++++++++++++++++ > >> 1 file changed, 59 insertions(+) > >> > >> diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c > >> index c1e788b67a74..34e76c6b3fb2 100644 > >> --- a/drivers/platform/x86/amd/pmc/pmc.c > >> +++ b/drivers/platform/x86/amd/pmc/pmc.c > >> @@ -570,6 +570,14 @@ static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev) > >> debugfs_remove_recursive(dev->dbgfs_dir); > >> } > >> > >> +static bool amd_pmc_use_acpi_constraints(struct amd_pmc_dev *dev) > >> +{ > >> + switch (dev->cpu_id) { > >> + default: > >> + return false; > >> + } > >> +} > >> + > >> static bool amd_pmc_is_stb_supported(struct amd_pmc_dev *dev) > >> { > >> switch (dev->cpu_id) { > >> @@ -741,6 +749,41 @@ static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) > >> return 0; > >> } > >> > >> +/* > >> + * Constraints are specified in the ACPI LPS0 device and specify what the > >> + * platform intended for the device. > >> + * > >> + * If a constraint is present and >= to ACPI_STATE_S3, then enable D3 for the > >> + * device. > >> + * If a constraint is not present or < ACPI_STATE_S3, then disable D3 for the > >> + * device. > >> + */ > >> +static enum bridge_d3_pref amd_pmc_d3_check(struct pci_dev *pci_dev) > >> +{ > >> + struct acpi_device *adev = ACPI_COMPANION(&pci_dev->dev); > >> + int constraint; > >> + > >> + if (!adev) > >> + return BRIDGE_PREF_UNSET; > >> + > >> + constraint = acpi_get_lps0_constraint(adev); > >> + dev_dbg(&pci_dev->dev, "constraint is %d\n", constraint); > >> + > >> + switch (constraint) { > >> + case ACPI_STATE_UNKNOWN: > >> + case ACPI_STATE_S0: > >> + case ACPI_STATE_S1: > >> + case ACPI_STATE_S2: > > > > I believe it's a typo? > > I think ACPI_STATE_Dx should be used for device state. > > Yes; typo thanks. > > > > >> + return BRIDGE_PREF_DRIVER_D0; > >> + case ACPI_STATE_S3: > >> + return BRIDGE_PREF_DRIVER_D3; > > > > I've seen both 3 (i.e. ACPI_STATE_D3_HOT) and 4 (i.e. > > ACPI_STATE_D3_COLD) defined in LPI constraint table. > > Should those two be treated differently? > > Was this AMD system or Intel system? AFAIK AMD doesn't use D3cold in > constraints, they are collectively "D3". Intel based system. So the final D3 state is decided by the presence of power resources? > > > > >> + default: > >> + break; > >> + } > >> + > >> + return BRIDGE_PREF_UNSET; > >> +} > >> + > >> static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) > >> { > >> struct rtc_device *rtc_device; > >> @@ -881,6 +924,11 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { > >> .restore = amd_pmc_s2idle_restore, > >> }; > >> > >> +static struct pci_d3_driver_ops amd_pmc_d3_ops = { > >> + .check = amd_pmc_d3_check, > >> + .priority = 50, > >> +}; > >> + > >> static int amd_pmc_suspend_handler(struct device *dev) > >> { > >> struct amd_pmc_dev *pdev = dev_get_drvdata(dev); > >> @@ -1074,10 +1122,19 @@ static int amd_pmc_probe(struct platform_device *pdev) > >> amd_pmc_quirks_init(dev); > >> } > >> > >> + if (amd_pmc_use_acpi_constraints(dev)) { > >> + err = pci_register_driver_d3_policy_cb(&amd_pmc_d3_ops); > >> + if (err) > >> + goto err_register_lps0; > >> + } > > > > Does this only apply to PCI? USB can have ACPI companion too. > > It only applies to things in the constraints, which is only "SOC > internal" devices. No internal USB devices. So sounds like it only applies to PCI devices? Kai-Heng > > > > > Kai-Heng > > > >> + > >> amd_pmc_dbgfs_register(dev); > >> pm_report_max_hw_sleep(U64_MAX); > >> return 0; > >> > >> +err_register_lps0: > >> + if (IS_ENABLED(CONFIG_SUSPEND)) > >> + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); > >> err_pci_dev_put: > >> pci_dev_put(rdev); > >> return err; > >> @@ -1089,6 +1146,8 @@ static void amd_pmc_remove(struct platform_device *pdev) > >> > >> if (IS_ENABLED(CONFIG_SUSPEND)) > >> acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); > >> + if (amd_pmc_use_acpi_constraints(dev)) > >> + pci_unregister_driver_d3_policy_cb(&amd_pmc_d3_ops); > >> amd_pmc_dbgfs_unregister(dev); > >> pci_dev_put(dev->rdev); > >> mutex_destroy(&dev->lock); > >> -- > >> 2.34.1 > >> >
>>> I've seen both 3 (i.e. ACPI_STATE_D3_HOT) and 4 (i.e. >>> ACPI_STATE_D3_COLD) defined in LPI constraint table. >>> Should those two be treated differently? >> >> Was this AMD system or Intel system? AFAIK AMD doesn't use D3cold in >> constraints, they are collectively "D3". > > Intel based system. > > So the final D3 state is decided by the presence of power resources? > Right. This is why I've mentioned in some of my series that pci_d3cold_enable() / pci_d3cold_disable() are misnamed and dev->no_d3cold can be misleading. FYI - I have (slowly) been modifying this series to use pci_d3_cold_enable()/pci_d3_cold_disable() instead of the extra overhead of the policy decision but I have some problems. I anticipate future version will also modify pci_bridge_d3_update(). >> >>> >>>> + default: >>>> + break; >>>> + } >>>> + >>>> + return BRIDGE_PREF_UNSET; >>>> +} >>>> + >>>> static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) >>>> { >>>> struct rtc_device *rtc_device; >>>> @@ -881,6 +924,11 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { >>>> .restore = amd_pmc_s2idle_restore, >>>> }; >>>> >>>> +static struct pci_d3_driver_ops amd_pmc_d3_ops = { >>>> + .check = amd_pmc_d3_check, >>>> + .priority = 50, >>>> +}; >>>> + >>>> static int amd_pmc_suspend_handler(struct device *dev) >>>> { >>>> struct amd_pmc_dev *pdev = dev_get_drvdata(dev); >>>> @@ -1074,10 +1122,19 @@ static int amd_pmc_probe(struct platform_device *pdev) >>>> amd_pmc_quirks_init(dev); >>>> } >>>> >>>> + if (amd_pmc_use_acpi_constraints(dev)) { >>>> + err = pci_register_driver_d3_policy_cb(&amd_pmc_d3_ops); >>>> + if (err) >>>> + goto err_register_lps0; >>>> + } >>> >>> Does this only apply to PCI? USB can have ACPI companion too. >> >> It only applies to things in the constraints, which is only "SOC >> internal" devices. No internal USB devices. > > So sounds like it only applies to PCI devices? Correct. > > Kai-Heng > >> >>> >>> Kai-Heng >>> >>>> + >>>> amd_pmc_dbgfs_register(dev); >>>> pm_report_max_hw_sleep(U64_MAX); >>>> return 0; >>>> >>>> +err_register_lps0: >>>> + if (IS_ENABLED(CONFIG_SUSPEND)) >>>> + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); >>>> err_pci_dev_put: >>>> pci_dev_put(rdev); >>>> return err; >>>> @@ -1089,6 +1146,8 @@ static void amd_pmc_remove(struct platform_device *pdev) >>>> >>>> if (IS_ENABLED(CONFIG_SUSPEND)) >>>> acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); >>>> + if (amd_pmc_use_acpi_constraints(dev)) >>>> + pci_unregister_driver_d3_policy_cb(&amd_pmc_d3_ops); >>>> amd_pmc_dbgfs_unregister(dev); >>>> pci_dev_put(dev->rdev); >>>> mutex_destroy(&dev->lock); >>>> -- >>>> 2.34.1 >>>> >>
diff --git a/drivers/platform/x86/amd/pmc/pmc.c b/drivers/platform/x86/amd/pmc/pmc.c index c1e788b67a74..34e76c6b3fb2 100644 --- a/drivers/platform/x86/amd/pmc/pmc.c +++ b/drivers/platform/x86/amd/pmc/pmc.c @@ -570,6 +570,14 @@ static void amd_pmc_dbgfs_unregister(struct amd_pmc_dev *dev) debugfs_remove_recursive(dev->dbgfs_dir); } +static bool amd_pmc_use_acpi_constraints(struct amd_pmc_dev *dev) +{ + switch (dev->cpu_id) { + default: + return false; + } +} + static bool amd_pmc_is_stb_supported(struct amd_pmc_dev *dev) { switch (dev->cpu_id) { @@ -741,6 +749,41 @@ static int amd_pmc_czn_wa_irq1(struct amd_pmc_dev *pdev) return 0; } +/* + * Constraints are specified in the ACPI LPS0 device and specify what the + * platform intended for the device. + * + * If a constraint is present and >= to ACPI_STATE_S3, then enable D3 for the + * device. + * If a constraint is not present or < ACPI_STATE_S3, then disable D3 for the + * device. + */ +static enum bridge_d3_pref amd_pmc_d3_check(struct pci_dev *pci_dev) +{ + struct acpi_device *adev = ACPI_COMPANION(&pci_dev->dev); + int constraint; + + if (!adev) + return BRIDGE_PREF_UNSET; + + constraint = acpi_get_lps0_constraint(adev); + dev_dbg(&pci_dev->dev, "constraint is %d\n", constraint); + + switch (constraint) { + case ACPI_STATE_UNKNOWN: + case ACPI_STATE_S0: + case ACPI_STATE_S1: + case ACPI_STATE_S2: + return BRIDGE_PREF_DRIVER_D0; + case ACPI_STATE_S3: + return BRIDGE_PREF_DRIVER_D3; + default: + break; + } + + return BRIDGE_PREF_UNSET; +} + static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg) { struct rtc_device *rtc_device; @@ -881,6 +924,11 @@ static struct acpi_s2idle_dev_ops amd_pmc_s2idle_dev_ops = { .restore = amd_pmc_s2idle_restore, }; +static struct pci_d3_driver_ops amd_pmc_d3_ops = { + .check = amd_pmc_d3_check, + .priority = 50, +}; + static int amd_pmc_suspend_handler(struct device *dev) { struct amd_pmc_dev *pdev = dev_get_drvdata(dev); @@ -1074,10 +1122,19 @@ static int amd_pmc_probe(struct platform_device *pdev) amd_pmc_quirks_init(dev); } + if (amd_pmc_use_acpi_constraints(dev)) { + err = pci_register_driver_d3_policy_cb(&amd_pmc_d3_ops); + if (err) + goto err_register_lps0; + } + amd_pmc_dbgfs_register(dev); pm_report_max_hw_sleep(U64_MAX); return 0; +err_register_lps0: + if (IS_ENABLED(CONFIG_SUSPEND)) + acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); err_pci_dev_put: pci_dev_put(rdev); return err; @@ -1089,6 +1146,8 @@ static void amd_pmc_remove(struct platform_device *pdev) if (IS_ENABLED(CONFIG_SUSPEND)) acpi_unregister_lps0_dev(&amd_pmc_s2idle_dev_ops); + if (amd_pmc_use_acpi_constraints(dev)) + pci_unregister_driver_d3_policy_cb(&amd_pmc_d3_ops); amd_pmc_dbgfs_unregister(dev); pci_dev_put(dev->rdev); mutex_destroy(&dev->lock);
The default kernel policy will allow modern machines to effectively put all PCIe bridges into PCI D3. This policy doesn't match what Windows uses. In Windows the driver stack includes a "Power Engine Plugin" (uPEP driver) to decide the policy for integrated devices using PEP device constraints. Device constraints are expressed as a number in the _DSM of the PNP0D80 device and exported by the kernel in acpi_get_lps0_constraint(). Add support for SoCs to use constraints on Linux as well for deciding target state for integrated PCI bridges. No SoCs are introduced by default with this change, they will be added later on a case by case basis. Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/platform/x86/amd/pmc/pmc.c | 59 ++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)