mbox series

[RESEND,v1,net-next,0/5] net: stmmac: enable multi-vector MSI

Message ID 20210316121823.18659-1-weifeng.voon@intel.com
Headers show
Series net: stmmac: enable multi-vector MSI | expand

Message

Voon, Weifeng March 16, 2021, 12:18 p.m. UTC
This patchset adds support for multi MSI interrupts in addition to
current single common interrupt implementation. Each MSI interrupt is tied
to a newly introduce interrupt service routine(ISR). Hence, each interrupt
will only go through the corresponding ISR.

In order to increase the efficiency, enabling multi MSI interrupt will
automatically select the interrupt mode configuration INTM=1. When INTM=1,
the TX/RX transfer complete signal will only asserted on corresponding
sbd_perch_tx_intr_o[] or sbd_perch_rx_intr_o[] without asserting signal
on the common sbd_intr_o. Hence, for each TX/RX interrupts, only the
corresponding ISR will be triggered.

Every vendor might have different MSI vector assignment. So, this patchset
only includes multi-vector MSI assignment for Intel platform.

Ong Boon Leong (4):
  net: stmmac: introduce DMA interrupt status masking per traffic
    direction
  net: stmmac: make stmmac_interrupt() function more friendly to MSI
  net: stmmac: introduce MSI Interrupt routines for mac, safety, RX & TX
  stmmac: intel: add support for multi-vector msi and msi-x

Wong, Vee Khee (1):
  net: stmmac: use interrupt mode INTM=1 for multi-MSI

 drivers/net/ethernet/stmicro/stmmac/common.h  |  21 +
 .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 112 +++-
 .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c |  24 +-
 .../net/ethernet/stmicro/stmmac/dwmac4_dma.c  |   8 +
 .../net/ethernet/stmicro/stmmac/dwmac4_dma.h  |  24 +-
 .../net/ethernet/stmicro/stmmac/dwmac4_lib.c  |  30 +-
 .../net/ethernet/stmicro/stmmac/dwmac_dma.h   |  22 +-
 .../net/ethernet/stmicro/stmmac/dwmac_lib.c   |   8 +-
 .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |   6 +
 .../ethernet/stmicro/stmmac/dwxgmac2_dma.c    |   8 +-
 drivers/net/ethernet/stmicro/stmmac/hwif.h    |   2 +-
 drivers/net/ethernet/stmicro/stmmac/stmmac.h  |  16 +
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 482 +++++++++++++++---
 include/linux/stmmac.h                        |   9 +
 14 files changed, 676 insertions(+), 96 deletions(-)

Comments

Jakub Kicinski March 16, 2021, 9:29 p.m. UTC | #1
On Tue, 16 Mar 2021 20:18:21 +0800 Voon Weifeng wrote:
> From: Ong Boon Leong <boon.leong.ong@intel.com>
> 
> Now we introduce MSI interrupt service routines and hook these routines
> up if stmmac_open() sees valid irq line being requested:-
> 
> stmmac_mac_interrupt()    :- MAC (dev->irq), WOL (wol_irq), LPI (lpi_irq)
> stmmac_safety_interrupt() :- Safety Feat Correctible Error (sfty_ce_irq)
>                              & Uncorrectible Error (sfty_ue_irq)
> stmmac_msi_intr_rx()      :- For all RX MSI irq (rx_irq)
> stmmac_msi_intr_tx()      :- For all TX MSI irq (tx_irq)

Do you split RX and TX irqs out on purpose? Most commonly one queue
pair maps to one CPU, so using single IRQ for Rx and Tx results in
fewer IRQs being triggered and better system performance.

> Each of IRQs will have its unique name so that we can differentiate
> them easily under /proc/interrupts.
> 
> Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>

> +static int stmmac_request_irq(struct net_device *dev)

This function is a one huge if statement, please factor out both sides
into separate subfunctions.

> +	netdev_info(priv->dev, "PASS: requesting IRQs\n");

Does the user really need to know interrupts were requested on every
probe?

> +	return ret;

return 0; ?

> +irq_error:
> +	stmmac_free_irq(dev, irq_err, irq_idx);
> +	return ret;
> +}
Voon, Weifeng March 24, 2021, 8:43 a.m. UTC | #2
> On Tue, 16 Mar 2021 20:18:21 +0800 Voon Weifeng wrote:

> > From: Ong Boon Leong <boon.leong.ong@intel.com>

> >

> > Now we introduce MSI interrupt service routines and hook these

> > routines up if stmmac_open() sees valid irq line being requested:-

> >

> > stmmac_mac_interrupt()    :- MAC (dev->irq), WOL (wol_irq), LPI (lpi_irq)

> > stmmac_safety_interrupt() :- Safety Feat Correctible Error (sfty_ce_irq)

> >                              & Uncorrectible Error (sfty_ue_irq)

> > stmmac_msi_intr_rx()      :- For all RX MSI irq (rx_irq)

> > stmmac_msi_intr_tx()      :- For all TX MSI irq (tx_irq)

> 

> Do you split RX and TX irqs out on purpose? Most commonly one queue pair

> maps to one CPU, so using single IRQ for Rx and Tx results in fewer IRQs

> being triggered and better system performance.


Yes, the RX and TX irqs are split out on purpose as the hardware is designed
to have independent MSI vector. You can refer the 4th patch in the this patchset.
https://patchwork.kernel.org/project/netdevbpf/patch/20210316121823.18659-5-weifeng.voon@intel.com/  
This design also gives us the flexibility to group RX/TX MSI vectors to specific CPU freely.

Weifeng


> > Each of IRQs will have its unique name so that we can differentiate

> > them easily under /proc/interrupts.

> >

> > Signed-off-by: Ong Boon Leong <boon.leong.ong@intel.com>

> > Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>

> 

> > +static int stmmac_request_irq(struct net_device *dev)

> 

> This function is a one huge if statement, please factor out both sides into

> separate subfunctions.


Noted. Will do.

> 

> > +	netdev_info(priv->dev, "PASS: requesting IRQs\n");

> 

> Does the user really need to know interrupts were requested on every probe?


Will remove.

> 

> > +	return ret;

> 

> return 0; ?


Good catch, will fix.

> 

> > +irq_error:

> > +	stmmac_free_irq(dev, irq_err, irq_idx);

> > +	return ret;

> > +}