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

Jordan Crouse jordan.crouse at amd.com
Tue Sep 25 13:34:10 EDT 2007


Andres and I discussed this situation in length last night.  Andres
argued that the fb_power() function was not needed and confusing -
really what we are doing when we freeze the DCON is blanking the
underlying video unit.  I agree to a certain extent, but I also
think there is value in having direct control over the power state
of the GPU itself, but thats an argument for another day and another
forum.

taking Andres suggestion, I have modified the previous patch and 
existing code, completely getting rid of the powerup() and powerdown()
functions.  We now use fb_blank(FB_BLANK_POWERDOWN) when we freeze
the DCON and fb_blank(FB_BLANK_UNBLANK) when we unfreeze it.  In the
code, the POWERDOWN mode will duplicate the behavior we had before, to
wit, we turn off the hsync, vsync, video, crt, timing generator and fifo
fetcher.  When POWERDOWN is disengaged, we turn all the stuff back on 
again.  The only think I am concerned about is that the latency on 
resume might be too high for !OLPC users - i.e. regular customers who use
a LCD or a CRT.  I told Andres that this approach would be acceptable,
assuming the regular customers were not negatively affected, and he agreed.
I don't think it should, but its something that we need to be aware of.

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

Jordan
-------------- next part --------------
[OLPC] use the fb_blank() infrastructure to turn off the GPU

From: Jordan Crouse <jordan.crouse at amd.com>

Remove the powerup and powerdown API hooks, and instead use 
fb_blank() to power off the video unit.

Signed-off-by: Jordan Crouse <jordan.crouse at amd.com>
---

 drivers/video/fbmem.c           |   47 --------------
 drivers/video/geode/gxfb_core.c |   11 ---
 drivers/video/geode/lxfb.h      |    5 +-
 drivers/video/geode/lxfb_core.c |    8 +-
 drivers/video/geode/lxfb_ops.c  |  129 ++++++++++++++++++++++-----------------
 drivers/video/geode/video_gx.c  |   28 --------
 drivers/video/olpc_dcon.c       |   19 ++----
 include/linux/fb.h              |    9 ---
 8 files changed, 82 insertions(+), 174 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index 61d7659..38c2e25 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -736,53 +736,6 @@ static void try_to_load(int fb)
 #endif /* CONFIG_KMOD */
 
 int
