2022-04-07 15:45:20

by Ping-Ke Shih

[permalink] [raw]
Subject: [PATCH 3/6] rtw88: Add update beacon flow for AP mode

From: Po-Hao Huang <[email protected]>

To support stations in power saving mode, AP should notify stations
that there are frames buffered at the AP via TIM during beacons.
Driver used to transmit identical beacons that were downloaded to
hardware during the initiation phase. This beacon will become
obsolete over time.

If the beacon does not contain sufficient information, station would
not be able to percept that there is data to receive. Hence it won't
wake up and start the PS-poll procedure, this could lead to timeout
and/or lost data segments. In order to resolve this issue, driver will
now download beacon to hardware whenever the content is updated.

Enable hardware to update dtim_count for more efficiency, this reduces
the overhead of downloading beacon at every beacon interval since most
of the time only the dtim_count needs to be updated.

Change queue mapping for broadcast/multicast frames to high queue, so
these frames can be prioritized and sent when dtim_count is zero.

Signed-off-by: Po-Hao Huang <[email protected]>
Signed-off-by: Ping-Ke Shih <[email protected]>
---
drivers/net/wireless/realtek/rtw88/fw.c | 4 +++-
drivers/net/wireless/realtek/rtw88/fw.h | 1 +
drivers/net/wireless/realtek/rtw88/mac80211.c | 17 ++++++++++++++++-
drivers/net/wireless/realtek/rtw88/main.c | 6 ++++++
drivers/net/wireless/realtek/rtw88/main.h | 2 ++
drivers/net/wireless/realtek/rtw88/pci.c | 3 +++
drivers/net/wireless/realtek/rtw88/reg.h | 2 ++
drivers/net/wireless/realtek/rtw88/tx.c | 17 +++++++++++++++++
drivers/net/wireless/realtek/rtw88/tx.h | 4 ++++
9 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c
index 941df27f0b692..c1af2704bb866 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.c
+++ b/drivers/net/wireless/realtek/rtw88/fw.c
@@ -1047,6 +1047,7 @@ static struct sk_buff *rtw_get_rsvd_page_skb(struct ieee80211_hw *hw,
struct rtw_vif *rtwvif;
struct sk_buff *skb_new;
struct cfg80211_ssid *ssid;
+ u16 tim_offset;

if (rsvd_pkt->type == RSVD_DUMMY) {
skb_new = alloc_skb(1, GFP_KERNEL);
@@ -1065,7 +1066,8 @@ static struct sk_buff *rtw_get_rsvd_page_skb(struct ieee80211_hw *hw,

switch (rsvd_pkt->type) {
case RSVD_BEACON:
- skb_new = ieee80211_beacon_get(hw, vif);
+ skb_new = ieee80211_beacon_get_tim(hw, vif, &tim_offset, NULL);
+ rsvd_pkt->tim_offset = tim_offset;
break;
case RSVD_PS_POLL:
skb_new = ieee80211_pspoll_get(hw, vif);
diff --git a/drivers/net/wireless/realtek/rtw88/fw.h b/drivers/net/wireless/realtek/rtw88/fw.h
index 3486eb9585def..734113fba184e 100644
--- a/drivers/net/wireless/realtek/rtw88/fw.h
+++ b/drivers/net/wireless/realtek/rtw88/fw.h
@@ -172,6 +172,7 @@ struct rtw_rsvd_page {
struct sk_buff *skb;
enum rtw_rsvd_packet_type type;
u8 page;
+ u16 tim_offset;
bool add_txdesc;
struct cfg80211_ssid *ssid;
u16 probe_req_size;
diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
index c9341af493645..1ee41dfda5e1b 100644
--- a/drivers/net/wireless/realtek/rtw88/mac80211.c
+++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
@@ -402,8 +402,10 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
coex_stat->wl_beacon_interval = conf->beacon_int;
}

- if (changed & BSS_CHANGED_BEACON)
+ if (changed & BSS_CHANGED_BEACON) {
+ rtw_set_dtim_period(rtwdev, conf->dtim_period);
rtw_fw_download_rsvd_page(rtwdev);
+ }

if (changed & BSS_CHANGED_BEACON_ENABLED) {
if (conf->enable_beacon)
@@ -474,6 +476,18 @@ static int rtw_ops_sta_remove(struct ieee80211_hw *hw,
return 0;
}

+static int rtw_ops_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
+ bool set)
+{
+ struct rtw_dev *rtwdev = hw->priv;
+
+ mutex_lock(&rtwdev->mutex);
+ rtw_fw_download_rsvd_page(rtwdev);
+ mutex_unlock(&rtwdev->mutex);
+
+ return 0;
+}
+
static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
struct ieee80211_vif *vif, struct ieee80211_sta *sta,
struct ieee80211_key_conf *key)
@@ -875,6 +889,7 @@ const struct ieee80211_ops rtw_ops = {
.conf_tx = rtw_ops_conf_tx,
.sta_add = rtw_ops_sta_add,
.sta_remove = rtw_ops_sta_remove,
+ .set_tim = rtw_ops_set_tim,
.set_key = rtw_ops_set_key,
.ampdu_action = rtw_ops_ampdu_action,
.can_aggregate_in_amsdu = rtw_ops_can_aggregate_in_amsdu,
diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 9a0a4babf1b60..8fb102cb04c37 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -664,6 +664,12 @@ void rtw_set_rx_freq_band(struct rtw_rx_pkt_stat *pkt_stat, u8 channel)
}
EXPORT_SYMBOL(rtw_set_rx_freq_band);

+void rtw_set_dtim_period(struct rtw_dev *rtwdev, int dtim_period)
+{
+ rtw_write32_set(rtwdev, REG_TCR, BIT_TCR_UPDATE_TIMIE);
+ rtw_write8(rtwdev, REG_DTIM_COUNTER_ROOT, dtim_period - 1);
+}
+
void rtw_get_channel_params(struct cfg80211_chan_def *chandef,
struct rtw_channel_params *chan_params)
{
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index f694a2d86e0b2..2743074a42560 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -580,6 +580,7 @@ struct rtw_tx_pkt_info {
u32 tx_pkt_size;
u8 offset;
u8 pkt_offset;
+ u8 tim_offset;
u8 mac_id;
u8 rate_id;
u8 rate;
@@ -2131,6 +2132,7 @@ static inline int rtw_chip_dump_fw_crash(struct rtw_dev *rtwdev)
}

void rtw_set_rx_freq_band(struct rtw_rx_pkt_stat *pkt_stat, u8 channel);
+void rtw_set_dtim_period(struct rtw_dev *rtwdev, int dtim_period);
void rtw_get_channel_params(struct cfg80211_chan_def *chandef,
struct rtw_channel_params *ch_param);
bool check_hw_ready(struct rtw_dev *rtwdev, u32 addr, u32 mask, u32 target);
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index a0991d3f15c01..8e122e121e854 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -689,6 +689,9 @@ static u8 rtw_hw_queue_mapping(struct sk_buff *skb)
queue = RTW_TX_QUEUE_BCN;
else if (unlikely(ieee80211_is_mgmt(fc) || ieee80211_is_ctl(fc)))
queue = RTW_TX_QUEUE_MGMT;
+ else if (is_broadcast_ether_addr(hdr->addr1) ||
+ is_multicast_ether_addr(hdr->addr1))
+ queue = RTW_TX_QUEUE_HI0;
else if (WARN_ON_ONCE(q_mapping >= ARRAY_SIZE(ac_to_hwq)))
queue = ac_to_hwq[IEEE80211_AC_BE];
else
diff --git a/drivers/net/wireless/realtek/rtw88/reg.h b/drivers/net/wireless/realtek/rtw88/reg.h
index 84ba9ec489c37..03bd8dc53f72a 100644
--- a/drivers/net/wireless/realtek/rtw88/reg.h
+++ b/drivers/net/wireless/realtek/rtw88/reg.h
@@ -389,12 +389,14 @@
#define BIT_EN_FREE_CNT BIT(3)
#define BIT_DIS_SECOND_CCA (BIT(0) | BIT(1))
#define REG_HIQ_NO_LMT_EN 0x5A7
+#define REG_DTIM_COUNTER_ROOT 0x5A8
#define BIT_HIQ_NO_LMT_EN_ROOT BIT(0)
#define REG_TIMER0_SRC_SEL 0x05B4
#define BIT_TSFT_SEL_TIMER0 (BIT(4) | BIT(5) | BIT(6))

#define REG_TCR 0x0604
#define BIT_PWRMGT_HWDATA_EN BIT(7)
+#define BIT_TCR_UPDATE_TIMIE BIT(5)
#define REG_RCR 0x0608
#define BIT_APP_FCS BIT(31)
#define BIT_APP_MIC BIT(30)
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index 94d1089f40220..ec440c8969a5b 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -67,6 +67,10 @@ void rtw_tx_fill_tx_desc(struct rtw_tx_pkt_info *pkt_info, struct sk_buff *skb)
SET_TX_DESC_HW_SSN_SEL(txdesc, pkt_info->hw_ssn_sel);
SET_TX_DESC_NAVUSEHDR(txdesc, pkt_info->nav_use_hdr);
SET_TX_DESC_BT_NULL(txdesc, pkt_info->bt_null);
+ if (pkt_info->tim_offset) {
+ SET_TX_DESC_TIM_EN(txdesc, 1);
+ SET_TX_DESC_TIM_OFFSET(txdesc, pkt_info->tim_offset);
+ }
}
EXPORT_SYMBOL(rtw_tx_fill_tx_desc);

@@ -448,6 +452,19 @@ void rtw_tx_rsvd_page_pkt_info_update(struct rtw_dev *rtwdev,
if (type == RSVD_QOS_NULL)
pkt_info->bt_null = true;

+ if (type == RSVD_BEACON) {
+ struct rtw_rsvd_page *rsvd_pkt;
+ int hdr_len;
+
+ rsvd_pkt = list_first_entry_or_null(&rtwdev->rsvd_page_list,
+ struct rtw_rsvd_page,
+ build_list);
+ if (rsvd_pkt && rsvd_pkt->tim_offset != 0) {
+ hdr_len = sizeof(struct ieee80211_hdr_3addr);
+ pkt_info->tim_offset = rsvd_pkt->tim_offset - hdr_len;
+ }
+ }
+
rtw_tx_pkt_info_update_sec(rtwdev, pkt_info, skb);

/* TODO: need to change hw port and hw ssn sel for multiple vifs */
diff --git a/drivers/net/wireless/realtek/rtw88/tx.h b/drivers/net/wireless/realtek/rtw88/tx.h
index 56371eff9f7ff..8419603adce4a 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.h
+++ b/drivers/net/wireless/realtek/rtw88/tx.h
@@ -33,6 +33,10 @@
le32p_replace_bits((__le32 *)(txdesc) + 0x05, value, GENMASK(6, 5))
#define SET_TX_DESC_SW_SEQ(txdesc, value) \
le32p_replace_bits((__le32 *)(txdesc) + 0x09, value, GENMASK(23, 12))
+#define SET_TX_DESC_TIM_EN(txdesc, value) \
+ le32p_replace_bits((__le32 *)(txdesc) + 0x09, value, BIT(7))
+#define SET_TX_DESC_TIM_OFFSET(txdesc, value) \
+ le32p_replace_bits((__le32 *)(txdesc) + 0x09, value, GENMASK(6, 0))
#define SET_TX_DESC_MAX_AGG_NUM(txdesc, value) \
le32p_replace_bits((__le32 *)(txdesc) + 0x03, value, GENMASK(21, 17))
#define SET_TX_DESC_USE_RTS(tx_desc, value) \
--
2.25.1


2022-05-26 00:13:29

by Ondřej Jirman

[permalink] [raw]
Subject: Re: [PATCH 3/6] rtw88: Add update beacon flow for AP mode

Hello Ping-Ke,

please see bugreport below:

On Thu, Apr 07, 2022 at 05:58:55PM +0800, Ping-Ke Shih wrote:
> From: Po-Hao Huang <[email protected]>
>
> To support stations in power saving mode, AP should notify stations
> that there are frames buffered at the AP via TIM during beacons.
> Driver used to transmit identical beacons that were downloaded to
> hardware during the initiation phase. This beacon will become
> obsolete over time.
>
> If the beacon does not contain sufficient information, station would
> not be able to percept that there is data to receive. Hence it won't
> wake up and start the PS-poll procedure, this could lead to timeout
> and/or lost data segments. In order to resolve this issue, driver will
> now download beacon to hardware whenever the content is updated.
>
> Enable hardware to update dtim_count for more efficiency, this reduces
> the overhead of downloading beacon at every beacon interval since most
> of the time only the dtim_count needs to be updated.
>
> Change queue mapping for broadcast/multicast frames to high queue, so
> these frames can be prioritized and sent when dtim_count is zero.
>
> Signed-off-by: Po-Hao Huang <[email protected]>
> Signed-off-by: Ping-Ke Shih <[email protected]>
> ---
>
> [...]
>
> diff --git a/drivers/net/wireless/realtek/rtw88/mac80211.c b/drivers/net/wireless/realtek/rtw88/mac80211.c
> index c9341af493645..1ee41dfda5e1b 100644
> --- a/drivers/net/wireless/realtek/rtw88/mac80211.c
> +++ b/drivers/net/wireless/realtek/rtw88/mac80211.c
> @@ -402,8 +402,10 @@ static void rtw_ops_bss_info_changed(struct ieee80211_hw *hw,
> coex_stat->wl_beacon_interval = conf->beacon_int;
> }
>
> - if (changed & BSS_CHANGED_BEACON)
> + if (changed & BSS_CHANGED_BEACON) {
> + rtw_set_dtim_period(rtwdev, conf->dtim_period);
> rtw_fw_download_rsvd_page(rtwdev);
> + }
>
> if (changed & BSS_CHANGED_BEACON_ENABLED) {
> if (conf->enable_beacon)
> @@ -474,6 +476,18 @@ static int rtw_ops_sta_remove(struct ieee80211_hw *hw,
> return 0;
> }
>
> +static int rtw_ops_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
> + bool set)
> +{
> + struct rtw_dev *rtwdev = hw->priv;
> +
> + mutex_lock(&rtwdev->mutex);
> + rtw_fw_download_rsvd_page(rtwdev);
> + mutex_unlock(&rtwdev->mutex);

set_tim is supposed to be atomic. See: https://elixir.bootlin.com/linux/latest/source/include/net/mac80211.h#L3500

This is causing some scheduling in atomic warnings in my kernel:

BUG: scheduling while atomic: swapper/1/0/0x00000700
Modules linked in:
CPU: 1 PID: 0 Comm: swapper/1 Tainted: G W 5.18.0-rc7-00703-g33b5ee09a0c1 #4
Hardware name: Pine64 RK3566 Quartz64-A Board (DT)
Call trace:
dump_backtrace.part.0+0xc4/0xd0
show_stack+0x14/0x60
dump_stack_lvl+0x60/0x78
dump_stack+0x14/0x2c
__schedule_bug+0x5c/0x70
__schedule+0x5c4/0x630
schedule+0x44/0xb0
schedule_preempt_disabled+0xc/0x14
__mutex_lock.constprop.0+0x538/0x56c
__mutex_lock_slowpath+0x10/0x20
mutex_lock+0x54/0x60
rtw_ops_set_tim+0x20/0x40
__sta_info_recalc_tim+0x150/0x250
sta_info_recalc_tim+0x10/0x20
invoke_tx_handlers_early+0x4e4/0x5c0
ieee80211_tx+0x78/0x110
ieee80211_xmit+0x94/0xc0
__ieee80211_subif_start_xmit+0x818/0xd20
ieee80211_subif_start_xmit+0x44/0x2d0
dev_hard_start_xmit+0xd0/0x150
__dev_queue_xmit+0x250/0xb30
dev_queue_xmit+0x10/0x20
br_dev_queue_push_xmit+0x94/0x174
br_forward_finish+0x90/0xa0
__br_forward+0xc0/0x13c
br_forward+0x108/0x134
br_dev_xmit+0x1cc/0x3a4
dev_hard_start_xmit+0xd0/0x150
__dev_queue_xmit+0x250/0xb30
dev_queue_xmit+0x10/0x20
arp_xmit+0x6c/0x7c
arp_send_dst+0x8c/0xc0
arp_solicit+0xd4/0x1e0
neigh_probe+0x58/0xa0
neigh_timer_handler+0x27c/0x380
call_timer_fn.constprop.0+0x20/0x80
__run_timers.part.0+0x230/0x280
run_timer_softirq+0x38/0x70
_stext+0x104/0x278
__irq_exit_rcu+0xa4/0xdc
irq_exit_rcu+0xc/0x14
el1_interrupt+0x34/0x50
el1h_64_irq_handler+0x14/0x20
el1h_64_irq+0x64/0x68
arch_cpu_idle+0x14/0x20
do_idle+0x208/0x290
cpu_startup_entry+0x20/0x30
secondary_start_kernel+0x130/0x144
__secondary_switched+0x54/0x58

kind regards,
o.

> + return 0;
> +}
> +
> static int rtw_ops_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> struct ieee80211_vif *vif, struct ieee80211_sta *sta,
> struct ieee80211_key_conf *key)
> @@ -875,6 +889,7 @@ const struct ieee80211_ops rtw_ops = {
> .conf_tx = rtw_ops_conf_tx,
> .sta_add = rtw_ops_sta_add,
> .sta_remove = rtw_ops_sta_remove,
> + .set_tim = rtw_ops_set_tim,
> .set_key = rtw_ops_set_key,
> .ampdu_action = rtw_ops_ampdu_action,
> .can_aggregate_in_amsdu = rtw_ops_can_aggregate_in_amsdu,

2022-05-26 01:29:45

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH 3/6] rtw88: Add update beacon flow for AP mode


> -----Original Message-----
> From: Ond?ej Jirman <[email protected]>
> Sent: Thursday, May 26, 2022 7:01 AM
> To: Ping-Ke Shih <[email protected]>
> Cc: [email protected]; [email protected]; [email protected]; Bernie Huang
> <[email protected]>
> Subject: Re: [PATCH 3/6] rtw88: Add update beacon flow for AP mode
>
> Hello Ping-Ke,
>
> please see bugreport below:
>
> >
> > +static int rtw_ops_set_tim(struct ieee80211_hw *hw, struct ieee80211_sta *sta,
> > + bool set)
> > +{
> > + struct rtw_dev *rtwdev = hw->priv;
> > +
> > + mutex_lock(&rtwdev->mutex);
> > + rtw_fw_download_rsvd_page(rtwdev);
> > + mutex_unlock(&rtwdev->mutex);
>
> set_tim is supposed to be atomic. See:
> https://elixir.bootlin.com/linux/latest/source/include/net/mac80211.h#L3500
>

Thanks for pointing out.
We will fix it.

Ping-Ke