[PATCH 0/3] wifi: rtw88/89: add PS calculation to repair PS mode

[PATCH 0/3] wifi: rtw88/89: add PS calculation to repair PS mode

[Ping-Ke Shih]

The commit 28977e790b5d ("wifi: mac80211: skip powersave recalc if driver SUPPORTS_DYNAMIC_PS")
introduced by kernel 5.20 will skip to recalculate IEEE80211_CONF_PS           
of hw->conf.flags if driver sets SUPPORTS_DYNAMIC_PS.   

Since this problem was happened since 5.20, patches 1/3 and 2/3 should be
applied to kernel 6.1+.

Patch 3/3 is to remove redundant/unnecessary check, so this can be seen as
an independent patch that can go either wireless or wireless-next tree.
Note, I can't combine this into patch 2/3, because this redundant check
is introduced by upcoming kernel 6.4, but patch 2/3 would go kernel 6.1+.

Ping-Ke Shih (3):
  wifi: rtw88: correct PS calculation for SUPPORTS_DYNAMIC_PS
  wifi: rtw89: correct PS calculation for SUPPORTS_DYNAMIC_PS
  wifi: rtw89: remove redundant check of entering LPS

 drivers/net/wireless/realtek/rtw88/mac80211.c | 14 +++---
 drivers/net/wireless/realtek/rtw88/main.c     |  4 +-
 drivers/net/wireless/realtek/rtw88/ps.c       | 43 +++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/ps.h       |  2 +
 drivers/net/wireless/realtek/rtw89/core.c     |  3 --
 drivers/net/wireless/realtek/rtw89/mac80211.c | 15 +++----
 drivers/net/wireless/realtek/rtw89/ps.c       | 26 +++++++++++
 drivers/net/wireless/realtek/rtw89/ps.h       |  1 +
 8 files changed, 85 insertions(+), 23 deletions(-)

-- 
2.25.1

[PATCH 1/3] wifi: rtw88: correct PS calculation for SUPPORTS_DYNAMIC_PS

[Ping-Ke Shih]

This driver relies on IEEE80211_CONF_PS of hw->conf.flags to turn off PS or
turn on dynamic PS controlled by driver and firmware. Though this would be
incorrect, it did work before because the flag is always recalculated until
the commit 28977e790b5d ("wifi: mac80211: skip powersave recalc if driver SUPPORTS_DYNAMIC_PS")
is introduced by kernel 5.20 to skip to recalculate IEEE80211_CONF_PS
of hw->conf.flags if driver sets SUPPORTS_DYNAMIC_PS.

Correct this by doing recalculation while BSS_CHANGED_PS is changed and
interface is added or removed. It is allowed to enter PS only if single
one station vif is working. Without this fix, driver doesn't enter PS
anymore that causes higher power consumption.

Fixes: bcde60e599fb ("rtw88: remove misleading module parameter rtw_fw_support_lps")
Cc: stable@vger.kernel.org # 6.1+
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/mac80211.c | 14 +++---
 drivers/net/wireless/realtek/rtw88/main.c     |  4 +-
 drivers/net/wireless/realtek/rtw88/ps.c       | 43 +++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/ps.h       |  2 +
 4 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 02cd19ee6e4c4..95808130a76a5 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -88,15 +88,6 @@ static int rtw_ops_config(struct ieee80211_hw *hw, u32 changed)
 		}
 	}
 
-	if (changed & IEEE80211_CONF_CHANGE_PS) {
-		if (hw->conf.flags & IEEE80211_CONF_PS) {
-			rtwdev->ps_enabled = true;
-		} else {
-			rtwdev->ps_enabled = false;
-			rtw_leave_lps(rtwdev);
-		}
-	}
-
 	if (changed & IEEE80211_CONF_CHANGE_CHANNEL)
 		rtw_set_channel(rtwdev);
 
