netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
@ 2020-11-23 14:52 stefanc
  2020-11-23 15:10 ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 10+ messages in thread
From: stefanc @ 2020-11-23 14:52 UTC (permalink / raw)
  To: netdev
  Cc: thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel, stefanc,
	kuba, linux, mw, antoine.tenart, andrew, rmk+kernel

From: Stefan Chulski <stefanc@marvell.com>

Tx/Rx FIFO is a HW resource limited by total size, but shared
by all ports of same CP110 and impacting port-performance.
Do not divide the FIFO for ports which are not enabled in DTS,
so active ports could have more FIFO.

The active port mapping should be done in probe before FIFO-init.

Signed-off-by: Stefan Chulski <stefanc@marvell.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2.h      |  23 +++--
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 129 +++++++++++++++++-------
 2 files changed, 108 insertions(+), 44 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 8347758..6bd7e40 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -695,6 +695,9 @@
 /* Maximum number of supported ports */
 #define MVPP2_MAX_PORTS			4
 
+/* Loopback port index */
+#define MVPP2_LOOPBACK_PORT_INDEX	3
+
 /* Maximum number of TXQs used by single port */
 #define MVPP2_MAX_TXQ			8
 
@@ -729,22 +732,21 @@
 #define MVPP2_TX_DESC_ALIGN		(MVPP2_DESC_ALIGNED_SIZE - 1)
 
 /* RX FIFO constants */
+#define MVPP2_RX_FIFO_PORT_DATA_SIZE_44KB	0xb000
 #define MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB	0x8000
 #define MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB	0x2000
 #define MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB	0x1000
-#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_32KB	0x200
-#define MVPP2_RX_FIFO_PORT_ATTR_SIZE_8KB	0x80
+#define MVPP2_RX_FIFO_PORT_ATTR_SIZE(data_size)	((data_size) >> 6)
 #define MVPP2_RX_FIFO_PORT_ATTR_SIZE_4KB	0x40
 #define MVPP2_RX_FIFO_PORT_MIN_PKT		0x80
 
 /* TX FIFO constants */
-#define MVPP22_TX_FIFO_DATA_SIZE_10KB		0xa
-#define MVPP22_TX_FIFO_DATA_SIZE_3KB		0x3
-#define MVPP2_TX_FIFO_THRESHOLD_MIN		256
-#define MVPP2_TX_FIFO_THRESHOLD_10KB	\
-	(MVPP22_TX_FIFO_DATA_SIZE_10KB * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
-#define MVPP2_TX_FIFO_THRESHOLD_3KB	\
-	(MVPP22_TX_FIFO_DATA_SIZE_3KB * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
+#define MVPP22_TX_FIFO_DATA_SIZE_16KB		16
+#define MVPP22_TX_FIFO_DATA_SIZE_10KB		10
+#define MVPP22_TX_FIFO_DATA_SIZE_3KB		3
+#define MVPP2_TX_FIFO_THRESHOLD_MIN		256 /* Bytes */
+#define MVPP2_TX_FIFO_THRESHOLD(kb)	\
+		((kb) * 1024 - MVPP2_TX_FIFO_THRESHOLD_MIN)
 
 /* RX buffer constants */
 #define MVPP2_SKB_SHINFO_SIZE \
@@ -946,6 +948,9 @@ struct mvpp2 {
 	/* List of pointers to port structures */
 	int port_count;
 	struct mvpp2_port *port_list[MVPP2_MAX_PORTS];
+	/* Map of enabled ports */
+	unsigned long port_map;
+
 	struct mvpp2_tai *tai;
 
 	/* Number of Tx threads used */
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index f6616c8..08c237a 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6601,32 +6601,56 @@ static void mvpp2_rx_fifo_init(struct mvpp2 *priv)
 	mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1);
 }
 
-static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
+static void mvpp22_rx_fifo_set_hw(struct mvpp2 *priv, int port, int data_size)
 {
-	int port;
+	int attr_size = MVPP2_RX_FIFO_PORT_ATTR_SIZE(data_size);
 
-	/* The FIFO size parameters are set depending on the maximum speed a
-	 * given port can handle:
-	 * - Port 0: 10Gbps
-	 * - Port 1: 2.5Gbps
-	 * - Ports 2 and 3: 1Gbps
-	 */
+	mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(port), data_size);
+	mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(port), attr_size);
+}
 
-	mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(0),
-		    MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB);
-	mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(0),
-		    MVPP2_RX_FIFO_PORT_ATTR_SIZE_32KB);
+/* Initialize TX FIFO's: the total FIFO size is 48kB on PPv2.2.
+ * 4kB fixed space must be assigned for the loopback port.
+ * Redistribute remaining avialable 44kB space among all active ports.
+ * Guarantee minimum 32kB for 10G port and 8kB for port 1, capable of 2.5G
+ * SGMII link.
+ */
+static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
+{
+	int remaining_ports_count;
+	unsigned long port_map;
+	int size_remainder;
+	int port, size;
+
+	/* The loopback requires fixed 4kB of the FIFO space assignment. */
+	mvpp22_rx_fifo_set_hw(priv, MVPP2_LOOPBACK_PORT_INDEX,
+			      MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB);
+	port_map = priv->port_map & ~BIT(MVPP2_LOOPBACK_PORT_INDEX);
+
+	/* Set RX FIFO size to 0 for inactive ports. */
+	for_each_clear_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX)
+		mvpp22_rx_fifo_set_hw(priv, port, 0);
+
+	/* Assign remaining RX FIFO space among all active ports. */
+	size_remainder = MVPP2_RX_FIFO_PORT_DATA_SIZE_44KB;
+	remaining_ports_count = hweight_long(port_map);
+
+	for_each_set_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX) {
+		if (remaining_ports_count == 1)
+			size = size_remainder;
+		else if (port == 0)
+			size = max(size_remainder / remaining_ports_count,
+				   MVPP2_RX_FIFO_PORT_DATA_SIZE_32KB);
+		else if (port == 1)
+			size = max(size_remainder / remaining_ports_count,
+				   MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB);
+		else
+			size = size_remainder / remaining_ports_count;
 
-	mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(1),
-		    MVPP2_RX_FIFO_PORT_DATA_SIZE_8KB);
-	mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(1),
-		    MVPP2_RX_FIFO_PORT_ATTR_SIZE_8KB);
+		size_remainder -= size;
+		remaining_ports_count--;
 
-	for (port = 2; port < MVPP2_MAX_PORTS; port++) {
-		mvpp2_write(priv, MVPP2_RX_DATA_FIFO_SIZE_REG(port),
-			    MVPP2_RX_FIFO_PORT_DATA_SIZE_4KB);
-		mvpp2_write(priv, MVPP2_RX_ATTR_FIFO_SIZE_REG(port),
-			    MVPP2_RX_FIFO_PORT_ATTR_SIZE_4KB);
+		mvpp22_rx_fifo_set_hw(priv, port, size);
 	}
 
 	mvpp2_write(priv, MVPP2_RX_MIN_PKT_SIZE_REG,
@@ -6634,24 +6658,53 @@ static void mvpp22_rx_fifo_init(struct mvpp2 *priv)
 	mvpp2_write(priv, MVPP2_RX_FIFO_INIT_REG, 0x1);
 }
 
-/* Initialize Tx FIFO's: the total FIFO size is 19kB on PPv2.2 and 10G
- * interfaces must have a Tx FIFO size of 10kB. As only port 0 can do 10G,
- * configure its Tx FIFO size to 10kB and the others ports Tx FIFO size to 3kB.
+static void mvpp22_tx_fifo_set_hw(struct mvpp2 *priv, int port, int size)
+{
+	int threshold = MVPP2_TX_FIFO_THRESHOLD(size);
+
+	mvpp2_write(priv, MVPP22_TX_FIFO_SIZE_REG(port), size);
+	mvpp2_write(priv, MVPP22_TX_FIFO_THRESH_REG(port), threshold);
+}
+
+/* Initialize TX FIFO's: the total FIFO size is 19kB on PPv2.2.
+ * 3kB fixed space must be assigned for the loopback port.
+ * Redistribute remaining avialable 16kB space among all active ports.
+ * The 10G interface should use 10kB (which is maximum possible size
+ * per single port).
  */
 static void mvpp22_tx_fifo_init(struct mvpp2 *priv)
 {
-	int port, size, thrs;
-
-	for (port = 0; port < MVPP2_MAX_PORTS; port++) {
-		if (port == 0) {
+	int remaining_ports_count;
+	unsigned long port_map;
+	int size_remainder;
+	int port, size;
+
+	/* The loopback requires fixed 3kB of the FIFO space assignment. */
+	mvpp22_tx_fifo_set_hw(priv, MVPP2_LOOPBACK_PORT_INDEX,
+			      MVPP22_TX_FIFO_DATA_SIZE_3KB);
+	port_map = priv->port_map & ~BIT(MVPP2_LOOPBACK_PORT_INDEX);
+
+	/* Set TX FIFO size to 0 for inactive ports. */
+	for_each_clear_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX)
+		mvpp22_tx_fifo_set_hw(priv, port, 0);
+
+	/* Assign remaining TX FIFO space among all active ports. */
+	size_remainder = MVPP22_TX_FIFO_DATA_SIZE_16KB;
+	remaining_ports_count = hweight_long(port_map);
+
+	for_each_set_bit(port, &port_map, MVPP2_LOOPBACK_PORT_INDEX) {
+		if (remaining_ports_count == 1)
+			size = min(size_remainder,
+				   MVPP22_TX_FIFO_DATA_SIZE_10KB);
+		else if (port == 0)
 			size = MVPP22_TX_FIFO_DATA_SIZE_10KB;
-			thrs = MVPP2_TX_FIFO_THRESHOLD_10KB;
-		} else {
-			size = MVPP22_TX_FIFO_DATA_SIZE_3KB;
-			thrs = MVPP2_TX_FIFO_THRESHOLD_3KB;
-		}
-		mvpp2_write(priv, MVPP22_TX_FIFO_SIZE_REG(port), size);
-		mvpp2_write(priv, MVPP22_TX_FIFO_THRESH_REG(port), thrs);
+		else
+			size = size_remainder / remaining_ports_count;
+
+		size_remainder -= size;
+		remaining_ports_count--;
+
+		mvpp22_tx_fifo_set_hw(priv, port, size);
 	}
 }
 
@@ -6952,6 +7005,12 @@ static int mvpp2_probe(struct platform_device *pdev)
 			goto err_axi_clk;
 	}
 
