[PATCH] OLPC: decouple sleep/resume from powerdown/powerup (take 3)

2007-09-25 Thread Bernardo Innocenti
This patch merges the fb_powerup and fb_powerdown hooks in a single
operation fb_power with an additional state parameter ranging
from 0 (running) to 3 (poweroff).

The geodefb uses state 2 as an intermediate low-power mode, where
the LX or GX video unit is turned off, but the GPU may still be working.
Notably, the GPU register set does not get overwritten when resuming
to state 0, so the system can safely keep using the GPU while in state 2.
The DCON driver now uses this new suspend state to let the X server draw
in the background while the screen frozen.

This fixes the rather amusing OLPC bug #3603 that made the toolbar icons
come up tinted in green when pretty boot was enabled.  Tested on both
B2 (GX) and B3 (LX) machines.  Both the freeze/unfreeze and suspend/resume
codepaths work as expected.

take2 - ensure the GX registers are saved in both the suspend and powerdown
cases, otherwise we restore stale register state when we unfreeze.

take3 - fix a condition check in the fb powerdown failure path.  Thanks to
roel for pointing it out.  Mark patch as OLPC, not GEODE.

Signed-off-by: Bernardo Innocenti [EMAIL PROTECTED]
---
 drivers/video/fbmem.c   |   52 +++---
 drivers/video/geode/geodefb.h   |   13 +++--
 drivers/video/geode/gxfb_core.c |   17 +++
 drivers/video/geode/lxfb.h  |3 +-
 drivers/video/geode/lxfb_core.c |   12 ++---
 drivers/video/geode/lxfb_ops.c  |   47 
 drivers/video/geode/video_gx.c  |   94 --
 drivers/video/geode/video_gx.h  |3 +-
 drivers/video/olpc_dcon.c   |8 ++--
 include/linux/fb.h  |7 +--
 10 files changed, 113 insertions(+), 143 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 61d7659..3ce903c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -736,49 +736,29 @@ static void try_to_load(int fb)
 #endif /* CONFIG_KMOD */
 
 int
-fb_powerup(struct fb_info *info)
+fb_power(struct fb_info *info, int state)
 {
int ret = 0;
 
-   if (!info || info-state == FBINFO_STATE_RUNNING)
-   return 0;
-
-   if (info-fbops-fb_powerup)
-   ret = info-fbops-fb_powerup(info);
-
-   if (!ret) {
-   acquire_console_sem();
-   fb_set_suspend(info, 0);
-   release_console_sem();
-   }
-
-   return ret;
-}
-
-int
-fb_powerdown(struct fb_info *info)
-{
-   int ret = 0;
-
-   if (!info || info-state == FBINFO_STATE_SUSPENDED)
-   return 0;
+   if (state  0 || state  3)
+   return -EINVAL;
 
-   /* Tell everybody that the fbdev is going down */
acquire_console_sem();
-   fb_set_suspend(info, 1);
-   release_console_sem();
-
-   if (info-fbops-fb_powerdown)
-   ret = info-fbops-fb_powerdown(info);
-
-   /* If the power down failed, then un-notify */
-
-   if (ret) {
-   acquire_console_sem();
+   /* Powerdown? */
+   if (state == 3  info-state != FBINFO_STATE_SUSPENDED)
+   /* Tell everybody that the fbdev is going down */
+   fb_set_suspend(info, 1);
+
+   if (info-fbops-fb_power)
+   ret = info-fbops-fb_power(info, state);
+
+   /* Power down failed, or powerup succeeded? */
+   if (((state == 3  ret) || (state != 3  !ret))
+(info-state != FBINFO_STATE_RUNNING))
+   /* Tell everybody that the fbdev is back up */
fb_set_suspend(info, 0);
-   release_console_sem();
-   }
 
+   release_console_sem();
return ret;
 }
 
diff --git a/drivers/video/geode/geodefb.h b/drivers/video/geode/geodefb.h
index 0214d11..cd1ce6e 100644
--- a/drivers/video/geode/geodefb.h
+++ b/drivers/video/geode/geodefb.h
@@ -12,12 +12,14 @@
 #ifndef __GEODEFB_H__
 #define __GEODEFB_H__
 
-#define FB_POWER_STATE_OFF  0
-#define FB_POWER_STATE_SUSPEND  1
-#define FB_POWER_STATE_ON   2
-
 struct geodefb_info;
 
+/* For geodefb_par-power_state */
+#define FB_POWER_STATE_OFF  3
+#define FB_POWER_STATE_SUSPEND  2
+#define FB_POWER_STATE_UNUSED   1 /* Reserved */
+#define FB_POWER_STATE_ON   0
+
 struct geode_dc_ops {
void (*set_mode)(struct fb_info *);
void (*set_palette_reg)(struct fb_info *, unsigned, unsigned, unsigned, 
unsigned);
@@ -42,7 +44,8 @@ struct geodefb_par {
struct geode_dc_ops  *dc_ops;
struct geode_vid_ops *vid_ops;
 
-   int state;
+   /* See FB_POWER_STATE_* definitions above */
+   int power_state;
 };
 
 #endif /* !__GEODEFB_H__ */
diff --git a/drivers/video/geode/gxfb_core.c b/drivers/video/geode/gxfb_core.c
index 3eabc53..cd43c8e 100644
--- a/drivers/video/geode/gxfb_core.c
+++ b/drivers/video/geode/gxfb_core.c
@@ -383,8 +383,7 @@ static struct fb_ops gxfb_ops = {
.fb_fillrect= cfb_fillrect,
.fb_copyarea= cfb_copyarea,
.fb_imageblit   = cfb_imageblit,
-  

Re: OLPC: decouple sleep/resume from powerdown/powerup (take 3)

2007-09-25 Thread Bernardo Innocenti
Jordan Crouse wrote:

 So anyway, long story short, its all below in black and white.  Please
 reply with comments.

As I said, I also like this design.

The only thing I could suggest is moving the code snippets that implement
the actual video unit powerdown and powerup to separate functions, so they
wouldn't be duplicated between lx_blank_display() and 
lx_graphics_{dis,en}able().

-- 
 \___/
 |___|  Bernardo Innocenti - http://www.codewiz.org/
  \___\ One Laptop Per Child - http://www.laptop.org/
___
Devel mailing list
Devel@lists.laptop.org
http://lists.laptop.org/listinfo/devel