wifi: iwlwifi: mvm: fix power save for MLD
PATCH 1/1 wifi: iwlwifi: mvm: fix power save for MLD
From: Sultan Alsawaf <[email protected]>
The conversion from bss_info_changed() to vif_cfg/link_info_changed() removed a call to iwl_mvm_bss_info_changed_station_common() from what became the vif_cfg_changed() path. As a result, BSS_CHANGED_PS and other power save changes sent via ieee80211_vif_cfg_change_notify() are ignored for MLD, which breaks power save entirely. This has a noticeable impact on power consumption, causing idle package power to hover at 10 W on my i9-13980HX laptop with an AX210 while connected to WiFi.
Add the missing iwl_mvm_bss_info_changed_station_common() call to the vif_cfg_changed() path to fix power save for MLD. This brings idle package power down to 1 W on my laptop, a 9 W improvement.
--- drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c +2 -0
@@ -731,6 +731,8 @@ static void iwl_mvm_mld_vif_cfg_changed_station(struct iwl_mvm *mvm, mvmvif->associated = vif->cfg.assoc; +iwl_mvm_bss_info_changed_station_common (mvm, vif, &vif->bss_conf, + changes);if (!(changes &BSS_CHANGED_ASSOC ))return ;
Signature · 1 line
2.41.0
Hi,
Quoted 25 lines · Sultan Alsawaf
From: Sultan Alsawaf <[email protected]>
The conversion from
bss_info_changed()to vif_cfg/link_info_changed() removed a call toiwl_mvm_bss_info_changed_station_common()from what became thevif_cfg_changed()path. As a result,BSS_CHANGED_PSand other power save changes sent viaieee80211_vif_cfg_change_notify()are ignored for MLD, which breaks power save entirely. This has a noticeable impact on power consumption, causing idle package power to hover at 10 W on my i9-13980HX laptop with an AX210 while connected to WiFi.Add the missing
iwl_mvm_bss_info_changed_station_common()call to thevif_cfg_changed()path to fix power save for MLD. This brings idle package power down to 1 W on my laptop, a 9 W improvement.Fixes 22c588343529 ("wifi: iwlwifi: mvm: replace bss_info_changed() with vif_cfg/link_info_changed()")Signed-off-by Sultan Alsawaf <[email protected]>---
drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c | 2 ++1 file changed, 2 insertions(+)drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c +0
-0@@ -731,6 +731,8 @@ static void iwl_mvm_mld_vif_cfg_changed_station(struct iwl_mvm *mvm,
Quoted 8 lines
mvmvif->associated =vif->cfg.assoc;
+ iwl_mvm_bss_info_changed_station_common(mvm, vif,&vif->bss_conf, + changes);
if (!(changes &BSS_CHANGED_ASSOC))
return;
I agree that it looks like the call to iwl_mvm_bss_info_changed_station_common() is missing here, but I think if is is added here then the call to iwl_mvm_power_update_mac() at the end of this function is not needed anymore?
Quoted 32 lines · Gregory
Hi,
Quoted 29 lines · Sultan Alsawaf
From: Sultan Alsawaf <[email protected]>
The conversion from
bss_info_changed()to vif_cfg/link_info_changed() removed a call toiwl_mvm_bss_info_changed_station_common()from what became thevif_cfg_changed()path. As a result,BSS_CHANGED_PSand other power save changes sent viaieee80211_vif_cfg_change_notify()are ignored for MLD, which breaks power save entirely. This has a noticeable impact on power consumption, causing idle package power to hover at 10 W on my i9-13980HX laptop with an AX210 while connected to WiFi.Add the missing
iwl_mvm_bss_info_changed_station_common()call to thevif_cfg_changed()path to fix power save for MLD. This brings idle package power down to 1 W on my laptop, a 9 W improvement.Fixes 22c588343529 ("wifi: iwlwifi: mvm: replace bss_info_changed() with vif_cfg/link_info_changed()")Signed-off-by Sultan Alsawaf <[email protected]>---
drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c | 2 ++1 file changed, 2 insertions(+)drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c +0
-0@@ -731,6 +731,8 @@ static void iwl_mvm_mld_vif_cfg_changed_station(struct iwl_mvm *mvm,
mvmvif->associated =vif->cfg.assoc;
+ iwl_mvm_bss_info_changed_station_common(mvm, vif,&vif->bss_conf,
Quoted 8 lines
Quoted 4 lines
+ changes); if (!(changes &
BSS_CHANGED_ASSOC)) return;I agree that it looks like the call to
iwl_mvm_bss_info_changed_station_common()is missing here, but I think if is is added here then the call toiwl_mvm_power_update_mac()at the end of this function is not needed anymore?
That call to iwl_mvm_power_update_mac() only exists in the backport driver [1] at the moment, hence why power save is currently broken in the upstream driver.
I wasn't aware that the backport driver had that change until after I sent this, FWIW. :)
I agree that the iwl_mvm_power_update_mac() call in the backport driver would be redundant with this patch.
How would you like to go about this? Shall I send a v2 reformatted for the backport driver, with an updated commit message since power save isn't broken in the backport driver?
Or could this be staged upstream with [1] subsequently getting dropped from the backport driver?
Thanks,
Sultan
Quoted 55 lines · Sultan Alsawaf
Quoted 32 lines · Gregory
Hi,
Quoted 29 lines · Sultan Alsawaf
From: Sultan Alsawaf <[email protected]>
The conversion from
bss_info_changed()to vif_cfg/link_info_changed() removed a call toiwl_mvm_bss_info_changed_station_common()from what became thevif_cfg_changed()path. As a result,BSS_CHANGED_PSand other power save changes sent viaieee80211_vif_cfg_change_notify()are ignored for MLD, which breaks power save entirely. This has a noticeable impact on power consumption, causing idle package power to hover at 10 W on my i9-13980HX laptop with an AX210 while connected to WiFi.Add the missing
iwl_mvm_bss_info_changed_station_common()call to thevif_cfg_changed()path to fix power save for MLD. This brings idle package power down to 1 W on my laptop, a 9 W improvement.Fixes 22c588343529 ("wifi: iwlwifi: mvm: replace bss_info_changed() with vif_cfg/link_info_changed()")Signed-off-by Sultan Alsawaf <[email protected]>---
drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c | 2 ++1 file changed, 2 insertions(+)drivers/net/wireless/intel/iwlwifi/mvm/mld-mac80211.c +0
-0@@ -731,6 +731,8 @@ static void iwl_mvm_mld_vif_cfg_changed_station(struct iwl_mvm *mvm,
mvmvif->associated =vif->cfg.assoc;
+ iwl_mvm_bss_info_changed_station_common(mvm, vif,&vif->bss_conf,Quoted 8 lines
Quoted 4 lines
+ changes); if (!(changes &
BSS_CHANGED_ASSOC)) return;I agree that it looks like the call to
iwl_mvm_bss_info_changed_station_common()is missing here, but I think if is is added here then the call toiwl_mvm_power_update_mac()at the end of this function is not needed anymore?That call to
iwl_mvm_power_update_mac()only exists in the backport driver [1] at the moment, hence why power save is currently broken in the upstream driver.I wasn't aware that the backport driver had that change until after I sent this, FWIW. :)
I agree that the
iwl_mvm_power_update_mac()call in the backport driver would be redundant with this patch.How would you like to go about this? Shall I send a v2 reformatted for the backport driver, with an updated commit message since power save isn't broken in the backport driver?
Quoted 7 lines
Or could this be staged upstream with [1] subsequently getting dropped from the backport driver?
Thanks,
Sultan
Sorry for the delay. I've sent the patch in [1] from our backport tree. Can I ask you to try it?
Thanks,
Gregory