[RFC] mac80211 dynamic powersave

[RFC] mac80211 dynamic powersave

[Kalle Valo]

Here's my first version of dynamic powersave patches. I have tested them
only with stlc45xx on Nokia N810. Unfortunately stlc45xx's psm command is
buggy and I haven't been able to run any longer tests, yet. Next week I will 
also test with iwl3945.

These are RFC patches, not to be applied.

TODO:
o Johannes' subif_start_xmit suggestion
o test with intel hardware (only in-tree driver using IEEE80211_CONF_PS)
o disable power save when software scanning
o association check to the work structs?
o cancel_work_sync()

[RFC 1/2] mac80211: enable IEEE80211_CONF_PS only when associated

[Kalle Valo]

Also disable power save when disassociated.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---
 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   15 +++++++++++++--
 net/mac80211/wext.c        |   30 ++++++++++++++++++++++++------
 3 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 155a204..3f25955 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -688,6 +688,7 @@ struct ieee80211_local {
 				*/
 	int wifi_wme_noack_test;
 	unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
+	bool powersave;
 
 #ifdef CONFIG_MAC80211_DEBUGFS
 	struct local_debugfsdentries {
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 12064fb..5a48b2b 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -747,6 +747,11 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
 	bss_info_changed |= BSS_CHANGED_BASIC_RATES;
 	ieee80211_bss_info_change_notify(sdata, bss_info_changed);
 
+	if (local->powersave) {
+		local->hw.conf.flags |= IEEE80211_CONF_PS;
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	}
+
 	netif_tx_start_all_queues(sdata->dev);
 	netif_carrier_on(sdata->dev);
 
@@ -815,7 +820,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 {
 	struct ieee80211_local *local = sdata->local;
 	struct sta_info *sta;
-	u32 changed = 0;
+	u32 changed = 0, config_changed = 0;
 
 	rcu_read_lock();
 
@@ -865,8 +870,14 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 	sta_info_destroy(sta);
 
 	local->hw.conf.ht.enabled = false;
-	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_HT);
+	config_changed |= IEEE80211_CONF_CHANGE_HT;
+
+	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
+		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+		config_changed |= IEEE80211_CONF_PS;
+	}
 
+	ieee80211_hw_config(local, config_changed);
 	ieee80211_bss_info_change_notify(sdata, changed);
 }
 
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index 63f36e9..febcd41 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -945,25 +945,44 @@ static int ieee80211_ioctl_siwpower(struct net_device *dev,
 				    struct iw_param *wrq,
 				    char *extra)
 {
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
+	struct ieee80211_if_sta *ifsta = &sdata->u.sta;
 	struct ieee80211_conf *conf = &local->hw.conf;
+	int ret = 0;
+	bool ps;
 
 	if (wrq->disabled) {
-		conf->flags &= ~IEEE80211_CONF_PS;
-		return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+		ps = false;
+		goto set;
 	}
 
 	switch (wrq->flags & IW_POWER_MODE) {
 	case IW_POWER_ON:       /* If not specified */
 	case IW_POWER_MODE:     /* If set all mask */
 	case IW_POWER_ALL_R:    /* If explicitely state all */
-		conf->flags |= IEEE80211_CONF_PS;
+		ps = true;
 		break;
 	default:                /* Otherwise we don't support it */
 		return -EINVAL;
 	}
 
-	return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	if (ps == local->powersave)
+		return ret;
+
+set:
+	local->powersave = ps;
+
+	if (ifsta->flags && IEEE80211_STA_ASSOCIATED) {
+		if (local->powersave)
+			conf->flags |= IEEE80211_CONF_PS;
+		else
+			conf->flags &= ~IEEE80211_CONF_PS;
+
+		ret = ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	}
+
+	return ret;
 }
 
 static int ieee80211_ioctl_giwpower(struct net_device *dev,
@@ -972,9 +991,8 @@ static int ieee80211_ioctl_giwpower(struct net_device *dev,
 				    char *extra)
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
-	struct ieee80211_conf *conf = &local->hw.conf;
 
-	wrqu->power.disabled = !(conf->flags & IEEE80211_CONF_PS);
+	wrqu->power.disabled = !local->powersave;
 
 	return 0;
 }
