Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Applications
  • Assigned To
    Buschel
  • Operating System PortalPlayer-based
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Buschel - 2008-03-01
Last edited by Buschel - 2011-11-13

FS#8668 - battery runtime: experimental gui boost

We could take this kind of “quick-and-dirty” solution as a starting point for further work on GUI boost for iPod’s.

The attached patch does the following:
- returning BUTTON_SCROLL_BACK/BUTTON_SCROLL_FWD out of scrollwheel driver (which was missing before)
- boost CPU on each received button and unboost after 1s, if the were no further button events

This patch should work for all iPod’s with a 4G scrollwheel for now.

Closed by  Buschel
2011-11-13 18:04
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Reworked to use timeout API, submitted with r30975.

Next experimental version does set NORMAL_CLOCK to 15MHz (svn = 30MHz) to achieve better runtime for high efficiency codecs (like mpc and especially flac).

Remark 1: mpc-playback without WPS needs ~26.5MHz. With this patch the runtime is about 14.5h (svn = 14h).

Remark 2: flac-playback without WPS needs ~15MHz. I hope to see runtime >16h with this patch – measurement will follow.

Remark 3: This patch is just to get an impression of the gui boost and to go ahead for the maximum possible runtime, it does not take any care about plugin-behaviour. This patch is no svn-candidate.

Remark 4: Should also work for non-iPod PP-targets.

Additional small patch attached which disabled the lineout of the WM8758 which is used in 5G (no_lineout_v01.patch). Through this another 0.5-0.6mA can be saved – of course only if you do not need the line out at all.


Battery bench results with gui boost (@15MHz) and disabled lineout (details attached):

MPC:
overall = 14:24h
90%-10% = 12:09h

FLAC:
overall = 12:46h
90%-10% = 11:10h

FLAC saves a lot of current via very low CPU-usage (~4mA vs. MPC), but needs a lot of additional current for the buffering from HDD (~7.5mA vs. MPC).


Attached a 3rd version of gui boost with 24MHz normal clock – as this seems to be more effective than running @15MHz normal clock (gui_boost_v03.patch).

Using the 24MHz version fo the gui boost + lineout disable:

MPC:
overall = 14:42h
90%-10% = 12:31h

(for a 5.5G 30GB of course)

updated patch against svn.

updated patch against svn.

Is this something that is intended to eventually be committed, or is it a more base work that uses techniques that wouldn’t be approved in the normal builds? I’m asking because I do think this would be very useful, and it seems like you’ve done good work here (you know, because you NEVER do good work). Do you want/need people to test it for you? Now that I actually know how to patch and junk, I’d be glad to help out if I can. Even if it’s just simple stuff (actually, that’s probably all I’m capable of at this point).

Let me know!

Hi, any test efforts are welcome. Of special interest is the behaviour when using plugins (e.g. games). Is there any sideffect visible through the boost/unboost after short scrollwhell activity (unboost happens 1s after last scroll-activity)?

I’m beginning to test this now. The only thing I’ve noticed so far is when you start to scroll when it first boosts it tends to be a bit jerky at first. This may be just me getting used to the wheel responding so much better though. I’ll make a point to put it through the motions in plugins and get back to you. I went ahead and included this in the build I put out so if there are any really negative side effects I’ll know shortly. Just so you know I really will be scientific about this :-), if I have bug reports I will reproduce them in a build with only this patch first before I bother you with bugs that have nothing to do with this patch.

All right, after testing all plugins, here are a couple that were affected by this patch: Brickmania, bubbles and spacerocks were the most obviously borked games… all three became more jumpy (or maybe just erratic) when using the scroll wheel to move around. In terms of demos, many of them would speed up for a moment when I scrolled. Plasma is probably the best example of this… upon scrolling, the animation goes into overdrive for about a second (the amount of time the cpu is boosted). I wanted to test pictureflow, but haven’t yet as my database isn’t currently enabled. I’ll test it later if tdtooke does not.

I hope this is helpful, let me know if you want any more tests run.

Thanks for testing. This leads to the assumption that I might take some time to check the possibility to only boost when in list-context… I’ll be back :)

You may or may not want to also let it apply to the WPS context, as well. It might be useful in making volume changes or whatnot more robust, but I can’t recall how that worked out in testing.

Pictureflow is very jerky as well with this patch so sticking to list context and maybe WPS context might be the way to go.

