diff mbox series

[v8,12/12] wifi: ath11k: discard BSS_CHANGED_TXPOWER when EXT_TPC_REG_SUPPORT for 6 GHz

Message ID 20231204081323.5582-13-quic_bqiang@quicinc.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: ath11k: add support for 6 GHz station for various modes : LPI, SP and VLP | expand

Commit Message

Baochen Qiang Dec. 4, 2023, 8:13 a.m. UTC 
From: Wen Gong <quic_wgong@quicinc.com>

When station is connected to a 6 GHz AP, it has 2 ways to configure
the power limit to firmware. The first way is to send 2 WMI commands
WMI_PDEV_PARAM_TXPOWER_LIMIT2G/WMI_PDEV_PARAM_TXPOWER_LIMIT5G to
firmware, the second way is to send WMI_VDEV_SET_TPC_POWER_CMDID to
firmware which include more parameters for power control.

When firmware support SERVICE_EXT_TPC_REG, it means firmware support
the second way for WMI_VDEV_SET_TPC_POWER_CMDID, then ath11k discard
BSS_CHANGED_TXPOWER flag from mac80211 which is used to the first way
for 6 GHz band and select the second way.

Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23

Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
---
v8:
 no change.
v7:
 1. squash original patch 11 and patch 13.
 2. s/wmi command/WMI commands/
 3. s/way/ways/
 4. add TODO tag for CSA.

 drivers/net/wireless/ath/ath11k/mac.c | 33 +++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

Comments

Aditya Kumar Singh Dec. 4, 2023, 3:53 p.m. UTC | #1 
On 12/4/23 13:43, Baochen Qiang wrote:
> From: Wen Gong <quic_wgong@quicinc.com>
> 
> When station is connected to a 6 GHz AP, it has 2 ways to configure
> the power limit to firmware. The first way is to send 2 WMI commands
> WMI_PDEV_PARAM_TXPOWER_LIMIT2G/WMI_PDEV_PARAM_TXPOWER_LIMIT5G to
> firmware, the second way is to send WMI_VDEV_SET_TPC_POWER_CMDID to
> firmware which include more parameters for power control.
> 
> When firmware support SERVICE_EXT_TPC_REG, it means firmware support
> the second way for WMI_VDEV_SET_TPC_POWER_CMDID, then ath11k discard
> BSS_CHANGED_TXPOWER flag from mac80211 which is used to the first way
> for 6 GHz band and select the second way.
> 
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
> 
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
> ---
...snip...
> @@ -3596,9 +3608,13 @@ static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>   	if (changed & BSS_CHANGED_TXPOWER) {
>   		ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "vdev_id %i txpower %d\n",
>   			   arvif->vdev_id, info->txpower);
> -
> -		arvif->txpower = info->txpower;
> -		ath11k_mac_txpower_recalc(ar);
> +		if (ath11k_mac_supports_station_tpc(ar, arvif, &info->chandef)) {
> +			ath11k_dbg(ar->ab, ATH11K_DBG_MAC,
> +				   "discard tx power, change to set TPC power\n");
> +		} else {
> +			arvif->txpower = info->txpower;
> +			ath11k_mac_txpower_recalc(ar);
> +		}

Could you check v6 once? I remember Wen told he would drop this check 
and let FW take the min value. If we do like this, then user could not 
set his own desired value even if that is well inside the reg limits.
Baochen Qiang Dec. 6, 2023, 5:34 a.m. UTC | #2 
On 12/4/2023 11:53 PM, Aditya Kumar Singh wrote:
> On 12/4/23 13:43, Baochen Qiang wrote:
>> From: Wen Gong <quic_wgong@quicinc.com>
>>
>> When station is connected to a 6 GHz AP, it has 2 ways to configure
>> the power limit to firmware. The first way is to send 2 WMI commands
>> WMI_PDEV_PARAM_TXPOWER_LIMIT2G/WMI_PDEV_PARAM_TXPOWER_LIMIT5G to
>> firmware, the second way is to send WMI_VDEV_SET_TPC_POWER_CMDID to
>> firmware which include more parameters for power control.
>>
>> When firmware support SERVICE_EXT_TPC_REG, it means firmware support
>> the second way for WMI_VDEV_SET_TPC_POWER_CMDID, then ath11k discard
>> BSS_CHANGED_TXPOWER flag from mac80211 which is used to the first way
>> for 6 GHz band and select the second way.
>>
>> Tested-on: WCN6855 hw2.0 PCI 
>> WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
>>
>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
>> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
>> ---
> ...snip...
>> @@ -3596,9 +3608,13 @@ static void 
>> ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>>       if (changed & BSS_CHANGED_TXPOWER) {
>>           ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "vdev_id %i txpower %d\n",
>>                  arvif->vdev_id, info->txpower);
>> -
>> -        arvif->txpower = info->txpower;
>> -        ath11k_mac_txpower_recalc(ar);
>> +        if (ath11k_mac_supports_station_tpc(ar, arvif, 
>> &info->chandef)) {
>> +            ath11k_dbg(ar->ab, ATH11K_DBG_MAC,
>> +                   "discard tx power, change to set TPC power\n");
>> +        } else {
>> +            arvif->txpower = info->txpower;
>> +            ath11k_mac_txpower_recalc(ar);
>> +        }
> 
> Could you check v6 once? I remember Wen told he would drop this check 
> and let FW take the min value. If we do like this, then user could not 
> set his own desired value even if that is well inside the reg limits.
I did notice this comment in V6, but came out of a different opinion: it 
is OK to discard the TX power here, because that will be sent to 
firmware using WMI_VDEV_SET_TPC_POWER_CMDID command in another patch. 
Please correct me if wrong.