-- 
1.5.6.5

Re: [RFC 1/2] mac80211: enable IEEE80211_CONF_PS only when associated

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

On Sun, 2008-11-09 at 17:43 +0200, Kalle Valo wrote:


> @@ -865,8 +870,14 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>  	sta_info_destroy(sta);
>  
>  	local->hw.conf.ht.enabled = false;
> -	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_HT);
> +	config_changed |= IEEE80211_CONF_CHANGE_HT;
> +
> +	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
> +		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
> +		config_changed |= IEEE80211_CONF_PS;
> +	}

That latter flag should be CONF_CHANGED_PS.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 1/2] mac80211: enable IEEE80211_CONF_PS only when associated

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Sun, 2008-11-09 at 17:43 +0200, Kalle Valo wrote:
>
>
>> @@ -865,8 +870,14 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
>>  	sta_info_destroy(sta);
>>  
>>  	local->hw.conf.ht.enabled = false;
>> -	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_HT);
>> +	config_changed |= IEEE80211_CONF_CHANGE_HT;
>> +
>> +	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
>> +		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
>> +		config_changed |= IEEE80211_CONF_PS;
>> +	}
>
> That latter flag should be CONF_CHANGED_PS.

Oops, I'll fix it. Thanks for finding this.

-- 
Kalle Valo

[RFC 2/2] mac80211: implement dynamic power save

[Kalle Valo]

Enable power save only after an idle transmit timeout. The timeout can
be enabled with:

iwconfig wlan0 power timeout 500m

Thanks to Johannes Berg for the help with the design.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---
 net/mac80211/ieee80211_i.h |    9 +++++++++
 net/mac80211/main.c        |    5 +++++
 net/mac80211/mlme.c        |   43 +++++++++++++++++++++++++++++++++++++++++--
 net/mac80211/tx.c          |   18 ++++++++++++++++++
 net/mac80211/wext.c        |    8 ++++++--
 5 files changed, 79 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 3f25955..ab0e2df 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -688,7 +688,12 @@ struct ieee80211_local {
 				*/
 	int wifi_wme_noack_test;
 	unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
+
 	bool powersave;
+	int dynamic_ps_timeout;
+	struct work_struct ps_enable_work;
+	struct work_struct ps_disable_work;
+	struct timer_list dynamic_ps_timer;
 
 #ifdef CONFIG_MAC80211_DEBUGFS
 	struct local_debugfsdentries {
@@ -973,6 +978,10 @@ int ieee80211_set_freq(struct ieee80211_sub_if_data *sdata, int freq);
 u64 ieee80211_mandatory_rates(struct ieee80211_local *local,
 			      enum ieee80211_band band);
 
+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);
+
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
 #else
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index d631dc9..1336f24 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -701,6 +701,11 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 
 	INIT_DELAYED_WORK(&local->scan_work, ieee80211_scan_work);
 
+	INIT_WORK(&local->ps_enable_work, ieee80211_ps_enable_work);
+	INIT_WORK(&local->ps_disable_work, ieee80211_ps_disable_work);
+	setup_timer(&local->dynamic_ps_timer,
+		    ieee80211_dynamic_ps_timer, (unsigned long) local);
+
 	sta_info_init(local);
 
 	tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 5a48b2b..c897723 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -748,8 +748,14 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
 	ieee80211_bss_info_change_notify(sdata, bss_info_changed);
 
 	if (local->powersave) {
-		local->hw.conf.flags |= IEEE80211_CONF_PS;
-		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+		if (local->dynamic_ps_timeout > 0)
+			mod_timer(&local->dynamic_ps_timer, jiffies +
+				  msecs_to_jiffies(local->dynamic_ps_timeout));
+		else {
+			conf->flags |= IEEE80211_CONF_PS;
+			ieee80211_hw_config(local,
+					    IEEE80211_CONF_CHANGE_PS);
+		}
 	}
 
 	netif_tx_start_all_queues(sdata->dev);
@@ -2620,3 +2626,36 @@ void ieee80211_notify_mac(struct ieee80211_hw *hw,
 	}
 }
 EXPORT_SYMBOL(ieee80211_notify_mac);
