[PATCH iwl-next v4 0/2] igc: Change Tx mode for MQPRIO offloading

[PATCH iwl-next v4 0/2] igc: Change Tx mode for MQPRIO offloading

[Kurt Kanzenbach]

Hi,

this small series switches the Tx mode for MQPRIO offload to harmonize the 
current implementation with TAPRIO and to allow to integrate FPE easier.

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
Changes in v4:
- Split patches (Simon)
- Add tags
- Link to v3: https://lore.kernel.org/r/20250303-igc_mqprio_tx_mode-v3-1-0efce85e6ae0@linutronix.de

Changes in v3:
- Commit message (Paul)
- Link to v2: https://lore.kernel.org/r/20250224-igc_mqprio_tx_mode-v2-1-9666da13c8d8@linutronix.de

Changes in v2:
- Add comma to commit message (Faizal)
- Simplify if condition (Faizal)
- Link to v1: https://lore.kernel.org/r/20250217-igc_mqprio_tx_mode-v1-1-3a402fe1f326@linutronix.de

---
Kurt Kanzenbach (2):
      igc: Limit netdev_tc calls to MQPRIO
      igc: Change Tx mode for MQPRIO offloading

 drivers/net/ethernet/intel/igc/igc.h      |  4 +---
 drivers/net/ethernet/intel/igc/igc_main.c | 18 +++++++++++++-
 drivers/net/ethernet/intel/igc/igc_tsn.c  | 40 ++-----------------------------
 3 files changed, 20 insertions(+), 42 deletions(-)
---
base-commit: 6f13bec53a48c7120dc6dc358cacea13251a471f
change-id: 20250214-igc_mqprio_tx_mode-22c19c564605

Best regards,
-- 
Kurt Kanzenbach <kurt@linutronix.de>

[PATCH iwl-next v4 1/2] igc: Limit netdev_tc calls to MQPRIO

[Kurt Kanzenbach]