Came back to this since you mentioned it in Boris’ other patch. Did any more work get done on this? I remember getting very solid improvement from the work you did do. Just curious.

I did not further work on this patch, but I keep it updated for my local build since ages. With a small change though: 100MHz boosted + 24MHz normal clock.
From discussions on irc the solution provided here is not wanted in svn. Until there has a proper solution been found, I will just keep using this implementation as I am not experiencing any issues with gaming (I just never used games :)

I have updated the patch against r20076. Patch v6 uses 100MHz maximum clock for PP5022/24 and keeps 80MHz for PP5020. As this runs stable on my 5.5G for >1 year now I do not expect any drawbacks. Main advantage is faster buffering which may save some power on the HDD-targets.

It’s been forever since I patched my own build, but I do remember that combining this patch with dircache made my responsiveness absolutely fierce. Thanks for the updated patch, I think I may take a trip down memory lane with this one. It’s a shame this won’t be in SVN, I also don’t use games ,so it was never a problem for me. The implementation wasn’t able to be confined to list context? Or did it not matter, possibly it was seen as hackish? Just wondering where the discussion ended up.

Also, it’s a total pipe dream, but I totally wish there was some sort of advanced mode for rbutil that you could feed patches. I don’t mind the process terribly, but it’s a process nonetheless.

Thanks again!

Well, the patch’s approach for gui boost is quite dumb. Boost with short timeout on each button activity. So, the advantage is that the gui becomes responsive as soon and as long as you interact with it. The bad thing is that this approach leads to jumpy behaviour of other plugins (especially games or demos) because of the boost timing out – fast on button interaction, slow in between. Maybe the easiest thing would be to forbid boosting in plugin context…

Other discussed approaches try to boost the CPU each time an update of the display (drawing) is taking place. Doing so will need seemless clockswitching (no sync time for PLL) to be able to boost/unboost several times per second. Nevertheless I do personally not like this idea as I expect rockbox to waste power for unneeded boosting: e.g. WPS-updates run smooth @24MHz, but they will be boosted then. Therefor rockbox would need to use a very low normal clock (<24MHz) which allows the codecs to use such boosts for buffer filling – otherwise the boost for e.g. WPS-updates come on top of the codec boosts. But very low clocks (I have done runtime measurements with 15MHz, see above) seem to be less efficient in terms of mA/MHz.

