[RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Ivo van Doorn]

Add a delayed work structure which can be used to wakeup the device
just before a beacon from the AP is expected. This then allows the device
to receive the beacon, check if there are pending broadcast or multicast frames.

TODO: Split out mac80211 changes into a separate patch. We also need
to know if we need this check in mac80211 or in the driver. Personally I think the
check belongs in mac80211, but at this time that work has been deferred to the drivers.
However this means that a lot of logic is needed in the driver to check if the correct IE
is available, and then check the value, while mac80211 will obtains that exact IE anyway
during RX processing anyway...

I'm not familliar enough with the Powersaving mode in mac80211 to be sure this current
approach is completely correct, so please review carefully :)

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 5d91561..7b169c8 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -561,6 +561,7 @@ static int rt2800usb_probe_hw(struct rt2x00_dev *rt2x00dev)
 	 */
 	__set_bit(DRIVER_REQUIRE_FIRMWARE, &rt2x00dev->flags);
 	__set_bit(DRIVER_REQUIRE_L2PAD, &rt2x00dev->flags);
+	__set_bit(DRIVER_REQUIRE_PS_AUTOWAKE, &rt2x00dev->flags);
 	if (!modparam_nohwcrypt)
 		__set_bit(CONFIG_SUPPORT_HW_CRYPTO, &rt2x00dev->flags);
 	__set_bit(DRIVER_SUPPORT_LINK_TUNING, &rt2x00dev->flags);
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 39bc2fa..fd4fe33 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -659,6 +659,7 @@ enum rt2x00_flags {
 	DRIVER_REQUIRE_L2PAD,
 	DRIVER_REQUIRE_TXSTATUS_FIFO,
 	DRIVER_REQUIRE_TASKLET_CONTEXT,
+	DRIVER_REQUIRE_PS_AUTOWAKE,
 
 	/*
 	 * Driver features
@@ -882,6 +883,11 @@ struct rt2x00_dev {
 	struct work_struct txdone_work;
 
 	/*
+	 * Powersaving work
+	 */
+	struct delayed_work autowakeup_work;
+
+	/*
 	 * Data queue arrays for RX, TX and Beacon.
 	 * The Beacon array also contains the Atim queue
 	 * if that is supported by the device.
diff --git a/drivers/net/wireless/rt2x00/rt2x00config.c b/drivers/net/wireless/rt2x00/rt2x00config.c
index e7f67d5..efc8496 100644
--- a/drivers/net/wireless/rt2x00/rt2x00config.c
+++ b/drivers/net/wireless/rt2x00/rt2x00config.c
@@ -192,6 +192,11 @@ void rt2x00lib_config(struct rt2x00_dev *rt2x00dev,
 		       sizeof(libconf.channel));
 	}
 
+	if (test_bit(DRIVER_REQUIRE_PS_AUTOWAKE, &rt2x00dev->flags) &&
+	    (ieee80211_flags & IEEE80211_CONF_CHANGE_PS) &&
+	    !(conf->flags & IEEE80211_CONF_PS))
+		cancel_delayed_work_sync(&rt2x00dev->autowakeup_work);	
+
 	/*
 	 * Start configuration.
 	 */
@@ -204,6 +209,13 @@ void rt2x00lib_config(struct rt2x00_dev *rt2x00dev,
 	if (ieee80211_flags & IEEE80211_CONF_CHANGE_CHANNEL)
 		rt2x00link_reset_tuner(rt2x00dev, false);
 
+	if (test_bit(DRIVER_REQUIRE_PS_AUTOWAKE, &rt2x00dev->flags) &&
+	    (ieee80211_flags & IEEE80211_CONF_CHANGE_PS) &&
+	    (conf->flags & IEEE80211_CONF_PS))
+		queue_delayed_work(rt2x00dev->workqueue,
+				   &rt2x00dev->autowakeup_work,
+				   msecs_to_jiffies(rt2x00dev->beacon_int) - 1);
+
 	rt2x00dev->curr_band = conf->channel->band;
 	rt2x00dev->curr_freq = conf->channel->center_freq;
 	rt2x00dev->tx_power = conf->power_level;
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index d91afbb..17022bf 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -138,6 +138,16 @@ static void rt2x00lib_intf_scheduled(struct work_struct *work)
 					    rt2x00dev);
 }
 