Limit netdev_tc calls to MQPRIO. Currently these calls are made in
igc_tsn_enable_offload() and igc_tsn_disable_offload() which are used by
TAPRIO and ETF as well. However, these are only required for MQPRIO.

Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 18 +++++++++++++++++-
 drivers/net/ethernet/intel/igc/igc_tsn.c  | 20 --------------------
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 472f009630c98e60c7166ceb8d05cb094f6c837b..240b6075197fb1e61077a736ddf8f9e67c1ed5cd 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -6679,13 +6679,14 @@ static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 				 struct tc_mqprio_qopt_offload *mqprio)
 {
 	struct igc_hw *hw = &adapter->hw;
-	int i;
+	int err, i;
 
 	if (hw->mac.type != igc_i225)
 		return -EOPNOTSUPP;
 
 	if (!mqprio->qopt.num_tc) {
 		adapter->strict_priority_enable = false;
+		netdev_reset_tc(adapter->netdev);
 		goto apply;
 	}
 
@@ -6716,6 +6717,21 @@ static int igc_tsn_enable_mqprio(struct igc_adapter *adapter,
 	igc_save_mqprio_params(adapter, mqprio->qopt.num_tc,
 			       mqprio->qopt.offset);
 
+	err = netdev_set_num_tc(adapter->netdev, adapter->num_tc);
+	if (err)
+		return err;
+
+	for (i = 0; i < adapter->num_tc; i++) {
+		err = netdev_set_tc_queue(adapter->netdev, i, 1,
+					  adapter->queue_per_tc[i]);
+		if (err)
+			return err;
+	}
+
+	/* In case the card is configured with less than four queues. */
+	for (; i < IGC_MAX_TX_QUEUES; i++)
+		adapter->queue_per_tc[i] = i;
+
 	mqprio->qopt.hw = TC_MQPRIO_HW_OFFLOAD_TCS;
 
 apply:
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 1e44374ca1ffbb86e9893266c590f318984ef574..67632356083708f82a099141a4b68ba10e06f952 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -157,9 +157,6 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
 	wr32(IGC_QBVCYCLET_S, 0);
 	wr32(IGC_QBVCYCLET, NSEC_PER_SEC);
 
-	/* Reset mqprio TC configuration. */
-	netdev_reset_tc(adapter->netdev);
-
 	/* Restore the default Tx arbitration: Priority 0 has the highest
 	 * priority and is assigned to queue 0 and so on and so forth.
 	 */
@@ -206,23 +203,6 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 		igc_tsn_set_retx_qbvfullthreshold(adapter);
 
 	if (adapter->strict_priority_enable) {
-		int err;
-
-		err = netdev_set_num_tc(adapter->netdev, adapter->num_tc);
-		if (err)
-			return err;
-
-		for (i = 0; i < adapter->num_tc; i++) {
-			err = netdev_set_tc_queue(adapter->netdev, i, 1,
-						  adapter->queue_per_tc[i]);
-			if (err)
-				return err;
-		}
-
-		/* In case the card is configured with less than four queues. */
-		for (; i < IGC_MAX_TX_QUEUES; i++)
-			adapter->queue_per_tc[i] = i;
-
 		/* Configure queue priorities according to the user provided
 		 * mapping.
 		 */

-- 
2.39.5

Re: [PATCH iwl-next v4 1/2] igc: Limit netdev_tc calls to MQPRIO

[Simon Horman]

On Fri, Mar 21, 2025 at 02:52:38PM +0100, Kurt Kanzenbach wrote:
> Limit netdev_tc calls to MQPRIO. Currently these calls are made in
> igc_tsn_enable_offload() and igc_tsn_disable_offload() which are used by
> TAPRIO and ETF as well. However, these are only required for MQPRIO.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>

Hi Kurt,

Thanks for the update. And I apologise that I now have question.

I see that:

* This patch moves logic from igc_tsn_disable_offload()
  and igc_tsn_enable_offload() to igc_tsn_enable_mqprio().

* That both igc_tsn_disable_offload() and igc_tsn_enable_offload()
  are only called from igc_tsn_reset().

* And that based on the description, this looks good for the case
  where igc_tsn_reset() is called from igc_tsn_offload_apply().
  This is because igc_tsn_offload_apply() is called from
  igc_tsn_enable_mqprio().

All good so far.

But my question is about the case where igc_tsn_reset() is called from
igc_reset(). Does the logic previously present in igc_tsn_enable_offload()
and igc_tsn_disable_offload() need to run in that case? And, if so,
how is that handled?

Re: [PATCH iwl-next v4 1/2] igc: Limit netdev_tc calls to MQPRIO

[Kurt Kanzenbach]

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

On Mon Apr 07 2025, Simon Horman wrote:
> On Fri, Mar 21, 2025 at 02:52:38PM +0100, Kurt Kanzenbach wrote:
>> Limit netdev_tc calls to MQPRIO. Currently these calls are made in
>> igc_tsn_enable_offload() and igc_tsn_disable_offload() which are used by
>> TAPRIO and ETF as well. However, these are only required for MQPRIO.
>> 
>> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>
> Hi Kurt,
>
> Thanks for the update. And I apologise that I now have question.
>
> I see that:
>
> * This patch moves logic from igc_tsn_disable_offload()
>   and igc_tsn_enable_offload() to igc_tsn_enable_mqprio().
>
> * That both igc_tsn_disable_offload() and igc_tsn_enable_offload()
>   are only called from igc_tsn_reset().
>
> * And that based on the description, this looks good for the case
>   where igc_tsn_reset() is called from igc_tsn_offload_apply().
>   This is because igc_tsn_offload_apply() is called from
>   igc_tsn_enable_mqprio().
>
> All good so far.
>
> But my question is about the case where igc_tsn_reset() is called from
> igc_reset(). Does the logic previously present in igc_tsn_enable_offload()
> and igc_tsn_disable_offload() need to run in that case?

This patch moves the netdev_tc calls only. These do not have to run in
this case. The hardware configuration is still applied in
igc_tsn_enable_offload() and igc_tsn_disable_offload().

Thanks,
Kurt

> And, if so, how is that handled?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH iwl-next v4 1/2] igc: Limit netdev_tc calls to MQPRIO

[Simon Horman]

On Tue, Apr 08, 2025 at 02:04:21PM +0200, Kurt Kanzenbach wrote:
> On Mon Apr 07 2025, Simon Horman wrote:
> > On Fri, Mar 21, 2025 at 02:52:38PM +0100, Kurt Kanzenbach wrote:
> >> Limit netdev_tc calls to MQPRIO. Currently these calls are made in
> >> igc_tsn_enable_offload() and igc_tsn_disable_offload() which are used by
> >> TAPRIO and ETF as well. However, these are only required for MQPRIO.
> >> 
> >> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> >
> > Hi Kurt,
> >
> > Thanks for the update. And I apologise that I now have question.
> >
> > I see that:
> >
> > * This patch moves logic from igc_tsn_disable_offload()
> >   and igc_tsn_enable_offload() to igc_tsn_enable_mqprio().
> >
> > * That both igc_tsn_disable_offload() and igc_tsn_enable_offload()
> >   are only called from igc_tsn_reset().
> >
> > * And that based on the description, this looks good for the case
> >   where igc_tsn_reset() is called from igc_tsn_offload_apply().
> >   This is because igc_tsn_offload_apply() is called from
> >   igc_tsn_enable_mqprio().
> >
> > All good so far.
> >
> > But my question is about the case where igc_tsn_reset() is called from
> > igc_reset(). Does the logic previously present in igc_tsn_enable_offload()
> > and igc_tsn_disable_offload() need to run in that case?
> 
> This patch moves the netdev_tc calls only. These do not have to run in
> this case. The hardware configuration is still applied in
> igc_tsn_enable_offload() and igc_tsn_disable_offload().

Thanks for clarifying, in that case this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next v4 1/2] igc: Limit netdev_tc calls to MQPRIO

[Mor Bar-Gabay]

On 21/03/2025 15:52, Kurt Kanzenbach wrote:
> Limit netdev_tc calls to MQPRIO. Currently these calls are made in
> igc_tsn_enable_offload() and igc_tsn_disable_offload() which are used by
> TAPRIO and ETF as well. However, these are only required for MQPRIO.
> 
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> Reviewed-by: Simon Horman <horms@kernel.org>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 18 +++++++++++++++++-
>   drivers/net/ethernet/intel/igc/igc_tsn.c  | 20 --------------------
>   2 files changed, 17 insertions(+), 21 deletions(-)
> 
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>

[PATCH iwl-next v4 2/2] igc: Change Tx mode for MQPRIO offloading

[Kurt Kanzenbach]

The current MQPRIO offload implementation uses the legacy TSN Tx mode. In
this mode the hardware uses four packet buffers and considers queue
priorities.

In order to harmonize the TAPRIO implementation with MQPRIO, switch to the
regular TSN Tx mode. This mode also uses four packet buffers and considers
queue priorities. In addition to the legacy mode, transmission is always
coupled to Qbv. The driver already has mechanisms to use a dummy schedule
of 1 second with all gates open for ETF. Simply use this for MQPRIO too.

This reduces code and makes it easier to add support for frame preemption
later.

Tested on i225 with real time application using high priority queue, iperf3
using low priority queue and network TAP device.

Acked-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
---
 drivers/net/ethernet/intel/igc/igc.h     |  4 +---
 drivers/net/ethernet/intel/igc/igc_tsn.c | 20 ++------------------
 2 files changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index cd1d7b6c1782352094f6867a31b6958c929bbbf4..16d85bdf55a7e9c412c47acf727bca6bc7154c61 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -388,11 +388,9 @@ extern char igc_driver_name[];
 #define IGC_FLAG_RX_LEGACY		BIT(16)
 #define IGC_FLAG_TSN_QBV_ENABLED	BIT(17)
 #define IGC_FLAG_TSN_QAV_ENABLED	BIT(18)
-#define IGC_FLAG_TSN_LEGACY_ENABLED	BIT(19)
 
 #define IGC_FLAG_TSN_ANY_ENABLED				\
-	(IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED |	\
-	 IGC_FLAG_TSN_LEGACY_ENABLED)
+	(IGC_FLAG_TSN_QBV_ENABLED | IGC_FLAG_TSN_QAV_ENABLED)
 
 #define IGC_FLAG_RSS_FIELD_IPV4_UDP	BIT(6)
 #define IGC_FLAG_RSS_FIELD_IPV6_UDP	BIT(7)
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 67632356083708f82a099141a4b68ba10e06f952..7c28f3e7bb576f0e6a21c883e934ede4d53096f4 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -37,18 +37,13 @@ static unsigned int igc_tsn_new_flags(struct igc_adapter *adapter)
 {
 	unsigned int new_flags = adapter->flags & ~IGC_FLAG_TSN_ANY_ENABLED;
 
-	if (adapter->taprio_offload_enable)
-		new_flags |= IGC_FLAG_TSN_QBV_ENABLED;
-
-	if (is_any_launchtime(adapter))
+	if (adapter->taprio_offload_enable || is_any_launchtime(adapter) ||
+	    adapter->strict_priority_enable)
 		new_flags |= IGC_FLAG_TSN_QBV_ENABLED;
 
 	if (is_cbs_enabled(adapter))
 		new_flags |= IGC_FLAG_TSN_QAV_ENABLED;
 
-	if (adapter->strict_priority_enable)
-		new_flags |= IGC_FLAG_TSN_LEGACY_ENABLED;
-
 	return new_flags;
 }
 
@@ -163,7 +158,6 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
 	igc_tsn_tx_arb(adapter, queue_per_tc);
 
 	adapter->flags &= ~IGC_FLAG_TSN_QBV_ENABLED;
-	adapter->flags &= ~IGC_FLAG_TSN_LEGACY_ENABLED;
 
 	return 0;
 }
