| Message ID | 20220601022900.7985-2-kai.heng.feng@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix sub-optimal I210 network speed | expand |
How about we wait until this gets merged upstream before applying it to a 22.04 kernel that we have to maintain for the next 10 years ? The fewer SAUCE patches, the better. Your bug report also does not state what the current performance is, so its hard to tell if the gains are worth the extra code maintenance. rtg On 5/31/22 20:29, Kai-Heng Feng wrote: > BugLink: https://bugs.launchpad.net/bugs/1976438 > > Intel I210 on some Intel Alder Lake platforms can only achieve ~750Mbps > Tx speed via iperf. The RR2DCDELAY shows around 0x2xxx DMA delay, which > will be significantly lower when 1) ASPM is disabled or 2) SoC package > c-state stays above PC3. When the RR2DCDELAY is around 0x1xxx the Tx > speed can reach to ~950Mbps. > > According to the I210 datasheet "8.26.1 PCIe Misc. Register - PCIEMISC", > "DMA Idle Indication" doesn't seem to tie to DMA coalesce anymore, so > set it to 1b for "DMA is considered idle when there is no Rx or Tx AND > when there are no TLPs indicating that CPU is active detected on the > PCIe link (such as the host executes CSR or Configuration register read > or write operation)" and performing Tx should also fall under "active > CPU on PCIe link" case. > > In addition to that, commit b6e0c419f040 ("igb: Move DMA Coalescing init > code to separate function.") seems to wrongly changed from enabling > E1000_PCIEMISC_LX_DECISION to disabling it, also fix that. > > Fixes: b6e0c419f040 ("igb: Move DMA Coalescing init code to separate function.") > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > (cherry picked from commit 071ab8cbab26883af46b6ff1eb70616938350275 git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue) > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > --- > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c > index 68be2976f539f..c0d93fd19c1ed 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -9898,11 +9898,10 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) > struct e1000_hw *hw = &adapter->hw; > u32 dmac_thr; > u16 hwm; > + u32 reg; > > if (hw->mac.type > e1000_82580) { > if (adapter->flags & IGB_FLAG_DMAC) { > - u32 reg; > - > /* force threshold to 0. */ > wr32(E1000_DMCTXTH, 0); > > @@ -9935,7 +9934,6 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) > /* Disable BMC-to-OS Watchdog Enable */ > if (hw->mac.type != e1000_i354) > reg &= ~E1000_DMACR_DC_BMC2OSW_EN; > - > wr32(E1000_DMACR, reg); > > /* no lower threshold to disable > @@ -9952,12 +9950,12 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) > */ > wr32(E1000_DMCTXTH, (IGB_MIN_TXPBSIZE - > (IGB_TX_BUF_4096 + adapter->max_frame_size)) >> 6); > + } > > - /* make low power state decision controlled > - * by DMA coal > - */ > + if (hw->mac.type >= e1000_i210 || > + (adapter->flags & IGB_FLAG_DMAC)) { > reg = rd32(E1000_PCIEMISC); > - reg &= ~E1000_PCIEMISC_LX_DECISION; > + reg |= E1000_PCIEMISC_LX_DECISION; > wr32(E1000_PCIEMISC, reg); > } /* endif adapter->dmac is not disabled */ > } else if (hw->mac.type == e1000_82580) {
On Wed, Jun 1, 2022 at 10:16 PM Tim Gardner <tim.gardner@canonical.com> wrote: > > How about we wait until this gets merged upstream before applying it to > a 22.04 kernel that we have to maintain for the next 10 years ? The > fewer SAUCE patches, the better. Yea I guess we can skip Jammy for now. Upstream stable will probably pick this up automatically anyway. > > Your bug report also does not state what the current performance is, so > its hard to tell if the gains are worth the extra code maintenance. Without the fix the iperf speed is around 700Mbps. Kai-Heng > > rtg > > On 5/31/22 20:29, Kai-Heng Feng wrote: > > BugLink: https://bugs.launchpad.net/bugs/1976438 > > > > Intel I210 on some Intel Alder Lake platforms can only achieve ~750Mbps > > Tx speed via iperf. The RR2DCDELAY shows around 0x2xxx DMA delay, which > > will be significantly lower when 1) ASPM is disabled or 2) SoC package > > c-state stays above PC3. When the RR2DCDELAY is around 0x1xxx the Tx > > speed can reach to ~950Mbps. > > > > According to the I210 datasheet "8.26.1 PCIe Misc. Register - PCIEMISC", > > "DMA Idle Indication" doesn't seem to tie to DMA coalesce anymore, so > > set it to 1b for "DMA is considered idle when there is no Rx or Tx AND > > when there are no TLPs indicating that CPU is active detected on the > > PCIe link (such as the host executes CSR or Configuration register read > > or write operation)" and performing Tx should also fall under "active > > CPU on PCIe link" case. > > > > In addition to that, commit b6e0c419f040 ("igb: Move DMA Coalescing init > > code to separate function.") seems to wrongly changed from enabling > > E1000_PCIEMISC_LX_DECISION to disabling it, also fix that. > > > > Fixes: b6e0c419f040 ("igb: Move DMA Coalescing init code to separate function.") > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > (cherry picked from commit 071ab8cbab26883af46b6ff1eb70616938350275 git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue) > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > --- > > drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++------- > > 1 file changed, 5 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c > > index 68be2976f539f..c0d93fd19c1ed 100644 > > --- a/drivers/net/ethernet/intel/igb/igb_main.c > > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > > @@ -9898,11 +9898,10 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) > > struct e1000_hw *hw = &adapter->hw; > > u32 dmac_thr; > > u16 hwm; > > + u32 reg; > > > > if (hw->mac.type > e1000_82580) { > > if (adapter->flags & IGB_FLAG_DMAC) { > > - u32 reg; > > - > > /* force threshold to 0. */ > > wr32(E1000_DMCTXTH, 0); > > > > @@ -9935,7 +9934,6 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) > > /* Disable BMC-to-OS Watchdog Enable */ > > if (hw->mac.type != e1000_i354) > > reg &= ~E1000_DMACR_DC_BMC2OSW_EN; > > - > > wr32(E1000_DMACR, reg); > > > > /* no lower threshold to disable > > @@ -9952,12 +9950,12 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) > > */ > > wr32(E1000_DMCTXTH, (IGB_MIN_TXPBSIZE - > > (IGB_TX_BUF_4096 + adapter->max_frame_size)) >> 6); > > + } > > > > - /* make low power state decision controlled > > - * by DMA coal > > - */ > > + if (hw->mac.type >= e1000_i210 || > > + (adapter->flags & IGB_FLAG_DMAC)) { > > reg = rd32(E1000_PCIEMISC); > > - reg &= ~E1000_PCIEMISC_LX_DECISION; > > + reg |= E1000_PCIEMISC_LX_DECISION; > > wr32(E1000_PCIEMISC, reg); > > } /* endif adapter->dmac is not disabled */ > > } else if (hw->mac.type == e1000_82580) { > > -- > ----------- > Tim Gardner > Canonical, Inc
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 68be2976f539f..c0d93fd19c1ed 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -9898,11 +9898,10 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) struct e1000_hw *hw = &adapter->hw; u32 dmac_thr; u16 hwm; + u32 reg; if (hw->mac.type > e1000_82580) { if (adapter->flags & IGB_FLAG_DMAC) { - u32 reg; - /* force threshold to 0. */ wr32(E1000_DMCTXTH, 0); @@ -9935,7 +9934,6 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) /* Disable BMC-to-OS Watchdog Enable */ if (hw->mac.type != e1000_i354) reg &= ~E1000_DMACR_DC_BMC2OSW_EN; - wr32(E1000_DMACR, reg); /* no lower threshold to disable @@ -9952,12 +9950,12 @@ static void igb_init_dmac(struct igb_adapter *adapter, u32 pba) */ wr32(E1000_DMCTXTH, (IGB_MIN_TXPBSIZE - (IGB_TX_BUF_4096 + adapter->max_frame_size)) >> 6); + } - /* make low power state decision controlled - * by DMA coal - */ + if (hw->mac.type >= e1000_i210 || + (adapter->flags & IGB_FLAG_DMAC)) { reg = rd32(E1000_PCIEMISC); - reg &= ~E1000_PCIEMISC_LX_DECISION; + reg |= E1000_PCIEMISC_LX_DECISION; wr32(E1000_PCIEMISC, reg); } /* endif adapter->dmac is not disabled */ } else if (hw->mac.type == e1000_82580) {
BugLink: https://bugs.launchpad.net/bugs/1976438 Intel I210 on some Intel Alder Lake platforms can only achieve ~750Mbps Tx speed via iperf. The RR2DCDELAY shows around 0x2xxx DMA delay, which will be significantly lower when 1) ASPM is disabled or 2) SoC package c-state stays above PC3. When the RR2DCDELAY is around 0x1xxx the Tx speed can reach to ~950Mbps. According to the I210 datasheet "8.26.1 PCIe Misc. Register - PCIEMISC", "DMA Idle Indication" doesn't seem to tie to DMA coalesce anymore, so set it to 1b for "DMA is considered idle when there is no Rx or Tx AND when there are no TLPs indicating that CPU is active detected on the PCIe link (such as the host executes CSR or Configuration register read or write operation)" and performing Tx should also fall under "active CPU on PCIe link" case. In addition to that, commit b6e0c419f040 ("igb: Move DMA Coalescing init code to separate function.") seems to wrongly changed from enabling E1000_PCIEMISC_LX_DECISION to disabling it, also fix that. Fixes: b6e0c419f040 ("igb: Move DMA Coalescing init code to separate function.") Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> (cherry picked from commit 071ab8cbab26883af46b6ff1eb70616938350275 git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue.git dev-queue) Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/net/ethernet/intel/igb/igb_main.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)