netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll
@ 2023-03-06 21:17 Heiner Kallweit
  2023-03-06 21:23 ` [PATCH net-next 1/6] r8169: use spinlock to protect mac ocp register access Heiner Kallweit
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:17 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

This is a rework of ideas from Kai-Heng on how to avoid the known
ASPM issues whilst still allowing for a maximum of ASPM-related power
savings. As a prerequisite some locking is added first.

Heiner Kallweit (6):
  r8169: use spinlock to protect mac ocp register access
  r8169: use spinlock to protect access to registers Config2 and Config5
  r8169: enable cfg9346 config register access in atomic context
  r8169: prepare rtl_hw_aspm_clkreq_enable for usage in atomic context
  r8169: disable ASPM during NAPI poll
  r8169: remove ASPM restrictions now that ASPM is disabled during NAPI
    poll

 drivers/net/ethernet/realtek/r8169_main.c | 145 +++++++++++++++-------
 1 file changed, 100 insertions(+), 45 deletions(-)

-- 
2.39.2

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH net-next 1/6] r8169: use spinlock to protect mac ocp register access
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
@ 2023-03-06 21:23 ` Heiner Kallweit
  2023-03-06 21:24 ` [PATCH net-next 2/6] r8169: use spinlock to protect access to registers Config2 and Config5 Heiner Kallweit
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:23 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

For disabling ASPM during NAPI poll we'll have to access mac ocp
registers in atomic context. This could result in races because
a mac ocp read consists of a write to register OCPDR, followed
by a read from the same register. Therefore add a spinlock to
protect access to mac ocp registers.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 37 ++++++++++++++++++++---
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 45147a101..259eac5b0 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -613,6 +613,8 @@ struct rtl8169_private {
 		struct work_struct work;
 	} wk;
 
+	spinlock_t mac_ocp_lock;
+
 	unsigned supports_gmii:1;
 	unsigned aspm_manageable:1;
 	dma_addr_t counters_phys_addr;
@@ -847,7 +849,7 @@ static int r8168_phy_ocp_read(struct rtl8169_private *tp, u32 reg)
 		(RTL_R32(tp, GPHY_OCP) & 0xffff) : -ETIMEDOUT;
 }
 
-static void r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
+static void __r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 {
 	if (rtl_ocp_reg_failure(reg))
 		return;
@@ -855,7 +857,16 @@ static void r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
 	RTL_W32(tp, OCPDR, OCPAR_FLAG | (reg << 15) | data);
 }
 
-static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
+static void r8168_mac_ocp_write(struct rtl8169_private *tp, u32 reg, u32 data)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&tp->mac_ocp_lock, flags);
+	__r8168_mac_ocp_write(tp, reg, data);
+	spin_unlock_irqrestore(&tp->mac_ocp_lock, flags);
+}
+
+static u16 __r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
 {
 	if (rtl_ocp_reg_failure(reg))
 		return 0;
@@ -865,12 +876,28 @@ static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
 	return RTL_R32(tp, OCPDR);
 }
 
+static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
+{
+	unsigned long flags;
+	u16 val;
+
+	spin_lock_irqsave(&tp->mac_ocp_lock, flags);
+	val = __r8168_mac_ocp_read(tp, reg);
+	spin_unlock_irqrestore(&tp->mac_ocp_lock, flags);
+
+	return val;
+}
+
 static void r8168_mac_ocp_modify(struct rtl8169_private *tp, u32 reg, u16 mask,
 				 u16 set)
 {
-	u16 data = r8168_mac_ocp_read(tp, reg);
+	unsigned long flags;
+	u16 data;
 
-	r8168_mac_ocp_write(tp, reg, (data & ~mask) | set);
+	spin_lock_irqsave(&tp->mac_ocp_lock, flags);
+	data = __r8168_mac_ocp_read(tp, reg);
+	__r8168_mac_ocp_write(tp, reg, (data & ~mask) | set);
+	spin_unlock_irqrestore(&tp->mac_ocp_lock, flags);
 }
 
 /* Work around a hw issue with RTL8168g PHY, the quirk disables
@@ -5176,6 +5203,8 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->eee_adv = -1;
 	tp->ocp_base = OCP_STD_PHY_BASE;
 
+	spin_lock_init(&tp->mac_ocp_lock);
+
 	dev->tstats = devm_netdev_alloc_pcpu_stats(&pdev->dev,
 						   struct pcpu_sw_netstats);
 	if (!dev->tstats)
-- 
2.39.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 2/6] r8169: use spinlock to protect access to registers Config2 and Config5
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
  2023-03-06 21:23 ` [PATCH net-next 1/6] r8169: use spinlock to protect mac ocp register access Heiner Kallweit