@@ -207,16 +201,6 @@ static int igc_tsn_enable_offload(struct igc_adapter *adapter)
 		 * mapping.
 		 */
 		igc_tsn_tx_arb(adapter, adapter->queue_per_tc);
-
-		/* Enable legacy TSN mode which will do strict priority without
-		 * any other TSN features.
-		 */
-		tqavctrl = rd32(IGC_TQAVCTRL);
-		tqavctrl |= IGC_TQAVCTRL_TRANSMIT_MODE_TSN;
-		tqavctrl &= ~IGC_TQAVCTRL_ENHANCED_QAV;
-		wr32(IGC_TQAVCTRL, tqavctrl);
-
-		return 0;
 	}
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {

-- 
2.39.5

Re: [PATCH iwl-next v4 2/2] igc: Change Tx mode for MQPRIO offloading

[Simon Horman]

On Fri, Mar 21, 2025 at 02:52:39PM +0100, Kurt Kanzenbach wrote:
> The current MQPRIO offload implementation uses the legacy TSN Tx mode. In
> this mode the hardware uses four packet buffers and considers queue
> priorities.
> 
> In order to harmonize the TAPRIO implementation with MQPRIO, switch to the
> regular TSN Tx mode. This mode also uses four packet buffers and considers
> queue priorities. In addition to the legacy mode, transmission is always
> coupled to Qbv. The driver already has mechanisms to use a dummy schedule
> of 1 second with all gates open for ETF. Simply use this for MQPRIO too.
> 
> This reduces code and makes it easier to add support for frame preemption
> later.
> 
> Tested on i225 with real time application using high priority queue, iperf3
> using low priority queue and network TAP device.
> 
> Acked-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next v4 2/2] igc: Change Tx mode for MQPRIO offloading

