RE: [PATCH v1 0/2] PCI/ASPM: Refine L1 PM Substates support

From: Chen, Kenji
Date: Tue Dec 05 2017 - 20:55:48 EST


https://pcisig.com/sites/default/files/specification_documents/ECN_L1_PM_Substates_with_CLKREQ_31_May_2013_Rev10a.pdf
Pls also check My Intel Doc# 545845 for the bits fields description.

-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
Sent: Wednesday, December 6, 2017 3:41 AM
To: linux-pci@xxxxxxxxxxxxxxx
Cc: Chen, Kenji <kenji.chen@xxxxxxxxx>; Thierry Reding <treding@xxxxxxxxxx>; Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>; David Daney <david.daney@xxxxxxxxxx>; Krishna Kishore <kthota@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; Vidya Sagar <vidyas@xxxxxxxxxx>; Julia Lawall <Julia.Lawall@xxxxxxx>; linux-tegra@xxxxxxxxxxxxxxx; Patrick Georgi <pgeorgi@xxxxxxxxxxxx>; Rajat Jain <rajatja@xxxxxxxxxx>; Yinghai Lu <yinghai@xxxxxxxxxx>; Stefan Reinauer <stefan.reinauer@xxxxxxxxxxxx>; Duncan Laurie <dlaurie@xxxxxxxxxxxx>
Subject: Re: [PATCH v1 0/2] PCI/ASPM: Refine L1 PM Substates support

On Sat, Dec 02, 2017 at 03:45:38PM -0600, Bjorn Helgaas wrote:
> The PCIe active link power state is L0. ASPM defines two low-power
> states: L0s and L1. The L1 PM Substates feature defines two
> additional low-power states: L1.1 and L2.2.
>
> The L1.2 state may have substantial entry/exit latency. Downstream
> devices can use the Latency Tolerance Reporting (LTR) feature to
> report how much latency they can tolerate. The L1 PM Substates
> capability includes a programmable L1.2 threshold register. If the
> downstream devices can tolerate the latency indicated by the
> threshold, L1.2 may be used.
>
> If LTR is not enabled, the L1.2 threshold is ignored and L1.2 can be
> used whenever it is enabled and CLKREQ# is de-asserted. Linux
> currently never enables LTR, but firmware may have done so.
>
> The current L1 PM substates support in Linux sets the L1.2 threshold
> to a fixed value (163.84us) copied from coreboot [1]. I don't know
> where that value came from, and it is incorrect for some platforms,
> e.g., Tegra [2].
>
> These patches change that so we calculate the L1.2 threshold based on
> values reported by the hardware, and we enable LTR whenever possible.
>
> This is all just my understanding from reading the spec. I'd be glad
> to be corrected if I'm going wrong.
>
> I'm particularly puzzled about the coreboot code in
> pciexp_L1_substate_commit() that sets LTR_L1.2_THRESHOLD:
>
> + pcie_update_cfg(root, root_cap + 0x08, ~0xe3ff0000,
> + (1 << 21) | (1 << 23) | (1 << 30));
>
> This is writing the L1 PM Substates Control 1 register, but the shifts
> don't match up with any of the fields in the register, so this is an
> awfully funny way to write the threshold.
>
> [1]
> https://mail.coreboot.org/pipermail/coreboot-gerrit/2015-March/021134.
> html [2]
> https://lkml.kernel.org/r/1510492674-12786-1-git-send-email-vidyas@nvi
> dia.com
>
> ---
>
> Bjorn Helgaas (2):
> PCI/ASPM: Calculate LTR_L1.2_THRESHOLD from device characteristics
> PCI/ASPM: Enable Latency Tolerance Reporting when supported
>
>
> drivers/pci/pcie/aspm.c | 71 +++++++++++++++++++++++++++++++----------------
> drivers/pci/probe.c | 33 ++++++++++++++++++++++
> include/linux/pci.h | 2 +
> 3 files changed, 82 insertions(+), 24 deletions(-)

I applied these with Vidya's Reviewed-by to pci/aspm for v4.16.

I'd still really like to hear from the coreboot folks about the rationale for the current coreboot code. You folks are in a much better position to actually understand the hardware details.

Bjorn