+
+void ieee80211_ps_enable_work(struct work_struct *work)
+{
+	struct ieee80211_local *local =
+		container_of(work, struct ieee80211_local, ps_enable_work);
+
+	if (local->hw.conf.flags && IEEE80211_CONF_PS)
+		return;
+
+	local->hw.conf.flags |= IEEE80211_CONF_PS;
+
+	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+}
+
+void ieee80211_ps_disable_work(struct work_struct *work)
+{
+	struct ieee80211_local *local =
+		container_of(work, struct ieee80211_local, ps_disable_work);
+
+	if (!(local->hw.conf.flags && IEEE80211_CONF_PS))
+		return;
+
+	local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+
+	ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+}
+
+void ieee80211_dynamic_ps_timer(unsigned long data)
+{
+	struct ieee80211_local *local = (void *) data;
+
+	queue_work(local->hw.workqueue, &local->ps_enable_work);
+}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 0855cac..7a08b0f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -181,6 +181,23 @@ static int inline is_ieee80211_device(struct ieee80211_local *local,
 /* tx handlers */
 
 static ieee80211_tx_result debug_noinline
+ieee80211_tx_h_dynamic_ps(struct ieee80211_tx_data *tx)
+{
+	struct ieee80211_local *local = tx->local;
+
+	if (local->dynamic_ps_timeout == 0)
+		return TX_CONTINUE;
+
+	if (local->hw.conf.flags & IEEE80211_CONF_PS)
+		queue_work(local->hw.workqueue, &local->ps_disable_work);
+
+	mod_timer(&local->dynamic_ps_timer, jiffies +
+		  msecs_to_jiffies(local->dynamic_ps_timeout));
+
+	return TX_CONTINUE;
+}
+
+static ieee80211_tx_result debug_noinline
 ieee80211_tx_h_check_assoc(struct ieee80211_tx_data *tx)
 {
 
@@ -1105,6 +1122,7 @@ static int invoke_tx_handlers(struct ieee80211_tx_data *tx)
 		goto txh_done;
 
 	CALL_TXH(ieee80211_tx_h_check_assoc)
+	CALL_TXH(ieee80211_tx_h_dynamic_ps)
 	CALL_TXH(ieee80211_tx_h_ps_buf)
 	CALL_TXH(ieee80211_tx_h_select_key)
 	CALL_TXH(ieee80211_tx_h_michael_mic_add)
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index febcd41..2814fef 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -954,6 +954,7 @@ static int ieee80211_ioctl_siwpower(struct net_device *dev,
 
 	if (wrq->disabled) {
 		ps = false;
+		local->dynamic_ps_timeout = 0;
 		goto set;
 	}
 
@@ -963,10 +964,13 @@ static int ieee80211_ioctl_siwpower(struct net_device *dev,
 	case IW_POWER_ALL_R:    /* If explicitely state all */
 		ps = true;
 		break;
-	default:                /* Otherwise we don't support it */
-		return -EINVAL;
+	default:                /* Otherwise we ignore */
+		break;
 	}
 
+	if (wrq->flags & IW_POWER_TIMEOUT)
+		local->dynamic_ps_timeout = wrq->value / 1000;
+
 	if (ps == local->powersave)
 		return ret;
 
-- 
1.5.6.5

Re: [RFC 2/2] mac80211: implement dynamic power save

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

On Sun, 2008-11-09 at 17:43 +0200, Kalle Valo wrote:

>  	bool powersave;
> +	int dynamic_ps_timeout;
> +	struct work_struct ps_enable_work;
> +	struct work_struct ps_disable_work;
> +	struct timer_list dynamic_ps_timer;

I think you could use a delayed work for the enable-work+timer, that
combines them already.

Looks good to me.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 2/2] mac80211: implement dynamic power save

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Sun, 2008-11-09 at 17:43 +0200, Kalle Valo wrote:
>
>>  	bool powersave;
>> +	int dynamic_ps_timeout;
>> +	struct work_struct ps_enable_work;
>> +	struct work_struct ps_disable_work;
>> +	struct timer_list dynamic_ps_timer;
>
> I think you could use a delayed work for the enable-work+timer, that
> combines them already.

I was planning to but I didn't find mod_timer() equivalent for
delayed_work. I need it because I need to cancel the timer all the
time. Is there similar functionality with delayed_work?

> Looks good to me.

Thanks a lot for looking at these.

-- 
Kalle Valo

Re: [RFC 2/2] mac80211: implement dynamic power save

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 728 bytes --]

On Mon, 2008-11-10 at 11:49 +0200, Kalle Valo wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> > On Sun, 2008-11-09 at 17:43 +0200, Kalle Valo wrote:
> >
> >>  	bool powersave;
> >> +	int dynamic_ps_timeout;
> >> +	struct work_struct ps_enable_work;
> >> +	struct work_struct ps_disable_work;
> >> +	struct timer_list dynamic_ps_timer;
> >
> > I think you could use a delayed work for the enable-work+timer, that
> > combines them already.
> 
> I was planning to but I didn't find mod_timer() equivalent for
> delayed_work. I need it because I need to cancel the timer all the
> time. Is there similar functionality with delayed_work?

Ah, you're right, that doesn't seem to exist.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC 2/2] mac80211: implement dynamic power save

