* [Intel-wired-lan] [PATCH v2] igc: Correct the launchtime offset
@ 2022-09-17 4:58 Muhammad Husaini Zulkifli
2022-09-17 14:55 ` Paul Menzel
0 siblings, 1 reply; 3+ messages in thread
From: Muhammad Husaini Zulkifli @ 2022-09-17 4:58 UTC (permalink / raw)
To: intel-wired-lan; +Cc: pmenzel, muhammad.husaini.zulkifli
The launchtime offset should be corrected according to sections 7.5.2.6
Transmit Scheduling Latency of the Intel Ethernet I225/I226 Software
User Manual.
Software can compensate the latency between the transmission scheduling
and the time that packet is transmitted to the network by setting this
GTxOffset register. Without setting this register, there may be a
significant delay between the packet scheduling and the network point.
This patch help to reduce the latency for each of the link speed.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
---
drivers/net/ethernet/intel/igc/igc_defines.h | 6 ++++
drivers/net/ethernet/intel/igc/igc_main.c | 6 ++++
drivers/net/ethernet/intel/igc/igc_regs.h | 1 +
drivers/net/ethernet/intel/igc/igc_tsn.c | 30 ++++++++++++++++++++
drivers/net/ethernet/intel/igc/igc_tsn.h | 1 +
5 files changed, 44 insertions(+)
diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
index 4f9d7f013a95..46de1dc26ef0 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -400,6 +400,12 @@
#define IGC_DTXMXPKTSZ_TSN 0x19 /* 1600 bytes of max TX DMA packet size */
#define IGC_DTXMXPKTSZ_DEFAULT 0x98 /* 9728-byte Jumbo frames */
+/* Transmit Scheduling Latency */
+#define IGC_TXOFFSET_SPEED_10 0x000034BC
+#define IGC_TXOFFSET_SPEED_100 0x00000578
+#define IGC_TXOFFSET_SPEED_1000 0x0000012C
+#define IGC_TXOFFSET_SPEED_2500 0x00000578
+
/* Time Sync Interrupt Causes */
#define IGC_TSICR_SYS_WRAP BIT(0) /* SYSTIM Wrap around. */
#define IGC_TSICR_TXTS BIT(1) /* Transmit Timestamp. */
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index bf6c461e1a2a..70e0ae7f99d9 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -5382,6 +5382,12 @@ static void igc_watchdog_task(struct work_struct *work)
break;
}
+ /* Once the launch time has been set on the wire, there is a delay
+ * before the link speed can be determined based on link up activity.
+ * Write into the register as soon as we know the correct link speed.
+ */
+ igc_tsn_adjust_txtime_offset(adapter);
+
if (adapter->link_speed != SPEED_1000)
goto no_wait;
diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
index c0d8214148d1..01c86d36856d 100644
--- a/drivers/net/ethernet/intel/igc/igc_regs.h
+++ b/drivers/net/ethernet/intel/igc/igc_regs.h
@@ -224,6 +224,7 @@
/* Transmit Scheduling Registers */
#define IGC_TQAVCTRL 0x3570
#define IGC_TXQCTL(_n) (0x3344 + 0x4 * (_n))
+#define IGC_GTXOFFSET 0x3310
#define IGC_BASET_L 0x3314
#define IGC_BASET_H 0x3318
#define IGC_QBVCYCLET 0x331C
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 0fce22de2ab8..f975ed807da1 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -48,6 +48,35 @@ static unsigned int igc_tsn_new_flags(struct igc_adapter *adapter)
return new_flags;
}
+void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter)
+{
+ struct igc_hw *hw = &adapter->hw;
+ u16 txoffset;
+
+ if (!is_any_launchtime(adapter))
+ return;
+
+ switch (adapter->link_speed) {
+ case SPEED_10:
+ txoffset = IGC_TXOFFSET_SPEED_10;
+ break;
+ case SPEED_100:
+ txoffset = IGC_TXOFFSET_SPEED_100;
+ break;
+ case SPEED_1000:
+ txoffset = IGC_TXOFFSET_SPEED_1000;
+ break;
+ case SPEED_2500:
+ txoffset = IGC_TXOFFSET_SPEED_2500;
+ break;
+ default:
+ txoffset = 0;
+ break;
+ }
+
+ wr32(IGC_GTXOFFSET, txoffset);
+}
+
/* Returns the TSN specific registers to their default values after
* the adapter is reset.
*/
@@ -57,6 +86,7 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
u32 tqavctrl;
int i;
+ wr32(IGC_GTXOFFSET, 0);
wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT);
wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT);
diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h
index 1512307f5a52..b53e6af560b7 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.h
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
@@ -6,5 +6,6 @@
int igc_tsn_offload_apply(struct igc_adapter *adapter);
int igc_tsn_reset(struct igc_adapter *adapter);
+void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter);
#endif /* _IGC_BASE_H */
--
2.17.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Intel-wired-lan] [PATCH v2] igc: Correct the launchtime offset
2022-09-17 4:58 [Intel-wired-lan] [PATCH v2] igc: Correct the launchtime offset Muhammad Husaini Zulkifli
@ 2022-09-17 14:55 ` Paul Menzel
2022-09-17 16:57 ` Zulkifli, Muhammad Husaini
0 siblings, 1 reply; 3+ messages in thread
From: Paul Menzel @ 2022-09-17 14:55 UTC (permalink / raw)
To: Muhammad Husaini Zulkifli; +Cc: intel-wired-lan
Dear Muhammad,
Thank you for the patch.
Am 17.09.22 um 06:58 schrieb Muhammad Husaini Zulkifli:
> The launchtime offset should be corrected according to sections 7.5.2.6
> Transmit Scheduling Latency of the Intel Ethernet I225/I226 Software
> User Manual.
Please mention describe the current state. What is the current
launchtime offset, and what problems does it cause.
> Software can compensate the latency between the transmission scheduling
> and the time that packet is transmitted to the network by setting this
> GTxOffset register. Without setting this register, there may be a
> significant delay between the packet scheduling and the network point.
Please document the test setup and numbers in the commit message.
> This patch help to reduce the latency for each of the link speed.
help*s*
> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@intel.com>
> ---
> drivers/net/ethernet/intel/igc/igc_defines.h | 6 ++++
> drivers/net/ethernet/intel/igc/igc_main.c | 6 ++++
> drivers/net/ethernet/intel/igc/igc_regs.h | 1 +
> drivers/net/ethernet/intel/igc/igc_tsn.c | 30 ++++++++++++++++++++
> drivers/net/ethernet/intel/igc/igc_tsn.h | 1 +
> 5 files changed, 44 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
> index 4f9d7f013a95..46de1dc26ef0 100644
> --- a/drivers/net/ethernet/intel/igc/igc_defines.h
> +++ b/drivers/net/ethernet/intel/igc/igc_defines.h
> @@ -400,6 +400,12 @@
> #define IGC_DTXMXPKTSZ_TSN 0x19 /* 1600 bytes of max TX DMA packet size */
> #define IGC_DTXMXPKTSZ_DEFAULT 0x98 /* 9728-byte Jumbo frames */
>
> +/* Transmit Scheduling Latency */
> +#define IGC_TXOFFSET_SPEED_10 0x000034BC
> +#define IGC_TXOFFSET_SPEED_100 0x00000578
> +#define IGC_TXOFFSET_SPEED_1000 0x0000012C
> +#define IGC_TXOFFSET_SPEED_2500 0x00000578
> +
Latency sounds like a time value. Can a unit be added to the macros?
Maybe add as a comment, what latency it actually is.
> /* Time Sync Interrupt Causes */
> #define IGC_TSICR_SYS_WRAP BIT(0) /* SYSTIM Wrap around. */
> #define IGC_TSICR_TXTS BIT(1) /* Transmit Timestamp. */
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index bf6c461e1a2a..70e0ae7f99d9 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -5382,6 +5382,12 @@ static void igc_watchdog_task(struct work_struct *work)
> break;
> }
>
> + /* Once the launch time has been set on the wire, there is a delay
> + * before the link speed can be determined based on link up activity.
link-up or linkup?
> + * Write into the register as soon as we know the correct link speed.
> + */
> + igc_tsn_adjust_txtime_offset(adapter);
> +
> if (adapter->link_speed != SPEED_1000)
> goto no_wait;
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
> index c0d8214148d1..01c86d36856d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_regs.h
> +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
> @@ -224,6 +224,7 @@
> /* Transmit Scheduling Registers */
> #define IGC_TQAVCTRL 0x3570
> #define IGC_TXQCTL(_n) (0x3344 + 0x4 * (_n))
> +#define IGC_GTXOFFSET 0x3310
> #define IGC_BASET_L 0x3314
> #define IGC_BASET_H 0x3318
> #define IGC_QBVCYCLET 0x331C
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 0fce22de2ab8..f975ed807da1 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -48,6 +48,35 @@ static unsigned int igc_tsn_new_flags(struct igc_adapter *adapter)
> return new_flags;
> }
>
> +void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter)
> +{
> + struct igc_hw *hw = &adapter->hw;
> + u16 txoffset;
> +
> + if (!is_any_launchtime(adapter))
> + return;
> +
> + switch (adapter->link_speed) {
> + case SPEED_10:
> + txoffset = IGC_TXOFFSET_SPEED_10;
> + break;
> + case SPEED_100:
> + txoffset = IGC_TXOFFSET_SPEED_100;
> + break;
> + case SPEED_1000:
> + txoffset = IGC_TXOFFSET_SPEED_1000;
> + break;
> + case SPEED_2500:
> + txoffset = IGC_TXOFFSET_SPEED_2500;
> + break;
> + default:
> + txoffset = 0;
> + break;
> + }
> +
> + wr32(IGC_GTXOFFSET, txoffset);
Shouldn’t txoffset be u32 then?
> +}
> +
> /* Returns the TSN specific registers to their default values after
> * the adapter is reset.
> */
> @@ -57,6 +86,7 @@ static int igc_tsn_disable_offload(struct igc_adapter *adapter)
> u32 tqavctrl;
> int i;
>
> + wr32(IGC_GTXOFFSET, 0);
> wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT);
> wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT);
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h b/drivers/net/ethernet/intel/igc/igc_tsn.h
> index 1512307f5a52..b53e6af560b7 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.h
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
> @@ -6,5 +6,6 @@
>
> int igc_tsn_offload_apply(struct igc_adapter *adapter);
> int igc_tsn_reset(struct igc_adapter *adapter);
> +void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter);
>
> #endif /* _IGC_BASE_H */
Kind regards,
Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Intel-wired-lan] [PATCH v2] igc: Correct the launchtime offset
2022-09-17 14:55 ` Paul Menzel
@ 2022-09-17 16:57 ` Zulkifli, Muhammad Husaini
0 siblings, 0 replies; 3+ messages in thread
From: Zulkifli, Muhammad Husaini @ 2022-09-17 16:57 UTC (permalink / raw)
To: Paul Menzel; +Cc: intel-wired-lan
Hi Paul,
Thanks for the review.
> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Saturday, 17 September, 2022 10:56 PM
> To: Zulkifli, Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>
> Cc: intel-wired-lan@osuosl.org; Gomes, Vinicius <vinicius.gomes@intel.com>
> Subject: Re: [PATCH v2] igc: Correct the launchtime offset
>
> Dear Muhammad,
>
>
> Thank you for the patch.
>
> Am 17.09.22 um 06:58 schrieb Muhammad Husaini Zulkifli:
> > The launchtime offset should be corrected according to sections
> > 7.5.2.6 Transmit Scheduling Latency of the Intel Ethernet I225/I226
> > Software User Manual.
>
> Please mention describe the current state. What is the current launchtime
> offset, and what problems does it cause.
Noted. Will add this.
>
> > Software can compensate the latency between the transmission
> > scheduling and the time that packet is transmitted to the network by
> > setting this GTxOffset register. Without setting this register, there
> > may be a significant delay between the packet scheduling and the network
> point.
>
> Please document the test setup and numbers in the commit message.
Same as above.
>
> > This patch help to reduce the latency for each of the link speed.
>
> help*s*
Noted. Will update this.
>
> > Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
> > Signed-off-by: Muhammad Husaini Zulkifli
> > <muhammad.husaini.zulkifli@intel.com>
> > ---
> > drivers/net/ethernet/intel/igc/igc_defines.h | 6 ++++
> > drivers/net/ethernet/intel/igc/igc_main.c | 6 ++++
> > drivers/net/ethernet/intel/igc/igc_regs.h | 1 +
> > drivers/net/ethernet/intel/igc/igc_tsn.c | 30 ++++++++++++++++++++
> > drivers/net/ethernet/intel/igc/igc_tsn.h | 1 +
> > 5 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h
> > b/drivers/net/ethernet/intel/igc/igc_defines.h
> > index 4f9d7f013a95..46de1dc26ef0 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_defines.h
> > +++ b/drivers/net/ethernet/intel/igc/igc_defines.h
> > @@ -400,6 +400,12 @@
> > #define IGC_DTXMXPKTSZ_TSN 0x19 /* 1600 bytes of max TX DMA
> packet size */
> > #define IGC_DTXMXPKTSZ_DEFAULT 0x98 /* 9728-byte Jumbo frames */
> >
> > +/* Transmit Scheduling Latency */
> > +#define IGC_TXOFFSET_SPEED_10 0x000034BC
> > +#define IGC_TXOFFSET_SPEED_100 0x00000578
> > +#define IGC_TXOFFSET_SPEED_1000 0x0000012C
> > +#define IGC_TXOFFSET_SPEED_2500 0x00000578
> > +
>
> Latency sounds like a time value. Can a unit be added to the macros?
> Maybe add as a comment, what latency it actually is.
Will add the comment like this "Latency between transmission scheduling (launch time) and the time
the packet is transmitted to the network in Nano Seconds". Will remain the macros as it is if possible
without the unit.
>
> > /* Time Sync Interrupt Causes */
> > #define IGC_TSICR_SYS_WRAP BIT(0) /* SYSTIM Wrap around. */
> > #define IGC_TSICR_TXTS BIT(1) /* Transmit Timestamp. */
> > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> > b/drivers/net/ethernet/intel/igc/igc_main.c
> > index bf6c461e1a2a..70e0ae7f99d9 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > @@ -5382,6 +5382,12 @@ static void igc_watchdog_task(struct
> work_struct *work)
> > break;
> > }
> >
> > + /* Once the launch time has been set on the wire,
> there is a delay
> > + * before the link speed can be determined based on
> link up activity.
>
> link-up or linkup?
Will change to link-up. Thanks!
>
> > + * Write into the register as soon as we know the
> correct link speed.
> > + */
> > + igc_tsn_adjust_txtime_offset(adapter);
> > +
> > if (adapter->link_speed != SPEED_1000)
> > goto no_wait;
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h
> > b/drivers/net/ethernet/intel/igc/igc_regs.h
> > index c0d8214148d1..01c86d36856d 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_regs.h
> > +++ b/drivers/net/ethernet/intel/igc/igc_regs.h
> > @@ -224,6 +224,7 @@
> > /* Transmit Scheduling Registers */
> > #define IGC_TQAVCTRL 0x3570
> > #define IGC_TXQCTL(_n) (0x3344 + 0x4 * (_n))
> > +#define IGC_GTXOFFSET 0x3310
> > #define IGC_BASET_L 0x3314
> > #define IGC_BASET_H 0x3318
> > #define IGC_QBVCYCLET 0x331C
> > diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c
> > b/drivers/net/ethernet/intel/igc/igc_tsn.c
> > index 0fce22de2ab8..f975ed807da1 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> > @@ -48,6 +48,35 @@ static unsigned int igc_tsn_new_flags(struct
> igc_adapter *adapter)
> > return new_flags;
> > }
> >
> > +void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter) {
> > + struct igc_hw *hw = &adapter->hw;
> > + u16 txoffset;
> > +
> > + if (!is_any_launchtime(adapter))
> > + return;
> > +
> > + switch (adapter->link_speed) {
> > + case SPEED_10:
> > + txoffset = IGC_TXOFFSET_SPEED_10;
> > + break;
> > + case SPEED_100:
> > + txoffset = IGC_TXOFFSET_SPEED_100;
> > + break;
> > + case SPEED_1000:
> > + txoffset = IGC_TXOFFSET_SPEED_1000;
> > + break;
> > + case SPEED_2500:
> > + txoffset = IGC_TXOFFSET_SPEED_2500;
> > + break;
> > + default:
> > + txoffset = 0;
> > + break;
> > + }
> > +
> > + wr32(IGC_GTXOFFSET, txoffset);
>
> Shouldn’t txoffset be u32 then?
I accidentally replied 2 message previously. Txoffset should be 16 bit only.
That is the reason I used u16.
>
> > +}
> > +
> > /* Returns the TSN specific registers to their default values after
> > * the adapter is reset.
> > */
> > @@ -57,6 +86,7 @@ static int igc_tsn_disable_offload(struct igc_adapter
> *adapter)
> > u32 tqavctrl;
> > int i;
> >
> > + wr32(IGC_GTXOFFSET, 0);
> > wr32(IGC_TXPBS, I225_TXPBSIZE_DEFAULT);
> > wr32(IGC_DTXMXPKTSZ, IGC_DTXMXPKTSZ_DEFAULT);
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.h
> > b/drivers/net/ethernet/intel/igc/igc_tsn.h
> > index 1512307f5a52..b53e6af560b7 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_tsn.h
> > +++ b/drivers/net/ethernet/intel/igc/igc_tsn.h
> > @@ -6,5 +6,6 @@
> >
> > int igc_tsn_offload_apply(struct igc_adapter *adapter);
> > int igc_tsn_reset(struct igc_adapter *adapter);
> > +void igc_tsn_adjust_txtime_offset(struct igc_adapter *adapter);
> >
> > #endif /* _IGC_BASE_H */
>
>
> Kind regards,
>
> Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-17 16:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-17 4:58 [Intel-wired-lan] [PATCH v2] igc: Correct the launchtime offset Muhammad Husaini Zulkifli
2022-09-17 14:55 ` Paul Menzel
2022-09-17 16:57 ` Zulkifli, Muhammad Husaini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.