[PATCH v5 0/3] mac80211 dynamic power save

[PATCH v5 0/3] mac80211 dynamic power save

[Kalle Valo]

Here is v5 of my dynamic powersave patches. I have tested them with
stlc45xx on Nokia N810.

I think (and hope) that the patches are close to inclusion now. Please
review.

changes in v5:

o check STA interface earlier in siwpower()
o moved queue track patch before dynamic power save patch
o add new functions ieee80211_wake_queues_by_reason() and
  ieee80211_stop_queues_by_reason() for optimised locking
o dropped "move dynamic_ps_timeout to hw.conf" for now, will resubmit
  it later when I have example driver implementation
o renamed the hw flag to IEEE80211_HW_NO_STACK_DYNAMIC_PS and inverted
  it's functionality, now drivers need to explicitly disable the feature
o folded hw flag patch to the main dynamic power save patch

changes in v4:
o add check for station interface to siwpower()
o call hw_config() also for dynamic timeout changes
o add dynamic_ prefix to workqueues
o test for IEEE80211_HW_NO_DYNAMIC_PS in siwpower()
o wrote documentation for IEEE80211_HW_NO_DYNAMIC_PS
o wrote documentation for conf->dynamic_ps_timeout

changes in v3:
o add a hw flag to notify that driver doesn't support dynamic power save and
  which will enable mac80211 implementation
o delete dynamic_ps_timer and cancel ps_enable_work during
  association, hopefully it's race free now
o provide timeout value to the drivers, for example iwlwifi should use
  it (example patch under works)
o fix a bad bug in testing IEEE80211_STA_ASSOCIATED flag with && operator

Power save related features which I'll implement later:
o disable power save when software scanning
o move dynamic_ps_timeout to hw.conf so that drivers can use it in
  case they need to
o beacon period (wakeup period) to the drivers, can be set from user
  space but otherwise is bss's DTIM value

---

Kalle Valo (3):
      mac80211: implement dynamic power save
      mac80211: track master queue status
      mac80211: enable IEEE80211_CONF_PS only when associated


 include/net/mac80211.h     |    6 +++
 net/mac80211/ieee80211_i.h |   23 +++++++++++-
 net/mac80211/main.c        |    9 +++++
 net/mac80211/mlme.c        |   60 ++++++++++++++++++++++++++++++-
 net/mac80211/tx.c          |   13 +++++++
 net/mac80211/util.c        |   86 +++++++++++++++++++++++++++++++++++++++++---
 net/mac80211/wext.c        |   46 +++++++++++++++++++-----
 7 files changed, 226 insertions(+), 17 deletions(-)

[PATCH v5 1/3] mac80211: enable IEEE80211_CONF_PS only when associated

[Kalle Valo]

Also disable power save when disassociated. It makes no sense to have
power save enabled while disassociated.

iwlwifi seems to have this check in the driver, but it's better to do this
in mac80211 instead.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 net/mac80211/ieee80211_i.h |    1 +
 net/mac80211/mlme.c        |   15 +++++++++++++--
 net/mac80211/wext.c        |   32 ++++++++++++++++++++++++++------
 3 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a7dabae..18b9160 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 9a06905..96493e2 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -746,6 +746,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);
 
@@ -818,7 +823,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,
 
 	local->hw.conf.ht.enabled = false;
 	local->oper_channel_type = NL80211_CHAN_NO_HT;
-	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);
 
 	rcu_read_lock();
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index 1542804..f6640d0 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -830,25 +830,46 @@ 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_conf *conf = &local->hw.conf;
+	int ret = 0;
+	bool ps;
+
+	if (sdata->vif.type != NL80211_IFTYPE_STATION)
+		return -EINVAL;
 
 	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 (sdata->u.sta.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,
@@ -857,9 +878,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;
 }

Re: [PATCH v5 1/3] mac80211: enable IEEE80211_CONF_PS only when associated

[Johannes Berg]

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

On Thu, 2008-12-18 at 23:16 +0200, Kalle Valo wrote:

