Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <60aaa1fc-99cf-476c-af51-e5ad425792f9@quicinc.com>
Date:   Fri, 22 Sep 2023 18:55:03 +0530
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v6 13/13] wifi: ath11k: send TPC power to firmware for 6
 GHz station
Content-Language: en-US
To:     Wen Gong <quic_wgong@quicinc.com>, <ath11k@lists.infradead.org>
CC:     <linux-wireless@vger.kernel.org>, <kvalo@kernel.org>,
        <quic_jjohnson@quicinc.com>
References: <20230920082349.29111-1-quic_wgong@quicinc.com>
 <20230920082349.29111-14-quic_wgong@quicinc.com>
 <eb08e0f2-c932-4d79-b2ee-813c2999d1a5@quicinc.com>
 <fbad2af6-9c3f-c241-b820-7820b4200bf4@quicinc.com>
From:   Aditya Kumar Singh <quic_adisi@quicinc.com>
In-Reply-To: <fbad2af6-9c3f-c241-b820-7820b4200bf4@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 9/22/23 15:42, Wen Gong wrote:
> On 9/22/2023 5:24 PM, Aditya Kumar Singh wrote:
>> On 9/20/23 13:53, Wen Gong wrote:
>>> When station is connected to a 6 GHz AP, it has 2 way to configure
>>> the power limit to firmware. The first way is to send 2 wmi command
>>> 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.
>>>
>>> The first way is disabled in previous patch
>>> "ath11k: discard BSS_CHANGED_TXPOWER when EXT_TPC_REG_SUPPORT for 6 
>>> GHz".
>>>
>>> Prepare the parameter for wmi command WMI_VDEV_SET_TPC_POWER_CMDID and
>>> send the firmware after vdev start response success from firmware, it
>>> is for the second way of power control.
>>>
>>> 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>
>>> ---
>>>   drivers/net/wireless/ath/ath11k/mac.c | 8 +++++++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath11k/mac.c 
>>> b/drivers/net/wireless/ath/ath11k/mac.c
>>> index a8ae281d2635..f8b907a758b1 100644
>>> --- a/drivers/net/wireless/ath/ath11k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath11k/mac.c
>>> @@ -7296,6 +7296,12 @@ ath11k_mac_vdev_start_restart(struct 
>>> ath11k_vif *arvif,
>>>           return ret;
>>>       }
>>>   +    if (ath11k_mac_supports_station_tpc(ar, arvif, chandef)) {
>>> +        ath11k_mac_fill_reg_tpc_info(ar, arvif->vif, &arvif->chanctx);
>> So we are passing local copy of channel context stored in 
>> arvif->chanctx. Do we need to update it when channel changes?
>>
>> I see that during assignment time, we are copying/updating it and 
>> accordingly the command will be sent to firmware, but what about when 
>> STA moves channel? arvif->chanctx should be updated and tpc command 
>> should be sent again in that case?
> 
> This has been discussed before here per question of Johannes:"Could this 
> information change? Should we track it in beacons?":
> 
> [PATCH 9/9] mac80211: save transmit power envelope element and power 
> constraint
> 
> https://lore.kernel.org/linux-wireless/38e7d9d2eebafa7245a36a0a0396094526eb3efd.camel@sipsolutions.net/
That's fine. That's w.r.t to TX power change. I'm saying here about CSA? 
What when AP tries to switch channel? For that client need not 
disassociate and associate back right?

In that case, channel context in mac80211 layer will change. But our 
driver's arvif->chanctx will have previous one only. We are using 
channel context to get the ieee80211_channel which has the PSD value, 
and that value we are sending to firmware via TPC command during intial 
association time. So when channel changes, firmware also should be 
updated with the latest PSD values via TPC command for the latest 
channel right?