Re: [PATCH net-next v4 5/7] net: stmmac: support fp parameter of tc-mqprio
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Date: 2024-08-20 15:00:37
Also in:
lkml, netdev
From: Furong Xu <redacted> Date: Tue, 20 Aug 2024 17:38:33 +0800
tc-mqprio can select whether traffic classes are express or preemptible. After some traffic tests, MAC merge layer statistics are all good.
[...]
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c index 4c91fa766b13..1e87dbc9a406 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.c@@ -670,3 +670,55 @@ void dwmac5_fpe_set_add_frag_size(void __iomem *ioaddr, u32 add_frag_size) writel(value, ioaddr + MTL_FPE_CTRL_STS); } + +int dwmac5_fpe_set_preemptible_tcs(struct net_device *ndev, + struct netlink_ext_ack *extack, + unsigned long tcs) +{ + struct stmmac_priv *priv = netdev_priv(ndev);
Can't it be const?
+ void __iomem *ioaddr = priv->ioaddr; + unsigned long queue_tcs = 0;
Why is @tcs and @queue_tcs unsigned long? You write @queue_tcs via writel(), IOW it can't go past %U32_MAX.
+ int num_tc = ndev->num_tc; + u32 value, queue_weight; + u16 offset, count;
Just use u32 here for all three.
+ int tc, i;
+
+ if (!tcs)
+ goto __update_queue_tcs;
+
+ for (tc = 0; tc < num_tc; tc++) {@tc can be declared right here in the loop declaration. Also it's u32 as it can't be negative.
+ count = ndev->tc_to_txq[tc].count;
+ offset = ndev->tc_to_txq[tc].offset;
+
+ if (tcs & BIT(tc))
+ queue_tcs |= GENMASK(offset + count - 1, offset);
+
+ /* This is 1:1 mapping, go to next TC */
+ if (count == 1)
+ continue;
+
+ if (priv->plat->tx_sched_algorithm == MTL_TX_ALGORITHM_SP) {
+ NL_SET_ERR_MSG_MOD(extack, "TX algorithm SP is not suitable for one TC to multiple TXQs mapping");
+ return -EINVAL;
+ }
+
+ queue_weight = priv->plat->tx_queues_cfg[offset].weight;
+ for (i = 1; i < count; i++) {Same as with @tc for everything.
+ if (queue_weight != priv->plat->tx_queues_cfg[offset + i].weight) {
+ NL_SET_ERR_MSG_FMT_MOD(extack, "TXQ weight [%u] differs across other TXQs in TC: [%u]",
+ queue_weight, tc);
+ return -EINVAL;
+ }
+ }
+ }
+
+__update_queue_tcs:Again underscores.
+ value = readl(ioaddr + MTL_FPE_CTRL_STS); + + value &= ~PEC; + value |= FIELD_PREP(PEC, queue_tcs); + + writel(value, ioaddr + MTL_FPE_CTRL_STS);
These are also u32_replace_bits() as in the previous patch.
quoted hunk ↗ jump to hunk
+ + return 0; +}diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h index e369e65920fc..d3191c48354d 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac5.h +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac5.h@@ -40,6 +40,7 @@ #define MAC_PPSx_WIDTH(x) (0x00000b8c + ((x) * 0x10)) #define MTL_FPE_CTRL_STS 0x00000c90 +#define PEC GENMASK(15, 8)
Again some driver prefix would be nice to see.
#define AFSZ GENMASK(1, 0) #define MTL_RXP_CONTROL_STATUS 0x00000ca0
[...]
quoted hunk ↗ jump to hunk
@@ -1174,6 +1175,13 @@ static int tc_query_caps(struct stmmac_priv *priv, struct tc_query_caps_base *base) { switch (base->type) { + case TC_SETUP_QDISC_MQPRIO: { + struct tc_mqprio_caps *caps = base->caps; + + caps->validate_queue_counts = true;
Why not base->caps->blah directly? I think it would fit 80 cols?
quoted hunk ↗ jump to hunk
+ + return 0; + } case TC_SETUP_QDISC_TAPRIO: { struct tc_taprio_caps *caps = base->caps;@@ -1190,6 +1198,78 @@ static int tc_query_caps(struct stmmac_priv *priv, } } +static void stmmac_reset_tc_mqprio(struct net_device *ndev, + struct netlink_ext_ack *extack) +{ + struct stmmac_priv *priv = netdev_priv(ndev); + + netdev_reset_tc(ndev); + netif_set_real_num_tx_queues(ndev, priv->plat->tx_queues_to_use); + + stmmac_fpe_set_preemptible_tcs(priv, ndev, extack, 0); +} + +static int tc_setup_mqprio(struct stmmac_priv *priv, + struct tc_mqprio_qopt_offload *mqprio) +{ + struct netlink_ext_ack *extack = mqprio->extack; + struct tc_mqprio_qopt *qopt = &mqprio->qopt; + struct net_device *ndev = priv->dev; + int num_stack_tx_queues = 0; + int num_tc = qopt->num_tc; + u16 offset, count;
Also u32 for most of these.
+ int tc, err;
+
+ if (!num_tc) {
+ stmmac_reset_tc_mqprio(ndev, extack);
+ return 0;
+ }
+
+ if (mqprio->preemptible_tcs && !ethtool_dev_mm_supported(ndev)) {
+ NL_SET_ERR_MSG_MOD(extack, "Device does not support preemption");
+ return -EOPNOTSUPP;
+ }
+
+ err = netdev_set_num_tc(ndev, num_tc);
+ if (err)
+ return err;
+
+ /* DWMAC CORE4+ can not programming TC:TXQ mapping to hardware."can't program" or "can't do programming" or "doesn't support programming".
+ * Synopsys Databook: + * "The number of Tx DMA channels is equal to the number of Tx queues, + * and is direct one-to-one mapping." + * + * Luckily, DWXGMAC CORE can. + * + * Thus preemptible_tcs should be handled in a per core manner. + */
[...] Thanks, Olek