-fb_powerup(struct fb_info *info)
-{
-	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;
-
-	/* 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();
-		fb_set_suspend(info, 0);
-		release_console_sem();
-	}
-
-	return ret;
-}
-
-int
 fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
 {
 	struct fb_fix_screeninfo *fix = &info->fix;
diff --git a/drivers/video/geode/gxfb_core.c b/drivers/video/geode/gxfb_core.c
index 3eabc53..0d8422b 100644
--- a/drivers/video/geode/gxfb_core.c
+++ b/drivers/video/geode/gxfb_core.c
@@ -383,8 +383,6 @@ static struct fb_ops gxfb_ops = {
 	.fb_fillrect	= cfb_fillrect,
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
-	.fb_powerdown   = gxfb_powerdown,
-	.fb_powerup     = gxfb_powerup,
 };
 
 static struct fb_info * __init gxfb_init_fbinfo(struct device *dev)
@@ -454,8 +452,6 @@ static int gxfb_suspend(struct pci_dev *pdev,  pm_message_t state)
 	if (state.event == PM_EVENT_SUSPEND) {
 	 
 		acquire_console_sem();
-		gxfb_powerdown(info);
-
 		par->state = FB_POWER_STATE_OFF;
 		fb_set_suspend(info, 1);
 		
@@ -471,12 +467,6 @@ static int gxfb_resume(struct pci_dev *pdev)
 	struct fb_info *info = pci_get_drvdata(pdev);
 
 	acquire_console_sem();
-	
-	/* Turn the engine completely on */
-
-	if (gxfb_powerup(info))
-	  printk(KERN_ERR "gxfb:  Powerup failed\n");
-
 	fb_set_suspend(info, 0);
 	release_console_sem();
 
@@ -559,7 +549,6 @@ static int __init gxfb_probe(struct pci_dev *pdev,
 	/* We are powered up */
 	par->state = FB_POWER_STATE_ON;
 
-
 	console_event_register(&gxfb_console_notifier);
 
 	if (register_framebuffer(gxfb_info) < 0) {
diff --git a/drivers/video/geode/lxfb.h b/drivers/video/geode/lxfb.h
index 28d8ff3..f3ce77b 100644
--- a/drivers/video/geode/lxfb.h
+++ b/drivers/video/geode/lxfb.h
@@ -25,13 +25,12 @@ void lx_set_mode(struct fb_info *);
 void lx_get_gamma(struct fb_info *, unsigned int *, int);
 void lx_set_gamma(struct fb_info *, unsigned int *, int);
 unsigned int lx_framebuffer_size(void);
-int lx_shutdown(struct fb_info *);
-int lx_powerup(struct fb_info *);
 int lx_blank_display(struct fb_info *, int);
 void lx_set_palette_reg(struct fb_info *, unsigned int, unsigned int,
 			unsigned int, unsigned int);
 
-
+void lx_save_regs(struct fb_info *);
+void lx_restore_regs(struct fb_info *);
 
 /* ioctl() defines */
 
diff --git a/drivers/video/geode/lxfb_core.c b/drivers/video/geode/lxfb_core.c
index 29197e2..5fe8c30 100644
--- a/drivers/video/geode/lxfb_core.c
+++ b/drivers/video/geode/lxfb_core.c
@@ -302,8 +302,6 @@ static struct fb_ops lxfb_ops = {
 	.fb_fillrect	= cfb_fillrect,
 	.fb_copyarea	= cfb_copyarea,
 	.fb_imageblit	= cfb_imageblit,
-	.fb_powerdown	= lx_shutdown,
-	.fb_powerup	= lx_powerup,
 };
 
 static struct fb_info * __init lxfb_init_fbinfo(struct device *dev)
@@ -357,7 +355,7 @@ static int lxfb_suspend(struct pci_dev *pdev,  pm_message_t state)
 	if (state.event == PM_EVENT_SUSPEND) {
 
 		acquire_console_sem();
-		lx_shutdown(info);
+		lx_save_regs(info);
 		fb_set_suspend(info, 1);
 		release_console_sem();
 	}
@@ -372,9 +370,9 @@ static int lxfb_resume(struct pci_dev *pdev)
 
 	acquire_console_sem();
 
-	/* Turn the engine completely on */
+	/* Restore the engine to its former glory */
 
-	lx_powerup(info);
+	lx_restore_regs(info);
 	fb_set_suspend(info, 0);
 	release_console_sem();
 
diff --git a/drivers/video/geode/lxfb_ops.c b/drivers/video/geode/lxfb_ops.c
index b2ecb4d..8e97641 100644
--- a/drivers/video/geode/lxfb_ops.c
+++ b/drivers/video/geode/lxfb_ops.c
@@ -526,68 +526,110 @@ void lx_set_palette_reg(struct fb_info *info, unsigned regno,
 	writel(val, par->dc_regs + DC_PAL_DATA);
 }
 
+static int lx_blank_mode = FB_BLANK_UNBLANK;
+
 int lx_blank_display(struct fb_info *info, int blank_mode)
 {
 	struct lxfb_par *par = info->par;
-	u32 dcfg, fp_pm;
-	int blank, hsync, vsync;
+	u32 dcfg, val;
+
+	if (blank_mode == lx_blank_mode)
+		return 0;
+
+	writel(DC_UNLOCK_CODE, par->dc_regs + DC_UNLOCK);
+
+	if (lx_blank_mode == FB_BLANK_POWERDOWN) {
+		val = readl(par->df_regs + DF_FP_PM);
+		writel(val | DF_FP_PM_P, par->df_regs + DF_FP_PM);
 
-	/* CRT power saving modes. */
-	switch (blank_mode) {
+		val = readl(par->df_regs + DF_MISC) & ~DF_MISC_DAC_PWRDN;
+		writel(val, par->df_regs + DF_MISC);
+
+		val = readl(par->dc_regs + DC_DISPLAY_CFG) | DC_DCFG_TGEN;
+		writel(val, par->dc_regs + DC_DISPLAY_CFG);
+
+		val = readl(par->dc_regs + DC_GENERAL_CFG);
+		val |= DC_GCFG_CMPE | DC_GCFG_DECE | DC_GCFG_DFLE;
+		writel(val, par->dc_regs + DC_GENERAL_CFG);
+	}
+
+	dcfg = readl(par->dc_regs + DF_DISPLAY_CFG);
+
+	switch(blank_mode) {
 	case FB_BLANK_UNBLANK:
-		blank = 0; hsync = 1; vsync = 1;
+		dcfg |= DF_DCFG_CRT_EN | DF_DCFG_HSYNC_EN | DF_DCFG_VSYNC_EN;
+		dcfg &= ~DF_DCFG_DAC_BL_EN;
 		break;
 	case FB_BLANK_NORMAL:
-		blank = 1; hsync = 1; vsync = 1;
+		dcfg |= DF_DCFG_DAC_BL_EN | DF_DCFG_CRT_EN;
+		dcfg &= ~(DF_DCFG_HSYNC_EN | DF_DCFG_VSYNC_EN);
 		break;
 	case FB_BLANK_VSYNC_SUSPEND:
-		blank = 1; hsync = 1; vsync = 0;
+		dcfg |= DF_DCFG_CRT_EN | DF_DCFG_DAC_BL_EN | DF_DCFG_HSYNC_EN;
+		dcfg &= ~DF_DCFG_VSYNC_EN;
 		break;
 	case FB_BLANK_HSYNC_SUSPEND:
-		blank = 1; hsync = 0; vsync = 1;
+		dcfg |= DF_DCFG_CRT_EN | DF_DCFG_DAC_BL_EN | DF_DCFG_VSYNC_EN;
+		dcfg &= ~DF_DCFG_HSYNC_EN;
 		break;
+
 	case FB_BLANK_POWERDOWN:
-		blank = 1; hsync = 0; vsync = 0;
+		dcfg |= DF_DCFG_DAC_BL_EN;
+		dcfg &= ~(DF_DCFG_CRT_EN | DF_DCFG_HSYNC_EN | DF_DCFG_VSYNC_EN);
 		break;
 	default:
+		writel(0, par->dc_regs + DC_UNLOCK);
 		return -EINVAL;
 	}
 
-	dcfg = readl(par->df_regs + DF_DISPLAY_CFG);
-	dcfg &= ~(DF_DCFG_DAC_BL_EN
-		  | DF_DCFG_HSYNC_EN | DF_DCFG_VSYNC_EN);
-	if (!blank)
-		dcfg |= DF_DCFG_DAC_BL_EN;
-	if (hsync)
-		dcfg |= DF_DCFG_HSYNC_EN;
-	if (vsync)
-		dcfg |= DF_DCFG_VSYNC_EN;
+	/* Write the display configuration */
 	writel(dcfg, par->df_regs + DF_DISPLAY_CFG);
 
-	/* Power on/off flat panel */
+	/* Turn off the engine when we are in power down mode */
 
-	if (par->output & OUTPUT_PANEL) {
-		fp_pm = readl(par->df_regs + DF_FP_PM);
-		if (blank_mode == FB_BLANK_POWERDOWN)
-			fp_pm &= ~DF_FP_PM_P;
-		else
-			fp_pm |= DF_FP_PM_P;
-		writel(fp_pm, par->df_regs + DF_FP_PM);
+	if (blank_mode == FB_BLANK_POWERDOWN) {
+		u32 gcfg;
+
+		val = readl(par->df_regs + DF_MISC) | DF_MISC_DAC_PWRDN;
+		writel(val, par->df_regs + DF_MISC);
+
+		gcfg = readl(par->dc_regs + DC_GENERAL_CFG);
+		gcfg &= ~(DC_GCFG_CMPE | DC_GCFG_DECE);
+		writel(gcfg, par->dc_regs + DC_GENERAL_CFG);
+
+		val = readl(par->dc_regs + DC_DISPLAY_CFG);
+		val &= ~DC_DCFG_TGEN;
+		writel(val, par->dc_regs + DC_DISPLAY_CFG);
+
+		udelay(1000);
+
+		gcfg &= ~DC_GCFG_DFLE;
+		writel(gcfg, par->dc_regs + DC_GENERAL_CFG);
+
+		val = readl(par->df_regs + DF_FP_PM);
+		writel(val & ~DF_FP_PM_P, par->df_regs + DF_FP_PM);
 	}
 
+	writel(0, par->dc_regs + DC_UNLOCK);
+
+	lx_blank_mode = blank_mode;
 	return 0;
 }
 
 static struct geoderegs saved_regs;
 
-static void lx_save_regs(struct fb_info *info, struct geoderegs *regs)
+void lx_save_regs(struct fb_info *info)
 {
 	struct lxfb_par *par = info->par;
+	struct geoderegs *regs = &saved_regs;
 	int i;
 
 	/* Wait for the command buffer to empty */
 	while(!(readl(par->gp_regs + 0x44) & (1 << 4)));
 
+	/* Unlock the DC */
+	writel(DC_UNLOCK_CODE, par->dc_regs + DC_UNLOCK);
+
 	rdmsrl(MSR_LX_DF_PADSEL, regs->msr.padsel);
 	rdmsrl(MSR_LX_GLCP_DOTPLL, regs->msr.dotpll);
 	rdmsrl(MSR_LX_DF_GLCONFIG, regs->msr.dfglcfg);
@@ -612,9 +654,10 @@ static void lx_save_regs(struct fb_info *info, struct geoderegs *regs)
 		regs->gamma[i] = readl(par->df_regs + 0x40);
 }
 
-static void lx_restore_regs(struct fb_info *info, struct geoderegs *regs)
+void lx_restore_regs(struct fb_info *info)
 {
 	struct lxfb_par *par = info->par;
+	struct geoderegs *regs = &saved_regs;
 	u32 val, i;
 
 	/* == DOTPLL == */
@@ -827,35 +870,7 @@ static void lx_restore_regs(struct fb_info *info, struct geoderegs *regs)
 	writel((u32) regs->vp.r.vcfg, par->df_regs + 0x00);
 	writel((u32) regs->vp.r.dcfg, par->df_regs + 0x08);
 	writel(regs->dc.r.dcfg, par->dc_regs + 0x08);
-}
-
-static int lx_power_on = 1;
-
-int lx_shutdown(struct fb_info *info)
-{
-	struct lxfb_par *par = info->par;
-
-	if (lx_power_on == 0)
-		return 0;
 
-	writel(DC_UNLOCK_CODE, par->dc_regs + DC_UNLOCK);
-	lx_save_regs(info, &saved_regs);
-	lx_graphics_disable(info);
-
-	lx_power_on = 0;
-	return 0;
-}
-
-int lx_powerup(struct fb_info *info)
-{
-	struct lxfb_par *par = info->par;
-
-	if (lx_power_on == 1)
-		return 0;
-
-	lx_restore_regs(info, &saved_regs);
+	/* Lock the DC */
 	writel(0, par->dc_regs + DC_UNLOCK);
-
-	lx_power_on = 1;
-	return 0;
 }
diff --git a/drivers/video/geode/video_gx.c b/drivers/video/geode/video_gx.c
index e282e74..62a9f69 100644
--- a/drivers/video/geode/video_gx.c
+++ b/drivers/video/geode/video_gx.c
@@ -494,34 +494,6 @@ static int gx_blank_display(struct fb_info *info, int blank_mode)
 	return 0;
 }
 
-extern struct fb_info *gxfb_info;
-
-/* This function controls the flatpanel power sequencing - this is used
-   by the OLPC power management engine to enable the FP sequencing much
-   earlier in the resume process
-*/
-
-void gxfb_flatpanel_control(int state)
-{
-	struct geodefb_par *par = gxfb_info->par;
-	u32 val, fp = readl(par->vid_regs + GX_FP_PM);
-	val  = fp;
-
-	/* Turn on the panel if it isn't aleady */
-
-	if (state) {
-		if (!(val & 0x01))
-			val |= GX_FP_PM_P;
-	}
-	else {
-		if (!(val & 0x02))
-			val &= ~GX_FP_PM_P;
-	}
-
-	if (val != fp)
-		writel(val, par->vid_regs + GX_FP_PM);
-}
-
 struct geode_vid_ops gx_vid_ops = {
 	.set_dclk	   = gx_set_dclk_frequency,
 	.get_dclk          = gx_get_dclk,
diff --git a/drivers/video/olpc_dcon.c b/drivers/video/olpc_dcon.c
index 47ade47..486d4a1 100644
--- a/drivers/video/olpc_dcon.c
+++ b/drivers/video/olpc_dcon.c
@@ -208,19 +208,11 @@ static void dcon_source_switch(struct work_struct *work)
 		if (!dcon_switched)
 			printk(KERN_ERR "olpc-dcon:  Timeout entering CPU mode; expect a screen glitch.\n");
 
-		/*
-		 * Ideally we'd like to disable interrupts here so that the
-		 * fb_powerup and DCON turn on happen at a known time value;
-		 * however, we can't do that right now with fb_set_suspend
-		 * messing with semaphores.
-		 *
-		 * For now, we just hope..
+		/* FIXME:  Now that we use fb_blank() we may be able to 
+		 * turn off interrupts here
 		 */
-		if (fb_powerup(fbinfo)) {
-			printk(KERN_ERR "olpc-dcon:  Failed to enter CPU mode\n");
-			dcon_pending = DCON_SOURCE_DCON;
-			return;
-		}
+
+		fb_blank(fbinfo, FB_BLANK_UNBLANK);
 
 		/* And turn off the DCON */
 		outl(1<<11, gpio_base + GPIOx_OUT_VAL);
@@ -250,8 +242,7 @@ static void dcon_source_switch(struct work_struct *work)
 		if (!dcon_switched)
 			printk(KERN_ERR "olpc-dcon: Timeout entering DCON mode; expect a screen glitch.\n");
 
-		/* Turn off the graphics engine completely */
-		fb_powerdown(fbinfo);
+		fb_blank(fbinfo, FB_BLANK_POWERDOWN);
 
 		printk(KERN_INFO "olpc-dcon: The DCON has control\n");
 		break;
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 5a82c83..6622682 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -661,12 +661,6 @@ struct fb_ops {
 	/* restore saved state */
 	void (*fb_restore_state)(struct fb_info *info);
 
-	/* Shut down the graphics engine to save power */
-	int (*fb_powerdown)(struct fb_info *info);
-
-	/* Power it back up */
-	int (*fb_powerup)(struct fb_info *info);
-
 	/* get capability given var */
 	void (*fb_get_caps)(struct fb_info *info, struct fb_blit_caps *caps,
 			    struct fb_var_screeninfo *var);
@@ -946,9 +940,6 @@ extern int fb_get_color_depth(struct fb_var_screeninfo *var,
 extern int fb_get_options(char *name, char **option);
 extern int fb_new_modelist(struct fb_info *info);
 
-extern int fb_powerdown(struct fb_info *info);
-extern int fb_powerup(struct fb_info *info);
-
 extern struct fb_info *registered_fb[FB_MAX];
 extern int num_registered_fb;
 extern struct class *fb_class;


More information about the Devel mailing list