yashsingh1/xdaforums huggingface dataset:
[Nexus S Android Development] :
Cheers. I noticed a massive decrease in drain during audio playback.

One issue I have noticed though, although unrelated to IDLE2, is that if you run a graphics benchmark, some clock or power gating is not activated afterwards so it stops IDLE2 mode, which is rather irritating.

The power drain during audio playback goes from about 6% in 2 hours to about 20% in 2 hours when that occurs. I noticed it earlier and realised that the drain only started immediately after I ran 0xbench 2D benchmarks.

I will debug it and find out which clock is causing it and hopefully fix the offending code to turn the damn thing back off, or perhaps remove the check if it's safe to do so, but it's not an urgent issue and it's completely unrelated to my code, which is good. :laugh: :laugh:
[Nexus S Android Development] madd0g:
I presume IDLE2 power saving will also be available while using GPS ? I listen to music mainly while working out and using the Sports Tracker app.
[Nexus S Android Development] :
Not sure. I have to be honest, I didn't test it with any GPS apps. I didn't even think about GPS.

I have a feeling it won't work with TOP OFF enabled. :confused:

If not, I'll figure something out. My code is quite extendable now, so I could easily add a gps hook in with a minimum of fuss. Damn, there is too much stuff to think about on our phones.

On the plus side, I fixed the issue which I mentioned in my previous post. It was related to some power gating checks which I took from deep-idle and didn't really investigate too closely, and which I should have looked at more as it turns out that they aren't needed.

The S5PV210_PD_CAM power domain remains enabled after certain apps are run, mainly benchmarking apps, which stops cpuidle from going into IDLE2 mode. So I removed the checks and it now works perfectly after the troublesome apps have been run.

I would actually appreciate if someone would do some power consumption benchmarks. I have shown a 2/3 reduction in drain with it enabled so far, in my very unscientific checks.

If power consumption tests show that there is a decent amount of saving, I might give TOP ON a chance again and if that saves any power at all, I will implement it for use when either bluetooth or gps are active, to give those people a bit of saving. With the 0.200 update, I have a robust code base to be able to implement another power state with the minimum of hassle.
[Nexus S Android Development] :
Strangely enough, I just looked at the code for 10 minutes and found myself implementing half the logic required for the TOP ON state. All that needs adding now is the function which actually takes it into that state and some more logic for when we should use that state.

So I think I'll fully implement it at some point in the near future for use when bluetooth / GPS are active. I might as well, it's only a couple more tests per code path and less than a hundred lines of code, most of which I can copy from the pre-existing code with some alterations. :)
[Nexus S Android Development] :
This happens with Ezekeel's DI when you run AnTuTu: it can no longer enter DI mode. In fact, you only have to open that app to kill DI.
[Nexus S Android Development] :
Fixed now. It's an unnecessary power gating check.

Well, I say unnecessary. It's unnecessary for my implementation because I hook into earlysuspend and don't enable IDLE2 until that is activated, so I don't need to check the status of power gating then anyway, because the only reason to check them is to ensure the device isn't in use and I know it isn't in use because the screen is off because earlysuspend would not be activated if it was on. So, I don't need to worry about checking 3D, CAM, LCD etc power domains.

With deep idle, it probably is necessary because it doesn't hook into earlysuspend, so it checks a bunch of power domains on every invocation to see if they are enabled.

Not the best or cheapest way of doing it really, but that is what this project is about, making a better, more stable implementation.
[Nexus S Android Development] madd0g:
I'm sure that's a common use case too, beside the BT audio ;] That would be awesome, thank you. Hope GPS plays nicely with it and doesn't drop on random occasions or something, NS is finally a device I'm completely satisfied with its GPS capabilities.
[Nexus S Android Development] :
Yeah. Well, I've done half the implementation for TOP ON, so I'll finish it and hook GPS into it. Bluetooth I probably won't bother with, as I was told there was gaps during playback when IDLE2 mode was enabled with TOP ON.

So I dunno. If anyone is still using v0.140 or earlier perhaps you could provide some feedback on bluetooth audio and whether there are gaps or not? If not, I'll implement TOP ON for bluetooth also, it's only a small code change, but I want to get it right.
[Nexus S Android Development] jay_993:
Really appreciate the work you are doing :)
[Nexus S Android Development] Killasmoke:
All I can say is this kernel just made CM10 Rom really quick , fast I can see it & its not placebo I've had left the kernel the Rom came in for 2 or 3 days just flashed this a little bit ago but I know for sure the ui & opening apps just feels & looks faster Thalamus much appreciated it deserves 5 stars & :thumbup::thumbup:
[Nexus S Android Development] :
Thanks for the compliments, but this thread isn't about my kernels as such, it's about my implementation of idle2. :)

For kernel releases / discussion, please see here.
[Nexus S Android Development] :
Excerpt from my thread:

I'm going to quit my approach, and use yours. Much better! Thanks for the education! :D
[Nexus S Android Development] :
Haha, thanks. I think my rewrite makes it a lot better.

My commenting is a bit poor though, so if you don't understand why I am doing something in a certain place, feel free to ask.

I've got another patch coming today implementing TOP ON as a 3rd power state which will be used when either bluetooth or GPS is active. Just doing final testing on it now. Early tests show it goes into TOP ON ~140 times a second and TOP OFF about ~110 times a second when each respective state is active so the code has pretty much tripled in speed since the first version.
[Nexus S Android Development] :
So, just some output from the latest patch, I booted the device, stuck music on, enabled a GPS app and turned the screen off, which put IDLE2 into TOP ON mode. When the screen turned off, the app stopped using the GPS, after a few seconds the GPS will have timed out, which caused some work to be queued with a 10 second delay which flips the needs_topon flag to false which allows TOP OFF to be activated.

As you can see, all 3 states have been used in that situation, as they should have been. :)

root@android:/ # cat /sys/devices/system/cpu/cpu0/cpuidle/state*/desc
ARM clock gating - WFI
ARM/TOP/SUB Power gating - WFI
ARM Power gating - WFI
root@android:/ # cat /sys/devices/system/cpu/cpu0/cpuidle/state*/time
140632155
152150521
17191380
root@android:/ # cat /sys/devices/system/cpu/cpu0/cpuidle/state*/usage
547619
21540
2738

Just need to test bluetooth then I'll publish the patch and a new kernel.
[Nexus S Android Development] :
So, the release is done. New standalone patch for v0.210 is here

New kernels are available here.

Changelog:

Implementation of TOP ON for use when bluetooth or GPS are active

Many micro-optimisations of variables and functions in the hot paths to make the code faster and cheaper (less branching).

Changed some debugging printk messages to KERN_DEBUG

I think we are finally getting some stability when it comes to code churn.

I'm reasonably happy with the implementation as it stands, now I am intending to let it relax for a bit and give it the test of time, to see what bugs crop up with everyday usage.
[Nexus S Android Development] :
Sorry for being thick:

kernel/power/earlysuspend.c: In function 'early_suspend':
kernel/power/earlysuspend.c:142:2: error: implicit declaration of function 'earlysuspend_active_fn' [-Werror=implicit-function-declaration]

...I'm getting this build error because I've forgotten where to put the bit that tells the compiler that I'm including all your ifdefs I think.

What have I forgotten to add and where? Tell me and I will undoubtedly slap my own forehead.

EDIT: oh is it this kconfig...
[Nexus S Android Development] :
Why don't you just apply the entire patch as is? I've fixed a several serious bugs, many poor choices, slow idle code paths and omissions when I was developing IDLE2. I don't doubt for 1 second that it is more efficient and the code is certainly much more elegant and cheaper.

To be honest though, I feel that it's a bit cheeky and slightly offensive that you are asking in my thread for help when you aren't using the patch but instead are cherry-picking parts of my hard work. I'm not saying cherry-picking code is bad, but cherry-picking bits you want then asking the author for help to fix your mess, that is out of order in my opinion.

But I will say that the error suggests you have probably forgotten to include cpuidle.h, where the prototype is declared, but then, the usage of grep would have told you that without my help.
[Nexus S Android Development] DerTeufel1980:
great stuff. going to give your implementation a try in a test kernel of mine. will report back :)
[Nexus S Android Development] :
i9000? If so, nice!

That should be a fairly trivial port, it should just need modifications in the i9000 equivalent of herring-rfkill.c, the rest *should* just work. (Famous last words).

If you get stuck, give me a shout in IRC, link in my sig.
[Nexus S Android Development] DerTeufel1980:
i guess you are right about rfkill. everything else should work just fine.

if i get issues, i'll tell you.
[Nexus S Android Development] madd0g:
Ok, I ran for 30 minutes in Airplane mode with Sports Tracker and listening to music with Power Amp. Battery went from 18% to 12%. Kernel used is http://thalamus.ineige.org/kernels/...8-23JB_test-1-g9ac621f-JB-20120726-121445.zip, on Jelly Belly 0.1.4.

