2019-12-10 19:56:28

by Jose Abreu

[permalink] [raw]
Subject: [PATCH net-next 0/4] net: stmmac: Improvements for -next

Improvements for stmmac.

1) Adds more information regarding HW Caps in the DebugFS file.

2) Prevents incostant bandwidth because of missing interrupts.

3) Allows interrupts to be independently enabled or disabled so that we don't
have to schedule both TX and RX NAPIs.

4) Stops using a magic number in coalesce timer re-arm.

---
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Jose Abreu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Chen-Yu Tsai <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---

Jose Abreu (4):
net: stmmac: Print more information in DebugFS DMA Capabilities file
net: stmmac: Always arm TX Timer at end of transmission start
net: stmmac: Let TX and RX interrupts be independently
enabled/disabled
net: stmmac: Always use TX coalesce timer value when rescheduling

drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 24 +++++-
drivers/net/ethernet/stmicro/stmmac/dwmac4_dma.h | 11 ++-
drivers/net/ethernet/stmicro/stmmac/dwmac4_lib.c | 47 +++++++++--
drivers/net/ethernet/stmicro/stmmac/dwmac_dma.h | 6 +-
drivers/net/ethernet/stmicro/stmmac/dwmac_lib.c | 22 ++++-
drivers/net/ethernet/stmicro/stmmac/dwxgmac2.h | 2 +
drivers/net/ethernet/stmicro/stmmac/dwxgmac2_dma.c | 24 +++++-
drivers/net/ethernet/stmicro/stmmac/hwif.h | 6 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 94 +++++++++++++++-------
10 files changed, 183 insertions(+), 54 deletions(-)

--
2.7.4


2019-12-10 19:56:57

by Jose Abreu

[permalink] [raw]
Subject: [PATCH net-next 2/4] net: stmmac: Always arm TX Timer at end of transmission start

If TX Coalesce timer is enabled we should always arm it, otherwise we
may hit the case where an interrupt is missed and the TX Queue will
timeout.

Arming the timer does not necessarly mean it will run the tx_clean()
because this function is wrapped around NAPI launcher.

Signed-off-by: Jose Abreu <[email protected]>

---
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Jose Abreu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 41d4188fc7bc..ae499fdd47bc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3053,8 +3053,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
tx_q->tx_count_frames = 0;
stmmac_set_tx_ic(priv, desc);
priv->xstats.tx_set_ic_bit++;
- } else {
- stmmac_tx_timer_arm(priv, queue);
}

/* We've used all descriptors we need for this skb, however,
@@ -3125,6 +3123,7 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)

tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx * sizeof(*desc));
stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
+ stmmac_tx_timer_arm(priv, queue);

return NETDEV_TX_OK;

@@ -3276,8 +3275,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
tx_q->tx_count_frames = 0;
stmmac_set_tx_ic(priv, desc);
priv->xstats.tx_set_ic_bit++;
- } else {
- stmmac_tx_timer_arm(priv, queue);
}

/* We've used all descriptors we need for this skb, however,
@@ -3366,6 +3363,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)

tx_q->tx_tail_addr = tx_q->dma_tx_phy + (tx_q->cur_tx * sizeof(*desc));
stmmac_set_tx_tail_ptr(priv, priv->ioaddr, tx_q->tx_tail_addr, queue);
+ stmmac_tx_timer_arm(priv, queue);

return NETDEV_TX_OK;

--
2.7.4

2019-12-10 19:58:02

by Jose Abreu

[permalink] [raw]
Subject: [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling

When we have pending packets we re-arm the TX timer with a magic value.
Change this from the hardcoded value to the pre-defined TX coalesce
timer value.

Signed-off-by: Jose Abreu <[email protected]>

---
Cc: Giuseppe Cavallaro <[email protected]>
Cc: Alexandre Torgue <[email protected]>
Cc: Jose Abreu <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f61780ae30ac..726a17d9cc35 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1975,7 +1975,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue)

/* We still have pending packets, let's call for a new scheduling */
if (tx_q->dirty_tx != tx_q->cur_tx)
- mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10));
+ mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer));

__netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue));

--
2.7.4

2019-12-14 20:30:43

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next 4/4] net: stmmac: Always use TX coalesce timer value when rescheduling

On Tue, 10 Dec 2019 20:54:44 +0100, Jose Abreu wrote:
> When we have pending packets we re-arm the TX timer with a magic value.
> Change this from the hardcoded value to the pre-defined TX coalesce
> timer value.

s/pre-defined/user controlled/ ?

> Signed-off-by: Jose Abreu <[email protected]>
> ---
> Cc: Giuseppe Cavallaro <[email protected]>
> Cc: Alexandre Torgue <[email protected]>
> Cc: Jose Abreu <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f61780ae30ac..726a17d9cc35 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1975,7 +1975,7 @@ static int stmmac_tx_clean(struct stmmac_priv *priv, int budget, u32 queue)
>
> /* We still have pending packets, let's call for a new scheduling */
> if (tx_q->dirty_tx != tx_q->cur_tx)
> - mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(10));
> + mod_timer(&tx_q->txtimer, STMMAC_COAL_TIMER(priv->tx_coal_timer));

I think intent of this code is to re-check the ring soon. The same
value of 10 is used in stmmac_tx_timer() for quick re-check.

tx_coal_timer defaults to 1000, so it's quite a jump from 10 to 1000.

I think the commit message leaves too much unsaid.

Also if you want to change to the ethtool timeout value, could you move
stmmac_tx_timer_arm() and reuse that helper?

>
> __netif_tx_unlock_bh(netdev_get_tx_queue(priv->dev, queue));
>