[PATCH 2/2] wifi: ath12k: add get_txpower mac ops
Rameshkumar Sundaram
rameshkumar.sundaram at oss.qualcomm.com
Fri Jan 31 10:57:57 PST 2025
On 1/30/2025 11:55 AM, Mahendran P wrote:
> On 1/27/2025 10:52 PM, Rameshkumar Sundaram wrote:
>> From: Aditya Kumar Singh <aditya.kumar.singh at oss.qualcomm.com>
>>
>> Driver does not support get_txpower mac ops because of which
>> cfg80211 returns vif->bss_conf.txpower to user space. bss_conf.txpower
>> gets its value from ieee80211_channel->max_reg_power. However, the final
>> txpower is dependent on few other parameters apart from max regulatory
>> supported power. It is the firmware which knows about all these
>> parameters and considers the minimum for each packet transmission.
>>
>> All ath12k firmware reports the final TX power in firmware pdev stats
>> which falls under fw_stats. add get_txpower mac ops to get the TX power
>> from firmware leveraging fw_stats and return it accordingly.
>>
>> While at it, there is a possibility that repeated stats request WMI
>> commands are queued to FW if mac80211/userspace does get tx power back
>> to back(in Multiple BSS cases). This could potentially consume the WMI
>> queue completely. Hence limit this by fetching the power only for every
>> 5 seconds and reusing the value until the refresh timeout or when there
>> is a change in channel.
>>
>> Also remove init_completion(&ar->fw_stats_complete) in
>> ath12k_mac_hw_register() as ath12k_fw_stats_init() takes care of
>> it for each ar.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Aditya Kumar Singh <aditya.kumar.singh at oss.qualcomm.com>
>> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram at oss.qualcomm.com>
>> ---
>> drivers/net/wireless/ath/ath12k/core.h | 1 +
>> drivers/net/wireless/ath/ath12k/mac.c | 155 +++++++++++++++++++------
>> drivers/net/wireless/ath/ath12k/mac.h | 3 +
>> 3 files changed, 123 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index e4f51ad6a59f..42da19870713 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -731,6 +731,7 @@ struct ath12k {
>> u32 mlo_setup_status;
>> u8 ftm_msgref;
>> struct ath12k_fw_stats fw_stats;
>> + unsigned long last_tx_power_update;
>> };
>>
>> struct ath12k_hw {
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 4fb7e235be66..54fe3a2c9c0b 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -4280,6 +4280,120 @@ static int ath12k_start_scan(struct ath12k *ar,
>> return 0;
>> }
>>
>> +static int ath12k_mac_get_fw_stats(struct ath12k *ar, u32 pdev_id,
>> + u32 vdev_id, u32 stats_id)
>> +{
>> + struct ath12k_base *ab = ar->ab;
>> + struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
>> + unsigned long time_left;
>> + int ret;
>> +
>> + guard(mutex)(&ah->hw_mutex);
>> +
>> + if (ah->state != ATH12K_HW_STATE_ON)
>> + return -ENETDOWN;
>> +
>> + spin_lock_bh(&ar->data_lock);
>> + ar->fw_stats.fw_stats_done = false;
>> + ath12k_fw_stats_free(&ar->fw_stats);
>> + spin_unlock_bh(&ar->data_lock);
> rename ath12k_debugfs_fw_stats_reset and reuse instead of the above 4 lines
Sure, will do it in next version
>
>> + reinit_completion(&ar->fw_stats_complete);
>> +
>> + ret = ath12k_wmi_send_stats_request_cmd(ar, stats_id, vdev_id, pdev_id);
>> +
>> + if (ret) {
>> + ath12k_warn(ab, "failed to request fw stats: stats id %u ret %d\n",
>> + stats_id, ret);
>> + return ret;
>> + }
>> +
>> + ath12k_dbg(ab, ATH12K_DBG_WMI,
>> + "get fw stat pdev id %d vdev id %d stats id 0x%x\n",
>> + pdev_id, vdev_id, stats_id);
>> +
>> + time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
>> +
>> + if (!time_left)
>> + ath12k_warn(ab, "time out while waiting for get fw stats\n");
>> +
> suggestion is to create a separate function and move some of the common code in ath12k_mac_get_fw_stats and ath12k_debugfs_fw_stats_request
Will move the wait logic from ath12k_debugfs_fw_stats_request() to
ath12k_mac_get_fw_stats() and use the same on both places.
>
>> + return ret;
>> +}
>> +
>> +static int ath12k_mac_op_get_txpower(struct ieee80211_hw *hw,
>> + struct ieee80211_vif *vif,
>> + unsigned int link_id,
>> + int *dbm)
>> +{
>> + struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
>> + struct ath12k_fw_stats_pdev *pdev;
>> + struct ath12k_hw *ah = hw->priv;
>> + struct ath12k_link_vif *arvif;
>> + struct ath12k_base *ab;
>> + struct ath12k *ar;
>> + int ret;
>> +
>> + /* Final Tx power is minimum of Target Power, CTL power, Regulatory
>> + * Power, PSD EIRP Power. We just know the Regulatory power from the
>> + * regulatory rules obtained. FW knows all these power and sets the min
>> + * of these. Hence, we request the FW pdev stats in which FW reports
>> + * the minimum of all vdev's channel Tx power.
>> + */
>> + lockdep_assert_wiphy(hw->wiphy);
>> +
>> + arvif = wiphy_dereference(ah->hw->wiphy, ahvif->link[link_id]);
>> + if (!arvif || !arvif->ar)
>> + return -EINVAL;
>> +
>> + ar = arvif->ar;
>> + ab = ar->ab;
>> + if (ah->state != ATH12K_HW_STATE_ON)
>> + goto err_fallback;
>> +
>> + if (test_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags))
>> + return -EAGAIN;
>> +
>> + /* Limit the requests to Firmware for fetching the tx power */
>> + if (ar->chan_tx_pwr != ATH12K_PDEV_TX_POWER_INVALID &&
>> + time_before(jiffies,
>> + msecs_to_jiffies(ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS) +
>> + ar->last_tx_power_update))
>> + goto send_tx_power;
>> +
>> + ret = ath12k_mac_get_fw_stats(ar, ar->pdev->pdev_id, arvif->vdev_id,
>> + WMI_REQUEST_PDEV_STAT);
>> + if (ret) {
>> + ath12k_warn(ab, "failed to request fw pdev stats: %d\n", ret);
>> + goto err_fallback;
>> + }
>> +
>> + spin_lock_bh(&ar->data_lock);
>> + pdev = list_first_entry_or_null(&ar->fw_stats.pdevs,
>> + struct ath12k_fw_stats_pdev, list);
>> + if (!pdev) {
>> + spin_unlock_bh(&ar->data_lock);
>> + goto err_fallback;
>> + }
>> +
>> + ar->chan_tx_pwr = pdev->chan_tx_power;
> It is better to divide and store
>
>> + spin_unlock_bh(&ar->data_lock);
>> + ar->last_tx_power_update = jiffies;
>> +
>> +send_tx_power:
>> + /* tx power reported by firmware is in units of 0.5 dBm */
>> + *dbm = ar->chan_tx_pwr / 2;
> based on the above comment, we dont need to do divide everytime here during repeated calls
Thanks for the suggestion, will do it in next version
>
>> + ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware %d, reported %d dBm\n",
>> + ar->chan_tx_pwr, *dbm);
>> + return 0;
>> +
>> +err_fallback:
>> + /* We didn't get txpower from FW. Hence, relying on vif->bss_conf.txpower */
>> + *dbm = vif->bss_conf.txpower;
>> + ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware NaN, reported %d dBm\n",
>> + *dbm);
>> + return 0;
>> +}
>> +
>> static u8
>> ath12k_mac_find_link_id_by_ar(struct ath12k_vif *ahvif, struct ath12k *ar)
>> {
>> @@ -7433,6 +7547,7 @@ static int ath12k_mac_start(struct ath12k *ar)
>> ar->num_created_vdevs = 0;
>> ar->num_peers = 0;
>> ar->allocated_vdev_map = 0;
>> + ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
> ar->chan_tx_pwr type u32..and assigning signed value. fix it.
Thanks for pointing out, will fix it by assigning u32 max for
ATH12K_PDEV_TX_POWER_INVALID
>
>>
>> /* Configure monitor status ring with default rx_filter to get rx status
>> * such as rssi, rx_duration.
>> @@ -8638,6 +8753,7 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
>> */
>> ar->rx_channel = ctx->def.chan;
>> spin_unlock_bh(&ar->data_lock);
>> + ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
>>
>> return 0;
>> }
>> @@ -8666,6 +8782,7 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
>>
More information about the ath12k
mailing list