I ran the commands on previous thread page and got this:

cat /sys/devices/system/cpu/cpu0/cpuidle/state*/desc
ARM clock gating - WFI
ARM/TOP/SUB Power gating - WFI
ARM Power gating - WFI

cat /sys/devices/system/cpu/cpu0/cpuidle/state*/time
2934303335
2779416
0

cat /sys/devices/system/cpu/cpu0/cpuidle/state*/usage
5701040
635
195687

No idea what this means though :]
[Nexus S Android Development] :
I have applied your patch as is! I've used grep to remove all old deep idle code. I think i stuffed something up when resolving conflicts. You'll be able to see once I get it working and push to github.

EDIT: Oh wait, just checked my .config and just needed to flag =y.

Okay it's built and it works. But I did get one SOD when in airplane mode and no music. I'm still not sure of the code I wrote that fixes the frequency when the screen is off. That might be the culprit. So far I haven't been able to reproduce it. It's so weird to have a temperamental bug like that.

EDIT: Yeah, I think it's my fixed frequency DI code. I've taken it out and it's working fine.

EDIT: oh... I see there is included fixed frequency when in IDLE2. Nice.
[Nexus S Android Development] madd0g:
Did the same today on the newest JellyBelly kernel, 34 minutes of Sportstracker + Poweramp + it wasn't in Airplane mode. Went down 6% with results:

cat /sys/devices/system/cpu/cpu0/cpuidle/state*/desc
ARM clock gating - WFI
ARM/TOP/SUB Power gating - WFI
ARM Power gating - WFI

cat /sys/devices/system/cpu/cpu0/cpuidle/state*/time
1874286031
1070240073
1534394820

cat /sys/devices/system/cpu/cpu0/cpuidle/state*/usage
5530884
168398
274747

Guess todays kernel is better, could gain even more if it was in Airplane mode as usual :]
[Nexus S Android Development] :
I spent a lot of time yesterday tracking down a really irritating bug which surfaced. :(

The issue was that sometimes when the screen was turned on when the device was in IDLE2 mode, the screen would remain black, systemui would throw an ANR, and it would then soft reboot. It would also happen when the device was in a call and the proximity sensor was activated.

This drove me demented on Thursday afternoon, to the point where I did a hard reset on my git and wiped out all my attempted fixes, logged off the computer and did something else for the evening, which I think I needed to do to be honest.

So, this morning, I was determined to find the bug. I figured it was probably a timing issue which I had introduced. I assumed it probably laid with the cpufreq locking / unlocking code, so I added a delay to that code using a delayed work function, and it still occurred.

This lead me to look more closely at earlysuspend.c, as it was borking at exactly the moment when late_resume() was being called. I realised that due to the positioning of my call to earlysuspend_active_fn() that this would be called after the system had been notified of the pending resume. Also, it was being called outside the spinlock, so it would not be atomic and may be preempted if something else needed to run.

It hadn't been an issue previously, but due to my micro-optimisations of the idle2 hot paths, I had inadvertently ended up with a race condition due to being too fast, which is quite ironic, but good in a way because it shows my improvements really worked.

So, I changed the code to call earlysuspend_active_fn() from an atomic context (from inside the spinlock) before the system had been notified of the pending resume, which fixes the issue.

So, we have 2 new patches, v0.211 and v0.212

Changelog:

cpuidle: idle2: v0.211
Fixed issue causing ANR in systemui due to timing of disabling of IDLE2 mode
when late_resume is called. We now disable *before* the suspend status is
changed.
Issue introduced in v0.210 due to code optimisations making the IDLE2 code
too fast, which is ironic.

cpuidle: idle2: v0.212
Restore idle2_cpufreq_lock code to v0.210 version.
Few more micro-optimisations

v0.212 was mainly a cleanup of code I added in v0.211 thinking that the cpufreq locking code was the culprit, but I also did some more micro-optimisations to make the hot paths even faster.

v0.211 fixed the race condition issue.

I'll upload a squashed patch of v0.212 to the IDLE2-JB branch at some point in the near future.
[Nexus S Android Development] :
I had sporadic ANRs after introducing my performance governor thing. Because it was sporadic, I knew it must be something to do with a timing issue... although my understanding of it is very limited, I wondered if it was something to do with the the CPU cache being flushed. Uh I dunno... like perhaps the null was because the data needed to be put back, but my code arrived first....? Am I correct in this?

Anyway, congrats on squashing that. It pissed me off no end, and my debugging skills are, shall we say, somewhat lacking... :p
[Nexus S Android Development] :
It was horribly frustrating, but simple to actually fix once I ruled out the frequency switching code, the only code left was the late_resume() function.

Your issue wouldn't have been the same though I wouldn't have thought.

It was specific to my implementation, it was my bug that I introduced through lack of experience and bad placement of a function call, but it shows how easy it is to introduce a critical bug into the kernel.

And no, it wouldn't be anything to do with the cache. Perhaps something being preempted and then completed later, which was what my issue was, which throws stuff out of sync, hence why one uses atomic sections and spinlocks. ;)
[Nexus S Android Development] :
Milestone 3

So, next milestone for IDLE2 will be milestone 3 - v0.300.

The plan is to stick with the current codebase with perhaps some refactoring and cleanup.

The main aim of this milestone is to get IDLE2 into such a state that it can be permanently enabled whilst the screen is off with no fallout or instability.

Harbb made this suggestion, because he believes there is powersaving to be gained when suspend wakelocks are held whilst the screen is off. I agree with this so am interested in exploring this possibility and I suspect if we do it right and cheaply, then this will save power.

Some tests are in progess with a few people running debug kernel builds configured to use the lowest power state possible whilst the screen is off, whether it be IDLE, DEEP-IDLE TOP ON, or DEEP IDLE TOP OFF.

In about 60 hours testing in total, there have been no issues reported related to this configuration.

What is definitely unaffected:

Calls

Texts

Syncing

Media streaming (Wifi & 3G) - TuneIn Radio / Xiialive / Google Play Music

Bluetooth (drops to TOP ON state as previously discussed)

GPS (same as bluetooth)

NFC (screen is on anyway so not affected)

Ability to wake the device

Wifi / 3G - both stay powered and active

I'm sure I'm missing stuff that I can't think of here...if anyone has more suggestions of potential issues, then please do point them out.

If anyone reasonably tech savvy who knows how to dump logs and use a linux terminal wants to test this experimental build out and has fastboot available to flash the zimage, then PM me or join #thalamus-hacking on Freenode.
[Nexus S Android Development] madd0g:
What about headset button controls, are they affected ? I remember when using the 23 kernel, phone didn't respond to the button at first, needed to wait a bit, then it worked. Didn't try it on the recent kernel though.
[Nexus S Android Development] :
Works fine, I just tested it. :)

Was there any situation where it was more noticeable that it didn't want to respond?
[Nexus S Android Development] madd0g:
Ah great. No, they were dodgy only the first time I tried to change the song, it didn't respond at first, but after that they worked fine. I just tried it on the latest JellyBelly kernel you pushed yesterday and it responded fine. :] You didn't comment on the logs I posted earlier in this thread, about the readings I got from testing with IDLE2 and GPS. I wondered what that 0 meant in the state*/time log.
[Nexus S Android Development] :
0 in the time log means that it hasn't *actually* used that state, so it must have been falling back for whatever reason. There are a number of checks which are done and if it doesn't pass, it won't use the state. Although, that could have been a bug because fixed a couple of logic errors recently.

0 in usage means it hasn't attempted to use that state, which would be normal for TOP ON if you aren't using GPS/BT or for TOP OFF if you are permanently using BT/GPS.

Your last stats look pretty good, it's using all IDLE states as it should be.
[Nexus S Android Development] madd0g:
Ah, I presumed correctly then about the 0 time. Thanks for the explanation :]
[Nexus S Android Development] DerTeufel1980:
compiling your idle2 for my i9000 (sgs1) now. changing herring-rfkill to aries-rfkill was enough for porting ;)

[edit]

i also had to change herring to aries in idle2.h

[/edit]

one thing, i always was wondering about....

are you also getting these warnings during compiling?

i also have had them with the patch from ezekeel, and it was working well. just wondering about it ;)
[Nexus S Android Development] :
Nice. Make sure you apply the latest 2 patches 0.211 and 0.212 though. :)

Not sure, could just be a false compiler warning, I don't get it personally, which GCC version are you using?
[Nexus S Android Development] DerTeufel1980:
i have already applied them :)

i am using gcc 4.7.1, but i also got these warnings with stock 4.4.3
[Nexus S Android Development] :
Wonder why I don't get them then? I use GCC 4.6.3 generally and no warnings in any of my code. I'll have a look.
[Nexus S Android Development] DerTeufel1980:
well, i guess it is sgs1 related then, as i got them with deep_idle2, too.

