950124 - Powersave filters are not working in with wifi

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Wifi, defect)

Description

[Greg Grisco]

WE are seeing that power save filters (MC/BC, BET, DTIM, MDTIM) are not working in FFOS build. This is due to no suspend indication from Framework.

WLAN driver depends on the SETSUSPENDMODE indication to enable these power save optimisations.

Suggestion is for "DRIVER SETSUSPENDMODE 1" and "DRIVER SETSUSPENDMODE 0" to be sent to driver at the appropriate times.

Comment 1

[Greg Grisco]

You can look here for android usage:

https://www.codeaurora.org/cgit/quic/la/platform/frameworks/base/tree/wifi/java/android/net/wifi/WifiNative.java?h=jb_mr2#n345

Comment 2

[Dylan Oliver [:doliver]]

Ken, this is one of the high priority blockers for bug 942267. Can you find an owner to help diagnose this one?

Comment 3

[Ken Chang[:kenkai]]

(In reply to Dylan Oliver [:doliver] from comment #2)
> Ken, this is one of the high priority blockers for bug 942267. Can you find
> an owner to help diagnose this one?
Why do you think it is a blocker? Does the wifi chip which we used support wifi low power mode?

Chuck, can you please take this bug?

Comment 4

[Ken Chang[:kenkai]]

Chuck, could you please take this bug.

Comment 5

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

I think this is a new feature, it's better not put it into 1.3 at this time.

We don't have corresponding logic in wifi manager now.
And I need time to check how this function is uesd in android - I didn't know this function before, and have no other reference here.
Also QA needs find a way to test this function does work on all devices.

There's more work to do than implement the command in comment 1.

Comment 6

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Also it seems that different driver use different driver command for setSuspendOptimization, changing the command might affect other devices.

Comment 7

[Preeti Raghunath(:Preeti)]

Please don't move this to 1.4? till QC confirms.

Comment 8

[Ken Chang[:kenkai]]

(In reply to Chuck Lee [:chucklee] from comment #6)
> Also it seems that different driver use different driver command for
> setSuspendOptimization, changing the command might affect other devices.
But for WPA_Supplicant, this command seems have been defined in Android.
We still can add this command, right? Although, it may not work in some device not
supporting this feature in driver level.

Comment 9

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to Ken Chang[:ken] from comment #8)
> (In reply to Chuck Lee [:chucklee] from comment #6)
> > Also it seems that different driver use different driver command for
> > setSuspendOptimization, changing the command might affect other devices.
> But for WPA_Supplicant, this command seems have been defined in Android.
> We still can add this command, right? Although, it may not work in some
> device not supporting this feature in driver level.

Vincent found that Android uses SETSUSPENDOPT before ICS[1] and use SETSUSPENDMODE after JB, we can support commands according to the sdk version.
Although gonk is mostly compatible with Android platform, partners still need to check/modify the command set[3] to ensure they are supported in specific platform.

[1] http://androidxref.com/4.0.4/xref/frameworks/base/core/jni/android_net_wifi_Wifi.cpp#504
[2] http://androidxref.com/4.1.1/xref/frameworks/base/wifi/java/android/net/wifi/WifiNative.java#337
[3] http://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiCommand.jsm

Comment 10

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Attachment: Use different powersave command based on sdk version.

Comment 11

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Hi Gerg,
  Please help check this patch fit your requirements.

Thanks.

Comment 12

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 8358203 [details] [diff] [review]
Use different powersave command based on sdk version.

Review of attachment 8358203 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thank you.

Comment 13

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

Try : https://tbpl.mozilla.org/?tree=Try&rev=dd61443d674b

Comment 14

[Greg Grisco]

(In reply to Chuck Lee [:chucklee] from comment #11)
> Hi Gerg,
>   Please help check this patch fit your requirements.

Hi Chuck, this looks good to me, thanks!

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/bbc7ef6e8fba

Comment 16

[vasanth]

Sorry for the delay. We got to test this patch today and seems this code path is never reached?
I see setSuspendOptimizations is called by SetPowerSavingMode.
However Gaia/Gecko never calls DOM SetPowerSavingMode?

In Android, power save mode is enabled by WifiStateMachine.java, when 3 conditions are satisfied [1], else disabled
1. Screen state is off [2]
2. DHCP set up is not going on [3][4]
3. High performance mode is disabled. [5]

Was able to enable power save mode by checking #1 similar to some other observer topics used here [6]
Not sure how to do #2 and we may need a performance user pref from gaia for #3


[1] https://www.codeaurora.org/cgit/quic/la/platform/frameworks/base/tree/wifi/java/android/net/wifi/WifiStateMachine.java?h=jb_3.2#n391
[2] https://www.codeaurora.org/cgit/quic/la/platform/frameworks/base/tree/wifi/java/android/net/wifi/WifiStateMachine.java?h=jb_3.2#n1268
[3] https://www.codeaurora.org/cgit/quic/la/platform/frameworks/base/tree/wifi/java/android/net/wifi/WifiStateMachine.java?h=jb_3.2#n1877
[4] https://www.codeaurora.org/cgit/quic/la/platform/frameworks/base/tree/wifi/java/android/net/wifi/WifiStateMachine.java?h=jb_3.2#n1911
[5] https://www.codeaurora.org/cgit/quic/la/platform/frameworks/base/tree/wifi/java/android/net/wifi/WifiStateMachine.java?h=jb_3.2#n1138
[6] http://dxr.mozilla.org/mozilla-central/source/dom/wifi/WifiWorker.js#25

Comment 17

[Preeti Raghunath(:Preeti)]

Ken

Please take a look to resolve the Wifi issue per #6

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bbc7ef6e8fba

Comment 19

[Ken Chang[:kenkai]]

(In reply to Preeti Raghunath(:Preeti) from comment #17)
> Ken
> 
> Please take a look to resolve the Wifi issue per #6

I think QC want a new user scenario like Android, but not a test interface.
In current design, Wifi will be turn off after entering suspend mode, so this power saving mechanism is not such effective for firefox OS now. But it will be very useful after we finish 1.4/1.5 features, bug 930969 and bug 930972. I would suggest moving this bug to be a 1.4/1.5 bug and also putting it into Gaia system team's backlog. Thanks.

Comment 20

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to vasanth from comment #16)
> Sorry for the delay. We got to test this patch today and seems this code
> path is never reached?
> I see setSuspendOptimizations is called by SetPowerSavingMode.
> However Gaia/Gecko never calls DOM SetPowerSavingMode?
> 
> In Android, power save mode is enabled by WifiStateMachine.java, when 3
> conditions are satisfied [1], else disabled
> 1. Screen state is off [2]
> 2. DHCP set up is not going on [3][4]
> 3. High performance mode is disabled. [5]
> 
> Was able to enable power save mode by checking #1 similar to some other
> observer topics used here [6]
> Not sure how to do #2 and we may need a performance user pref from gaia for
> #3

#1 scenario is controlled by gaia system app[1][2], you can use SetPowerSavingMode() instead of change wifi setting for your test today.

[1] http://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/js/wifi.js#l218
[2] http://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/js/wifi.js#l198

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/210e31f22de1

Comment 22

[vasanth]

(In reply to Ken Chang[:ken] from comment #19)
> In current design, Wifi will be turn off after entering suspend mode

Hi Ken, this doesn't happen for us.
Even when device is in suspend mode, still wifi is on, able to ping the device and after coming out suspend, status bar also shows Wifi is still connected.

1. How long it takes to turn off wifi after entering suspend mode?
2. Can you please point me to the code which does this?

Comment 23

[Ken Chang[:kenkai]]

(In reply to vasanth from comment #22)
> 1. How long it takes to turn off wifi after entering suspend mode?
> 2. Can you please point me to the code which does this?

I think Chuck had answered this in comment 20.

Comment 24

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to vasanth from comment #22)
> (In reply to Ken Chang[:ken] from comment #19)
> > In current design, Wifi will be turn off after entering suspend mode
> 
> Hi Ken, this doesn't happen for us.
> Even when device is in suspend mode, still wifi is on, able to ping the
> device and after coming out suspend, status bar also shows Wifi is still
> connected.
> 
> 1. How long it takes to turn off wifi after entering suspend mode?

It depends on the value of setting "wifi.screen_off_timeout", the system default is 10 minutes [1].
Also it only turn off wifi when screen is off, no wifi wakelock, and not charging [2].

> 2. Can you please point me to the code which does this?

Please refer to wifi enable/disable code in comment 20.

[1] http://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/js/wifi.js#l81
[2] http://git.mozilla.org/?p=releases/gaia.git;a=blob;f=apps/system/js/wifi.js#l148

Comment 25

[vasanth]

(In reply to Chuck Lee [:chucklee] from comment #24)
> It depends on the value of setting "wifi.screen_off_timeout", the system
> default is 10 minutes [1].

Thanks for the clarifications.

One more question is why the default wifi timeout is 10 minutes? Whether it can be reduced?
As you mentioned wifi wakelock should take care of some background internet tasks if there is something.
If nothing else is going on, in the 10 minutes device may get many longer wakeups which will increase power consumption.

Comment 26

[Chuck Lee [:chucklee, Triggered by NI, Review, and Feedback requests]]

(In reply to vasanth from comment #25)
> (In reply to Chuck Lee [:chucklee] from comment #24)
> > It depends on the value of setting "wifi.screen_off_timeout", the system
> > default is 10 minutes [1].
> 
> Thanks for the clarifications.
> 
> One more question is why the default wifi timeout is 10 minutes? Whether it
> can be reduced?
> As you mentioned wifi wakelock should take care of some background internet
> tasks if there is something.
> If nothing else is going on, in the 10 minutes device may get many longer
> wakeups which will increase power consumption.

I don't know how this value is chosen, and you can optimize timeout value by settings per comment 24.
If you are talking about changing the default value in Gaia master, I am not sure if it will be accepted to merge into Gaia master. You can open a bug for this.