Il 07/01/25 03:18, Jianjun Wang (王建军) ha scritto:
On Fri, 2025-01-03 at 10:16 +0100, AngeloGioacchino Del Regno wrote:
External email : Please do not click links or open attachments until
you have verified the sender or the content.
Il 03/01/25 07:00, Jianjun Wang ha scritto:
Disable ASPM L0s support because it does not significantly save
power
but impacts performance.
That may be a good idea but, without numbers to support your
statement, it's a bit
difficult to say.
How much power does ASPM L0s save on MediaTek SoCs, in microwatts?
How is the performance impacted, and on which specific device(s) on
the PCIe bus?
It's hard to tell the exact number because it is difficult to measure,
and the number of entries into the L0s state may vary even in the same
test scenario.
However, we have encountered some compatibility issues when connected
with some PCIe EPs, and disabling the L0s can fix it. I think disabling
L0s might be the better way, since we usually use L1ss for power-saving
when the link is idle.
To actually decide, we should know what's actually broken, then.
Is the MediaTek controller broken, or is the device broken?
So, is it a MTK quirk, or a device quirk?
If the problem is actually device-related, then this should be handled as
a device-specific quirk, as not just MediaTek platforms would be affected
by compatibility issues.
If the MediaTek PCIe controller is at fault, instead, I agree about just
disabling L0s at the controller level - but then this shall be mentioned
in the commit message, and should have a Fixes tag as well.
Cheers,
Angelo
Thanks.
Cheers,
Angelo
Signed-off-by: Jianjun Wang <jianjun.wang@xxxxxxxxxxxx>
---
drivers/pci/controller/pcie-mediatek-gen3.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
b/drivers/pci/controller/pcie-mediatek-gen3.c
index ed3c0614486c..4bd3b39eebe2 100644
--- a/drivers/pci/controller/pcie-mediatek-gen3.c
+++ b/drivers/pci/controller/pcie-mediatek-gen3.c
@@ -84,6 +84,9 @@
#define PCIE_MSI_SET_ENABLE_REG 0x190
#define PCIE_MSI_SET_ENABLE GENMASK(PCIE_MSI_SET_NUM - 1,
0)
+#define PCIE_LOW_POWER_CTRL_REG 0x194
+#define PCIE_FORCE_DIS_L0S BIT(8)
+
#define PCIE_PIPE4_PIE8_REG 0x338
#define PCIE_K_FINETUNE_MAX GENMASK(5, 0)
#define PCIE_K_FINETUNE_ERR GENMASK(7, 6)
@@ -458,6 +461,14 @@ static int mtk_pcie_startup_port(struct
mtk_gen3_pcie *pcie)
val &= ~PCIE_INTX_ENABLE;
writel_relaxed(val, pcie->base + PCIE_INT_ENABLE_REG);
+ /*
+ * Disable L0s support because it does not significantly save
power
+ * but impacts performance.
+ */
+ val = readl_relaxed(pcie->base + PCIE_LOW_POWER_CTRL_REG);
+ val |= PCIE_FORCE_DIS_L0S;
+ writel_relaxed(val, pcie->base + PCIE_LOW_POWER_CTRL_REG);
+
/* Disable DVFSRC voltage request */
val = readl_relaxed(pcie->base + PCIE_MISC_CTRL_REG);
val |= PCIE_DISABLE_DVFSRC_VLT_REQ;
