mac80211: Enhancements to dynamic power save.
RFC 1/1 mac80211: Enhancements to dynamic power save.
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.
--- net/mac80211/ieee80211_i.h | 3 +++ net/mac80211/mlme.c | 40 ++++++++++++++++++++++++++++++++++++++-- net/mac80211/scan.c | 2 +- net/mac80211/wext.c | 14 ++++++++++---- 4 files changed, 52 insertions(+), 7 deletions(-)
net/mac80211/ieee80211_i.h +3 -0
@@ -979,6 +979,9 @@ u64 ieee80211_mandatory_rates(struct ieee80211_local *local,void ieee80211_ps_enable_work (struct work_struct *work);void ieee80211_ps_disable_work (struct work_struct *work);void ieee80211_dynamic_ps_timer (unsigned long data); +void ieee80211_send_nullfunc (struct ieee80211_local *local, +struct ieee80211_sub_if_data *sdata, +int powersave);#ifdef CONFIG_MAC80211_NOINLINE #define debug_noinline noinline
net/mac80211/mlme.c +38 -2
@@ -568,6 +568,30 @@ static void ieee80211_sta_wmm_params(struct ieee80211_local *local, } } +static bool check_tim (struct ieee802_11_elems *elems,u16 aid,bool *is_mc) +{ +u8 mask; +u8 index, indexn1, indexn2; +struct ieee80211_tim_ie *tim = (struct ieee80211_tim_ie *) elems->tim; + + aid &=0x3fff ; + index = aid /8 ; + mask =1 << (aid &7 ); + + indexn1 = tim->bitmap_ctrl &0xfe ; + indexn2 = elems->tim_len + indexn1 -4 ; + +if (index < indexn1 || index > indexn2) +return false; + + index -= indexn1; + +if (tim->bitmap_ctrl &0x01 ) + *is_mc = true; + +return (bool )(tim->virtual_map[index] & mask); +} +static u32 ieee80211_handle_bss_capability (struct ieee80211_sub_if_data *sdata,u16 capab,bool erp_valid,u8 erp) {
@@ -752,6 +776,7 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata, msecs_to_jiffies(local->dynamic_ps_timeout)); else { conf->flags |=IEEE80211_CONF_PS ; +ieee80211_send_nullfunc (local, sdata,1 );ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); } }
@@ -1711,7 +1736,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,struct ieee802_11_elems elems;struct ieee80211_local *local = sdata->local;u32 changed =0 ; -bool erp_valid; +bool erp_valid, directed_tim, is_mc = false;u8 erp_value =0 ;/* Process beacon from the current BSS */
@@ -1734,6 +1759,15 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,ieee80211_sta_wmm_params (local, ifsta, elems.wmm_param, elems.wmm_param_len); + directed_tim =check_tim (&elems, ifsta->aid, &is_mc); + +if (directed_tim || is_mc) { +if (local->hw.conf.flags &&IEEE80211_CONF_PS ) { + local->hw.conf.flags &= ~IEEE80211_CONF_PS ; +ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); +ieee80211_send_nullfunc (local, sdata,0 ); + } + }if (elems.erp_info && elems.erp_info_len >=1 ) { erp_valid = true;
@@ -2616,13 +2650,15 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)void ieee80211_ps_enable_work (struct work_struct *work) { -struct ieee80211_local *local =container_of (work, +struct ieee80211_local *local =container_of (work, struct ieee80211_local, ps_enable_work); +struct ieee80211_sub_if_data *sdata = local->scan_sdata;if (local->hw.conf.flags &&IEEE80211_CONF_PS )return ; +ieee80211_send_nullfunc (local, sdata,1 ); local->hw.conf.flags |=IEEE80211_CONF_PS ;ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); }
net/mac80211/scan.c +1 -1
@@ -395,7 +395,7 @@ ieee80211_scan_rx(struct ieee80211_sub_if_data *sdata, struct sk_buff *skb,return RX_QUEUED ; } -static void ieee80211_send_nullfunc (struct ieee80211_local *local, +void ieee80211_send_nullfunc (struct ieee80211_local *local,struct ieee80211_sub_if_data *sdata,int powersave) {
net/mac80211/wext.c +10 -4
@@ -985,12 +985,18 @@ set: local->powersave = ps;if (ifsta->flags &&IEEE80211_STA_ASSOCIATED ) { -if (local->powersave) +if (local->powersave) { +ieee80211_send_nullfunc (local, sdata,1 ); conf->flags |=IEEE80211_CONF_PS ; -else + ret =ieee80211_hw_config (local, +IEEE80211_CONF_CHANGE_PS ); + } +else { conf->flags &= ~IEEE80211_CONF_PS ; - - ret =ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); + ret =ieee80211_hw_config (local, +IEEE80211_CONF_CHANGE_PS ); +ieee80211_send_nullfunc (local, sdata,0 ); + } }return ret; }
Signature · 1 line
1.6.0.1
Johannes Berg
maintainerQuoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.
Nice! Looks pretty much good to me, but could use some documentation (Kalle will hopefully add some too). For instance, a casual observer might wonder how the hardware can be saving power when the host software has to parse beacons -- obviously things will only fall into place once we support beacon miss offload.
Quoted 10 lines
net/mac80211/mlme.c +10
-0+static bool check_tim (struct ieee802_11_elems *elems,u16 aid,bool *is_mc) +{ +u8 mask; +u8 index, indexn1, indexn2; +struct ieee80211_tim_ie *tim = (struct ieee80211_tim_ie *) elems->tim; + + aid &=0x3fff ; + index = aid /8 ; + mask =1 << (aid &7 ); +
net/mac80211/mlme.c static void ieee80211_sta_wmm_params(struct ieee80211_local *local, · line 590
Quoted 10 lines
net/mac80211/mlme.c +10
-0+ indexn1 = tim->bitmap_ctrl &0xfe ; + indexn2 = elems->tim_len + indexn1 -4 ; + +if (index < indexn1 || index > indexn2) +return false; + + index -= indexn1; + +if (tim->bitmap_ctrl &0x01 ) + *is_mc = true;
Shouldn't you update *is_mc before the above return false? And also set it to false in the other case, I think.
net/mac80211/mlme.c static void ieee80211_sta_wmm_params(struct ieee80211_local *local, · line 592
Quoted 1 line
net/mac80211/mlme.c +1
-0+return (bool )(tim->virtual_map[index] & mask);
I think
return !!(... & mask);
would be more appropriate.
net/mac80211/mlme.c static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, · line 1770
Quoted 13 lines
net/mac80211/mlme.c +9
-0@@ -1734,6 +1759,15 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,ieee80211_sta_wmm_params (local, ifsta, elems.wmm_param, elems.wmm_param_len); + directed_tim =check_tim (&elems, ifsta->aid, &is_mc); + +if (directed_tim || is_mc) { +if (local->hw.conf.flags &&IEEE80211_CONF_PS ) { + local->hw.conf.flags &= ~IEEE80211_CONF_PS ; +ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); +ieee80211_send_nullfunc (local, sdata,0 ); + } + }
How will we get back into PS mode after
net/mac80211/mlme.c void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local) · line 2655
Quoted 8 lines
net/mac80211/mlme.c +1
-1@@ -2616,13 +2650,15 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)void ieee80211_ps_enable_work (struct work_struct *work) { -struct ieee80211_local *local =container_of (work, +struct ieee80211_local *local =container_of (work, struct ieee80211_local, ps_enable_work);
Huh, what's the change here?
Quoted 7 lines
net/mac80211/wext.c +0
-0@@ -985,12 +985,18 @@ set: local->powersave = ps;
net/mac80211/wext.c set: · line 994
Quoted 10 lines
net/mac80211/wext.c +6
-2if (ifsta->flags &&IEEE80211_STA_ASSOCIATED ) { -if (local->powersave) +if (local->powersave) { +ieee80211_send_nullfunc (local, sdata,1 ); conf->flags |=IEEE80211_CONF_PS ; -else + ret =ieee80211_hw_config (local, +IEEE80211_CONF_CHANGE_PS ); + } +else {
The indentation here looks a bit odd, please put the closing brace before the else
johannes
Quoted 10 lines · Johannes Berg
net/mac80211/mlme.c static void ieee80211_sta_wmm_params(struct ieee80211_local *local, · line 590
Quoted 7 lines
net/mac80211/mlme.c +7
-0+if (index < indexn1 || index > indexn2) +return false; + + index -= indexn1; + +if (tim->bitmap_ctrl &0x01 ) + *is_mc = true;Shouldn't you update
*is_mcbefore the above return false? And also set it to false in the other case, I think.
Yes, you are right.Thanks.
Quoted 5 lines
Quoted 1 line
+ return (bool)(tim->virtual_map[index] & mask);
I think
return !!(... & mask);would be more appropriate.
Thanks. I will modify it.
Quoted 4 lines
Quoted 4 lines
net/mac80211/mlme.c +0
-0@@ -1734,6 +1759,15 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,ieee80211_sta_wmm_params (local, ifsta, elems.wmm_param, elems.wmm_param_len);
Quoted 11 lines
net/mac80211/mlme.c static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, · line 1770
Quoted 9 lines
net/mac80211/mlme.c +9
-0+ directed_tim =check_tim (&elems, ifsta->aid, &is_mc); + +if (directed_tim || is_mc) { +if (local->hw.conf.flags &&IEEE80211_CONF_PS ) { + local->hw.conf.flags &= ~IEEE80211_CONF_PS ; +ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); +ieee80211_send_nullfunc (local, sdata,0 ); + } + }How will we get back into PS mode after
Once we send a null frame with pm bit off and CONF_PS changed, the tx_idle timer kicks in and we once again move to sleep state after the timeout. That piece of code is already there in Kalle's patch.
Quoted 1 line
One thing I forgot, do we need a special iwlwifi flag to turn this off?
Kalle added a flag 'IEEE80211_HW_NO_DYNAMIC_PS' in v3 RFC. May be, it can be used for iwlwifi to use its own power save and null frames as it does now instead of using from mac80211. We can check this flag and then change CONF_PS wherever it is used.
Thanks,
Vivek.
Quoted 1 line · Vivek Natarajan
Quoted 1 line
One thing I forgot, do we need a special iwlwifi flag to turn this off?
Or a flag to turn it on? Either of them are fine, I guess.
Quoted 4 lines
Kalle added a flag '
IEEE80211_HW_NO_DYNAMIC_PS' in v3 RFC. May be, it can be used for iwlwifi to use its own power save and null frames as it does now instead of using from mac80211. We can check this flag and then changeCONF_PSwherever it is used.
I think we should have a separate hw flag for null frames.
Signature · 1 line
Kalle Valo
Quoted 9 lines · Johannes Berg
Quoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.Nice! Looks pretty much good to me, but could use some documentation (Kalle will hopefully add some too).
I'm adding some documentation in my next round of patches, but we definitely need more documentation.
Quoted 2 lines
For instance, a casual observer might wonder how the hardware can be saving power when the host software has to parse beacons --
Even without beacon filtering and if there is no traffic, we can turn off the radios for the time between beacons. For example, with beacon interval 100 ms and dtim 1 the radios might be turned on for only 10 ms per dtim period. So 90% of the time the radios would be turned off and we would save power. Of course this is very much hardware specific, but just to give an idea.
Quoted 2 lines
obviously things will only fall into place once we support beacon miss offload.
Beacon filtering improves cpu power consumption.
Signature · 1 line
Kalle Valo
Quoted 7 lines · Johannes Berg
Quoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.
Quoted 5 lines
Nice! Looks pretty much good to me, but could use some documentation (Kalle will hopefully add some too). For instance, a casual observer might wonder how the hardware can be saving power when the host software has to parse beacons -- obviously things will only fall into place once we support beacon miss offload.
BTW does broadcom hw support beacon miss stuff? We need to consider carefully how we add beacon miss support because if we want to rely on it *and* also disable say the beacon filter we will be missing out on DFS channel switch announcements and HT channel badwidth changes from the AP.
Luis
Johannes Berg
maintainerQuoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.
One thing I forgot, do we need a special iwlwifi flag to turn this off? Or can iwlwifi be programmed to let the host handle this (*), which might be beneficial for interoperability and uniformity purposes?
johannes
(*) it would need to implement a beacon filter that passes up only modified beacons (where modified doesn't take into account the TIM IE except for the own AID/bcast bits), and turns off the null function stuff in firmware
Quoted 10 lines · Johannes Berg
Quoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.One thing I forgot, do we need a special iwlwifi flag to turn this off? Or can iwlwifi be programmed to let the host handle this (*), which might be beneficial for interoperability and uniformity purposes?
Having the null frame handling in firmware is faster and we might get some power savings from that. But is the saving observable, that's an another question :)
Signature · 1 line
Kalle Valo
Quoted 9 lines · Kalle Valo
Quoted 7 lines · Johannes Berg
Quoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.
Quoted 7 lines
Quoted 3 lines
One thing I forgot, do we need a special iwlwifi flag to turn this off? Or can iwlwifi be programmed to let the host handle this (*), which might be beneficial for interoperability and uniformity purposes?
Having the null frame handling in firmware is faster and we might get some power savings from that. But is the saving observable, that's an another question :)
This patch is not much relevant in actual saving , since this is the waking up part, you are powering the whole nic up anyway.But sending 2 null packets instead of one doesn't really help power save either.
If you want numbers you can measure power consumption on power lines of the chip, simple loop and power probe with scope will do the the trick.
The firmware handling of power save is more relevant in going to sleep phase and sustaining where decision about sending the null frame is faster then from host. The NIC is powered down in stages up to state where only slow clock is ticking so this is rather time sensitive
Hope this helps
Tomas
Quoted 14 lines · Tomas Winkler
Quoted 13 lines · Kalle Valo
Quoted 10 lines · Johannes Berg
Quoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.One thing I forgot, do we need a special iwlwifi flag to turn this off? Or can iwlwifi be programmed to let the host handle this (*), which might be beneficial for interoperability and uniformity purposes?
Quoted 6 lines
Quoted 3 lines
Having the null frame handling in firmware is faster and we might get some power savings from that. But is the saving observable, that's an another question :)
This patch is not much relevant in actual saving , since this is the waking up part, you are powering the whole nic up anyway.
I agree but with PM bit on-off, this is what one can do to reduce some power consumption although not significant. If more significant savings is needed, we may need to implement PS-POLL or the APSD mechanism. But this is a good start to power save in mac80211.
Quoted 4 lines
The firmware handling of power save is more relevant in going to sleep phase and sustaining where decision about sending the null frame is faster then from host. The NIC is powered down in stages up to state where only slow clock is ticking so this is rather time sensitive
AFAIK, only iwlwifi has this firmware support and we need mac80211's support in this for ath9k/ath5k,stlc45xx and b43. I believe a powerful 2GHz host processor can manage these timings as a Windows driver for these vendor cards seems to support power save without firmware support.
--
Vivek.
Quoted 19 lines · Vivek Natarajan
Quoted 18 lines · Tomas Winkler
Quoted 16 lines · Kalle Valo
Quoted 10 lines · Johannes Berg
Quoted 5 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
This patch enables mac80211 to send a null frame and also to check for tim in the beacon if power save is enabled.One thing I forgot, do we need a special iwlwifi flag to turn this off? Or can iwlwifi be programmed to let the host handle this (*), which might be beneficial for interoperability and uniformity purposes?
Having the null frame handling in firmware is faster and we might get some power savings from that. But is the saving observable, that's an another question :)
Quoted 7 lines
Quoted 2 lines
This patch is not much relevant in actual saving , since this is the waking up part, you are powering the whole nic up anyway.
I agree but with PM bit on-off, this is what one can do to reduce some power consumption although not significant. If more significant savings is needed, we may need to implement PS-POLL or the APSD mechanism. But this is a good start to power save in mac80211.
I was more referring to Kalle comment whether more power is saved when host or firmware sends the null packet,
then to actual implementation of PM.
PM be done through either toggling PM bit or PS-POLL. I'm not sure which one is better. In PS-POLL the station has more timing control but there is additional overhead since for each packet PS-POLL frame has to be issued.
Simplified explanation to APSD would be this as APSD is fine grained PM per AC category, if I'm not confusing with something else :)
Quoted 10 lines
Quoted 5 lines
The firmware handling of power save is more relevant in going to sleep phase and sustaining where decision about sending the null frame is faster then from host. The NIC is powered down in stages up to state where only slow clock is ticking so this is rather time sensitive
AFAIK, only iwlwifi has this firmware support and we need mac80211's support in this for ath9k/ath5k,stlc45xx and b43. I believe a powerful 2GHz host processor can manage these timings as a Windows driver for these vendor cards seems to support power save without firmware support.
I think I think what you meant is that only iwlwifi issues null packets from the firmware but I believe that all other NICs supports PM in HW/firmware a.k.a shutting down some parts of the HW. So to be good also to all NICs null packet shell not be sent for PM for iwlwifi driver. In good case it's just redundant in worst it can screw up the PM state machine. I suggest to add flag that indicates whether null packet is required or not.
Thanks
Tomas
Quoted 3 lines · Vivek Natarajan
This patch is based on Kalle's initial RFC patches on dynamic power save. Since ath9k/ath5k,stlc45xx and b43 need the driver to send the null frame, it is appropriate to do it from mac80211.
Yes, I agree. Actually the way I implemented this in stlc45xx was to use PS-Poll. In future, it would be nice to have it supported as well, but we can add it later.
net/mac80211/mlme.c static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, · line 1770
Quoted 9 lines
net/mac80211/mlme.c +9
-0+ directed_tim =check_tim (&elems, ifsta->aid, &is_mc); + +if (directed_tim || is_mc) { +if (local->hw.conf.flags &&IEEE80211_CONF_PS ) { + local->hw.conf.flags &= ~IEEE80211_CONF_PS ; +ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); +ieee80211_send_nullfunc (local, sdata,0 ); + } + }
Maybe I have been working too much with slow embedded hardware, but can ath5k really wake up for the beacon, send the beacon to the host, mac80211 read the beacon and then wakeup the hardware to listen for multicast frames following the beacon? The multicast frames are sent immeaditely after the dtim beacon, so this should happen really quick. But most probably I'm just missing something here.
In stlc45xx the firmware listens automatically for the multicast beacons following dtim beacons and mac80211 doesn't have to do anything for them.
net/mac80211/wext.c set: · line 998
Quoted 4 lines
net/mac80211/wext.c +3
-1- ret =ieee80211_hw_config (local,IEEE80211_CONF_CHANGE_PS ); + ret =ieee80211_hw_config (local, +IEEE80211_CONF_CHANGE_PS ); +ieee80211_send_nullfunc (local, sdata,0 );
Maybe we should have a seperate function for enabling and disabling power save mode. So that we don't forget sending of the null frame.
Signature · 1 line
Kalle Valo