On 18.01.2022 00:35, Bjorn Helgaas wrote: > [+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? > Don't know what the exact symptoms of the original problem are. I'd more see a certain risk that this change reveals bugs in RPM usage of PCI device drivers. There's not a fixed list of potential symptoms. One example: igb driver caused a hang on system shutdown when RPM was enabled due to a RTNL deadlock in RPM resume path. > - 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? For each device in sysfs: power/control: "auto" -> "on" > >> [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 >>
