Re: [PATCH net-next 2/2] net: stmmac: TBS support for platform driver

From: Abhishek Chauhan (ABC)
Date: Wed Jan 10 2024 - 15:22:26 EST


Qualcomm had similar discussions with respect to enabling of TBS for a particular queue.
We had similar discussion on these terms yesterday with Redhat. Adding Andrew from Redhat here

What we discovered as part of the discussions is listed below.

1. Today upstream stmmac code is designed in such a way that TBS flag is put as
part of queue configurations(see below snippet) and as well know that stmmac queue
configuration comes from the dtsi file.

//ndo_open => stmmac_open
int tbs_en = priv->plat->tx_queues_cfg[chan].tbs_en;(comes from tx_queues_cfg)

/* Setup per-TXQ tbs flag before TX descriptor alloc */
tx_q->tbs |= tbs_en ? STMMAC_TBS_AVAIL : 0;

2. There is a no way to do this dynamically from user space because we don't have any
API exposed which can do it from user space and also TBS rely on special descriptors
aka enhanced desc this cannot be done run time and stmmac has to be aware of it before
we do DMA/MAC/MTL start. To do this dynamically would only mean stopping DMA/MAC/MTL
realloc resources for enhanced desc and the starting MAC/DMA/MTL. This means we are
disrupting other traffic(By stopping MAC block).

3. I dont think there is a way we can enable this dynamically today. I would like upstream
community to share your thoughts as well.

4. I agree with Rohan's patch here and want upstream community to accept it. This will allow
use to configure the queues where TBS needs to be enabled as hardcoding in the code unless upstream
has better way to this using userspace.

Please let us know if you think otherwise.


On 9/27/2023 6:09 AM, Rohan G Thomas wrote:
> Enable Time Based Scheduling(TBS) support for Tx queues through the
> stmmac platform driver. For this a new per-queue tx-config property,
> tbs-enabled is added to the devicetree.
>
> Commit 7eadf57290ec ("net: stmmac: pci: Enable TBS on GMAC5 IPK PCI
> entry") enables similar support for the stmmac pci driver.
>
> Also add check whether TBS support is available for a Tx DMA channel
> before enabling TBS support for that Tx DMA channel.
>
> Signed-off-by: Rohan G Thomas <rohan.g.thomas@xxxxxxxxx>
> ---
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 25 +++++++++++++++----
> .../ethernet/stmicro/stmmac/stmmac_platform.c | 4 +++
> 2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 81b6f3ecdf92..7333f0640b3d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -3773,12 +3773,18 @@ stmmac_setup_dma_desc(struct stmmac_priv *priv, unsigned int mtu)
> dma_conf->dma_rx_size = DMA_DEFAULT_RX_SIZE;
>
> /* Earlier check for TBS */
> - for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++) {
> - struct stmmac_tx_queue *tx_q = &dma_conf->tx_queue[chan];
> - int tbs_en = priv->plat->tx_queues_cfg[chan].tbs_en;
> + if (priv->dma_cap.tbssel) {
> + /* TBS is available only for tbs_ch_num of Tx DMA channels,
> + * starting from the highest Tx DMA channel.
> + */
> + chan = priv->dma_cap.number_tx_channel - priv->dma_cap.tbs_ch_num;
> + for (; chan < priv->plat->tx_queues_to_use; chan++) {
> + struct stmmac_tx_queue *tx_q = &dma_conf->tx_queue[chan];
> + int tbs_en = priv->plat->tx_queues_cfg[chan].tbs_en;
>
> - /* Setup per-TXQ tbs flag before TX descriptor alloc */
> - tx_q->tbs |= tbs_en ? STMMAC_TBS_AVAIL : 0;
> + /* Setup per-TXQ tbs flag before TX descriptor alloc */
> + tx_q->tbs |= tbs_en ? STMMAC_TBS_AVAIL : 0;
> + }
> }
>
> ret = alloc_dma_desc_resources(priv, dma_conf);
> @@ -7505,6 +7511,15 @@ int stmmac_dvr_probe(struct device *device,
> }
> }
>
> + /* If TBS feature is supported(i.e. tbssel is true), then at least 1 Tx
> + * DMA channel supports TBS. So if tbs_ch_num is 0 and tbssel is true,
> + * assume all Tx DMA channels support TBS. TBS_CH field, which gives
> + * number of Tx DMA channels with TBS support is only available only for
> + * DW xGMAC IP. For other DWMAC IPs all Tx DMA channels can support TBS.
> + */
> + if (priv->dma_cap.tbssel && !priv->dma_cap.tbs_ch_num)
> + priv->dma_cap.tbs_ch_num = priv->dma_cap.number_tx_channel;
> +
> ndev->features |= ndev->hw_features | NETIF_F_HIGHDMA;
> ndev->watchdog_timeo = msecs_to_jiffies(watchdog);
> #ifdef STMMAC_VLAN_TAG_USED
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 843bd8804bfa..6c0191c84071 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -279,6 +279,10 @@ static int stmmac_mtl_setup(struct platform_device *pdev,
> plat->tx_queues_cfg[queue].coe_unsupported =
> of_property_read_bool(q_node, "snps,coe-unsupported");
>
> + /* Select TBS for supported queues */
> + plat->tx_queues_cfg[queue].tbs_en =
> + of_property_read_bool(q_node, "snps,tbs-enabled");
> +
> queue++;
> }
> if (queue != plat->tx_queues_to_use) {