| Message ID | 20231102100302.22160-4-shayne.chen@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/8] wifi: mt76: change txpower init to per-phy | expand |
On 11/2/23 03:02, Shayne Chen wrote: > From: Benjamin Lin <benjamin-jw.lin@mediatek.com> > > During runtime, the GI value in the WTBL is not updated in real-time. To > obtain the latest results for the TX GI, switch to use an MCU command. Hello, I do not see this callback happening on my system. What firmware version is needed for this to work? And where to find it... Thanks, Ben
On Mon, 2023-12-04 at 13:57 -0800, Ben Greear wrote: > > External email : Please do not click links or open attachments until > you have verified the sender or the content. > On 11/2/23 03:02, Shayne Chen wrote: > > From: Benjamin Lin <benjamin-jw.lin@mediatek.com> > > > > During runtime, the GI value in the WTBL is not updated in real- > time. To > > obtain the latest results for the TX GI, switch to use an MCU > command. > > Hello, Hi Ben, > > I do not see this callback happening on my system. What firmware > version > is needed for this to work? > > And where to find it... > Please get testing firmware files from the following link to see if it works on your environment: https://github.com/csyuanc/linux-firmware Thanks, Shayne > Thanks, > Ben >
On 12/7/23 09:07, shayne.chen@mediatek.com wrote: > On Mon, 2023-12-04 at 13:57 -0800, Ben Greear wrote: >> >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> On 11/2/23 03:02, Shayne Chen wrote: >>> From: Benjamin Lin <benjamin-jw.lin@mediatek.com> >>> >>> During runtime, the GI value in the WTBL is not updated in real- >> time. To >>> obtain the latest results for the TX GI, switch to use an MCU >> command. >> >> Hello, > > Hi Ben, >> >> I do not see this callback happening on my system. What firmware >> version >> is needed for this to work? >> >> And where to find it... >> > > Please get testing firmware files from the following link to see if it > works on your environment: > https://github.com/csyuanc/linux-firmware I do see the callback happening with that latest firmware, thanks for the link to that. tx_gi is always reported at zero though, which seems unlikely to be correct. [ 625.875443] mt7996e 0000:0d:00.0: ERROR: MCU: Sequence mismatch in response, seq: 13 rxd->seq: 12 cmd: 130022 [ 625.988751] update-tx-gi, mode: 8 tx_gi: 0 [ 625.988755] update-tx-gi, mode: 0 tx_gi: 0 [ 626.091378] mt7996e 0000:0d:00.0: ERROR: MCU: Sequence mismatch in response, seq: 1 rxd->seq: 15 cmd: 130022 [ 626.204917] update-tx-gi, mode: 8 tx_gi: 0 [ 626.204920] update-tx-gi, mode: 0 tx_gi: 0 [ 626.307376] mt7996e 0000:0d:00.0: ERROR: MCU: Sequence mismatch in response, seq: 4 rxd->seq: 3 cmd: 130022 [ 626.421332] update-tx-gi, mode: 8 tx_gi: 0 [ 626.421335] update-tx-gi, mode: 0 tx_gi: 0 [ 626.523436] mt7996e 0000:0d:00.0: ERROR: MCU: Sequence mismatch in response, seq: 7 rxd->seq: 6 cmd: 130022 [ 626.636900] update-tx-gi, mode: 8 tx_gi: 0 [ 626.636904] update-tx-gi, mode: 0 tx_gi: 0 [ 626.739408] mt7996e 0000:0d:00.0: ERROR: MCU: Sequence mismatch in response, seq: 10 rxd->seq: 9 cmd: 130022 [ 626.852727] update-tx-gi, mode: 8 tx_gi: 0 [ 626.852731] update-tx-gi, mode: 0 tx_gi: 0 [ 626.955475] mt7996e 0000:0d:00.0: ERROR: MCU: Sequence mismatch in response, seq: 13 rxd->seq: 12 cmd: 130022 diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c index 407894c13d91..8aad38a21cd4 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c @@ -534,6 +534,9 @@ mt7996_mcu_ie_countdown(struct mt7996_dev *dev, struct sk_buff *skb) static int mt7996_mcu_update_tx_gi(struct rate_info *rate, struct all_sta_trx_rate *mcu_rate) { + pr_info("update-tx-gi, mode: %d tx_gi: %d\n", + mcu_rate->tx_mode, mcu_rate->tx_gi); + switch (mcu_rate->tx_mode) { case MT_PHY_TYPE_CCK: case MT_PHY_TYPE_OFDM: [greearb@ben-dt5 mediatek]$ And lots of seq mismatch warnings in this firmware...could that be because of the new guard-interval callback perhaps? Do the messages sent from FW increment the seq number? The parse-response code appears to assume that it's responses are the only thing that would increment the seq number... Here's my code to debug the seq mismatch: static int mt7996_mcu_parse_response(struct mt76_dev *mdev, int cmd, struct sk_buff *skb, int seq) { struct mt7996_mcu_rxd *rxd; struct mt7996_mcu_uni_event *event; int mcu_cmd = FIELD_GET(__MCU_CMD_FIELD_ID, cmd); int ret = 0; if (!skb) { const char* first = "Secondary"; mdev->mcu_timeouts++; if (!mdev->first_failed_mcu_cmd) first = "Initial"; dev_err(mdev->dev, "MCU: %s Failure: Message %08x (cid %lx ext_cid: %lx seq %d) timeout (%d/%d). Last successful cmd: 0x%x\n", first, cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd), FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq, mdev->mcu_timeouts, MAX_MCU_TIMEOUTS, mdev->last_successful_mcu_cmd); if (!mdev->first_failed_mcu_cmd) mdev->first_failed_mcu_cmd = cmd; return -ETIMEDOUT; } mdev->mcu_timeouts = 0; mdev->last_successful_mcu_cmd = cmd; if (mdev->first_failed_mcu_cmd) { dev_err(mdev->dev, "MCU: First success after failure: Message %08x (cid %lx ext_cid: %lx seq %d)\n", cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd), FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq); mdev->first_failed_mcu_cmd = 0; } else { /* verbose debugging dev_err(mdev->dev, "MCU: OK response to message %08x (cid %lx ext_cid: %lx seq %d)\n", cmd, FIELD_GET(__MCU_CMD_FIELD_ID, cmd), FIELD_GET(__MCU_CMD_FIELD_EXT_ID, cmd), seq); */ } rxd = (struct mt7996_mcu_rxd *)skb->data; if (seq != rxd->seq) { dev_err(mdev->dev, "ERROR: MCU: Sequence mismatch in response, seq: %d rxd->seq: %d cmd: %0x\n", seq, rxd->seq, cmd); return -EAGAIN; } if (cmd == MCU_CMD(PATCH_SEM_CONTROL)) { skb_pull(skb, sizeof(*rxd) - 4); ret = *skb->data; } else if ((rxd->option & MCU_UNI_CMD_EVENT) && rxd->eid == MCU_UNI_EVENT_RESULT) { skb_pull(skb, sizeof(*rxd)); event = (struct mt7996_mcu_uni_event *)skb->data; ret = le32_to_cpu(event->status); /* skip invalid event */ if (mcu_cmd != event->cid) ret = -EAGAIN; } else { skb_pull(skb, sizeof(struct mt7996_mcu_rxd)); } return ret; } Thanks, Ben
diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h index 65844de6dccd..0185804d8ce3 100644 --- a/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h +++ b/drivers/net/wireless/mediatek/mt76/mt76_connac_mcu.h @@ -1328,7 +1328,7 @@ enum { }; enum UNI_ALL_STA_INFO_TAG { - UNI_ALL_STA_TX_RATE, + UNI_ALL_STA_TXRX_RATE, UNI_ALL_STA_TX_STAT, UNI_ALL_STA_TXRX_ADM_STAT, UNI_ALL_STA_TXRX_AIR_TIME, diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c index 56dfbeb51504..9db610e2645f 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c @@ -102,7 +102,6 @@ static void mt7996_mac_sta_poll(struct mt7996_dev *dev) }; struct ieee80211_sta *sta; struct mt7996_sta *msta; - struct rate_info *rate; u32 tx_time[IEEE80211_NUM_ACS], rx_time[IEEE80211_NUM_ACS]; LIST_HEAD(sta_poll_list); int i; @@ -118,7 +117,6 @@ static void mt7996_mac_sta_poll(struct mt7996_dev *dev) u32 addr, val; u16 idx; s8 rssi[4]; - u8 bw; spin_lock_bh(&dev->mt76.sta_poll_lock); if (list_empty(&sta_poll_list)) { @@ -174,49 +172,6 @@ static void mt7996_mac_sta_poll(struct mt7996_dev *dev) ieee80211_sta_register_airtime(sta, tid, tx_cur, rx_cur); } - /* We don't support reading GI info from txs packets. - * For accurate tx status reporting and AQL improvement, - * we need to make sure that flags match so polling GI - * from per-sta counters directly. - */ - rate = &msta->wcid.rate; - - switch (rate->bw) { - case RATE_INFO_BW_320: - bw = IEEE80211_STA_RX_BW_320; - break; - case RATE_INFO_BW_160: - bw = IEEE80211_STA_RX_BW_160; - break; - case RATE_INFO_BW_80: - bw = IEEE80211_STA_RX_BW_80; - break; - case RATE_INFO_BW_40: - bw = IEEE80211_STA_RX_BW_40; - break; - default: - bw = IEEE80211_STA_RX_BW_20; - break; - } - - addr = mt7996_mac_wtbl_lmac_addr(dev, idx, 6); - val = mt76_rr(dev, addr); - if (rate->flags & RATE_INFO_FLAGS_EHT_MCS) { - addr = mt7996_mac_wtbl_lmac_addr(dev, idx, 5); - val = mt76_rr(dev, addr); - rate->eht_gi = FIELD_GET(GENMASK(25, 24), val); - } else if (rate->flags & RATE_INFO_FLAGS_HE_MCS) { - u8 offs = 24 + 2 * bw; - - rate->he_gi = (val & (0x3 << offs)) >> offs; - } else if (rate->flags & - (RATE_INFO_FLAGS_VHT_MCS | RATE_INFO_FLAGS_MCS)) { - if (val & BIT(12 + bw)) - rate->flags |= RATE_INFO_FLAGS_SHORT_GI; - else - rate->flags &= ~RATE_INFO_FLAGS_SHORT_GI; - } - /* get signal strength of resp frames (CTS/BA/ACK) */ addr = mt7996_mac_wtbl_lmac_addr(dev, idx, 34); val = mt76_rr(dev, addr); @@ -1298,6 +1253,8 @@ mt7996_mac_add_txs_skb(struct mt7996_dev *dev, struct mt76_wcid *wcid, goto out; rate.flags = RATE_INFO_FLAGS_VHT_MCS; + if (wcid->rate.flags & RATE_INFO_FLAGS_SHORT_GI) + rate.flags |= RATE_INFO_FLAGS_SHORT_GI; break; case MT_PHY_TYPE_HE_SU: case MT_PHY_TYPE_HE_EXT_SU: @@ -2312,6 +2269,7 @@ void mt7996_mac_work(struct work_struct *work) mt7996_mac_update_stats(phy); + mt7996_mcu_get_all_sta_info(phy, UNI_ALL_STA_TXRX_RATE); if (mtk_wed_device_active(&phy->dev->mt76.mmio.wed)) { mt7996_mcu_get_all_sta_info(phy, UNI_ALL_STA_TXRX_ADM_STAT); mt7996_mcu_get_all_sta_info(phy, UNI_ALL_STA_TXRX_MSDU_COUNT); diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/main.c b/drivers/net/wireless/mediatek/mt76/mt7996/main.c index 7336eaa7b9ae..d9ba57ae9fdc 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/main.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/main.c @@ -998,6 +998,7 @@ static void mt7996_sta_statistics(struct ieee80211_hw *hw, sinfo->txrate.he_gi = txrate->he_gi; sinfo->txrate.he_dcm = txrate->he_dcm; sinfo->txrate.he_ru_alloc = txrate->he_ru_alloc; + sinfo->txrate.eht_gi = txrate->eht_gi; } sinfo->txrate.flags = txrate->flags; sinfo->filled |= BIT_ULL(NL80211_STA_INFO_TX_BITRATE); diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c index 8141c24ade50..b8d0b52be1e7 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c @@ -449,6 +449,43 @@ mt7996_mcu_ie_countdown(struct mt7996_dev *dev, struct sk_buff *skb) } } +static int +mt7996_mcu_update_tx_gi(struct rate_info *rate, struct all_sta_trx_rate *mcu_rate) +{ + switch (mcu_rate->tx_mode) { + case MT_PHY_TYPE_CCK: + case MT_PHY_TYPE_OFDM: + break; + case MT_PHY_TYPE_HT: + case MT_PHY_TYPE_HT_GF: + case MT_PHY_TYPE_VHT: + if (mcu_rate->tx_gi) + rate->flags |= RATE_INFO_FLAGS_SHORT_GI; + else + rate->flags &= ~RATE_INFO_FLAGS_SHORT_GI; + break; + case MT_PHY_TYPE_HE_SU: + case MT_PHY_TYPE_HE_EXT_SU: + case MT_PHY_TYPE_HE_TB: + case MT_PHY_TYPE_HE_MU: + if (mcu_rate->tx_gi > NL80211_RATE_INFO_HE_GI_3_2) + return -EINVAL; + rate->he_gi = mcu_rate->tx_gi; + break; + case MT_PHY_TYPE_EHT_SU: + case MT_PHY_TYPE_EHT_TRIG: + case MT_PHY_TYPE_EHT_MU: + if (mcu_rate->tx_gi > NL80211_RATE_INFO_EHT_GI_3_2) + return -EINVAL; + rate->eht_gi = mcu_rate->tx_gi; + break; + default: + return -EINVAL; + } + + return 0; +} + static void mt7996_mcu_rx_all_sta_info_event(struct mt7996_dev *dev, struct sk_buff *skb) { @@ -465,6 +502,16 @@ mt7996_mcu_rx_all_sta_info_event(struct mt7996_dev *dev, struct sk_buff *skb) struct mt76_wcid *wcid; switch (le16_to_cpu(res->tag)) { + case UNI_ALL_STA_TXRX_RATE: + wlan_idx = le16_to_cpu(res->rate[i].wlan_idx); + wcid = rcu_dereference(dev->mt76.wcid[wlan_idx]); + + if (!wcid) + break; + + if (mt7996_mcu_update_tx_gi(&wcid->rate, &res->rate[i])) + dev_err(dev->mt76.dev, "Failed to update TX GI\n"); + break; case UNI_ALL_STA_TXRX_ADM_STAT: wlan_idx = le16_to_cpu(res->adm_stat[i].wlan_idx); wcid = rcu_dereference(dev->mt76.wcid[wlan_idx]); diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.h b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.h index 1562c8a6a821..328edc354165 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.h +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.h @@ -175,6 +175,27 @@ struct mt7996_mcu_mib { __le64 data; } __packed; +struct all_sta_trx_rate { + __le16 wlan_idx; + u8 __rsv1[2]; + u8 tx_mode; + u8 flags; + u8 tx_stbc; + u8 tx_gi; + u8 tx_bw; + u8 tx_ldpc; + u8 tx_mcs; + u8 tx_nss; + u8 rx_rate; + u8 rx_mode; + u8 rx_nsts; + u8 rx_gi; + u8 rx_coding; + u8 rx_stbc; + u8 rx_bw; + u8 __rsv2; +} __packed; + struct mt7996_mcu_all_sta_info_event { u8 rsv[4]; __le16 tag; @@ -185,6 +206,7 @@ struct mt7996_mcu_all_sta_info_event { u8 rsv3[2]; union { + struct all_sta_trx_rate rate[0]; struct { __le16 wlan_idx; u8 rsv[2];