@@ -215,6 +206,7 @@ static int rtw_ops_add_interface(struct ieee80211_hw *hw,
 	config |= PORT_SET_BCN_CTRL;
 	rtw_vif_port_config(rtwdev, rtwvif, config);
 	rtw_core_port_switch(rtwdev, vif);
+	rtw_recalc_lps(rtwdev, vif);
 
 	mutex_unlock(&rtwdev->mutex);
 
@@ -246,6 +238,7 @@ static void rtw_ops_remove_interface(struct ieee80211_hw *hw,
 	config |= PORT_SET_BCN_CTRL;
 	rtw_vif_port_config(rtwdev, rtwvif, config);
 	clear_bit(rtwvif->port, rtwdev->hw_port);
+	rtw_recalc_lps(rtwdev, NULL);
 
 	mutex_unlock(&rtwdev->mutex);
 }
@@ -440,6 +433,9 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
 	if (changed & BSS_CHANGED_ERP_SLOT)
 		rtw_conf_tx(rtwdev, rtwvif);
 
+	if (changed & BSS_CHANGED_PS)
+		rtw_recalc_lps(rtwdev, NULL);
+
 	rtw_vif_port_config(rtwdev, rtwvif, config);
 
 	mutex_unlock(&rtwdev->mutex);
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 5bf6b45815578..d49019d972387 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -271,8 +271,8 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	 * more than two stations associated to the AP, then we can not enter
 	 * lps, because fw does not handle the overlapped beacon interval
 	 *
-	 * mac80211 should iterate vifs and determine if driver can enter
-	 * ps by passing IEEE80211_CONF_PS to us, all we need to do is to
+	 * rtw_recalc_lps() iterate vifs and determine if driver can enter
+	 * ps by vif->type and vif->cfg.ps, all we need to do here is to
 	 * get that vif and check if device is having traffic more than the
 	 * threshold.
 	 */
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 996365575f44f..53933fb38a330 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -299,3 +299,46 @@ void rtw_leave_lps_deep(struct rtw_dev *rtwdev)
 
 	__rtw_leave_lps_deep(rtwdev);
 }
+
+struct rtw_vif_recalc_lps_iter_data {
+	struct rtw_dev *rtwdev;
+	struct ieee80211_vif *found_vif;
+	int count;
+};
+
+static void __rtw_vif_recalc_lps(struct rtw_vif_recalc_lps_iter_data *data,
+				 struct ieee80211_vif *vif)
+{
+	if (data->count < 0)
+		return;
+
+	if (vif->type != NL80211_IFTYPE_STATION) {
+		data->count = -1;
+		return;
+	}
+
+	data->count++;
+	data->found_vif = vif;
+}
+
+static void rtw_vif_recalc_lps_iter(void *data, u8 *mac,
+				    struct ieee80211_vif *vif)
+{
+	__rtw_vif_recalc_lps(data, vif);
+}
+
+void rtw_recalc_lps(struct rtw_dev *rtwdev, struct ieee80211_vif *new_vif)
+{
+	struct rtw_vif_recalc_lps_iter_data data = { .rtwdev = rtwdev };
+
+	if (new_vif)
+		__rtw_vif_recalc_lps(&data, new_vif);
+	rtw_iterate_vifs(rtwdev, rtw_vif_recalc_lps_iter, &data);
+
+	if (data.count == 1 && data.found_vif->cfg.ps) {
+		rtwdev->ps_enabled = true;
+	} else {
+		rtwdev->ps_enabled = false;
+		rtw_leave_lps(rtwdev);
+	}
+}
diff --git a/drivers/net/wireless/realtek/rtw88/ps.h b/drivers/net/wireless/realtek/rtw88/ps.h
index c194386f6db53..5ae83d2526cfd 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.h
+++ b/drivers/net/wireless/realtek/rtw88/ps.h
@@ -23,4 +23,6 @@ void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id);
 void rtw_leave_lps(struct rtw_dev *rtwdev);
 void rtw_leave_lps_deep(struct rtw_dev *rtwdev);
 enum rtw_lps_deep_mode rtw_get_lps_deep_mode(struct rtw_dev *rtwdev);
