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@xxxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxxx>
Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@xxxxxxxxxxxxxxxx>
---
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,
