video: amba_clcd: Clock balance fixes and power optimization changes
These patches fix clock balance issues (clk_disable called when clk_enabled was not previously called) and optimizes clocking for register access.
[PATCH 1/2] video: amba_clcd: Fix driver clock enable/disable balance issues [PATCH 2/2] video: amba_clcd: Seperate controller and register clock enables
Patch 1 is basically RK's suggested fix repackaged here.
The register clocking fix was written to avoid calling the AMBA clock enable and disable functions as nested calls (ie, 2 calls to disable). The number of calls can be reduced by a few calls if nesting can be used. This depends on the arch's clock drivers and whether they support nesting. The method used here is safe for clock drivers that don't like nested calls.
These patches can be pulled from:
git://lpclinux.com/linux-2.6-lpc pl11x_clk_fixes
PATCH 1/2 video: amba_clcd: Fix driver clock enable/disable balance issues
From: Kevin Wells <[email protected]>
The amba clcd clock enable and disable calls can become unbalanced. This usually occurs on the first call to clcdfb_set_par() during LCD init, but can also occur when the LCD enters and exits blank state. This fix prevents clk_disable() from being called until clk_enable() has been called first.
--- drivers/video/amba-clcd.c | 10 ++++++++-- include/linux/amba/clcd.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-)
drivers/video/amba-clcd.c +8 -2
@@ -80,7 +80,10 @@ static void clcdfb_disable(struct clcd_fb *fb)/* * Disable CLCD clock source. */ -clk_disable (fb->clk); +if (fb->clk_enabled) { + fb->clk_enabled = false; +clk_disable (fb->clk); + } }static void clcdfb_enable (struct clcd_fb *fb,u32 cntl)
@@ -88,7 +91,10 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)/* * Enable the CLCD clock source. */ -clk_enable (fb->clk); +if (!fb->clk_enabled) { + fb->clk_enabled = true; +clk_enable (fb->clk); + }/* * Bring up by first enabling..
include/linux/amba/clcd.h +1 -0
@@ -142,6 +142,7 @@ struct clcd_fb {struct fb_info fb;struct amba_device *dev;struct clk *clk; +bool clk_enabled;struct clcd_panel *panel;struct clcd_board *board;void *board_data;
Signature · 1 line
1.7.1.1
PATCH 1/2 video: amba_clcd: Fix driver clock enable/disable balance issues
Quoted 9 lines · On Wed, Aug 11, 2010 at 04:03:03PM -0700, wellsk40 at gmail.com wrote:
From: Kevin Wells <[email protected]>
The amba clcd clock enable and disable calls can become unbalanced. This usually occurs on the first call to
clcdfb_set_par()during LCD init, but can also occur when the LCD enters and exits blank state. This fix preventsclk_disable()from being called untilclk_enable()has been called first.Signed-off-by Kevin Wells <[email protected]>
Not quite the right way to do things, because this makes yourself the author of the patch which is obviously wrong.
I'll commit it myself now, as I'm about to send stuff to Linus. I'm going to assume you've tested it, but as I don't know positively I can't add an attributation for it.
PATCH 1/2 video: amba_clcd: Fix driver clock enable/disable balance issues
Quoted 10 lines · On Wed, Aug 11, 2010 at 04:03:03PM -0700, wellsk40 at gmail.com wrote:
Quoted 8 lines
From: Kevin Wells <[email protected]>
The amba clcd clock enable and disable calls can become unbalanced. This usually occurs on the first call to
clcdfb_set_par()during LCD init, but can also occur when the LCD enters and exits blank state. This fix preventsclk_disable()from being called untilclk_enable()has been called first.
Quoted 9 lines
Quoted 1 line
Signed-off-by Kevin Wells <[email protected]>Not quite the right way to do things, because this makes yourself the author of the patch which is obviously wrong.
I'll commit it myself now, as I'm about to send stuff to Linus. I'm going to assume you've tested it, but as I don't know positively I can't add an attributation for it.
Thanks for checking that in.
Sorry - I had sent another mail earlier a few weeks ago asking if you wanted to check in that change (patch 1), but I think it got lost in the shuffle.
Both patches are tested. You can add a "Tested by" from me if it's not too late.
PATCH 2/2 video: amba_clcd: Seperate controller and register clock enables
In reply to wellsk40 at gmail.comFrom: Kevin Wells <[email protected]>
To optimize power for architectures that disable the clock to the clcd primecell, clock enable logic for the controller (maintaining the LCD) and accessing the registers (AMBA bus clock) has been seperated.
When accessing registers, the register clock is enabled prior to access and then disabled when accesses are complete. The clcd probe function will disable the clock enabled by the AMBA bus driver prior to exiting probe. The clock is re-enabled prior to exiting remove. amba_pclk_enable() and amba_pclk_disable() functions macros are used for register clock enable/disable. All calls to the macros are balanced and un-nested.
--- drivers/video/amba-clcd.c | 20 ++++++++++++++++++++ 1 files changed, 20 insertions(+), 0 deletions(-)
drivers/video/amba-clcd.c +20 -0
@@ -65,6 +65,8 @@ static void clcdfb_disable(struct clcd_fb *fb)if (fb->board->disable) fb->board->disable (fb); +amba_pclk_enable (fb->dev); + val =readl (fb->regs + fb->off_cntl);if (val &CNTL_LCDPWR ) { val &= ~CNTL_LCDPWR ;
@@ -77,6 +79,8 @@ static void clcdfb_disable(struct clcd_fb *fb)writel (val, fb->regs + fb->off_cntl); } +amba_pclk_disable (fb->dev); +/* * Disable CLCD clock source. */
@@ -96,6 +100,8 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl)clk_enable (fb->clk); } +amba_pclk_enable (fb->dev); +/* * Bring up by first enabling.. */
@@ -110,6 +116,8 @@ static void clcdfb_enable(struct clcd_fb *fb, u32 cntl) cntl |=CNTL_LCDPWR ;writel (cntl, fb->regs + fb->off_cntl); +amba_pclk_disable (fb->dev); +/* * finally, enable the interface. */
@@ -217,6 +225,7 @@ static int clcdfb_set_par(struct fb_info *info) fb->board->decode (fb, ®s);clcdfb_disable (fb); +amba_pclk_enable (fb->dev);writel (regs.tim0, fb->regs +CLCD_TIM0 );writel (regs.tim1, fb->regs +CLCD_TIM1 );
@@ -229,9 +238,11 @@ static int clcdfb_set_par(struct fb_info *info) fb->clcd_cntl = regs.cntl; +amba_pclk_disable (fb->dev);clcdfb_enable (fb, regs.cntl);#ifdef DEBUG +amba_pclk_enable (fb->dev);printk (KERN_INFO "CLCD: Registers set to\n" " %08x %08x %08x %08x\n"
@@ -240,6 +251,7 @@ static int clcdfb_set_par(struct fb_info *info)readl (fb->regs +CLCD_TIM2 ),readl (fb->regs +CLCD_TIM3 ),readl (fb->regs +CLCD_UBAS ),readl (fb->regs +CLCD_LBAS ), readl(fb->regs + fb->off_ienb), readl(fb->regs + fb->off_cntl)); +amba_pclk_disable (fb->dev);#endif return 0 ;
@@ -290,8 +302,10 @@ clcdfb_setcolreg(unsigned int regno, unsigned int red, unsigned int green, mask =0xffff0000 ; } +amba_pclk_enable (fb->dev); val =readl (fb->regs + hw_reg) & mask;writel (val | newval, fb->regs + hw_reg); +amba_pclk_disable (fb->dev); }return regno >255 ;
@@ -492,6 +506,9 @@ static int clcdfb_probe(struct amba_device *dev, struct amba_id *id) ret =clcdfb_register (fb);if (ret ==0 ) { +/* Disable AMBA PCLK */ +amba_pclk_disable (dev); +amba_set_drvdata (dev, fb); goto out; }
@@ -524,6 +541,9 @@ static int clcdfb_remove(struct amba_device *dev)amba_release_regions (dev); +/* Restore AMBA PCLK */ +amba_pclk_enable (dev); +return 0 ; }
Signature · 1 line
1.7.1.1
Quoted 7 lines · On Wed, Aug 11, 2010 at 4:03 PM, <[email protected]> wrote:
These patches fix clock balance issues (
clk_disablecalled whenclk_enabledwas not previously called) and optimizes clocking for register access.[PATCH 1/2] video:
amba_clcd: Fix driver clock enable/disable balance issues [PATCH 2/2] video:amba_clcd: Seperate controller and register clock enables
Quoted 9 lines
Patch 1 is basically RK's suggested fix repackaged here.
The register clocking fix was written to avoid calling the AMBA clock enable and disable functions as nested calls (ie, 2 calls to disable). The number of calls can be reduced by a few calls if nesting can be used. This depends on the arch's clock drivers and whether they support nesting. The method used here is safe for clock drivers that don't like nested calls.
I haven't heard any feedback on these patch suggestions. Patch 1 is needed. Patch 2 is highly desirable to keep power usage to a minimum when the lcd is off.
Quoted 4 lines
These patches can be pulled from:
? ? ? ?git://lpclinux.com/linux-2.6-lpcpl11x_clk_fixes