> +	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
> +		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
> +		config_changed |= IEEE80211_CONF_PS;

IEEE80211_CONF_CHANGE_PS

other than that,

Acked-by: Johannes Berg <johannes@sipsolutions.net>

johannes

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

Re: [PATCH v5 1/3] mac80211: enable IEEE80211_CONF_PS only when associated

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Thu, 2008-12-18 at 23:16 +0200, Kalle Valo wrote:
>
>> +	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
>> +		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
>> +		config_changed |= IEEE80211_CONF_PS;
>
> IEEE80211_CONF_CHANGE_PS

Will fix in v6.

> other than that,
>
> Acked-by: Johannes Berg <johannes@sipsolutions.net>

Thanks!

-- 
Kalle Valo

[PATCH v5 2/3] mac80211: track master queue status

[Kalle Valo]

This is a preparation for the dynamic power save support. In future there are
two paths to stop the master queues and we need to track this properly to
avoid starting queues incorrectly. Implement this by adding a status
array for each queue.

The original idea and design is from Johannes Berg, I just did
the implementation based on his notes. All the bugs are mine, of course.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 net/mac80211/ieee80211_i.h |   12 ++++++
 net/mac80211/main.c        |    2 +
 net/mac80211/util.c        |   86 +++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 93 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 18b9160..a74d673 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -538,6 +538,10 @@ enum {
 	IEEE80211_ADDBA_MSG	= 4,
 };
 
+enum queue_stop_reason {
+	IEEE80211_QUEUE_STOP_REASON_DRIVER,
+};
+
 /* maximum number of hardware queues we support. */
 #define QD_MAX_QUEUES (IEEE80211_MAX_AMPDU_QUEUES + IEEE80211_MAX_QUEUES)
 
@@ -554,7 +558,8 @@ struct ieee80211_local {
 	const struct ieee80211_ops *ops;
 
 	unsigned long queue_pool[BITS_TO_LONGS(QD_MAX_QUEUES)];
-
+	unsigned long queue_stop_reasons[IEEE80211_MAX_QUEUES];
+	spinlock_t queue_stop_reason_lock;
 	struct net_device *mdev; /* wmaster# - "master" 802.11 device */
 	int open_count;
 	int monitors, cooked_mntrs;
@@ -972,6 +977,11 @@ 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_wake_queues_by_reason(struct ieee80211_hw *hw,
+				     enum queue_stop_reason reason);
+void ieee80211_stop_queues_by_reason(struct ieee80211_hw *hw,
+				     enum queue_stop_reason reason);
+
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
 #else
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index a0371ca..7c6e90a 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -724,6 +724,8 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 
 	spin_lock_init(&local->key_lock);
 
+	spin_lock_init(&local->queue_stop_reason_lock);
+
 	INIT_DELAYED_WORK(&local->scan_work, ieee80211_scan_work);
 
 	sta_info_init(local);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 71a8391..fb89e1d 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -330,10 +330,20 @@ __le16 ieee80211_ctstoself_duration(struct ieee80211_hw *hw,
 }
 EXPORT_SYMBOL(ieee80211_ctstoself_duration);
 
-void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
+static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,
+				   enum queue_stop_reason reason)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
 
+	/* we don't need to track ampdu queues */
+	if (queue < ieee80211_num_regular_queues(hw)) {
+		__clear_bit(reason, &local->queue_stop_reasons[queue]);
+
+		if (local->queue_stop_reasons[queue] != 0)
+			/* someone still has this queue stopped */
+			return;
+	}
+
 	if (test_bit(queue, local->queues_pending)) {
 		set_bit(queue, local->queues_pending_run);
 		tasklet_schedule(&local->tx_pending_tasklet);
@@ -341,22 +351,74 @@ void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
 		netif_wake_subqueue(local->mdev, queue);
 	}
 }