+void rtw_recalc_lps(struct rtw_dev *rtwdev, struct ieee80211_vif *new_vif);
+
 #endif
-- 
2.25.1

[PATCH 2/3] wifi: rtw89: correct PS calculation for SUPPORTS_DYNAMIC_PS

[Ping-Ke Shih]

This driver relies on IEEE80211_CONF_PS of hw->conf.flags to turn off PS or
turn on dynamic PS controlled by driver and firmware. Though this would be
incorrect, it did work before because the flag is always recalculated until
the commit 28977e790b5d ("wifi: mac80211: skip powersave recalc if driver SUPPORTS_DYNAMIC_PS")
is introduced by kernel 5.20 to skip to recalculate IEEE80211_CONF_PS
of hw->conf.flags if driver sets SUPPORTS_DYNAMIC_PS.

Correct this by doing recalculation while BSS_CHANGED_PS is changed and
interface is added or removed. For now, it is allowed to enter PS only if
single one station vif is working, and it could possible to have PS per
vif after firmware can support it. Without this fix, driver doesn't
enter PS anymore that causes higher power consumption.

Fixes: e3ec7017f6a2 ("rtw89: add Realtek 802.11ax driver")
Cc: stable@vger.kernel.org # 6.1+
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/mac80211.c | 15 +++++------
 drivers/net/wireless/realtek/rtw89/ps.c       | 26 +++++++++++++++++++
 drivers/net/wireless/realtek/rtw89/ps.h       |  1 +
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/mac80211.c b/drivers/net/wireless/realtek/rtw89/mac80211.c
index f40d70f016e4c..0ffd7fb489a5e 100644
--- a/drivers/net/wireless/realtek/rtw89/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw89/mac80211.c
@@ -89,15 +89,6 @@ static int rtw89_ops_config(struct ieee80211_hw *hw, u32 changed)
 	    !(hw->conf.flags & IEEE80211_CONF_IDLE))
 		rtw89_leave_ips(rtwdev);
 