[Luis R. Rodriguez]

On Sun, Nov 09, 2008 at 07:43:59AM -0800, Kalle Valo wrote:
> Enable power save only after an idle transmit timeout. The timeout can
> be enabled with:
> 
> iwconfig wlan0 power timeout 500m
> 
> Thanks to Johannes Berg for the help with the design.
> 
> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
> ---
>  net/mac80211/ieee80211_i.h |    9 +++++++++
>  net/mac80211/main.c        |    5 +++++
>  net/mac80211/mlme.c        |   43 +++++++++++++++++++++++++++++++++++++++++--
>  net/mac80211/tx.c          |   18 ++++++++++++++++++
>  net/mac80211/wext.c        |    8 ++++++--
>  5 files changed, 79 insertions(+), 4 deletions(-)
> 
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 3f25955..ab0e2df 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -688,7 +688,12 @@ struct ieee80211_local {
>                                 */
>         int wifi_wme_noack_test;
>         unsigned int wmm_acm; /* bit field of ACM bits (BIT(802.1D tag)) */
> +
>         bool powersave;
> +       int dynamic_ps_timeout;
> +       struct work_struct ps_enable_work;
> +       struct work_struct ps_disable_work;
> +       struct timer_list dynamic_ps_timer;
> 
>  #ifdef CONFIG_MAC80211_DEBUGFS
>         struct local_debugfsdentries {
> @@ -973,6 +978,10 @@ int ieee80211_set_freq(struct ieee80211_sub_if_data *sdata, int freq);
>  u64 ieee80211_mandatory_rates(struct ieee80211_local *local,
>                               enum ieee80211_band band);
> 
> +void ieee80211_ps_enable_work(struct work_struct *work);
> +void ieee80211_ps_disable_work(struct work_struct *work);

Should these also be prefixed with _dynamic ? May confuse other people
working on regular ps mode I think.

Also can you elaborate a bit in the patch/log as to what dynamic power save
concept is, what drivers who implement it are expected to do as well?

Thanks,

  Luis

Re: [RFC 2/2] mac80211: implement dynamic power save

[Kalle Valo]

Luis R. Rodriguez <lrodriguez@atheros.com> writes:

> On Sun, Nov 09, 2008 at 07:43:59AM -0800, Kalle Valo wrote:
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -973,6 +978,10 @@ int ieee80211_set_freq(struct ieee80211_sub_if_data *sdata, int freq);
>>  u64 ieee80211_mandatory_rates(struct ieee80211_local *local,
>>                               enum ieee80211_band band);
>> 
>> +void ieee80211_ps_enable_work(struct work_struct *work);
>> +void ieee80211_ps_disable_work(struct work_struct *work);
>
> Should these also be prefixed with _dynamic ? May confuse other people
> working on regular ps mode I think.

I think we should. Actually there are three types of power save now:
AP mode buffering for clients in power save, the normal ("static")
power save for client mode and the dynamic version. So this will get
very confusing.

> Also can you elaborate a bit in the patch/log as to what dynamic power save
> concept is, what drivers who implement it are expected to do as well?

I will do that in the next round.

While talking about power save, can you give me any hints how power
save in client mode works in ath5k/ath9k? I haven't looked at the
drivers yet, but any hints (even how small) always help and save time.
I'm trying to understand how power save is implemented in different
hardware so that I can take that into account in my patches.

-- 
Kalle Valo

Re: [RFC 2/2] mac80211: implement dynamic power save

[Vivek Natarajan]

On Tue, Nov 11, 2008 at 12:44 PM, Kalle Valo <kalle.valo@nokia.com> wrote:

> While talking about power save, can you give me any hints how power
> save in client mode works in ath5k/ath9k? I haven't looked at the
> drivers yet, but any hints (even how small) always help and save time.
> I'm trying to understand how power save is implemented in different
> hardware so that I can take that into account in my patches.

Currently there is no support for powersave in ath9k( Not sure about
ath5k though). There is a code level implementation for the following
although we do not enable them:
 Support for network sleep mode:
     If a request for network sleep comes from higher layers, the
sleep duration having been configured based on beacon interval and the
TIM offset in the beacon based on AID is all configured, then the
chip will happily go to sleep and wake up for the beacon to read TIM.

But the code for calulating TIM offset, reading TIM, to decide the power
level based on TIM, extending awake time based on DTIM needs to be
done. Since these are common to all drivers, these may get into
mac80211( Not sure if it is already implemented). Please correct me if
I am wrong here.

I believe all these are done in the firmware for Intel since Tomas mentioned
sometime back that all the power save features works in iwlwifi.

>  Actually there are three types of power save now:
> AP mode buffering for clients in power save, the normal ("static")
> power save for client mode and the dynamic version. So this will get
> very confusing.
I have come across  this dynamic power save(Switch off radio when there
is no RX/Tx for sometime) and also buffering in AP mode. I would be happy to
know about the static power save.

Regards,
Vivek.

Re: [RFC 2/2] mac80211: implement dynamic power save

[Kalle Valo]

Vivek Natarajan <vivek.natraj@gmail.com> writes:

> Currently there is no support for powersave in ath9k( Not sure about
> ath5k though). There is a code level implementation for the following
> although we do not enable them:
>  Support for network sleep mode:
>      If a request for network sleep comes from higher layers, the
> sleep duration having been configured based on beacon interval and the
> TIM offset in the beacon based on AID is all configured, then the
> chip will happily go to sleep and wake up for the beacon to read TIM.

> But the code for calulating TIM offset, reading TIM, to decide the power
> level based on TIM, extending awake time based on DTIM needs to be
> done. 

Thanks, this was useful. Is the interface for these power save
commands available somewhere? I would like to take a look so that I
understand what kind of paremeters the driver can set.

> Since these are common to all drivers, these may get into mac80211(
> Not sure if it is already implemented). Please correct me if I am
> wrong here.

Yes, if both stlc45xx and ath5/9k need these it would make sense to
add support to mac80211.

> I believe all these are done in the firmware for Intel since Tomas mentioned
> sometime back that all the power save features works in iwlwifi.

Yes. I have seen other hardware as well which have the power save
support in firmware.

>>  Actually there are three types of power save now:
>> AP mode buffering for clients in power save, the normal ("static")
>> power save for client mode and the dynamic version. So this will get
>> very confusing.
>
> I have come across  this dynamic power save(Switch off radio when there
> is no RX/Tx for sometime) and also buffering in AP mode. I would be happy to
> know about the static power save.

The term "static power save" is just my own invention and it means
opposite of dynamic power save. For me static power save means that
the chip sleeps as much as possible, for example transmits a frame and
goes back to sleep immediately after receiving the ack.

Sorry for bad wording, all the power save terms should be standardised :)

-- 
Kalle Valo

Re: [RFC 2/2] mac80211: implement dynamic power save

[Nick Kossifidis]

2008/11/11 Kalle Valo <kalle.valo@nokia.com>:
> Vivek Natarajan <vivek.natraj@gmail.com> writes:
>
>> Currently there is no support for powersave in ath9k( Not sure about
>> ath5k though). There is a code level implementation for the following
>> although we do not enable them:
>>  Support for network sleep mode:
>>      If a request for network sleep comes from higher layers, the
>> sleep duration having been configured based on beacon interval and the
>> TIM offset in the beacon based on AID is all configured, then the
>> chip will happily go to sleep and wake up for the beacon to read TIM.
>
>> But the code for calulating TIM offset, reading TIM, to decide the power
>> level based on TIM, extending awake time based on DTIM needs to be
>> done.
>
> Thanks, this was useful. Is the interface for these power save
> commands available somewhere? I would like to take a look so that I
> understand what kind of paremeters the driver can set.
>

Check out these 2 functions...
http://git.kernel.org/?p=linux/kernel/git/linville/wireless-testing.git;a=blob;f=drivers/net/wireless/ath5k/base.c;h=8c9961525cc44bee2ff4be4619264faa3f02962d;hb=HEAD#l2035
http://git.kernel.org/?p=linux/kernel/git/linville/wireless-testing.git;a=blob;f=drivers/net/wireless/ath5k/pcu.c;h=d7f0c1017bda55a86a18acbb1384ebee34377eb5;hb=HEAD#l674

We set some intervals during beacon_update_timers and init_beacon is
called to pass them on hw, init_beacon can (should) also set enhanced
sleep registers on AR5212, for more infos you can check legacy-hal or
read reg.h
http://git.kernel.org/?p=linux/kernel/git/linville/wireless-testing.git;a=blob;f=drivers/net/wireless/ath5k/reg.h;h=69755fc2f9bee78ab78489bec6cd1ceb782ef914;hb=HEAD#l1602


-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [RFC] mac80211 dynamic powersave

[Vivek Natarajan]

On Sun, Nov 9, 2008 at 9:13 PM, Kalle Valo <kalle.valo@nokia.com> wrote:
>
> Here's my first version of dynamic powersave patches. I have tested them
> only with stlc45xx on Nokia N810. Unfortunately stlc45xx's psm command is
> buggy and I haven't been able to run any longer tests, yet. Next week I will
> also test with iwl3945.
>
> These are RFC patches, not to be applied.
>
> TODO:
> o Johannes' subif_start_xmit suggestion
> o test with intel hardware (only in-tree driver using IEEE80211_CONF_PS)
> o disable power save when software scanning
> o association check to the work structs?
> o cancel_work_sync()
>
Hello Kalle,
  I have a few questions on the PS implementation. Here they are,

1) I see from your patch that power save is enabled whenever there
   is no Tx for the specified 'timeout' period. Should we also take into
   account the type of power connection( i.e battery or direct ac supply)
   before enabling PS?

