diff mbox series

wifi: ath12k: fix tx power, max reg power update to firmware

Message ID 20240909073049.3423035-1-quic_santrame@quicinc.com
State New
Headers show
Series wifi: ath12k: fix tx power, max reg power update to firmware | expand

Commit Message

Santhosh Ramesh Sept. 9, 2024, 7:30 a.m. UTC 
From: Sathishkumar Muruganandam <quic_murugana@quicinc.com>

Currently, when the vdev start WMI cmd is sent from host, vdev related
parameters such as max_reg_power, max_power, and max_antenna_gain are
multiplied by 2 before being sent to the firmware. This is incorrect
because the firmware uses 1 dBm steps for power calculations.

This leads to incorrect power values being used in the firmware and
radio, potentially causing incorrect behavior.

Fix the update of max_reg_power, max_power, and max_antenna_gain values
in the ath12k_mac_vdev_start_restart function, ensuring accurate
power settings in the firmware by sending these values as-is,
without multiplication.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00214-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: Sathishkumar Muruganandam <quic_murugana@quicinc.com>
Signed-off-by: Santhosh Ramesh <quic_santrame@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


base-commit: 903aaf66edc97dd5b9e3118d19677291051a9c40

Comments

Nicolas Escande Sept. 9, 2024, 8:20 a.m. UTC | #1 
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 ?

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'
Nicolas Escande Sept. 9, 2024, 10:23 a.m. UTC | #2 
On Mon Sep 9, 2024 at 11:07 AM CEST, Kalle Valo wrote:
> "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.
>
> What hardware and what firmware did you use? The firmware branches can
> behave differently so whenever testing something it's good to include
> that info.

Well what I tested was with the additionnal patch provided. So depending on
Santhosh response, I'll either give the proprer tested by tag, or if the patch
isn't needed. I'll need to find some time to test it :)
Kalle Valo Sept. 9, 2024, 11:15 a.m. UTC | #3 
"Nicolas Escande" <nico.escande@gmail.com> writes:

> On Mon Sep 9, 2024 at 11:07 AM CEST, Kalle Valo wrote:
>
>> "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.
>>
>> What hardware and what firmware did you use? The firmware branches can
>> behave differently so whenever testing something it's good to include
>> that info.
>
> Well what I tested was with the additionnal patch provided. So depending on
> Santhosh response, I'll either give the proprer tested by tag, or if the patch
> isn't needed. I'll need to find some time to test it :)

Can you say what hardware (QCN9274 or WCN7850) you are using? I'm not
asking for Tested-on tag, just some extra information to help the
review.
diff mbox series

Patch

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;
 
 	arg.pref_tx_streams = ar->num_tx_chains;
 	arg.pref_rx_streams = ar->num_rx_chains;