linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/15] rtw88: Add support for deep PS mode
@ 2019-09-16  7:03 yhchuang
  2019-09-16  7:03 ` [PATCH 01/15] rtw88: remove redundant flag check helper function yhchuang
                   ` (14 more replies)
  0 siblings, 15 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

RTL8822B/RTL8822C series devices are capable of entering deep PS
mode. In contrast to Leisure Power Save (LPS) which turns off RF
components periodically between beacons, deep PS mode turns off
more hardware circuits. But device should enter LPS before enter
deep PS mode, otherwise the state of the firmware is undefined.

Under deep PS mode, driver can not read/write registers, and also
the TX path is reuiqred to be idle. To make sure it, driver should
acquired rtwdev->mutex lock, and then leave deep PS mode before
operating on the hardware, otherwise the behavior of the device
is unexpected and undefined.

To add deep PS mode, some modifications for the driver are needed.

For PCI part, drivers need to keep tracking on the SKBs delivered
to the device and see if there will have any activity on the TX
path. For the others, driver should make sure to acquire lock and
leave deep PS mode.

Also remove a misleading module parameter named "rtw_fw_support_lps".
It is not representing property of the firmware, but to let driver
decide if it wants to use LPS mode. But IEEE80211_CONF_PS can
handle it, by setting power save [on/off] through user space.
So just remove it, and listen to IEEE80211_CONF_CHANGE_PS.

Yan-Hsuan Chuang (15):
  rtw88: remove redundant flag check helper function
  rtw88: configure firmware after HCI started
  rtw88: pci: reset H2C queue indexes in a single write
  rtw88: pci: extract skbs free routine for trx rings
  rtw88: pci: release tx skbs DMAed when stop
  rtw88: not to enter or leave PS under IRQ
  rtw88: not to control LPS by each vif
  rtw88: remove unused lps state check helper
  rtw88: LPS enter/leave should be protected by lock
  rtw88: leave PS state for dynamic mechanism
  rtw88: add deep power save support
  rtw88: not to enter LPS by coex strategy
  rtw88: select deep PS mode when module is inserted
  rtw88: add deep PS PG mode for 8822c
  rtw88: remove misleading module parameter rtw_fw_support_lps

 drivers/net/wireless/realtek/rtw88/coex.c     |  14 +--
 drivers/net/wireless/realtek/rtw88/debug.h    |   1 +
 drivers/net/wireless/realtek/rtw88/fw.c       |  77 ++++++++++++
 drivers/net/wireless/realtek/rtw88/fw.h       |  29 +++++
 drivers/net/wireless/realtek/rtw88/hci.h      |   6 +
 drivers/net/wireless/realtek/rtw88/mac.c      |   5 +-
 drivers/net/wireless/realtek/rtw88/mac80211.c |  43 +++++--
 drivers/net/wireless/realtek/rtw88/main.c     |  88 ++++++++------
 drivers/net/wireless/realtek/rtw88/main.h     |  32 +++--
 drivers/net/wireless/realtek/rtw88/pci.c      | 125 +++++++++++++++++---
 drivers/net/wireless/realtek/rtw88/phy.c      |   2 +-
 drivers/net/wireless/realtek/rtw88/ps.c       | 162 +++++++++++++++++---------
 drivers/net/wireless/realtek/rtw88/ps.h       |  14 ++-
 drivers/net/wireless/realtek/rtw88/rtw8822b.c |   1 +
 drivers/net/wireless/realtek/rtw88/rtw8822c.c |   1 +
 drivers/net/wireless/realtek/rtw88/rx.c       |   2 -
 drivers/net/wireless/realtek/rtw88/sec.c      |  21 ++++
 drivers/net/wireless/realtek/rtw88/sec.h      |   1 +
 drivers/net/wireless/realtek/rtw88/tx.c       |   2 -
 19 files changed, 471 insertions(+), 155 deletions(-)

-- 
2.7.4


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

* [PATCH 01/15] rtw88: remove redundant flag check helper function
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-10-01  9:17   ` Kalle Valo
  2019-09-16  7:03 ` [PATCH 02/15] rtw88: configure firmware after HCI started yhchuang
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

These helper functions seems useless. And in some cases
we want to use test_and_[set/clear]_bit, these helpers
will make the code more complicated. So remove them.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/coex.c     |  4 ++--
 drivers/net/wireless/realtek/rtw88/mac.c      |  2 +-
 drivers/net/wireless/realtek/rtw88/mac80211.c | 10 +++++-----
 drivers/net/wireless/realtek/rtw88/main.c     | 20 ++++++++++----------
 drivers/net/wireless/realtek/rtw88/main.h     | 15 ---------------
 drivers/net/wireless/realtek/rtw88/phy.c      |  2 +-
 drivers/net/wireless/realtek/rtw88/ps.c       | 10 +++++-----
 7 files changed, 24 insertions(+), 39 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c
index 793b40b..4a0b569 100644
--- a/drivers/net/wireless/realtek/rtw88/coex.c
+++ b/drivers/net/wireless/realtek/rtw88/coex.c
@@ -383,9 +383,9 @@ static void rtw_coex_update_wl_link_info(struct rtw_dev *rtwdev, u8 reason)
 	u8 rssi_step;
 	u8 rssi;
 
-	scan = rtw_flag_check(rtwdev, RTW_FLAG_SCANNING);
+	scan = test_bit(RTW_FLAG_SCANNING, rtwdev->flags);
 	coex_stat->wl_connected = !!rtwdev->sta_cnt;
-	coex_stat->wl_gl_busy = rtw_flag_check(rtwdev, RTW_FLAG_BUSY_TRAFFIC);
+	coex_stat->wl_gl_busy = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags);
 
 	if (stats->tx_throughput > stats->rx_throughput)
 		coex_stat->wl_tput_dir = COEX_WL_TPUT_TX;
diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index fc14b37..f438376 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -710,7 +710,7 @@ int rtw_download_firmware(struct rtw_dev *rtwdev, struct rtw_fw_state *fw)
 	rtw_fw_send_general_info(rtwdev);
 	rtw_fw_send_phydm_info(rtwdev);
 
-	rtw_flag_set(rtwdev, RTW_FLAG_FW_RUNNING);
+	set_bit(RTW_FLAG_FW_RUNNING, rtwdev->flags);
 
 	return 0;
 
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index e5e3605..6d5cce0 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -19,7 +19,7 @@ static void rtw_ops_tx(struct ieee80211_hw *hw,
 	struct rtw_dev *rtwdev = hw->priv;
 	struct rtw_tx_pkt_info pkt_info = {0};
 
-	if (!rtw_flag_check(rtwdev, RTW_FLAG_RUNNING))
+	if (!test_bit(RTW_FLAG_RUNNING, rtwdev->flags))
 		goto out;
 
 	rtw_tx_pkt_info_update(rtwdev, &pkt_info, control, skb);
@@ -474,8 +474,8 @@ static void rtw_ops_sw_scan_start(struct ieee80211_hw *hw,
 
 	rtw_coex_scan_notify(rtwdev, COEX_SCAN_START);
 
-	rtw_flag_set(rtwdev, RTW_FLAG_DIG_DISABLE);
-	rtw_flag_set(rtwdev, RTW_FLAG_SCANNING);
+	set_bit(RTW_FLAG_DIG_DISABLE, rtwdev->flags);
+	set_bit(RTW_FLAG_SCANNING, rtwdev->flags);
 
 	mutex_unlock(&rtwdev->mutex);
 }
@@ -489,8 +489,8 @@ static void rtw_ops_sw_scan_complete(struct ieee80211_hw *hw,
 
 	mutex_lock(&rtwdev->mutex);
 
-	rtw_flag_clear(rtwdev, RTW_FLAG_SCANNING);
-	rtw_flag_clear(rtwdev, RTW_FLAG_DIG_DISABLE);
+	clear_bit(RTW_FLAG_SCANNING, rtwdev->flags);
+	clear_bit(RTW_FLAG_DIG_DISABLE, rtwdev->flags);
 
 	ether_addr_copy(rtwvif->mac_addr, vif->addr);
 	config |= PORT_SET_MAC_ADDR;
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index fc8f6213..0068d4d 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -150,20 +150,20 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	struct rtw_dev *rtwdev = container_of(work, struct rtw_dev,
 					      watch_dog_work.work);
 	struct rtw_watch_dog_iter_data data = {};
-	bool busy_traffic = rtw_flag_check(rtwdev, RTW_FLAG_BUSY_TRAFFIC);
+	bool busy_traffic = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags);
 
-	if (!rtw_flag_check(rtwdev, RTW_FLAG_RUNNING))
+	if (!test_bit(RTW_FLAG_RUNNING, rtwdev->flags))
 		return;
 
 	ieee80211_queue_delayed_work(rtwdev->hw, &rtwdev->watch_dog_work,
 				     RTW_WATCH_DOG_DELAY_TIME);
 
 	if (rtwdev->stats.tx_cnt > 100 || rtwdev->stats.rx_cnt > 100)
-		rtw_flag_set(rtwdev, RTW_FLAG_BUSY_TRAFFIC);
+		set_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags);
 	else
-		rtw_flag_clear(rtwdev, RTW_FLAG_BUSY_TRAFFIC);
+		clear_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags);
 
-	if (busy_traffic != rtw_flag_check(rtwdev, RTW_FLAG_BUSY_TRAFFIC))
+	if (busy_traffic != test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags))
 		rtw_coex_wl_status_change_notify(rtwdev);
 
 	/* reset tx/rx statictics */
@@ -183,7 +183,7 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	    data.rtwvif && !data.active && data.assoc_cnt == 1)
 		rtw_enter_lps(rtwdev, data.rtwvif);
 
-	if (rtw_flag_check(rtwdev, RTW_FLAG_SCANNING))
+	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
 		return;
 
 	rtw_phy_dynamic_mechanism(rtwdev);
@@ -311,7 +311,7 @@ void rtw_set_channel(struct rtw_dev *rtwdev)
 	if (hal->current_band_type == RTW_BAND_5G) {
 		rtw_coex_switchband_notify(rtwdev, COEX_SWITCH_TO_5G);
 	} else {
-		if (rtw_flag_check(rtwdev, RTW_FLAG_SCANNING))
+		if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
 			rtw_coex_switchband_notify(rtwdev, COEX_SWITCH_TO_24G);
 		else
 			rtw_coex_switchband_notify(rtwdev, COEX_SWITCH_TO_24G_NOFORSCAN);
@@ -733,7 +733,7 @@ int rtw_core_start(struct rtw_dev *rtwdev)
 	ieee80211_queue_delayed_work(rtwdev->hw, &rtwdev->watch_dog_work,
 				     RTW_WATCH_DOG_DELAY_TIME);
 
-	rtw_flag_set(rtwdev, RTW_FLAG_RUNNING);
+	set_bit(RTW_FLAG_RUNNING, rtwdev->flags);
 
 	return 0;
 }
