Skip to contentSkip to conversation

atmel_lcdfb: backlight control

PATCH In review Nicolas Ferre 3 participants 4 messages Jan 18, 2008 – Jan 24, 2008

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 atmel_lcdfb: backlight control

From: David Brownell <[email protected]>

On the sam9 EK boards, the LCD backlight is hooked up to a PWM output from the LCD controller. It's controlled by "contrast" registers though.

This patch lets boards declare that they have that kind of backlight control. The driver can then export this control, letting screenblank and other operations actually take effect ... reducing the typically substantial power drain from the backlight.

---
Note that it's not fully cooked
- doesn't force backlight off during system suspend
- the "power" and "blank" events may not be done right

This should be easily added in the future.

 arch/arm/mach-at91/board-sam9261ek.c |    1 
 arch/arm/mach-at91/board-sam9263ek.c |    1 
 drivers/video/atmel_lcdfb.c          |  114 +++++++++++++++++++++++++++++++++--
 drivers/video/backlight/Kconfig      |   13 +++
 include/video/atmel_lcdc.h           |   11 ++-
 5 files changed, 134 insertions(+), 6 deletions(-)
arch/arm/mach-at91/board-sam9261ek.c +1 -0
@@ -322,6 +322,7 @@ static void at91_lcdc_power_control(int 
 
 /* Driver datas */
 static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
+	.lcdcon_is_backlight		= true,
 	.default_bpp			= 16,
 	.default_dmacon			= ATMEL_LCDC_DMAEN,
 	.default_lcdcon2		= AT91SAM9261_DEFAULT_LCDCON2,
arch/arm/mach-at91/board-sam9263ek.c +1 -0
@@ -250,6 +250,7 @@ static void at91_lcdc_power_control(int 
 
 /* Driver datas */
 static struct atmel_lcdfb_info __initdata ek_lcdc_data = {
+	.lcdcon_is_backlight		= true,
 	.default_bpp			= 16,
 	.default_dmacon			= ATMEL_LCDC_DMAEN,
 	.default_lcdcon2		= AT91SAM9263_DEFAULT_LCDCON2,
drivers/video/atmel_lcdfb.c +110 -4

156 lines not on this page · raw message

drivers/video/backlight/Kconfig +13 -0
@@ -50,6 +50,19 @@ config BACKLIGHT_CLASS_DEVICE
 	  To have support for your specific LCD panel you will have to
 	  select the proper drivers which depend on this option.
 
+config BACKLIGHT_ATMEL_LCDC
+	bool "Atmel LCDC Contrast-as-Backlight control"
+	depends on BACKLIGHT_CLASS_DEVICE && FB_ATMEL
+	default y if MACH_SAM9261EK || MACH_SAM9263EK
+	help
+	  This provides a backlight control internal to the Atmel LCDC
+	  driver.  If the LCD "contrast control" on your board is wired
+	  so it controls the backlight brightness, select this option to
+	  export this as a PWM-based backlight control.
+
+	  If in doubt, it's safe to enable this option; it doesn't kick
+	  in unless the board's description says it's wired that way.
+
 config BACKLIGHT_CORGI
 	tristate "Generic (aka Sharp Corgi) Backlight Driver"
 	depends on BACKLIGHT_CLASS_DEVICE
include/video/atmel_lcdc.h +9 -2
@@ -22,7 +22,7 @@
 #ifndef __ATMEL_LCDC_H__
 #define __ATMEL_LCDC_H__
 
- /* LCD Controller info data structure */
+ /* LCD Controller info data structure, stored in device platform_data */
 struct atmel_lcdfb_info {
 	spinlock_t		lock;
 	struct fb_info		*info;
@@ -33,7 +33,14 @@ struct atmel_lcdfb_info {
 	struct platform_device	*pdev;
 	struct clk		*bus_clk;
 	struct clk		*lcdc_clk;
-	unsigned int		default_bpp;
+
+#ifdef CONFIG_BACKLIGHT_ATMEL_LCDC
+	struct backlight_device	*backlight;
+	u8			bl_power;
+#endif
+	bool			lcdcon_is_backlight;
+
+	u8			default_bpp;
 	unsigned int		default_lcdcon2;
 	unsigned int		default_dmacon;
 	void (*atmel_lcdfb_power_control)(int on);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#1 lore raw
Quoted 5 lines · On Fri, 18 Jan 2008 15:44:35 +0100 Nicolas Ferre <[email protected]>…

From: David Brownell <[email protected]>

On the sam9 EK boards, the LCD backlight is hooked up to a PWM output from the LCD controller. It's controlled by "contrast" registers though.

Quoted 7 lines

This patch lets boards declare that they have that kind of backlight control. The driver can then export this control, letting screenblank and other operations actually take effect ... reducing the typically substantial power drain from the backlight.

Looks good to me. It won't affect any current AVR32 boards, although that may change in the future.

A couple of minor comments below.

drivers/video/atmel_lcdfb.c static void atmel_lcdfb_update_dma2d(str · line 119

Quoted 4 lines
drivers/video/atmel_lcdfb.c +4 -0
+static struct backlight_ops atmel_lcdc_bl_ops = {
+	.update_status = atmel_bl_update_status,
+	.get_brightness = atmel_bl_get_brightness,
+};

This can be const, no?

No it can't, since backlight_device_register() takes a non-const pointer for some reason...

drivers/video/atmel_lcdfb.c static void atmel_lcdfb_update_dma2d(str · line 162

Quoted 12 lines
drivers/video/atmel_lcdfb.c +12 -0
+#else
+
+static inline void init_backlight(struct atmel_lcdfb_info *sinfo)
+{
+	dev_warn(&sinfo->pdev->dev, "backlight control is not available\n");
+}
+
+static inline void exit_contrast(struct atmel_lcdfb_info *sinfo)
+{
+}
+
+#endif

This looks a bit asymmetric...

Quoted 43 lines
drivers/video/atmel_lcdfb.c +14 -4
+static inline void init_contrast(struct atmel_lcdfb_info *sinfo)
+{
+	/* have some default contrast/backlight settings */
+	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, contrast_ctr);
+	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, ATMEL_LCDC_CVAL_DEFAULT);
+
+	if (sinfo->lcdcon_is_backlight)
+		init_backlight(sinfo);
+}
+
 
 static struct fb_fix_screeninfo atmel_lcdfb_fix __initdata = {
 	.type		= FB_TYPE_PACKED_PIXELS,
@@ -370,10 +472,6 @@ static int atmel_lcdfb_set_par(struct fb
 	/* Disable all interrupts */
 	lcdc_writel(sinfo, ATMEL_LCDC_IDR, ~0UL);
 
-	/* Set contrast */
-	value = ATMEL_LCDC_PS_DIV8 | ATMEL_LCDC_POL_POSITIVE | ATMEL_LCDC_ENA_PWMENABLE;
-	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, value);
-	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_VAL, ATMEL_LCDC_CVAL_DEFAULT);
 	/* ...wait for DMA engine to become idle... */
 	while (lcdc_readl(sinfo, ATMEL_LCDC_DMACON) & ATMEL_LCDC_DMABUSY)
 		msleep(10);
@@ -577,6 +675,7 @@ static int __init atmel_lcdfb_probe(stru
 		sinfo->default_monspecs = pdata_sinfo->default_monspecs;
 		sinfo->atmel_lcdfb_power_control = pdata_sinfo->atmel_lcdfb_power_control;
 		sinfo->guard_time = pdata_sinfo->guard_time;
+		sinfo->lcdcon_is_backlight = pdata_sinfo->lcdcon_is_backlight;
 	} else {
 		dev_err(dev, "cannot get default configuration\n");
 		goto free_info;
@@ -670,6 +769,9 @@ static int __init atmel_lcdfb_probe(stru
 		goto release_mem;
 	}
 
+	/* Initialize PWM for contrast or backlight ("off") */
+	init_contrast(sinfo);
+
 	/* interrupt */
 	ret = request_irq(sinfo->irq_base, atmel_lcdfb_interrupt, 0, pdev->name, info);
 	if (ret) {

drivers/video/atmel_lcdfb.c static int __exit atmel_lcdfb_remove(str · line 860

Quoted 4 lines
drivers/video/atmel_lcdfb.c +1 -0
@@ -755,6 +857,7 @@ static int __exit atmel_lcdfb_remove(str
 	if (!sinfo)
 		return 0;
 
+	exit_contrast(sinfo);

Missing exit_contrast() in probe() error path?

Haavard

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#2 lore raw
In reply to Nicolas Ferre
Quoted 11 lines · On Fri, 18 Jan 2008 15:44:35 +0100 Nicolas Ferre <[email protected]>…

From: David Brownell <[email protected]>

On the sam9 EK boards, the LCD backlight is hooked up to a PWM output from the LCD controller. It's controlled by "contrast" registers though.

This patch lets boards declare that they have that kind of backlight control. The driver can then export this control, letting screenblank and other operations actually take effect ... reducing the typically substantial power drain from the backlight.

drivers/video/atmel_lcdfb.c static void atmel_lcdfb_update_dma2d(str · line 164

Quoted 5 lines
drivers/video/atmel_lcdfb.c +5 -0
+static inline void init_backlight(struct atmel_lcdfb_info *sinfo)
+static inline void exit_contrast(struct atmel_lcdfb_info *sinfo)
+static inline void init_backlight(struct atmel_lcdfb_info *sinfo)
+static inline void exit_contrast(struct atmel_lcdfb_info *sinfo)
+static inline void init_contrast(struct atmel_lcdfb_info *sinfo)

You really don't need to add manual inlining. The compiler takes care of it.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

#3 lore raw

Re: [Linux-fbdev-devel] [PATCH] atmel_lcdfb: backlight control - tiny rework

In reply to Nicolas Ferre

Tiny patch that removes not needed "inline" and renames function : exit_contrast() -> exit_backlight(). It adds the missing exit_backlight() in probe() error path.

---
Follows comments from Andrew Morton and Haavard Skinnemoen. Thanks to both of you.

drivers/video/atmel_lcdfb.c | 13 +++++++------

1 file changed, 7 insertions(+), 6 deletions(-)
drivers/video/atmel_lcdfb.c +0 -0
@@ -118,7 +118,7 @@ static struct backlight_ops atmel_lcdc_b

.get_brightness = atmel_bl_get_brightness,
};

-static inline void init_backlight(struct atmel_lcdfb_info *sinfo) +static void init_backlight(struct atmel_lcdfb_info *sinfo)

{
	struct backlight_device	*bl;

@@ -142,7 +142,7 @@ static inline void init_backlight(struct

	bl->props.brightness = atmel_bl_get_brightness(bl);
}

-static inline void exit_contrast(struct atmel_lcdfb_info *sinfo) +static void exit_backlight(struct atmel_lcdfb_info *sinfo)

{
	if (sinfo->backlight)
		backlight_device_unregister(sinfo->backlight);

@@ -150,18 +150,18 @@ static inline void exit_contrast(struct

#else

-static inline void init_backlight(struct atmel_lcdfb_info *sinfo) +static void init_backlight(struct atmel_lcdfb_info *sinfo)

{
	dev_warn(&sinfo->pdev->dev, "backlight control is not available\n");
}

-static inline void exit_contrast(struct atmel_lcdfb_info *sinfo) +static void exit_backlight(struct atmel_lcdfb_info *sinfo) {
}

#endif

-static inline void init_contrast(struct atmel_lcdfb_info *sinfo) +static void init_contrast(struct atmel_lcdfb_info *sinfo)

{
	/* have some default contrast/backlight settings */
	lcdc_writel(sinfo, ATMEL_LCDC_CONTRAST_CTR, contrast_ctr);
@@ -823,6 +823,7 @@ free_cmap:
unregister_irqs:
	free_irq(sinfo->irq_base, info);
unmap_mmio:
+	exit_backlight(sinfo);
	iounmap(sinfo->mmio);
release_mem:
 	release_mem_region(info->fix.mmio_start, info->fix.mmio_len);
@@ -857,7 +858,7 @@ static int __exit atmel_lcdfb_remove(str
	if (!sinfo)
		return 0;

- exit_contrast(sinfo);

+	exit_backlight(sinfo);
	if (sinfo->atmel_lcdfb_power_control)
		sinfo->atmel_lcdfb_power_control(0);
	unregister_framebuffer(info);