@ 2023-03-06 21:24 ` Heiner Kallweit
  2023-03-06 21:24 ` [PATCH net-next 3/6] r8169: enable cfg9346 config register access in atomic context Heiner Kallweit
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:24 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

For disabling ASPM during NAPI poll we'll have to access both registers
in atomic context. Use a spinlock to protect access.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 47 ++++++++++++++++++-----
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 259eac5b0..e6f3f1947 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -613,6 +613,7 @@ struct rtl8169_private {
 		struct work_struct work;
 	} wk;
 
+	spinlock_t config25_lock;
 	spinlock_t mac_ocp_lock;
 
 	unsigned supports_gmii:1;
@@ -677,6 +678,28 @@ static void rtl_pci_commit(struct rtl8169_private *tp)
 	RTL_R8(tp, ChipCmd);
 }
 
+static void rtl_mod_config2(struct rtl8169_private *tp, u8 clear, u8 set)
+{
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&tp->config25_lock, flags);
+	val = RTL_R8(tp, Config2);
+	RTL_W8(tp, Config2, (val & ~clear) | set);
+	spin_unlock_irqrestore(&tp->config25_lock, flags);
+}
+
+static void rtl_mod_config5(struct rtl8169_private *tp, u8 clear, u8 set)
+{
+	unsigned long flags;
+	u8 val;
+
+	spin_lock_irqsave(&tp->config25_lock, flags);
+	val = RTL_R8(tp, Config5);
+	RTL_W8(tp, Config5, (val & ~clear) | set);
+	spin_unlock_irqrestore(&tp->config25_lock, flags);
+}
+
 static bool rtl_is_8125(struct rtl8169_private *tp)
 {
 	return tp->mac_version >= RTL_GIGA_MAC_VER_61;
@@ -1363,6 +1386,7 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
 		{ WAKE_MAGIC, Config3, MagicPacket }
 	};
 	unsigned int i, tmp = ARRAY_SIZE(cfg);
+	unsigned long flags;
 	u8 options;
 
 	rtl_unlock_config_regs(tp);
@@ -1381,12 +1405,14 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
 			r8168_mac_ocp_modify(tp, 0xc0b6, BIT(0), 0);
 	}
 
+	spin_lock_irqsave(&tp->config25_lock, flags);
 	for (i = 0; i < tmp; i++) {
 		options = RTL_R8(tp, cfg[i].reg) & ~cfg[i].mask;
 		if (wolopts & cfg[i].opt)
 			options |= cfg[i].mask;
 		RTL_W8(tp, cfg[i].reg, options);
 	}
+	spin_unlock_irqrestore(&tp->config25_lock, flags);
 
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_02 ... RTL_GIGA_MAC_VER_06:
@@ -1398,10 +1424,10 @@ static void __rtl8169_set_wol(struct rtl8169_private *tp, u32 wolopts)
 	case RTL_GIGA_MAC_VER_34:
 	case RTL_GIGA_MAC_VER_37:
 	case RTL_GIGA_MAC_VER_39 ... RTL_GIGA_MAC_VER_63:
