FS#9195 - LCD disable for sansa c200

Attached to Project: Rockbox
Opened by Bertrik Sikken (bertrik) - Tuesday, 15 July 2008, 18:37 GMT
Last edited by Bertrik Sikken (bertrik) - Tuesday, 05 August 2008, 20:19 GMT
Task Type Patches
Category LCD
Status Closed
Assigned To No-one
Operating System Sansa c200
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


Attached patch adds LCD enable/disable support for the sansa c200.

The sansa c200 has a display that is not readable when the backlight is off, so it makes sense to turn off the display and put the display controller in standby to save a little more power when the backlight is turned off. The actual amount of power saved by this patch has not been measured yet.
This task depends upon

Closed by  Bertrik Sikken (bertrik)
Tuesday, 05 August 2008, 20:19 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed as r18198
Comment by Bertrik Sikken (bertrik) - Thursday, 17 July 2008, 21:52 GMT
I just checked the OF (1.00.06) and it _doesn't_ disable the display (can still be seen faintly when shining a flashlight on it).
Comment by Michael Sevakis (MikeS) - Sunday, 20 July 2008, 05:25 GMT
You should enable the display_on notifications so the mpegplayer can update the overlays. See e200, x5 etc. to see it implemented. The display_on notification isn't really critical if the GRAM contents are kept up-to-date despite going into standby.
Comment by Bertrik Sikken (bertrik) - Sunday, 20 July 2008, 10:35 GMT
Michael, I don't understand what you mean. Currently the patch just disables/enables the display and puts the controller in/out of standby. What else is needed? Is that documented somewhere?

I'll have a look how it behaves now with mpegplayer.
Comment by Bertrik Sikken (bertrik) - Sunday, 20 July 2008, 14:01 GMT
The first battery bench run shows about 40 minutes improvement in runtime (total runtime without patch was 14:01 h), or about 4-5%.
The patch could use a little more sophistication, like not updating the display if it is disabled anyway. This may save some additional power, since the c200 display is updated through a slow LCD bridge requiring busy-waits that burn cpu cycles.
I wonder if we can/should implement the lcd_sleep functions too.
As far as I understand the datasheet, turning the display off does just that, keeping the image contents. I'm not so sure if they are retained when going into standby.
Comment by Michael Sevakis (MikeS) - Sunday, 20 July 2008, 19:55 GMT
If you cut the updates out during the "off" phase to avoid the busy waits, the screen needs to be refreshed from the rockbox framebuffer when turning the display on again which erases the YUV overlay. The driver calls lcd_update to bring the visible display in sync with the RB framebuffer. The driver calls lcd_call_enable_hook which calls any callback registered (or just returns of no callback is registered) which can perform any task desired - mpegplayer uses its registered callback to update the YUV overlay after it was erased by lcd_update.

Basically for external GRAM type displays it goes like so:

if (enable) {
... turn hardware ON ...
lcd_enabled = true; /* Unblock display updates before trying to perform them */
lcd_update(); /* Sync interal GRAM */
lcd_call_enable_hook(); /* Let whoever cares know - they can update items drawn with performance functions */
} else {
lcd_enabled = false; /* Block display updates before hardware is turned off */
... turn hardware OFF ...
Comment by Bertrik Sikken (bertrik) - Thursday, 24 July 2008, 20:52 GMT
Thanks Mike,
I tried to implement something like that (i.e. not updating the display when the display is off and forcing an update when the display comes back on), see attached patch.

It works fine for the menus and the WPS. When playing a video with the mpeg plugin, the display seems to get corrupted after turning the display on and off a few times (I use the hold switch for this, with display off on hold). When it goes wrong I usually just see a blank screen, but I've also seen a rather 'stripey' screen and a screen where only on the left side some text would be visible.
Comment by Bertrik Sikken (bertrik) - Sunday, 27 July 2008, 13:13 GMT
I tested just c200_lcd_disable.patch (only disable LCD, no holding off of lcd_update()'s) with the mpegplayer plugin and it appears to be free of the side effects that are in the 18117 version. I tested the following scenarios: 1) quickly switching hold on and off during mpeg playback, 2) quickly switching hold on and off with mpeg playback paused 3) pause mpeg, wait for display to turn off and turn it on by pressing a volume button.

I still don't know for sure why the second patch fails with mpegplayer, but I'm guessing it has something to do with concurrent access to the display between the main cpu and the cop (mpegplayer explicitly uses the cop, right?)
I'm inclined to commit the first simple patch and perhaps later refine it with the callback.
Comment by Bertrik Sikken (bertrik) - Monday, 04 August 2008, 17:27 GMT
Simple patch again, but now with a check to do nothing if display is already in the desired setting (on/off).