[Mor Bar-Gabay]

On 21/03/2025 15:52, Kurt Kanzenbach wrote:
> The current MQPRIO offload implementation uses the legacy TSN Tx mode. In
> this mode the hardware uses four packet buffers and considers queue
> priorities.
> 
> In order to harmonize the TAPRIO implementation with MQPRIO, switch to the
> regular TSN Tx mode. This mode also uses four packet buffers and considers
> queue priorities. In addition to the legacy mode, transmission is always
> coupled to Qbv. The driver already has mechanisms to use a dummy schedule
> of 1 second with all gates open for ETF. Simply use this for MQPRIO too.
> 
> This reduces code and makes it easier to add support for frame preemption
> later.
> 
> Tested on i225 with real time application using high priority queue, iperf3
> using low priority queue and network TAP device.
> 
> Acked-by: Faizal Rahim <faizal.abdul.rahim@linux.intel.com>
> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
> Reviewed-by: Simon Horman <horms@kernel.org>
> ---
>   drivers/net/ethernet/intel/igc/igc.h     |  4 +---
>   drivers/net/ethernet/intel/igc/igc_tsn.c | 20 ++------------------
>   2 files changed, 3 insertions(+), 21 deletions(-)
> 
Tested-by: Mor Bar-Gabay <morx.bar.gabay@intel.com>