2) Do you intend to leave the NULL frame formation, waking up the chip
     for TIM and modifying sleep/awake time according to  DTIM to
    the driver since I did not see this in your TODO list?( I understand from
    the iwlwifi code that it just sends a request to the firmware for sending
    NULL frame and the firmware takes care of the rest. But it may not be
    so in the case of other vendor drivers.)

3) Is the PS-POLL concept(or M frames in N milliseconds concept of Johannes)
     dropped after the DNS response timings related discussion that you had
     since it might save some more power in low-traffic conditions?

Thanks,
Vivek.

Re: [RFC] mac80211 dynamic powersave

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1323 bytes --]

On Tue, 2008-11-11 at 10:02 +0530, Vivek Natarajan wrote:

> 1) I see from your patch that power save is enabled whenever there
>    is no Tx for the specified 'timeout' period. Should we also take into
>    account the type of power connection( i.e battery or direct ac supply)
>    before enabling PS?

That's userspace policy, we only provide the mechanism to set the
required state.

> 2) Do you intend to leave the NULL frame formation, waking up the chip
>      for TIM and modifying sleep/awake time according to  DTIM to
>     the driver since I did not see this in your TODO list?( I understand from
>     the iwlwifi code that it just sends a request to the firmware for sending
>     NULL frame and the firmware takes care of the rest. But it may not be
>     so in the case of other vendor drivers.)