+
+void ieee80211_wake_queue_by_reason(struct ieee80211_hw *hw, int queue,
+				    enum queue_stop_reason reason)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	unsigned long flags;
+
+	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+	__ieee80211_wake_queue(hw, queue, reason);
+	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+}
+
+void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
+{
+	ieee80211_wake_queue_by_reason(hw, queue,
+				       IEEE80211_QUEUE_STOP_REASON_DRIVER);
+}
 EXPORT_SYMBOL(ieee80211_wake_queue);
 
-void ieee80211_stop_queue(struct ieee80211_hw *hw, int queue)
+static void __ieee80211_stop_queue(struct ieee80211_hw *hw, int queue,
+				   enum queue_stop_reason reason)
 {
 	struct ieee80211_local *local = hw_to_local(hw);
 
+	/* we don't need to track ampdu queues */
+	if (queue < ieee80211_num_regular_queues(hw))
+		__set_bit(reason, &local->queue_stop_reasons[queue]);
+
 	netif_stop_subqueue(local->mdev, queue);
 }
+
+void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
+				    enum queue_stop_reason reason)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+	unsigned long flags;
+
+	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+	__ieee80211_stop_queue(hw, queue, reason);
+	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+}
+
+void ieee80211_stop_queue(struct ieee80211_hw *hw, int queue)
+{
+	ieee80211_stop_queue_by_reason(hw, queue,
+				       IEEE80211_QUEUE_STOP_REASON_DRIVER);
+}
 EXPORT_SYMBOL(ieee80211_stop_queue);
 
-void ieee80211_stop_queues(struct ieee80211_hw *hw)
+void ieee80211_stop_queues_by_reason(struct ieee80211_hw *hw,
+				    enum queue_stop_reason reason)
 {
+	struct ieee80211_local *local = hw_to_local(hw);
+	unsigned long flags;
 	int i;
 
+	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+
 	for (i = 0; i < ieee80211_num_queues(hw); i++)
-		ieee80211_stop_queue(hw, i);
+		__ieee80211_stop_queue(hw, i, reason);
+
+	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+}
+
+void ieee80211_stop_queues(struct ieee80211_hw *hw)
+{
+	ieee80211_stop_queues_by_reason(hw,
+					IEEE80211_QUEUE_STOP_REASON_DRIVER);
 }
 EXPORT_SYMBOL(ieee80211_stop_queues);
 
@@ -367,12 +429,24 @@ int ieee80211_queue_stopped(struct ieee80211_hw *hw, int queue)
 }
 EXPORT_SYMBOL(ieee80211_queue_stopped);
 
-void ieee80211_wake_queues(struct ieee80211_hw *hw)
+void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
+				     enum queue_stop_reason reason)
 {
+	struct ieee80211_local *local = hw_to_local(hw);
+	unsigned long flags;
 	int i;
 
+	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+
 	for (i = 0; i < hw->queues + hw->ampdu_queues; i++)
-		ieee80211_wake_queue(hw, i);
+		__ieee80211_wake_queue(hw, i, reason);
+
+	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+}
+
+void ieee80211_wake_queues(struct ieee80211_hw *hw)
+{
+	ieee80211_wake_queues_by_reason(hw, IEEE80211_QUEUE_STOP_REASON_DRIVER);
 }
 EXPORT_SYMBOL(ieee80211_wake_queues);
 

Re: [PATCH v5 2/3] mac80211: track master queue status

[Johannes Berg]

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

On Thu, 2008-12-18 at 23:17 +0200, Kalle Valo wrote:
> This is a preparation for the dynamic power save support. In future there are
> two paths to stop the master queues and we need to track this properly to
> avoid starting queues incorrectly. Implement this by adding a status
> array for each queue.
> 
> The original idea and design is from Johannes Berg, I just did
> the implementation based on his notes. All the bugs are mine, of course.
> 
> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>

Looks good, thanks

Acked-by: Johannes Berg <johannes@sipsolutions.net>

