diff mbox series

[v2,1/5] rtw88: 8821c: add cck pd settings

Message ID 20200603094218.19942-2-yhchuang@realtek.com (mailing list archive)
State Accepted
Commit 11fcb119a758e1e03ec77e20b386f4b93ae06601
Delegated to: Kalle Valo
Headers show
Series rtw88: 8821c: the rest patches to support 8821c | expand

Commit Message

Tony Chuang June 3, 2020, 9:42 a.m. UTC 
From: Tzu-En Huang <tehuang@realtek.com>

CCK PD can reduce the number of false alarm of the CCK rates.
It dynamically adjusts the power threshold and CS ratio.
The values are compared to the values of the previous level, if
the level is changed, set new values of power threshold and CS
ratio.

Implement rtw_chip_ops::cck_pd_set() for 8821c.

Signed-off-by: Tzu-En Huang <tehuang@realtek.com>
Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
---
 drivers/net/wireless/realtek/rtw88/main.h     |  1 +
 drivers/net/wireless/realtek/rtw88/rtw8821c.c | 25 +++++++++++++++++++
 drivers/net/wireless/realtek/rtw88/rtw8821c.h |  3 +++
 3 files changed, 29 insertions(+)

Comments

Sebastian Andrzej Siewior June 5, 2020, 10:53 a.m. UTC | #1 
On 2020-06-03 17:42:14 [+0800], yhchuang@realtek.com wrote:
> @@ -596,6 +597,29 @@ static void rtw8821c_phy_calibration(struct rtw_dev *rtwdev)
>  	rtw8821c_do_iqk(rtwdev);
>  }
>  
> +static void rtw8821c_phy_cck_pd_set(struct rtw_dev *rtwdev, u8 new_lvl)
> +{
> +	struct rtw_dm_info *dm_info = &rtwdev->dm_info;
> +	u8 pd[CCK_PD_LV_MAX] = {3, 7, 13, 13, 13};
> +
> +	if (dm_info->min_rssi > 60) {
> +		new_lvl = 4;
> +		pd[4] = 0x1d;
replace 4 with CCK_PD_LV4 ?

> +		goto set_cck_pd;
> +	}

Sebastian
Kalle Valo July 15, 2020, 8:50 a.m. UTC | #2 
<yhchuang@realtek.com> writes:

> From: Tzu-En Huang <tehuang@realtek.com>
>
> CCK PD can reduce the number of false alarm of the CCK rates.
> It dynamically adjusts the power threshold and CS ratio.
> The values are compared to the values of the previous level, if
> the level is changed, set new values of power threshold and CS
> ratio.
>
> Implement rtw_chip_ops::cck_pd_set() for 8821c.
>
> Signed-off-by: Tzu-En Huang <tehuang@realtek.com>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

[...]

> --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
> +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
> @@ -102,6 +102,7 @@ static void rtw8821c_phy_set_param(struct rtw_dev *rtwdev)
>  	rtwdev->chip->ch_param[2] = rtw_read32_mask(rtwdev, REG_TXFILTER, MASKDWORD);
>  
>  	rtw_phy_init(rtwdev);
> +	rtwdev->dm_info.cck_pd_default = rtw_read8(rtwdev, REG_CSRATIO) & 0x1f;
>  }
>  
>  static int rtw8821c_mac_init(struct rtw_dev *rtwdev)
> @@ -596,6 +597,29 @@ static void rtw8821c_phy_calibration(struct rtw_dev *rtwdev)
>  	rtw8821c_do_iqk(rtwdev);
>  }
>  
> +static void rtw8821c_phy_cck_pd_set(struct rtw_dev *rtwdev, u8 new_lvl)
> +{
> +	struct rtw_dm_info *dm_info = &rtwdev->dm_info;
> +	u8 pd[CCK_PD_LV_MAX] = {3, 7, 13, 13, 13};
> +
> +	if (dm_info->min_rssi > 60) {
> +		new_lvl = 4;
> +		pd[4] = 0x1d;
> +		goto set_cck_pd;
> +	}
> +
> +	if (dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] == new_lvl)
> +		return;
> +
> +	dm_info->cck_fa_avg = CCK_FA_AVG_RESET;
> +
> +set_cck_pd:
> +	dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] = new_lvl;
> +	rtw_write32_mask(rtwdev, REG_PWRTH, 0x3f0000, pd[new_lvl]);
> +	rtw_write32_mask(rtwdev, REG_PWRTH2, 0x1f0000,
> +			 dm_info->cck_pd_default + new_lvl * 2);
> +}