Yeah, I remember us talking briefly (just on this page) about not using the boost within the context of plugins. Which also seems a bit odd, but the benefits to responsiveness speak for themselves. I can see why it would be frowned upon though. It’s a shame that the other ideas are such a tightrope walk between responsiveness and efficiency. Hopefully an eventual middle ground can be met (though I suspect that middle ground may be our current setup.

Either way, I’ll probably make use of this patch for a while. I appreciate it.

Just let me know whether you experience any problems with the 100MHz clock.

This latest patch with 100MHz clock seems to cause fatal problems on my 5g 60GB ipod - “Prefetch abort at D1B71758” right after Rockbox logo. Also appears to have made my ipod unresponsive to the menu+select reset. That’s on a fresh svn build (build without the patch, works fine, add in the patch and rebuild, → death)

Wow. I did not expect such as my 5.5G 30GB runs @100MHz without any problems since >1 year…

Yeah the same happens for me too. I cannot go past the rockbox logo, I’m greeted with a data abort error every time.

D-Shadow, what device do you use? Is it a 5G?

It’s the 5.5G 80 Gig version.

I have a 30GB iPod video 5.5G, and i installed this patch on to rockbox. I definately noticed changes in how the plugins were affected like how others have metioned but also with snake2 and xobox. Those 2 seemed to repsond to exactly what the patch is doing (the game will speed up when interacted then slow down when not). But overall it really helped with running the games and music loading seemed to be faster (i could be wrong though with the music).

I like the idea though of having the overclocking being used when drawing is occuring, that would most likely help with the overall smoothness of games and the OCing of the just the general screen!

Patched build seems unable to build for non-arm devices, but I supposed that’s expected? Error due to USEC_TIMER not defined in button.c

stripwax, in fact I have never tried to build the patch for non-arm…

do you think a patch for this will ever be made that has like an on/off feature in the menus? For i think that might be a useful idea, because it would give you the choice to run the boost when you want to and vice versa.

No, I don’t think this patch will be committed as a new option. The reason for having this gui boost is that you need it (at least for iPod Video) with a lower “normal” clock. A lower clock leads to massive savings in battery runtime.
There are some discussions going about alternative solutions for gui boost – like boosting while drawing.

New version which does not use USEC_TIMER but current_tick. Should compile for non-PP targets.

Earlier, I used v06 for a while on my 5G 30GB iPod, and I didn’t get any crashes. Now I get what seem to be heat-related crashes with r20590 and both v06 and v07. First, my iPod was charging and it was impossible to get to the main menu. Then after resting a while it started up ok but crashed while starting Doom. Then I again couldn’t get to the main menu. The next morning it again crashed while starting Doom, and after that I wasn’t able to get to the main menu. (I’m not going to list the crashes here. They’re the sort of nonsensical crashes one would expect from unstable hardware.)

Well, it seems I have one of the seldom PP5021’s which can be clocked with high frequency. Somehow the idea of PP5021 being selected PP5022 with lower maximum clock comes up again…

I hope the attached 80MHz-version is patchable (cannot test at work). Otherwise I’ll update in a few hours.

Thanks! The 80 Mhz patch is stable.

The previous version interacted badly with Pictureflow (essentially prevented scrolling repeatedly fast through covers - any continuous scrolling would seem to be interpreted as scrolling through a couple of album covers only, and you would need to stop scrolling and then start scrolling again to skip through the next two or three covers - essentially unusable). I’ll see if the newer version using current_tick fixes this issue

What is the problem with pictureflow? AFAIK you’re not supposed to be able to skip covers.

By skip I just mean ‘move from one to the next’. Without the patch you can keep twirling your finger on the scrollwheel and the covers will keep whooshing past. With the past, about one or maybe two will whoosh past and then you stop, you need to physically take your finger off the scrollwheel, put it back on, and twirl your finger some more.
The version using current_tick does not appear to fix the issue.

Update patch against r25276.

Sync’ed against r29005.

I just applied the 100 MHz version: it works great, in a quiet long menu inside a wide viewport (like the file browser with theme Cabbie), I feel the browse motion confortable.

Since I’ve applied also the piezo click  FS#5111 , I ear a double piezo click for each movement.
The cursor movements still remain correct.

cpu98 commented on 2011-02-17 11:28

I’ve used 100MHz for months. Even it survives electric bed warmer.
I hear some clicks while listening quiet pieces(with few bands of eq enabled)
I suspect it is generated while it is doing underclocking.(no clicks when it plays flac with constant 24MHz)
Debug screen shows no trace of buffer underrun.
Just amazed no one has noticed that.

cpu98 commented on 2011-02-18 02:07

Oh I forgot to mention constant 100MHz is just fine.

Some questions from my side:
- You do not hear such clicking when using the standard build (30/80 MHz)?
- You assume to hear the clicking when the clocks are changing and not when the clock is unboosted?

This patch disables the PLL when underclocking. Are sure that this clicking does not happen with svn (30/80 MHz) and you are sure svn is also chaning the clocks while playback? If so this might be a hint.

cpu98 commented on 2011-02-18 10:33

Oooops. I hear some clicks with standard build.
Altered boost_state rapidly to 1-0-1-0-… heard no clicks with 24-100MHz.

My conclusion: No hardware related issue with extended frequency range nor PLL noise
but the buffering thread priority has some problem so standard 30MHz also has audible click and because of lowered clock(24MHz) it happens more.
I’ll update if I find more.

cpu98 commented on 2011-02-19 06:21

I don’t hear such noise on latest build with and without patch applied.

I do hear that noise (unpatched) and have mentioned it previously in irc. A faint click which sounds like a few dropped samples, which I think correlates with when clock speed changes. Most noticeable (for some reason) when playing games but not obviously correlated with the game itself. I guess playing games boosts (or causes playback thread to boost) at certain moments which makes it easier to notice.

stripwax, if this is audible for you in svn you should write a bug report.

IMO this task should be closed (since it’s not developing towards inclusion into SVN). Or work on make something commitable.

I just remove the modification to the file target/arm/ipod/button-clickwheel.c and the double piezo click, as reported by myself at http://www.rockbox.org/tracker/task/8668#comment38722 Has anybody an idea about if this is correct?
Thanks for any hint.

Much simpler version which only boosts in the CONTEXT_STD context (i.e lists) so it doesnt affect the plugins at all. Doesnt change the default frequencies but that can be merged easily.
https://github.com/jdgordon/rockbox/tree/gui_boost_on_wheel

edit: this unboosts on the BUTTON_REL event so happens as soon as the user stops moving the wheel. the video feels much less sluggish with this patch compared to svn

I applied guiboost.diff to r30835, and in the debug screen, I see 80 Mhz and boost_counter: 1. I don’t understand the if conditions in guiboost.diff, but I don’t know for sure if they’re wrong because I’m not familiar with this part of Rockbox. It seems like it doesn’t care about context when boosting, but it only unboosts in CONTEXT_STD.

Yeah, i messed it up.. this one is correct

Thanks. Yeah, that seems to work properly. List speed isn’t something that I usually notice or feel like complaining about, but this is a very visible improvement. It feels better, and I think it might lead to more accurate list navigation. While analyzing behaviour a bit at 30 MHz, I noticed how the screen image could be delayed relative to input. I suspect that’s what sometimes makes me overshoot the desired position. With boost, the displayed position seems solidly linked to input.

Agreed. is there any reason we shouldnt be boosting so often?

Jd, thanks for this! I will test this evening. Maybe this approach will lead to more acceptance to bring it to svn. Now we could also easily create a new setting to enable and disable this boost on scrollwheel wheel targets, correct?

The difference on the video between scrolling boosted and unboosted is too huge to bother with a setting. I would be very surprised if anyone chose not to use it.

IIRC the origional idea for gui boosting was to always boost on input which this doesnt exactly solve (if this is joined with lowering the default clock speed), but this does make my video much nicer to use

The original idea of this patch was too have a low unboosted clock to save several hours of runtime. But this required some boosting to have a responsive gui.

I updated the patch and changed the implementation a bit. Now the boost is triggered with each scroll wheel action in the defined contexts, the unboost is done after timeout instead of being triggered by button release. This is to support debouncing of the scrollwheel events.

Another question: To which targets should HAVE_GUI_BOOST_ON_WHEEL be added? All targets with HAVE_SCROLL_WHEEL?

The scrollwheel should already be debounced by the driver, so there is no need to use a timeout to unboost. unboosting on scrollbutton|BUTTON_REL will unboost as soon as the user stops using the wheel whch is what is wanted, no need to delay it.

The wheel ipods for sure need it, I don’t remember the e200 or fuze needing it.

An update of the patch to prepare the submit:
- the define is called HAVE_GUI_BOOST to not have this limited to scroll wheel targets
- added this define to all iPods with a 4G keypad

I’d still prefer explicit unboosting instead of timed, but meh. go for it!

In get_action_worker, right before the new code, there can be a blocking button_get(true) if timeout == TIMEOUT_BLOCK. If such a blocking call is made before the timeout expires, boost may persist until another button is pressed.

A grep through the code shows various potential trouble spots. One example is the list of lines in the text_editor plugin, at “button = rb→get_action(CONTEXT_LIST,TIMEOUT_BLOCK);”. I confirmed the problem using a patch (attached here) which prints characters when boosting and un-boosting. To reproduce, run the text_editor plugin, and scroll. You’ll see the B when you scroll, unless already boosted, and you’ll see the X when you press play while boosted, after the timeut expires.

I have 2 ideas for a possible solution:
- the first timeout could be limited to the remaining boost time, and after unboosting a second call could be made if necessary.
- do timeout based un-boosting elsewhere, such as a tick task

Understood. I prefer to add a tick task as this best fits the concept. Maybe I will be able to do this over the weekend. If anybody else comes up with a proposal/patch I am of course also fine with it :)

Isn’t a tick task a bit heavy? But if you do please use the timeout api instead of a plain tick task.

There’s another issue: the button_* functions are used directly some parts of Rockbox, including many plugins. If GUI boost is on when entering such a plugin, the CPU will remain boosted while in the plugin. Starfield is one example. There, you don’t need gui_boost_indicator.patch because the stars move faster when the CPU is boosted.

Because of this it’s impossible to unboost from get_action_worker. (My first idea in the last post won’t work.) The timeout API seems appropriate here. I wonder if that requires use of a mutex in gui_boost when tick tasks are happening on the COP.

I dont see any of that as issues worth thinking about. Either way, the next time any activity happens (including idleing in the wps or menus) the cpu will get unboosted. The unboost check is done even if button == BUTTON_NONE which wil be quite often, so yeah, an extra potential few seconds of boost is not such a big deal.

The attached patch uses the timeout API to unboost the cpu. Please review and give feedback.

New version ensures that the timeout api is available. Thanks to kugel for pointing out.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing