linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/2] Configure root port MPS during host probing
@ 2025-06-20 15:55 Hans Zhang
  2025-06-20 15:55 ` [PATCH v5 1/2] PCI: " Hans Zhang
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Hans Zhang @ 2025-06-20 15:55 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang
  Cc: pali, neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Hans Zhang

Current PCIe initialization exhibits a key optimization gap: Root Ports
may operate with non-optimal Maximum Payload Size (MPS) settings. While
downstream device configuration is handled during bus enumeration, Root
Port MPS values inherited from firmware or hardware defaults often fail
to utilize the full capabilities supported by controller hardware. This
results in suboptimal data transfer efficiency throughout the PCIe
hierarchy.

This patch series addresses this by:

1.  Core PCI enhancement (Patch 1):
- Proactively configures Root Port MPS during host controller probing
- Sets initial MPS to hardware maximum (128 << dev->pcie_mpss)
- Conditional on PCIe bus tuning being enabled (PCIE_BUS_TUNE_OFF unset)
- Maintains backward compatibility via PCIE_BUS_TUNE_OFF check
- Preserves standard MPS negotiation during downstream enumeration

2.  Driver cleanup (Patch 2):
- Removes redundant MPS configuration from Meson PCIe controller driver
- Functionality is now centralized in PCI core
- Simplifies driver maintenance long-term

---
Changes for v5:
- Use pcie_set_mps directly instead of pcie_write_mps.
- The patch 1 commit message were modified.

Changes for v4:
- The patch [v4 1/2] add a comment to explain why it was done this way.
- The patch [v4 2/2] have not been modified.
- Drop patch [v3 3/3]. The Maintainer of the pci-aardvark.c file suggests
  that this patch cannot be submitted. In addition, Mani also suggests
  dropping this patch until this series of issues is resolved.

Changes for v3:
- The new split is patch 2/3 and 3/3.
- Modify the patch 1/3 according to Niklas' suggestion.

Changes for v2:
- According to the Maintainer's suggestion, limit the setting of MPS
  changes to platforms with controller drivers.
- Delete the MPS code set by the SOC manufacturer.
---

Hans Zhang (2):
  PCI: Configure root port MPS during host probing
  PCI: dwc: Remove redundant MPS configuration

 drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
 drivers/pci/probe.c                    | 10 ++++++++++
 2 files changed, 10 insertions(+), 17 deletions(-)


base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
-- 
2.25.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 1/2] PCI: Configure root port MPS during host probing
  2025-06-20 15:55 [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
@ 2025-06-20 15:55 ` Hans Zhang
  2025-06-23 10:58   ` Manivannan Sadhasivam
  2025-09-02 17:48   ` Bjorn Helgaas
  2025-06-20 15:55 ` [PATCH v5 2/2] PCI: dwc: Remove redundant MPS configuration Hans Zhang
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 12+ messages in thread
From: Hans Zhang @ 2025-06-20 15:55 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang
  Cc: pali, neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Hans Zhang, Niklas Cassel

Current PCIe initialization logic may leave root ports operating with
non-optimal Maximum Payload Size (MPS) settings. While downstream device
configuration is handled during bus enumeration, root port MPS values
inherited from firmware or hardware defaults might not utilize the full
capabilities supported by the controller hardware. This can result in
suboptimal data transfer efficiency across the PCIe hierarchy.

During host controller probing phase, when PCIe bus tuning is enabled,
the implementation now configures root port MPS settings to their
hardware-supported maximum values. Specifically, when configuring the MPS
for a PCIe device, if the device is a root port and the bus tuning is not
disabled (PCIE_BUS_TUNE_OFF), the MPS is set to 128 << dev->pcie_mpss to
match the Root Port's maximum supported payload size. The Max Read Request
Size (MRRS) is subsequently adjusted through existing companion logic to
maintain compatibility with PCIe specifications.

Note that this initial setting of the root port MPS to the maximum might
be reduced later during the enumeration of downstream devices if any of
those devices do not support the maximum MPS of the root port.

Explicit initialization at host probing stage ensures consistent PCIe
topology configuration before downstream devices perform their own MPS
negotiations. This proactive approach addresses platform-specific
requirements where controller drivers depend on properly initialized
root port settings, while maintaining backward compatibility through
PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
utilized without altering existing device negotiation behaviors.

Suggested-by: Niklas Cassel <cassel@kernel.org>
Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/probe.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 4b8693ec9e4c..9f8803da914c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2178,6 +2178,16 @@ static void pci_configure_mps(struct pci_dev *dev)
 		return;
 	}
 
+	/*
+	 * Unless MPS strategy is PCIE_BUS_TUNE_OFF (don't touch MPS at all),
+	 * start off by setting root ports' MPS to MPSS. Depending on the MPS
+	 * strategy, and the MPSS of the devices below the root port, the MPS
+	 * of the root port might get overridden later.
+	 */
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
+	    pcie_bus_config != PCIE_BUS_TUNE_OFF)
+		pcie_set_mps(dev, 128 << dev->pcie_mpss);
+
 	if (!bridge || !pci_is_pcie(bridge))
 		return;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v5 2/2] PCI: dwc: Remove redundant MPS configuration
  2025-06-20 15:55 [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
  2025-06-20 15:55 ` [PATCH v5 1/2] PCI: " Hans Zhang
@ 2025-06-20 15:55 ` Hans Zhang
  2025-06-20 16:05 ` [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Hans Zhang @ 2025-06-20 15:55 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang
  Cc: pali, neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Hans Zhang

The Meson PCIe controller driver manually configures maximum payload
size (MPS) through meson_set_max_payload, duplicating functionality now
centralized in the PCI core.  Deprecating redundant code simplifies the
driver and aligns it with the consolidated MPS management strategy,
improving long-term maintainability.

Signed-off-by: Hans Zhang <18255117159@163.com>
---
 drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
index 787469d1b396..3d12e1a9bb0c 100644
--- a/drivers/pci/controller/dwc/pci-meson.c
+++ b/drivers/pci/controller/dwc/pci-meson.c
@@ -261,22 +261,6 @@ static int meson_size_to_payload(struct meson_pcie *mp, int size)
 	return fls(size) - 8;
 }
 
-static void meson_set_max_payload(struct meson_pcie *mp, int size)
-{
-	struct dw_pcie *pci = &mp->pci;
-	u32 val;
-	u16 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
-	int max_payload_size = meson_size_to_payload(mp, size);
-
-	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
-	val &= ~PCI_EXP_DEVCTL_PAYLOAD;
-	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
-
-	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_DEVCTL);
-	val |= PCIE_CAP_MAX_PAYLOAD_SIZE(max_payload_size);
-	dw_pcie_writel_dbi(pci, offset + PCI_EXP_DEVCTL, val);
-}
-
 static void meson_set_max_rd_req_size(struct meson_pcie *mp, int size)
 {
 	struct dw_pcie *pci = &mp->pci;
@@ -381,7 +365,6 @@ static int meson_pcie_host_init(struct dw_pcie_rp *pp)
 
 	pp->bridge->ops = &meson_pci_ops;
 
-	meson_set_max_payload(mp, MAX_PAYLOAD_SIZE);
 	meson_set_max_rd_req_size(mp, MAX_READ_REQ_SIZE);
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/2] Configure root port MPS during host probing
  2025-06-20 15:55 [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
  2025-06-20 15:55 ` [PATCH v5 1/2] PCI: " Hans Zhang
  2025-06-20 15:55 ` [PATCH v5 2/2] PCI: dwc: Remove redundant MPS configuration Hans Zhang
@ 2025-06-20 16:05 ` Hans Zhang
  2025-06-23  9:03   ` Niklas Cassel
  2025-07-16  7:41 ` Niklas Cassel
  2025-09-02  8:33 ` Niklas Cassel
  4 siblings, 1 reply; 12+ messages in thread
From: Hans Zhang @ 2025-06-20 16:05 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang
  Cc: pali, neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip

Hi all,

I'm very sorry. I forgot the review label of Niklas. If it can be 
merged, please ask the maintainer to help add this tag. Thank you very much.

Reviewed-by: Niklas Cassel <cassel@kernel.org>

Best regards,
Hans

On 2025/6/20 23:55, Hans Zhang wrote:
> Current PCIe initialization exhibits a key optimization gap: Root Ports
> may operate with non-optimal Maximum Payload Size (MPS) settings. While
> downstream device configuration is handled during bus enumeration, Root
> Port MPS values inherited from firmware or hardware defaults often fail
> to utilize the full capabilities supported by controller hardware. This
> results in suboptimal data transfer efficiency throughout the PCIe
> hierarchy.
> 
> This patch series addresses this by:
> 
> 1.  Core PCI enhancement (Patch 1):
> - Proactively configures Root Port MPS during host controller probing
> - Sets initial MPS to hardware maximum (128 << dev->pcie_mpss)
> - Conditional on PCIe bus tuning being enabled (PCIE_BUS_TUNE_OFF unset)
> - Maintains backward compatibility via PCIE_BUS_TUNE_OFF check
> - Preserves standard MPS negotiation during downstream enumeration
> 
> 2.  Driver cleanup (Patch 2):
> - Removes redundant MPS configuration from Meson PCIe controller driver
> - Functionality is now centralized in PCI core
> - Simplifies driver maintenance long-term
> 
> ---
> Changes for v5:
> - Use pcie_set_mps directly instead of pcie_write_mps.
> - The patch 1 commit message were modified.
> 
> Changes for v4:
> - The patch [v4 1/2] add a comment to explain why it was done this way.
> - The patch [v4 2/2] have not been modified.
> - Drop patch [v3 3/3]. The Maintainer of the pci-aardvark.c file suggests
>    that this patch cannot be submitted. In addition, Mani also suggests
>    dropping this patch until this series of issues is resolved.
> 
> Changes for v3:
> - The new split is patch 2/3 and 3/3.
> - Modify the patch 1/3 according to Niklas' suggestion.
> 
> Changes for v2:
> - According to the Maintainer's suggestion, limit the setting of MPS
>    changes to platforms with controller drivers.
> - Delete the MPS code set by the SOC manufacturer.
> ---
> 
> Hans Zhang (2):
>    PCI: Configure root port MPS during host probing
>    PCI: dwc: Remove redundant MPS configuration
> 
>   drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
>   drivers/pci/probe.c                    | 10 ++++++++++
>   2 files changed, 10 insertions(+), 17 deletions(-)
> 
> 
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/2] Configure root port MPS during host probing
  2025-06-20 16:05 ` [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
@ 2025-06-23  9:03   ` Niklas Cassel
  0 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-06-23  9:03 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip

On Sat, Jun 21, 2025 at 12:05:43AM +0800, Hans Zhang wrote:
> Hi all,
> 
> I'm very sorry. I forgot the review label of Niklas. If
> it can be merged, please ask the maintainer to help add
> this tag. Thank you very much.
> 
> Reviewed-by: Niklas Cassel <cassel@kernel.org>

If the maintainer is using b4, it should pick up the tag above automatically.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 1/2] PCI: Configure root port MPS during host probing
  2025-06-20 15:55 ` [PATCH v5 1/2] PCI: " Hans Zhang
@ 2025-06-23 10:58   ` Manivannan Sadhasivam
  2025-09-02 17:48   ` Bjorn Helgaas
  1 sibling, 0 replies; 12+ messages in thread
From: Manivannan Sadhasivam @ 2025-06-23 10:58 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Niklas Cassel

On Fri, Jun 20, 2025 at 11:55:06PM +0800, Hans Zhang wrote:
> Current PCIe initialization logic may leave root ports operating with
> non-optimal Maximum Payload Size (MPS) settings. While downstream device
> configuration is handled during bus enumeration, root port MPS values
> inherited from firmware or hardware defaults might not utilize the full
> capabilities supported by the controller hardware. This can result in
> suboptimal data transfer efficiency across the PCIe hierarchy.
> 
> During host controller probing phase, when PCIe bus tuning is enabled,
> the implementation now configures root port MPS settings to their
> hardware-supported maximum values. Specifically, when configuring the MPS
> for a PCIe device, if the device is a root port and the bus tuning is not
> disabled (PCIE_BUS_TUNE_OFF), the MPS is set to 128 << dev->pcie_mpss to
> match the Root Port's maximum supported payload size. The Max Read Request
> Size (MRRS) is subsequently adjusted through existing companion logic to
> maintain compatibility with PCIe specifications.
> 
> Note that this initial setting of the root port MPS to the maximum might
> be reduced later during the enumeration of downstream devices if any of
> those devices do not support the maximum MPS of the root port.
> 
> Explicit initialization at host probing stage ensures consistent PCIe
> topology configuration before downstream devices perform their own MPS
> negotiations. This proactive approach addresses platform-specific
> requirements where controller drivers depend on properly initialized
> root port settings, while maintaining backward compatibility through
> PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
> utilized without altering existing device negotiation behaviors.
> 
> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Hans Zhang <18255117159@163.com>

Acked-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  drivers/pci/probe.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4b8693ec9e4c..9f8803da914c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2178,6 +2178,16 @@ static void pci_configure_mps(struct pci_dev *dev)
>  		return;
>  	}
>  
> +	/*
> +	 * Unless MPS strategy is PCIE_BUS_TUNE_OFF (don't touch MPS at all),
> +	 * start off by setting root ports' MPS to MPSS. Depending on the MPS
> +	 * strategy, and the MPSS of the devices below the root port, the MPS
> +	 * of the root port might get overridden later.
> +	 */
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
> +	    pcie_bus_config != PCIE_BUS_TUNE_OFF)
> +		pcie_set_mps(dev, 128 << dev->pcie_mpss);
> +
>  	if (!bridge || !pci_is_pcie(bridge))
>  		return;
>  
> -- 
> 2.25.1
> 

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/2] Configure root port MPS during host probing
  2025-06-20 15:55 [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
                   ` (2 preceding siblings ...)
  2025-06-20 16:05 ` [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
@ 2025-07-16  7:41 ` Niklas Cassel
  2025-08-13 12:42   ` Niklas Cassel
  2025-09-02  8:33 ` Niklas Cassel
  4 siblings, 1 reply; 12+ messages in thread
From: Niklas Cassel @ 2025-07-16  7:41 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip

On Fri, Jun 20, 2025 at 11:55:05PM +0800, Hans Zhang wrote:
(snip)
> ---
> 
> Hans Zhang (2):
>   PCI: Configure root port MPS during host probing
>   PCI: dwc: Remove redundant MPS configuration
> 
>  drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
>  drivers/pci/probe.c                    | 10 ++++++++++
>  2 files changed, 10 insertions(+), 17 deletions(-)
> 
> 
> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> -- 
> 2.25.1
> 

Any chance of this series getting picked up?


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/2] Configure root port MPS during host probing
  2025-07-16  7:41 ` Niklas Cassel
@ 2025-08-13 12:42   ` Niklas Cassel
  0 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-08-13 12:42 UTC (permalink / raw)
  To: bhelgaas
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Hans Zhang

On Wed, Jul 16, 2025 at 09:41:54AM +0200, Niklas Cassel wrote:
> On Fri, Jun 20, 2025 at 11:55:05PM +0800, Hans Zhang wrote:
> (snip)
> > ---
> > 
> > Hans Zhang (2):
> >   PCI: Configure root port MPS during host probing
> >   PCI: dwc: Remove redundant MPS configuration
> > 
> >  drivers/pci/controller/dwc/pci-meson.c | 17 -----------------
> >  drivers/pci/probe.c                    | 10 ++++++++++
> >  2 files changed, 10 insertions(+), 17 deletions(-)
> > 
> > 
> > base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
> > -- 
> > 2.25.1
> > 
> 
> Any chance of this series getting picked up?

Gentle ping.


Kind regards,
Niklas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/2] Configure root port MPS during host probing
  2025-06-20 15:55 [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
                   ` (3 preceding siblings ...)
  2025-07-16  7:41 ` Niklas Cassel
@ 2025-09-02  8:33 ` Niklas Cassel
  4 siblings, 0 replies; 12+ messages in thread
From: Niklas Cassel @ 2025-09-02  8:33 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip

On Fri, Jun 20, 2025 at 11:55:05PM +0800, Hans Zhang wrote:
> Current PCIe initialization exhibits a key optimization gap: Root Ports
> may operate with non-optimal Maximum Payload Size (MPS) settings. While
> downstream device configuration is handled during bus enumeration, Root
> Port MPS values inherited from firmware or hardware defaults often fail
> to utilize the full capabilities supported by controller hardware. This
> results in suboptimal data transfer efficiency throughout the PCIe
> hierarchy.

(snip)

Gentle ping.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 1/2] PCI: Configure root port MPS during host probing
  2025-06-20 15:55 ` [PATCH v5 1/2] PCI: " Hans Zhang
  2025-06-23 10:58   ` Manivannan Sadhasivam
@ 2025-09-02 17:48   ` Bjorn Helgaas
  2025-09-03 17:11     ` Hans Zhang
  1 sibling, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2025-09-02 17:48 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Niklas Cassel

On Fri, Jun 20, 2025 at 11:55:06PM +0800, Hans Zhang wrote:
> Current PCIe initialization logic may leave root ports operating with
> non-optimal Maximum Payload Size (MPS) settings. While downstream device
> configuration is handled during bus enumeration, root port MPS values
> inherited from firmware or hardware defaults ...

Apparently Root Port MPS configuration is different from that for
downstream devices?

> might not utilize the full
> capabilities supported by the controller hardware. This can result in
> suboptimal data transfer efficiency across the PCIe hierarchy.
> 
> During host controller probing phase, when PCIe bus tuning is enabled,
> the implementation now configures root port MPS settings to their
> hardware-supported maximum values. Specifically, when configuring the MPS
> for a PCIe device, if the device is a root port and the bus tuning is not
> disabled (PCIE_BUS_TUNE_OFF), the MPS is set to 128 << dev->pcie_mpss to
> match the Root Port's maximum supported payload size. The Max Read Request
> Size (MRRS) is subsequently adjusted through existing companion logic to
> maintain compatibility with PCIe specifications.
> 
> Note that this initial setting of the root port MPS to the maximum might
> be reduced later during the enumeration of downstream devices if any of
> those devices do not support the maximum MPS of the root port.
> 
> Explicit initialization at host probing stage ensures consistent PCIe
> topology configuration before downstream devices perform their own MPS
> negotiations. This proactive approach addresses platform-specific
> requirements where controller drivers depend on properly initialized
> root port settings, while maintaining backward compatibility through
> PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
> utilized without altering existing device negotiation behaviors.

This last paragraph seems kind of like marketing without any real
content.  Is there something important in there?

Nits:
s/root port/Root Port/

Reword "implementation now configures" to be clear about whether "now"
refers to before this patch or after.

Update the MRRS "to maintain compatibility" part.  I'm dubious about
there being a spec compatibility issue with respect to MRRS.  Cite the
relevant section if there is an issue.

> Suggested-by: Niklas Cassel <cassel@kernel.org>
> Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
> Signed-off-by: Hans Zhang <18255117159@163.com>
> ---
>  drivers/pci/probe.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 4b8693ec9e4c..9f8803da914c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2178,6 +2178,16 @@ static void pci_configure_mps(struct pci_dev *dev)
>  		return;
>  	}
>  
> +	/*
> +	 * Unless MPS strategy is PCIE_BUS_TUNE_OFF (don't touch MPS at all),
> +	 * start off by setting root ports' MPS to MPSS. Depending on the MPS
> +	 * strategy, and the MPSS of the devices below the root port, the MPS
> +	 * of the root port might get overridden later.
> +	 */
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
> +	    pcie_bus_config != PCIE_BUS_TUNE_OFF)
> +		pcie_set_mps(dev, 128 << dev->pcie_mpss);
> +
>  	if (!bridge || !pci_is_pcie(bridge))
>  		return;
>  
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 1/2] PCI: Configure root port MPS during host probing
  2025-09-02 17:48   ` Bjorn Helgaas
@ 2025-09-03 17:11     ` Hans Zhang
  2025-09-03 17:35       ` Bjorn Helgaas
  0 siblings, 1 reply; 12+ messages in thread
From: Hans Zhang @ 2025-09-03 17:11 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Niklas Cassel



On 2025/9/3 01:48, Bjorn Helgaas wrote:
> On Fri, Jun 20, 2025 at 11:55:06PM +0800, Hans Zhang wrote:
>> Current PCIe initialization logic may leave root ports operating with
>> non-optimal Maximum Payload Size (MPS) settings. While downstream device
>> configuration is handled during bus enumeration, root port MPS values
>> inherited from firmware or hardware defaults ...
> 
> Apparently Root Port MPS configuration is different from that for
> downstream devices?

Dear Bjorn,

Thank you very much for your reply.

Yes, at the very beginning, the situation I tested was like the previous 
reply:
https://lore.kernel.org/linux-pci/bb40385c-6839-484c-90b2-d6c7ecb95ba9@163.com/


Niklas helped find the documentation description of RK3588 TRM:
https://lore.kernel.org/linux-pci/aACoEpueUHBLjgbb@ryzen/


Dear Niklas,

If I have misunderstood Bjorn's review opinion, please help me clarify 
it together. Thank you again for helping me ping the Maintainer. When I 
wanted to ping, you did it before me.


> 
>> might not utilize the full
>> capabilities supported by the controller hardware. This can result in
>> suboptimal data transfer efficiency across the PCIe hierarchy.
>>
>> During host controller probing phase, when PCIe bus tuning is enabled,
>> the implementation now configures root port MPS settings to their
>> hardware-supported maximum values. Specifically, when configuring the MPS
>> for a PCIe device, if the device is a root port and the bus tuning is not
>> disabled (PCIE_BUS_TUNE_OFF), the MPS is set to 128 << dev->pcie_mpss to
>> match the Root Port's maximum supported payload size. The Max Read Request
>> Size (MRRS) is subsequently adjusted through existing companion logic to
>> maintain compatibility with PCIe specifications.
>>
>> Note that this initial setting of the root port MPS to the maximum might
>> be reduced later during the enumeration of downstream devices if any of
>> those devices do not support the maximum MPS of the root port.
>>
>> Explicit initialization at host probing stage ensures consistent PCIe
>> topology configuration before downstream devices perform their own MPS
>> negotiations. This proactive approach addresses platform-specific
>> requirements where controller drivers depend on properly initialized
>> root port settings, while maintaining backward compatibility through
>> PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
>> utilized without altering existing device negotiation behaviors.
> 
> This last paragraph seems kind of like marketing without any real
> content.  Is there something important in there?

I think the above elaboration is sufficient. I will delete it.

> 
> Nits:
> s/root port/Root Port/

Will change.

> 
> Reword "implementation now configures" to be clear about whether "now"
> refers to before this patch or after.

Will be modified.

> 
> Update the MRRS "to maintain compatibility" part.  I'm dubious about
> there being a spec compatibility issue with respect to MRRS.  Cite the
> relevant section if there is an issue.

The description is inaccurate. I will correct it.



I plan to modify the commit message as follows:
If there are any incorrect descriptions, please correct them. Thank you 
very much.

Current PCIe initialization logic may leave Root Ports operating with
non-optimal Maximum Payload Size (MPS) settings. While downstream device
configuration is handled during bus enumeration, Root Port MPS values
inherited from firmware or hardware defaults might not utilize the full
capabilities supported by the controller hardware. This can result in
suboptimal data transfer efficiency across the PCIe hierarchy.

With this patch, during the host controller probing phase and when PCIe
bus tuning is enabled, the implementation configures Root Port MPS
settings to their hardware-supported maximum values. Specifically, when
configuring the MPS for a PCIe device, if the device is a Root Port and
the bus tuning is not disabled (PCIE_BUS_TUNE_OFF), the MPS is set to
128 << dev->pcie_mpss to match the Root Port's maximum supported payload
size. The Max Read Request Size (MRRS) is subsequently adjusted by
existing logic in pci_configure_mps() to ensure it is not less than the
MPS, maintaining compliance with PCIe specifications (see PCIe r7.0,
sec 7.5.3.4).

Note that this initial setting of the Root Port MPS to the maximum might
be reduced later during the enumeration of downstream devices if any of
those devices do not support the maximum MPS of the Root Port.

Best regards,
Hans

> 
>> Suggested-by: Niklas Cassel <cassel@kernel.org>
>> Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
>> Signed-off-by: Hans Zhang <18255117159@163.com>
>> ---
>>   drivers/pci/probe.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 4b8693ec9e4c..9f8803da914c 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2178,6 +2178,16 @@ static void pci_configure_mps(struct pci_dev *dev)
>>   		return;
>>   	}
>>   
>> +	/*
>> +	 * Unless MPS strategy is PCIE_BUS_TUNE_OFF (don't touch MPS at all),
>> +	 * start off by setting root ports' MPS to MPSS. Depending on the MPS
>> +	 * strategy, and the MPSS of the devices below the root port, the MPS
>> +	 * of the root port might get overridden later.
>> +	 */
>> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT &&
>> +	    pcie_bus_config != PCIE_BUS_TUNE_OFF)
>> +		pcie_set_mps(dev, 128 << dev->pcie_mpss);
>> +
>>   	if (!bridge || !pci_is_pcie(bridge))
>>   		return;
>>   
>> -- 
>> 2.25.1
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 1/2] PCI: Configure root port MPS during host probing
  2025-09-03 17:11     ` Hans Zhang
@ 2025-09-03 17:35       ` Bjorn Helgaas
  0 siblings, 0 replies; 12+ messages in thread
From: Bjorn Helgaas @ 2025-09-03 17:35 UTC (permalink / raw)
  To: Hans Zhang
  Cc: lpieralisi, kwilczynski, bhelgaas, heiko, mani, yue.wang, pali,
	neil.armstrong, robh, jingoohan1, khilman, jbrunet,
	martin.blumenstingl, linux-pci, linux-kernel, linux-arm-kernel,
	linux-amlogic, linux-rockchip, Niklas Cassel

On Thu, Sep 04, 2025 at 01:11:22AM +0800, Hans Zhang wrote:
> On 2025/9/3 01:48, Bjorn Helgaas wrote:
> > On Fri, Jun 20, 2025 at 11:55:06PM +0800, Hans Zhang wrote:
> > > Current PCIe initialization logic may leave root ports operating with
> > > non-optimal Maximum Payload Size (MPS) settings. While downstream device
> > > configuration is handled during bus enumeration, root port MPS values
> > > inherited from firmware or hardware defaults ...
> > 
> > Apparently Root Port MPS configuration is different from that for
> > downstream devices?

> Yes, at the very beginning, the situation I tested was like the previous
> reply:
> https://lore.kernel.org/linux-pci/bb40385c-6839-484c-90b2-d6c7ecb95ba9@163.com/

I meant that this commit log suggests the *code path* is different for
Root Ports than other devices.

> > > might not utilize the full
> > > capabilities supported by the controller hardware. This can result in
> > > suboptimal data transfer efficiency across the PCIe hierarchy.
> > > 
> > > During host controller probing phase, when PCIe bus tuning is enabled,
> > > the implementation now configures root port MPS settings to their
> > > hardware-supported maximum values. Specifically, when configuring the MPS
> > > for a PCIe device, if the device is a root port and the bus tuning is not
> > > disabled (PCIE_BUS_TUNE_OFF), the MPS is set to 128 << dev->pcie_mpss to
> > > match the Root Port's maximum supported payload size. The Max Read Request
> > > Size (MRRS) is subsequently adjusted through existing companion logic to
> > > maintain compatibility with PCIe specifications.
> > > 
> > > Note that this initial setting of the root port MPS to the maximum might
> > > be reduced later during the enumeration of downstream devices if any of
> > > those devices do not support the maximum MPS of the root port.
> > > 
> > > Explicit initialization at host probing stage ensures consistent PCIe
> > > topology configuration before downstream devices perform their own MPS
> > > negotiations. This proactive approach addresses platform-specific
> > > requirements where controller drivers depend on properly initialized
> > > root port settings, while maintaining backward compatibility through
> > > PCIE_BUS_TUNE_OFF conditional checks. Hardware capabilities are fully
> > > utilized without altering existing device negotiation behaviors.