-		options = RTL_R8(tp, Config2) & ~PME_SIGNAL;
 		if (wolopts)
-			options |= PME_SIGNAL;
-		RTL_W8(tp, Config2, options);
+			rtl_mod_config2(tp, 0, PME_SIGNAL);
+		else
+			rtl_mod_config2(tp, PME_SIGNAL, 0);
 		break;
 	default:
 		break;
@@ -2704,8 +2730,8 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 {
 	/* Don't enable ASPM in the chip if OS can't control ASPM */
 	if (enable && tp->aspm_manageable) {
-		RTL_W8(tp, Config5, RTL_R8(tp, Config5) | ASPM_en);
-		RTL_W8(tp, Config2, RTL_R8(tp, Config2) | ClkReqEn);
+		rtl_mod_config5(tp, 0, ASPM_en);
+		rtl_mod_config2(tp, 0, ClkReqEn);
 
 		switch (tp->mac_version) {
 		case RTL_GIGA_MAC_VER_46 ... RTL_GIGA_MAC_VER_48:
@@ -2728,8 +2754,8 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 			break;
 		}
 
-		RTL_W8(tp, Config2, RTL_R8(tp, Config2) & ~ClkReqEn);
-		RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~ASPM_en);
+		rtl_mod_config2(tp, ClkReqEn, 0);
+		rtl_mod_config5(tp, ASPM_en, 0);
 	}
 
 	udelay(10);
@@ -2890,7 +2916,7 @@ static void rtl_hw_start_8168e_1(struct rtl8169_private *tp)
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | TXPLA_RST);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) & ~TXPLA_RST);
 
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~Spi_en);
+	rtl_mod_config5(tp, Spi_en, 0);
 }
 
 static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
@@ -2923,7 +2949,7 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
 
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~Spi_en);
+	rtl_mod_config5(tp, Spi_en, 0);
 
 	rtl_hw_aspm_clkreq_enable(tp, true);
 }
@@ -2946,7 +2972,7 @@ static void rtl_hw_start_8168f(struct rtl8169_private *tp)
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) | PFM_EN);
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | PWM_EN);
-	RTL_W8(tp, Config5, RTL_R8(tp, Config5) & ~Spi_en);
+	rtl_mod_config5(tp, Spi_en, 0);
 
 	rtl8168_config_eee_mac(tp);
 }
@@ -5203,6 +5229,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->eee_adv = -1;
 	tp->ocp_base = OCP_STD_PHY_BASE;
 
+	spin_lock_init(&tp->config25_lock);
 	spin_lock_init(&tp->mac_ocp_lock);
 
 	dev->tstats = devm_netdev_alloc_pcpu_stats(&pdev->dev,
-- 
2.39.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 3/6] r8169: enable cfg9346 config register access in atomic context
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
  2023-03-06 21:23 ` [PATCH net-next 1/6] r8169: use spinlock to protect mac ocp register access Heiner Kallweit
  2023-03-06 21:24 ` [PATCH net-next 2/6] r8169: use spinlock to protect access to registers Config2 and Config5 Heiner Kallweit
@ 2023-03-06 21:24 ` Heiner Kallweit
  2023-03-06 21:25 ` [PATCH net-next 4/6] r8169: prepare rtl_hw_aspm_clkreq_enable for usage " Heiner Kallweit
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:24 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