Can you elaborate on this? I don't think we're concerned with that in
dynPS.

> 3) Is the PS-POLL concept(or M frames in N milliseconds concept of Johannes)
>      dropped after the DNS response timings related discussion that you had
>      since it might save some more power in low-traffic conditions?

It might save more power, but it seems that the compromise is too
strongly in favour of power for the dynPS mode, so you can just do
static PS all the time instead.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFC] mac80211 dynamic powersave

[Vivek Natarajan]

Johannes Berg <johannes@sipsolutions.net> wrote:

> Vivek Natarajan wrote:
>> 2) Do you intend to leave the NULL frame formation, waking up the chip
>>      for TIM and modifying sleep/awake time according to  DTIM to
>>     the driver since I did not see this in your TODO list?( I understand from
>>     the iwlwifi code that it just sends a request to the firmware for sending
>>     NULL frame and the firmware takes care of the rest. But it may not be
>>     so in the case of other vendor drivers.)
>
> Can you elaborate on this? I don't think we're concerned with that in
> dynPS.

I'm listing  whatever I have understood about 'Power save'.
It might be too basic but do correct me if I'm wrong somewhere.

NULL DATA frame:
a) A NULL data frame with PM bit ON is to be sent to the AP before
going to sleep.
b) After sending Null frame, the chip should not go to sleep before
getting ack: move to sleep pending state
c) In sleep pending state, wait for one more 'timeout' period before
sending null frame again
d) If ack is received, goto sleep.
    This is to make sure that the AP buffers frames for us.
 Sanity check:
Check if the hw has been sleeping too long! It could imply that the
sleep timers have gotten out of sync with the TSF. If so, force wake
until the sleep timers get resynced.

Tx:
If there is a frame to transmit, send null data with PM bit off and start Tx.
Rx:
If there is any buffered frames, it is indicated in TIM.
TIPS on TIM:
Some hardware processes the TIM IE and fires an interrupt when the TIM
bit is set.
For hardware that doesn't,  we should process the TIM in software as
part of processing the received Beacon
→ A function to check if TIM bit is set based on the offset in the beacon.
→ A function to put the chip to awake(if TIM is set) and send a null
data with PM bit off.

DTIM:
The period after which multicast traffic may be transmitted. So be
awake even if there is no unicast packets
as indicated by TIM.
There are some registers related to TIM and DTIM periods in ath5k
registers.(Thanks Nick for pointing
them out)Need to dig more to see if it is enough to program these
registers to be awake for post-DTIM muticast traffic.

--
Vivek

Re: [RFC] mac80211 dynamic powersave

[Luis R. Rodriguez]

On Tue, Nov 11, 2008 at 10:17:54PM -0800, Vivek Natarajan wrote:
> Johannes Berg <johannes@sipsolutions.net> wrote:
>=20
> > Vivek Natarajan wrote:
> >> 2) Do you intend to leave the NULL frame formation, waking up the =
chip
> >>      for TIM and modifying sleep/awake time according to  DTIM to
> >>     the driver since I did not see this in your TODO list?( I unde=
rstand from
> >>     the iwlwifi code that it just sends a request to the firmware =
for sending
> >>     NULL frame and the firmware takes care of the rest. But it may=
 not be
> >>     so in the case of other vendor drivers.)
> >
> > Can you elaborate on this? I don't think we're concerned with that =
in
> > dynPS.
>=20
> I'm listing  whatever I have understood about 'Power save'.
> It might be too basic but do correct me if I'm wrong somewhere.
>=20
> NULL DATA frame:
> a) A NULL data frame with PM bit ON is to be sent to the AP before
> going to sleep.
> b) After sending Null frame, the chip should not go to sleep before
> getting ack: move to sleep pending state
> c) In sleep pending state, wait for one more 'timeout' period before
> sending null frame again
> d) If ack is received, goto sleep.
>     This is to make sure that the AP buffers frames for us.
>  Sanity check:
> Check if the hw has been sleeping too long! It could imply that the
> sleep timers have gotten out of sync with the TSF. If so, force wake
> until the sleep timers get resynced.
>=20
> Tx:
> If there is a frame to transmit, send null data with PM bit off and s=
tart Tx.
> Rx:
> If there is any buffered frames, it is indicated in TIM.
> TIPS on TIM:
> Some hardware processes the TIM IE and fires an interrupt when the TI=
M
> bit is set.
> For hardware that doesn't,  we should process the TIM in software as
> part of processing the received Beacon
> =E2=86=92 A function to check if TIM bit is set based on the offset i=
n the beacon.
> =E2=86=92 A function to put the chip to awake(if TIM is set) and send=
 a null
