Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

schemas: pci: bridge: Document "supports-d3" property #127

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

Mani-Sadhasivam
Copy link

There is no standard way to detect the D3 support of the PCI-PCI bridges. So let's add a new property, "supports-d3" to specify that the bridge supports transitioning to D3 states.

@l1k
Copy link

l1k commented Feb 3, 2024

There's a reason D3 support cannot be determined through the Power Management Control/Status Register (unlike D1 and D2, which can): It's mandatory for all functions per PCIe r6.1 sec 5.3.1:

All Functions must support the D0 and D3 states (both D3Hot and D3Cold). The D1 and D2 states are optional.

Unfortunately in practice, for PCIe bridges, D3 is not always supported.

Please add the above to the commit message.

Even though the PCIe spec states that all PCIe devices should support D0 and D3
states (both D3 hot and D3 cold), PCIe bridges do not follow that in practice.

Spec r3.0, section 5.3.1:

"All Functions must support the D0 and D3 states (both D3 hot and D3 cold ).
The D1 and D2 states are optional."

Also, unlike D1 and D2, D3 support cannot be determined using the standard
Power Management Control/Status (PMCSR) Register. So the OS needs to know the
D3 support using platform specific ways. Hence, for DT based platforms, let's
add a new property called "supports-d3" that specifies that the PCI bridge is
D3 capable.

Signed-off-by: Manivannan Sadhasivam <[email protected]>
@Mani-Sadhasivam
Copy link
Author

@l1k Done!

@robherring
Copy link
Member

If the default in the spec is supported, then the property should be to disable it.

@Mani-Sadhasivam
Copy link
Author

If the default in the spec is supported, then the property should be to disable it.

Spec supports it by default, but not all the bridges. That's why the property has to enable it.

@robherring
Copy link
Member

If the default in the spec is supported, then the property should be to disable it.

Spec supports it by default, but not all the bridges. That's why the property has to enable it.

No. Default is D3 supported because the spec says it should be supported, so a property should be a quirk to disable it.

@Mani-Sadhasivam
Copy link
Author

No. Default is D3 supported because the spec says it should be supported, so a property should be a quirk to disable it.

That wouldn't accurately describe the HW. As you know, PCIe devices have a history of violating specs, so if we add the property based on the spec, then there would be a disconnect w.r.t the implementation. For instance, if we name it disable-d3, then it implies that the bridges that don't have this property are all supporting D3, but in reality, they aren't. And trying to enable D3, will break the bridges that don't support it.

@robherring
Copy link
Member

What's the behavior for a PCI bridge with no firmware node? D3 is disabled? Does this go back to the long standing problem that runtime PM is disabled for PCI?

@Mani-Sadhasivam
Copy link
Author

Yes, D3 is disabled for the bridge by default. The goal here is to safely enable D3 for capable bridges.

@robherring robherring merged commit 4548397 into devicetree-org:main Feb 13, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants