Re: [PATCH v3 00/10] arm64: dts: qcom: sc8280xp: PCIe fixes and GICv3 ITS enable

[Manivannan Sadhasivam]

On Tue, Mar 05, 2024 at 09:10:55AM +0100, Johan Hovold wrote:
> This series addresses a few problems with the sc8280xp PCIe
> implementation.
> 
> The DWC PCIe controller can either use its internal MSI controller or an
> external one such as the GICv3 ITS. Enabling the latter allows for
> assigning affinity to individual interrupts, but results in a large
> amount of Correctable Errors being logged on both the Lenovo ThinkPad
> X13s and the sc8280xp-crd reference design.
> 
> It turns out that these errors are always generated, but for some yet to
> be determined reason, the AER interrupts are never received when using
> the internal MSI controller, which makes the link errors harder to
> notice.
> 
> On the X13s, there is a large number of errors generated when bringing
> up the link on boot. This is related to the fact that UEFI firmware has
> already enabled the Wi-Fi PCIe link at Gen2 speed and restarting the
> link at Gen3 generates a massive amount of errors until the Wi-Fi
> firmware is restarted. This has now also been shown to cause the Wi-Fi
> to sometimes not start at all on boot for some users.
> 
> A recent commit enabling ASPM on certain Qualcomm platforms introduced
> further errors when using the Wi-Fi on the X13s as well as when
> accessing the NVMe on the CRD. The exact reason for this has not yet
> been identified, but disabling ASPM L0s makes the errors go away. This
> could suggest that either the current ASPM implementation is incomplete
> or that L0s is not supported with these devices.
> 
> Note that the X13s and CRD use the same Wi-Fi controller, but the errors
> are only generated on the X13s. The NVMe controller on my X13s does not
> support L0s so there are no issues there, unlike on the CRD which uses a
> different controller. The modem on the CRD does not generate any errors,
> but both the NVMe and modem keeps bouncing in and out of L0s/L1 also
> when not used, which could indicate that there are bigger problems with
> the ASPM implementation. I don't have a modem on my X13s so I have not
> been able to test whether L0s causes any trouble there.
> 
> Enabling AER error reporting on sc8280xp could similarly also reveal
> existing problems with the related sa8295p and sa8540p platforms as they
> share the base dtsi.
> 
> After discussing this with Bjorn Andersson at Qualcomm we have decided
> to go ahead and disable L0s for all controllers on the CRD and the
> X13s.
> 

Just received confirmation from Qcom that L0s is not supported for any of the
PCIe instances in sc8280xp (and its derivatives). Please move the property to
SoC dtsi.

- Mani

> Note that disabling ASPM L0s for the X13s Wi-Fi does not seem to have a
> significant impact on the power consumption (and there are indications
> that this applies generally for L0s on these platforms).
> 
> ***
> 
> As we are now at 6.8-rc7, I've rebased this series on the Qualcomm PCIe
> binding rework in linux-next so that the whole series can be merged for
> 6.9 (the 'aspm-no-l0s' support and devicetree fixes are all marked for
> stable backport anyway).
> 
> The DT bindings and PCI patch are expected to go through the PCI tree,
> while Bjorn A takes the devicetree updates through the Qualcomm tree.
> 
> Johan
> 
> 
> Changes in v3
>  - drop the two wifi link speed patches which have been picked up for
>    6.8
>  - rebase on binding rework in linux-next and add the properties also to
>    the new qcom,pcie-common.yaml
>    - https://lore.kernel.org/linux-pci/20240126-dt-bindings-pci-qcom-split-v3-0-f23cda4d74c0@xxxxxxxxxx/
>  - fix an 'L0s' typo in one commit message
> 
> Changes in v2
>  - drop RFC from ASPM patches and add stable tags
>  - reorder patches and move ITS patch last
>  - fix s/GB/MB/ typo in Gen2 speed commit messages
>  - fix an incorrect Fixes tag
>  - amend commit message X13 wifi link speed patch after user
>    confirmation that this fixes the wifi startup issue
>  - disable L0s also for modem and wifi on CRD
>  - disable L0s also for nvme and modem on X13s
> 
> 
> Johan Hovold (10):
>   dt-bindings: PCI: qcom: Allow 'required-opps'
>   dt-bindings: PCI: qcom: Do not require 'msi-map-mask'
>   dt-bindings: PCI: qcom: Allow 'aspm-no-l0s'
>   PCI: qcom: Add support for disabling ASPM L0s in devicetree
>   arm64: dts: qcom: sc8280xp: add missing PCIe minimum OPP
>   arm64: dts: qcom: sc8280xp-crd: disable ASPM L0s for NVMe
>   arm64: dts: qcom: sc8280xp-crd: disable ASPM L0s for modem and Wi-Fi
>   arm64: dts: qcom: sc8280xp-x13s: disable ASPM L0s for Wi-Fi
>   arm64: dts: qcom: sc8280xp-x13s: disable ASPM L0s for NVMe and modem
>   arm64: dts: qcom: sc8280xp: enable GICv3 ITS for PCIe
> 
>  .../bindings/pci/qcom,pcie-common.yaml        |  6 +++++-
>  .../devicetree/bindings/pci/qcom,pcie.yaml    |  6 +++++-
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts     |  5 +++++
>  .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    |  5 +++++
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi        | 17 +++++++++++++++-
>  drivers/pci/controller/dwc/pcie-qcom.c        | 20 +++++++++++++++++++
>  6 files changed, 56 insertions(+), 3 deletions(-)
> 
> -- 
> 2.43.0
> 

-- 
மணிவண்ணன் சதாசிவம்