> data with PM bit off.
>=20
> DTIM:
> The period after which multicast traffic may be transmitted. So be
> awake even if there is no unicast packets
> as indicated by TIM.
> There are some registers related to TIM and DTIM periods in ath5k
> registers.(Thanks Nick for pointing
> them out)Need to dig more to see if it is enough to program these
> registers to be awake for post-DTIM muticast traffic.

Also check out:

http://wireless.kernel.org/en/developers/Documentation/ieee80211/power-=
savings

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] mac80211 dynamic powersave

[Kalle Valo]

Vivek Natarajan <vivek.natraj@gmail.com> writes:

> Hello Kalle,

Hello Vivek,

>   I have a few questions on the PS implementation. Here they are,
>
> 1) I see from your patch that power save is enabled whenever there
>    is no Tx for the specified 'timeout' period. Should we also take into
>    account the type of power connection( i.e battery or direct ac supply)
>    before enabling PS?

That's a policy decision and should be in user space. User space can
enable or disable the dynamic PS as it sees fit based on different
parameters.

> 2) Do you intend to leave the NULL frame formation

[to the driver]

Yes, that was my and Johannes' plan.

> waking up the chip for TIM

What do you mean by this? Sending a beacon interval setting to the chip?

> and modifying sleep/awake time according to DTIM to the driver since
> I did not see this in your TODO list?

I think these should be in mac80211, just haven't thought that far
yet. I'll add it to the TODO list.

> ( I understand from the iwlwifi code that it just sends a request to
> the firmware for sending NULL frame and the firmware takes care of
> the rest. But it may not be so in the case of other vendor drivers.)

Yes, that's how I have understood iwlwifi to work. Currently stlc45xx
is the only hardware which requires the driver to create the null
frames, but if we are more than one hardware I think we need to
considering adding the feature to mac80211 (with a new HW flag, of
course).

> 3) Is the PS-POLL concept(or M frames in N milliseconds concept of Johannes)
>
>      dropped after the DNS response timings related discussion that you had
>      since it might save some more power in low-traffic conditions?

I have dropped it for now. I try to get a simple implementation
approved first, we can add advanced stuff later.

-- 
Kalle Valo

Re: [RFC] mac80211 dynamic powersave

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 419 bytes --]

On Tue, 2008-11-11 at 14:29 +0200, Kalle Valo wrote:

> Yes, that's how I have understood iwlwifi to work. Currently stlc45xx
> is the only hardware which requires the driver to create the null
> frames, but if we are more than one hardware I think we need to
> considering adding the feature to mac80211 (with a new HW flag, of
> course).

b43 will need that too, if we get PS mode working there.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]