Re: [PATCH iwl-next v2] igc: Add MQPRIO offload support

[Kurt Kanzenbach <kurt@linutronix.de>]

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

On Thu Mar 28 2024, Simon Horman wrote:
> On Tue, Mar 26, 2024 at 02:34:54PM +0100, Kurt Kanzenbach wrote:
>> Add support for offloading MQPRIO. The hardware has four priorities as well
>> as four queues. Each queue must be a assigned with a unique priority.
>> 
>> However, the priorities are only considered in TSN Tx mode. There are two
>> TSN Tx modes. In case of MQPRIO the Qbv capability is not required.
>> Therefore, use the legacy TSN Tx mode, which performs strict priority
>> arbitration.
>> 
>> Example for mqprio with hardware offload:
>> 
>> |tc qdisc replace dev ${INTERFACE} handle 100 parent root mqprio num_tc 4 \
>> |   map 0 0 0 0 0 1 2 3 0 0 0 0 0 0 0 0 \
>> |   queues 1@0 1@1 1@2 1@3 \
>> |   hw 1
>> 
>> The mqprio Qdisc also allows to configure the `preemptible_tcs'. However,
>> frame preemption is not supported yet.
>> 
>> Tested on Intel i225 and implemented by following data sheet section 7.5.2,
>> Transmit Scheduling.
>> 
>> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>
> ...
>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
>> index 5f92b3c7c3d4..73502a0b4df7 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_defines.h
>> +++ b/drivers/net/ethernet/intel/igc/igc_defines.h
>> @@ -547,6 +547,15 @@
>>  
>>  #define IGC_MAX_SR_QUEUES		2
>>  
>> +#define IGC_TXARB_TXQ_PRIO_0_SHIFT	0
>> +#define IGC_TXARB_TXQ_PRIO_1_SHIFT	2
>> +#define IGC_TXARB_TXQ_PRIO_2_SHIFT	4
>> +#define IGC_TXARB_TXQ_PRIO_3_SHIFT	6
>> +#define IGC_TXARB_TXQ_PRIO_0_MASK	GENMASK(1, 0)
>> +#define IGC_TXARB_TXQ_PRIO_1_MASK	GENMASK(3, 2)
>> +#define IGC_TXARB_TXQ_PRIO_2_MASK	GENMASK(5, 4)
>> +#define IGC_TXARB_TXQ_PRIO_3_MASK	GENMASK(7, 6)
>> +
>>  /* Receive Checksum Control */
>>  #define IGC_RXCSUM_CRCOFL	0x00000800   /* CRC32 offload enable */
>>  #define IGC_RXCSUM_PCSD		0x00002000   /* packet checksum disabled */
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
>
> ...
>
>> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
>
> ...
>
>> @@ -106,7 +109,26 @@ 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.
>> +	 */
>> +	txarb = rd32(IGC_TXARB);
>> +	txarb &= ~(IGC_TXARB_TXQ_PRIO_0_MASK |
>> +		   IGC_TXARB_TXQ_PRIO_1_MASK |
>> +		   IGC_TXARB_TXQ_PRIO_2_MASK |
>> +		   IGC_TXARB_TXQ_PRIO_3_MASK);
>> +
>> +	txarb |= 0x00 << IGC_TXARB_TXQ_PRIO_0_SHIFT;
>> +	txarb |= 0x01 << IGC_TXARB_TXQ_PRIO_1_SHIFT;
>> +	txarb |= 0x02 << IGC_TXARB_TXQ_PRIO_2_SHIFT;
>> +	txarb |= 0x03 << IGC_TXARB_TXQ_PRIO_3_SHIFT;
>> +	wr32(IGC_TXARB, txarb);
>
> Hi Kurt,
>
> It looks like the above would be a good candidate for using FIELD_PREP,
> in which case the _SHIFT #defines can likely be removed.

OK.

>
> Also, the logic above seems to be replicated in igc_tsn_enable_offload.
> Perhaps a helper is appropriate.

Makes sense.

Thanks,
Kurt

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