I'm starting to see the trend of using magic values in rtw88 on the rise
again. Please be careful with this, the source code should not be full
of undocument values. In some special cases (eg calibration data etc)
using undocumented values is ok, but most of cases should have proper
defines for documenting what's happening.
Kalle Valo July 15, 2020, 9:10 a.m. UTC | #3 
<yhchuang@realtek.com> wrote:

> From: Tzu-En Huang <tehuang@realtek.com>
> 
> CCK PD can reduce the number of false alarm of the CCK rates.
> It dynamically adjusts the power threshold and CS ratio.
> The values are compared to the values of the previous level, if
> the level is changed, set new values of power threshold and CS
> ratio.
> 
> Implement rtw_chip_ops::cck_pd_set() for 8821c.
> 
> Signed-off-by: Tzu-En Huang <tehuang@realtek.com>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

5 patches applied to wireless-drivers-next.git, thanks.

11fcb119a758 rtw88: 8821c: add cck pd settings
3a4312828ce1 rtw88: 8821c: add power tracking
5f4eab883c6a rtw88: 8821c: add beamformee support
d47e7371b23a rtw88: single rf path chips don't support TX STBC
f745eb9ca5bf rtw88: 8821c: Add 8821CE to Kconfig and Makefile
Tony Chuang July 15, 2020, 9:29 a.m. UTC | #4 
> <yhchuang@realtek.com> writes:
> 
> > From: Tzu-En Huang <tehuang@realtek.com>
> >
> > CCK PD can reduce the number of false alarm of the CCK rates.
> > It dynamically adjusts the power threshold and CS ratio.
> > The values are compared to the values of the previous level, if
> > the level is changed, set new values of power threshold and CS
> > ratio.
> >
> > Implement rtw_chip_ops::cck_pd_set() for 8821c.
> >
> > Signed-off-by: Tzu-En Huang <tehuang@realtek.com>
> > Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> [...]
> 
> > --- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
> > +++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
> > @@ -102,6 +102,7 @@ static void rtw8821c_phy_set_param(struct rtw_dev
> *rtwdev)
> >  	rtwdev->chip->ch_param[2] = rtw_read32_mask(rtwdev, REG_TXFILTER,
> MASKDWORD);
> >
> >  	rtw_phy_init(rtwdev);
> > +	rtwdev->dm_info.cck_pd_default = rtw_read8(rtwdev, REG_CSRATIO) &
> 0x1f;
> >  }
> >
> >  static int rtw8821c_mac_init(struct rtw_dev *rtwdev)
> > @@ -596,6 +597,29 @@ static void rtw8821c_phy_calibration(struct
> rtw_dev *rtwdev)
> >  	rtw8821c_do_iqk(rtwdev);
> >  }
> >
> > +static void rtw8821c_phy_cck_pd_set(struct rtw_dev *rtwdev, u8 new_lvl)
> > +{
> > +	struct rtw_dm_info *dm_info = &rtwdev->dm_info;
> > +	u8 pd[CCK_PD_LV_MAX] = {3, 7, 13, 13, 13};
> > +
> > +	if (dm_info->min_rssi > 60) {
> > +		new_lvl = 4;
> > +		pd[4] = 0x1d;
> > +		goto set_cck_pd;
> > +	}
> > +
> > +	if (dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] ==
> new_lvl)
> > +		return;
> > +
> > +	dm_info->cck_fa_avg = CCK_FA_AVG_RESET;
> > +
> > +set_cck_pd:
> > +	dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] = new_lvl;
> > +	rtw_write32_mask(rtwdev, REG_PWRTH, 0x3f0000, pd[new_lvl]);
> > +	rtw_write32_mask(rtwdev, REG_PWRTH2, 0x1f0000,
> > +			 dm_info->cck_pd_default + new_lvl * 2);
> > +}
> 
> I'm starting to see the trend of using magic values in rtw88 on the rise
> again. Please be careful with this, the source code should not be full
> of undocument values. In some special cases (eg calibration data etc)
> using undocumented values is ok, but most of cases should have proper
> defines for documenting what's happening.
> 

