Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#8523 - Disable WPS updating when the backlight is off.

Attached to Project: Rockbox
Opened by Magnus Holmgren (learman) - Sunday, 27 January 2008, 23:01 GMT+1
Task Type Patches
Category WPS
Status New
Assigned To Magnus Holmgren (learman)
Player type All players
Severity Low
Priority Normal
Reported Version current build
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Private No

Details

This is a proof-of-concept patch to stop the WPS from updating the screen if the backlight is off. For now, it is only enabled for the Sansa e200, as I have one and the e200 screen cannot usually be read with the backlight off.

The reason for doing it is that it can increase the runtime a little, depending on the WPS used, though we are probably not talking about very big differences.

Should this be applied at all? If so, how? I.e., should it only be enabled for targets where it makes sense, or should an option be added for all targets?

The patch also includes some minor code policing changes.
   wps.patch (5.2 KiB)
This task depends upon

Comment by Peter D'Hoye (petur) - Monday, 28 January 2008, 14:18 GMT+1
1) I have seen this discussion before and it was agreed that runtime would not be influenced a lot unless the codec was doing (a lot of) boosting
2) several targets have a display that is perfectly readable without backlight
3) why mix functional code with code policing in one patch?
Comment by Thomas Martitz (kugel.) - Monday, 28 January 2008, 17:30 GMT+1
Still, would be be nice if someone could test it in case of e200 :) Maybe I do when I got some time.
Comment by Magnus Holmgren (learman) - Monday, 28 January 2008, 21:35 GMT+1
1) Maybe that discussion was made when WPS:s were a bit simpler? With album art, lots of bitmaps and conditionals on sometimes big screens, things could be a bit different.

Also, battery benchmarks collected as a part of the PortalPlayer power optimization tests suggests there is a (small but noticeable) difference on some targets. As for boosting, when playing Vorbis files (q 4.5), with some sound processing, I get 40-50 % boost ratios on my e200.

2) I'm well aware of that, which is why the patch only does it for the e200 for now.

3) Yeah, I probably should remove it. But it stared me in the face when I was editing that function... :)
Comment by Peter D'Hoye (petur) - Sunday, 03 February 2008, 14:25 GMT+1
Unless I overlooked, I don't see a display refresh forced when backlight is switched on. Or is the normal refresh done fast enough to be ready by the time the backlight is on again?
Comment by Thomas Martitz (kugel.) - Sunday, 03 February 2008, 16:40 GMT+1
In  FS#8379  Frank Gevaerts said that showing the wps or the main menu makes a 15% difference in runtime. This makes me think, that updating the screen permanently or not makes indeed a difference.
Comment by Jacob Brooks (jac0b) - Monday, 04 February 2008, 15:37 GMT+1
Has this been tested on the gigabeat?
Comment by Michael Hahn (disorganizer) - Monday, 04 February 2008, 22:44 GMT+1
fyi:
doesnt patch together with  FS#8385  (viewport-wps).
needs manually modification for viewports, but works then.

now regarding the general idea:
i understood that this patch stops updating the wps, so graphics are not drawn any more.
so far so good, but is the lcd also completely turned off? or is this handled by any other power saving patch?
Comment by Thomas Martitz (kugel.) - Monday, 04 February 2008, 22:47 GMT+1
The LCD is turned of either way. This patch just changes the rendering of the wps to NOT happen when the LCD is off, which saves CPU time and thus some energy.
Comment by Magnus Holmgren (learman) - Wednesday, 06 February 2008, 15:19 GMT+1
Normal refresh is 5 times per second. This was fast enough when I tested it on an e200 (I couldn't see any "stale" graphics or text).
Comment by Magnus Holmgren (learman) - Thursday, 07 February 2008, 21:09 GMT+1
Better version of the patch. Don't stop scrolling when not updating the WPS. Removed the code policing. Still only enabled for the e200 (easy to fix if you know basic C :).
   wps-2.patch (3.5 KiB)
Comment by Jacob Brooks (jac0b) - Friday, 15 February 2008, 00:44 GMT+1
I think this will work for the Gigabeat only. I did 2 tests and it looks like it didn't update the screen until I changed the volume or did something to make the backlight come on.
   disable-wps-update-gigabeat.p... (3.5 KiB)
Comment by Jacob Brooks (jac0b) - Friday, 15 February 2008, 01:18 GMT+1
I think this will work for the Gigabeat only. I did 2 tests and it looks like it didn't update the screen until I changed the volume or did something to make the backlight come on.
   disable-wps-update-gigabeat.p... (3.5 KiB)
Comment by Jacob Brooks (jac0b) - Tuesday, 08 April 2008, 01:03 GMT+1
Resync
   disable-wps-update-gigabeat.p... (3.5 KiB)
Comment by Thomas Martitz (kugel.) - Sunday, 18 May 2008, 03:37 GMT+1
This patch should work for both, Gigabeat F and Sansa e200.

I simply added

/* Define this if the screen is unreadable without backlight */
#define NEED_BACKLIGHT

into their config files.

I also put it into every other config-target, just commented out of course. I hope I didn't forget a target (I tripple checked it).

Feel free to add your player if you know that the screen is unreadable without backlight.

PS: I rather like that patch. It doesn't change much actually, but it potentially increases battery runtime.
   disable-wps-update.patch (21.5 KiB)
Comment by Bob (viperman3) - Wednesday, 21 May 2008, 19:53 GMT+1
The patch doesn't compile for iriver H320 and X5 - errors in gwps-common.c. I tried it for the e200 and it does compile.
Comment by Thomas Martitz (kugel.) - Sunday, 25 May 2008, 21:56 GMT+1
Hmm, I don't have the compiler for H3xx/X5 installed, so I can't really test-build. The sims for both compiled fine though.

It would be helpful if you could post the make error to www.pastebin.ca.
Comment by Bob (viperman3) - Tuesday, 03 June 2008, 23:02 GMT+1
Just tried this patch on build r17647 with option to compile for sim. There same problem persists for the X5. This is the last snippit of the error when the build fails:

gui/gwps-common.c: In function `gui_wps_refresh':
gui/gwps-common.c:2030: error: too few arguments to function `is_remote_backlight_on'
make[1]: *** [/home/bpanesar/rockbox-r17647/rockbox-r17647/build/apps/gui/gwps-common.o] Error 1
make: *** [build] Error 2


patch 8523 is the only one I applied to the svn build mentioned so as to identify if there was another patch that was causing a problem.

Comment by Thomas Martitz (kugel.) - Tuesday, 24 June 2008, 15:23 GMT+1
"If you have alternating text in your WPS, once you reactivate the LCD it cycles really fast through all the updates that were not "printed on screen" while the backlight was off."

Can anyone look into that issue?
Comment by Magnus Holmgren (learman) - Tuesday, 24 June 2008, 20:59 GMT+1
Don't see how that could happen. There's simply no way for the updates to be queued up. At most the old LCD contents could be visible briefly before the up-to-date information is rendered.
Comment by Thomas Martitz (kugel.) - Tuesday, 24 June 2008, 21:06 GMT+1
Well, no.

The longer the backlight was off, the longer the alternating lines cycle through (at a fast rate). So I expect they are indeed "queued".
Comment by Magnus Holmgren (learman) - Wednesday, 25 June 2008, 19:03 GMT+1
Ah, so it is subline specific (I don't use sublines myself). When updating is disabled, the subline_expire_time isn't incremented, hence the fast cycling when it is incremented again. Maybe the reset_subline check at line 1741 in gwps-common.c can be removed (and always based it on current_tick)?

Loading...