> ---
> 
>  net/mac80211/ieee80211_i.h |   12 ++++++
>  net/mac80211/main.c        |    2 +
>  net/mac80211/util.c        |   86 +++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 93 insertions(+), 7 deletions(-)
> 
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index 18b9160..a74d673 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -538,6 +538,10 @@ enum {
>  	IEEE80211_ADDBA_MSG	= 4,
>  };
>  
> +enum queue_stop_reason {
> +	IEEE80211_QUEUE_STOP_REASON_DRIVER,
> +};
> +
>  /* maximum number of hardware queues we support. */
>  #define QD_MAX_QUEUES (IEEE80211_MAX_AMPDU_QUEUES + IEEE80211_MAX_QUEUES)
>  
> @@ -554,7 +558,8 @@ struct ieee80211_local {
>  	const struct ieee80211_ops *ops;
>  
>  	unsigned long queue_pool[BITS_TO_LONGS(QD_MAX_QUEUES)];
> -
> +	unsigned long queue_stop_reasons[IEEE80211_MAX_QUEUES];
> +	spinlock_t queue_stop_reason_lock;
>  	struct net_device *mdev; /* wmaster# - "master" 802.11 device */
>  	int open_count;
>  	int monitors, cooked_mntrs;
> @@ -972,6 +977,11 @@ 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_wake_queues_by_reason(struct ieee80211_hw *hw,
> +				     enum queue_stop_reason reason);
> +void ieee80211_stop_queues_by_reason(struct ieee80211_hw *hw,
> +				     enum queue_stop_reason reason);
> +
>  #ifdef CONFIG_MAC80211_NOINLINE
>  #define debug_noinline noinline
>  #else
> diff --git a/net/mac80211/main.c b/net/mac80211/main.c
> index a0371ca..7c6e90a 100644
> --- a/net/mac80211/main.c
> +++ b/net/mac80211/main.c
> @@ -724,6 +724,8 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
>  
>  	spin_lock_init(&local->key_lock);
>  
> +	spin_lock_init(&local->queue_stop_reason_lock);
> +
>  	INIT_DELAYED_WORK(&local->scan_work, ieee80211_scan_work);
>  
>  	sta_info_init(local);
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 71a8391..fb89e1d 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -330,10 +330,20 @@ __le16 ieee80211_ctstoself_duration(struct ieee80211_hw *hw,
>  }
>  EXPORT_SYMBOL(ieee80211_ctstoself_duration);
>  
> -void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
> +static void __ieee80211_wake_queue(struct ieee80211_hw *hw, int queue,
> +				   enum queue_stop_reason reason)
>  {
>  	struct ieee80211_local *local = hw_to_local(hw);
>  
> +	/* we don't need to track ampdu queues */
> +	if (queue < ieee80211_num_regular_queues(hw)) {
> +		__clear_bit(reason, &local->queue_stop_reasons[queue]);
> +
> +		if (local->queue_stop_reasons[queue] != 0)
> +			/* someone still has this queue stopped */
> +			return;
> +	}
> +
>  	if (test_bit(queue, local->queues_pending)) {
>  		set_bit(queue, local->queues_pending_run);
>  		tasklet_schedule(&local->tx_pending_tasklet);
> @@ -341,22 +351,74 @@ void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
>  		netif_wake_subqueue(local->mdev, queue);
>  	}
>  }
> +
> +void ieee80211_wake_queue_by_reason(struct ieee80211_hw *hw, int queue,
> +				    enum queue_stop_reason reason)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
> +	__ieee80211_wake_queue(hw, queue, reason);
> +	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
> +}
> +
> +void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
> +{
> +	ieee80211_wake_queue_by_reason(hw, queue,
> +				       IEEE80211_QUEUE_STOP_REASON_DRIVER);
> +}
>  EXPORT_SYMBOL(ieee80211_wake_queue);
>  
> -void ieee80211_stop_queue(struct ieee80211_hw *hw, int queue)
> +static void __ieee80211_stop_queue(struct ieee80211_hw *hw, int queue,
> +				   enum queue_stop_reason reason)
>  {
>  	struct ieee80211_local *local = hw_to_local(hw);
>  
> +	/* we don't need to track ampdu queues */
> +	if (queue < ieee80211_num_regular_queues(hw))
> +		__set_bit(reason, &local->queue_stop_reasons[queue]);
> +
>  	netif_stop_subqueue(local->mdev, queue);
>  }
> +
> +void ieee80211_stop_queue_by_reason(struct ieee80211_hw *hw, int queue,
> +				    enum queue_stop_reason reason)
> +{
> +	struct ieee80211_local *local = hw_to_local(hw);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
> +	__ieee80211_stop_queue(hw, queue, reason);
> +	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
> +}
> +
> +void ieee80211_stop_queue(struct ieee80211_hw *hw, int queue)
> +{
> +	ieee80211_stop_queue_by_reason(hw, queue,
> +				       IEEE80211_QUEUE_STOP_REASON_DRIVER);
> +}
>  EXPORT_SYMBOL(ieee80211_stop_queue);
>  
> -void ieee80211_stop_queues(struct ieee80211_hw *hw)
> +void ieee80211_stop_queues_by_reason(struct ieee80211_hw *hw,
> +				    enum queue_stop_reason reason)
>  {
> +	struct ieee80211_local *local = hw_to_local(hw);
> +	unsigned long flags;
>  	int i;
>  
> +	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
> +
>  	for (i = 0; i < ieee80211_num_queues(hw); i++)
> -		ieee80211_stop_queue(hw, i);
> +		__ieee80211_stop_queue(hw, i, reason);
> +
> +	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
> +}
> +
> +void ieee80211_stop_queues(struct ieee80211_hw *hw)
> +{
> +	ieee80211_stop_queues_by_reason(hw,
> +					IEEE80211_QUEUE_STOP_REASON_DRIVER);
>  }
>  EXPORT_SYMBOL(ieee80211_stop_queues);
>  
> @@ -367,12 +429,24 @@ int ieee80211_queue_stopped(struct ieee80211_hw *hw, int queue)
>  }
>  EXPORT_SYMBOL(ieee80211_queue_stopped);
>  
> -void ieee80211_wake_queues(struct ieee80211_hw *hw)
> +void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
> +				     enum queue_stop_reason reason)
>  {
> +	struct ieee80211_local *local = hw_to_local(hw);
> +	unsigned long flags;
>  	int i;
>  
> +	spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
> +
>  	for (i = 0; i < hw->queues + hw->ampdu_queues; i++)
> -		ieee80211_wake_queue(hw, i);
> +		__ieee80211_wake_queue(hw, i, reason);
> +
> +	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
> +}
> +
> +void ieee80211_wake_queues(struct ieee80211_hw *hw)
> +{
> +	ieee80211_wake_queues_by_reason(hw, IEEE80211_QUEUE_STOP_REASON_DRIVER);
>  }
>  EXPORT_SYMBOL(ieee80211_wake_queues);
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[PATCH v5 3/3] mac80211: implement dynamic power save