+static void rt2x00lib_autowakeup(struct work_struct *work)
+{
+	struct rt2x00_dev *rt2x00dev =
+	    container_of(work, struct rt2x00_dev, autowakeup_work.work);
+
+	if (rt2x00dev->ops->lib->set_device_state(rt2x00dev, STATE_AWAKE)) {
+		ERROR(rt2x00dev, "Device failed to wakeup.\n");
+	}
+}
+
 /*
  * Interrupt context handlers.
  */
@@ -1004,6 +1014,7 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
 	}
 
 	INIT_WORK(&rt2x00dev->intf_work, rt2x00lib_intf_scheduled);
+	INIT_DELAYED_WORK(&rt2x00dev->autowakeup_work, rt2x00lib_autowakeup);
 
 	/*
 	 * Let the driver probe the device to detect the capabilities.
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 45fbb9e..9035f33 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1616,6 +1616,7 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 	struct ieee80211_local *local = sdata->local;
 	u32 changed = 0;
 	bool erp_valid, directed_tim = false;
+	bool ps_buffered = false;
 	u8 erp_value = 0;
 	u32 ncrc;
 	u8 *bssid;
@@ -1712,6 +1713,20 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata,
 		directed_tim = ieee80211_check_tim(elems.tim, elems.tim_len,
 						   ifmgd->aid);
 
+	if (likely(elems.tim && elems.tim_len >= sizeof(*elems.tim))) {
+		ps_buffered = !!(elems.tim->bitmap_ctrl & 0x01);
+
+		if (ps_buffered && local->hw.conf.flags & IEEE80211_CONF_PS) {
+			local->hw.conf.flags &= ~IEEE80211_CONF_PS;
+			ieee80211_hw_config(local,
+					    IEEE80211_CONF_CHANGE_PS);
+		} else if (!(local->hw.conf.flags & IEEE80211_CONF_PS)) {
+			local->hw.conf.flags |= IEEE80211_CONF_PS;
+			ieee80211_hw_config(local,
+					    IEEE80211_CONF_CHANGE_PS);
+		}
+	}
+
 	if (ncrc != ifmgd->beacon_crc || !ifmgd->beacon_crc_valid) {
 		ieee80211_rx_bss_info(sdata, mgmt, len, rx_status, &elems,
 				      true);

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Berg]

On Mon, 2011-01-31 at 16:00 +0100, Ivo van Doorn wrote:
> Add a delayed work structure which can be used to wakeup the device
> just before a beacon from the AP is expected. This then allows the device
> to receive the beacon, check if there are pending broadcast or multicast frames.
> 
> TODO: Split out mac80211 changes into a separate patch. We also need
> to know if we need this check in mac80211 or in the driver. Personally I think the
> check belongs in mac80211, but at this time that work has been deferred to the drivers.
> However this means that a lot of logic is needed in the driver to check if the correct IE
> is available, and then check the value, while mac80211 will obtains that exact IE anyway
> during RX processing anyway...

Hmm. I've always said that there's no efficient way to do this in
mac80211. If you do it close to the beacon, unexpected latency means
that we miss -- this can be severe if something else is happening in the
system, especially with USB devices. If you do it early before the
beacon, then you don't save much power. As a consequence, I don't really
like this in mac80211.

However, it seems you only added "stay awake after beacon" code to
mac80211 that checks the TIM. Assuming the device actually stays awake
for multicast traffic by itself that seems like an option, though I
wouldn't wait for the next beacon before going to sleep again --
wouldn't that happen with the timer anyway?

johannes

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Ivo Van Doorn]

Hi,

>> Add a delayed work structure which can be used to wakeup the device
>> just before a beacon from the AP is expected. This then allows the device
>> to receive the beacon, check if there are pending broadcast or multicast frames.
>>
>> TODO: Split out mac80211 changes into a separate patch. We also need
>> to know if we need this check in mac80211 or in the driver. Personally I think the
>> check belongs in mac80211, but at this time that work has been deferred to the drivers.
>> However this means that a lot of logic is needed in the driver to check if the correct IE
>> is available, and then check the value, while mac80211 will obtains that exact IE anyway
>> during RX processing anyway...
>
> Hmm. I've always said that there's no efficient way to do this in
> mac80211. If you do it close to the beacon, unexpected latency means
> that we miss -- this can be severe if something else is happening in the
> system, especially with USB devices. If you do it early before the
> beacon, then you don't save much power. As a consequence, I don't really
> like this in mac80211.
>
> However, it seems you only added "stay awake after beacon" code to
> mac80211 that checks the TIM. Assuming the device actually stays awake
> for multicast traffic by itself that seems like an option, though I
> wouldn't wait for the next beacon before going to sleep again --
> wouldn't that happen with the timer anyway?

The timer is needed to automatically wake the device up in time to receive
the beacon. When the beacon is received all the driver needs to know if the
device should go back to sleep again for the next beacon period.

This is a similar behavior as carl9170 driver, which currently checks for
pending Mcast/Bcast frames in the RX path just before the frame is send
to mac80211.

Ivo

Re: [rt2x00-users] [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Stezenbach]

On Mon, Jan 31, 2011 at 04:38:16PM +0100, Ivo Van Doorn wrote:
> >>
> >> TODO: Split out mac80211 changes into a separate patch. We also need
> >> to know if we need this check in mac80211 or in the driver. Personally I think the
> >> check belongs in mac80211, but at this time that work has been deferred to the drivers.
> >> However this means that a lot of logic is needed in the driver to check if the correct IE
> >> is available, and then check the value, while mac80211 will obtains that exact IE anyway
> >> during RX processing anyway...
> >
> > Hmm. I've always said that there's no efficient way to do this in
> > mac80211. If you do it close to the beacon, unexpected latency means
> > that we miss -- this can be severe if something else is happening in the
> > system, especially with USB devices. If you do it early before the
> > beacon, then you don't save much power. As a consequence, I don't really
> > like this in mac80211.
> >
> > However, it seems you only added "stay awake after beacon" code to
> > mac80211 that checks the TIM. Assuming the device actually stays awake
> > for multicast traffic by itself that seems like an option, though I
> > wouldn't wait for the next beacon before going to sleep again --
> > wouldn't that happen with the timer anyway?
> 
> The timer is needed to automatically wake the device up in time to receive
> the beacon. When the beacon is received all the driver needs to know if the
> device should go back to sleep again for the next beacon period.
> 
> This is a similar behavior as carl9170 driver, which currently checks for
> pending Mcast/Bcast frames in the RX path just before the frame is send
> to mac80211.

FWIW, this bc/mc check is also on the mac80211 TODO list:
http://wireless.kernel.org/en/developers/todo-list#power_saving


Johannes

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Stezenbach]

Hi,

On Mon, Jan 31, 2011 at 04:00:38PM +0100, Ivo van Doorn wrote:
> --- a/drivers/net/wireless/rt2x00/rt2x00config.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00config.c
> @@ -192,6 +192,11 @@ void rt2x00lib_config(struct rt2x00_dev *rt2x00dev,
>  		       sizeof(libconf.channel));
>  	}
>  
> +	if (test_bit(DRIVER_REQUIRE_PS_AUTOWAKE, &rt2x00dev->flags) &&
> +	    (ieee80211_flags & IEEE80211_CONF_CHANGE_PS) &&
> +	    !(conf->flags & IEEE80211_CONF_PS))
> +		cancel_delayed_work_sync(&rt2x00dev->autowakeup_work);	
> +
>  	/*
>  	 * Start configuration.
>  	 */
> @@ -204,6 +209,13 @@ void rt2x00lib_config(struct rt2x00_dev *rt2x00dev,
>  	if (ieee80211_flags & IEEE80211_CONF_CHANGE_CHANNEL)
>  		rt2x00link_reset_tuner(rt2x00dev, false);
>  
> +	if (test_bit(DRIVER_REQUIRE_PS_AUTOWAKE, &rt2x00dev->flags) &&
> +	    (ieee80211_flags & IEEE80211_CONF_CHANGE_PS) &&
> +	    (conf->flags & IEEE80211_CONF_PS))
> +		queue_delayed_work(rt2x00dev->workqueue,
> +				   &rt2x00dev->autowakeup_work,
> +				   msecs_to_jiffies(rt2x00dev->beacon_int) - 1);
> +

I have doubts about this part.  Doesn't this just catch
one beacon?  I'm not sure how often mac80211 calls the .config
op, I thought it is called once when going to sleep, and then the
hw/fw/driver is responsible to wakeup every n'th beacon
(according to the configured listen interval).

Also, I think rt2x00dev->beacon_int is in TUs, not msecs.


Johannes

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Ivo Van Doorn]

