Re: [PATCH] wifi: ath12k: fix tx power, max reg power update to firmware

Santhosh Ramesh
2024-09-20T04:43:58Z

"Nicolas Escande" <nico.escande@gmail.com> writes:
> On Mon Sep 9, 2024 at 9:30 AM CEST, Santhosh Ramesh wrote:
>
> [...]
>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 137394c36460..6d7a005d83b4 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -7217,9 +7217,9 @@ ath12k_mac_vdev_start_restart(struct ath12k_vif *arvif,
>> chandef->chan->band,
>> arvif->vif->type);
>> arg.min_power = 0;
>> - arg.max_power = chandef->chan->max_power * 2;
>> - arg.max_reg_power = chandef->chan->max_reg_power * 2;
>> - arg.max_antenna_gain = chandef->chan->max_antenna_gain * 2;
>> + arg.max_power = chandef->chan->max_power;
>> + arg.max_reg_power = chandef->chan->max_reg_power;
>> + arg.max_antenna_gain = chandef->chan->max_antenna_gain;
>>
>
> For what it's worth, I already got a similar patch from QCA a few month ago, and
> it fixed some tx power problems when scanning so I know there is truth in this.
>
>> arg.pref_tx_streams = ar->num_tx_chains;
>> arg.pref_rx_streams = ar->num_rx_chains;
>>
>> base-commit: 903aaf66edc97dd5b9e3118d19677291051a9c40
>
> But there also was more in the patch:
>
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index a97e1399eeed..87e3ae49afa5 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -2563,6 +2563,8 @@ int ath12k_wmi_send_scan_chan_list_cmd(struct ath12k *ar,
> WMI_CHAN_REG_INFO1_REG_CLS);
> *reg2 |= le32_encode_bits(channel_arg->antennamax,
> WMI_CHAN_REG_INFO2_ANT_MAX);
> + *reg2 |= le32_encode_bits(channel_arg->maxregpower,
> + WMI_CHAN_REG_INFO2_MAX_TX_PWR);
>
> ath12k_dbg(ar->ab, ATH12K_DBG_WMI,
> "WMI chan scan list chan[%d] = %u, chan_info->info %8x\n",
>
> It seems that ath11k has a similar piece of code in the equivalent function
> ath11k_wmi_send_scan_chan_list_cmd . Isn't this needed ?

This change is in a separate patch, which will be sent for public review soon.

>
> Also I see that in ath12k/reg.c in ath12k_reg_update_chan_list we also send to
> the firmware:
> ch->maxpower = channel->max_power * 2;
> ch->maxregpower = channel->max_reg_power * 2;
> ch->antennamax = channel->max_antenna_gain * 2;
>
> As it is in a different wmi message, I guess the units are different. 0.5 dBm
> increment in struct ath12k_wmi_channel_arg right ? Or should we remove the '*2'
>

Yes, the firmware expects these values in 0.5 dBm increments, so we multiply
them by 2 in the ath12k_reg_update_chan_list function.