[Kalle Valo]

This patch implements dynamic power save for mac80211. Basically it
means enabling power save mode after an idle period. Implementing it
dynamically gives a good compromise of low power consumption and low
latency. Some hardware have support for this in firmware, but some
require the host to do it.

The dynamic power save is implemented by adding an timeout to
ieee80211_subif_start_xmit(). The timeout can be enabled from userspace
with Wireless Extensions. For example, the command below enables the
dynamic power save and sets the time timeout to 500 ms:

iwconfig wlan0 power timeout 500m

Power save now only works with devices which handle power save in firmware.
It's also disabled by default and the heuristics when and how to enable is
considered as a policy decision and will be left for the userspace to handle.
In case the firmware has support for this, drivers can disable this feature
with IEEE80211_HW_NO_STACK_DYNAMIC_PS.

Big thanks to Johannes Berg for the help with the design and code.

Signed-off-by: Kalle Valo <kalle.valo@nokia.com>
---

 include/net/mac80211.h     |    6 +++++
 net/mac80211/ieee80211_i.h |   10 +++++++++
 net/mac80211/main.c        |    7 ++++++
 net/mac80211/mlme.c        |   49 ++++++++++++++++++++++++++++++++++++++++++--
 net/mac80211/tx.c          |   13 ++++++++++++
 net/mac80211/wext.c        |   30 ++++++++++++++++++---------
 6 files changed, 103 insertions(+), 12 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 9428d3e..b3bd00a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -854,6 +854,11 @@ enum ieee80211_tkip_key_type {
  *
  * @IEEE80211_HW_AMPDU_AGGREGATION:
  *	Hardware supports 11n A-MPDU aggregation.
+ *
+ * @IEEE80211_HW_NO_STACK_DYNAMIC_PS:
+ *	Hardware which has dynamic power save support, meaning
+ *	that power save is enabled in idle periods, and don't need support
+ *	from stack.
  */
 enum ieee80211_hw_flags {
 	IEEE80211_HW_RX_INCLUDES_FCS			= 1<<1,
@@ -866,6 +871,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_NOISE_DBM				= 1<<8,
 	IEEE80211_HW_SPECTRUM_MGMT			= 1<<9,
 	IEEE80211_HW_AMPDU_AGGREGATION			= 1<<10,
+	IEEE80211_HW_NO_STACK_DYNAMIC_PS		= 1<<11,
 };
 
 /**
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a74d673..f3eec98 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -540,6 +540,7 @@ enum {
 
 enum queue_stop_reason {
 	IEEE80211_QUEUE_STOP_REASON_DRIVER,
+	IEEE80211_QUEUE_STOP_REASON_PS,
 };
 
 /* maximum number of hardware queues we support. */
@@ -693,7 +694,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 dynamic_ps_enable_work;
+	struct work_struct dynamic_ps_disable_work;
+	struct timer_list dynamic_ps_timer;
 
 #ifdef CONFIG_MAC80211_DEBUGFS
 	struct local_debugfsdentries {
@@ -977,6 +983,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_dynamic_ps_enable_work(struct work_struct *work);
+void ieee80211_dynamic_ps_disable_work(struct work_struct *work);
+void ieee80211_dynamic_ps_timer(unsigned long data);
+
 void ieee80211_wake_queues_by_reason(struct ieee80211_hw *hw,
 				     enum queue_stop_reason reason);
 void ieee80211_stop_queues_by_reason(struct ieee80211_hw *hw,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 7c6e90a..a4f413e 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -728,6 +728,13 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 
 	INIT_DELAYED_WORK(&local->scan_work, ieee80211_scan_work);
 
+	INIT_WORK(&local->dynamic_ps_enable_work,
+		  ieee80211_dynamic_ps_enable_work);
+	INIT_WORK(&local->dynamic_ps_disable_work,
+		  ieee80211_dynamic_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 96493e2..b63b934 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -747,8 +747,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);
@@ -872,6 +878,9 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
 	local->oper_channel_type = NL80211_CHAN_NO_HT;
 	config_changed |= IEEE80211_CONF_CHANGE_HT;
 
+	del_timer_sync(&local->dynamic_ps_timer);
+	cancel_work_sync(&local->dynamic_ps_enable_work);
+
 	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
 		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
 		config_changed |= IEEE80211_CONF_PS;
@@ -2620,3 +2629,39 @@ void ieee80211_mlme_notify_scan_completed(struct ieee80211_local *local)
 		ieee80211_restart_sta_timer(sdata);
 	rcu_read_unlock();
 }
+
+void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
+{
+	struct ieee80211_local *local =
+		container_of(work, struct ieee80211_local,
+			     dynamic_ps_disable_work);
+
+	if (local->hw.conf.flags & IEEE80211_CONF_PS) {
+		local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
+	}
+
+	ieee80211_wake_queues_by_reason(&local->hw,
+					IEEE80211_QUEUE_STOP_REASON_PS);
+}
+
+void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
+{
+	struct ieee80211_local *local =
+		container_of(work, struct ieee80211_local,
+			     dynamic_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_dynamic_ps_timer(unsigned long data)
+{
+	struct ieee80211_local *local = (void *) data;
+
+	queue_work(local->hw.workqueue, &local->dynamic_ps_enable_work);
+}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 22702e7..27474cf 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1475,6 +1475,19 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
 		goto fail;
 	}
 
+	if (!(local->hw.flags & IEEE80211_HW_NO_STACK_DYNAMIC_PS) &&
+	    local->dynamic_ps_timeout > 0) {
+		if (local->hw.conf.flags & IEEE80211_CONF_PS) {
+			ieee80211_stop_queues_by_reason(&local->hw,
+							IEEE80211_QUEUE_STOP_REASON_PS);
+			queue_work(local->hw.workqueue,
+				   &local->dynamic_ps_disable_work);
+		}
+
+		mod_timer(&local->dynamic_ps_timer, jiffies +
+			  msecs_to_jiffies(local->dynamic_ps_timeout));
+	}
+
 	nh_pos = skb_network_header(skb) - skb->data;
 	h_pos = skb_transport_header(skb) - skb->data;
 
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index f6640d0..7162d58 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -833,7 +833,7 @@ static int ieee80211_ioctl_siwpower(struct net_device *dev,
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_conf *conf = &local->hw.conf;
-	int ret = 0;
+	int ret = 0, timeout = 0;
 	bool ps;
 
 	if (sdata->vif.type != NL80211_IFTYPE_STATION)
@@ -841,6 +841,7 @@ static int ieee80211_ioctl_siwpower(struct net_device *dev,
 
 	if (wrq->disabled) {
 		ps = false;
+		timeout = 0;
 		goto set;
 	}
 
@@ -850,22 +851,31 @@ 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 (ps == local->powersave)
-		return ret;
+	if (wrq->flags & IW_POWER_TIMEOUT)
+		timeout = wrq->value / 1000;
 
 set:
+	if (ps == local->powersave && timeout == local->dynamic_ps_timeout)
+		return ret;
+
 	local->powersave = ps;
+	local->dynamic_ps_timeout = timeout;
 
 	if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
-		if (local->powersave)
-			conf->flags |= IEEE80211_CONF_PS;
-		else
-			conf->flags &= ~IEEE80211_CONF_PS;
-
+		if (!(local->hw.flags & IEEE80211_HW_NO_STACK_DYNAMIC_PS) &&
+		    local->dynamic_ps_timeout > 0)
+			mod_timer(&local->dynamic_ps_timer, jiffies +
+				  msecs_to_jiffies(local->dynamic_ps_timeout));
+		else {
+			if (local->powersave)
+				conf->flags |= IEEE80211_CONF_PS;
+			else
+				conf->flags &= ~IEEE80211_CONF_PS;
+		}
 		ret = ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
 	}
 

Re: [PATCH v5 3/3] mac80211: implement dynamic power save

[Johannes Berg]

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

On Thu, 2008-12-18 at 23:17 +0200, Kalle Valo wrote:
> This patch implements dynamic power save for mac80211. Basically it
> means enabling power save mode after an idle period. Implementing it
> dynamically gives a good compromise of low power consumption and low
> latency. Some hardware have support for this in firmware, but some
> require the host to do it.
> 
> The dynamic power save is implemented by adding an timeout to
> ieee80211_subif_start_xmit(). The timeout can be enabled from userspace
> with Wireless Extensions. For example, the command below enables the
> dynamic power save and sets the time timeout to 500 ms:
> 
> iwconfig wlan0 power timeout 500m
> 
> Power save now only works with devices which handle power save in firmware.
> It's also disabled by default and the heuristics when and how to enable is
> considered as a policy decision and will be left for the userspace to handle.
> In case the firmware has support for this, drivers can disable this feature
> with IEEE80211_HW_NO_STACK_DYNAMIC_PS.
> 
> Big thanks to Johannes Berg for the help with the design and code.
> 
> Signed-off-by: Kalle Valo <kalle.valo@nokia.com>

Looks good to me.

Acked-by: Johannes Berg <johannes@sipsolutions.net>

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

Re: [PATCH v5 3/3] mac80211: implement dynamic power save

[Matthew Garrett]

Are we able to estimate the worst-case latency that will be introduced 
by this? If so, it would be helpful to tie it into the pm_qos framework.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v5 3/3] mac80211: implement dynamic power save

[Kalle Valo]

Matthew Garrett <mjg59@srcf.ucam.org> writes:

> Are we able to estimate the worst-case latency that will be introduced 
> by this? If so, it would be helpful to tie it into the pm_qos framework.

I'm not familiar with pm_qos framework, unfortunately, but there's
some work to do still to get mac80211 client power save implemention
into shape and I would like get the basics implemented first. After
that is done, we could start looking at the pm_qos framework and see
if it makes sense to use it.

-- 
Kalle Valo

Re: [PATCH v5 3/3] mac80211: implement dynamic power save

[Matthew Garrett]

On Sun, Dec 28, 2008 at 02:59:35PM +0200, Kalle Valo wrote:
> Matthew Garrett <mjg59@srcf.ucam.org> writes:
> 
> > Are we able to estimate the worst-case latency that will be introduced 
> > by this? If so, it would be helpful to tie it into the pm_qos framework.
> 
> I'm not familiar with pm_qos framework, unfortunately, but there's
> some work to do still to get mac80211 client power save implemention
> into shape and I would like get the basics implemented first. After
> that is done, we could start looking at the pm_qos framework and see
> if it makes sense to use it.

It allows userspace applications to register their latency requirements. 
There's a notifier chain that then allows drivers (or subsystems) to be 
notified of the current requirements, the idea being that they can then 
limit their deepest power saving mode to one that still satisfies the 
userspace constraints. But yeah, it's not really worth worrying about 
until the code works :)

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH v5 3/3] mac80211: implement dynamic power save

[Kalle Valo]

Matthew Garrett <mjg59@srcf.ucam.org> writes:

> On Sun, Dec 28, 2008 at 02:59:35PM +0200, Kalle Valo wrote:
>> Matthew Garrett <mjg59@srcf.ucam.org> writes:
>> 
>> > Are we able to estimate the worst-case latency that will be introduced 
>> > by this? If so, it would be helpful to tie it into the pm_qos framework.
>> 
>> I'm not familiar with pm_qos framework, unfortunately, but there's
>> some work to do still to get mac80211 client power save implemention
>> into shape and I would like get the basics implemented first. After
>> that is done, we could start looking at the pm_qos framework and see
>> if it makes sense to use it.
>
> It allows userspace applications to register their latency requirements. 
> There's a notifier chain that then allows drivers (or subsystems) to be 
> notified of the current requirements, the idea being that they can then 
> limit their deepest power saving mode to one that still satisfies the 
> userspace constraints.

Do these constrainsts also include network latency? Do you have any
suggestions for bedtime reading? :)

-- 
Kalle Valo

Re: [PATCH v5 3/3] mac80211: implement dynamic power save

[Matthew Garrett]

On Sun, Dec 28, 2008 at 03:13:32PM +0200, Kalle Valo wrote:
> Matthew Garrett <mjg59@srcf.ucam.org> writes:
> > It allows userspace applications to register their latency requirements. 
> > There's a notifier chain that then allows drivers (or subsystems) to be 
> > notified of the current requirements, the idea being that they can then 
> > limit their deepest power saving mode to one that still satisfies the 
> > userspace constraints.
> 
> Do these constrainsts also include network latency? Do you have any
> suggestions for bedtime reading? :)

Yes, one of the chains is network latency. It's documented in 
Documentation/power/pm_qos_interface.txt.

-- 
Matthew Garrett | mjg59@srcf.ucam.org