I'll take care of that, many thanks.

Yen-Hsuan
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 82b6accf4744..605a70e311fc 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -1483,6 +1483,7 @@  struct rtw_dm_info {
 	/* [bandwidth 0:20M/1:40M][number of path] */
 	u8 cck_pd_lv[2][RTW_RF_PATH_MAX];
 	u32 cck_fa_avg;
+	u8 cck_pd_default;
 
 	/* save the last rx phy status for debug */
 	s8 rx_snr[RTW_RF_PATH_MAX];
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.c b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
index 4bd4164d23ef..904eb494ccad 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.c
@@ -102,6 +102,7 @@  static void rtw8821c_phy_set_param(struct rtw_dev *rtwdev)
 	rtwdev->chip->ch_param[2] = rtw_read32_mask(rtwdev, REG_TXFILTER, MASKDWORD);
 
 	rtw_phy_init(rtwdev);
+	rtwdev->dm_info.cck_pd_default = rtw_read8(rtwdev, REG_CSRATIO) & 0x1f;
 }
 
 static int rtw8821c_mac_init(struct rtw_dev *rtwdev)
@@ -596,6 +597,29 @@  static void rtw8821c_phy_calibration(struct rtw_dev *rtwdev)
 	rtw8821c_do_iqk(rtwdev);
 }
 
+static void rtw8821c_phy_cck_pd_set(struct rtw_dev *rtwdev, u8 new_lvl)
+{
+	struct rtw_dm_info *dm_info = &rtwdev->dm_info;
+	u8 pd[CCK_PD_LV_MAX] = {3, 7, 13, 13, 13};
+
+	if (dm_info->min_rssi > 60) {
+		new_lvl = 4;
+		pd[4] = 0x1d;
+		goto set_cck_pd;
+	}
+
+	if (dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] == new_lvl)
+		return;
+
+	dm_info->cck_fa_avg = CCK_FA_AVG_RESET;
+
+set_cck_pd:
+	dm_info->cck_pd_lv[RTW_CHANNEL_WIDTH_20][RF_PATH_A] = new_lvl;
+	rtw_write32_mask(rtwdev, REG_PWRTH, 0x3f0000, pd[new_lvl]);
+	rtw_write32_mask(rtwdev, REG_PWRTH2, 0x1f0000,
+			 dm_info->cck_pd_default + new_lvl * 2);
+}
+
 static struct rtw_pwr_seq_cmd trans_carddis_to_cardemu_8821c[] = {
 	{0x0086,
 	 RTW_PWR_CUT_ALL_MSK,
@@ -1035,6 +1059,7 @@  static struct rtw_chip_ops rtw8821c_ops = {
 	.cfg_ldo25		= rtw8821c_cfg_ldo25,
 	.false_alarm_statistics	= rtw8821c_false_alarm_statistics,
 	.phy_calibration	= rtw8821c_phy_calibration,
+	.cck_pd_set		= rtw8821c_phy_cck_pd_set,
 };
 
 struct rtw_chip_info rtw8821c_hw_spec = {
diff --git a/drivers/net/wireless/realtek/rtw88/rtw8821c.h b/drivers/net/wireless/realtek/rtw88/rtw8821c.h
index 3b7d12bf7728..bc66b91cd961 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8821c.h
+++ b/drivers/net/wireless/realtek/rtw88/rtw8821c.h
@@ -186,11 +186,14 @@  _rtw_write32s_mask(struct rtw_dev *rtwdev, u32 addr, u32 mask, u32 data)
 #define REG_FAS		0x9a4
 #define REG_RXSB	0xa00
 #define REG_ADCINI	0xa04
+#define REG_PWRTH	0xa08
 #define REG_TXSF2	0xa24
 #define REG_TXSF6	0xa28
 #define REG_FA_CCK	0xa5c
 #define REG_RXDESC	0xa2c
 #define REG_ENTXCCK	0xa80
+#define REG_PWRTH2	0xaa8
+#define REG_CSRATIO	0xaaa
 #define REG_TXFILTER	0xaac
 #define REG_CNTRST	0xb58
 #define REG_AGCTR_A	0xc08