Hi,

>> +     if (test_bit(DRIVER_REQUIRE_PS_AUTOWAKE, &rt2x00dev->flags) &&
>> +         (ieee80211_flags & IEEE80211_CONF_CHANGE_PS) &&
>> +         (conf->flags & IEEE80211_CONF_PS))
>> +             queue_delayed_work(rt2x00dev->workqueue,
>> +                                &rt2x00dev->autowakeup_work,
>> +                                msecs_to_jiffies(rt2x00dev->beacon_int) - 1);
>> +
>
> I have doubts about this part.  Doesn't this just catch
> one beacon?  I'm not sure how often mac80211 calls the .config
> op, I thought it is called once when going to sleep, and then the
> hw/fw/driver is responsible to wakeup every n'th beacon
> (according to the configured listen interval).

Yes, I was also doubting about that. But is the listen_interval is TU, msecs,
or the number of beacons?

> Also, I think rt2x00dev->beacon_int is in TUs, not msecs.

Nope, it is in msecs :)

Ivo

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Stezenbach]

Hi,

On Mon, Jan 31, 2011 at 08:00:09PM +0100, Ivo Van Doorn wrote:
> 
> > Also, I think rt2x00dev->beacon_int is in TUs, not msecs.
> 
> Nope, it is in msecs :)

