diff mbox series

[3/5] PCI: mediatek-gen3: Disable ASPM L0s

Message ID 20250103060035.30688-4-jianjun.wang@mediatek.com (mailing list archive)
State New
Headers show
Series PCI: mediatek-gen3: Add MT8196 support | expand

Commit Message

Jianjun Wang Jan. 3, 2025, 6 a.m. UTC
Disable ASPM L0s support because it does not significantly save power
but impacts performance.

Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
---
 drivers/pci/controller/pcie-mediatek-gen3.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

AngeloGioacchino Del Regno Jan. 3, 2025, 9:16 a.m. UTC | #1
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?

Cheers,
Angelo

> Signed-off-by: Jianjun Wang <jianjun.wang@mediatek.com>
> ---
>   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;
Bjorn Helgaas Jan. 3, 2025, 7:15 p.m. UTC | #2
On Fri, Jan 03, 2025 at 02:00:13PM +0800, Jianjun Wang wrote:
> Disable ASPM L0s support because it does not significantly save power
> but impacts performance.

This seems like a user/administrator decision, not a driver decision.

L0s reduces power at the cost of performance for *all* PCIe devices,
although the actual numbers may vary.
diff mbox series

Patch

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;