| Message ID | 20250103075107.1337533-1-anarsoul@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Ping-Ke Shih |
| Headers | show |
| Series | wifi: rtw88: 8703b: Fix RX/TX issues | expand |
Vasily Khoruzhick <anarsoul@gmail.com> wrote: > Fix 3 typos in 8703b driver. 2 typos in calibration routines are not > fatal and do not seem to have any impact, just fix them to match vendor > driver. Just curious how you can find these typos? > > However the last one in rtw8703b_set_channel_bb() clears too many bits > in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes > above MCS0-MCS1). Vendor driver clears only 2 most significant bits. > > With the last typo fixed, the driver is able to reach MCS7 on Pinebook > > Cc: stable@vger.kernel.org > Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> Acked-by: Ping-Ke Shih <pkshih@realtek.com> Is this urgent? If not, I will take this via rtw-next tree.
Am 03.01.25 um 11:12 schrieb Ping-Ke Shih: > Vasily Khoruzhick <anarsoul@gmail.com> wrote: >> Fix 3 typos in 8703b driver. 2 typos in calibration routines are not >> fatal and do not seem to have any impact, just fix them to match vendor >> driver. > > Just curious how you can find these typos? > >> >> However the last one in rtw8703b_set_channel_bb() clears too many bits >> in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes >> above MCS0-MCS1). Vendor driver clears only 2 most significant bits. >> >> With the last typo fixed, the driver is able to reach MCS7 on Pinebook >> >> Cc: stable@vger.kernel.org >> Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") >> Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > Acked-by: Ping-Ke Shih <pkshih@realtek.com> > > Is this urgent? If not, I will take this via rtw-next tree. It's a huge performance improvement, 10x more RX throughput in Iperf tests, not just in the reported bit rate (residential, moderately noisy environment). TX improved too, but not as massively. Tested-by: Fiona Klute <fiona.klute@gmx.de> Thank you very much Vasily for digging into this! Best regards, Fiona
On Fri, Jan 3, 2025 at 1:13 AM Ping-Ke Shih <pkshih@realtek.com> wrote: > > Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > Fix 3 typos in 8703b driver. 2 typos in calibration routines are not > > fatal and do not seem to have any impact, just fix them to match vendor > > driver. > > Just curious how you can find these typos? I added traces to sdio_* functions in linux (see [1]), so I can capture register access traces. I captured the traces from both rtw88 and the vendor driver and wrote a simple parser that decodes the traces, see [2]. I guess it would be easier with an USB device, where we have usbmon. I really wish there was something like usbmon for SDIO. I also added traces for C2H messages to both drivers, since they go through sdio_memcpy_fromio() that I don't trace. Once I had the traces, I manually compared them (along with register dumps, rtw88 has it in debugfs, vendor driver in proc) trying to find the writes that do not match. Unfortunately, rtw88 and vendor driver flows are different enough, so I couldn't come up with a way to compare it automatically Adrian and Bitterblue supported me on #linux-wireless on IRC, and one of the typos in IQK calibration was actually found by Bitterblue. It took ~5 evenings and 1 weekend to get to REG_OFDM0_TX_PSD_NOISE (0xce4). Once I changed it from 0 to 0x10000000 via reg_write over debugfs, it magically fixed the issue. I changed it back to 0 to confirm that it breaks it again, and then back to 0x10000000 to see it working. Then it was just a matter of grep to find where this register is written in rtw88 and compare the corresponding code to the vendor driver. [1] https://github.com/anarsoul/rtl8723cs-re/blob/master/sdio_traces.patch [2] https://github.com/anarsoul/rtl8723cs-re > > However the last one in rtw8703b_set_channel_bb() clears too many bits > > in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes > > above MCS0-MCS1). Vendor driver clears only 2 most significant bits. > > > > With the last typo fixed, the driver is able to reach MCS7 on Pinebook > > > > Cc: stable@vger.kernel.org > > Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > Acked-by: Ping-Ke Shih <pkshih@realtek.com> > > Is this urgent? If not, I will take this via rtw-next tree. Not really, since there aren't not a lot of users of 8723cs, but it makes rtw88_8723cs driver usable on rtl8723cs. I don't really have any preference on what tree it goes in Regards, Vasily >
On 25-01-02 23:50, Vasily Khoruzhick wrote: > Fix 3 typos in 8703b driver. 2 typos in calibration routines are not > fatal and do not seem to have any impact, just fix them to match vendor > driver. > > However the last one in rtw8703b_set_channel_bb() clears too many bits > in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes > above MCS0-MCS1). Vendor driver clears only 2 most significant bits. > > With the last typo fixed, the driver is able to reach MCS7 on Pinebook > > Cc: stable@vger.kernel.org > Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> Tested-by: Andrey Skvortsov <andrej.skvortzov@gmail.com> Thank you, Vasily, for fixing that. Performance is much better with the fix. Here are iperf results made on PinePhone: 1. without the patch using rtw88 driver 1.98 Mbits/sec 2. with the patch using rtw88 driver 14.0 Mbits/sec 3. using old vendor 8723cs driver 23.6 Mbits/sec
On Fri, Jan 3, 2025 at 11:02 AM Andrey Skvortsov <andrej.skvortzov@gmail.com> wrote: > > On 25-01-02 23:50, Vasily Khoruzhick wrote: > > Fix 3 typos in 8703b driver. 2 typos in calibration routines are not > > fatal and do not seem to have any impact, just fix them to match vendor > > driver. > > > > However the last one in rtw8703b_set_channel_bb() clears too many bits > > in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes > > above MCS0-MCS1). Vendor driver clears only 2 most significant bits. > > > > With the last typo fixed, the driver is able to reach MCS7 on Pinebook > > > > Cc: stable@vger.kernel.org > > Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > Tested-by: Andrey Skvortsov <andrej.skvortzov@gmail.com> > > Thank you, Vasily, for fixing that. Performance is much better with > the fix. Here are iperf results made on PinePhone: Thanks for testing! > 1. without the patch using rtw88 driver > 1.98 Mbits/sec > > 2. with the patch using rtw88 driver > 14.0 Mbits/sec > > 3. using old vendor 8723cs driver > 23.6 Mbits/sec Interesting, I get 30-50 Mbit/s on both rtw88 and vendor driver on Pinebook for either TX or RX, however I am pretty close (a few meters) to the AP and it isn't a noisy env (Pinebook can pick up just 4 other 2.4G APs besides mine, none on the same channel). Could you try disabling bluetooth (run bluetoothctl, and do "power off") and re-run the test? I noticed that btcoex is implemented differently in rtw88 and in the vendor driver, so it might not be working correctly. > > -- > Best regards, > Andrey Skvortsov
On Fri, Jan 3, 2025 at 3:55 AM Fiona Klute <fiona.klute@gmx.de> wrote: > > Is this urgent? If not, I will take this via rtw-next tree. > It's a huge performance improvement, 10x more RX throughput in Iperf > tests, not just in the reported bit rate (residential, moderately noisy > environment). TX improved too, but not as massively. > > Tested-by: Fiona Klute <fiona.klute@gmx.de> Thanks for testing! Can you please share your iperf numbers? > Thank you very much Vasily for digging into this! Thanks for implementing 8723cs support in the first place! :) > Best regards, > Fiona >
On 25-01-03 11:10, Vasily Khoruzhick wrote: > On Fri, Jan 3, 2025 at 11:02 AM Andrey Skvortsov > <andrej.skvortzov@gmail.com> wrote: > > > > On 25-01-02 23:50, Vasily Khoruzhick wrote: > > > Fix 3 typos in 8703b driver. 2 typos in calibration routines are not > > > fatal and do not seem to have any impact, just fix them to match vendor > > > driver. > > > > > > However the last one in rtw8703b_set_channel_bb() clears too many bits > > > in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes > > > above MCS0-MCS1). Vendor driver clears only 2 most significant bits. > > > > > > With the last typo fixed, the driver is able to reach MCS7 on Pinebook > > > > > > Cc: stable@vger.kernel.org > > > Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > > > Tested-by: Andrey Skvortsov <andrej.skvortzov@gmail.com> > > > > Thank you, Vasily, for fixing that. Performance is much better with > > the fix. Here are iperf results made on PinePhone: > > Thanks for testing! > > > 1. without the patch using rtw88 driver > > 1.98 Mbits/sec > > > > 2. with the patch using rtw88 driver > > 14.0 Mbits/sec > > > > 3. using old vendor 8723cs driver > > 23.6 Mbits/sec > > Interesting, I get 30-50 Mbit/s on both rtw88 and vendor driver on > Pinebook for either TX or RX, however I am pretty close (a few meters) > to the AP and it isn't a noisy env (Pinebook can pick up just 4 other > 2.4G APs besides mine, none on the same channel). Could you try > disabling bluetooth (run bluetoothctl, and do "power off") and re-run > the test? yes, previous test were done with Bluetooth enabled. With disabled (power off) Bluetooth performance is slightly better. Here is how I tested to make sure, that we are on the same page: 1. on device 'iperf -s' 2. on PC 'iperf -c <device> -e -i 10 -t 250' Here are more detailed testing results: 1. without the patch using rtw88 driver [ 1] (icwnd/mss/irtt=14/1448/66504) (ct=66.55 ms) [ ID] Interval Transfer Bandwidth Write/Err Rtry InF(pkts)/Cwnd(pkts)/RTT(var) NetPwr [ 1] 0.0000-10.0000 sec 2.38 MBytes 2.00 Mbits/sec 20/0 73 22K(16)/24K(17)/56167(15843) us 4.448847 [ 1] 10.0000-20.0000 sec 5.88 MBytes 4.93 Mbits/sec 47/0 38 178K(126)/178K(126)/462265(40709) us 1.332652 [ 1] 20.0000-30.0000 sec 2.76 MBytes 2.31 Mbits/sec 25/0 182 12K(9)/12K(9)/32519(11137) us 8.892008 [ 1] 30.0000-40.0000 sec 2.80 MBytes 2.35 Mbits/sec 23/0 48 45K(32)/45K(32)/186889(21282) us 1.569355 [ 1] 40.0000-50.0000 sec 6.31 MBytes 5.29 Mbits/sec 51/0 24 188K(133)/188K(133)/574842(11821) us 1.151013 [ 1] 50.0000-60.0000 sec 5.06 MBytes 4.25 Mbits/sec 43/0 8 346K(245)/346K(245)/412227(75277) us 1.287768 [ 1] 60.0000-70.0000 sec 4.29 MBytes 3.60 Mbits/sec 38/0 81 284K(201)/284K(201)/339931(17905) us 1.323076 2. with the patch using rtw88 driver [ 1] (icwnd/mss/irtt=14/1448/40241) (ct=40.27 ms) [ ID] Interval Transfer Bandwidth Write/Err Rtry InF(pkts)/Cwnd(pkts)/RTT(var) NetPwr [ 1] 0.0000-10.0000 sec 22.0 MBytes 18.5 Mbits/sec 176/0 0 1069K(756)/1090K(771)/466471(10659) us 4.945374 [ 1] 10.0000-20.0000 sec 20.5 MBytes 17.2 Mbits/sec 164/0 0 2075K(1468)/3919K(2772)/1453362(10765) us 1.479040 [ 1] 20.0000-30.0000 sec 16.7 MBytes 14.0 Mbits/sec 137/0 140 2099K(1485)/2589K(1831)/1466588(44692) us 1.193315 [ 1] 30.0000-40.0000 sec 16.8 MBytes 14.1 Mbits/sec 136/1 169 446K(316)/1914K(1354)/1511938(187) us 1.166601 [ 1] 40.0000-50.0000 sec 14.8 MBytes 12.4 Mbits/sec 121/1 6 1278K(904)/1278K(904)/4145469(8168) us 0.374480 [ 1] 50.0000-60.0000 sec 22.0 MBytes 18.5 Mbits/sec 177/1 26 905K(640)/1084K(767)/428675(700) us 5.389748 [ 1] 60.0000-70.0000 sec 18.1 MBytes 15.2 Mbits/sec 146/0 247 229K(162)/254K(180)/129034(4979) us 14.74 3. using old vendor 8723cs driver [ 1] (icwnd/mss/irtt=14/1448/91025) (ct=91.07 ms) [ ID] Interval Transfer Bandwidth Write/Err Rtry InF(pkts)/Cwnd(pkts)/RTT(var) NetPwr [ 1] 0.0000-10.0000 sec 38.3 MBytes 32.1 Mbits/sec 306/0 0 1938K(1371)/1949K(1379)/419011(4587) us 9.572087 [ 1] 10.0000-20.0000 sec 36.8 MBytes 30.8 Mbits/sec 294/0 0 2258K(1597)/3809K(2694)/856058(270) us 4.501467 [ 1] 20.0000-30.0000 sec 38.4 MBytes 32.2 Mbits/sec 307/0 0 2360K(1669)/3809K(2694)/727066(112) us 5.534450 [ 1] 30.0000-40.0000 sec 38.5 MBytes 32.3 Mbits/sec 308/0 0 2371K(1677)/3809K(2694)/786411(100) us 5.133470 [ 1] 40.0000-50.0000 sec 40.8 MBytes 34.2 Mbits/sec 326/0 0 2563K(1813)/3809K(2694)/652361(18130) us 6.549973 [ 1] 50.0000-60.0000 sec 35.4 MBytes 29.7 Mbits/sec 283/0 0 2524K(1785)/5953K(4210)/670626(151) us 5.531157 [ 1] 60.0000-70.0000 sec 39.5 MBytes 33.1 Mbits/sec 316/0 0 2502K(1770)/5953K(4210)/707670(140) us 5.852834 > I noticed that btcoex is implemented differently in rtw88 > and in the vendor driver, so it might not be working correctly. yeah, I couldn't make good-enough VoIP-calls on WiFi using PinePhone with rtw88, but I could make it just fine with old vendor driver. But I've switched to rtw88 anyway.
On Fri, Jan 3, 2025 at 12:19 PM Andrey Skvortsov
<andrej.skvortzov@gmail.com> wrote:
> Here are more detailed testing results:
I was able to reproduce it with an AP located 2 floors away.
Basically, in perfect conditions rtw88 is able to match vendor driver
performance, however when signal strength is low, rtw88 is ~2x slower
On 25-01-03 12:53, Vasily Khoruzhick wrote: > On Fri, Jan 3, 2025 at 12:19 PM Andrey Skvortsov > <andrej.skvortzov@gmail.com> wrote: > > > Here are more detailed testing results: > > I was able to reproduce it with an AP located 2 floors away. > Basically, in perfect conditions rtw88 is able to match vendor driver > performance, however when signal strength is low, rtw88 is ~2x slower In my case AP was one meter away from the device. Maybe PinePhone's antenna isn't well designed as for PineBook.
Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > On Fri, Jan 3, 2025 at 1:13 AM Ping-Ke Shih <pkshih@realtek.com> wrote: > > > > Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > > Fix 3 typos in 8703b driver. 2 typos in calibration routines are not > > > fatal and do not seem to have any impact, just fix them to match vendor > > > driver. > > > > Just curious how you can find these typos? > > I added traces to sdio_* functions in linux (see [1]), so I can > capture register access traces. I captured the traces from both rtw88 > and the vendor driver and wrote a simple parser that decodes the > traces, see [2]. I guess it would be easier with an USB device, where > we have usbmon. I really wish there was something like usbmon for > SDIO. > > I also added traces for C2H messages to both drivers, since they go > through sdio_memcpy_fromio() that I don't trace. > > Once I had the traces, I manually compared them (along with register > dumps, rtw88 has it in debugfs, vendor driver in proc) trying to find > the writes that do not match. Unfortunately, rtw88 and vendor driver > flows are different enough, so I couldn't come up with a way to > compare it automatically > > Adrian and Bitterblue supported me on #linux-wireless on IRC, and one > of the typos in IQK calibration was actually found by Bitterblue. > > It took ~5 evenings and 1 weekend to get to REG_OFDM0_TX_PSD_NOISE > (0xce4). Once I changed it from 0 to 0x10000000 via reg_write over > debugfs, it magically fixed the issue. I changed it back to 0 to > confirm that it breaks it again, and then back to 0x10000000 to see it > working. Then it was just a matter of grep to find where this register > is written in rtw88 and compare the corresponding code to the vendor > driver. > > [1] https://github.com/anarsoul/rtl8723cs-re/blob/master/sdio_traces.patch > [2] https://github.com/anarsoul/rtl8723cs-re > Cool. I did similar works when we rewrite rtw89 from vendor drivers. Sometimes we also found flaws of vendor driver by their difference. Ping-Ke
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8703b.c b/drivers/net/wireless/realtek/rtw88/rtw8703b.c index a19b94d022ee..1d232adbdd7e 100644 --- a/drivers/net/wireless/realtek/rtw88/rtw8703b.c +++ b/drivers/net/wireless/realtek/rtw88/rtw8703b.c @@ -903,7 +903,7 @@ static void rtw8703b_set_channel_bb(struct rtw_dev *rtwdev, u8 channel, u8 bw, rtw_write32_mask(rtwdev, REG_FPGA0_RFMOD, BIT_MASK_RFMOD, 0x0); rtw_write32_mask(rtwdev, REG_FPGA1_RFMOD, BIT_MASK_RFMOD, 0x0); rtw_write32_mask(rtwdev, REG_OFDM0_TX_PSD_NOISE, - GENMASK(31, 20), 0x0); + GENMASK(31, 30), 0x0); rtw_write32(rtwdev, REG_BBRX_DFIR, 0x4A880000); rtw_write32(rtwdev, REG_OFDM0_A_TX_AFE, 0x19F60000); break; @@ -1198,9 +1198,9 @@ static u8 rtw8703b_iqk_rx_path(struct rtw_dev *rtwdev, rtw_write32(rtwdev, REG_RXIQK_TONE_A_11N, 0x38008c1c); rtw_write32(rtwdev, REG_TX_IQK_TONE_B, 0x38008c1c); rtw_write32(rtwdev, REG_RX_IQK_TONE_B, 0x38008c1c); - rtw_write32(rtwdev, REG_TXIQK_PI_A_11N, 0x8216000f); + rtw_write32(rtwdev, REG_TXIQK_PI_A_11N, 0x8214030f); rtw_write32(rtwdev, REG_RXIQK_PI_A_11N, 0x28110000); - rtw_write32(rtwdev, REG_TXIQK_PI_B, 0x28110000); + rtw_write32(rtwdev, REG_TXIQK_PI_B, 0x82110000); rtw_write32(rtwdev, REG_RXIQK_PI_B, 0x28110000); /* LOK setting */ @@ -1372,7 +1372,7 @@ void rtw8703b_iqk_fill_a_matrix(struct rtw_dev *rtwdev, const s32 result[]) return; tmp_rx_iqi |= FIELD_PREP(BIT_MASK_RXIQ_S1_X, result[IQK_S1_RX_X]); - tmp_rx_iqi |= FIELD_PREP(BIT_MASK_RXIQ_S1_Y1, result[IQK_S1_RX_X]); + tmp_rx_iqi |= FIELD_PREP(BIT_MASK_RXIQ_S1_Y1, result[IQK_S1_RX_Y]); rtw_write32(rtwdev, REG_A_RXIQI, tmp_rx_iqi); rtw_write32_mask(rtwdev, REG_RXIQK_MATRIX_LSB_11N, BIT_MASK_RXIQ_S1_Y2, BIT_SET_RXIQ_S1_Y2(result[IQK_S1_RX_Y]));
Fix 3 typos in 8703b driver. 2 typos in calibration routines are not fatal and do not seem to have any impact, just fix them to match vendor driver. However the last one in rtw8703b_set_channel_bb() clears too many bits in REG_OFDM0_TX_PSD_NOISE, causing TX and RX issues (neither rate goes above MCS0-MCS1). Vendor driver clears only 2 most significant bits. With the last typo fixed, the driver is able to reach MCS7 on Pinebook Cc: stable@vger.kernel.org Fixes: 9bb762b3a957 ("wifi: rtw88: Add definitions for 8703b chip") Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- drivers/net/wireless/realtek/rtw88/rtw8703b.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)