Please forgive my insistence, but I'm not content with your answer ;-/

rt2x00lib_config_erp() passes it to rt2800_config_erp()
which writes beacon_int * 16 to BCN_TIME_CFG_BEACON_INTERVAL,
which is dcoumented as "in unit of 1/16 TU".


Johannes

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Kalle Valo]

Ivo van Doorn <ivdoorn@gmail.com> writes:

> Add a delayed work structure which can be used to wakeup the device
> just before a beacon from the AP is expected. This then allows the
> device to receive the beacon, check if there are pending broadcast
> or multicast frames.

So the firmware/hardware doesn't have support for tracking and waking
up for beacons? I didn't even know that such hardware exists :) Waking
up for beacons from host sounds very inefficient and unrealiable. All
the devices I have seen either do this in firmware or hardware due to
time constraints. Even at76c50x-usb, which is ancient, does all this
in firmware.

Are you sure there's no way to do this with hardware?

> TODO: Split out mac80211 changes into a separate patch. We also need
> to know if we need this check in mac80211 or in the driver.
> Personally I think the check belongs in mac80211, but at this time
> that work has been deferred to the drivers. However this means that
> a lot of logic is needed in the driver to check if the correct IE is
> available, and then check the value, while mac80211 will obtains
> that exact IE anyway during RX processing anyway...

I'm worried that client power save support in mac80211 is getting
quite complex already. How many drivers need this feature?

And if hardware is so broken, is it really worth the effort of trying
to implement power save with ugly hacks like this? If power
consumption is important for the user, I would recommend just buying
better hardware.

-- 
Kalle Valo

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Ivo Van Doorn]

Hi,

>> Add a delayed work structure which can be used to wakeup the device
>> just before a beacon from the AP is expected. This then allows the
>> device to receive the beacon, check if there are pending broadcast
>> or multicast frames.
>
> So the firmware/hardware doesn't have support for tracking and waking
> up for beacons? I didn't even know that such hardware exists :) Waking
> up for beacons from host sounds very inefficient and unrealiable. All
> the devices I have seen either do this in firmware or hardware due to
> time constraints. Even at76c50x-usb, which is ancient, does all this
> in firmware.
>
> Are you sure there's no way to do this with hardware?

Well the official Ralink driver also uses a timer to wakeup the hardware,
there are however registers which suggest support for autowake functionality.

Jay, Dennis, Eddy, can you explain a but how the autowake functionality is
implemented in the rt2800usb firmware?

>> TODO: Split out mac80211 changes into a separate patch. We also need
>> to know if we need this check in mac80211 or in the driver.
>> Personally I think the check belongs in mac80211, but at this time
>> that work has been deferred to the drivers. However this means that
>> a lot of logic is needed in the driver to check if the correct IE is
>> available, and then check the value, while mac80211 will obtains
>> that exact IE anyway during RX processing anyway...
>
> I'm worried that client power save support in mac80211 is getting
> quite complex already. How many drivers need this feature?

At least rt2800usb and carl9170. But I haven't done a full round
through all drivers though.

Ivo

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Stezenbach]

On Wed, Feb 02, 2011 at 07:42:51PM +0200, Kalle Valo wrote:
> So the firmware/hardware doesn't have support for tracking and waking
> up for beacons? I didn't even know that such hardware exists :) Waking
> up for beacons from host sounds very inefficient and unrealiable. All
> the devices I have seen either do this in firmware or hardware due to
> time constraints. Even at76c50x-usb, which is ancient, does all this
> in firmware.

IMHO this is a bit exaggerated.  Given that the beacon interval
is typically ~100 msecs, and typical worst cast irq latencies
on Linux 2.6 are a few 100 usecs, we can sleep e.g. 90msecs
and wake up in time to catch the beacon with high probability, thus
saving 90% on PHY power.  Not perfect, but good enough, isn't it?


Johannes

Re: [rt2x00-users] [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Aleksandar Milivojevic]

On Wed, Feb 2, 2011 at 1:29 PM, Johannes Stezenbach <js@sig21.net> wrote:
> On Wed, Feb 02, 2011 at 07:42:51PM +0200, Kalle Valo wrote:
>> So the firmware/hardware doesn't have support for tracking and waking
>> up for beacons? I didn't even know that such hardware exists :) Waking
>> up for beacons from host sounds very inefficient and unrealiable. All
>> the devices I have seen either do this in firmware or hardware due to
>> time constraints. Even at76c50x-usb, which is ancient, does all this
>> in firmware.
>
> IMHO this is a bit exaggerated.  Given that the beacon interval
> is typically ~100 msecs, and typical worst cast irq latencies
> on Linux 2.6 are a few 100 usecs, we can sleep e.g. 90msecs
> and wake up in time to catch the beacon with high probability, thus
> saving 90% on PHY power.  Not perfect, but good enough, isn't it?

Half-related to above, and purely from user perspective.  Is there a
way to disable power saving for WiFi cards globally by default (for
example, by passing an option to device driver)?  Having power saving
turned on by default makes sense for laptops, but doesn't make much
sense for desktops.  And in my case (due to problem with at least some
rt2xxx chips that Ivo described) it causes relatively frequent
disconnects/reconnects from/to AP due to lost beacons.

For my desktop system, I'd rather pass an option to device driver to
have power saving default to off, and have a smoother network
experience.

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Kalle Valo]

Johannes Stezenbach <js@sig21.net> writes:

> On Wed, Feb 02, 2011 at 07:42:51PM +0200, Kalle Valo wrote:
>> So the firmware/hardware doesn't have support for tracking and waking
>> up for beacons? I didn't even know that such hardware exists :) Waking
>> up for beacons from host sounds very inefficient and unrealiable. All
>> the devices I have seen either do this in firmware or hardware due to
>> time constraints. Even at76c50x-usb, which is ancient, does all this
>> in firmware.
>
> IMHO this is a bit exaggerated.

Ok, you have a point that maybe I was exaggerating a bit too much.

>  Given that the beacon interval is typically ~100 msecs, and typical
> worst cast irq latencies on Linux 2.6 are a few 100 usecs, we can
> sleep e.g. 90msecs and wake up in time to catch the beacon with high
> probability, thus saving 90% on PHY power. Not perfect, but good
> enough, isn't it?

But it's not only about irq latency. If we would implement this in
mac80211 at least one context switch is involved before the beacon is
processed. Also driver might create more latency. And then there's
waking up chip from the sleep mode, that usually takes time. And what
happens when the host is under severe load? I just think that there is
just too much ucertainty here and I don't trust this to be reliable.

So I have my concerns if it really makes sense to implement this in
mac80211. Extra complexity (and extra maintenance) for two sub-optimal
(from power save point of view) hardware.

(dropping rt2x00 list because it spams me about not being subscribed)

-- 
Kalle Valo

Re: [rt2x00-users] [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Kalle Valo]

Aleksandar Milivojevic <alex@milivojevic.org> writes:

> Half-related to above, and purely from user perspective.  Is there a
> way to disable power saving for WiFi cards globally by default (for
> example, by passing an option to device driver)?  Having power saving
> turned on by default makes sense for laptops, but doesn't make much
> sense for desktops.  And in my case (due to problem with at least some
> rt2xxx chips that Ivo described) it causes relatively frequent
> disconnects/reconnects from/to AP due to lost beacons.
>
> For my desktop system, I'd rather pass an option to device driver to
> have power saving default to off, and have a smoother network
> experience.

There is CONFIG_CFG80211_DEFAULT_PS kconfig option, but I'm not aware
of any other way to globally do this. I can only think of using iw or
iwconfig to disable power save per device.

-- 
Kalle Valo

Re: [rt2x00-users] [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Aleksandar Milivojevic]

On Tue, Feb 8, 2011 at 12:48 AM, Kalle Valo <kvalo@adurom.com> wrote:
> Aleksandar Milivojevic <alex@milivojevic.org> writes:
>
>> Half-related to above, and purely from user perspective.  Is there a
>> way to disable power saving for WiFi cards globally by default (for
>> example, by passing an option to device driver)?  Having power saving
>> turned on by default makes sense for laptops, but doesn't make much
>> sense for desktops.  And in my case (due to problem with at least some
>> rt2xxx chips that Ivo described) it causes relatively frequent
>> disconnects/reconnects from/to AP due to lost beacons.
>>
>> For my desktop system, I'd rather pass an option to device driver to
>> have power saving default to off, and have a smoother network
>> experience.
>
> There is CONFIG_CFG80211_DEFAULT_PS kconfig option, but I'm not aware
> of any other way to globally do this. I can only think of using iw or
> iwconfig to disable power save per device.

I'm using iwconfig currently to disable power saving for wireless card
on my desktop after it boots up.  However, from user perspective of
things, it would be awesome if default was not hard coded in the
kernel.

Though, I must admit that if it wasn't for the problem discussed
earlier in this thread, I probably wouldn't even notice power saving
was set to on.  I'm not sure if there are any other more subtle
downsides of having power saving enabled.

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Stezenbach]

On Tue, Feb 08, 2011 at 10:27:02AM +0200, Kalle Valo wrote:
> So I have my concerns if it really makes sense to implement this in
> mac80211. Extra complexity (and extra maintenance) for two sub-optimal
> (from power save point of view) hardware.

Agreed the timer thing is ugly and should stay in the driver,
provided it survives stress testing.

But the mc/bc check Ivo suggested as part of his RFC patch is already on
the mac80211 TODO list because of ath9k (ath_beacon_dtim_pending_cab()):
http://wireless.kernel.org/en/developers/todo-list#power_saving

I'm not sure if the mac80211 changes suggested by Ivo would work for
ath9k (and carl9170 which also has a similar check).  Maybe someone
could comment on that.


Johannes

Re: [rt2x00-users] [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Kalle Valo]

Aleksandar Milivojevic <alex@milivojevic.org> writes:

> On Tue, Feb 8, 2011 at 12:48 AM, Kalle Valo <kvalo@adurom.com> wrote:
>> Aleksandar Milivojevic <alex@milivojevic.org> writes:
>>
>> There is CONFIG_CFG80211_DEFAULT_PS kconfig option, but I'm not aware
>> of any other way to globally do this. I can only think of using iw or
>> iwconfig to disable power save per device.
>
> I'm using iwconfig currently to disable power saving for wireless card
> on my desktop after it boots up.  However, from user perspective of
> things, it would be awesome if default was not hard coded in the
> kernel.

Well, we have the kconfig option which you can use to achieve that.

> Though, I must admit that if it wasn't for the problem discussed
> earlier in this thread, I probably wouldn't even notice power saving
> was set to on.  I'm not sure if there are any other more subtle
> downsides of having power saving enabled.

Yeah, with a proper power save implemention user should not even
notice any difference, apart from the reduced power consumptin of
course. But if something in power save is broken (either in the client
or in the AP) all sort of weird problems start to happen: network
drops, randomly lost packets, packects going only one way and so on.

-- 
Kalle Valo

Re: [rt2x00-users] [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Stezenbach]

On Tue, Feb 08, 2011 at 11:11:26AM -0800, Aleksandar Milivojevic wrote:
> On Tue, Feb 8, 2011 at 12:48 AM, Kalle Valo <kvalo@adurom.com> wrote:
> >
> > There is CONFIG_CFG80211_DEFAULT_PS kconfig option, but I'm not aware
> > of any other way to globally do this. I can only think of using iw or
> > iwconfig to disable power save per device.
> 
> I'm using iwconfig currently to disable power saving for wireless card
> on my desktop after it boots up.  However, from user perspective of
> things, it would be awesome if default was not hard coded in the
> kernel.

E.g. on Debian you can add iwconfig settings to /etc/network/interfaces,
see http://linux.die.net/man/7/wireless but I've no idea if that
works with NetworkManager or wicd etc.

I am not sure a global config setting for PS would not be an improvement
because you usually want to enable it for the hw which supports it
(unless you have specific low latency requirements).

But I think on some distributions acpid scripts run iwconfig to change
PS depending on if you're on AC or battery power.

FWIW, I had sent a patch to change the rt2800 USB driver
to default to PS off but Ivo seemed determined to
fix power saving instead of disabling it.
http://rt2x00.serialmonkey.com/pipermail/users_rt2x00.serialmonkey.com/2011-January/003017.html
(This RFC patch only changes the default, if it turns out PS
really cannot be fixed we should remove IEEE80211_HW_SUPPORTS_PS instead.)


Johannes

RE: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[RA-Jay Hung]

Hi Ivo,

> Well the official Ralink driver also uses a timer to wakeup the hardware,
> there are however registers which suggest support for autowake functionality.

> Jay, Dennis, Eddy, can you explain a but how the autowake functionality is
> implemented in the rt2800usb firmware?

Current power saving and wake up procedure as below.

PCI:
1. Driver sends power-save command to firmware to turn off RF/BBP and configure
mac register (AUTO_WAKEUP_CFG) to set a hardware timer counter.
2. When hardware timer timeout, issue an interrupt and notify the driver.
3. Driver send wake up command to firmware to turn on RF/BBP and ready to process
packets

USB:
1. Driver sends power-save command to firmware to turn off RF/BBP and set
a software timer to emulate the purpose of hardware timer.
2. When software timer timeout, driver sends wake up command to firmware to
turn on RF/BBP and ready to process packets.

Note: for future projects, hw or firmware will take more psm mechanism to reduce
some latency between firmware and host to save more power.

BR,
Jay
CONFIDENTIALITY STATEMENT : The information, attachments and any rights attaching in this e-mail are confidential and privileged; it is intended only for the individual or entity named as the recipient hereof.Any disclosure, copying, distribution, dissemination or use of the contents of this e-mail by persons other than the intended recipient is STRICTLY PROHIBITED and may violate applicable laws.If you have received this e-mail in error, please delete the original message and notify us by return email or collect call immediately. Thank you.

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Ivo Van Doorn]

Hi,

>> Well the official Ralink driver also uses a timer to wakeup the hardware,
>> there are however registers which suggest support for autowake functionality.
>
>> Jay, Dennis, Eddy, can you explain a but how the autowake functionality is
>> implemented in the rt2800usb firmware?
>
> Current power saving and wake up procedure as below.
>
> PCI:
> 1. Driver sends power-save command to firmware to turn off RF/BBP and configure
> mac register (AUTO_WAKEUP_CFG) to set a hardware timer counter.
> 2. When hardware timer timeout, issue an interrupt and notify the driver.
> 3. Driver send wake up command to firmware to turn on RF/BBP and ready to process
> packets
>
> USB:
> 1. Driver sends power-save command to firmware to turn off RF/BBP and set
> a software timer to emulate the purpose of hardware timer.
> 2. When software timer timeout, driver sends wake up command to firmware to
> turn on RF/BBP and ready to process packets.

Thanks. So I'll add this routine to the rt2800usb driver then. :)

Johannes, if you don't want to add the complexity for checking the
"pending MC/BC frames" in
a received frame. Would you mind if I add some utility functions to
mac80211 then? That way the
logic is still shared for the drivers that need it, but it doesn't
impact the code paths in mac80211 itself...

> Note: for future projects, hw or firmware will take more psm mechanism to reduce
> some latency between firmware and host to save more power.

Cool :)

Thanks for the info.

Ivo

Re: [RFC] rt2x00: Add autowakeup timer for receiving beacons while in powersave mode

[Johannes Berg]

On Mon, 2011-02-14 at 11:14 +0100, Ivo Van Doorn wrote:

> Thanks. So I'll add this routine to the rt2800usb driver then. :)
> 
> Johannes, if you don't want to add the complexity for checking the
> "pending MC/BC frames" in
> a received frame. Would you mind if I add some utility functions to
> mac80211 then? That way the
> logic is still shared for the drivers that need it, but it doesn't
> impact the code paths in mac80211 itself...

Sure, that's ok I suppose. FWIW, if it makes the implementation a lot
simpler, adding it in mac80211 would be acceptable, but I'm not
convinced that CONF_PS is the right hook for it, since mac80211
throughout assumes CONF_PS will automatically wake up for beacons etc.,
and changing that would imho exponentially increase the already high
complexity of the powersaving implementation.

johannes