On 12/12/2023 13:25, Rafael J. Wysocki wrote:
On Mon, Dec 4, 2023 at 7:07 AM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
If pci_d3cold_enable() or pci_d3cold_disable() is called on a root
port it is ignored because there is no upstream bridge.
The kerneldoc comment of pci_bridge_d3_update() explains what that
function is for which also covers why it does not take effect when
called on root ports.
I'm sorry but can you clarify the intent of your comment?
Are you suggesting we should introduce a different function/logic for
root ports, kernel doc should be updated, or root ports should be
special cased in that function?
If called on a root port, use `no_d3cold` variable to decide policy
It is unclear that this is about pci_bridge_d3_possible() which
applies to both D3hot and D3cold, not just D3cold AFAICS. I don't
think that no_d3cold should affect the D3hot behavior.
IMO the semantics are confusing depending upon what device you called
pci_d3cold_disable()/pci_d3cold_enable() with as an argument.
Both devices and root ports are used by existing driver in the kernel.
If you called pci_d3cold_disable() with a device, that actually prevents
the /bridge above it/ from going to D3hot as well (bridge_d3 is set to
the result)
and also immediately refresh whether D3 is possible.
Which isn't correct AFAICS.
Why?
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/pci/pci.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 72505794cc72..3d4aaecda457 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3023,6 +3023,9 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
if (pci_bridge_d3_disable)
return false;
+ if (bridge->no_d3cold)
+ return false;
+
/*
* Hotplug ports handled by firmware in System Management Mode
* may not be put into D3 by the OS (Thunderbolt on non-Macs).
@@ -3098,7 +3101,11 @@ void pci_bridge_d3_update(struct pci_dev *dev)
bool d3cold_ok = true;
bridge = pci_upstream_bridge(dev);
- if (!bridge || !pci_bridge_d3_possible(bridge))
+ if (!bridge) {
+ dev->bridge_d3 = pci_bridge_d3_possible(dev);
+ return;
+ }
+ if (!pci_bridge_d3_possible(bridge))
return;
/*
--
2.34.1
