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
> + 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
> + 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
> + 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.
>
> /* 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,
> */
> ar->rx_channel = NULL;
> spin_unlock_bh(&ar->data_lock);
> + ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
> }
>
> static enum wmi_phy_mode
> @@ -10109,40 +10226,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
> 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;
> -
> - 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: %d\n", 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");
> -
> - return ret;
> -}
> -
> static void ath12k_mac_op_sta_statistics(struct ieee80211_hw *hw,
> struct ieee80211_vif *vif,
> struct ieee80211_sta *sta,
> @@ -10431,6 +10514,7 @@ static const struct ieee80211_ops ath12k_ops = {
> .assign_vif_chanctx = ath12k_mac_op_assign_vif_chanctx,
> .unassign_vif_chanctx = ath12k_mac_op_unassign_vif_chanctx,
> .switch_vif_chanctx = ath12k_mac_op_switch_vif_chanctx,
> + .get_txpower = ath12k_mac_op_get_txpower,
> .set_rts_threshold = ath12k_mac_op_set_rts_threshold,
> .set_frag_threshold = ath12k_mac_op_set_frag_threshold,
> .set_bitrate_mask = ath12k_mac_op_set_bitrate_mask,
> @@ -11178,11 +11262,10 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
> goto err_unregister_hw;
> }
>
> + ath12k_fw_stats_init(ar);
> ath12k_debugfs_register(ar);
> }
>
> - init_completion(&ar->fw_stats_complete);
> -
> return 0;
>
> err_unregister_hw:
> diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h
> index 1acaf3f68292..af0d3c6a2a6c 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.h
> +++ b/drivers/net/wireless/ath/ath12k/mac.h
> @@ -33,6 +33,9 @@ struct ath12k_generic_iter {
> #define ATH12K_KEEPALIVE_MAX_IDLE 3895
> #define ATH12K_KEEPALIVE_MAX_UNRESPONSIVE 3900
>
> +#define ATH12K_PDEV_TX_POWER_INVALID (-1)
> +#define ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS 5000 /* msecs */
> +
> /* FIXME: should these be in ieee80211.h? */
> #define IEEE80211_VHT_MCS_SUPPORT_0_11_MASK GENMASK(23, 16)
> #define IEEE80211_DISABLE_VHT_MCS_SUPPORT_0_11 BIT(24)
