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.
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/[email protected]/
- 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
Whether the 'msi-map-mask' property is needed or not depends on how the
MSI interrupts are mapped and it should therefore not be described as
required.
Note that the current schema fails to detect omissions of the mask
property if the internal MSI controller properties are also present.
Acked-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml | 1 -
Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 1 -
2 files changed, 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
index 8d570669650a..0d1b23523f62 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
@@ -93,7 +93,6 @@ anyOf:
- "#interrupt-cells"
- required:
- msi-map
- - msi-map-mask
allOf:
- $ref: /schemas/pci/pci-bus.yaml#
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index 1d7a6a520fef..efaab5f82b47 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -134,7 +134,6 @@ anyOf:
- "#interrupt-cells"
- required:
- msi-map
- - msi-map-mask
allOf:
- $ref: /schemas/pci/pci-bus.yaml#
--
2.43.0
There are indications that ASPM L0s is not working very well on this
machine so disable it also for the modem and Wi-Fi controllers for now.
This specifically avoids having the modem and Wi-Fi controllers bounce
in an out of L0s when not used (the modem still bounces in and out of
L1) as well as intermittent Correctable errors on the Wi-Fi link when
not used.
Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: [email protected] # 6.7
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 7e94a68d5d9f..8fc0380f65a0 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -546,6 +546,8 @@ &pcie2a_phy {
};
&pcie3a {
+ aspm-no-l0s;
+
perst-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
@@ -566,6 +568,7 @@ &pcie3a_phy {
&pcie4 {
max-link-speed = <2>;
+ aspm-no-l0s;
perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
--
2.43.0
Add the missing PCIe CX performance level votes to avoid relying on
other drivers (e.g. USB or UFS) to maintain the nominal performance
level required for Gen3 speeds.
Fixes: 813e83157001 ("arm64: dts: qcom: sc8280xp/sa8540p: add PCIe2-4 nodes")
Cc: [email protected] # 6.2
Reviewed-by: Konrad Dybcio <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index c8e84c53935c..424d143ee26a 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1780,6 +1780,7 @@ pcie4: pcie@1c00000 {
reset-names = "pci";
power-domains = <&gcc PCIE_4_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
phys = <&pcie4_phy>;
phy-names = "pciephy";
@@ -1878,6 +1879,7 @@ pcie3b: pcie@1c08000 {
reset-names = "pci";
power-domains = <&gcc PCIE_3B_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
phys = <&pcie3b_phy>;
phy-names = "pciephy";
@@ -1976,6 +1978,7 @@ pcie3a: pcie@1c10000 {
reset-names = "pci";
power-domains = <&gcc PCIE_3A_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
phys = <&pcie3a_phy>;
phy-names = "pciephy";
@@ -2077,6 +2080,7 @@ pcie2b: pcie@1c18000 {
reset-names = "pci";
power-domains = <&gcc PCIE_2B_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
phys = <&pcie2b_phy>;
phy-names = "pciephy";
@@ -2175,6 +2179,7 @@ pcie2a: pcie@1c20000 {
reset-names = "pci";
power-domains = <&gcc PCIE_2A_GDSC>;
+ required-opps = <&rpmhpd_opp_nom>;
phys = <&pcie2a_phy>;
phy-names = "pciephy";
--
2.43.0
There are indications that ASPM L0s is not working very well on this
machine so disable it also for the NVMe and modem controllers for now.
Note that this is done as a precaution based on problems with the Wi-Fi
on the X13s as well as the NVMe, modem and Wi-Fi on the sc8280xp-crd
reference design (the NVMe controller on my X13s does not support L0s
and the machine lacks a modem).
Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: [email protected] # 6.7
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 9567b82db9a5..057e4d9d3c0f 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -695,6 +695,8 @@ keyboard@68 {
};
&pcie2a {
+ aspm-no-l0s;
+
perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
@@ -714,6 +716,8 @@ &pcie2a_phy {
};
&pcie3a {
+ aspm-no-l0s;
+
perst-gpios = <&tlmm 151 GPIO_ACTIVE_LOW>;
wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
--
2.43.0
Some Qualcomm SoCs require a minimum performance level for the power
domain so add 'required-opps' to the binding.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml | 3 +++
Documentation/devicetree/bindings/pci/qcom,pcie.yaml | 3 +++
2 files changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
index 125136176f93..8d570669650a 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie-common.yaml
@@ -59,6 +59,9 @@ properties:
power-domains:
maxItems: 1
+ required-opps:
+ maxItems: 1
+
resets:
minItems: 1
maxItems: 12
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index c8f36978a94c..1d7a6a520fef 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -111,6 +111,9 @@ properties:
description: GPIO controlled connection to PERST# signal
maxItems: 1
+ required-opps:
+ maxItems: 1
+
wake-gpios:
description: GPIO controlled connection to WAKE# signal
maxItems: 1
--
2.43.0
The DWC PCIe controller can be used with its internal MSI controller or
with an external one such as the GICv3 Interrupt Translation Service
(ITS).
Add the msi-map properties needed to use the GIC ITS. This will also
make Linux switch to the ITS implementation, which allows for assigning
affinity to individual MSIs.
Note that using the GIC ITS on SC8280XP will cause Advanced Error
Reporting (AER) interrupts to be received on errors unlike when using
the internal MSI controller. This will specifically lead to
notifications about Correctable Errors being logged for the Wi-Fi
controller on the Lenovo ThinkPad X13s when ASPM L0s is enabled.
Suggested-by: Manivannan Sadhasivam <[email protected]>
Reviewed-by: Konrad Dybcio <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 424d143ee26a..4708ba28c4d5 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1737,6 +1737,8 @@ pcie4: pcie@1c00000 {
linux,pci-domain = <6>;
num-lanes = <1>;
+ msi-map = <0x0 &its 0xe0000 0x10000>;
+
interrupts = <GIC_SPI 141 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>,
@@ -1838,6 +1840,8 @@ pcie3b: pcie@1c08000 {
linux,pci-domain = <5>;
num-lanes = <2>;
+ msi-map = <0x0 &its 0xd0000 0x10000>;
+
interrupts = <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 146 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 147 IRQ_TYPE_LEVEL_HIGH>,
@@ -1937,6 +1941,8 @@ pcie3a: pcie@1c10000 {
linux,pci-domain = <4>;
num-lanes = <4>;
+ msi-map = <0x0 &its 0xc0000 0x10000>;
+
interrupts = <GIC_SPI 312 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 313 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 314 IRQ_TYPE_LEVEL_HIGH>,
@@ -2039,6 +2045,8 @@ pcie2b: pcie@1c18000 {
linux,pci-domain = <3>;
num-lanes = <2>;
+ msi-map = <0x0 &its 0xb0000 0x10000>;
+
interrupts = <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
@@ -2138,6 +2146,8 @@ pcie2a: pcie@1c20000 {
linux,pci-domain = <2>;
num-lanes = <4>;
+ msi-map = <0x0 &its 0xa0000 0x10000>;
+
interrupts = <GIC_SPI 86 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 523 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 524 IRQ_TYPE_LEVEL_HIGH>,
@@ -4342,7 +4352,7 @@ intc: interrupt-controller@17a00000 {
#size-cells = <2>;
ranges;
- msi-controller@17a40000 {
+ its: msi-controller@17a40000 {
compatible = "arm,gic-v3-its";
reg = <0 0x17a40000 0 0x20000>;
msi-controller;
--
2.43.0
Enabling ASPM L0s on the Lenovo Thinkpad X13s results in Correctable
Errors (BadTLP, Timeout) when accessing the Wi-Fi controller so disable
it for now.
Fixes: 9f4f3dfad8cf ("PCI: qcom: Enable ASPM for platforms supporting 1.9.0 ops")
Cc: [email protected] # 6.7
Reviewed-by: Manivannan Sadhasivam <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index eb8a16aa233e..9567b82db9a5 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -734,6 +734,7 @@ &pcie3a_phy {
&pcie4 {
max-link-speed = <2>;
+ aspm-no-l0s;
perst-gpios = <&tlmm 141 GPIO_ACTIVE_LOW>;
wake-gpios = <&tlmm 139 GPIO_ACTIVE_LOW>;
--
2.43.0
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/[email protected]/
> - 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
>
--
மணிவண்ணன் சதாசிவம்
On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
> 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.
> > 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.
Ok, thanks for confirming. But then the devicetree property is not the
right way to handle this, and we should disable L0s based on the
compatible string instead.
> > 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).
I'll respin the series. Looks like we've already missed the chance to
enable ITS in 6.9 anyway.
Johan
On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
> > 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.
>
> > > 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.
>
> Ok, thanks for confirming. But then the devicetree property is not the
> right way to handle this, and we should disable L0s based on the
> compatible string instead.
>
Hmm. I checked further and got the info that there is no change in the IP, but
the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
there will be AERs when L0s is enabled on any controller instance. And there
will be no updated PHY sequence in the future also for this chipset.
So yeah, let's disable it in the driver instead.
> > > 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).
>
> I'll respin the series. Looks like we've already missed the chance to
> enable ITS in 6.9 anyway.
>
Sounds good, thanks!
- Mani
--
மணிவண்ணன் சதாசிவம்
On Wed, 6 Mar 2024 at 10:39, Manivannan Sadhasivam
<[email protected]> wrote:
>
> On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> > On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
> > > 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.
> >
> > > > 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.
> >
> > Ok, thanks for confirming. But then the devicetree property is not the
> > right way to handle this, and we should disable L0s based on the
> > compatible string instead.
> >
>
> Hmm. I checked further and got the info that there is no change in the IP, but
> the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
> there will be AERs when L0s is enabled on any controller instance. And there
> will be no updated PHY sequence in the future also for this chipset.
Why? If it is a bug in the PHY driver, it should be fixed there
instead of adding workarounds.
>
> So yeah, let's disable it in the driver instead.
>
> > > > 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).
> >
> > I'll respin the series. Looks like we've already missed the chance to
> > enable ITS in 6.9 anyway.
> >
>
> Sounds good, thanks!
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
>
--
With best wishes
Dmitry
On Wed, Mar 06, 2024 at 10:48:30AM +0200, Dmitry Baryshkov wrote:
> On Wed, 6 Mar 2024 at 10:39, Manivannan Sadhasivam
> <[email protected]> wrote:
> > On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> > > On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
> > > > 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.
> > > Ok, thanks for confirming. But then the devicetree property is not the
> > > right way to handle this, and we should disable L0s based on the
> > > compatible string instead.
> > Hmm. I checked further and got the info that there is no change in the IP, but
> > the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
> > there will be AERs when L0s is enabled on any controller instance. And there
> > will be no updated PHY sequence in the future also for this chipset.
>
> Why? If it is a bug in the PHY driver, it should be fixed there
> instead of adding workarounds.
ASPM L0s is currently broken on these platforms and, as far as I
understand, both under Windows and Linux. Since Qualcomm hasn't been
able to come up with the necessary PHY init sequences for these
platforms yet, I doubt they will suddenly appear in the near future.
So we need to disable L0s for now. If an updated PHY init sequence later
appears, we can always enable it again.
> > So yeah, let's disable it in the driver instead.
Johan
On Wed, Mar 06, 2024 at 10:12:31AM +0100, Johan Hovold wrote:
> On Wed, Mar 06, 2024 at 10:48:30AM +0200, Dmitry Baryshkov wrote:
> > On Wed, 6 Mar 2024 at 10:39, Manivannan Sadhasivam
> > <[email protected]> wrote:
> > > On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> > > > On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
>
> > > > > 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.
>
> > > > Ok, thanks for confirming. But then the devicetree property is not the
> > > > right way to handle this, and we should disable L0s based on the
> > > > compatible string instead.
>
> > > Hmm. I checked further and got the info that there is no change in the IP, but
> > > the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
> > > there will be AERs when L0s is enabled on any controller instance. And there
> > > will be no updated PHY sequence in the future also for this chipset.
> >
> > Why? If it is a bug in the PHY driver, it should be fixed there
> > instead of adding workarounds.
>
> ASPM L0s is currently broken on these platforms and, as far as I
> understand, both under Windows and Linux. Since Qualcomm hasn't been
> able to come up with the necessary PHY init sequences for these
> platforms yet, I doubt they will suddenly appear in the near future.
>
> So we need to disable L0s for now. If an updated PHY init sequence later
> appears, we can always enable it again.
>
It could be the same case for all 'non-mobile' chipsets (automotive, compute,
modem). So instead of using the compatible, please add a flag and set that for
all non-mobile SoCs. Like the ones starting with SAxxx, SCxxx, SDxxx.
- Mani
--
மணிவண்ணன் சதாசிவம்
On Wed, 6 Mar 2024 at 11:12, Johan Hovold <[email protected]> wrote:
>
> On Wed, Mar 06, 2024 at 10:48:30AM +0200, Dmitry Baryshkov wrote:
> > On Wed, 6 Mar 2024 at 10:39, Manivannan Sadhasivam
> > <[email protected]> wrote:
> > > On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> > > > On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
>
> > > > > 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.
>
> > > > Ok, thanks for confirming. But then the devicetree property is not the
> > > > right way to handle this, and we should disable L0s based on the
> > > > compatible string instead.
>
> > > Hmm. I checked further and got the info that there is no change in the IP, but
> > > the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
> > > there will be AERs when L0s is enabled on any controller instance. And there
> > > will be no updated PHY sequence in the future also for this chipset.
> >
> > Why? If it is a bug in the PHY driver, it should be fixed there
> > instead of adding workarounds.
>
> ASPM L0s is currently broken on these platforms and, as far as I
> understand, both under Windows and Linux. Since Qualcomm hasn't been
> able to come up with the necessary PHY init sequences for these
> platforms yet, I doubt they will suddenly appear in the near future.
I see. Ok, I retract my comment.
>
> So we need to disable L0s for now. If an updated PHY init sequence later
> appears, we can always enable it again.
>
> > > So yeah, let's disable it in the driver instead.
>
> Johan
--
With best wishes
Dmitry
On Wed, Mar 06, 2024 at 03:08:57PM +0530, Manivannan Sadhasivam wrote:
> On Wed, Mar 06, 2024 at 10:12:31AM +0100, Johan Hovold wrote:
> > On Wed, Mar 06, 2024 at 10:48:30AM +0200, Dmitry Baryshkov wrote:
> > > On Wed, 6 Mar 2024 at 10:39, Manivannan Sadhasivam
> > > <[email protected]> wrote:
> > > > On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> > > > > Ok, thanks for confirming. But then the devicetree property is not the
> > > > > right way to handle this, and we should disable L0s based on the
> > > > > compatible string instead.
> >
> > > > Hmm. I checked further and got the info that there is no change in the IP, but
> > > > the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
> > > > there will be AERs when L0s is enabled on any controller instance. And there
> > > > will be no updated PHY sequence in the future also for this chipset.
> > >
> > > Why? If it is a bug in the PHY driver, it should be fixed there
> > > instead of adding workarounds.
> >
> > ASPM L0s is currently broken on these platforms and, as far as I
> > understand, both under Windows and Linux. Since Qualcomm hasn't been
> > able to come up with the necessary PHY init sequences for these
> > platforms yet, I doubt they will suddenly appear in the near future.
> >
> > So we need to disable L0s for now. If an updated PHY init sequence later
> > appears, we can always enable it again.
>
> It could be the same case for all 'non-mobile' chipsets (automotive, compute,
> modem). So instead of using the compatible, please add a flag and set that for
> all non-mobile SoCs. Like the ones starting with SAxxx, SCxxx, SDxxx.
I've already updated the series and was just about to post it. Disabling
for further platforms would also require matching on the compatible
string and we can easily do that in a follow-up patch once we have some
confirmation that it is needed.
Johan
On Wed, Mar 06, 2024 at 10:48:30AM +0200, Dmitry Baryshkov wrote:
> On Wed, 6 Mar 2024 at 10:39, Manivannan Sadhasivam
> <[email protected]> wrote:
> >
> > On Wed, Mar 06, 2024 at 08:20:16AM +0100, Johan Hovold wrote:
> > > On Wed, Mar 06, 2024 at 12:03:02PM +0530, Manivannan Sadhasivam wrote:
> > > > 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.
> > >
> > > > > 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.
> > >
> > > Ok, thanks for confirming. But then the devicetree property is not the
> > > right way to handle this, and we should disable L0s based on the
> > > compatible string instead.
> > >
> >
> > Hmm. I checked further and got the info that there is no change in the IP, but
> > the PHY sequence is not tuned correctly for L0s (as I suspected earlier). So
> > there will be AERs when L0s is enabled on any controller instance. And there
> > will be no updated PHY sequence in the future also for this chipset.
>
> Why? If it is a bug in the PHY driver, it should be fixed there
> instead of adding workarounds.
>
Fixing the L0s support requires the expertise of the PHY team and they will only
do if there is any real demand (like in the case of mobile chipsets). For
compute chipsets, they didn't do because most of the NVMe devices out there in
the market only support L1 and L1ss.
So we have to live with this limitation for now.
- Mani
> >
> > So yeah, let's disable it in the driver instead.
> >
> > > > > 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).
> > >
> > > I'll respin the series. Looks like we've already missed the chance to
> > > enable ITS in 6.9 anyway.
> > >
> >
> > Sounds good, thanks!
> >
> > - Mani
> >
> > --
> > மணிவண்ணன் சதாசிவம்
> >
>
>
> --
> With best wishes
> Dmitry
--
மணிவண்ணன் சதாசிவம்