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

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

[Santhosh Ramesh]

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(-)

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;

base-commit: 903aaf66edc97dd5b9e3118d19677291051a9c40
-- 
2.34.1

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

[Nicolas Escande]

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'

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

[Kalle Valo]

"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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

[Nicolas Escande]

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 :)

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

[Kalle Valo]

"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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

[Nicolas Escande]

On Mon Sep 9, 2024 at 1:15 PM CEST, Kalle Valo wrote:
> "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.

Sure, I tested it with a QCN9274, in 2*2x2 (2.4GHz + 5GHz) in station mode with
a special firmware WLAN.WBE.1.2.1-00148.1-QCAHKSWPL_SILICONZ-8.
Without this, all probe requests sent by the firwmare during a scan (both bands)
were send with around 15/20 dBm less power that the data frames.

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

[Santhosh Ramesh]

"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.

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

[Nicolas Escande]

On Fri Sep 20, 2024 at 6:43 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.

OK

>
> > 
> > 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.
Good

Then good news this gets mainlined by the way, but it's a shame it takes this
long (like 10 months or so...)

Reviewed-by: Nicolas Escande <nico.escande@gmail.com>

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

[Kalle Valo]

Santhosh Ramesh <quic_santrame@quicinc.com> wrote:

> 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>

Acked-by: Kalle Valo <kvalo@kernel.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20240909073049.3423035-1-quic_santrame@quicinc.com/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
https://docs.kernel.org/process/submitting-patches.html

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

[Nicolas Escande]

On Fri Sep 20, 2024 at 6:43 AM CEST, Santhosh Ramesh wrote:
> "Nicolas Escande" <nico.escande@gmail.com> writes:
[...]
> > 
> > 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.

Hey Santhosh,

Unless I'm mistaken (which is very much possible) I didn't see any patch posted
to fix that. Weren't you guys supposed to have done it by now ?

> > 
> > 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.

Thanks

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

[Aditya Kumar Singh]

On 9/9/24 13:00, Santhosh Ramesh wrote:
> 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
> 

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")

> Signed-off-by: Sathishkumar Muruganandam <quic_murugana@quicinc.com>
> Signed-off-by: Santhosh Ramesh <quic_santrame@quicinc.com>
> ---

Hi Jeff/Kalle,

It would be good, if Fixes tag can be added while applying this patch.


-- 
Aditya

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

[Jeff Johnson]

On Mon, 09 Sep 2024 13:00:49 +0530, Santhosh Ramesh wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] wifi: ath12k: fix tx power, max reg power update to firmware
      commit: 3540bba855b4b422e8b977d11aa8173ccb4f089d

Best regards,
-- 
Jeff Johnson <jeff.johnson@oss.qualcomm.com>