+	/* Map DTS-active ports. Should be done before FIFO mvpp2_init */
+	fwnode_for_each_available_child_node(fwnode, port_fwnode) {
+		if (!fwnode_property_read_u32(port_fwnode, "port-id", &i))
+			priv->port_map |= BIT(i);
+	}
+
 	/* Initialize network controller */
 	err = mvpp2_init(pdev, priv);
 	if (err < 0) {
-- 
1.9.1


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

* Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 14:52 [PATCH v1] net: mvpp2: divide fifo for dts-active ports only stefanc
@ 2020-11-23 15:10 ` Russell King - ARM Linux admin
  2020-11-23 15:26   ` [EXT] " Stefan Chulski
  2020-11-23 15:30   ` Russell King - ARM Linux admin
  0 siblings, 2 replies; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-23 15:10 UTC (permalink / raw)
  To: stefanc
  Cc: netdev, thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel,
	kuba, mw, antoine.tenart, andrew

Hi,

On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> From: Stefan Chulski <stefanc@marvell.com>
> 
> Tx/Rx FIFO is a HW resource limited by total size, but shared
> by all ports of same CP110 and impacting port-performance.
> Do not divide the FIFO for ports which are not enabled in DTS,
> so active ports could have more FIFO.
> 
> The active port mapping should be done in probe before FIFO-init.

It would be nice to know what the effect is from this - is it a
small or large boost in performance?

What is the effect when the ports on a CP110 are configured for
10G, 1G, and 2.5G in that order, as is the case on the Macchiatobin
board?

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 15:10 ` Russell King - ARM Linux admin
@ 2020-11-23 15:26   ` Stefan Chulski
  2020-11-23 15:33     ` Russell King - ARM Linux admin
  2020-11-23 15:30   ` Russell King - ARM Linux admin
  1 sibling, 1 reply; 10+ messages in thread
From: Stefan Chulski @ 2020-11-23 15:26 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, antoine.tenart, andrew

> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Monday, November 23, 2020 5:11 PM
> To: Stefan Chulski <stefanc@marvell.com>
> Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> mw@semihalf.com; antoine.tenart@bootlin.com; andrew@lunn.ch
> Subject: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi,
> 
> On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > From: Stefan Chulski <stefanc@marvell.com>
> >
> > Tx/Rx FIFO is a HW resource limited by total size, but shared by all
> > ports of same CP110 and impacting port-performance.
> > Do not divide the FIFO for ports which are not enabled in DTS, so
> > active ports could have more FIFO.
> >
> > The active port mapping should be done in probe before FIFO-init.
> 
> It would be nice to know what the effect is from this - is it a small or large
> boost in performance?

I didn't saw any significant improvement with LINUX bridge or forwarding, but
this reduced PPv2 overruns drops, reduced CRC sent errors with DPDK user space application.
So this improved zero loss throughput. Probably with XDP we would see a similar effect.

> What is the effect when the ports on a CP110 are configured for 10G, 1G, and
> 2.5G in that order, as is the case on the Macchiatobin board?

Macchiatobin has two CP's.  CP1 has 3 ports, so the distribution of FIFO would be the same as before.
On CP0 which has a single port, all FIFO would be allocated for 10G port.

Regards.


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

* Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 15:10 ` Russell King - ARM Linux admin
  2020-11-23 15:26   ` [EXT] " Stefan Chulski
@ 2020-11-23 15:30   ` Russell King - ARM Linux admin
  1 sibling, 0 replies; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-23 15:30 UTC (permalink / raw)
  To: stefanc
  Cc: netdev, thomas.petazzoni, davem, nadavh, ymarkman, linux-kernel,
	kuba, mw, andrew

On Mon, Nov 23, 2020 at 03:10:49PM +0000, Russell King - ARM Linux admin wrote:
> Hi,
> 
> On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > From: Stefan Chulski <stefanc@marvell.com>
> > 
> > Tx/Rx FIFO is a HW resource limited by total size, but shared
> > by all ports of same CP110 and impacting port-performance.
> > Do not divide the FIFO for ports which are not enabled in DTS,
> > so active ports could have more FIFO.
> > 
> > The active port mapping should be done in probe before FIFO-init.
> 
> It would be nice to know what the effect is from this - is it a
> small or large boost in performance?
> 
> What is the effect when the ports on a CP110 are configured for
> 10G, 1G, and 2.5G in that order, as is the case on the Macchiatobin
> board?

(dropped Antoine, his email is bouncing.)

I've rechecked, and on Macchiatobin, it certainly is:
Port 0 = 10G SFP/88x3310
Port 1 = 1G dedicated ethernet
Port 2 = 1G/2.5G SFP slot

and we do run the SFP slot at 2.5G speeds.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 15:26   ` [EXT] " Stefan Chulski
@ 2020-11-23 15:33     ` Russell King - ARM Linux admin
  2020-11-23 15:44       ` Stefan Chulski
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-23 15:33 UTC (permalink / raw)
  To: Stefan Chulski
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, antoine.tenart, andrew

On Mon, Nov 23, 2020 at 03:26:11PM +0000, Stefan Chulski wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Monday, November 23, 2020 5:11 PM
> > To: Stefan Chulski <stefanc@marvell.com>
> > Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> > davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> > <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> > mw@semihalf.com; antoine.tenart@bootlin.com; andrew@lunn.ch
> > Subject: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
> > 
> > External Email
> > 
> > ----------------------------------------------------------------------
> > Hi,
> > 
> > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > > From: Stefan Chulski <stefanc@marvell.com>
> > >
> > > Tx/Rx FIFO is a HW resource limited by total size, but shared by all
> > > ports of same CP110 and impacting port-performance.
> > > Do not divide the FIFO for ports which are not enabled in DTS, so
> > > active ports could have more FIFO.
> > >
> > > The active port mapping should be done in probe before FIFO-init.
> > 
> > It would be nice to know what the effect is from this - is it a small or large
> > boost in performance?
> 
> I didn't saw any significant improvement with LINUX bridge or forwarding, but
> this reduced PPv2 overruns drops, reduced CRC sent errors with DPDK user space application.
> So this improved zero loss throughput. Probably with XDP we would see a similar effect.
> 
> > What is the effect when the ports on a CP110 are configured for 10G, 1G, and
> > 2.5G in that order, as is the case on the Macchiatobin board?
> 
> Macchiatobin has two CP's.  CP1 has 3 ports, so the distribution of FIFO would be the same as before.
> On CP0 which has a single port, all FIFO would be allocated for 10G port.

Your code allocates for CP1:

32K to port 0 (the 10G port on Macchiatobin)
8K to port 1 (the 1G dedicated ethernet port on Macchiatobin)
4K to port 2 (the 1G/2.5G SFP port on Macchiatobin)

I'm questioning that allocation for port 1 and 2.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 15:33     ` Russell King - ARM Linux admin
@ 2020-11-23 15:44       ` Stefan Chulski
  2020-11-23 15:51         ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Chulski @ 2020-11-23 15:44 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, andrew

> > > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > > > From: Stefan Chulski <stefanc@marvell.com>
> > > >
> > > > Tx/Rx FIFO is a HW resource limited by total size, but shared by
> > > > all ports of same CP110 and impacting port-performance.
> > > > Do not divide the FIFO for ports which are not enabled in DTS, so
> > > > active ports could have more FIFO.
> > > >
> > > > The active port mapping should be done in probe before FIFO-init.
> > >
> > > It would be nice to know what the effect is from this - is it a
> > > small or large boost in performance?
> >
> > I didn't saw any significant improvement with LINUX bridge or
> > forwarding, but this reduced PPv2 overruns drops, reduced CRC sent errors
> with DPDK user space application.
> > So this improved zero loss throughput. Probably with XDP we would see a
> similar effect.
> >
> > > What is the effect when the ports on a CP110 are configured for 10G,
> > > 1G, and 2.5G in that order, as is the case on the Macchiatobin board?
> >
> > Macchiatobin has two CP's.  CP1 has 3 ports, so the distribution of FIFO would
> be the same as before.
> > On CP0 which has a single port, all FIFO would be allocated for 10G port.
> 
> Your code allocates for CP1:
> 
> 32K to port 0 (the 10G port on Macchiatobin) 8K to port 1 (the 1G dedicated
> ethernet port on Macchiatobin) 4K to port 2 (the 1G/2.5G SFP port on
> Macchiatobin)
> 
> I'm questioning that allocation for port 1 and 2.

Yes, but this allocation exists also in current code.
From HW point of view(MAC and PPv2) maximum supported speed
in CP110: port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G.
in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G.

So this allocation looks correct at least for CP115.
Problem that we cannot reallocate FIFO during runtime, after specific speed negotiation.

Regards.



 


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

* Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 15:44       ` Stefan Chulski
@ 2020-11-23 15:51         ` Russell King - ARM Linux admin
  2020-11-23 16:03           ` Stefan Chulski
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-23 15:51 UTC (permalink / raw)
  To: Stefan Chulski
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, andrew

On Mon, Nov 23, 2020 at 03:44:05PM +0000, Stefan Chulski wrote:
> > > > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > > > > From: Stefan Chulski <stefanc@marvell.com>
> > > > >
> > > > > Tx/Rx FIFO is a HW resource limited by total size, but shared by
> > > > > all ports of same CP110 and impacting port-performance.
> > > > > Do not divide the FIFO for ports which are not enabled in DTS, so
> > > > > active ports could have more FIFO.
> > > > >
> > > > > The active port mapping should be done in probe before FIFO-init.
> > > >
> > > > It would be nice to know what the effect is from this - is it a
> > > > small or large boost in performance?
> > >
> > > I didn't saw any significant improvement with LINUX bridge or
> > > forwarding, but this reduced PPv2 overruns drops, reduced CRC sent errors
> > with DPDK user space application.
> > > So this improved zero loss throughput. Probably with XDP we would see a
> > similar effect.
> > >
> > > > What is the effect when the ports on a CP110 are configured for 10G,
> > > > 1G, and 2.5G in that order, as is the case on the Macchiatobin board?
> > >
> > > Macchiatobin has two CP's.  CP1 has 3 ports, so the distribution of FIFO would
> > be the same as before.
> > > On CP0 which has a single port, all FIFO would be allocated for 10G port.
> > 
> > Your code allocates for CP1:
> > 
> > 32K to port 0 (the 10G port on Macchiatobin) 8K to port 1 (the 1G dedicated
> > ethernet port on Macchiatobin) 4K to port 2 (the 1G/2.5G SFP port on
> > Macchiatobin)
> > 
> > I'm questioning that allocation for port 1 and 2.
> 
> Yes, but this allocation exists also in current code.
> From HW point of view(MAC and PPv2) maximum supported speed
> in CP110: port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G.
> in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G.
> 
> So this allocation looks correct at least for CP115.
> Problem that we cannot reallocate FIFO during runtime, after specific speed negotiation.

We could do much better. DT has a "max-speed" property for ethernet
controllers. If we have that property, then I think we should use
that to determine the initialisation time FIFO allocation.

As I say, on Macchiatobin, the allocations we end up with are just
crazy when you consider the port speeds that the hardware supports.
Maybe that should be done as a follow-on patch - but I think it
needs to be done.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 15:51         ` Russell King - ARM Linux admin
@ 2020-11-23 16:03           ` Stefan Chulski
  2020-11-23 17:30             ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Chulski @ 2020-11-23 16:03 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, andrew



> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Monday, November 23, 2020 5:52 PM
> To: Stefan Chulski <stefanc@marvell.com>
> Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> mw@semihalf.com; andrew@lunn.ch
> Subject: Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports
> only
> 
> On Mon, Nov 23, 2020 at 03:44:05PM +0000, Stefan Chulski wrote:
> > Yes, but this allocation exists also in current code.
> > From HW point of view(MAC and PPv2) maximum supported speed in CP110:
> > port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G.
> > in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G.
> >
> > So this allocation looks correct at least for CP115.
> > Problem that we cannot reallocate FIFO during runtime, after specific speed
> negotiation.
> 
> We could do much better. DT has a "max-speed" property for ethernet
> controllers. If we have that property, then I think we should use that to
> determine the initialisation time FIFO allocation.
> 
> As I say, on Macchiatobin, the allocations we end up with are just crazy when
> you consider the port speeds that the hardware supports.
> Maybe that should be done as a follow-on patch - but I think it needs to be
> done.

I agree with you. We can use "max-speed" for better FIFO allocations.
I plan to upstream more fixes from the "Marvell" devel branch then I can prepare this patch.
So you OK with this patch and then follow-on improvement?

Regards.





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

* Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 16:03           ` Stefan Chulski
@ 2020-11-23 17:30             ` Russell King - ARM Linux admin
  2020-11-23 17:48               ` Stefan Chulski
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2020-11-23 17:30 UTC (permalink / raw)
  To: Stefan Chulski
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, andrew

On Mon, Nov 23, 2020 at 04:03:00PM +0000, Stefan Chulski wrote:
> > -----Original Message-----
> > From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> > Sent: Monday, November 23, 2020 5:52 PM
> > To: Stefan Chulski <stefanc@marvell.com>
> > Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> > davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> > <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> > mw@semihalf.com; andrew@lunn.ch
> > Subject: Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports
> > only
> > 
> > On Mon, Nov 23, 2020 at 03:44:05PM +0000, Stefan Chulski wrote:
> > > Yes, but this allocation exists also in current code.
> > > From HW point of view(MAC and PPv2) maximum supported speed in CP110:
> > > port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G.
> > > in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G.
> > >
> > > So this allocation looks correct at least for CP115.
> > > Problem that we cannot reallocate FIFO during runtime, after specific speed
> > negotiation.
> > 
> > We could do much better. DT has a "max-speed" property for ethernet
> > controllers. If we have that property, then I think we should use that to
> > determine the initialisation time FIFO allocation.
> > 
> > As I say, on Macchiatobin, the allocations we end up with are just crazy when
> > you consider the port speeds that the hardware supports.
> > Maybe that should be done as a follow-on patch - but I think it needs to be
> > done.
> 
> I agree with you. We can use "max-speed" for better FIFO allocations.
> I plan to upstream more fixes from the "Marvell" devel branch then I can prepare this patch.
> So you OK with this patch and then follow-on improvement?

Yes - but I would like to see the commit description say that this
results in no change the situation where all three ports are in use.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

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

* RE: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
  2020-11-23 17:30             ` Russell King - ARM Linux admin
@ 2020-11-23 17:48               ` Stefan Chulski
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Chulski @ 2020-11-23 17:48 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: netdev, thomas.petazzoni, davem, Nadav Haklai, Yan Markman,
	linux-kernel, kuba, mw, andrew



> -----Original Message-----
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Sent: Monday, November 23, 2020 7:30 PM
> To: Stefan Chulski <stefanc@marvell.com>
> Cc: netdev@vger.kernel.org; thomas.petazzoni@bootlin.com;
> davem@davemloft.net; Nadav Haklai <nadavh@marvell.com>; Yan Markman
> <ymarkman@marvell.com>; linux-kernel@vger.kernel.org; kuba@kernel.org;
> mw@semihalf.com; andrew@lunn.ch
> Subject: Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports
> only
> 
> On Mon, Nov 23, 2020 at 04:03:00PM +0000, Stefan Chulski wrote:
> > I agree with you. We can use "max-speed" for better FIFO allocations.
> > I plan to upstream more fixes from the "Marvell" devel branch then I can
> prepare this patch.
> > So you OK with this patch and then follow-on improvement?
> 
> Yes - but I would like to see the commit description say that this results in no
> change the situation where all three ports are in use.

Ok, I would repost patch.

Regards.

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

end of thread, other threads:[~2020-11-23 17:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-23 14:52 [PATCH v1] net: mvpp2: divide fifo for dts-active ports only stefanc
2020-11-23 15:10 ` Russell King - ARM Linux admin
2020-11-23 15:26   ` [EXT] " Stefan Chulski
2020-11-23 15:33     ` Russell King - ARM Linux admin
2020-11-23 15:44       ` Stefan Chulski
2020-11-23 15:51         ` Russell King - ARM Linux admin
2020-11-23 16:03           ` Stefan Chulski
2020-11-23 17:30             ` Russell King - ARM Linux admin
2020-11-23 17:48               ` Stefan Chulski
2020-11-23 15:30   ` Russell King - ARM Linux admin

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).