if you really want to look at it anyways, there have been more warnings at this point of compiling. i just did not compy them all. if you want to have all of them, i can recompile it.
[Nexus S Android Development] :
I think it's actually line 119 that is the issue. If CONFIG_S3C_DEV_HSMMC/1/2/3 are not defined, then the array size becomes smaller.

Line 119 references onenand at array position 3, which is correct on herring, but might be wrong on aries if it has less than 3 MMC channels defined, which will cause that warning. Either way, I should fix that really, it's bad coding (Samsung :P)

What is the output of

grep HSMMC .config

in your kernel tree?
[Nexus S Android Development] DerTeufel1980:
i guess you are right about it:

first test with your idle implementation are looking good to me. :)
[Nexus S Android Development] :
Hmmm, it shouldn't warn because you have 3 defined, which puts onenand in the 3rd position in the array as it should be.

I'll have another look at the code and see if I can see something else. I didn't write that part though, Samsung did.
[Nexus S Android Development] :
I don't see any issues with the array, I suspect it is just the compiler being stupid.

On herring, these 3 are defined:

CONFIG_S3C_DEV_HSMMC=y
CONFIG_S3C_DEV_HSMMC2=y
CONFIG_S3C_DEV_HSMMC3=y

I don't think there is a problem, although I've an idea which might silence the warning.

diff --git a/arch/arm/mach-s5pv210/include/mach/idle2.h b/arch/arm/mach-s5pv210/include/mach/idle2.h
index 25a44e3..81a9e9a 100644
--- a/arch/arm/mach-s5pv210/include/mach/idle2.h
+++ b/arch/arm/mach-s5pv210/include/mach/idle2.h
@@ -19,7 +19,7 @@
#include <linux/cpufreq.h>
#include <linux/interrupt.h>

-#define MAX_CHK_DEV 0xf
+#define MAX_CHK_DEV 5
#define IDLE2_FREQ (800 * 1000) /* Use 800MHz when entering idle2 */
#define DISABLE_FURTHER_CPUFREQ 0x10
#define ENABLE_FURTHER_CPUFREQ 0x20

I don't see why it needs to go round the loop 15 times anyway, when there is only 5 items in the array.
[Nexus S Android Development] DerTeufel1980:
thank for all your hard work. going to try your suggestion tomorrow :)

i just have released some test kernels with your new idle2. tomorrow i'll give you some feedback (if non of the users is doing it by themself).
[Nexus S Android Development] :
Yeah, cool. The more feedback the better, as long as it isn't bugs lol. ;) Nah, joking aside, bugs are welcome too.

I've got a couple of bugfix patches queued up for v0.213 already.
[Nexus S Android Development] habylab:
Sorry, off topic apparently.
[Nexus S Android Development] :
This is completely the wrong thread for off topic questions, please take it elsewhere.

Back on topic:

3 bugfixes queued up so far for v0.213:

commit 7466bbbee75275614566ae1673fc5f12392c906d
Author: Will Tisdale <willtisdale@gmail.com>
Date: Mon Jul 30 06:40:34 2012 +0100

idle2: Reduce loop iterations in s5p_init_remap()
Previously we were looping 15 times, and as there are only 5 items in
the struct it is a bit pointless. Reduce it to 5.

commit ac7b4973fd2b37dc450b32f9a100128f09de4324
Author: Will Tisdale <willtisdale@gmail.com>
Date: Sun Jul 29 17:01:47 2012 +0100

serial: Move idle2 calls from samsung to core
Calls placed in samsung.c were interfering with display intitialisation
for some users.

commit 3597ea8d8bcb8b6e8962c4cf8ea5e77a79ca073a
Author: Will Tisdale <willtisdale@gmail.com>
Date: Sat Jul 28 23:46:35 2012 +0100

earlysuspend: Add missing ifdef for IDLE2

Going to wait to see if there are any bugs from the i9000 testing before I squash these and release v0.213.
[Nexus S Android Development] Segner:
I am running DerTeufel's latest test kernel with IDLE2. It seems to run as well as with the original kernel. I don't use btkill.

I am running it with no bluetooth, and I test by switching music on/off and Wifi on/off and screen off. After a while, I switch the screen back on, and check the times of the three possible idle states: "cat /sys/devices/system/cpu/cpu0/cpuidle/state*/time"

The command gives me three numbers (as expected). The third number (TOP ON?) is always zero. The second number (TOP OFF?) increases when playing music during screen off. It does not increase during Wifi on and screen off (I mention that because the classical deep idle also works during Wifi on).

Does that help? Do you want me to run more tests?

Thank you for your effort. I like the way, you talk about your development considerations. It teaches me...