Re: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 01:22:35PM +, Stefan Chulski wrote: > > Ditto. > > > > I don't think these need to be fixed in the net tree, but it would still be > > nice > > to fix the problem. Please do so, as an initial patch in your series - so > > we can > > then backport if it turns out to eventually be necessary. > > > > Thanks. > > My series already has 15 patches and patchwork not happy about series with > over 15 patches. > Maybe I can send this as separate patch to net-next(or net) first and base > this series on this net-next tree with this patch? In that case, send the fixes as a separate series and get that merged first. It shouldn't take very long to get the fixes merged. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Re: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 01:02:14PM +, Stefan Chulski wrote:
> > On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> > > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > index 761f745..8b4073c 100644
> > > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > > @@ -1133,14 +1133,19 @@ static inline void
> > > mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) static
> > > void mvpp2_interrupts_mask(void *arg) {
> > > struct mvpp2_port *port = arg;
> > > + int cpu = smp_processor_id();
> > > + u32 thread;
> > >
> > > /* If the thread isn't used, don't do anything */
> > > - if (smp_processor_id() > port->priv->nthreads)
> > > + if (cpu > port->priv->nthreads)
> > > return;
> >
> > What happened to a patch fixing this? Did I miss it? Was it submitted
> > independently to the net tree?
>
> Some reviewers asked to remove this from the series. I would send it as
> separate patch to net.
It is not a regression, and although it is a fix, as you explained when
I first raised it, it isn't a condition that can be reached due to:
priv->nthreads = min_t(unsigned int, num_present_cpus(),
MVPP2_MAX_THREADS);
and I don't think we support a dynamic present CPU mask on any platform
that is currently supported by this driver.
If we did, then it would be possible for the off-by-one issue to be
triggered.
No matter what, it should happen _before_ this patch set is merged.
Trying to do it afterwards guarantees more pain if stable trees decide
they want to backport the fix.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
RE: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
> Ditto. > > I don't think these need to be fixed in the net tree, but it would still be > nice > to fix the problem. Please do so, as an initial patch in your series - so we > can > then backport if it turns out to eventually be necessary. > > Thanks. My series already has 15 patches and patchwork not happy about series with over 15 patches. Maybe I can send this as separate patch to net-next(or net) first and base this series on this net-next tree with this patch? Regards, Stefan.
RE: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
> -Original Message-
> From: Russell King - ARM Linux admin
> Sent: Thursday, February 11, 2021 2:50 PM
> To: Stefan Chulski
> Cc: net...@vger.kernel.org; thomas.petazz...@bootlin.com;
> da...@davemloft.net; Nadav Haklai ; Yan
> Markman ; linux-kernel@vger.kernel.org;
> k...@kernel.org; m...@semihalf.com; and...@lunn.ch;
> aten...@kernel.org; devicet...@vger.kernel.org; robh...@kernel.org;
> sebastian.hesselba...@gmail.com; gregory.clem...@bootlin.com; linux-
> arm-ker...@lists.infradead.org
> Subject: [EXT] Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non
> occupied descriptor threshold
>
> External Email
>
> --
> On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > index 761f745..8b4073c 100644
> > --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > @@ -1133,14 +1133,19 @@ static inline void
> > mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec) static
> > void mvpp2_interrupts_mask(void *arg) {
> > struct mvpp2_port *port = arg;
> > + int cpu = smp_processor_id();
> > + u32 thread;
> >
> > /* If the thread isn't used, don't do anything */
> > - if (smp_processor_id() > port->priv->nthreads)
> > + if (cpu > port->priv->nthreads)
> > return;
>
> What happened to a patch fixing this? Did I miss it? Was it submitted
> independently to the net tree?
Some reviewers asked to remove this from the series. I would send it as
separate patch to net.
Regards.
Re: [PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
On Thu, Feb 11, 2021 at 12:48:55PM +0200, stef...@marvell.com wrote:
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 761f745..8b4073c 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -1133,14 +1133,19 @@ static inline void
> mvpp2_qvec_interrupt_disable(struct mvpp2_queue_vector *qvec)
> static void mvpp2_interrupts_mask(void *arg)
> {
> struct mvpp2_port *port = arg;
> + int cpu = smp_processor_id();
> + u32 thread;
>
> /* If the thread isn't used, don't do anything */
> - if (smp_processor_id() > port->priv->nthreads)
> + if (cpu > port->priv->nthreads)
> return;
What happened to a patch fixing this? Did I miss it? Was it submitted
independently to the net tree?
> @@ -1150,20 +1155,25 @@ static void mvpp2_interrupts_mask(void *arg)
> static void mvpp2_interrupts_unmask(void *arg)
> {
> struct mvpp2_port *port = arg;
> - u32 val;
> + int cpu = smp_processor_id();
> + u32 val, thread;
>
> /* If the thread isn't used, don't do anything */
> - if (smp_processor_id() > port->priv->nthreads)
> + if (cpu > port->priv->nthreads)
> return;
Ditto.
I don't think these need to be fixed in the net tree, but it would still
be nice to fix the problem. Please do so, as an initial patch in your
series - so we can then backport if it turns out to eventually be
necessary.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
[PATCH v13 net-next 08/15] net: mvpp2: add FCA RXQ non occupied descriptor threshold
From: Stefan Chulski
The firmware needs to monitor the RX Non-occupied descriptor
bits for flow control to move to XOFF mode.
These bits need to be unmasked to be functional, but they will
not raise interrupts as we leave the RX exception summary
bit in MVPP2_ISR_RX_TX_MASK_REG clear.
Signed-off-by: Stefan Chulski
Acked-by: Marcin Wojtas
---
drivers/net/ethernet/marvell/mvpp2/mvpp2.h | 3 ++
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 44
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
index 9239d80..d2cc513c 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2.h
@@ -295,6 +295,8 @@
#define MVPP2_PON_CAUSE_TXP_OCCUP_DESC_ALL_MASK0x3fc0
#define MVPP2_PON_CAUSE_MISC_SUM_MASK BIT(31)
#define MVPP2_ISR_MISC_CAUSE_REG 0x55b0
+#define MVPP2_ISR_RX_ERR_CAUSE_REG(port) (0x5520 + 4 * (port))
+#define MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK 0x00ff
/* Buffer Manager registers */
#define MVPP2_BM_POOL_BASE_REG(pool) (0x6000 + ((pool) * 4))
@@ -763,6 +765,7 @@
/* MSS Flow control */
#define FC_QUANTA 0x
#define FC_CLK_DIVIDER 100
+#define MSS_THRESHOLD_STOP 768
/* RX buffer constants */
#define MVPP2_SKB_SHINFO_SIZE \
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 761f745..8b4073c 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1133,14 +1133,19 @@ static inline void mvpp2_qvec_interrupt_disable(struct
mvpp2_queue_vector *qvec)
static void mvpp2_interrupts_mask(void *arg)
{
struct mvpp2_port *port = arg;
+ int cpu = smp_processor_id();
+ u32 thread;
/* If the thread isn't used, don't do anything */
- if (smp_processor_id() > port->priv->nthreads)
+ if (cpu > port->priv->nthreads)
return;
- mvpp2_thread_write(port->priv,
- mvpp2_cpu_to_thread(port->priv, smp_processor_id()),
+ thread = mvpp2_cpu_to_thread(port->priv, cpu);
+
+ mvpp2_thread_write(port->priv, thread,
MVPP2_ISR_RX_TX_MASK_REG(port->id), 0);
+ mvpp2_thread_write(port->priv, thread,
+ MVPP2_ISR_RX_ERR_CAUSE_REG(port->id), 0);
}
/* Unmask the current thread's Rx/Tx interrupts.
@@ -1150,20 +1155,25 @@ static void mvpp2_interrupts_mask(void *arg)
static void mvpp2_interrupts_unmask(void *arg)
{
struct mvpp2_port *port = arg;
- u32 val;
+ int cpu = smp_processor_id();
+ u32 val, thread;
/* If the thread isn't used, don't do anything */
- if (smp_processor_id() > port->priv->nthreads)
+ if (cpu > port->priv->nthreads)
return;
+ thread = mvpp2_cpu_to_thread(port->priv, cpu);
+
val = MVPP2_CAUSE_MISC_SUM_MASK |
MVPP2_CAUSE_RXQ_OCCUP_DESC_ALL_MASK(port->priv->hw_version);
if (port->has_tx_irqs)
val |= MVPP2_CAUSE_TXQ_OCCUP_DESC_ALL_MASK;
- mvpp2_thread_write(port->priv,
- mvpp2_cpu_to_thread(port->priv, smp_processor_id()),
+ mvpp2_thread_write(port->priv, thread,
MVPP2_ISR_RX_TX_MASK_REG(port->id), val);
+ mvpp2_thread_write(port->priv, thread,
+ MVPP2_ISR_RX_ERR_CAUSE_REG(port->id),
+ MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK);
}
static void
@@ -1188,6 +1198,9 @@ static void mvpp2_interrupts_unmask(void *arg)
mvpp2_thread_write(port->priv, v->sw_thread_id,
MVPP2_ISR_RX_TX_MASK_REG(port->id), val);
+ mvpp2_thread_write(port->priv, v->sw_thread_id,
+ MVPP2_ISR_RX_ERR_CAUSE_REG(port->id),
+ MVPP2_ISR_RX_ERR_CAUSE_NONOCC_MASK);
}
}
@@ -2393,6 +2406,20 @@ static void mvpp2_txp_max_tx_size_set(struct mvpp2_port
*port)
}
}
+/* Set the number of non-occupied descriptors threshold */
+static void mvpp2_set_rxq_free_tresh(struct mvpp2_port *port,
+struct mvpp2_rx_queue *rxq)
+{
+ u32 val;
+
+ mvpp2_write(port->priv, MVPP2_RXQ_NUM_REG, rxq->id);
+
+ val = mvpp2_read(port->priv, MVPP2_RXQ_THRESH_REG);
+ val &= ~MVPP2_RXQ_NON_OCCUPIED_MASK;
+ val |= MSS_THRESHOLD_STOP << MVPP2_RXQ_NON_OCCUPIED_OFFSET;
+ mvpp2_write(port->priv, MVPP2_RXQ_THRESH_REG, val);
+}
+
/* Set the number of packets that will be received before Rx interrupt
* will be generated by HW.
*/
@@ -2648,6 +2675,9 @@ static int mvpp2_rxq_init(struct mvpp2_port *port,
mvpp2_rx_pkts_coal_set(port, rxq);
mvpp2_rx_time_coal_set(port, rxq);
+
