[+cc Kai-Heng, Lukas, Mika, since they were cc'd or commented on [0] below]
On Mon, Jan 17, 2022 at 11:51:54AM +0100, Heiner Kallweit wrote:
> Currently PCI core forbids RPM and requires opt-in from userspace,
> apart from few drivers calling pm_runtime_allow(). Reason is that some
> early ACPI PM implementations conflict with RPM, see [0].
> Note that as of today pm_runtime_forbid() is also called for non-ACPI
> systems. Maybe it's time to allow RPM per default for non-ACPI systems
> and recent enough ACPI versions. Let's allow RPM from ACPI 5.0 which
> was published in 2011.
Let's reword this to use the positive sense, e.g., something like
"enable runtime power management for non-ACPI and ACPI 5.0 and newer."
This feels like a potentially significant change that could cause
breakage.
- How would a user recognize that we're doing something different?
Maybe we need a note in dmesg?
- If a system broke because of this, what would it look like? How
would a user notice a problem, and how would he or she connect the
problem to this change?
- Is there a kernel parameter that will get the previous behavior of
disabling runtime PM as a workaround until a quirk can be added?
If so, we should probably mention it here. If not, should there
be?
> [0] https://lkml.org/lkml/2020/11/17/1548
Please use an https://lore.kernel.org/r/... link instead.
Let's mention bb910a7040e9 ("PCI/PM Runtime: Make runtime PM of PCI
devices inactive by default") as well to help connect the dots here.
> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
> ---
> drivers/pci/pci.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 428afd459..26e3a500c 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3101,7 +3101,12 @@ void pci_pm_init(struct pci_dev *dev)
> u16 status;
> u16 pmc;
>
> - pm_runtime_forbid(&dev->dev);
> +#ifdef CONFIG_ACPI
> + /* Some early ACPI PM implementations conflict with RPM. */
> + if (acpi_gbl_FADT.header.revision > 0 &&
> + acpi_gbl_FADT.header.revision < 5)
> + pm_runtime_forbid(&dev->dev);
> +#endif
> pm_runtime_set_active(&dev->dev);
> pm_runtime_enable(&dev->dev);
> device_enable_async_suspend(&dev->dev);
> --
> 2.34.1
>
