video: s3c-fb: Use runtime suspend while blanked
cover video: s3c-fb: Use runtime suspend while blanked
This patch series (the first two of which I posted the other day) improves the runtime power management in the s3c-fb driver by moving it to keeping the device runtime suspended when the screen is powered down by userspace. This saves a trivial amount of power in the framebuffer controller and allows the SoC core code to enter system wide idle states which offer much more substantial gains.
Currently it's only lightly tested but it certainly seems to the right thing for me.
Mark Brown (6):
video: s3c-fb: Make runtime PM functional again
video: s3c-fb: Use s3c_fb_enable() to enable the framebuffer
video: s3c-fb: Disable runtime PM in error paths from probe
video: s3c-fb: Take a runtime PM reference when unblanked
video: s3c-fb: Hold runtime PM references when touching registers
video: s3c-fb: Don't keep device runtime active when open
drivers/video/s3c-fb.c | 127 ++++++++++++++++++++++++++++++++++------------- 1 files changed, 92 insertions(+), 35 deletions(-)
PATCH 1/6 video: s3c-fb: Make runtime PM functional again
The change in "video: s3c-fb: modify runtime pm functions" (commit 35784b) renders the runtime power management for the device completely ineffectual as while it leaves runtime power management notionally enabled a runtime power reference is held for the entire time the device is registered meaning it will never actually do anything.
A further issue is introduced as runtime power management is added during the system suspend path which is not something which drivers are supposed to do and would interact poorly if there were any operations done in the runtime power management callbacks.
While this does make things simpler (the main motivation for the original change) it will not only cause us to use more power in the framebuffer controller but will also prevent us entering lower power domain and SoC wide states as we can never power down the domain containing the device. Since neither of these things is desirable revert the change.
--- drivers/video/s3c-fb.c | 51 +++++++++++++++++++++++++++++------------------ 1 files changed, 31 insertions(+), 20 deletions(-)
drivers/video/s3c-fb.c +31 -20
@@ -1038,8 +1038,30 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,return ret; } +static int s3c_fb_open (struct fb_info *info,int user) +{ +struct s3c_fb_win *win = info->par; +struct s3c_fb *sfb = win->parent; + +pm_runtime_get_sync (sfb->dev); + +return 0 ; +} + +static int s3c_fb_release (struct fb_info *info,int user) +{ +struct s3c_fb_win *win = info->par; +struct s3c_fb *sfb = win->parent; + +pm_runtime_put_sync (sfb->dev); + +return 0 ; +} +static struct fb_ops s3c_fb_ops = { .owner =THIS_MODULE , + .fb_open = s3c_fb_open, + .fb_release = s3c_fb_release, .fb_check_var = s3c_fb_check_var, .fb_set_par = s3c_fb_set_par, .fb_blank = s3c_fb_blank,
@@ -1446,6 +1468,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev) }platform_set_drvdata (pdev, sfb); +pm_runtime_put_sync (sfb->dev);return 0 ;
@@ -1485,6 +1508,8 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)struct s3c_fb *sfb =platform_get_drvdata (pdev);int win; +pm_runtime_get_sync (sfb->dev); +for (win =0 ; win <S3C_FB_MAX_WIN ; win++)if (sfb->windows[win])s3c_fb_release_win (sfb, sfb->windows[win]);
@@ -1510,7 +1535,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)return 0 ; } -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_PM static int s3c_fb_suspend (struct device *dev) {struct platform_device *pdev =to_platform_device (dev);
@@ -1531,8 +1556,6 @@ static int s3c_fb_suspend(struct device *dev)clk_disable (sfb->lcd_clk);clk_disable (sfb->bus_clk); -pm_runtime_put_sync (sfb->dev); -return 0 ; }
@@ -1544,7 +1567,6 @@ static int s3c_fb_resume(struct device *dev)struct s3c_fb_win *win;int win_no; -pm_runtime_get_sync (sfb->dev);clk_enable (sfb->bus_clk);if (!sfb->variant.has_clksel)
@@ -1583,19 +1605,11 @@ static int s3c_fb_resume(struct device *dev) return 0; } +#else +#define s3c_fb_suspend NULL +#define s3c_fb_resume NULL#endif -#ifdef CONFIG_PM_RUNTIME -static int s3c_fb_runtime_suspend (struct device *dev) -{ -return 0 ; -} - -static int s3c_fb_runtime_resume (struct device *dev) -{ -return 0 ; -} -#endif #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))#define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
@@ -1918,10 +1932,7 @@ static struct platform_device_id s3c_fb_driver_ids[] = { };MODULE_DEVICE_TABLE (platform, s3c_fb_driver_ids); -static const struct dev_pm_ops s3c_fb_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume) -SET_RUNTIME_PM_OPS (s3c_fb_runtime_suspend, s3c_fb_runtime_resume,NULL ) -}; +static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);static struct platform_driver s3c_fb_driver = { .probe = s3c_fb_probe,
@@ -1930,7 +1941,7 @@ static struct platform_driver s3c_fb_driver = { .driver = { .name = "s3c-fb", .owner = THIS_MODULE, - .pm = &s3c_fb_pm_ops, + .pm = &s3cfb_pm_ops, }, };
Signature · 1 line
1.7.7.3
PATCH 2/6 video: s3c-fb: Use s3c_fb_enable() to enable the framebuffer
The s3c-fb driver has a function called s3c_fb_enable() which turns on and off the physical output. However it is only actually used in paths which disable the screen, the enabling just writes to the register. Make the code less confusing by ensuring that the enable also goes through the same path.
--- drivers/video/s3c-fb.c | 55 ++++++++++++++++++++++++----------------------- 1 files changed, 28 insertions(+), 27 deletions(-)
drivers/video/s3c-fb.c +28 -27
@@ -441,6 +441,32 @@ static void shadow_protect_win(struct s3c_fb_win *win, bool protect) }/** +* s3c_fb_enable() - Set the state of the main LCD output +* @sfb: The main framebuffer state. +* @enable: The state to set. +*/ +static void s3c_fb_enable (struct s3c_fb *sfb,int enable) +{ +u32 vidcon0 =readl (sfb->regs +VIDCON0 ); + +if (enable) + vidcon0 |=VIDCON0_ENVID |VIDCON0_ENVID_F ; +else { +/* see the note in the framebuffer datasheet about +* why you cannot take both of these bits down at the +* same time. */ + +if (!(vidcon0 &VIDCON0_ENVID )) +return ; + + vidcon0 |=VIDCON0_ENVID ; + vidcon0 &= ~VIDCON0_ENVID_F ; + } + +writel (vidcon0, sfb->regs +VIDCON0 ); +} + +/** * s3c_fb_set_par() - framebuffer request to set new framebuffer state. * @info: The framebuffer to change. *
@@ -510,9 +536,10 @@ static int s3c_fb_set_par(struct fb_info *info)if (sfb->variant.is_2443) data |= (1 <<5 ); - data |=VIDCON0_ENVID |VIDCON0_ENVID_F ;writel (data, regs +VIDCON0 ); +s3c_fb_enable (sfb,1 ); + data =VIDTCON0_VBPD (var->upper_margin -1 ) |VIDTCON0_VFPD (var->lower_margin -1 ) |VIDTCON0_VSPW (var->vsync_len -1 );
@@ -761,32 +788,6 @@ static int s3c_fb_setcolreg(unsigned regno, }/** -* s3c_fb_enable() - Set the state of the main LCD output -* @sfb: The main framebuffer state. -* @enable: The state to set. -*/ -static void s3c_fb_enable (struct s3c_fb *sfb,int enable) -{ -u32 vidcon0 =readl (sfb->regs +VIDCON0 ); - -if (enable) - vidcon0 |=VIDCON0_ENVID |VIDCON0_ENVID_F ; -else { -/* see the note in the framebuffer datasheet about -* why you cannot take both of these bits down at the -* same time. */ - -if (!(vidcon0 &VIDCON0_ENVID )) -return ; - - vidcon0 |=VIDCON0_ENVID ; - vidcon0 &= ~VIDCON0_ENVID_F ; - } - -writel (vidcon0, sfb->regs +VIDCON0 ); -} - -/** * s3c_fb_blank() - blank or unblank the given window * @blank_mode: The blank state from FB_BLANK_* * @info: The framebuffer to blank.
Signature · 1 line
1.7.7.3
PATCH 3/6 video: s3c-fb: Disable runtime PM in error paths from probe
In reply to Mark Brown--- drivers/video/s3c-fb.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)
drivers/video/s3c-fb.c +5 -2
@@ -1464,7 +1464,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)dev_err (dev,"failed to create window %d\n" , win);for (; win >=0 ; win--)s3c_fb_release_win (sfb, sfb->windows[win]); - goto err_irq; + goto err_pm_runtime; } }
@@ -1473,7 +1473,8 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)return 0 ; -err_irq: +err_pm_runtime: +pm_runtime_put_sync (sfb->dev);free_irq (sfb->irq_no, sfb); err_ioremap:
@@ -1483,6 +1484,8 @@ err_req_region:release_mem_region (sfb->regs_res->start,resource_size (sfb->regs_res)); err_lcd_clk: +pm_runtime_disable (sfb->dev); +if (!sfb->variant.has_clksel) {clk_disable (sfb->lcd_clk);clk_put (sfb->lcd_clk);
Signature · 1 line
1.7.7.3
PATCH 4/6 video: s3c-fb: Take a runtime PM reference when unblanked
In reply to Mark BrownWhen the framebuffer is unblanked hold a runtime PM reference. This prevents us powering down when userspace has left an image on the framebuffer and prepares the way for being able to power down the hardware when an application still has the device open.
Since we now hold a runtime PM reference whenever the display is unblanked there is no need for the runtime power management to disable and enable the display, and doing so would lead to runtime PM trying to recurse into itself when called from the blanking code, so split the runtime PM into separate functions which only deal with the clocks. The PM core will runtime resume the device prior to system suspend.
--- drivers/video/s3c-fb.c | 66 +++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 54 insertions(+), 12 deletions(-)
drivers/video/s3c-fb.c +54 -12
@@ -192,6 +192,7 @@ struct s3c_fb_vsync {* @regs: The mapped hardware registers. * @variant: Variant information for this hardware. * @enabled: A bitmask of enabled hardware windows. +* @output_on: Flag if the physical output is enabled. * @pdata: The platform configuration data passed with the device. * @windows: The hardware windows that have been claimed. * @irq_no: IRQ line number
@@ -208,6 +209,7 @@ struct s3c_fb {struct s3c_fb_variant variant;unsigned char enabled; +bool output_on;struct s3c_fb_platdata *pdata;struct s3c_fb_win *windows[S3C_FB_MAX_WIN ];
@@ -449,21 +451,28 @@ static void s3c_fb_enable(struct s3c_fb *sfb, int enable) {u32 vidcon0 =readl (sfb->regs +VIDCON0 ); -if (enable) +if (enable && !sfb->output_on) +pm_runtime_get_sync (sfb->dev); + +if (enable) { vidcon0 |=VIDCON0_ENVID |VIDCON0_ENVID_F ; -else { + }else {/* see the note in the framebuffer datasheet about * why you cannot take both of these bits down at the * same time. */ -if (!(vidcon0 &VIDCON0_ENVID )) -return ; - - vidcon0 |=VIDCON0_ENVID ; - vidcon0 &= ~VIDCON0_ENVID_F ; +if (vidcon0 &VIDCON0_ENVID ) { + vidcon0 |=VIDCON0_ENVID ; + vidcon0 &= ~VIDCON0_ENVID_F ; + } }writel (vidcon0, sfb->regs +VIDCON0 ); + +if (!enable && sfb->output_on) +pm_runtime_put_sync (sfb->dev); + + sfb->output_on = enable; }/**
@@ -1539,7 +1548,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)return 0 ; } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP static int s3c_fb_suspend (struct device *dev) {struct platform_device *pdev =to_platform_device (dev);
@@ -1609,11 +1618,40 @@ static int s3c_fb_resume(struct device *dev)return 0 ; } -#else -#define s3c_fb_suspend NULL -#define s3c_fb_resume NULL#endif +#ifdef CONFIG_PM_RUNTIME +static int s3c_fb_runtime_suspend (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); +struct s3c_fb *sfb =platform_get_drvdata (pdev); + +if (!sfb->variant.has_clksel) +clk_disable (sfb->lcd_clk); + +clk_disable (sfb->bus_clk); + +return 0 ; +} + +static int s3c_fb_runtime_resume (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); +struct s3c_fb *sfb =platform_get_drvdata (pdev); +struct s3c_fb_platdata *pd = sfb->pdata; + +clk_enable (sfb->bus_clk); + +if (!sfb->variant.has_clksel) +clk_enable (sfb->lcd_clk); + +/* setup gpio and output polarity controls */ + pd->setup_gpio (); +writel (pd->vidcon1, sfb->regs +VIDCON1 ); + +return 0 ; +} +#endif #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))#define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))
@@ -1936,7 +1974,11 @@ static struct platform_device_id s3c_fb_driver_ids[] = { };MODULE_DEVICE_TABLE (platform, s3c_fb_driver_ids); -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL); +static const struct dev_pm_ops s3cfb_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume) +SET_RUNTIME_PM_OPS (s3c_fb_runtime_suspend, s3c_fb_runtime_resume, +NULL ) +};static struct platform_driver s3c_fb_driver = { .probe = s3c_fb_probe,
Signature · 1 line
1.7.7.3
PATCH 5/6 video: s3c-fb: Hold runtime PM references when touching registers
In reply to Mark BrownTake a runtime PM reference whenever updating registers in preparation for suspending the device when the framebuffer is blanked.
--- drivers/video/s3c-fb.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)
drivers/video/s3c-fb.c +22 -0
@@ -496,6 +496,8 @@ static int s3c_fb_set_par(struct fb_info *info)dev_dbg (sfb->dev,"setting framebuffer parameters\n" ); +pm_runtime_get_sync (sfb->dev); +shadow_protect_win (win,1 );switch (var->bits_per_pixel) {
@@ -692,6 +694,8 @@ static int s3c_fb_set_par(struct fb_info *info)shadow_protect_win (win,0 ); +pm_runtime_put_sync (sfb->dev); +return 0 ; }
@@ -763,6 +767,8 @@ static int s3c_fb_setcolreg(unsigned regno,dev_dbg (sfb->dev,"%s: win %d: %d => rgb=%d/%d/%d\n" , __func__, win->index, regno, red, green, blue); +pm_runtime_get_sync (sfb->dev); +switch (info->fix.visual) {case FB_VISUAL_TRUECOLOR :/* true-colour, use pseudo-palette */
@@ -790,9 +796,11 @@ static int s3c_fb_setcolreg(unsigned regno,break ;default : +pm_runtime_put_sync (sfb->dev);return 1 ;/* unknown type */ } +pm_runtime_put_sync (sfb->dev);return 0 ; }
@@ -812,6 +820,8 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)dev_dbg (sfb->dev,"blank mode %d\n" , blank_mode); +pm_runtime_get_sync (sfb->dev); + wincon =readl (sfb->regs + sfb->variant.wincon + (index *4 ));switch (blank_mode) {
@@ -839,6 +849,7 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)case FB_BLANK_VSYNC_SUSPEND :case FB_BLANK_HSYNC_SUSPEND :default : +pm_runtime_put_sync (sfb->dev);return 1 ; }
@@ -869,6 +880,8 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)shadow_protect_win (win,0 ); } +pm_runtime_put_sync (sfb->dev); +return 0 ; }
@@ -891,6 +904,8 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,void __iomem *buf = sfb->regs + win->index *8 ;unsigned int start_boff, end_boff; +pm_runtime_get_sync (sfb->dev); +/* Offset in bytes to the start of the displayed area */ start_boff = var->yoffset * info->fix.line_length;/* X offset depends on the current bpp */
@@ -909,6 +924,7 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,break ;default :dev_err (sfb->dev,"invalid bpp\n" ); +pm_runtime_put_sync (sfb->dev);return -EINVAL ; } }
@@ -924,6 +940,7 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,shadow_protect_win (win,0 ); +pm_runtime_put_sync (sfb->dev);return 0 ; }
@@ -1013,11 +1030,16 @@ static int s3c_fb_wait_for_vsync(struct s3c_fb *sfb, u32 crtc)if (crtc !=0 )return -ENODEV ; +pm_runtime_get_sync (sfb->dev); + count = sfb->vsync_info.count;s3c_fb_enable_irq (sfb); ret =wait_event_interruptible_timeout (sfb->vsync_info.wait, count != sfb->vsync_info.count,msecs_to_jiffies (VSYNC_TIMEOUT_MSEC )); + +pm_runtime_put_sync (sfb->dev); +if (ret =0 )return -ETIMEDOUT ;
Signature · 1 line
1.7.7.3
PATCH 6/6 video: s3c-fb: Don't keep device runtime active when open
In reply to Mark BrownAllow the controller to be runtime suspended when the screen is blanked by not taking a runtime reference while the device is open. This allows greater system wide power savings when used with a standard application layer and ensures that the screen does not blank unless requested.
--- drivers/video/s3c-fb.c | 22 ---------------------- 1 files changed, 0 insertions(+), 22 deletions(-)
drivers/video/s3c-fb.c +0 -22
@@ -1070,30 +1070,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,return ret; } -static int s3c_fb_open (struct fb_info *info,int user) -{ -struct s3c_fb_win *win = info->par; -struct s3c_fb *sfb = win->parent; - -pm_runtime_get_sync (sfb->dev); - -return 0 ; -} - -static int s3c_fb_release (struct fb_info *info,int user) -{ -struct s3c_fb_win *win = info->par; -struct s3c_fb *sfb = win->parent; - -pm_runtime_put_sync (sfb->dev); - -return 0 ; -} -static struct fb_ops s3c_fb_ops = { .owner =THIS_MODULE , - .fb_open = s3c_fb_open, - .fb_release = s3c_fb_release, .fb_check_var = s3c_fb_check_var, .fb_set_par = s3c_fb_set_par, .fb_blank = s3c_fb_blank,
Signature · 1 line
1.7.7.3
RE: [PATCH 1/6] video: s3c-fb: Make runtime PM functional again
In reply to Mark BrownQuoted 18 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Tuesday, December 27, 2011 11:16 PM
To: Jingoo Han; Florian Tobias Schandinat
Cc: [email protected]; [email protected]; Mark Brown
Subject: [PATCH 1/6] video: s3c-fb: Make runtime PM functional againThe change in "video: s3c-fb: modify runtime pm functions" (commit 35784b) renders the runtime power management for the device completely ineffectual as while it leaves runtime power management notionally enabled a runtime power reference is held for the entire time the device is registered meaning it will never actually do anything.
A further issue is introduced as runtime power management is added during the system suspend path which is not something which drivers are supposed to do and would interact poorly if there were any operations done in the runtime power management callbacks.
Quoted 8 lines
While this does make things simpler (the main motivation for the original change) it will not only cause us to use more power in the framebuffer controller but will also prevent us entering lower power domain and SoC wide states as we can never power down the domain containing the device. Since neither of these things is desirable revert the change.
Signed-off-by Mark Brown <[email protected]>
Quoted 128 lines
--- drivers/video/s3c-fb.c | 51 +++++++++++++++++++++++++++++------------------ 1 files changed, 31 insertions(+), 20 deletions(-)drivers/video/s3c-fb.c +31
-21@@ -1038,8 +1038,30 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,return ret; } +static int s3c_fb_open (struct fb_info *info,int user) +{ +struct s3c_fb_win *win = info->par; +struct s3c_fb *sfb = win->parent; + +pm_runtime_get_sync (sfb->dev); + +return 0 ; +} + +static int s3c_fb_release (struct fb_info *info,int user) +{ +struct s3c_fb_win *win = info->par; +struct s3c_fb *sfb = win->parent; + +pm_runtime_put_sync (sfb->dev); + +return 0 ; +} +static struct fb_ops s3c_fb_ops = { .owner =THIS_MODULE , + .fb_open = s3c_fb_open, + .fb_release = s3c_fb_release, .fb_check_var = s3c_fb_check_var, .fb_set_par = s3c_fb_set_par, .fb_blank = s3c_fb_blank,@@ -1446,6 +1468,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev) }platform_set_drvdata (pdev, sfb); +pm_runtime_put_sync (sfb->dev);return 0 ;@@ -1485,6 +1508,8 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)struct s3c_fb *sfb =platform_get_drvdata (pdev);int win; +pm_runtime_get_sync (sfb->dev); +for (win =0 ; win <S3C_FB_MAX_WIN ; win++)if (sfb->windows[win])s3c_fb_release_win (sfb, sfb->windows[win]);@@ -1510,7 +1535,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)return 0 ; } -#ifdef CONFIG_PM_SLEEP +#ifdef CONFIG_PM static int s3c_fb_suspend (struct device *dev) {struct platform_device *pdev =to_platform_device (dev);@@ -1531,8 +1556,6 @@ static int s3c_fb_suspend(struct device *dev)clk_disable (sfb->lcd_clk);clk_disable (sfb->bus_clk); -pm_runtime_put_sync (sfb->dev); -return 0 ; }@@ -1544,7 +1567,6 @@ static int s3c_fb_resume(struct device *dev)struct s3c_fb_win *win;int win_no; -pm_runtime_get_sync (sfb->dev);clk_enable (sfb->bus_clk);if (!sfb->variant.has_clksel)@@ -1583,19 +1605,11 @@ static int s3c_fb_resume(struct device *dev) return 0; } +#else +#define s3c_fb_suspend NULL +#define s3c_fb_resume NULL#endif -#ifdef CONFIG_PM_RUNTIME -static int s3c_fb_runtime_suspend (struct device *dev) -{ -return 0 ; -} - -static int s3c_fb_runtime_resume (struct device *dev) -{ -return 0 ; -} -#endif #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))#define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))@@ -1918,10 +1932,7 @@ static struct platform_device_id s3c_fb_driver_ids[] = { };MODULE_DEVICE_TABLE (platform, s3c_fb_driver_ids); -static const struct dev_pm_ops s3c_fb_pm_ops = { - SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume) -SET_RUNTIME_PM_OPS (s3c_fb_runtime_suspend, s3c_fb_runtime_resume,NULL ) -}; +static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL);static struct platform_driver s3c_fb_driver = { .probe = s3c_fb_probe,@@ -1930,7 +1941,7 @@ static struct platform_driver s3c_fb_driver = { .driver = { .name = "s3c-fb", .owner = THIS_MODULE, - .pm = &s3c_fb_pm_ops, + .pm = &s3cfb_pm_ops, }, }; --1.7.7.3
RE: [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe
In reply to Mark BrownQuoted 8 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Tuesday, December 27, 2011 11:16 PM
To: Jingoo Han; Florian Tobias Schandinat
Cc: [email protected]; [email protected]; Mark Brown
Subject: [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probeSigned-off-by Mark Brown <[email protected]>
Quoted 38 lines
--- drivers/video/s3c-fb.c | 7 +++++-- 1 files changed, 5 insertions(+), 2 deletions(-)drivers/video/s3c-fb.c +5
-3@@ -1464,7 +1464,7 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)dev_err (dev,"failed to create window %d\n" , win);for (; win >=0 ; win--)s3c_fb_release_win (sfb, sfb->windows[win]); - goto err_irq; + goto err_pm_runtime; } }@@ -1473,7 +1473,8 @@ static int __devinit s3c_fb_probe(struct platform_device *pdev)return 0 ; -err_irq: +err_pm_runtime: +pm_runtime_put_sync (sfb->dev);free_irq (sfb->irq_no, sfb); err_ioremap:@@ -1483,6 +1484,8 @@ err_req_region:release_mem_region (sfb->regs_res->start,resource_size (sfb->regs_res)); err_lcd_clk: +pm_runtime_disable (sfb->dev); +if (!sfb->variant.has_clksel) {clk_disable (sfb->lcd_clk);clk_put (sfb->lcd_clk); --1.7.7.3
RE: [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked
In reply to Mark BrownQuoted 12 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Tuesday, December 27, 2011 11:16 PM
To: Jingoo Han; Florian Tobias Schandinat
Cc: [email protected]; [email protected]; Mark Brown
Subject: [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblankedWhen the framebuffer is unblanked hold a runtime PM reference. This prevents us powering down when userspace has left an image on the framebuffer and prepares the way for being able to power down the hardware when an application still has the device open.
Quoted 8 lines
Since we now hold a runtime PM reference whenever the display is unblanked there is no need for the runtime power management to disable and enable the display, and doing so would lead to runtime PM trying to recurse into itself when called from the blanking code, so split the runtime PM into separate functions which only deal with the clocks. The PM core will runtime resume the device prior to system suspend.
Signed-off-by Mark Brown <[email protected]>
Quoted 128 lines
--- drivers/video/s3c-fb.c | 66 +++++++++++++++++++++++++++++++++++++++-------- 1 files changed, 54 insertions(+), 12 deletions(-)drivers/video/s3c-fb.c +54
-13@@ -192,6 +192,7 @@ struct s3c_fb_vsync {* @regs: The mapped hardware registers. * @variant: Variant information for this hardware. * @enabled: A bitmask of enabled hardware windows. +* @output_on: Flag if the physical output is enabled. * @pdata: The platform configuration data passed with the device. * @windows: The hardware windows that have been claimed. * @irq_no: IRQ line number @@ -208,6 +209,7 @@ struct s3c_fb {struct s3c_fb_variant variant;unsigned char enabled; +bool output_on;struct s3c_fb_platdata *pdata;struct s3c_fb_win *windows[S3C_FB_MAX_WIN ];@@ -449,21 +451,28 @@ static void s3c_fb_enable(struct s3c_fb *sfb, int enable) {u32 vidcon0 =readl (sfb->regs +VIDCON0 ); -if (enable) +if (enable && !sfb->output_on) +pm_runtime_get_sync (sfb->dev); + +if (enable) { vidcon0 |=VIDCON0_ENVID |VIDCON0_ENVID_F ; -else { + }else {/* see the note in the framebuffer datasheet about * why you cannot take both of these bits down at the * same time. */ -if (!(vidcon0 &VIDCON0_ENVID )) -return ; - - vidcon0 |=VIDCON0_ENVID ; - vidcon0 &= ~VIDCON0_ENVID_F ; +if (vidcon0 &VIDCON0_ENVID ) { + vidcon0 |=VIDCON0_ENVID ; + vidcon0 &= ~VIDCON0_ENVID_F ; + } }writel (vidcon0, sfb->regs +VIDCON0 ); + +if (!enable && sfb->output_on) +pm_runtime_put_sync (sfb->dev); + + sfb->output_on = enable; }/** @@ -1539,7 +1548,7 @@ static int __devexit s3c_fb_remove(struct platform_device *pdev)return 0 ; } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP static int s3c_fb_suspend (struct device *dev) {struct platform_device *pdev =to_platform_device (dev);@@ -1609,11 +1618,40 @@ static int s3c_fb_resume(struct device *dev)return 0 ; } -#else -#define s3c_fb_suspend NULL -#define s3c_fb_resume NULL#endif +#ifdef CONFIG_PM_RUNTIME +static int s3c_fb_runtime_suspend (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); +struct s3c_fb *sfb =platform_get_drvdata (pdev); + +if (!sfb->variant.has_clksel) +clk_disable (sfb->lcd_clk); + +clk_disable (sfb->bus_clk); + +return 0 ; +} + +static int s3c_fb_runtime_resume (struct device *dev) +{ +struct platform_device *pdev =to_platform_device (dev); +struct s3c_fb *sfb =platform_get_drvdata (pdev); +struct s3c_fb_platdata *pd = sfb->pdata; + +clk_enable (sfb->bus_clk); + +if (!sfb->variant.has_clksel) +clk_enable (sfb->lcd_clk); + +/* setup gpio and output polarity controls */ + pd->setup_gpio (); +writel (pd->vidcon1, sfb->regs +VIDCON1 ); + +return 0 ; +} +#endif #define VALID_BPP124 (VALID_BPP(1) | VALID_BPP(2) | VALID_BPP(4))#define VALID_BPP1248 (VALID_BPP124 | VALID_BPP(8))@@ -1936,7 +1974,11 @@ static struct platform_device_id s3c_fb_driver_ids[] = { };MODULE_DEVICE_TABLE (platform, s3c_fb_driver_ids); -static UNIVERSAL_DEV_PM_OPS(s3cfb_pm_ops, s3c_fb_suspend, s3c_fb_resume, NULL); +static const struct dev_pm_ops s3cfb_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(s3c_fb_suspend, s3c_fb_resume) +SET_RUNTIME_PM_OPS (s3c_fb_runtime_suspend, s3c_fb_runtime_resume, +NULL ) +};static struct platform_driver s3c_fb_driver = { .probe = s3c_fb_probe, --1.7.7.3
RE: [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers
In reply to Mark BrownQuoted 11 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Tuesday, December 27, 2011 11:16 PM
To: Jingoo Han; Florian Tobias Schandinat
Cc: [email protected]; [email protected]; Mark Brown
Subject: [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registersTake a runtime PM reference whenever updating registers in preparation for suspending the device when the framebuffer is blanked.
Signed-off-by Mark Brown <[email protected]>
Quoted 117 lines
--- drivers/video/s3c-fb.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)drivers/video/s3c-fb.c +22
-1@@ -496,6 +496,8 @@ static int s3c_fb_set_par(struct fb_info *info)dev_dbg (sfb->dev,"setting framebuffer parameters\n" ); +pm_runtime_get_sync (sfb->dev); +shadow_protect_win (win,1 );switch (var->bits_per_pixel) {@@ -692,6 +694,8 @@ static int s3c_fb_set_par(struct fb_info *info)shadow_protect_win (win,0 ); +pm_runtime_put_sync (sfb->dev); +return 0 ; }@@ -763,6 +767,8 @@ static int s3c_fb_setcolreg(unsigned regno,dev_dbg (sfb->dev,"%s: win %d: %d => rgb=%d/%d/%d\n" , __func__, win->index, regno, red, green, blue); +pm_runtime_get_sync (sfb->dev); +switch (info->fix.visual) {case FB_VISUAL_TRUECOLOR :/* true-colour, use pseudo-palette */ @@ -790,9 +796,11 @@ static int s3c_fb_setcolreg(unsigned regno,break ;default : +pm_runtime_put_sync (sfb->dev);return 1 ;/* unknown type */ } +pm_runtime_put_sync (sfb->dev);return 0 ; }@@ -812,6 +820,8 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)dev_dbg (sfb->dev,"blank mode %d\n" , blank_mode); +pm_runtime_get_sync (sfb->dev); + wincon =readl (sfb->regs + sfb->variant.wincon + (index *4 ));switch (blank_mode) {@@ -839,6 +849,7 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)case FB_BLANK_VSYNC_SUSPEND :case FB_BLANK_HSYNC_SUSPEND :default : +pm_runtime_put_sync (sfb->dev);return 1 ; }@@ -869,6 +880,8 @@ static int s3c_fb_blank(int blank_mode, struct fb_info *info)shadow_protect_win (win,0 ); } +pm_runtime_put_sync (sfb->dev); +return 0 ; }@@ -891,6 +904,8 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,void __iomem *buf = sfb->regs + win->index *8 ;unsigned int start_boff, end_boff; +pm_runtime_get_sync (sfb->dev); +/* Offset in bytes to the start of the displayed area */ start_boff = var->yoffset * info->fix.line_length;/* X offset depends on the current bpp */ @@ -909,6 +924,7 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,break ;default :dev_err (sfb->dev,"invalid bpp\n" ); +pm_runtime_put_sync (sfb->dev);return -EINVAL ; } }@@ -924,6 +940,7 @@ static int s3c_fb_pan_display(struct fb_var_screeninfo *var,shadow_protect_win (win,0 ); +pm_runtime_put_sync (sfb->dev);return 0 ; }@@ -1013,11 +1030,16 @@ static int s3c_fb_wait_for_vsync(struct s3c_fb *sfb, u32 crtc)if (crtc !=0 )return -ENODEV ; +pm_runtime_get_sync (sfb->dev); + count = sfb->vsync_info.count;s3c_fb_enable_irq (sfb); ret =wait_event_interruptible_timeout (sfb->vsync_info.wait, count != sfb->vsync_info.count,msecs_to_jiffies (VSYNC_TIMEOUT_MSEC )); + +pm_runtime_put_sync (sfb->dev); +if (ret =0 )return -ETIMEDOUT ; --1.7.7.3
RE: [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open
In reply to Mark BrownQuoted 7 lines
-----Original Message-----
From: Mark Brown [mailto:[email protected]]
Sent: Tuesday, December 27, 2011 11:16 PM
To: Jingoo Han; Florian Tobias Schandinat
Cc: [email protected]; [email protected]; Mark Brown
Subject: [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open
Quoted 6 lines
Allow the controller to be runtime suspended when the screen is blanked by not taking a runtime reference while the device is open. This allows greater system wide power savings when used with a standard application layer and ensures that the screen does not blank unless requested.
Signed-off-by Mark Brown <[email protected]>
Quoted 41 lines
--- drivers/video/s3c-fb.c | 22 ---------------------- 1 files changed, 0 insertions(+), 22 deletions(-)drivers/video/s3c-fb.c +0
-23@@ -1070,30 +1070,8 @@ static int s3c_fb_ioctl(struct fb_info *info, unsigned int cmd,return ret; } -static int s3c_fb_open (struct fb_info *info,int user) -{ -struct s3c_fb_win *win = info->par; -struct s3c_fb *sfb = win->parent; - -pm_runtime_get_sync (sfb->dev); - -return 0 ; -} - -static int s3c_fb_release (struct fb_info *info,int user) -{ -struct s3c_fb_win *win = info->par; -struct s3c_fb *sfb = win->parent; - -pm_runtime_put_sync (sfb->dev); - -return 0 ; -} -static struct fb_ops s3c_fb_ops = { .owner =THIS_MODULE , - .fb_open = s3c_fb_open, - .fb_release = s3c_fb_release, .fb_check_var = s3c_fb_check_var, .fb_set_par = s3c_fb_set_par, .fb_blank = s3c_fb_blank, --1.7.7.3
Quoted 9 lines · Mark Brown
This patch series (the first two of which I posted the other day) improves the runtime power management in the s3c-fb driver by moving it to keeping the device runtime suspended when the screen is powered down by userspace. This saves a trivial amount of power in the framebuffer controller and allows the SoC core code to enter system wide idle states which offer much more substantial gains.
Currently it's only lightly tested but it certainly seems to the right thing for me.
Applied all 6 patches of this series.
Thanks,
Florian Tobias Schandinat
Quoted 13 lines
Mark Brown (6):
video: s3c-fb: Make runtime PM functional again
video: s3c-fb: Uses3c_fb_enable()to enable the framebuffer
video: s3c-fb: Disable runtime PM in error paths from probe
video: s3c-fb: Take a runtime PM reference when unblanked
video: s3c-fb: Hold runtime PM references when touching registers
video: s3c-fb: Don't keep device runtime active when opendrivers/video/s3c-fb.c | 127 ++++++++++++++++++++++++++++++++++------------- 1 files changed, 92 insertions(+), 35 deletions(-)