>
Aditya Kumar Singh Dec. 7, 2023, 3:31 a.m. UTC | #3 
On 12/6/23 11:04, Baochen Qiang wrote:
> 
> 
> On 12/4/2023 11:53 PM, Aditya Kumar Singh wrote:
>> On 12/4/23 13:43, Baochen Qiang wrote:
>>> From: Wen Gong <quic_wgong@quicinc.com>
>>>
>>> When station is connected to a 6 GHz AP, it has 2 ways to configure
>>> the power limit to firmware. The first way is to send 2 WMI commands
>>> WMI_PDEV_PARAM_TXPOWER_LIMIT2G/WMI_PDEV_PARAM_TXPOWER_LIMIT5G to
>>> firmware, the second way is to send WMI_VDEV_SET_TPC_POWER_CMDID to
>>> firmware which include more parameters for power control.
>>>
>>> When firmware support SERVICE_EXT_TPC_REG, it means firmware support
>>> the second way for WMI_VDEV_SET_TPC_POWER_CMDID, then ath11k discard
>>> BSS_CHANGED_TXPOWER flag from mac80211 which is used to the first way
>>> for 6 GHz band and select the second way.
>>>
>>> Tested-on: WCN6855 hw2.0 PCI 
>>> WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
>>>
>>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
>>> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
>>> ---
>> ...snip...
>>> @@ -3596,9 +3608,13 @@ static void 
>>> ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>>>       if (changed & BSS_CHANGED_TXPOWER) {
>>>           ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "vdev_id %i txpower %d\n",
>>>                  arvif->vdev_id, info->txpower);
>>> -
>>> -        arvif->txpower = info->txpower;
>>> -        ath11k_mac_txpower_recalc(ar);
>>> +        if (ath11k_mac_supports_station_tpc(ar, arvif, 
>>> &info->chandef)) {
>>> +            ath11k_dbg(ar->ab, ATH11K_DBG_MAC,
>>> +                   "discard tx power, change to set TPC power\n");
>>> +        } else {
>>> +            arvif->txpower = info->txpower;
>>> +            ath11k_mac_txpower_recalc(ar);
>>> +        }
>>
>> Could you check v6 once? I remember Wen told he would drop this check 
>> and let FW take the min value. If we do like this, then user could not 
>> set his own desired value even if that is well inside the reg limits.
> I did notice this comment in V6, but came out of a different opinion: it 
> is OK to discard the TX power here, because that will be sent to 
> firmware using WMI_VDEV_SET_TPC_POWER_CMDID command in another patch. 
> Please correct me if wrong.
Yeah that is correct but applies only during initial bring up. What if 
after client gets connected and user still wants to lower power level by 
giving command "iw wlanX set txpower fixed 1000" something like this? 
This time again it will be ignored but it won't be sent to FW.
Baochen Qiang Dec. 11, 2023, 2:26 a.m. UTC | #4 
On 12/7/2023 11:31 AM, Aditya Kumar Singh wrote:
> On 12/6/23 11:04, Baochen Qiang wrote:
>>
>>
>> On 12/4/2023 11:53 PM, Aditya Kumar Singh wrote:
>>> On 12/4/23 13:43, Baochen Qiang wrote:
>>>> From: Wen Gong <quic_wgong@quicinc.com>
>>>>
>>>> When station is connected to a 6 GHz AP, it has 2 ways to configure
>>>> the power limit to firmware. The first way is to send 2 WMI commands
>>>> WMI_PDEV_PARAM_TXPOWER_LIMIT2G/WMI_PDEV_PARAM_TXPOWER_LIMIT5G to
>>>> firmware, the second way is to send WMI_VDEV_SET_TPC_POWER_CMDID to
>>>> firmware which include more parameters for power control.
>>>>
>>>> When firmware support SERVICE_EXT_TPC_REG, it means firmware support
>>>> the second way for WMI_VDEV_SET_TPC_POWER_CMDID, then ath11k discard
>>>> BSS_CHANGED_TXPOWER flag from mac80211 which is used to the first way
>>>> for 6 GHz band and select the second way.
>>>>
>>>> Tested-on: WCN6855 hw2.0 PCI 
>>>> WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3.6510.23
>>>>
>>>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
>>>> Signed-off-by: Baochen Qiang <quic_bqiang@quicinc.com>
>>>> ---
>>> ...snip...
>>>> @@ -3596,9 +3608,13 @@ static void 
>>>> ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>>>>       if (changed & BSS_CHANGED_TXPOWER) {
>>>>           ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "vdev_id %i txpower %d\n",
>>>>                  arvif->vdev_id, info->txpower);
>>>> -
>>>> -        arvif->txpower = info->txpower;
>>>> -        ath11k_mac_txpower_recalc(ar);
>>>> +        if (ath11k_mac_supports_station_tpc(ar, arvif, 
>>>> &info->chandef)) {
>>>> +            ath11k_dbg(ar->ab, ATH11K_DBG_MAC,
>>>> +                   "discard tx power, change to set TPC power\n");
>>>> +        } else {
>>>> +            arvif->txpower = info->txpower;
>>>> +            ath11k_mac_txpower_recalc(ar);
>>>> +        }
>>>
>>> Could you check v6 once? I remember Wen told he would drop this check 
>>> and let FW take the min value. If we do like this, then user could 
>>> not set his own desired value even if that is well inside the reg 
>>> limits.
>> I did notice this comment in V6, but came out of a different opinion: 
>> it is OK to discard the TX power here, because that will be sent to 
>> firmware using WMI_VDEV_SET_TPC_POWER_CMDID command in another patch. 
>> Please correct me if wrong.
> Yeah that is correct but applies only during initial bring up. What if 
> after client gets connected and user still wants to lower power level by 
> giving command "iw wlanX set txpower fixed 1000" something like this? 
> This time again it will be ignored but it won't be sent to FW.
Exactly, will drop this patch in V9.
>
diff mbox series

