Re: [PATCH v2 2/7] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI
From: Ferry Toth
Date: Wed Sep 22 2021 - 17:31:05 EST
Hi,
Op 20-09-2021 om 21:17 schreef Rafael J. Wysocki:
From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Using struct pci_platform_pm_ops for ACPI adds unnecessary
indirection to the interactions between the PCI core and ACPI PM,
which is also subject to retpolines.
Moreover, it is not particularly clear from the current code that,
as far as PCI PM is concerned, "platform" really means just ACPI
except for the special casess when Intel MID PCI PM is used or when
ACPI support is disabled (through the kernel config or command line,
or because there are no usable ACPI tables on the system).
To address the above, rework the PCI PM code to invoke ACPI PM
functions directly as needed and drop the acpi_pci_platform_pm
object that is not necessary any more.
Accordingly, update some of the ACPI PM functions in question to do
extra checks in case the ACPI support is disabled (which previously
was taken care of by avoiding to set the pci_platform_ops pointer
in those cases).
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
---
v1 -> v2:
* Rebase on top of the new [1/7] and move dropping struct
pci_platform_pm_ops to a separate patch.
I wanted to test this series on 5.15-rc2 but this patch 2/7 doesn't
apply (after 1/7 applied). Should I apply this on another tree?
---
drivers/pci/pci-acpi.c | 42 ++++++++++++++++++++----------------------
drivers/pci/pci.c | 22 +++++++++-------------
drivers/pci/pci.h | 38 +++++++++++++++++++++++++++++++++++++-
3 files changed, 66 insertions(+), 36 deletions(-)
Index: linux-pm/drivers/pci/pci-acpi.c
===================================================================
--- linux-pm.orig/drivers/pci/pci-acpi.c
+++ linux-pm/drivers/pci/pci-acpi.c
@@ -906,10 +906,13 @@ acpi_status pci_acpi_add_pm_notifier(str
* choose highest power _SxD or any lower power
*/
-static pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
+pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
{
int acpi_state, d_max;
+ if (acpi_pci_disabled)
+ return PCI_POWER_ERROR;
+
if (pdev->no_d3cold)
d_max = ACPI_STATE_D3_HOT;
else
@@ -965,7 +968,7 @@ int pci_dev_acpi_reset(struct pci_dev *d
return 0;
}
-static bool acpi_pci_power_manageable(struct pci_dev *dev)
+bool acpi_pci_power_manageable(struct pci_dev *dev)
{
struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
@@ -974,13 +977,13 @@ static bool acpi_pci_power_manageable(st
return acpi_device_power_manageable(adev);
}
-static bool acpi_pci_bridge_d3(struct pci_dev *dev)
+bool acpi_pci_bridge_d3(struct pci_dev *dev)
{
const union acpi_object *obj;
struct acpi_device *adev;
struct pci_dev *rpdev;
- if (!dev->is_hotplug_bridge)
+ if (acpi_pci_disabled || !dev->is_hotplug_bridge)
return false;
/* Assume D3 support if the bridge is power-manageable by ACPI. */
@@ -1008,7 +1011,7 @@ static bool acpi_pci_bridge_d3(struct pc
return obj->integer.value == 1;
}
-static int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
{
struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
static const u8 state_conv[] = {
@@ -1046,7 +1049,7 @@ static int acpi_pci_set_power_state(stru
return error;
}
-static pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
+pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
{
struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
static const pci_power_t state_conv[] = {
@@ -1068,7 +1071,7 @@ static pci_power_t acpi_pci_get_power_st
return state_conv[state];
}
-static void acpi_pci_refresh_power_state(struct pci_dev *dev)
+void acpi_pci_refresh_power_state(struct pci_dev *dev)
{
struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
@@ -1093,17 +1096,23 @@ static int acpi_pci_propagate_wakeup(str
return 0;
}
-static int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
+int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
{
+ if (acpi_pci_disabled)
+ return 0;
+
if (acpi_pm_device_can_wakeup(&dev->dev))
return acpi_pm_set_device_wakeup(&dev->dev, enable);
return acpi_pci_propagate_wakeup(dev->bus, enable);
}
-static bool acpi_pci_need_resume(struct pci_dev *dev)
+bool acpi_pci_need_resume(struct pci_dev *dev)
{
- struct acpi_device *adev = ACPI_COMPANION(&dev->dev);
+ struct acpi_device *adev;
+
+ if (acpi_pci_disabled)
+ return false;
/*
* In some cases (eg. Samsung 305V4A) leaving a bridge in suspend over
@@ -1118,6 +1127,7 @@ static bool acpi_pci_need_resume(struct
if (!adev || !acpi_device_power_manageable(adev))
return false;
+ adev = ACPI_COMPANION(&dev->dev);
if (adev->wakeup.flags.valid &&
device_may_wakeup(&dev->dev) != !!adev->wakeup.prepare_count)
return true;
@@ -1128,17 +1138,6 @@ static bool acpi_pci_need_resume(struct
return !!adev->power.flags.dsw_present;
}
-static const struct pci_platform_pm_ops acpi_pci_platform_pm = {
- .bridge_d3 = acpi_pci_bridge_d3,
- .is_manageable = acpi_pci_power_manageable,
- .set_state = acpi_pci_set_power_state,
- .get_state = acpi_pci_get_power_state,
- .refresh_state = acpi_pci_refresh_power_state,
- .choose_state = acpi_pci_choose_state,
- .set_wakeup = acpi_pci_wakeup,
- .need_resume = acpi_pci_need_resume,
-};
-
void acpi_pci_add_bus(struct pci_bus *bus)
{
union acpi_object *obj;
@@ -1448,7 +1447,6 @@ static int __init acpi_pci_init(void)
if (acpi_pci_disabled)
return 0;
- pci_set_platform_pm(&acpi_pci_platform_pm);
acpi_pci_slot_init();
acpiphp_init();
Index: linux-pm/drivers/pci/pci.h
===================================================================
--- linux-pm.orig/drivers/pci/pci.h
+++ linux-pm/drivers/pci/pci.h
@@ -725,17 +725,53 @@ int pci_acpi_program_hp_params(struct pc
extern const struct attribute_group pci_dev_acpi_attr_group;
void pci_set_acpi_fwnode(struct pci_dev *dev);
int pci_dev_acpi_reset(struct pci_dev *dev, bool probe);
+bool acpi_pci_power_manageable(struct pci_dev *dev);
+bool acpi_pci_bridge_d3(struct pci_dev *dev);
+int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state);
+pci_power_t acpi_pci_get_power_state(struct pci_dev *dev);
+void acpi_pci_refresh_power_state(struct pci_dev *dev);
+int acpi_pci_wakeup(struct pci_dev *dev, bool enable);
+bool acpi_pci_need_resume(struct pci_dev *dev);
+pci_power_t acpi_pci_choose_state(struct pci_dev *pdev);
#else
static inline int pci_dev_acpi_reset(struct pci_dev *dev, bool probe)
{
return -ENOTTY;
}
-
static inline void pci_set_acpi_fwnode(struct pci_dev *dev) {}
static inline int pci_acpi_program_hp_params(struct pci_dev *dev)
{
return -ENODEV;
}
+static inline bool acpi_pci_power_manageable(struct pci_dev *dev)
+{
+ return false;
+}
+static inline bool acpi_pci_bridge_d3(struct pci_dev *dev)
+{
+ return false;
+}
+static inline int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
+{
+ return -ENODEV;
+}
+static inline pci_power_t acpi_pci_get_power_state(struct pci_dev *dev)
+{
+ return PCI_UNKNOWN;
+}
+static inline void acpi_pci_refresh_power_state(struct pci_dev *dev) {}
+static inline int acpi_pci_wakeup(struct pci_dev *dev, bool enable)
+{
+ return -ENODEV;
+}
+static inline bool acpi_pci_need_resume(struct pci_dev *dev)
+{
+ return false;
+}
+static inline pci_power_t acpi_pci_choose_state(struct pci_dev *pdev)
+{
+ return PCI_POWER_ERROR;
+}
#endif
#ifdef CONFIG_PCIEASPM
Index: linux-pm/drivers/pci/pci.c
===================================================================
--- linux-pm.orig/drivers/pci/pci.c
+++ linux-pm/drivers/pci/pci.c
@@ -988,7 +988,7 @@ static inline bool platform_pci_power_ma
if (pci_use_mid_pm())
return true;
- return pci_platform_pm ? pci_platform_pm->is_manageable(dev) : false;
+ return acpi_pci_power_manageable(dev);
}
static inline int platform_pci_set_power_state(struct pci_dev *dev,
@@ -997,7 +997,7 @@ static inline int platform_pci_set_power
if (pci_use_mid_pm())
return mid_pci_set_power_state(dev, t);
- return pci_platform_pm ? pci_platform_pm->set_state(dev, t) : -ENOSYS;
+ return acpi_pci_set_power_state(dev, t);
}
static inline pci_power_t platform_pci_get_power_state(struct pci_dev *dev)
@@ -1005,13 +1005,13 @@ static inline pci_power_t platform_pci_g
if (pci_use_mid_pm())
return mid_pci_get_power_state(dev);
- return pci_platform_pm ? pci_platform_pm->get_state(dev) : PCI_UNKNOWN;
+ return acpi_pci_get_power_state(dev);
}
static inline void platform_pci_refresh_power_state(struct pci_dev *dev)
{
- if (!pci_use_mid_pm() && pci_platform_pm && pci_platform_pm->refresh_state)
- pci_platform_pm->refresh_state(dev);
+ if (!pci_use_mid_pm())
+ acpi_pci_refresh_power_state(dev);
}
static inline pci_power_t platform_pci_choose_state(struct pci_dev *dev)
@@ -1019,8 +1019,7 @@ static inline pci_power_t platform_pci_c
if (pci_use_mid_pm())
return PCI_POWER_ERROR;
- return pci_platform_pm ?
- pci_platform_pm->choose_state(dev) : PCI_POWER_ERROR;
+ return acpi_pci_choose_state(dev);
}
static inline int platform_pci_set_wakeup(struct pci_dev *dev, bool enable)
@@ -1028,8 +1027,7 @@ static inline int platform_pci_set_wakeu
if (pci_use_mid_pm())
return PCI_POWER_ERROR;
- return pci_platform_pm ?
- pci_platform_pm->set_wakeup(dev, enable) : -ENODEV;
+ return acpi_pci_wakeup(dev, enable);
}
static inline bool platform_pci_need_resume(struct pci_dev *dev)
@@ -1037,7 +1035,7 @@ static inline bool platform_pci_need_res
if (pci_use_mid_pm())
return false;
- return pci_platform_pm ? pci_platform_pm->need_resume(dev) : false;
+ return acpi_pci_need_resume(dev);
}
static inline bool platform_pci_bridge_d3(struct pci_dev *dev)
@@ -1045,9 +1043,7 @@ static inline bool platform_pci_bridge_d
if (pci_use_mid_pm())
return false;
- if (pci_platform_pm && pci_platform_pm->bridge_d3)
- return pci_platform_pm->bridge_d3(dev);
- return false;
+ return acpi_pci_bridge_d3(dev);
}
/**