> > Update the MRRS "to maintain compatibility" part.  I'm dubious about
> > there being a spec compatibility issue with respect to MRRS.  Cite the
> > relevant section if there is an issue.
> 
> The description is inaccurate. I will correct it.
> 
> I plan to modify the commit message as follows:
> If there are any incorrect descriptions, please correct them. Thank you very
> much.
> 
> Current PCIe initialization logic may leave Root Ports operating with
> non-optimal Maximum Payload Size (MPS) settings. While downstream device
> configuration is handled during bus enumeration, Root Port MPS values
> inherited from firmware or hardware defaults might not utilize the full
> capabilities supported by the controller hardware. This can result in
> suboptimal data transfer efficiency across the PCIe hierarchy.
> 
> With this patch, during the host controller probing phase and when PCIe
> bus tuning is enabled, the implementation configures Root Port MPS
> settings to their hardware-supported maximum values. Specifically, when
> configuring the MPS for a PCIe device, if the device is a Root Port and
> the bus tuning is not disabled (PCIE_BUS_TUNE_OFF), the MPS is set to
> 128 << dev->pcie_mpss to match the Root Port's maximum supported payload
> size. The Max Read Request Size (MRRS) is subsequently adjusted by
> existing logic in pci_configure_mps() to ensure it is not less than the
> MPS, maintaining compliance with PCIe specifications (see PCIe r7.0,
> sec 7.5.3.4).

AFAICS, sec 7.5.3.4 doesn't say anything about a relationship between
MRRS and MPS.  MPS is a constraint on the payload size of TLPs with
data (Memory Writes, Completions with Data, etc).  MRRS is a
constraint on the Length field in a Memory Read Request.  A single
Memory Read can be satisified with several Completions.  MPS is one of
the things that determines how many Completions are required.

This has more details and a lot of good discussion:
https://www.xilinx.com/support/documentation/white_papers/wp350.pdf

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-09-03 17:35 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-20 15:55 [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
2025-06-20 15:55 ` [PATCH v5 1/2] PCI: " Hans Zhang
2025-06-23 10:58   ` Manivannan Sadhasivam
2025-09-02 17:48   ` Bjorn Helgaas
2025-09-03 17:11     ` Hans Zhang
2025-09-03 17:35       ` Bjorn Helgaas
2025-06-20 15:55 ` [PATCH v5 2/2] PCI: dwc: Remove redundant MPS configuration Hans Zhang
2025-06-20 16:05 ` [PATCH v5 0/2] Configure root port MPS during host probing Hans Zhang
2025-06-23  9:03   ` Niklas Cassel
2025-07-16  7:41 ` Niklas Cassel
2025-08-13 12:42   ` Niklas Cassel
2025-09-02  8:33 ` Niklas Cassel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).