Patch

diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 2520f78c9c0b..254cb0427d12 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -3397,6 +3397,18 @@  static int ath11k_mac_config_obss_pd(struct ath11k *ar,
 	return 0;
 }
 
+static bool ath11k_mac_supports_station_tpc(struct ath11k *ar,
+					    struct ath11k_vif *arvif,
+					    const struct cfg80211_chan_def *chandef)
+{
+	return ath11k_wmi_supports_6ghz_cc_ext(ar) &&
+		test_bit(WMI_TLV_SERVICE_EXT_TPC_REG_SUPPORT, ar->ab->wmi_ab.svc_map) &&
+		arvif->vdev_type == WMI_VDEV_TYPE_STA &&
+		arvif->vdev_subtype == WMI_VDEV_SUBTYPE_NONE &&
+		chandef->chan &&
+		chandef->chan->band == NL80211_BAND_6GHZ;
+}
+
 static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 					   struct ieee80211_vif *vif,
 					   struct ieee80211_bss_conf *info,
@@ -3596,9 +3608,13 @@  static void ath11k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 	if (changed & BSS_CHANGED_TXPOWER) {
 		ath11k_dbg(ar->ab, ATH11K_DBG_MAC, "vdev_id %i txpower %d\n",
 			   arvif->vdev_id, info->txpower);
-
-		arvif->txpower = info->txpower;
-		ath11k_mac_txpower_recalc(ar);
+		if (ath11k_mac_supports_station_tpc(ar, arvif, &info->chandef)) {
+			ath11k_dbg(ar->ab, ATH11K_DBG_MAC,
+				   "discard tx power, change to set TPC power\n");
+		} else {
+			arvif->txpower = info->txpower;
+			ath11k_mac_txpower_recalc(ar);
+		}
 	}
 
 	if (changed & BSS_CHANGED_PS &&
@@ -7279,6 +7295,15 @@  ath11k_mac_vdev_start_restart(struct ath11k_vif *arvif,
 		return ret;
 	}
 
+	/* TODO: For now we only set TPC power here. However when
+	 * channel changes, say CSA, it should be updated again.
+	 */
+	if (ath11k_mac_supports_station_tpc(ar, arvif, chandef)) {
+		ath11k_mac_fill_reg_tpc_info(ar, arvif->vif, &arvif->chanctx);
+		ath11k_wmi_send_vdev_set_tpc_power(ar, arvif->vdev_id,
+						   &arvif->reg_tpc_info);
+	}
+
 	if (!restart)
 		ar->num_started_vdevs++;
 
@@ -8093,7 +8118,7 @@  ath11k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 			goto out;
 		}
 		ath11k_reg_handle_chan_list(ab, reg_info, power_type);
-
+		arvif->chanctx = *ctx;
 		ath11k_mac_parse_tx_pwr_env(ar, vif, ctx);
 	}