On Fri, Sep 09, 2022 at 01:11:54PM +0530, Lazar, Lijo wrote:
On 9/8/2022 11:27 PM, Bjorn Helgaas wrote:
On Thu, Sep 08, 2022 at 04:42:38PM +0000, Lazar, Lijo wrote:
I am not sure if ASPM settings can be generalized by PCIE core. Performance vs Power savings when ASPM is enabled will require some additional tuning and that will be device specific.
Can you elaborate on this? In the universe of drivers, very few do their own ASPM configuration, and it's usually to work around hardware defects, e.g., L1 doesn't work on some e1000e devices, L0s doesn't work on some iwlwifi devices, etc.
The core does know how to configure all the ASPM features defined in the PCIe spec, e.g., L0s, L1, L1.1, L1.2, and LTR.
In some of the other ASICs, this programming is done in VBIOS/SBIOS firmware. Having it in driver provides the advantage of additional tuning without forcing a VBIOS upgrade.
I think it's clearly the intent of the PCIe spec that ASPM configuration be done by generic code. Here are some things that require a system-level view, not just an individual device view:
L0s, L1, and L1 Substates cannot be enabled unless both ends support it (PCIe r6.0, secs 5.4.1.4, 7.5.3.7, 5.5.4).
Devices advertise the "Acceptable Latency" they can accept for transitions from L0s or L1 to L0, and the actual latency depends on the "Exit Latencies" of all the devices in the path to the Root Port (sec 5.4.1.3.2).
LTR (required by L1.2) cannot be enabled unless it is already enabled in all upstream devices (sec 6.18). This patch relies on "ltr_path", which works now but relies on the PCI core never reconfiguring the upstream path.
There might be amdgpu-specific features the driver needs to set up, but if drivers fiddle with architected features like LTR behind the PCI core's back, things are likely to break.
The programming is mostly related to entry conditions and spec leaves it to implementation.
From r4.0 spec - " This specification does not dictate when a component with an Upstream Port must initiate a transition to the L1 state. The interoperable mechanisms for transitioning into and out of L1 are defined within this specification; however, the specific ASPM policy governing when to transition into L1 is left to the implementer. ... Another approach would be for the Downstream device to initiate a transition to the L1 state once the Link has been idle in L0 for a set amount of time. "
Some of the programming like below relates to timings for entry.
def = data = RREG32_SOC15(NBIO, 0, regRCC_STRAP0_RCC_BIF_STRAP3); data |= 0x5DE0 <<RCC_BIF_STRAP3__STRAP_VLINK_ASPM_IDLE_TIMER__SHIFT; data |= 0x0010 << RCC_BIF_STRAP3__STRAP_VLINK_PM_L1_ENTRY_TIMER__SHIFT; if (def != data) WREG32_SOC15(NBIO, 0, regRCC_STRAP0_RCC_BIF_STRAP3, data);
Similarly for LTR, as it provides a dynamic mechanism to report tolerance while in L1 substates, the tolerance timings can be tuned through registers though there is a threshold.
I don't object to the driver programming device-specific things, although there might be issues if it does that after the core has already configured and enabled ASPM -- the driver might need to temporarily disable ASPM while it updates parameters, then re-enable it.
I *do* object to the driver programming PCIe-generic things that the PCI core thinks it owns. It's especially annoying if the driver uses device-specific #defines and access methods for generic PCIe things because then we can't even find potential conflicts.
From: Alex Deucher alexdeucher@gmail.com On Thu, Sep 8, 2022 at 12:12 PM Bjorn Helgaas helgaas@kernel.org wrote:
Do you know why the driver configures ASPM itself? If the PCI core is doing something wrong (and I'm sure it is, ASPM support is kind of a mess), I'd much prefer to fix up the core where *all* drivers can benefit from it.
This is the programming sequence we get from our hardware team and it is used on both windows and Linux. As far as I understand it windows doesn't handle this in the core, it's up to the individual drivers to enable it. I'm not familiar with how this should be enabled generically, but at least for our hardware, it seems to have some variation compared to what is done in the PCI core due to stability, etc. It seems to me that this may need asic specific implementations for a lot of hardware depending on the required programming sequences. E.g., various asics may need hardware workaround for bugs or platform issues, etc. I can ask for more details from our hardware team.
If the PCI core has stability issues, I want to fix them. This hardware may have its own stability issues, and I would ideally like to have drivers use interfaces like pci_disable_link_state() to avoid broken things. Maybe we need new interfaces for more subtle kinds of breakage.
Bjorn