| Message ID | 20241126151711.v5.1.Id0a0e78ab0421b6bce51c4b0b87e6aebdfc69ec7@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms | expand |
On Tue, Nov 26, 2024 at 03:17:11PM -0800, Brian Norris wrote: > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Unlike ACPI based platforms, there are no known issues with D3Hot for > the PCI bridges in Device Tree based platforms. Past discussions (Link > [1]) determined the restrictions around D3 should be relaxed for all > Device Tree systems. So let's allow the PCI bridges to go to D3Hot > during runtime. > > To match devm_pci_alloc_host_bridge() -> devm_of_pci_bridge_init(), we > look at the host bridge's parent when determining whether this is a > Device Tree based platform. Not all bridges have their own node, but the > parent (controller) should. > > Link: https://lore.kernel.org/linux-pci/20240227225442.GA249898@bhelgaas/ [1] > Link: https://lore.kernel.org/linux-pci/20240828210705.GA37859@bhelgaas/ [2] > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > [Brian: look at host bridge's parent, not bridge node; rewrite > description] > Signed-off-by: Brian Norris <briannorris@chromium.org> Thanks for picking it up! - Mani > --- > Based on prior work by Manivannan Sadhasivam that was part of a bigger > series that stalled: > > [PATCH v5 4/4] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms > https://lore.kernel.org/linux-pci/20240802-pci-bridge-d3-v5-4-2426dd9e8e27@linaro.org/ > > I'm resubmitting this single patch, since it's useful and seemingly had > agreement. I massaged it a bit to relax some restrictions on how the > Device Tree should look. > > Changes in v5: > - Pulled out of the larger series, as there were more controversial > changes in there, while this one had agreement (Link [2]). > - Rewritten with a relaxed set of rules, because the above patch > required us to modify many device trees to add bridge nodes. > > drivers/pci/pci.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index e278861684bc..5d898f5ea155 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3018,6 +3018,8 @@ static const struct dmi_system_id bridge_d3_blacklist[] = { > */ > bool pci_bridge_d3_possible(struct pci_dev *bridge) > { > + struct pci_host_bridge *host_bridge; > + > if (!pci_is_pcie(bridge)) > return false; > > @@ -3038,6 +3040,15 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > if (pci_bridge_d3_force) > return true; > > + /* > + * Allow D3 for all Device Tree based systems. We assume a host > + * bridge's parent will have a device node, even if this bridge > + * may not have its own. > + */ > + host_bridge = pci_find_host_bridge(bridge->bus); > + if (dev_of_node(host_bridge->dev.parent)) > + return true; > + > /* Even the oldest 2010 Thunderbolt controller supports D3. */ > if (bridge->is_thunderbolt) > return true; > -- > 2.47.0.338 >
Hi Bjorn, On Tue, Nov 26, 2024 at 03:17:11PM -0800, Brian Norris wrote: > From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Unlike ACPI based platforms, there are no known issues with D3Hot for > the PCI bridges in Device Tree based platforms. Past discussions (Link > [1]) determined the restrictions around D3 should be relaxed for all > Device Tree systems. So let's allow the PCI bridges to go to D3Hot > during runtime. > > To match devm_pci_alloc_host_bridge() -> devm_of_pci_bridge_init(), we > look at the host bridge's parent when determining whether this is a > Device Tree based platform. Not all bridges have their own node, but the > parent (controller) should. > > Link: https://lore.kernel.org/linux-pci/20240227225442.GA249898@bhelgaas/ [1] > Link: https://lore.kernel.org/linux-pci/20240828210705.GA37859@bhelgaas/ [2] > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > [Brian: look at host bridge's parent, not bridge node; rewrite > description] > Signed-off-by: Brian Norris <briannorris@chromium.org> > --- > Based on prior work by Manivannan Sadhasivam that was part of a bigger > series that stalled: > > [PATCH v5 4/4] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms > https://lore.kernel.org/linux-pci/20240802-pci-bridge-d3-v5-4-2426dd9e8e27@linaro.org/ > > I'm resubmitting this single patch, since it's useful and seemingly had > agreement. I massaged it a bit to relax some restrictions on how the > Device Tree should look. > > Changes in v5: > - Pulled out of the larger series, as there were more controversial > changes in there, while this one had agreement (Link [2]). > - Rewritten with a relaxed set of rules, because the above patch > required us to modify many device trees to add bridge nodes. I'm wondering if you have any thoughts on this. Manivannan seemed happy with this in his reply. I'd like to see this land in mainline, so I can avoid the hacks that everyone seems to be picking up (such as adding "pcie_port_pm=force" to their command lines). (While I'm at it ... apologies for the poor versioning. The subject here should probably be "v6", since I'm clearly quoting above that the prior version from Manivannan was v5.) Thanks, Brian
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index e278861684bc..5d898f5ea155 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3018,6 +3018,8 @@ static const struct dmi_system_id bridge_d3_blacklist[] = { */ bool pci_bridge_d3_possible(struct pci_dev *bridge) { + struct pci_host_bridge *host_bridge; + if (!pci_is_pcie(bridge)) return false; @@ -3038,6 +3040,15 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) if (pci_bridge_d3_force) return true; + /* + * Allow D3 for all Device Tree based systems. We assume a host + * bridge's parent will have a device node, even if this bridge + * may not have its own. + */ + host_bridge = pci_find_host_bridge(bridge->bus); + if (dev_of_node(host_bridge->dev.parent)) + return true; + /* Even the oldest 2010 Thunderbolt controller supports D3. */ if (bridge->is_thunderbolt) return true;