Here is v4 of my dynamic powersave patches. I have tested them only
with stlc45xx on Nokia N810. I'm also working with iwl3945 and iwlagn
patches based on these patches, but I haven't finished them yet.
These are RFC patches, not yet to be applied.
Patch 3 contains a new flag IEEE80211_HW_NO_DYNAMIC_PS. So all drivers
which firmware doesn't support dynamic power save should use this flag
to enable the feature in mac80211. This is a separate patch just for
easier review. If it's ok, I'll fold it to patch 2.
Patch 4 moves the dynamic_ps_timeout to ieee80211_conf struct. That
way the drivers which have dynamic power save support in firmware can
use the same timeout provided by user space. If this ok I'll fold this
to patch 2.
Patch 5 contains internal interface mac80211 to enable and disable
queues. The original idea and design is from Johannes Berg, I just did
the implementation based on those. All the bugs are mine, of course.
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
TODO:
o test with iwlwifi (only in-tree driver using IEEE80211_CONF_PS)
Power save related features which I'll implement later:
o disable power save when software scanning
o beacon period (wakeup period) to the drivers, can be set from user
space but otherwise is bss's DTIM value
---
Kalle Valo (5):
mac80211: track master queue status
mac80211: move dynamic_ps_timeout to hw.conf
mac80211: add IEEE80211_HW_NO_DYNAMIC_PS flag
mac80211: implement dynamic power save
mac80211: enable IEEE80211_CONF_PS only when associated
include/net/mac80211.h | 10 +++++++
net/mac80211/ieee80211_i.h | 22 +++++++++++++++-
net/mac80211/main.c | 9 +++++++
net/mac80211/mlme.c | 60 +++++++++++++++++++++++++++++++++++++++++++-
net/mac80211/tx.c | 13 ++++++++++
net/mac80211/util.c | 60 ++++++++++++++++++++++++++++++++++++++++----
net/mac80211/wext.c | 46 ++++++++++++++++++++++++++++------
7 files changed, 203 insertions(+), 17 deletions(-)
Signed-off-by: Kalle Valo <[email protected]>
---
include/net/mac80211.h | 5 +++++
net/mac80211/ieee80211_i.h | 1 -
net/mac80211/mlme.c | 4 ++--
net/mac80211/tx.c | 4 ++--
net/mac80211/wext.c | 8 ++++----
5 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 89affd8..047b20d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -546,6 +546,10 @@ enum ieee80211_conf_changed {
* @power_level: requested transmit power (in dBm)
* @channel: the channel to tune to
* @ht: the HT configuration for the device
+ * @dynamic_ps_timeout: Length of the tranmission idle period after the
+ * power save mode should be enabled. The timeout should be enabled only
+ * when %IEEE80211_CONF_PS is enabled. If zero, there should be no timeout.
+ * Unit is millisecond.
* @long_frame_max_tx_count: Maximum number of transmissions for a "long" frame
* (a frame not RTS protected), called "dot11LongRetryLimit" in 802.11,
* but actually means the number of transmissions not the number of retries
@@ -560,6 +564,7 @@ struct ieee80211_conf {
u16 listen_interval;
bool radio_enabled;
+ int dynamic_ps_timeout;
u8 long_frame_max_tx_count, short_frame_max_tx_count;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 83b4b08..f03261b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -690,7 +690,6 @@ struct ieee80211_local {
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;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index ee71b12..add3553 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -747,9 +747,9 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
ieee80211_bss_info_change_notify(sdata, bss_info_changed);
if (local->powersave) {
- if (local->dynamic_ps_timeout > 0)
+ if (local->hw.conf.dynamic_ps_timeout > 0)
mod_timer(&local->dynamic_ps_timer, jiffies +
- msecs_to_jiffies(local->dynamic_ps_timeout));
+ msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
else {
conf->flags |= IEEE80211_CONF_PS;
ieee80211_hw_config(local,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 9444a06..de3283b 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1477,7 +1477,7 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
}
if ((local->hw.flags & IEEE80211_HW_NO_DYNAMIC_PS) &&
- local->dynamic_ps_timeout > 0) {
+ local->hw.conf.dynamic_ps_timeout > 0) {
if (local->hw.conf.flags & IEEE80211_CONF_PS) {
netif_tx_stop_all_queues(local->mdev);
queue_work(local->hw.workqueue,
@@ -1485,7 +1485,7 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
}
mod_timer(&local->dynamic_ps_timer, jiffies +
- msecs_to_jiffies(local->dynamic_ps_timeout));
+ msecs_to_jiffies(local->hw.conf.dynamic_ps_timeout));
}
nh_pos = skb_network_header(skb) - skb->data;
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index a666ced..c4319cc 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -979,20 +979,20 @@ static int ieee80211_ioctl_siwpower(struct net_device *dev,
timeout = wrq->value / 1000;
set:
- if (ps == local->powersave && timeout == local->dynamic_ps_timeout)
+ if (ps == local->powersave && timeout == conf->dynamic_ps_timeout)
return ret;
local->powersave = ps;
- local->dynamic_ps_timeout = timeout;
+ conf->dynamic_ps_timeout = timeout;
if (sdata->vif.type != NL80211_IFTYPE_STATION)
return ret;
if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
if ((local->hw.flags & IEEE80211_HW_NO_DYNAMIC_PS) &&
- local->dynamic_ps_timeout > 0)
+ conf->dynamic_ps_timeout > 0)
mod_timer(&local->dynamic_ps_timer, jiffies +
- msecs_to_jiffies(local->dynamic_ps_timeout));
+ msecs_to_jiffies(conf->dynamic_ps_timeout));
else {
if (local->powersave)
conf->flags |= IEEE80211_CONF_PS;
Also disable power save when disassociated.
Signed-off-by: Kalle Valo <[email protected]>
---
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 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 88b0b4d..e7ade66 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();
@@ -868,8 +873,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 b3ce28d..b095697 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -953,25 +953,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 (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->vif.type != NL80211_IFTYPE_STATION)
+ return ret;
+
+ 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,
@@ -980,9 +1001,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;
}
On Tue, 2008-12-02 at 22:04 +0200, Kalle Valo wrote:
> +void __ieee80211_stop_queues(struct ieee80211_hw *hw,
> + enum queue_stop_reason reason)
> {
> int i;
>
> for (i = 0; i < ieee80211_num_queues(hw); i++)
> - ieee80211_stop_queue(hw, i);
> + __ieee80211_stop_queue(hw, i, reason);
> +}
> +
> +void ieee80211_stop_queues(struct ieee80211_hw *hw)
> +{
> + __ieee80211_stop_queues(hw, IEEE80211_QUEUE_STOP_REASON_DRIVER);
> }
> EXPORT_SYMBOL(ieee80211_stop_queues);
>
> @@ -367,12 +409,18 @@ 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(struct ieee80211_hw *hw,
> + enum queue_stop_reason reason)
> {
> int i;
>
> for (i = 0; i < hw->queues + hw->ampdu_queues; i++)
> - ieee80211_wake_queue(hw, i);
> + __ieee80211_wake_queue(hw, i, reason);
> +}
> +
> +void ieee80211_wake_queues(struct ieee80211_hw *hw)
> +{
> + __ieee80211_wake_queues(hw, IEEE80211_QUEUE_STOP_REASON_DRIVER);
> }
> EXPORT_SYMBOL(ieee80211_wake_queues);
I wonder if we should optimise the spinlock here?
Also, I'd prefer to first add this with just a single DRIVER reason
first, and then add the PS patches that add and use the second reason.
Other than that, it looks good to me, thanks!
johannes
Johannes Berg <[email protected]> writes:
> On Tue, 2008-12-02 at 22:03 +0200, Kalle Valo wrote:
>
>> - return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
>> + if (ps == local->powersave)
>> + return ret;
>> +
>> +set:
>> + local->powersave = ps;
>> +
>> + if (sdata->vif.type != NL80211_IFTYPE_STATION)
>> + return ret;
>
> Shouldn't that test be earlier? There's little sense in supporting PS if
> you have a non-STA interface.
I'll move the test.
> In fact, it might make sense to check that there's no other
> interface (except monitor) open?
I'll add this check as well.
Thank you for the comments.
--
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.
Thanks to Johannes Berg for the help with the design.
Signed-off-by: Kalle Valo <[email protected]>
---
net/mac80211/ieee80211_i.h | 9 ++++++++
net/mac80211/main.c | 7 ++++++
net/mac80211/mlme.c | 49 ++++++++++++++++++++++++++++++++++++++++++--
net/mac80211/tx.c | 11 ++++++++++
net/mac80211/wext.c | 29 +++++++++++++++++---------
5 files changed, 93 insertions(+), 12 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 3f25955..83b4b08 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 dynamic_ps_enable_work;
+ struct work_struct dynamic_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_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);
+
#ifdef CONFIG_MAC80211_NOINLINE
#define debug_noinline noinline
#else
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index cec9b6d..bebdd48 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -701,6 +701,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 e7ade66..ee71b12 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);
@@ -875,6 +881,9 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
local->hw.conf.ht.enabled = false;
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;
@@ -2608,3 +2617,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);
+ }
+
+ /* always wake queues to avoid having them stopped forever */
+ netif_tx_wake_all_queues(local->mdev);
+}
+
+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 0855cac..60cbc28 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1476,6 +1476,17 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
goto fail;
}
+ if (local->dynamic_ps_timeout > 0) {
+ if (local->hw.conf.flags & IEEE80211_CONF_PS) {
+ netif_tx_stop_all_queues(local->mdev);
+ 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 b095697..8815858 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -956,11 +956,12 @@ 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 (wrq->disabled) {
ps = false;
+ timeout = 0;
goto set;
}
@@ -970,25 +971,33 @@ 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->vif.type != NL80211_IFTYPE_STATION)
return ret;
if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
- if (local->powersave)
- conf->flags |= IEEE80211_CONF_PS;
- else
- conf->flags &= ~IEEE80211_CONF_PS;
-
+ if (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);
}
There are now two paths to stop the master queues and we need to track this
properly to avoid starting queue in the wrong time. Implement this by adding
an 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 <[email protected]>
---
net/mac80211/ieee80211_i.h | 13 +++++++++-
net/mac80211/main.c | 2 +
net/mac80211/mlme.c | 2 +
net/mac80211/tx.c | 3 +-
net/mac80211/util.c | 60 ++++++++++++++++++++++++++++++++++++++++----
5 files changed, 71 insertions(+), 9 deletions(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index f03261b..c2900ee 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -539,6 +539,11 @@ enum {
IEEE80211_ADDBA_MSG = 4,
};
+enum queue_stop_reason {
+ IEEE80211_QUEUE_STOP_REASON_PS,
+ IEEE80211_QUEUE_STOP_REASON_DRIVER,
+};
+
/* maximum number of hardware queues we support. */
#define QD_MAX_QUEUES (IEEE80211_MAX_AMPDU_QUEUES + IEEE80211_MAX_QUEUES)
@@ -555,7 +560,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;
@@ -981,6 +987,11 @@ 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_stop_queues(struct ieee80211_hw *hw,
+ enum queue_stop_reason reason);
+void __ieee80211_wake_queues(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 bebdd48..db8882b 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -699,6 +699,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);
INIT_WORK(&local->dynamic_ps_enable_work,
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index add3553..0e2db93 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2630,7 +2630,7 @@ void ieee80211_dynamic_ps_disable_work(struct work_struct *work)
}
/* always wake queues to avoid having them stopped forever */
- netif_tx_wake_all_queues(local->mdev);
+ __ieee80211_wake_queues(&local->hw, IEEE80211_QUEUE_STOP_REASON_PS);
}
void ieee80211_dynamic_ps_enable_work(struct work_struct *work)
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index de3283b..809c4f7 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1479,7 +1479,8 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
if ((local->hw.flags & IEEE80211_HW_NO_DYNAMIC_PS) &&
local->hw.conf.dynamic_ps_timeout > 0) {
if (local->hw.conf.flags & IEEE80211_CONF_PS) {
- netif_tx_stop_all_queues(local->mdev);
+ __ieee80211_stop_queues(&local->hw,
+ IEEE80211_QUEUE_STOP_REASON_PS);
queue_work(local->hw.workqueue,
&local->dynamic_ps_disable_work);
}
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 0f84131..1224dc0 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -330,9 +330,22 @@ __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);
+ unsigned long flags;
+
+ spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+
+ /* 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 */
+ goto out;
+ }
if (test_bit(queue, local->queues_pending)) {
set_bit(queue, local->queues_pending_run);
@@ -340,23 +353,52 @@ void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
} else {
netif_wake_subqueue(local->mdev, queue);
}
+
+out:
+ spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+}
+
+void ieee80211_wake_queue(struct ieee80211_hw *hw, int queue)
+{
+ __ieee80211_wake_queue(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);
+ unsigned long flags;
+
+ spin_lock_irqsave(&local->queue_stop_reason_lock, flags);
+
+ /* 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);
+
+ spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
+}
+
+void ieee80211_stop_queue(struct ieee80211_hw *hw, int queue)
+{
+ __ieee80211_stop_queue(hw, queue, IEEE80211_QUEUE_STOP_REASON_DRIVER);
}
EXPORT_SYMBOL(ieee80211_stop_queue);
-void ieee80211_stop_queues(struct ieee80211_hw *hw)
+void __ieee80211_stop_queues(struct ieee80211_hw *hw,
+ enum queue_stop_reason reason)
{
int i;
for (i = 0; i < ieee80211_num_queues(hw); i++)
- ieee80211_stop_queue(hw, i);
+ __ieee80211_stop_queue(hw, i, reason);
+}
+
+void ieee80211_stop_queues(struct ieee80211_hw *hw)
+{
+ __ieee80211_stop_queues(hw, IEEE80211_QUEUE_STOP_REASON_DRIVER);
}
EXPORT_SYMBOL(ieee80211_stop_queues);
@@ -367,12 +409,18 @@ 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(struct ieee80211_hw *hw,
+ enum queue_stop_reason reason)
{
int i;
for (i = 0; i < hw->queues + hw->ampdu_queues; i++)
- ieee80211_wake_queue(hw, i);
+ __ieee80211_wake_queue(hw, i, reason);
+}
+
+void ieee80211_wake_queues(struct ieee80211_hw *hw)
+{
+ __ieee80211_wake_queues(hw, IEEE80211_QUEUE_STOP_REASON_DRIVER);
}
EXPORT_SYMBOL(ieee80211_wake_queues);
Signed-off-by: Kalle Valo <[email protected]>
---
include/net/mac80211.h | 5 +++++
net/mac80211/tx.c | 3 ++-
net/mac80211/wext.c | 3 ++-
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6a1d4ea..89affd8 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -846,6 +846,10 @@ enum ieee80211_tkip_key_type {
*
* @IEEE80211_HW_AMPDU_AGGREGATION:
* Hardware supports 11n A-MPDU aggregation.
+ *
+ * @IEEE80211_HW_NO_DYNAMIC_PS:
+ * Hardware which don't have dynamic power save support, meaning
+ * that power save is enabled in idle periods.
*/
enum ieee80211_hw_flags {
IEEE80211_HW_RX_INCLUDES_FCS = 1<<1,
@@ -858,6 +862,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_DYNAMIC_PS = 1<<11,
};
/**
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 60cbc28..9444a06 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1476,7 +1476,8 @@ int ieee80211_subif_start_xmit(struct sk_buff *skb,
goto fail;
}
- if (local->dynamic_ps_timeout > 0) {
+ if ((local->hw.flags & IEEE80211_HW_NO_DYNAMIC_PS) &&
+ local->dynamic_ps_timeout > 0) {
if (local->hw.conf.flags & IEEE80211_CONF_PS) {
netif_tx_stop_all_queues(local->mdev);
queue_work(local->hw.workqueue,
diff --git a/net/mac80211/wext.c b/net/mac80211/wext.c
index 8815858..a666ced 100644
--- a/net/mac80211/wext.c
+++ b/net/mac80211/wext.c
@@ -989,7 +989,8 @@ set:
return ret;
if (sdata->u.sta.flags & IEEE80211_STA_ASSOCIATED) {
- if (local->dynamic_ps_timeout > 0)
+ if ((local->hw.flags & IEEE80211_HW_NO_DYNAMIC_PS) &&
+ local->dynamic_ps_timeout > 0)
mod_timer(&local->dynamic_ps_timer, jiffies +
msecs_to_jiffies(local->dynamic_ps_timeout));
else {
On Fri, 2008-12-05 at 07:51 +0200, Kalle Valo wrote:
> Johannes Berg <[email protected]> writes:
>
> > I wonder if we should optimise the spinlock here?
>
> That would be nice, but how to do that?
I only meant to lock/unlock it just once instead of for each possible
queue.
johannes
Johannes Berg <[email protected]> writes:
> On Fri, 2008-12-05 at 07:51 +0200, Kalle Valo wrote:
>> Johannes Berg <[email protected]> writes:
>>
>> > I wonder if we should optimise the spinlock here?
>>
>> That would be nice, but how to do that?
>
> I only meant to lock/unlock it just once instead of for each possible
> queue.
Ah, of course, stupid of me. I'll fix that.
--
Kalle Valo
Johannes Berg <[email protected]> writes:
> I wonder if we should optimise the spinlock here?
That would be nice, but how to do that?
> Also, I'd prefer to first add this with just a single DRIVER reason
> first, and then add the PS patches that add and use the second reason.
Yes, that's clean way to do it. I'll change it.
> Other than that, it looks good to me, thanks!
Thank you for all the help with this, much appreciated.
I'll send v5 of my patches later this week.
--
Kalle Valo
On Tue, 2008-12-02 at 22:03 +0200, Kalle Valo wrote:
> - return ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
> + if (ps == local->powersave)
> + return ret;
> +
> +set:
> + local->powersave = ps;
> +
> + if (sdata->vif.type != NL80211_IFTYPE_STATION)
> + return ret;
Shouldn't that test be earlier? There's little sense in supporting PS if
you have a non-STA interface. In fact, it might make sense to check that
there's no other interface (except monitor) open?
johannes
On Fri, 2008-12-05 at 07:47 +0200, Kalle Valo wrote:
> > In fact, it might make sense to check that there's no other
> > interface (except monitor) open?
>
> I'll add this check as well.
Actually, come to think of it, this doesn't make sense unless we also
add a check into the interface start() and then disable PS when another
interface was brought up, let's just leave it at "only on STA
interfaces" and if you manage to screw it up then tough luck.
johannes