For disabling ASPM during NAPI poll we'll have to unlock access
to the config registers in atomic context. Other code parts
running with config register access unlocked are partially
longer and can sleep. Add a usage counter to enable parallel
execution of code parts requiring unlocked config registers.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index e6f3f1947..61cbf498f 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -616,6 +616,9 @@ struct rtl8169_private {
 	spinlock_t config25_lock;
 	spinlock_t mac_ocp_lock;
 
+	spinlock_t cfg9346_usage_lock;
+	int cfg9346_usage_count;
+
 	unsigned supports_gmii:1;
 	unsigned aspm_manageable:1;
 	dma_addr_t counters_phys_addr;
@@ -664,12 +667,22 @@ static inline struct device *tp_to_dev(struct rtl8169_private *tp)
 
 static void rtl_lock_config_regs(struct rtl8169_private *tp)
 {
-	RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+	unsigned long flags;
+
+	spin_lock_irqsave(&tp->cfg9346_usage_lock, flags);
+	if (!--tp->cfg9346_usage_count)
+		RTL_W8(tp, Cfg9346, Cfg9346_Lock);
+	spin_unlock_irqrestore(&tp->cfg9346_usage_lock, flags);
 }
 
 static void rtl_unlock_config_regs(struct rtl8169_private *tp)
 {
-	RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+	unsigned long flags;
+
+	spin_lock_irqsave(&tp->cfg9346_usage_lock, flags);
+	if (!tp->cfg9346_usage_count++)
+		RTL_W8(tp, Cfg9346, Cfg9346_Unlock);
+	spin_unlock_irqrestore(&tp->cfg9346_usage_lock, flags);
 }
 
 static void rtl_pci_commit(struct rtl8169_private *tp)
@@ -5229,6 +5242,7 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	tp->eee_adv = -1;
 	tp->ocp_base = OCP_STD_PHY_BASE;
 
+	spin_lock_init(&tp->cfg9346_usage_lock);
 	spin_lock_init(&tp->config25_lock);
 	spin_lock_init(&tp->mac_ocp_lock);
 
-- 
2.39.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 4/6] r8169: prepare rtl_hw_aspm_clkreq_enable for usage in atomic context
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
                   ` (2 preceding siblings ...)
  2023-03-06 21:24 ` [PATCH net-next 3/6] r8169: enable cfg9346 config register access in atomic context Heiner Kallweit
@ 2023-03-06 21:25 ` Heiner Kallweit
  2023-03-06 21:26 ` [PATCH net-next 5/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:25 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

Bail out if the function is used with chip versions that don't support
ASPM configuration. In addition remove the delay, it tuned out that
it's not needed, also vendor driver r8125 doesn't have it.

Suggested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 61cbf498f..96af31aea 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -2741,6 +2741,9 @@ static void rtl_disable_exit_l1(struct rtl8169_private *tp)
 
 static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 {
+	if (tp->mac_version < RTL_GIGA_MAC_VER_32)
+		return;
+
 	/* Don't enable ASPM in the chip if OS can't control ASPM */
 	if (enable && tp->aspm_manageable) {
 		rtl_mod_config5(tp, 0, ASPM_en);
@@ -2770,8 +2773,6 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 		rtl_mod_config2(tp, ClkReqEn, 0);
 		rtl_mod_config5(tp, ASPM_en, 0);
 	}
-
-	udelay(10);
 }
 
 static void rtl_set_fifo_size(struct rtl8169_private *tp, u16 rx_stat,
-- 
2.39.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 5/6] r8169: disable ASPM during NAPI poll
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
                   ` (3 preceding siblings ...)
  2023-03-06 21:25 ` [PATCH net-next 4/6] r8169: prepare rtl_hw_aspm_clkreq_enable for usage " Heiner Kallweit
@ 2023-03-06 21:26 ` Heiner Kallweit
  2023-03-06 21:28 ` [PATCH net-next 6/6] r8169: remove ASPM restrictions now that ASPM is disabled " Heiner Kallweit
  2023-03-08 10:00 ` [PATCH net-next 0/6] r8169: disable ASPM " patchwork-bot+netdevbpf
  6 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:26 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

Several chip versions have problems with ASPM, what may result in
rx_missed errors or tx timeouts. The root cause isn't known but
experience shows that disabling ASPM during NAPI poll can avoid
these problems.

Suggested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 96af31aea..2897b9bf2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4577,6 +4577,10 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance)
 	}
 
 	if (napi_schedule_prep(&tp->napi)) {
+		rtl_unlock_config_regs(tp);
+		rtl_hw_aspm_clkreq_enable(tp, false);
+		rtl_lock_config_regs(tp);
+
 		rtl_irq_disable(tp);
 		__napi_schedule(&tp->napi);
 	}
@@ -4636,9 +4640,14 @@ static int rtl8169_poll(struct napi_struct *napi, int budget)
 
 	work_done = rtl_rx(dev, tp, budget);
 
-	if (work_done < budget && napi_complete_done(napi, work_done))
+	if (work_done < budget && napi_complete_done(napi, work_done)) {
 		rtl_irq_enable(tp);
 
+		rtl_unlock_config_regs(tp);
+		rtl_hw_aspm_clkreq_enable(tp, true);
+		rtl_lock_config_regs(tp);
+	}
+
 	return work_done;
 }
 
-- 
2.39.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH net-next 6/6] r8169: remove ASPM restrictions now that ASPM is disabled during NAPI poll
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
                   ` (4 preceding siblings ...)
  2023-03-06 21:26 ` [PATCH net-next 5/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
@ 2023-03-06 21:28 ` Heiner Kallweit
  2023-03-08 10:00 ` [PATCH net-next 0/6] r8169: disable ASPM " patchwork-bot+netdevbpf
  6 siblings, 0 replies; 8+ messages in thread
From: Heiner Kallweit @ 2023-03-06 21:28 UTC (permalink / raw)
  To: Paolo Abeni, Jakub Kicinski, David Miller, Eric Dumazet,
	Realtek linux nic maintainers
  Cc: netdev@vger.kernel.org, Simon Horman, Kai-Heng Feng,
	Holger Hoffstätte

Now that  ASPM is disabled during NAPI poll, we can remove all ASPM
restrictions. This allows for higher power savings if the network
isn't fully loaded.

Reviewed-by: Simon Horman <simon.horman@corigine.com>
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 27 +----------------------
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 2897b9bf2..6563e4c6a 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -620,7 +620,6 @@ struct rtl8169_private {
 	int cfg9346_usage_count;
 
 	unsigned supports_gmii:1;
-	unsigned aspm_manageable:1;
 	dma_addr_t counters_phys_addr;
 	struct rtl8169_counters *counters;
 	struct rtl8169_tc_offsets tc_offset;
@@ -2744,8 +2743,7 @@ static void rtl_hw_aspm_clkreq_enable(struct rtl8169_private *tp, bool enable)
 	if (tp->mac_version < RTL_GIGA_MAC_VER_32)
 		return;
 
-	/* Don't enable ASPM in the chip if OS can't control ASPM */
-	if (enable && tp->aspm_manageable) {
+	if (enable) {
 		rtl_mod_config5(tp, 0, ASPM_en);
 		rtl_mod_config2(tp, 0, ClkReqEn);
 
@@ -5221,16 +5219,6 @@ static void rtl_init_mac_address(struct rtl8169_private *tp)
 	rtl_rar_set(tp, mac_addr);
 }
 
-/* register is set if system vendor successfully tested ASPM 1.2 */
-static bool rtl_aspm_is_safe(struct rtl8169_private *tp)
-{
-	if (tp->mac_version >= RTL_GIGA_MAC_VER_61 &&
-	    r8168_mac_ocp_read(tp, 0xc0b2) & 0xf)
-		return true;
-
-	return false;
-}
-
 static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct rtl8169_private *tp;
@@ -5302,19 +5290,6 @@ static int rtl_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	tp->mac_version = chipset;
 
-	/* Disable ASPM L1 as that cause random device stop working
-	 * problems as well as full system hangs for some PCIe devices users.
-	 * Chips from RTL8168h partially have issues with L1.2, but seem
-	 * to work fine with L1 and L1.1.
-	 */
-	if (rtl_aspm_is_safe(tp))
-		rc = 0;
-	else if (tp->mac_version >= RTL_GIGA_MAC_VER_46)
-		rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L1_2);
-	else
-		rc = pci_disable_link_state(pdev, PCIE_LINK_STATE_L1);
-	tp->aspm_manageable = !rc;
-
 	tp->dash_type = rtl_check_dash(tp);
 
 	tp->cp_cmd = RTL_R16(tp, CPlusCmd) & CPCMD_MASK;
-- 
2.39.2




^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll
  2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
                   ` (5 preceding siblings ...)
  2023-03-06 21:28 ` [PATCH net-next 6/6] r8169: remove ASPM restrictions now that ASPM is disabled " Heiner Kallweit
@ 2023-03-08 10:00 ` patchwork-bot+netdevbpf
  6 siblings, 0 replies; 8+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-08 10:00 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: pabeni, kuba, davem, edumazet, nic_swsd, netdev, simon.horman,
	kai.heng.feng, holger

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Mon, 6 Mar 2023 22:17:28 +0100 you wrote:
> This is a rework of ideas from Kai-Heng on how to avoid the known
> ASPM issues whilst still allowing for a maximum of ASPM-related power
> savings. As a prerequisite some locking is added first.
> 
> Heiner Kallweit (6):
>   r8169: use spinlock to protect mac ocp register access
>   r8169: use spinlock to protect access to registers Config2 and Config5
>   r8169: enable cfg9346 config register access in atomic context
>   r8169: prepare rtl_hw_aspm_clkreq_enable for usage in atomic context
>   r8169: disable ASPM during NAPI poll
>   r8169: remove ASPM restrictions now that ASPM is disabled during NAPI
>     poll
> 
> [...]

Here is the summary with links:
  - [net-next,1/6] r8169: use spinlock to protect mac ocp register access
    https://git.kernel.org/netdev/net-next/c/91c8643578a2
  - [net-next,2/6] r8169: use spinlock to protect access to registers Config2 and Config5
    https://git.kernel.org/netdev/net-next/c/6bc6c4e6893e
  - [net-next,3/6] r8169: enable cfg9346 config register access in atomic context
    https://git.kernel.org/netdev/net-next/c/59ee97c0c1a8
  - [net-next,4/6] r8169: prepare rtl_hw_aspm_clkreq_enable for usage in atomic context
    https://git.kernel.org/netdev/net-next/c/49ef7d846d4b
  - [net-next,5/6] r8169: disable ASPM during NAPI poll
    https://git.kernel.org/netdev/net-next/c/e1ed3e4d9111
  - [net-next,6/6] r8169: remove ASPM restrictions now that ASPM is disabled during NAPI poll
    https://git.kernel.org/netdev/net-next/c/2ab19de62d67

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-03-08 10:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-06 21:17 [PATCH net-next 0/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
2023-03-06 21:23 ` [PATCH net-next 1/6] r8169: use spinlock to protect mac ocp register access Heiner Kallweit
2023-03-06 21:24 ` [PATCH net-next 2/6] r8169: use spinlock to protect access to registers Config2 and Config5 Heiner Kallweit
2023-03-06 21:24 ` [PATCH net-next 3/6] r8169: enable cfg9346 config register access in atomic context Heiner Kallweit
2023-03-06 21:25 ` [PATCH net-next 4/6] r8169: prepare rtl_hw_aspm_clkreq_enable for usage " Heiner Kallweit
2023-03-06 21:26 ` [PATCH net-next 5/6] r8169: disable ASPM during NAPI poll Heiner Kallweit
2023-03-06 21:28 ` [PATCH net-next 6/6] r8169: remove ASPM restrictions now that ASPM is disabled " Heiner Kallweit
2023-03-08 10:00 ` [PATCH net-next 0/6] r8169: disable ASPM " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).