@@ -748,8 +748,8 @@ void rtw_core_stop(struct rtw_dev *rtwdev)
 {
 	struct rtw_coex *coex = &rtwdev->coex;
 
-	rtw_flag_clear(rtwdev, RTW_FLAG_RUNNING);
-	rtw_flag_clear(rtwdev, RTW_FLAG_FW_RUNNING);
+	clear_bit(RTW_FLAG_RUNNING, rtwdev->flags);
+	clear_bit(RTW_FLAG_FW_RUNNING, rtwdev->flags);
 
 	cancel_delayed_work_sync(&rtwdev->watch_dog_work);
 	cancel_delayed_work_sync(&coex->bt_relink_work);
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index bede3f3..11ab9967 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1378,21 +1378,6 @@ struct rtw_dev {
 
 #include "hci.h"
 
-static inline bool rtw_flag_check(struct rtw_dev *rtwdev, enum rtw_flags flag)
-{
-	return test_bit(flag, rtwdev->flags);
-}
-
-static inline void rtw_flag_clear(struct rtw_dev *rtwdev, enum rtw_flags flag)
-{
-	clear_bit(flag, rtwdev->flags);
-}
-
-static inline void rtw_flag_set(struct rtw_dev *rtwdev, enum rtw_flags flag)
-{
-	set_bit(flag, rtwdev->flags);
-}
-
 static inline bool rtw_is_assoc(struct rtw_dev *rtwdev)
 {
 	return !!rtwdev->sta_cnt;
diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
index d3d3f40..8ebe1f4 100644
--- a/drivers/net/wireless/realtek/rtw88/phy.c
+++ b/drivers/net/wireless/realtek/rtw88/phy.c
@@ -394,7 +394,7 @@ static void rtw_phy_dig(struct rtw_dev *rtwdev)
 	u8 step[3];
 	bool linked;
 
-	if (rtw_flag_check(rtwdev, RTW_FLAG_DIG_DISABLE))
+	if (test_bit(RTW_FLAG_DIG_DISABLE, rtwdev->flags))
 		return;
 
 	if (rtw_phy_dig_check_damping(dm_info))
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 9ecd14fe..4896953 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -18,14 +18,14 @@ static int rtw_ips_pwr_up(struct rtw_dev *rtwdev)
 		rtw_err(rtwdev, "leave idle state failed\n");
 
 	rtw_set_channel(rtwdev);
-	rtw_flag_clear(rtwdev, RTW_FLAG_INACTIVE_PS);
+	clear_bit(RTW_FLAG_INACTIVE_PS, rtwdev->flags);
 
 	return ret;
 }
 
 int rtw_enter_ips(struct rtw_dev *rtwdev)
 {
-	rtw_flag_set(rtwdev, RTW_FLAG_INACTIVE_PS);
+	set_bit(RTW_FLAG_INACTIVE_PS, rtwdev->flags);
 
 	rtw_coex_ips_notify(rtwdev, COEX_IPS_ENTER);
 
@@ -71,7 +71,7 @@ static void rtw_leave_lps_core(struct rtw_dev *rtwdev)
 	conf->smart_ps = 0;
 
 	rtw_fw_set_pwr_mode(rtwdev);
-	rtw_flag_clear(rtwdev, RTW_FLAG_LEISURE_PS);
+	clear_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 
 	rtw_coex_lps_notify(rtwdev, COEX_LPS_DISABLE);
 }
@@ -88,7 +88,7 @@ static void rtw_enter_lps_core(struct rtw_dev *rtwdev)
 	rtw_coex_lps_notify(rtwdev, COEX_LPS_ENABLE);
 
 	rtw_fw_set_pwr_mode(rtwdev);
-	rtw_flag_set(rtwdev, RTW_FLAG_LEISURE_PS);
+	set_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 }
 
 void rtw_lps_work(struct work_struct *work)
@@ -137,7 +137,7 @@ void rtw_leave_lps_irqsafe(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
 
 bool rtw_in_lps(struct rtw_dev *rtwdev)
 {
-	return rtw_flag_check(rtwdev, RTW_FLAG_LEISURE_PS);
+	return test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 }
 
 void rtw_enter_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
-- 
2.7.4


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

* [PATCH 02/15] rtw88: configure firmware after HCI started
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
  2019-09-16  7:03 ` [PATCH 01/15] rtw88: remove redundant flag check helper function yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-21  5:51   ` Kalle Valo
  2019-09-16  7:03 ` [PATCH 03/15] rtw88: pci: reset H2C queue indexes in a single write yhchuang
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

After firmware has been downloaded, driver should send
some information to it through H2C commands. Those H2C
commands are transmitted through TX path.

But before HCI has been started, the TX path is not
working completely. Such as PCI interfaces, the interrupts
are not enabled, hence TX interrupts will not be issued
after H2C skb has been DMAed to the device. And the H2C
skbs will not be released until the device is powered off.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/mac.c  | 3 ---
 drivers/net/wireless/realtek/rtw88/main.c | 4 ++++
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index f438376..d8c5da3 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -707,9 +707,6 @@ int rtw_download_firmware(struct rtw_dev *rtwdev, struct rtw_fw_state *fw)
 	rtwdev->h2c.last_box_num = 0;
 	rtwdev->h2c.seq = 0;
 
-	rtw_fw_send_general_info(rtwdev);
-	rtw_fw_send_phydm_info(rtwdev);
-
 	set_bit(RTW_FLAG_FW_RUNNING, rtwdev->flags);
 
 	return 0;
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 0068d4d..36ba221 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -704,6 +704,10 @@ static int rtw_power_on(struct rtw_dev *rtwdev)
 		goto err_off;
 	}
 
+	/* send H2C after HCI has started */
+	rtw_fw_send_general_info(rtwdev);
+	rtw_fw_send_phydm_info(rtwdev);
+
 	wifi_only = !rtwdev->efuse.btcoex;
 	rtw_coex_power_on_setting(rtwdev);
 	rtw_coex_init_hw_config(rtwdev, wifi_only);
-- 
2.7.4


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

* [PATCH 03/15] rtw88: pci: reset H2C queue indexes in a single write
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
  2019-09-16  7:03 ` [PATCH 01/15] rtw88: remove redundant flag check helper function yhchuang
  2019-09-16  7:03 ` [PATCH 02/15] rtw88: configure firmware after HCI started yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings yhchuang
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

If the driver doesn't reset the host's and device's indexes
in a single write, the indexes will become different in a
short period. And it will confuse the DMA engine, make it
start to process non-existed entries.

Better to Write-1-to-reset the indexes, for the DMA engine
to know that this is a reset of the H2C queue, not a kick
off.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 3fdb52a..9cc068a 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -441,9 +441,9 @@ static void rtw_pci_reset_buf_desc(struct rtw_dev *rtwdev)
 	/* reset read/write point */
 	rtw_write32(rtwdev, RTK_PCI_TXBD_RWPTR_CLR, 0xffffffff);
 
-	/* rest H2C Queue index */
-	rtw_write32_set(rtwdev, RTK_PCI_TXBD_H2CQ_CSR, BIT_CLR_H2CQ_HOST_IDX);
-	rtw_write32_set(rtwdev, RTK_PCI_TXBD_H2CQ_CSR, BIT_CLR_H2CQ_HW_IDX);
+	/* reset H2C Queue index in a single write */
+	rtw_write32_set(rtwdev, RTK_PCI_TXBD_H2CQ_CSR,
+			BIT_CLR_H2CQ_HOST_IDX | BIT_CLR_H2CQ_HW_IDX);
 }
 
 static void rtw_pci_reset_trx_ring(struct rtw_dev *rtwdev)
-- 
2.7.4


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

* [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (2 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 03/15] rtw88: pci: reset H2C queue indexes in a single write yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-21  5:47   ` Kalle Valo
  2019-09-16  7:03 ` [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop yhchuang
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

These skbs free routines could be used when driver wants
to stop PCI bus, because some of the skbs remained in the
queue may not have been returned via DMA interrupt.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 36 +++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 9cc068a..3238161 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -90,16 +90,13 @@ static inline void *rtw_pci_get_tx_desc(struct rtw_pci_tx_ring *tx_ring, u8 idx)
 	return tx_ring->r.head + offset;
 }
 
-static void rtw_pci_free_tx_ring(struct rtw_dev *rtwdev,
-				 struct rtw_pci_tx_ring *tx_ring)
+static void rtw_pci_free_tx_ring_skbs(struct rtw_dev *rtwdev,
+				      struct rtw_pci_tx_ring *tx_ring)
 {
 	struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
 	struct rtw_pci_tx_data *tx_data;
 	struct sk_buff *skb, *tmp;
 	dma_addr_t dma;
-	u8 *head = tx_ring->r.head;
-	u32 len = tx_ring->r.len;
-	int ring_sz = len * tx_ring->r.desc_size;
 
 	/* free every skb remained in tx list */
 	skb_queue_walk_safe(&tx_ring->queue, skb, tmp) {
@@ -110,21 +107,30 @@ static void rtw_pci_free_tx_ring(struct rtw_dev *rtwdev,
 		pci_unmap_single(pdev, dma, skb->len, PCI_DMA_TODEVICE);
 		dev_kfree_skb_any(skb);
 	}
+}
+
+static void rtw_pci_free_tx_ring(struct rtw_dev *rtwdev,
+				 struct rtw_pci_tx_ring *tx_ring)
+{
+	struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
+	u8 *head = tx_ring->r.head;
+	u32 len = tx_ring->r.len;
+	int ring_sz = len * tx_ring->r.desc_size;
+
+	rtw_pci_free_tx_ring_skbs(rtwdev, tx_ring);
 
 	/* free the ring itself */
 	pci_free_consistent(pdev, ring_sz, head, tx_ring->r.dma);
 	tx_ring->r.head = NULL;
 }
 
-static void rtw_pci_free_rx_ring(struct rtw_dev *rtwdev,
-				 struct rtw_pci_rx_ring *rx_ring)
+static void rtw_pci_free_rx_ring_skbs(struct rtw_dev *rtwdev,
+				      struct rtw_pci_rx_ring *rx_ring)
 {
 	struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
 	struct sk_buff *skb;
-	dma_addr_t dma;
-	u8 *head = rx_ring->r.head;
 	int buf_sz = RTK_PCI_RX_BUF_SIZE;
-	int ring_sz = rx_ring->r.desc_size * rx_ring->r.len;
+	dma_addr_t dma;
 	int i;
 
 	for (i = 0; i < rx_ring->r.len; i++) {
@@ -137,6 +143,16 @@ static void rtw_pci_free_rx_ring(struct rtw_dev *rtwdev,
 		dev_kfree_skb(skb);
 		rx_ring->buf[i] = NULL;
 	}
+}
+
+static void rtw_pci_free_rx_ring(struct rtw_dev *rtwdev,
+				 struct rtw_pci_rx_ring *rx_ring)
+{
+	struct pci_dev *pdev = to_pci_dev(rtwdev->dev);
+	u8 *head = rx_ring->r.head;
+	int ring_sz = rx_ring->r.desc_size * rx_ring->r.len;
+
+	rtw_pci_free_rx_ring_skbs(rtwdev, rx_ring);
 
 	pci_free_consistent(pdev, ring_sz, head, rx_ring->r.dma);
 }
-- 
2.7.4


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

* [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (3 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-18  0:41   ` Brian Norris
  2019-09-16  7:03 ` [PATCH 06/15] rtw88: not to enter or leave PS under IRQ yhchuang
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Interrupt is disabled to stop PCI, which means the skbs
queued for each TX ring will not be released via DMA
interrupt. To avoid those skbs remained being left in
the skb queue until PCI has been removed, driver needs
to release skbs by itself.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 3238161..509743c 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -500,6 +500,17 @@ static void rtw_pci_dma_reset(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci)
 	rtwpci->rx_tag = 0;
 }
 
+static void rtw_pci_dma_release(struct rtw_dev *rtwdev, struct rtw_pci *rtwpci)
+{
+	struct rtw_pci_tx_ring *tx_ring;
+	u8 queue;
+
+	for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++) {
+		tx_ring = &rtwpci->tx_rings[queue];
+		rtw_pci_free_tx_ring_skbs(rtwdev, tx_ring);
+	}
+}
+
 static int rtw_pci_start(struct rtw_dev *rtwdev)
 {
 	struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
@@ -521,6 +532,7 @@ static void rtw_pci_stop(struct rtw_dev *rtwdev)
 
 	spin_lock_irqsave(&rtwpci->irq_lock, flags);
 	rtw_pci_disable_interrupt(rtwdev, rtwpci);
+	rtw_pci_dma_release(rtwdev, rtwpci);
 	spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
 }
 
-- 
2.7.4


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

* [PATCH 06/15] rtw88: not to enter or leave PS under IRQ
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (4 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 07/15] rtw88: not to control LPS by each vif yhchuang
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Remove PS related *_irqsafe functions to avoid entering/leaving PS
under interrupt context. Instead, make PS decision in watch_dog.
This could simplify the logic and make the code look clean.

But it could have a little side-effect that if the driver is having
heavy traffic before the every-2-second watch_dog detect the traffic
and decide to leave PS, the thoughput will be lower. Once traffic is
detected by watch_dog and left PS state, the throughput will resume
to the peak the hardware ought to have again.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/main.c |  3 ++-
 drivers/net/wireless/realtek/rtw88/main.h |  1 -
 drivers/net/wireless/realtek/rtw88/ps.c   | 44 -------------------------------
 drivers/net/wireless/realtek/rtw88/ps.h   |  3 ---
 drivers/net/wireless/realtek/rtw88/rx.c   |  2 --
 drivers/net/wireless/realtek/rtw88/tx.c   |  2 --
 6 files changed, 2 insertions(+), 53 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 36ba221..22fc5d6 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -182,6 +182,8 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	if (rtw_fw_support_lps &&
 	    data.rtwvif && !data.active && data.assoc_cnt == 1)
 		rtw_enter_lps(rtwdev, data.rtwvif);
+	else
+		rtw_leave_lps(rtwdev, rtwdev->lps_conf.rtwvif);
 
 	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
 		return;
@@ -1152,7 +1154,6 @@ int rtw_core_init(struct rtw_dev *rtwdev)
 		    rtw_tx_report_purge_timer, 0);
 
 	INIT_DELAYED_WORK(&rtwdev->watch_dog_work, rtw_watch_dog_work);
-	INIT_DELAYED_WORK(&rtwdev->lps_work, rtw_lps_work);
 	INIT_DELAYED_WORK(&coex->bt_relink_work, rtw_coex_bt_relink_work);
 	INIT_DELAYED_WORK(&coex->bt_reenable_work, rtw_coex_bt_reenable_work);
 	INIT_DELAYED_WORK(&coex->defreeze_work, rtw_coex_defreeze_work);
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 11ab9967..0955970 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1361,7 +1361,6 @@ struct rtw_dev {
 
 	/* lps power state & handler work */
 	struct rtw_lps_conf lps_conf;
-	struct delayed_work lps_work;
 
 	struct dentry *debugfs;
 
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 4896953..ffba3bd 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -91,50 +91,6 @@ static void rtw_enter_lps_core(struct rtw_dev *rtwdev)
 	set_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 }
 
-void rtw_lps_work(struct work_struct *work)
-{
-	struct rtw_dev *rtwdev = container_of(work, struct rtw_dev,
-					      lps_work.work);
-	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
-	struct rtw_vif *rtwvif = conf->rtwvif;
-
-	if (WARN_ON(!rtwvif))
-		return;
-
-	if (conf->mode == RTW_MODE_LPS)
-		rtw_enter_lps_core(rtwdev);
-	else
-		rtw_leave_lps_core(rtwdev);
-}
-
-void rtw_enter_lps_irqsafe(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
-{
-	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
-
-	if (rtwvif->in_lps)
-		return;
-
-	conf->mode = RTW_MODE_LPS;
-	conf->rtwvif = rtwvif;
-	rtwvif->in_lps = true;
-
-	ieee80211_queue_delayed_work(rtwdev->hw, &rtwdev->lps_work, 0);
-}
-
-void rtw_leave_lps_irqsafe(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
-{
-	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
-
-	if (!rtwvif->in_lps)
-		return;
-
-	conf->mode = RTW_MODE_ACTIVE;
-	conf->rtwvif = rtwvif;
-	rtwvif->in_lps = false;
-
-	ieee80211_queue_delayed_work(rtwdev->hw, &rtwdev->lps_work, 0);
-}
-
 bool rtw_in_lps(struct rtw_dev *rtwdev)
 {
 	return test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
diff --git a/drivers/net/wireless/realtek/rtw88/ps.h b/drivers/net/wireless/realtek/rtw88/ps.h
index 09e57405..0ac6c29 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.h
+++ b/drivers/net/wireless/realtek/rtw88/ps.h
@@ -10,9 +10,6 @@
 int rtw_enter_ips(struct rtw_dev *rtwdev);
 int rtw_leave_ips(struct rtw_dev *rtwdev);
 
-void rtw_lps_work(struct work_struct *work);
-void rtw_enter_lps_irqsafe(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);
-void rtw_leave_lps_irqsafe(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);
 void rtw_enter_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);
 void rtw_leave_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);
 bool rtw_in_lps(struct rtw_dev *rtwdev);
diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
index 48b9ed4..16b22eb 100644
--- a/drivers/net/wireless/realtek/rtw88/rx.c
+++ b/drivers/net/wireless/realtek/rtw88/rx.c
@@ -25,8 +25,6 @@ void rtw_rx_stats(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
 			rtwvif = (struct rtw_vif *)vif->drv_priv;
 			rtwvif->stats.rx_unicast += skb->len;
 			rtwvif->stats.rx_cnt++;
-			if (rtwvif->stats.rx_cnt > RTW_LPS_THRESHOLD)
-				rtw_leave_lps_irqsafe(rtwdev, rtwvif);
 		}
 	}
 }
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index 8eaa980..91bfd8c 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -27,8 +27,6 @@ void rtw_tx_stats(struct rtw_dev *rtwdev, struct ieee80211_vif *vif,
 			rtwvif = (struct rtw_vif *)vif->drv_priv;
 			rtwvif->stats.tx_unicast += skb->len;
 			rtwvif->stats.tx_cnt++;
-			if (rtwvif->stats.tx_cnt > RTW_LPS_THRESHOLD)
-				rtw_leave_lps_irqsafe(rtwdev, rtwvif);
 		}
 	}
 }
-- 
2.7.4


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

* [PATCH 07/15] rtw88: not to control LPS by each vif
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (5 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 06/15] rtw88: not to enter or leave PS under IRQ yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 08/15] rtw88: remove unused lps state check helper yhchuang
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

The original design of LPS enter/leave routines allows
to control the LPS state by each interface. But the
hardware cannot actually handle it that way. This means
the hardware can only enter LPS once with an associated
port, so there is no need to keep tracking the state of
each vif.

Hence the logic of enter/leave LPS state can be simple,
just to check the state of the device's flag. And for
leaving LPS state, it will get the same port id to send
to inform the hardware.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/coex.c     | 10 ++--------
 drivers/net/wireless/realtek/rtw88/mac80211.c |  2 +-
 drivers/net/wireless/realtek/rtw88/main.c     |  4 ++--
 drivers/net/wireless/realtek/rtw88/main.h     |  2 --
 drivers/net/wireless/realtek/rtw88/ps.c       | 19 +++++--------------
 drivers/net/wireless/realtek/rtw88/ps.h       |  4 ++--
 6 files changed, 12 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c
index 4a0b569..9d8cbd9 100644
--- a/drivers/net/wireless/realtek/rtw88/coex.c
+++ b/drivers/net/wireless/realtek/rtw88/coex.c
@@ -810,8 +810,6 @@ static void rtw_coex_ignore_wlan_act(struct rtw_dev *rtwdev, bool enable)
 static void rtw_coex_power_save_state(struct rtw_dev *rtwdev, u8 ps_type,
 				      u8 lps_val, u8 rpwm_val)
 {
-	struct rtw_lps_conf *lps_conf = &rtwdev->lps_conf;
-	struct rtw_vif *rtwvif;
 	struct rtw_coex *coex = &rtwdev->coex;
 	struct rtw_coex_stat *coex_stat = &coex->stat;
 	u8 lps_mode = 0x0;
@@ -823,18 +821,14 @@ static void rtw_coex_power_save_state(struct rtw_dev *rtwdev, u8 ps_type,
 		/* recover to original 32k low power setting */
 		coex_stat->wl_force_lps_ctrl = false;
 
-		rtwvif = lps_conf->rtwvif;
-		if (rtwvif && rtw_in_lps(rtwdev))
-			rtw_leave_lps(rtwdev, rtwvif);
+		rtw_leave_lps(rtwdev);
 		break;
 	case COEX_PS_LPS_OFF:
 		coex_stat->wl_force_lps_ctrl = true;
 		if (lps_mode)
 			rtw_fw_coex_tdma_type(rtwdev, 0x8, 0, 0, 0, 0);
 
-		rtwvif = lps_conf->rtwvif;
-		if (rtwvif && rtw_in_lps(rtwdev))
-			rtw_leave_lps(rtwdev, rtwvif);
+		rtw_leave_lps(rtwdev);
 		break;
 	default:
 		break;
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 6d5cce0..66c05c4 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -464,7 +464,7 @@ static void rtw_ops_sw_scan_start(struct ieee80211_hw *hw,
 	struct rtw_vif *rtwvif = (struct rtw_vif *)vif->drv_priv;
 	u32 config = 0;
 
-	rtw_leave_lps(rtwdev, rtwvif);
+	rtw_leave_lps(rtwdev);
 
 	mutex_lock(&rtwdev->mutex);
 
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 22fc5d6..85d83f1 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -181,9 +181,9 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	 */
 	if (rtw_fw_support_lps &&
 	    data.rtwvif && !data.active && data.assoc_cnt == 1)
-		rtw_enter_lps(rtwdev, data.rtwvif);
+		rtw_enter_lps(rtwdev, data.rtwvif->port);
 	else
-		rtw_leave_lps(rtwdev, rtwdev->lps_conf.rtwvif);
+		rtw_leave_lps(rtwdev);
 
 	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
 		return;
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 0955970..8472134c 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -533,8 +533,6 @@ enum rtw_pwr_state {
 };
 
 struct rtw_lps_conf {
-	/* the interface to enter lps */
-	struct rtw_vif *rtwvif;
 	enum rtw_lps_mode mode;
 	enum rtw_pwr_state state;
 	u8 awake_interval;
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index ffba3bd..a154177 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -96,36 +96,27 @@ bool rtw_in_lps(struct rtw_dev *rtwdev)
 	return test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 }
 
-void rtw_enter_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
+void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
 
-	if (WARN_ON(!rtwvif))
-		return;
-
-	if (rtwvif->in_lps)
+	if (test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags))
 		return;
 
 	conf->mode = RTW_MODE_LPS;
-	conf->rtwvif = rtwvif;
-	rtwvif->in_lps = true;
+	conf->port_id = port_id;
 
 	rtw_enter_lps_core(rtwdev);
 }
 
-void rtw_leave_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif)
+void rtw_leave_lps(struct rtw_dev *rtwdev)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
 
-	if (WARN_ON(!rtwvif))
-		return;
-
-	if (!rtwvif->in_lps)
+	if (!test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags))
 		return;
 
 	conf->mode = RTW_MODE_ACTIVE;
-	conf->rtwvif = rtwvif;
-	rtwvif->in_lps = false;
 
 	rtw_leave_lps_core(rtwdev);
 }
diff --git a/drivers/net/wireless/realtek/rtw88/ps.h b/drivers/net/wireless/realtek/rtw88/ps.h
index 0ac6c29..d2aae61 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.h
+++ b/drivers/net/wireless/realtek/rtw88/ps.h
@@ -10,8 +10,8 @@
 int rtw_enter_ips(struct rtw_dev *rtwdev);
 int rtw_leave_ips(struct rtw_dev *rtwdev);
 
-void rtw_enter_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);
-void rtw_leave_lps(struct rtw_dev *rtwdev, struct rtw_vif *rtwvif);
+void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id);
+void rtw_leave_lps(struct rtw_dev *rtwdev);
 bool rtw_in_lps(struct rtw_dev *rtwdev);
 
 #endif
-- 
2.7.4


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

* [PATCH 08/15] rtw88: remove unused lps state check helper
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (6 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 07/15] rtw88: not to control LPS by each vif yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 09/15] rtw88: LPS enter/leave should be protected by lock yhchuang
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

This is no more used, remove it.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/ps.c | 5 -----
 drivers/net/wireless/realtek/rtw88/ps.h | 1 -
 2 files changed, 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index a154177..d57996e 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -91,11 +91,6 @@ static void rtw_enter_lps_core(struct rtw_dev *rtwdev)
 	set_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 }
 
-bool rtw_in_lps(struct rtw_dev *rtwdev)
-{
-	return test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
-}
-
 void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
diff --git a/drivers/net/wireless/realtek/rtw88/ps.h b/drivers/net/wireless/realtek/rtw88/ps.h
index d2aae61..5aed11b 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.h
+++ b/drivers/net/wireless/realtek/rtw88/ps.h
@@ -12,6 +12,5 @@ int rtw_leave_ips(struct rtw_dev *rtwdev);
 
 void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id);
 void rtw_leave_lps(struct rtw_dev *rtwdev);
-bool rtw_in_lps(struct rtw_dev *rtwdev);
 
 #endif
-- 
2.7.4


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

* [PATCH 09/15] rtw88: LPS enter/leave should be protected by lock
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (7 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 08/15] rtw88: remove unused lps state check helper yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 10/15] rtw88: leave PS state for dynamic mechanism yhchuang
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Protect LPS enter/leave routine with rtwdev->mutex.
This helps to synchronize with driver's states correctly.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/mac80211.c | 4 ++--
 drivers/net/wireless/realtek/rtw88/main.c     | 9 +++++++--
 drivers/net/wireless/realtek/rtw88/ps.c       | 4 ++++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 66c05c4..984644f 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -464,10 +464,10 @@ static void rtw_ops_sw_scan_start(struct ieee80211_hw *hw,
 	struct rtw_vif *rtwvif = (struct rtw_vif *)vif->drv_priv;
 	u32 config = 0;
 
-	rtw_leave_lps(rtwdev);
-
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps(rtwdev);
+
 	ether_addr_copy(rtwvif->mac_addr, mac_addr);
 	config |= PORT_SET_MAC_ADDR;
 	rtw_vif_port_config(rtwdev, rtwvif, config);
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 85d83f1..f55eda9 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -152,8 +152,10 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	struct rtw_watch_dog_iter_data data = {};
 	bool busy_traffic = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags);
 
+	mutex_lock(&rtwdev->mutex);
+
 	if (!test_bit(RTW_FLAG_RUNNING, rtwdev->flags))
-		return;
+		goto unlock;
 
 	ieee80211_queue_delayed_work(rtwdev->hw, &rtwdev->watch_dog_work,
 				     RTW_WATCH_DOG_DELAY_TIME);
@@ -186,11 +188,14 @@ static void rtw_watch_dog_work(struct work_struct *work)
 		rtw_leave_lps(rtwdev);
 
 	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
-		return;
+		goto unlock;
 
 	rtw_phy_dynamic_mechanism(rtwdev);
 
 	rtwdev->watch_dog_cnt++;
+
+unlock:
+	mutex_unlock(&rtwdev->mutex);
 }
 
 static void rtw_c2h_work(struct work_struct *work)
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index d57996e..af5c7be 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -95,6 +95,8 @@ void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
 
+	lockdep_assert_held(&rtwdev->mutex);
+
 	if (test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags))
 		return;
 
@@ -108,6 +110,8 @@ void rtw_leave_lps(struct rtw_dev *rtwdev)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
 
+	lockdep_assert_held(&rtwdev->mutex);
+
 	if (!test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags))
 		return;
 
-- 
2.7.4


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

* [PATCH 10/15] rtw88: leave PS state for dynamic mechanism
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (8 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 09/15] rtw88: LPS enter/leave should be protected by lock yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 11/15] rtw88: add deep power save support yhchuang
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Dynamic mechanism requires BB/RF working to adjust
hardware settings. But PS state periodically turns
off BB/RF, could lead to wrong setting.

So leave PS state before DM to make sure it works.
And then check if we can enter PS state again.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/main.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index f55eda9..00ebf8c 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -174,6 +174,14 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	rtwdev->stats.tx_cnt = 0;
 	rtwdev->stats.rx_cnt = 0;
 
+	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
+		goto unlock;
+
+	/* make sure BB/RF is working for dynamic mech */
+	rtw_leave_lps(rtwdev);
+
+	rtw_phy_dynamic_mechanism(rtwdev);
+
 	/* use atomic version to avoid taking local->iflist_mtx mutex */
 	rtw_iterate_vifs_atomic(rtwdev, rtw_vif_watch_dog_iter, &data);
 
@@ -184,13 +192,6 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	if (rtw_fw_support_lps &&
 	    data.rtwvif && !data.active && data.assoc_cnt == 1)
 		rtw_enter_lps(rtwdev, data.rtwvif->port);
-	else
-		rtw_leave_lps(rtwdev);
-
-	if (test_bit(RTW_FLAG_SCANNING, rtwdev->flags))
-		goto unlock;
-
-	rtw_phy_dynamic_mechanism(rtwdev);
 
 	rtwdev->watch_dog_cnt++;
 
-- 
2.7.4


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

* [PATCH 11/15] rtw88: add deep power save support
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (9 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 10/15] rtw88: leave PS state for dynamic mechanism yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 12/15] rtw88: not to enter LPS by coex strategy yhchuang
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Deep power save allows firmware/hardware to operate in a
lower power state. And the deep power save mode depends on
LPS mode. So, before entering deep PS, driver must first
enter LPS mode.

Under Deep PS, most of hardware functions are shutdown,
driver will not be able to read/write registers and transfer
data to the device. Hence TX path must be protected by each
interface. Take PCI for example, DMA engine should be idle,
and no nore activities on the PCI bus.

If driver wants to operate on the device, such as register
read/write, it must first acquire the mutex lock and wake
up from Deep PS, otherwise the behavior is undefined.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/debug.h    |   1 +
 drivers/net/wireless/realtek/rtw88/hci.h      |   6 ++
 drivers/net/wireless/realtek/rtw88/mac80211.c |  14 ++++
 drivers/net/wireless/realtek/rtw88/main.c     |   1 +
 drivers/net/wireless/realtek/rtw88/main.h     |   2 +
 drivers/net/wireless/realtek/rtw88/pci.c      |  71 ++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/ps.c       | 100 ++++++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/ps.h       |   5 ++
 8 files changed, 195 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/debug.h b/drivers/net/wireless/realtek/rtw88/debug.h
index 45851cb..9449105 100644
--- a/drivers/net/wireless/realtek/rtw88/debug.h
+++ b/drivers/net/wireless/realtek/rtw88/debug.h
@@ -16,6 +16,7 @@ enum rtw_debug_mask {
 	RTW_DBG_RFK		= 0x00000080,
 	RTW_DBG_REGD		= 0x00000100,
 	RTW_DBG_DEBUGFS		= 0x00000200,
+	RTW_DBG_PS		= 0x00000400,
 
 	RTW_DBG_ALL		= 0xffffffff
 };
diff --git a/drivers/net/wireless/realtek/rtw88/hci.h b/drivers/net/wireless/realtek/rtw88/hci.h
index aba329c..4afbf0d 100644
--- a/drivers/net/wireless/realtek/rtw88/hci.h
+++ b/drivers/net/wireless/realtek/rtw88/hci.h
@@ -13,6 +13,7 @@ struct rtw_hci_ops {
 	int (*setup)(struct rtw_dev *rtwdev);
 	int (*start)(struct rtw_dev *rtwdev);
 	void (*stop)(struct rtw_dev *rtwdev);
+	void (*deep_ps)(struct rtw_dev *rtwdev, bool enter);
 
 	int (*write_data_rsvd_page)(struct rtw_dev *rtwdev, u8 *buf, u32 size);
 	int (*write_data_h2c)(struct rtw_dev *rtwdev, u8 *buf, u32 size);
@@ -47,6 +48,11 @@ static inline void rtw_hci_stop(struct rtw_dev *rtwdev)
 	rtwdev->hci.ops->stop(rtwdev);
 }
 
+static inline void rtw_hci_deep_ps(struct rtw_dev *rtwdev, bool enter)
+{
+	rtwdev->hci.ops->deep_ps(rtwdev, enter);
+}
+
 static inline int
 rtw_hci_write_data_rsvd_page(struct rtw_dev *rtwdev, u8 *buf, u32 size)
 {
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 984644f..e241d05 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -60,6 +60,8 @@ static int rtw_ops_config(struct ieee80211_hw *hw, u32 changed)
 
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps_deep(rtwdev);
+
 	if (changed & IEEE80211_CONF_CHANGE_IDLE) {
 		if (hw->conf.flags & IEEE80211_CONF_IDLE) {
 			rtw_enter_ips(rtwdev);
@@ -139,6 +141,8 @@ static int rtw_ops_add_interface(struct ieee80211_hw *hw,
 
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps_deep(rtwdev);
+
 	switch (vif->type) {
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_MESH_POINT:
@@ -181,6 +185,8 @@ static void rtw_ops_remove_interface(struct ieee80211_hw *hw,
 
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps_deep(rtwdev);
+
 	eth_zero_addr(rtwvif->mac_addr);
 	config |= PORT_SET_MAC_ADDR;
 	rtwvif->net_type = RTW_NET_NO_LINK;
@@ -204,6 +210,8 @@ static void rtw_ops_configure_filter(struct ieee80211_hw *hw,
 
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps_deep(rtwdev);
+
 	if (changed_flags & FIF_ALLMULTI) {
 		if (*new_flags & FIF_ALLMULTI)
 			rtwdev->hal.rcr |= BIT_AM | BIT_AB;
@@ -249,6 +257,8 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
 
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps_deep(rtwdev);
+
 	if (changed & BSS_CHANGED_ASSOC) {
 		struct rtw_chip_info *chip = rtwdev->chip;
 		enum rtw_net_type net_type;
@@ -266,6 +276,7 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
 			rtw_send_rsvd_page_h2c(rtwdev);
 			rtw_coex_media_status_notify(rtwdev, conf->assoc);
 		} else {
+			rtw_leave_lps(rtwdev);
 			net_type = RTW_NET_NO_LINK;
 			rtwvif->aid = 0;
 			rtw_reset_rsvd_page(rtwdev);
@@ -397,6 +408,8 @@ static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 
 	mutex_lock(&rtwdev->mutex);
 
+	rtw_leave_lps_deep(rtwdev);
+
 	if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE) {
 		hw_key_idx = rtw_sec_get_free_cam(sec);
 	} else {
@@ -508,6 +521,7 @@ static void rtw_ops_mgd_prepare_tx(struct ieee80211_hw *hw,
 	struct rtw_dev *rtwdev = hw->priv;
 
 	mutex_lock(&rtwdev->mutex);
+	rtw_leave_lps_deep(rtwdev);
 	rtw_coex_connect_notify(rtwdev, COEX_ASSOCIATE_START);
 	mutex_unlock(&rtwdev->mutex);
 }
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 00ebf8c..0d7ad17 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -922,6 +922,7 @@ static int rtw_chip_parameter_setup(struct rtw_dev *rtwdev)
 	switch (rtw_hci_type(rtwdev)) {
 	case RTW_HCI_TYPE_PCIE:
 		rtwdev->hci.rpwm_addr = 0x03d9;
+		rtwdev->hci.cpwm_addr = 0x03da;
 		break;
 	default:
 		rtw_err(rtwdev, "unsupported hci type\n");
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 8472134c..6e6b047 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -50,6 +50,7 @@ struct rtw_hci {
 	enum rtw_hci_type type;
 
 	u32 rpwm_addr;
+	u32 cpwm_addr;
 
 	u8 bulkout_num;
 };
@@ -309,6 +310,7 @@ enum rtw_flags {
 	RTW_FLAG_SCANNING,
 	RTW_FLAG_INACTIVE_PS,
 	RTW_FLAG_LEISURE_PS,
+	RTW_FLAG_LEISURE_PS_DEEP,
 	RTW_FLAG_DIG_DISABLE,
 	RTW_FLAG_BUSY_TRAFFIC,
 
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index 509743c..772a14d 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -9,6 +9,7 @@
 #include "tx.h"
 #include "rx.h"
 #include "fw.h"
+#include "ps.h"
 #include "debug.h"
 
 static bool rtw_disable_msi;
@@ -536,6 +537,69 @@ static void rtw_pci_stop(struct rtw_dev *rtwdev)
 	spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
 }
 
+static void rtw_pci_deep_ps_enter(struct rtw_dev *rtwdev)
+{
+	struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+	struct rtw_pci_tx_ring *tx_ring;
+	bool tx_empty = true;
+	u8 queue;
+
+	lockdep_assert_held(&rtwpci->irq_lock);
+
+	/* Deep PS state is not allowed to TX-DMA */
+	for (queue = 0; queue < RTK_MAX_TX_QUEUE_NUM; queue++) {
+		/* BCN queue is rsvd page, does not have DMA interrupt
+		 * H2C queue is managed by firmware
+		 */
+		if (queue == RTW_TX_QUEUE_BCN ||
+		    queue == RTW_TX_QUEUE_H2C)
+			continue;
+
+		tx_ring = &rtwpci->tx_rings[queue];
+
+		/* check if there is any skb DMAing */
+		if (skb_queue_len(&tx_ring->queue)) {
+			tx_empty = false;
+			break;
+		}
+	}
+
+	if (!tx_empty) {
+		rtw_dbg(rtwdev, RTW_DBG_PS,
+			"TX path not empty, cannot enter deep power save state\n");
+		return;
+	}
+
+	set_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags);
+	rtw_power_mode_change(rtwdev, true);
+}
+
+static void rtw_pci_deep_ps_leave(struct rtw_dev *rtwdev)
+{
+	struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+
+	lockdep_assert_held(&rtwpci->irq_lock);
+
+	if (test_and_clear_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags))
+		rtw_power_mode_change(rtwdev, false);
+}
+
+static void rtw_pci_deep_ps(struct rtw_dev *rtwdev, bool enter)
+{
+	struct rtw_pci *rtwpci = (struct rtw_pci *)rtwdev->priv;
+	unsigned long flags;
+
+	spin_lock_irqsave(&rtwpci->irq_lock, flags);
+
+	if (enter && !test_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags))
+		rtw_pci_deep_ps_enter(rtwdev);
+
+	if (!enter && test_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags))
+		rtw_pci_deep_ps_leave(rtwdev);
+
+	spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
+}
+
 static u8 ac_to_hwq[] = {
 	[IEEE80211_AC_VO] = RTW_TX_QUEUE_VO,
 	[IEEE80211_AC_VI] = RTW_TX_QUEUE_VI,
@@ -616,6 +680,7 @@ static int rtw_pci_xmit(struct rtw_dev *rtwdev,
 	u8 *pkt_desc;
 	struct rtw_pci_tx_buffer_desc *buf_desc;
 	u32 bd_idx;
+	unsigned long flags;
 
 	ring = &rtwpci->tx_rings[queue];
 
@@ -651,6 +716,10 @@ static int rtw_pci_xmit(struct rtw_dev *rtwdev,
 	tx_data = rtw_pci_get_tx_data(skb);
 	tx_data->dma = dma;
 	tx_data->sn = pkt_info->sn;
+
+	spin_lock_irqsave(&rtwpci->irq_lock, flags);
+
+	rtw_pci_deep_ps_leave(rtwdev);
 	skb_queue_tail(&ring->queue, skb);
 
 	/* kick off tx queue */
@@ -666,6 +735,7 @@ static int rtw_pci_xmit(struct rtw_dev *rtwdev,
 		reg_bcn_work |= BIT_PCI_BCNQ_FLAG;
 		rtw_write8(rtwdev, RTK_PCI_TXBD_BCN_WORK, reg_bcn_work);
 	}
+	spin_unlock_irqrestore(&rtwpci->irq_lock, flags);
 
 	return 0;
 }
@@ -1142,6 +1212,7 @@ static struct rtw_hci_ops rtw_pci_ops = {
 	.setup = rtw_pci_setup,
 	.start = rtw_pci_start,
 	.stop = rtw_pci_stop,
+	.deep_ps = rtw_pci_deep_ps,
 
 	.read8 = rtw_pci_read8,
 	.read16 = rtw_pci_read16,
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index af5c7be..4b57746 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -3,6 +3,7 @@
  */
 
 #include "main.h"
+#include "reg.h"
 #include "fw.h"
 #include "ps.h"
 #include "mac.h"
@@ -61,6 +62,59 @@ int rtw_leave_ips(struct rtw_dev *rtwdev)
 	return 0;
 }
 
+void rtw_power_mode_change(struct rtw_dev *rtwdev, bool enter)
+{
+	u8 request, confirm, polling;
+	u8 polling_cnt;
+	u8 retry_cnt = 0;
+
+retry:
+	request = rtw_read8(rtwdev, rtwdev->hci.rpwm_addr);
+	confirm = rtw_read8(rtwdev, rtwdev->hci.cpwm_addr);
+
+	/* toggle to request power mode, others remain 0 */
+	request ^= request | BIT_RPWM_TOGGLE;
+	if (!enter)
+		request |= POWER_MODE_ACK;
+	else
+		request |= POWER_MODE_LCLK;
+
+	rtw_write8(rtwdev, rtwdev->hci.rpwm_addr, request);
+
+	/* check confirm power mode has left power save state */
+	if (!enter) {
+		for (polling_cnt = 0; polling_cnt < 3; polling_cnt++) {
+			polling = rtw_read8(rtwdev, rtwdev->hci.cpwm_addr);
+			if ((polling ^ confirm) & BIT_RPWM_TOGGLE)
+				return;
+			mdelay(20);
+		}
+
+		/* in case of fw/hw missed the request, retry 3 times */
+		if (retry_cnt < 3) {
+			rtw_warn(rtwdev, "failed to leave deep PS, retry=%d\n",
+				 retry_cnt);
+			retry_cnt++;
+			goto retry;
+		}
+
+		/* Hit here means that driver failed to change hardware
+		 * power mode to active state after retry 3 times.
+		 * If the power state is locked at Deep sleep, most of
+		 * the hardware circuits is not working, even register
+		 * read/write. It should be treated as fatal error and
+		 * requires an entire analysis about the firmware/hardware
+		 */
+		WARN_ON("Hardware power state locked\n");
+	}
+}
+EXPORT_SYMBOL(rtw_power_mode_change);
+
+static void __rtw_leave_lps_deep(struct rtw_dev *rtwdev)
+{
+	rtw_hci_deep_ps(rtwdev, false);
+}
+
 static void rtw_leave_lps_core(struct rtw_dev *rtwdev)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
@@ -76,6 +130,17 @@ static void rtw_leave_lps_core(struct rtw_dev *rtwdev)
 	rtw_coex_lps_notify(rtwdev, COEX_LPS_DISABLE);
 }
 
+static void __rtw_enter_lps_deep(struct rtw_dev *rtwdev)
+{
+	if (!test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags)) {
+		rtw_dbg(rtwdev, RTW_DBG_PS,
+			"Should enter LPS before entering deep PS\n");
+		return;
+	}
+
+	rtw_hci_deep_ps(rtwdev, true);
+}
+
 static void rtw_enter_lps_core(struct rtw_dev *rtwdev)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
@@ -91,12 +156,10 @@ static void rtw_enter_lps_core(struct rtw_dev *rtwdev)
 	set_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags);
 }
 
-void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
+static void __rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
 
-	lockdep_assert_held(&rtwdev->mutex);
-
 	if (test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags))
 		return;
 
@@ -106,11 +169,15 @@ void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
 	rtw_enter_lps_core(rtwdev);
 }
 
-void rtw_leave_lps(struct rtw_dev *rtwdev)
+static void __rtw_leave_lps(struct rtw_dev *rtwdev)
 {
 	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
 
-	lockdep_assert_held(&rtwdev->mutex);
+	if (test_and_clear_bit(RTW_FLAG_LEISURE_PS_DEEP, rtwdev->flags)) {
+		rtw_dbg(rtwdev, RTW_DBG_PS,
+			"Should leave deep PS before leaving LPS\n");
+		__rtw_leave_lps_deep(rtwdev);
+	}
 
 	if (!test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags))
 		return;
@@ -119,3 +186,26 @@ void rtw_leave_lps(struct rtw_dev *rtwdev)
 
 	rtw_leave_lps_core(rtwdev);
 }
+
+void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
+{
+	lockdep_assert_held(&rtwdev->mutex);
+
+	__rtw_enter_lps(rtwdev, port_id);
+	__rtw_enter_lps_deep(rtwdev);
+}
+
+void rtw_leave_lps(struct rtw_dev *rtwdev)
+{
+	lockdep_assert_held(&rtwdev->mutex);
+
+	__rtw_leave_lps_deep(rtwdev);
+	__rtw_leave_lps(rtwdev);
+}
+
+void rtw_leave_lps_deep(struct rtw_dev *rtwdev)
+{
+	lockdep_assert_held(&rtwdev->mutex);
+
+	__rtw_leave_lps_deep(rtwdev);
+}
diff --git a/drivers/net/wireless/realtek/rtw88/ps.h b/drivers/net/wireless/realtek/rtw88/ps.h
index 5aed11b..03b08bd 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.h
+++ b/drivers/net/wireless/realtek/rtw88/ps.h
@@ -7,10 +7,15 @@
 
 #define RTW_LPS_THRESHOLD	2
 
+#define POWER_MODE_ACK		BIT(6)
+#define POWER_MODE_LCLK		BIT(0)
+
 int rtw_enter_ips(struct rtw_dev *rtwdev);
 int rtw_leave_ips(struct rtw_dev *rtwdev);
 
+void rtw_power_mode_change(struct rtw_dev *rtwdev, bool enter);
 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);
 
 #endif
-- 
2.7.4


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

* [PATCH 12/15] rtw88: not to enter LPS by coex strategy
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (10 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 11/15] rtw88: add deep power save support yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 13/15] rtw88: select deep PS mode when module is inserted yhchuang
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Sometimes LPS is not compatible with COEX's strategy, and
COEX will not allow driver to enter it.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/ps.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 4b57746..1661cc2 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -191,6 +191,9 @@ void rtw_enter_lps(struct rtw_dev *rtwdev, u8 port_id)
 {
 	lockdep_assert_held(&rtwdev->mutex);
 
+	if (rtwdev->coex.stat.wl_force_lps_ctrl)
+		return;
+
 	__rtw_enter_lps(rtwdev, port_id);
 	__rtw_enter_lps_deep(rtwdev);
 }
-- 
2.7.4


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

* [PATCH 13/15] rtw88: select deep PS mode when module is inserted
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (11 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 12/15] rtw88: not to enter LPS by coex strategy yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 14/15] rtw88: add deep PS PG mode for 8822c yhchuang
  2019-09-16  7:03 ` [PATCH 15/15] rtw88: remove misleading module parameter rtw_fw_support_lps yhchuang
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Add a module parameter to select deep PS mode. And the mode
cannot be changed after the module has been inserted and probed.
If anyone wants to change the deep mode, should change the mode
and probe the device again to setup the changed deep mode.

When the device is probed, driver will check the deep PS mode
with different IC's PS mode suppotability. If none of the
PS mode is matched, the deep PS mode is changed to NONE,
means deep PS is disabled.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/main.c     | 9 +++++++++
 drivers/net/wireless/realtek/rtw88/main.h     | 8 ++++++++
 drivers/net/wireless/realtek/rtw88/ps.c       | 3 +++
 drivers/net/wireless/realtek/rtw88/rtw8822b.c | 1 +
 drivers/net/wireless/realtek/rtw88/rtw8822c.c | 1 +
 5 files changed, 22 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 0d7ad17..3c366a3 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -14,13 +14,17 @@
 #include "efuse.h"
 #include "debug.h"
 
+unsigned int rtw_fw_lps_deep_mode;
+EXPORT_SYMBOL(rtw_fw_lps_deep_mode);
 static bool rtw_fw_support_lps;
 unsigned int rtw_debug_mask;
 EXPORT_SYMBOL(rtw_debug_mask);
 
+module_param_named(lps_deep_mode, rtw_fw_lps_deep_mode, uint, 0644);
 module_param_named(support_lps, rtw_fw_support_lps, bool, 0644);
 module_param_named(debug_mask, rtw_debug_mask, uint, 0644);
 
+MODULE_PARM_DESC(lps_deep_mode, "Deeper PS mode. If 0, deep PS is disabled");
 MODULE_PARM_DESC(support_lps, "Set Y to enable Leisure Power Save support, to turn radio off between beacons");
 MODULE_PARM_DESC(debug_mask, "Debugging mask");
 
@@ -1152,6 +1156,7 @@ EXPORT_SYMBOL(rtw_chip_info_setup);
 
 int rtw_core_init(struct rtw_dev *rtwdev)
 {
+	struct rtw_chip_info *chip = rtwdev->chip;
 	struct rtw_coex *coex = &rtwdev->coex;
 	int ret;
 
@@ -1183,6 +1188,10 @@ int rtw_core_init(struct rtw_dev *rtwdev)
 	rtwdev->sec.total_cam_num = 32;
 	rtwdev->hal.current_channel = 1;
 	set_bit(RTW_BC_MC_MACID, rtwdev->mac_id_map);
+	if (!(BIT(rtw_fw_lps_deep_mode) & chip->lps_deep_mode_supported))
+		rtwdev->lps_conf.deep_mode = LPS_DEEP_MODE_NONE;
+	else
+		rtwdev->lps_conf.deep_mode = rtw_fw_lps_deep_mode;
 
 	mutex_lock(&rtwdev->mutex);
 	rtw_add_rsvd_page(rtwdev, RSVD_BEACON, false);
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 6e6b047..a59cbae 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -27,6 +27,7 @@
 #define RTW_RF_PATH_MAX			4
 #define HW_FEATURE_LEN			13
 
+extern unsigned int rtw_fw_lps_deep_mode;
 extern unsigned int rtw_debug_mask;
 extern const struct ieee80211_ops rtw_ops;
 extern struct rtw_chip_info rtw8822b_hw_spec;
@@ -528,6 +529,11 @@ enum rtw_lps_mode {
 	RTW_MODE_WMM_PS	= 2,
 };
 
+enum rtw_lps_deep_mode {
+	LPS_DEEP_MODE_NONE	= 0,
+	LPS_DEEP_MODE_LCLK	= 1,
+};
+
 enum rtw_pwr_state {
 	RTW_RF_OFF	= 0x0,
 	RTW_RF_ON	= 0x4,
@@ -536,6 +542,7 @@ enum rtw_pwr_state {
 
 struct rtw_lps_conf {
 	enum rtw_lps_mode mode;
+	enum rtw_lps_deep_mode deep_mode;
 	enum rtw_pwr_state state;
 	u8 awake_interval;
 	u8 rlbm;
@@ -844,6 +851,7 @@ struct rtw_chip_info {
 
 	bool ht_supported;
 	bool vht_supported;
+	u8 lps_deep_mode_supported;
 
 	/* init values */
 	u8 sys_func_en;
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 1661cc2..02e104a 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -132,6 +132,9 @@ static void rtw_leave_lps_core(struct rtw_dev *rtwdev)
 
 static void __rtw_enter_lps_deep(struct rtw_dev *rtwdev)
 {
+	if (rtwdev->lps_conf.deep_mode == LPS_DEEP_MODE_NONE)
+		return;
+
 	if (!test_bit(RTW_FLAG_LEISURE_PS, rtwdev->flags)) {
 		rtw_dbg(rtwdev, RTW_DBG_PS,
 			"Should enter LPS before entering deep PS\n");
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822b.c b/drivers/net/wireless/realtek/rtw88/rtw8822b.c
index 63abda3..2b6cd7cf 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822b.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822b.c
@@ -1977,6 +1977,7 @@ struct rtw_chip_info rtw8822b_hw_spec = {
 	.dig_min = 0x1c,
 	.ht_supported = true,
 	.vht_supported = true,
+	.lps_deep_mode_supported = BIT(LPS_DEEP_MODE_LCLK),
 	.sys_func_en = 0xDC,
 	.pwr_on_seq = card_enable_flow_8822b,
 	.pwr_off_seq = card_disable_flow_8822b,
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.c b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
index 084c18d..b92940e5 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
@@ -3747,6 +3747,7 @@ struct rtw_chip_info rtw8822c_hw_spec = {
 	.dig_min = 0x20,
 	.ht_supported = true,
 	.vht_supported = true,
+	.lps_deep_mode_supported = BIT(LPS_DEEP_MODE_LCLK),
 	.sys_func_en = 0xD8,
 	.pwr_on_seq = card_enable_flow_8822c,
 	.pwr_off_seq = card_disable_flow_8822c,
-- 
2.7.4


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

* [PATCH 14/15] rtw88: add deep PS PG mode for 8822c
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (12 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 13/15] rtw88: select deep PS mode when module is inserted yhchuang
@ 2019-09-16  7:03 ` yhchuang
  2019-09-16  7:03 ` [PATCH 15/15] rtw88: remove misleading module parameter rtw_fw_support_lps yhchuang
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

Compare with LCLK mode, PG mode saves more power, by turning
off more circuits. Therefore, to recover from PG mode, driver
needs to backup some information into rsvd page. Such as CAM
entries, DPK results.

As CAM entries can change, it is required to re-download CAM
entries after set_key.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/fw.c       | 77 +++++++++++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/fw.h       | 29 ++++++++++
 drivers/net/wireless/realtek/rtw88/mac80211.c |  6 +++
 drivers/net/wireless/realtek/rtw88/main.h     |  3 ++
 drivers/net/wireless/realtek/rtw88/ps.c       | 10 +++-
 drivers/net/wireless/realtek/rtw88/ps.h       |  1 +
 drivers/net/wireless/realtek/rtw88/rtw8822c.c |  2 +-
 drivers/net/wireless/realtek/rtw88/sec.c      | 21 ++++++++
 drivers/net/wireless/realtek/rtw88/sec.h      |  1 +
 9 files changed, 147 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index b082e2c..430d73c 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -7,6 +7,7 @@
 #include "fw.h"
 #include "tx.h"
 #include "reg.h"
+#include "sec.h"
 #include "debug.h"
 
 static void rtw_fw_c2h_cmd_handle_ext(struct rtw_dev *rtwdev,
@@ -397,6 +398,24 @@ static u8 rtw_get_rsvd_page_location(struct rtw_dev *rtwdev,
 	return location;
 }
 
+void rtw_fw_set_pg_info(struct rtw_dev *rtwdev)
+{
+	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
+	u8 h2c_pkt[H2C_PKT_SIZE] = {0};
+	u8 loc_pg, loc_dpk;
+
+	loc_pg = rtw_get_rsvd_page_location(rtwdev, RSVD_LPS_PG_INFO);
+	loc_dpk = rtw_get_rsvd_page_location(rtwdev, RSVD_LPS_PG_DPK);
+
+	SET_H2C_CMD_ID_CLASS(h2c_pkt, H2C_CMD_LPS_PG_INFO);
+
+	LPS_PG_INFO_LOC(h2c_pkt, loc_pg);
+	LPS_PG_DPK_LOC(h2c_pkt, loc_dpk);
+	LPS_PG_SEC_CAM_EN(h2c_pkt, conf->sec_cam_backup);
+
+	rtw_fw_send_h2c_command(rtwdev, h2c_pkt);
+}
+
 void rtw_send_rsvd_page_h2c(struct rtw_dev *rtwdev)
 {
 	u8 h2c_pkt[H2C_PKT_SIZE] = {0};
@@ -442,6 +461,58 @@ rtw_beacon_get(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	return skb_new;
 }
 
+static struct sk_buff *rtw_lps_pg_dpk_get(struct ieee80211_hw *hw)
+{
+	struct rtw_dev *rtwdev = hw->priv;
+	struct rtw_chip_info *chip = rtwdev->chip;
+	struct rtw_dpk_info *dpk_info = &rtwdev->dm_info.dpk_info;
+	struct rtw_lps_pg_dpk_hdr *dpk_hdr;
+	struct sk_buff *skb;
+	u32 size;
+
+	size = chip->tx_pkt_desc_sz + sizeof(*dpk_hdr);
+	skb = alloc_skb(size, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+
+	skb_reserve(skb, chip->tx_pkt_desc_sz);
+	dpk_hdr = skb_put_zero(skb, sizeof(*dpk_hdr));
+	dpk_hdr->dpk_ch = dpk_info->dpk_ch;
+	dpk_hdr->dpk_path_ok = dpk_info->dpk_path_ok[0];
+	memcpy(dpk_hdr->dpk_txagc, dpk_info->dpk_txagc, 2);
+	memcpy(dpk_hdr->dpk_gs, dpk_info->dpk_gs, 4);
+	memcpy(dpk_hdr->coef, dpk_info->coef, 160);
+
+	return skb;
+}
+
+static struct sk_buff *rtw_lps_pg_info_get(struct ieee80211_hw *hw,
+					   struct ieee80211_vif *vif)
+{
+	struct rtw_dev *rtwdev = hw->priv;
+	struct rtw_chip_info *chip = rtwdev->chip;
+	struct rtw_lps_conf *conf = &rtwdev->lps_conf;
+	struct rtw_lps_pg_info_hdr *pg_info_hdr;
+	struct sk_buff *skb;
+	u32 size;
+
+	size = chip->tx_pkt_desc_sz + sizeof(*pg_info_hdr);
+	skb = alloc_skb(size, GFP_KERNEL);
+	if (!skb)
+		return NULL;
+
+	skb_reserve(skb, chip->tx_pkt_desc_sz);
+	pg_info_hdr = skb_put_zero(skb, sizeof(*pg_info_hdr));
+	pg_info_hdr->tx_bu_page_count = rtwdev->fifo.rsvd_drv_pg_num;
+	pg_info_hdr->macid = find_first_bit(rtwdev->mac_id_map, RTW_MAX_MAC_ID_NUM);
+	pg_info_hdr->sec_cam_count =
+		rtw_sec_cam_pg_backup(rtwdev, pg_info_hdr->sec_cam);
+
+	conf->sec_cam_backup = pg_info_hdr->sec_cam_count != 0;
+
+	return skb;
+}
+
 static struct sk_buff *rtw_get_rsvd_page_skb(struct ieee80211_hw *hw,
 					     struct ieee80211_vif *vif,
 					     enum rtw_rsvd_packet_type type)
@@ -464,6 +535,12 @@ static struct sk_buff *rtw_get_rsvd_page_skb(struct ieee80211_hw *hw,
 	case RSVD_QOS_NULL:
 		skb_new = ieee80211_nullfunc_get(hw, vif, true);
 		break;
+	case RSVD_LPS_PG_DPK:
+		skb_new = rtw_lps_pg_dpk_get(hw);
+		break;
+	case RSVD_LPS_PG_INFO:
+		skb_new = rtw_lps_pg_info_get(hw, vif);
+		break;
 	default:
 		return NULL;
 	}
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index e95d85b..f4028ef 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -58,6 +58,8 @@ enum rtw_rsvd_packet_type {
 	RSVD_PROBE_RESP,
 	RSVD_NULL,
 	RSVD_QOS_NULL,
+	RSVD_LPS_PG_DPK,
+	RSVD_LPS_PG_INFO,
 };
 
 enum rtw_fw_rf_type {
@@ -86,6 +88,25 @@ struct rtw_iqk_para {
 	u8 segment_iqk;
 };
 
+struct rtw_lps_pg_dpk_hdr {
+	u16 dpk_path_ok;
+	u8 dpk_txagc[2];
+	u16 dpk_gs[2];
+	u32 coef[2][20];
+	u8 dpk_ch;
+} __packed;
+
+struct rtw_lps_pg_info_hdr {
+	u8 macid;
+	u8 mbssid;
+	u8 pattern_count;
+	u8 mu_tab_group_id;
+	u8 sec_cam_count;
+	u8 tx_bu_page_count;
+	u16 rsvd;
+	u8 sec_cam[MAX_PG_CAM_BACKUP_NUM];
+} __packed;
+
 struct rtw_rsvd_page {
 	struct list_head list;
 	struct sk_buff *skb;
@@ -146,6 +167,7 @@ static inline void rtw_h2c_pkt_set_header(u8 *h2c_pkt, u8 sub_id)
 #define H2C_CMD_RSVD_PAGE		0x0
 #define H2C_CMD_MEDIA_STATUS_RPT	0x01
 #define H2C_CMD_SET_PWR_MODE		0x20
+#define H2C_CMD_LPS_PG_INFO		0x2b
 #define H2C_CMD_RA_INFO			0x40
 #define H2C_CMD_RSSI_MONITOR		0x42
 
@@ -177,6 +199,12 @@ static inline void rtw_h2c_pkt_set_header(u8 *h2c_pkt, u8 sub_id)
 	le32p_replace_bits((__le32 *)(h2c_pkt) + 0x01, value, GENMASK(7, 5))
 #define SET_PWR_MODE_SET_PWR_STATE(h2c_pkt, value)                             \
 	le32p_replace_bits((__le32 *)(h2c_pkt) + 0x01, value, GENMASK(15, 8))
+#define LPS_PG_INFO_LOC(h2c_pkt, value)                                        \
+	le32p_replace_bits((__le32 *)(h2c_pkt) + 0x00, value, GENMASK(23, 16))
+#define LPS_PG_DPK_LOC(h2c_pkt, value)                                         \
+	le32p_replace_bits((__le32 *)(h2c_pkt) + 0x00, value, GENMASK(31, 24))
+#define LPS_PG_SEC_CAM_EN(h2c_pkt, value)                                      \
+	le32p_replace_bits((__le32 *)(h2c_pkt) + 0x00, value, BIT(8))
 #define SET_RSSI_INFO_MACID(h2c_pkt, value)                                    \
 	le32p_replace_bits((__le32 *)(h2c_pkt) + 0x00, value, GENMASK(15, 8))
 #define SET_RSSI_INFO_RSSI(h2c_pkt, value)                                     \
@@ -270,6 +298,7 @@ void rtw_fw_send_phydm_info(struct rtw_dev *rtwdev);
 
 void rtw_fw_do_iqk(struct rtw_dev *rtwdev, struct rtw_iqk_para *para);
 void rtw_fw_set_pwr_mode(struct rtw_dev *rtwdev);
+void rtw_fw_set_pg_info(struct rtw_dev *rtwdev);
 void rtw_fw_query_bt_info(struct rtw_dev *rtwdev);
 void rtw_fw_wl_ch_info(struct rtw_dev *rtwdev, u8 link, u8 ch, u8 bw);
 void rtw_fw_query_bt_mp_info(struct rtw_dev *rtwdev,
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index e241d05..22b13e4 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -272,6 +272,8 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
 			rtw_add_rsvd_page(rtwdev, RSVD_PS_POLL, true);
 			rtw_add_rsvd_page(rtwdev, RSVD_QOS_NULL, true);
 			rtw_add_rsvd_page(rtwdev, RSVD_NULL, true);
+			rtw_add_rsvd_page(rtwdev, RSVD_LPS_PG_DPK, true);
+			rtw_add_rsvd_page(rtwdev, RSVD_LPS_PG_INFO, true);
 			rtw_fw_download_rsvd_page(rtwdev, vif);
 			rtw_send_rsvd_page_h2c(rtwdev);
 			rtw_coex_media_status_notify(rtwdev, conf->assoc);
@@ -435,6 +437,10 @@ static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		break;
 	}
 
+	/* download new cam settings for PG to backup */
+	if (rtw_fw_lps_deep_mode == LPS_DEEP_MODE_PG)
+		rtw_fw_download_rsvd_page(rtwdev, vif);
+
 out:
 	mutex_unlock(&rtwdev->mutex);
 
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index a59cbae..6221dc4 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -16,6 +16,7 @@
 
 #define RTW_MAX_MAC_ID_NUM		32
 #define RTW_MAX_SEC_CAM_NUM		32
+#define MAX_PG_CAM_BACKUP_NUM		8
 
 #define RTW_WATCH_DOG_DELAY_TIME	round_jiffies_relative(HZ * 2)
 
@@ -532,6 +533,7 @@ enum rtw_lps_mode {
 enum rtw_lps_deep_mode {
 	LPS_DEEP_MODE_NONE	= 0,
 	LPS_DEEP_MODE_LCLK	= 1,
+	LPS_DEEP_MODE_PG	= 2,
 };
 
 enum rtw_pwr_state {
@@ -548,6 +550,7 @@ struct rtw_lps_conf {
 	u8 rlbm;
 	u8 smart_ps;
 	u8 port_id;
+	bool sec_cam_backup;
 };
 
 enum rtw_hw_key_type {
diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index 02e104a..83db6cf 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -74,10 +74,13 @@ void rtw_power_mode_change(struct rtw_dev *rtwdev, bool enter)
 
 	/* toggle to request power mode, others remain 0 */
 	request ^= request | BIT_RPWM_TOGGLE;
-	if (!enter)
+	if (!enter) {
 		request |= POWER_MODE_ACK;
-	else
+	} else {
 		request |= POWER_MODE_LCLK;
+		if (rtw_fw_lps_deep_mode == LPS_DEEP_MODE_PG)
+			request |= POWER_MODE_PG;
+	}
 
 	rtw_write8(rtwdev, rtwdev->hci.rpwm_addr, request);
 
@@ -141,6 +144,9 @@ static void __rtw_enter_lps_deep(struct rtw_dev *rtwdev)
 		return;
 	}
 
+	if (rtw_fw_lps_deep_mode == LPS_DEEP_MODE_PG)
+		rtw_fw_set_pg_info(rtwdev);
+
 	rtw_hci_deep_ps(rtwdev, true);
 }
 
diff --git a/drivers/net/wireless/realtek/rtw88/ps.h b/drivers/net/wireless/realtek/rtw88/ps.h
index 03b08bd..18f687c 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.h
+++ b/drivers/net/wireless/realtek/rtw88/ps.h
@@ -8,6 +8,7 @@
 #define RTW_LPS_THRESHOLD	2
 
 #define POWER_MODE_ACK		BIT(6)
+#define POWER_MODE_PG		BIT(4)
 #define POWER_MODE_LCLK		BIT(0)
 
 int rtw_enter_ips(struct rtw_dev *rtwdev);
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822c.c b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
index b92940e5..a300efd 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822c.c
@@ -3747,7 +3747,7 @@ struct rtw_chip_info rtw8822c_hw_spec = {
 	.dig_min = 0x20,
 	.ht_supported = true,
 	.vht_supported = true,
-	.lps_deep_mode_supported = BIT(LPS_DEEP_MODE_LCLK),
+	.lps_deep_mode_supported = BIT(LPS_DEEP_MODE_LCLK) | BIT(LPS_DEEP_MODE_PG),
 	.sys_func_en = 0xD8,
 	.pwr_on_seq = card_enable_flow_8822c,
 	.pwr_off_seq = card_disable_flow_8822c,
diff --git a/drivers/net/wireless/realtek/rtw88/sec.c b/drivers/net/wireless/realtek/rtw88/sec.c
index c594fc0..d0d7fbb 100644
--- a/drivers/net/wireless/realtek/rtw88/sec.c
+++ b/drivers/net/wireless/realtek/rtw88/sec.c
@@ -96,6 +96,27 @@ void rtw_sec_clear_cam(struct rtw_dev *rtwdev,
 	rtw_write32(rtwdev, RTW_SEC_CMD_REG, command);
 }
 
+u8 rtw_sec_cam_pg_backup(struct rtw_dev *rtwdev, u8 *used_cam)
+{
+	struct rtw_sec_desc *sec = &rtwdev->sec;
+	u8 offset = 0;
+	u8 count, n;
+
+	if (!used_cam)
+		return 0;
+
+	for (count = 0; count < MAX_PG_CAM_BACKUP_NUM; count++) {
+		n = find_next_bit(sec->cam_map, RTW_MAX_SEC_CAM_NUM, offset);
+		if (n == RTW_MAX_SEC_CAM_NUM)
+			break;
+
+		used_cam[count] = n;
+		offset = n + 1;
+	}
+
+	return count;
+}
+
 void rtw_sec_enable_sec_engine(struct rtw_dev *rtwdev)
 {
 	struct rtw_sec_desc *sec = &rtwdev->sec;
diff --git a/drivers/net/wireless/realtek/rtw88/sec.h b/drivers/net/wireless/realtek/rtw88/sec.h
index 8c50a89..efcf454 100644
--- a/drivers/net/wireless/realtek/rtw88/sec.h
+++ b/drivers/net/wireless/realtek/rtw88/sec.h
@@ -34,6 +34,7 @@ void rtw_sec_write_cam(struct rtw_dev *rtwdev,
 void rtw_sec_clear_cam(struct rtw_dev *rtwdev,
 		       struct rtw_sec_desc *sec,
 		       u8 hw_key_idx);
+u8 rtw_sec_cam_pg_backup(struct rtw_dev *rtwdev, u8 *used_cam);
 void rtw_sec_enable_sec_engine(struct rtw_dev *rtwdev);
 
 #endif
-- 
2.7.4


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

* [PATCH 15/15] rtw88: remove misleading module parameter rtw_fw_support_lps
  2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
                   ` (13 preceding siblings ...)
  2019-09-16  7:03 ` [PATCH 14/15] rtw88: add deep PS PG mode for 8822c yhchuang
@ 2019-09-16  7:03 ` yhchuang
  14 siblings, 0 replies; 26+ messages in thread
From: yhchuang @ 2019-09-16  7:03 UTC (permalink / raw)
  To: kvalo; +Cc: linux-wireless, briannorris

From: Yan-Hsuan Chuang <yhchuang@realtek.com>

The module parameter rtw_fw_support_lps is misleading. It
is not used to represent the firmware's property, but to
determine if driver wants to ask firmware to enter LPS.

However, driver should better enable/disable PS through
cfg80211_ops::set_power_mgmt instead.
For example, one could use iw command to set PS state.

  $ sudo iw wlanX set power_save [on/off]

So rtw_fw_support_lps should be removed because it is
misleading and useless. Instead of checking the parameter,
set PS mode according to IEEE80211_CONF_PS.

Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/mac80211.c |  9 ++++++++
 drivers/net/wireless/realtek/rtw88/main.c     | 33 ++++++++++++---------------
 drivers/net/wireless/realtek/rtw88/main.h     |  1 +
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index 22b13e4..97777c7 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -74,6 +74,15 @@ 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);
 
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 3c366a3..a3e9f91 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -16,16 +16,13 @@
 
 unsigned int rtw_fw_lps_deep_mode;
 EXPORT_SYMBOL(rtw_fw_lps_deep_mode);
-static bool rtw_fw_support_lps;
 unsigned int rtw_debug_mask;
 EXPORT_SYMBOL(rtw_debug_mask);
 
 module_param_named(lps_deep_mode, rtw_fw_lps_deep_mode, uint, 0644);
-module_param_named(support_lps, rtw_fw_support_lps, bool, 0644);
 module_param_named(debug_mask, rtw_debug_mask, uint, 0644);
 
 MODULE_PARM_DESC(lps_deep_mode, "Deeper PS mode. If 0, deep PS is disabled");
-MODULE_PARM_DESC(support_lps, "Set Y to enable Leisure Power Save support, to turn radio off between beacons");
 MODULE_PARM_DESC(debug_mask, "Debugging mask");
 
 static struct ieee80211_channel rtw_channeltable_2g[] = {
@@ -117,8 +114,6 @@ static struct ieee80211_supported_band rtw_band_5ghz = {
 
 struct rtw_watch_dog_iter_data {
 	struct rtw_vif *rtwvif;
-	bool active;
-	u8 assoc_cnt;
 };
 
 static void rtw_vif_watch_dog_iter(void *data, u8 *mac,
@@ -127,18 +122,9 @@ static void rtw_vif_watch_dog_iter(void *data, u8 *mac,
 	struct rtw_watch_dog_iter_data *iter_data = data;
 	struct rtw_vif *rtwvif = (struct rtw_vif *)vif->drv_priv;
 
-	if (vif->type == NL80211_IFTYPE_STATION) {
-		if (vif->bss_conf.assoc) {
-			iter_data->assoc_cnt++;
+	if (vif->type == NL80211_IFTYPE_STATION)
+		if (vif->bss_conf.assoc)
 			iter_data->rtwvif = rtwvif;
-		}
-		if (rtwvif->stats.tx_cnt > RTW_LPS_THRESHOLD ||
-		    rtwvif->stats.rx_cnt > RTW_LPS_THRESHOLD)
-			iter_data->active = true;
-	} else {
-		/* only STATION mode can enter lps */
-		iter_data->active = true;
-	}
 
 	rtwvif->stats.tx_unicast = 0;
 	rtwvif->stats.rx_unicast = 0;
@@ -155,6 +141,7 @@ static void rtw_watch_dog_work(struct work_struct *work)
 					      watch_dog_work.work);
 	struct rtw_watch_dog_iter_data data = {};
 	bool busy_traffic = test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags);
+	bool ps_active;
 
 	mutex_lock(&rtwdev->mutex);
 
@@ -172,6 +159,12 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	if (busy_traffic != test_bit(RTW_FLAG_BUSY_TRAFFIC, rtwdev->flags))
 		rtw_coex_wl_status_change_notify(rtwdev);
 
+	if (rtwdev->stats.tx_cnt > RTW_LPS_THRESHOLD ||
+	    rtwdev->stats.rx_cnt > RTW_LPS_THRESHOLD)
+		ps_active = true;
+	else
+		ps_active = false;
+
 	/* reset tx/rx statictics */
 	rtwdev->stats.tx_unicast = 0;
 	rtwdev->stats.rx_unicast = 0;
@@ -192,9 +185,13 @@ static void rtw_watch_dog_work(struct work_struct *work)
 	/* fw supports only one station associated to enter lps, if there are
 	 * 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
+	 * get that vif and check if device is having traffic more than the
+	 * threshold.
 	 */
-	if (rtw_fw_support_lps &&
-	    data.rtwvif && !data.active && data.assoc_cnt == 1)
+	if (rtwdev->ps_enabled && data.rtwvif && !ps_active)
 		rtw_enter_lps(rtwdev, data.rtwvif->port);
 
 	rtwdev->watch_dog_cnt++;
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 6221dc4..161297a 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1372,6 +1372,7 @@ struct rtw_dev {
 
 	/* lps power state & handler work */
 	struct rtw_lps_conf lps_conf;
+	bool ps_enabled;
 
 	struct dentry *debugfs;
 
-- 
2.7.4


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

* Re: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-16  7:03 ` [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop yhchuang
@ 2019-09-18  0:41   ` Brian Norris
  2019-09-18  2:10     ` Tony Chuang
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Norris @ 2019-09-18  0:41 UTC (permalink / raw)
  To: Tony Chuang; +Cc: Kalle Valo, linux-wireless

May be a dumb question but:

On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@realtek.com> wrote:
>
> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>
> Interrupt is disabled to stop PCI, which means the skbs
> queued for each TX ring will not be released via DMA
> interrupt.

In what cases do you hit this? I think you do this when entering PS
mode, no? But then, see below.

> To avoid those skbs remained being left in
> the skb queue until PCI has been removed, driver needs
> to release skbs by itself.

Doesn't that also mean your dropping these packets? Shouldn't you be
delaying PS transitions until you've finished TX'ing?

Brian

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

* RE: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-18  0:41   ` Brian Norris
@ 2019-09-18  2:10     ` Tony Chuang
  2019-09-20  0:35       ` Brian Norris
  0 siblings, 1 reply; 26+ messages in thread
From: Tony Chuang @ 2019-09-18  2:10 UTC (permalink / raw)
  To: Brian Norris; +Cc: Kalle Valo, linux-wireless

> May be a dumb question but:
> 
> On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@realtek.com> wrote:
> >
> > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >
> > Interrupt is disabled to stop PCI, which means the skbs
> > queued for each TX ring will not be released via DMA
> > interrupt.
> 
> In what cases do you hit this? I think you do this when entering PS
> mode, no? But then, see below.

I'll hit this when ieee80211_ops::stop, or rtw_power_off.
Both are to turn off the device, so there's no more DMA activities.
If we don't release the SKBs that are not released by DMA interrupt
when powering off, these could be leaked.

> 
> > To avoid those skbs remained being left in
> > the skb queue until PCI has been removed, driver needs
> > to release skbs by itself.
> 
> Doesn't that also mean your dropping these packets? Shouldn't you be
> delaying PS transitions until you've finished TX'ing?
> 
> Brian
> 

Yan-Hsuan


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

* Re: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-18  2:10     ` Tony Chuang
@ 2019-09-20  0:35       ` Brian Norris
  2019-09-20  7:26         ` Kalle Valo
  0 siblings, 1 reply; 26+ messages in thread
From: Brian Norris @ 2019-09-20  0:35 UTC (permalink / raw)
  To: Tony Chuang; +Cc: Kalle Valo, linux-wireless

On Tue, Sep 17, 2019 at 7:10 PM Tony Chuang <yhchuang@realtek.com> wrote:
> > On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@realtek.com> wrote:
> > >
> > > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> > >
> > > Interrupt is disabled to stop PCI, which means the skbs
> > > queued for each TX ring will not be released via DMA
> > > interrupt.
> >
> > In what cases do you hit this? I think you do this when entering PS
> > mode, no? But then, see below.
>
> I'll hit this when ieee80211_ops::stop, or rtw_power_off.
> Both are to turn off the device, so there's no more DMA activities.
> If we don't release the SKBs that are not released by DMA interrupt
> when powering off, these could be leaked.

Ah, I was a bit confused. So it does get called from "PS" routines:
rtw_enter_ips() -> rtw_core_stop()
but that "IPS" mode means "Inactive" Power Save, and it's only used
when transitioning into idle states (IEEE80211_CONF_IDLE).

Incidentally, I think this also may explain many of the leaks I've
been seeing elsewhere, when I leave a device sitting and scanning for
a very long time -- each scan attempt is making a single transition
out-and-back to IPS mode, which meant it may be leaking any
outstanding TX DMA. And testing confirms this: if I just bring up the
interface, run a scan, then bring it down, I see many fewer unmaps
than maps. Doing this enough times, I run out of contiguous DMA memory
and the device stops working. This fixes that problem for me. So:

Reviewed-by: Brian Norris <briannorris@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>

I wonder if, given the problems I've seen (the driver can become
totally ineroperable), this patch and the previous patch (its only
real dependency) should be fast-tracked to the current release.

Brian

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

* Re: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-20  0:35       ` Brian Norris
@ 2019-09-20  7:26         ` Kalle Valo
  2019-09-20  8:29           ` Tony Chuang
  0 siblings, 1 reply; 26+ messages in thread
From: Kalle Valo @ 2019-09-20  7:26 UTC (permalink / raw)
  To: Brian Norris; +Cc: Tony Chuang, linux-wireless

Brian Norris <briannorris@chromium.org> writes:

> On Tue, Sep 17, 2019 at 7:10 PM Tony Chuang <yhchuang@realtek.com> wrote:
>> > On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@realtek.com> wrote:
>> > >
>> > > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> > >
>> > > Interrupt is disabled to stop PCI, which means the skbs
>> > > queued for each TX ring will not be released via DMA
>> > > interrupt.
>> >
>> > In what cases do you hit this? I think you do this when entering PS
>> > mode, no? But then, see below.
>>
>> I'll hit this when ieee80211_ops::stop, or rtw_power_off.
>> Both are to turn off the device, so there's no more DMA activities.
>> If we don't release the SKBs that are not released by DMA interrupt
>> when powering off, these could be leaked.
>
> Ah, I was a bit confused. So it does get called from "PS" routines:
> rtw_enter_ips() -> rtw_core_stop()
> but that "IPS" mode means "Inactive" Power Save, and it's only used
> when transitioning into idle states (IEEE80211_CONF_IDLE).
>
> Incidentally, I think this also may explain many of the leaks I've
> been seeing elsewhere, when I leave a device sitting and scanning for
> a very long time -- each scan attempt is making a single transition
> out-and-back to IPS mode, which meant it may be leaking any
> outstanding TX DMA. And testing confirms this: if I just bring up the
> interface, run a scan, then bring it down, I see many fewer unmaps
> than maps. Doing this enough times, I run out of contiguous DMA memory
> and the device stops working. This fixes that problem for me. So:
>
> Reviewed-by: Brian Norris <briannorris@chromium.org>
> Tested-by: Brian Norris <briannorris@chromium.org>
>
> I wonder if, given the problems I've seen (the driver can become
> totally ineroperable), this patch and the previous patch (its only
> real dependency) should be fast-tracked to the current release.

I agree, this sounds like a serious problem. So I'm planning to queue
patches 4 and 5 to v5.4, if it's ok for Tony.

-- 
Kalle Valo

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

* RE: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-20  7:26         ` Kalle Valo
@ 2019-09-20  8:29           ` Tony Chuang
  2019-09-20  8:35             ` Kalle Valo
  2019-09-20 22:33             ` Brian Norris
  0 siblings, 2 replies; 26+ messages in thread
From: Tony Chuang @ 2019-09-20  8:29 UTC (permalink / raw)
  To: Kalle Valo, Brian Norris; +Cc: linux-wireless

> Brian Norris <briannorris@chromium.org> writes:
> 
> > On Tue, Sep 17, 2019 at 7:10 PM Tony Chuang <yhchuang@realtek.com>
> wrote:
> >> > On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@realtek.com> wrote:
> >> > >
> >> > > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> >> > >
> >> > > Interrupt is disabled to stop PCI, which means the skbs
> >> > > queued for each TX ring will not be released via DMA
> >> > > interrupt.
> >> >
> >> > In what cases do you hit this? I think you do this when entering PS
> >> > mode, no? But then, see below.
> >>
> >> I'll hit this when ieee80211_ops::stop, or rtw_power_off.
> >> Both are to turn off the device, so there's no more DMA activities.
> >> If we don't release the SKBs that are not released by DMA interrupt
> >> when powering off, these could be leaked.
> >
> > Ah, I was a bit confused. So it does get called from "PS" routines:

I thought you're talking about IEEE80211_CONF_PS instead of
IEEE80211_CONF_IDLE.

> > rtw_enter_ips() -> rtw_core_stop()
> > but that "IPS" mode means "Inactive" Power Save, and it's only used
> > when transitioning into idle states (IEEE80211_CONF_IDLE).
> >
> > Incidentally, I think this also may explain many of the leaks I've
> > been seeing elsewhere, when I leave a device sitting and scanning for
> > a very long time -- each scan attempt is making a single transition
> > out-and-back to IPS mode, which meant it may be leaking any
> > outstanding TX DMA. And testing confirms this: if I just bring up the
> > interface, run a scan, then bring it down, I see many fewer unmaps
> > than maps. Doing this enough times, I run out of contiguous DMA memory
> > and the device stops working. This fixes that problem for me. So:
> >
> > Reviewed-by: Brian Norris <briannorris@chromium.org>
> > Tested-by: Brian Norris <briannorris@chromium.org>
> >
> > I wonder if, given the problems I've seen (the driver can become
> > totally ineroperable), this patch and the previous patch (its only
> > real dependency) should be fast-tracked to the current release.
> 
> I agree, this sounds like a serious problem. So I'm planning to queue
> patches 4 and 5 to v5.4, if it's ok for Tony.

It's OK for me, didn't realize that this is a serious problem, so I missed it.
Also if possible you should queue patch 2, that reordering will cause
two H2C skbs not be released because HCI hasn't started, everytime
enter/leave IDLE state (rtw_power_[on|off]).

Should I resend and add a v5.4 prefix or something?

Yan-Hsuan

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

* Re: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-20  8:29           ` Tony Chuang
@ 2019-09-20  8:35             ` Kalle Valo
  2019-09-20 22:33             ` Brian Norris
  1 sibling, 0 replies; 26+ messages in thread
From: Kalle Valo @ 2019-09-20  8:35 UTC (permalink / raw)
  To: Tony Chuang; +Cc: Brian Norris, linux-wireless

Tony Chuang <yhchuang@realtek.com> writes:

>> > I wonder if, given the problems I've seen (the driver can become
>> > totally ineroperable), this patch and the previous patch (its only
>> > real dependency) should be fast-tracked to the current release.
>> 
>> I agree, this sounds like a serious problem. So I'm planning to queue
>> patches 4 and 5 to v5.4, if it's ok for Tony.
>
> It's OK for me, didn't realize that this is a serious problem, so I missed it.
> Also if possible you should queue patch 2, that reordering will cause
> two H2C skbs not be released because HCI hasn't started, everytime
> enter/leave IDLE state (rtw_power_[on|off]).
>
> Should I resend and add a v5.4 prefix or something?

No need to resend, I'll try to apply patches 2, 4 and 5 as is and will
let you know if there are any problems.

-- 
Kalle Valo

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

* Re: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
  2019-09-20  8:29           ` Tony Chuang
  2019-09-20  8:35             ` Kalle Valo
@ 2019-09-20 22:33             ` Brian Norris
  1 sibling, 0 replies; 26+ messages in thread
From: Brian Norris @ 2019-09-20 22:33 UTC (permalink / raw)
  To: Tony Chuang; +Cc: Kalle Valo, linux-wireless

On Fri, Sep 20, 2019 at 1:29 AM Tony Chuang <yhchuang@realtek.com> wrote:
> > Brian Norris <briannorris@chromium.org> writes:
> > > Ah, I was a bit confused. So it does get called from "PS" routines:
>
> I thought you're talking about IEEE80211_CONF_PS instead of
> IEEE80211_CONF_IDLE.

Like I said, I was confused :)

On first glance, I just saw the codepath showing up in ps.c, but then
I noticed it's only for IDLE, not PS.

> Also if possible you should queue patch 2, that reordering will cause
> two H2C skbs not be released because HCI hasn't started, everytime
> enter/leave IDLE state (rtw_power_[on|off]).

That patch also looks good to me, FWIW.

Side note: it's a little bit strange that your driver can silently
misbehave so badly, just by TX'ing in the wrong state. Would this be a
good case to add some WARN_ON() or WARN_ON_ONCE() (e.g., in functions
like rtw_fw_send_h2c_packet()), to check for the appropriate "started"
state?

Brian

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

* Re: [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings
  2019-09-16  7:03 ` [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings yhchuang
@ 2019-09-21  5:47   ` Kalle Valo
  0 siblings, 0 replies; 26+ messages in thread
From: Kalle Valo @ 2019-09-21  5:47 UTC (permalink / raw)
  To: yhchuang; +Cc: linux-wireless, briannorris

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> These skbs free routines could be used when driver wants
> to stop PCI bus, because some of the skbs remained in the
> queue may not have been returned via DMA interrupt.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

2 patches applied to wireless-drivers.git, thanks.

dc579ca5cfea rtw88: pci: extract skbs free routine for trx rings
0e41edcdfe86 rtw88: pci: release tx skbs DMAed when stop

-- 
https://patchwork.kernel.org/patch/11146415/

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


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

* Re: [PATCH 02/15] rtw88: configure firmware after HCI started
  2019-09-16  7:03 ` [PATCH 02/15] rtw88: configure firmware after HCI started yhchuang
@ 2019-09-21  5:51   ` Kalle Valo
  0 siblings, 0 replies; 26+ messages in thread
From: Kalle Valo @ 2019-09-21  5:51 UTC (permalink / raw)
  To: yhchuang; +Cc: linux-wireless, briannorris

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> After firmware has been downloaded, driver should send
> some information to it through H2C commands. Those H2C
> commands are transmitted through TX path.
> 
> But before HCI has been started, the TX path is not
> working completely. Such as PCI interfaces, the interrupts
> are not enabled, hence TX interrupts will not be issued
> after H2C skb has been DMAed to the device. And the H2C
> skbs will not be released until the device is powered off.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

This didn't apply. Please rebase on top of wireless-drivers, mark this
as "[PATCH 5.4 v2]" and resend.

fatal: sha1 information is lacking or useless (drivers/net/wireless/realtek/rtw88/mac.c).
error: could not build fake ancestor
Applying: rtw88: configure firmware after HCI started
Patch failed at 0001 rtw88: configure firmware after HCI started
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/11146411/

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


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

* Re: [PATCH 01/15] rtw88: remove redundant flag check helper function
  2019-09-16  7:03 ` [PATCH 01/15] rtw88: remove redundant flag check helper function yhchuang
@ 2019-10-01  9:17   ` Kalle Valo
  0 siblings, 0 replies; 26+ messages in thread
From: Kalle Valo @ 2019-10-01  9:17 UTC (permalink / raw)
  To: yhchuang; +Cc: linux-wireless, briannorris

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> These helper functions seems useless. And in some cases
> we want to use test_and_[set/clear]_bit, these helpers
> will make the code more complicated. So remove them.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

Does not apply anymore, please rebase.

Recorded preimage for 'drivers/net/wireless/realtek/rtw88/mac.c'
error: Failed to merge in the changes.
Applying: rtw88: remove redundant flag check helper function
Using index info to reconstruct a base tree...
M	drivers/net/wireless/realtek/rtw88/mac.c
M	drivers/net/wireless/realtek/rtw88/main.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/realtek/rtw88/main.c
Auto-merging drivers/net/wireless/realtek/rtw88/mac.c
CONFLICT (content): Merge conflict in drivers/net/wireless/realtek/rtw88/mac.c
Patch failed at 0001 rtw88: remove redundant flag check helper function
The copy of the patch that failed is found in: .git/rebase-apply/patch

12 patches set to Changes Requested.

11146413 [01/15] rtw88: remove redundant flag check helper function
11146419 [03/15] rtw88: pci: reset H2C queue indexes in a single write
11146421 [06/15] rtw88: not to enter or leave PS under IRQ
11146441 [07/15] rtw88: not to control LPS by each vif
11146423 [08/15] rtw88: remove unused lps state check helper
11146437 [09/15] rtw88: LPS enter/leave should be protected by lock
11146425 [10/15] rtw88: leave PS state for dynamic mechanism
11146429 [11/15] rtw88: add deep power save support
11146435 [12/15] rtw88: not to enter LPS by coex strategy
11146427 [13/15] rtw88: select deep PS mode when module is inserted
11146433 [14/15] rtw88: add deep PS PG mode for 8822c
11146431 [15/15] rtw88: remove misleading module parameter rtw_fw_support_lps

-- 
https://patchwork.kernel.org/patch/11146413/

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


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

end of thread, other threads:[~2019-10-01  9:17 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16  7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
2019-09-16  7:03 ` [PATCH 01/15] rtw88: remove redundant flag check helper function yhchuang
2019-10-01  9:17   ` Kalle Valo
2019-09-16  7:03 ` [PATCH 02/15] rtw88: configure firmware after HCI started yhchuang
2019-09-21  5:51   ` Kalle Valo
2019-09-16  7:03 ` [PATCH 03/15] rtw88: pci: reset H2C queue indexes in a single write yhchuang
2019-09-16  7:03 ` [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings yhchuang
2019-09-21  5:47   ` Kalle Valo
2019-09-16  7:03 ` [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop yhchuang
2019-09-18  0:41   ` Brian Norris
2019-09-18  2:10     ` Tony Chuang
2019-09-20  0:35       ` Brian Norris
2019-09-20  7:26         ` Kalle Valo
2019-09-20  8:29           ` Tony Chuang
2019-09-20  8:35             ` Kalle Valo
2019-09-20 22:33             ` Brian Norris
2019-09-16  7:03 ` [PATCH 06/15] rtw88: not to enter or leave PS under IRQ yhchuang
2019-09-16  7:03 ` [PATCH 07/15] rtw88: not to control LPS by each vif yhchuang
2019-09-16  7:03 ` [PATCH 08/15] rtw88: remove unused lps state check helper yhchuang
2019-09-16  7:03 ` [PATCH 09/15] rtw88: LPS enter/leave should be protected by lock yhchuang
2019-09-16  7:03 ` [PATCH 10/15] rtw88: leave PS state for dynamic mechanism yhchuang
2019-09-16  7:03 ` [PATCH 11/15] rtw88: add deep power save support yhchuang
2019-09-16  7:03 ` [PATCH 12/15] rtw88: not to enter LPS by coex strategy yhchuang
2019-09-16  7:03 ` [PATCH 13/15] rtw88: select deep PS mode when module is inserted yhchuang
2019-09-16  7:03 ` [PATCH 14/15] rtw88: add deep PS PG mode for 8822c yhchuang
2019-09-16  7:03 ` [PATCH 15/15] rtw88: remove misleading module parameter rtw_fw_support_lps yhchuang

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).