Re: [PATCH] PCI: Forbid RPM on ACPI systems before 5.0 only

[Heiner Kallweit <hkallweit1@gmail.com>]

On 18.01.2022 17:28, Rafael J. Wysocki wrote:
> On Mon, Jan 17, 2022 at 11:52 AM Heiner Kallweit <hkallweit1@gmail.com> 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.
>>
>> [0] https://lkml.org/lkml/2020/11/17/1548
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  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
> 
> Well, there are two things here.
> 
> First, there were systems in which ACPI PM was not ready for changing
> power states in the S0 system state (ie. run-time) and it was assuming
> that power states would only be changed during transitions to sleep
> states (S1 - S4) and to S5.  This can be covered by the ACPI revicion
> check, but I'm not sure if ACPI 5.0 is the right one.  Why ACPI 5 and
> not ACPI 6, for instance?
> 
Just based on the assumption that ACPI 5.0 should be recent enough.
We can also go with ACPI 6.

> Second, there were PCI devices without ACPI PM where the PCI standard
> PM didn't work correctly.  This is not related to ACPI at all and I'm
> not sure why the ACPI revision check would be sufficient to cover
> these cases.
> 
Didn't know that there were such cases. Can you provide any examples or
links to reports about such misbehaving devices?

>>         pm_runtime_set_active(&dev->dev);
>>         pm_runtime_enable(&dev->dev);
>>         device_enable_async_suspend(&dev->dev);
>> --
>> 2.34.1
>>