atmel_lcdfb: suspend/resume support

From: David Brownell <[email protected]>

Teach atmel_lcdfb driver how to suspend/resume.

Note that the backlight control should probably do more of the same stuff:
turning off display power (more than just the backlight) and stopping the clocks (and dma to drive the no-longer-seen display). No point in wasting power to generate images that can't be observed, after all...

---
It applies on top of :
atmel_lcdfb-dont-initialize-a-pre-allocated-framebuffer.patch already in the -mm tree.

drivers/video/atmel_lcdfb.c | 36 ++++++++++++++++++++++++++++++++++--
include/video/atmel_lcdc.h | 1 +

2 files changed, 35 insertions(+), 2 deletions(-)

@@ -909,10 +909,42 @@ static int __exit atmel_lcdfb_remove(str

	return 0;
}

+#ifdef CONFIG_PM
+
+static int atmel_lcdfb_suspend(struct platform_device *pdev, pm_message_t mesg) +{
+ struct fb_info *info = platform_get_drvdata(pdev);
+ struct atmel_lcdfb_info *sinfo = info->par;
+
+ sinfo->saved_lcdcon = lcdc_readl(sinfo, ATMEL_LCDC_CONTRAST_VAL);
+ lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, 0);
+ if (sinfo->atmel_lcdfb_power_control)
+ sinfo->atmel_lcdfb_power_control(0);
+ atmel_lcdfb_stop_clock(sinfo);
+ return 0;
+}
+
+static int atmel_lcdfb_resume(struct platform_device *pdev)
+{
+ struct fb_info *info = platform_get_drvdata(pdev);
+ struct atmel_lcdfb_info *sinfo = info->par;
+
+ atmel_lcdfb_start_clock(sinfo);
+ if (sinfo->atmel_lcdfb_power_control)
+ sinfo->atmel_lcdfb_power_control(1);
+ lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);
+ return 0;
+}
+
+#else
+#define atmel_lcdfb_suspend NULL
+#define atmel_lcdfb_resume NULL
+#endif
+

static struct platform_driver atmel_lcdfb_driver = {
	.remove		= __exit_p(atmel_lcdfb_remove),

-
-// FIXME need suspend, resume
+ .suspend = atmel_lcdfb_suspend,
+ .resume = atmel_lcdfb_resume,

	.driver		= {
		.name	= "atmel_lcdfb",

@@ -39,6 +39,7 @@ struct atmel_lcdfb_info {

	u8			bl_power;
#endif
	bool			lcdcon_is_backlight;

+ u8 saved_lcdcon;

	u8			default_bpp;
	unsigned int		default_lcdcon2;

Yeah, why don't registers named LCDCONx have nothing to do with LCD CONtrast? Better to have named the PWM registers PWM ... and say they could be used for either contrast or backlight control.

The saved contrast/backlight counter value CONTRAST_VAL is one of two PWM control registers.

That's forces the contrast signal low and disables the PWM counter, so it won't "randomly" leave the PWM output high when the clock is stopped (leaving at least some displays lit during suspend).

It's possible that only CTR needed to be saved; all the backlight support in this driver still needs work.

Yeah, that looks wrong; the patch below makes more sense. Though it *does* behave right for some reason...

- Dave

@@ -926,7 +926,10 @@ static int atmel_lcdfb_resume(struct pla
 	atmel_lcdfb_start_clock(sinfo);
 	if (sinfo->atmel_lcdfb_power_control)
 		sinfo->atmel_lcdfb_power_control(1);
-	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, sinfo->saved_lcdcon);
+	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, sinfo->saved_lcdcon);
+	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR,
+			sinfo->saved_lcdcon ? contrast_ctr : 0);
+
 	return 0;
 }
 

While I admit you have a point, I think it's easier to change the name of that field than changing the hardware documentation at this point ;)

I know.

I'm not saying it's the wrong thing to do. I just think it's strange that you alter a different register than the one you saved.

Yes, I do think we can assume VAL stays unchanged during suspend.

Yes, that looks better. Perhaps you used a contrast value that happened to set the right bits when written to CTR?

I still think the name of the saved_lcdcon field is confusing though.

Haavard