-	if (changed & IEEE80211_CONF_CHANGE_PS) {
-		if (hw->conf.flags & IEEE80211_CONF_PS) {
-			rtwdev->lps_enabled = true;
-		} else {
-			rtw89_leave_lps(rtwdev);
-			rtwdev->lps_enabled = false;
-		}
-	}
-
 	if (changed & IEEE80211_CONF_CHANGE_CHANNEL) {
 		rtw89_config_entity_chandef(rtwdev, RTW89_SUB_ENTITY_0,
 					    &hw->conf.chandef);
@@ -168,6 +159,8 @@ static int rtw89_ops_add_interface(struct ieee80211_hw *hw,
 	rtw89_core_txq_init(rtwdev, vif->txq);
 
 	rtw89_btc_ntfy_role_info(rtwdev, rtwvif, NULL, BTC_ROLE_START);
+
+	rtw89_recalc_lps(rtwdev);
 out:
 	mutex_unlock(&rtwdev->mutex);
 
@@ -192,6 +185,7 @@ static void rtw89_ops_remove_interface(struct ieee80211_hw *hw,
 	rtw89_mac_remove_vif(rtwdev, rtwvif);
 	rtw89_core_release_bit_map(rtwdev->hw_port, rtwvif->port);
 	list_del_init(&rtwvif->list);
+	rtw89_recalc_lps(rtwdev);
 	rtw89_enter_ips_by_hwflags(rtwdev);
 
 	mutex_unlock(&rtwdev->mutex);
@@ -451,6 +445,9 @@ static void rtw89_ops_bss_info_changed(struct ieee80211_hw *hw,
 	if (changed & BSS_CHANGED_CQM)
 		rtw89_fw_h2c_set_bcn_fltr_cfg(rtwdev, vif, true);
 
+	if (changed & BSS_CHANGED_PS)
+		rtw89_recalc_lps(rtwdev);
+
 	mutex_unlock(&rtwdev->mutex);
 }
 
diff --git a/drivers/net/wireless/realtek/rtw89/ps.c b/drivers/net/wireless/realtek/rtw89/ps.c
index fa94335f699aa..84201ef19c176 100644
--- a/drivers/net/wireless/realtek/rtw89/ps.c
+++ b/drivers/net/wireless/realtek/rtw89/ps.c
@@ -252,3 +252,29 @@ void rtw89_process_p2p_ps(struct rtw89_dev *rtwdev, struct ieee80211_vif *vif)
 	rtw89_p2p_disable_all_noa(rtwdev, vif);
 	rtw89_p2p_update_noa(rtwdev, vif);
 }
+
+void rtw89_recalc_lps(struct rtw89_dev *rtwdev)
+{
+	struct ieee80211_vif *vif, *found_vif = NULL;
+	struct rtw89_vif *rtwvif;
+	int count = 0;
+
+	rtw89_for_each_rtwvif(rtwdev, rtwvif) {
+		vif = rtwvif_to_vif(rtwvif);
+
+		if (vif->type != NL80211_IFTYPE_STATION) {
+			count = 0;
+			break;
+		}
+
+		count++;
+		found_vif = vif;
+	}
+
+	if (count == 1 && found_vif->cfg.ps) {
+		rtwdev->lps_enabled = true;
+	} else {
+		rtw89_leave_lps(rtwdev);
+		rtwdev->lps_enabled = false;
+	}
+}
diff --git a/drivers/net/wireless/realtek/rtw89/ps.h b/drivers/net/wireless/realtek/rtw89/ps.h
index 73c008db04264..4c18f49204b28 100644
--- a/drivers/net/wireless/realtek/rtw89/ps.h
+++ b/drivers/net/wireless/realtek/rtw89/ps.h
@@ -15,6 +15,7 @@ void rtw89_enter_ips(struct rtw89_dev *rtwdev);
 void rtw89_leave_ips(struct rtw89_dev *rtwdev);
 void rtw89_set_coex_ctrl_lps(struct rtw89_dev *rtwdev, bool btc_ctrl);
 void rtw89_process_p2p_ps(struct rtw89_dev *rtwdev, struct ieee80211_vif *vif);
+void rtw89_recalc_lps(struct rtw89_dev *rtwdev);
 
 static inline void rtw89_leave_ips_by_hwflags(struct rtw89_dev *rtwdev)
 {
-- 
2.25.1

[PATCH 3/3] wifi: rtw89: remove redundant check of entering LPS

[Ping-Ke Shih]

Originally, add this check rule to prevent entering LPS if more than one
vif (in station mode) connect to AP. Since we have checked this by previous
commit, remove this redundant check.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/core.c b/drivers/net/wireless/realtek/rtw89/core.c
index 101047686fffb..65581ca506d3f 100644
--- a/drivers/net/wireless/realtek/rtw89/core.c
+++ b/drivers/net/wireless/realtek/rtw89/core.c
@@ -2619,9 +2619,6 @@ static void rtw89_vif_enter_lps(struct rtw89_dev *rtwdev, struct rtw89_vif *rtwv
 	    rtwvif->tdls_peer)
 		return;
 
-	if (rtwdev->total_sta_assoc > 1)
-		return;
-
 	if (rtwvif->offchan)
 		return;
 
-- 
2.25.1

Re: [PATCH 3/3] wifi: rtw89: remove redundant check of entering LPS

[kernel test robot]

Hi,

Thanks for your patch.

FYI: kernel test robot notices the stable kernel rule is not satisfied.

Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.'
Subject: [PATCH 3/3] wifi: rtw89: remove redundant check of entering LPS
Link: https://lore.kernel.org/stable/20230527082939.11206-4-pkshih%40realtek.com

The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 3/3] wifi: rtw89: remove redundant check of entering LPS

[Ping-Ke Shih]

On Sat, 2023-05-27 at 16:30 +0800, kernel test robot wrote:
> 
> Hi,
> 
> Thanks for your patch.
> 
> FYI: kernel test robot notices the stable kernel rule is not satisfied.
> 
> Rule: 'Cc: stable@vger.kernel.org' or 'commit <sha1> upstream.'
> Subject: [PATCH 3/3] wifi: rtw89: remove redundant check of entering LPS
> Link: https://lore.kernel.org/stable/20230527082939.11206-4-pkshih%40realtek.com
> 
> The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 

I sent this patchset with the same To/Cc mail addresses, but I don't want
to apply this patch to stable kernel, so please stable maintainers
ignore this patch. I will take care about this by next submission.

Ping-Ke

Re: [PATCH 0/3] wifi: rtw88/89: add PS calculation to repair PS mode

[Larry Finger]

On 5/27/23 03:29, Ping-Ke Shih wrote:
> The commit 28977e790b5d ("wifi: mac80211: skip powersave recalc if driver SUPPORTS_DYNAMIC_PS")
> introduced by kernel 5.20 will skip to recalculate IEEE80211_CONF_PS
> of hw->conf.flags if driver sets SUPPORTS_DYNAMIC_PS.
> 
> Since this problem was happened since 5.20, patches 1/3 and 2/3 should be
> applied to kernel 6.1+.
> 
> Patch 3/3 is to remove redundant/unnecessary check, so this can be seen as
> an independent patch that can go either wireless or wireless-next tree.
> Note, I can't combine this into patch 2/3, because this redundant check
> is introduced by upcoming kernel 6.4, but patch 2/3 would go kernel 6.1+.
> 
> Ping-Ke Shih (3):
>    wifi: rtw88: correct PS calculation for SUPPORTS_DYNAMIC_PS
>    wifi: rtw89: correct PS calculation for SUPPORTS_DYNAMIC_PS
>    wifi: rtw89: remove redundant check of entering LPS

The patch for rtw88 has been added to the GitHub repo, and fixed the issue. [1]

Thanks Ping-Ke,

Larry

[1] https://github.com/lwfinger/rtw88/issues/129

Re: [PATCH 1/3] wifi: rtw88: correct PS calculation for SUPPORTS_DYNAMIC_PS

[Kalle Valo]

Ping-Ke Shih <pkshih@realtek.com> wrote:

> This driver relies on IEEE80211_CONF_PS of hw->conf.flags to turn off PS or
> turn on dynamic PS controlled by driver and firmware. Though this would be
> incorrect, it did work before because the flag is always recalculated until
> the commit 28977e790b5d ("wifi: mac80211: skip powersave recalc if driver SUPPORTS_DYNAMIC_PS")
> is introduced by kernel 5.20 to skip to recalculate IEEE80211_CONF_PS
> of hw->conf.flags if driver sets SUPPORTS_DYNAMIC_PS.
> 
> Correct this by doing recalculation while BSS_CHANGED_PS is changed and
> interface is added or removed. It is allowed to enter PS only if single
> one station vif is working. Without this fix, driver doesn't enter PS
> anymore that causes higher power consumption.
> 
> Fixes: bcde60e599fb ("rtw88: remove misleading module parameter rtw_fw_support_lps")
> Cc: stable@vger.kernel.org # 6.1+
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

3 patches applied to wireless.git, thanks.

3918dd0177ee wifi: rtw88: correct PS calculation for SUPPORTS_DYNAMIC_PS
26a125f550a3 wifi: rtw89: correct PS calculation for SUPPORTS_DYNAMIC_PS
b408f33b35a4 wifi: rtw89: remove redundant check of entering LPS

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230527082939.11206-2-pkshih@realtek.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches