* [PATCH 2/6] video: s3c-fb: Use s3c_fb_enable() to enable the framebuffer
2011-12-27 14:16 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Mark Brown
@ 2011-12-27 14:16 ` Mark Brown
2011-12-27 14:16 ` [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe Mark Brown
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2011-12-27 14:16 UTC (permalink / raw)
To: Jingoo Han, Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Mark Brown
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.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/video/s3c-fb.c | 55 ++++++++++++++++++++++++-----------------------
1 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index b1a75a0..be4c218 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -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.
--
1.7.7.3
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe
2011-12-27 14:16 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Mark Brown
2011-12-27 14:16 ` [PATCH 2/6] video: s3c-fb: Use s3c_fb_enable() to enable the framebuffer Mark Brown
@ 2011-12-27 14:16 ` Mark Brown
2011-12-28 23:54 ` Jingoo Han
2011-12-27 14:16 ` [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked Mark Brown
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-12-27 14:16 UTC (permalink / raw)
To: Jingoo Han, Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Mark Brown
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/video/s3c-fb.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index be4c218..2e0eef0 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe
2011-12-27 14:16 ` [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe Mark Brown
@ 2011-12-28 23:54 ` Jingoo Han
0 siblings, 0 replies; 13+ messages in thread
From: Jingoo Han @ 2011-12-28 23:54 UTC (permalink / raw)
To: 'Mark Brown', 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel
> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Tuesday, December 27, 2011 11:16 PM
> To: Jingoo Han; Florian Tobias Schandinat
> Cc: linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Brown
> Subject: [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/s3c-fb.c | 7 +++++--
> 1 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index be4c218..2e0eef0 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked
2011-12-27 14:16 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Mark Brown
2011-12-27 14:16 ` [PATCH 2/6] video: s3c-fb: Use s3c_fb_enable() to enable the framebuffer Mark Brown
2011-12-27 14:16 ` [PATCH 3/6] video: s3c-fb: Disable runtime PM in error paths from probe Mark Brown
@ 2011-12-27 14:16 ` Mark Brown
2011-12-28 23:54 ` Jingoo Han
2011-12-27 14:16 ` [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers Mark Brown
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-12-27 14:16 UTC (permalink / raw)
To: Jingoo Han, Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Mark Brown
When 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.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/video/s3c-fb.c | 66 +++++++++++++++++++++++++++++++++++++++--------
1 files changed, 54 insertions(+), 12 deletions(-)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 2e0eef0..688b9d8 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked
2011-12-27 14:16 ` [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked Mark Brown
@ 2011-12-28 23:54 ` Jingoo Han
0 siblings, 0 replies; 13+ messages in thread
From: Jingoo Han @ 2011-12-28 23:54 UTC (permalink / raw)
To: 'Mark Brown', 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel
> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Tuesday, December 27, 2011 11:16 PM
> To: Jingoo Han; Florian Tobias Schandinat
> Cc: linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Brown
> Subject: [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked
>
> When 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.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/s3c-fb.c | 66 +++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 2e0eef0..688b9d8 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers
2011-12-27 14:16 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Mark Brown
` (2 preceding siblings ...)
2011-12-27 14:16 ` [PATCH 4/6] video: s3c-fb: Take a runtime PM reference when unblanked Mark Brown
@ 2011-12-27 14:16 ` Mark Brown
2011-12-28 23:55 ` Jingoo Han
2011-12-27 14:16 ` [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open Mark Brown
2011-12-28 23:54 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Jingoo Han
5 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-12-27 14:16 UTC (permalink / raw)
To: Jingoo Han, Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Mark Brown
Take a runtime PM reference whenever updating registers in preparation
for suspending the device when the framebuffer is blanked.
Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
drivers/video/s3c-fb.c | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 688b9d8..84cf631 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers
2011-12-27 14:16 ` [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers Mark Brown
@ 2011-12-28 23:55 ` Jingoo Han
0 siblings, 0 replies; 13+ messages in thread
From: Jingoo Han @ 2011-12-28 23:55 UTC (permalink / raw)
To: 'Mark Brown', 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel
> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Tuesday, December 27, 2011 11:16 PM
> To: Jingoo Han; Florian Tobias Schandinat
> Cc: linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Brown
> Subject: [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers
>
> Take a runtime PM reference whenever updating registers in preparation
> for suspending the device when the framebuffer is blanked.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/s3c-fb.c | 22 ++++++++++++++++++++++
> 1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 688b9d8..84cf631 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open
2011-12-27 14:16 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Mark Brown
` (3 preceding siblings ...)
2011-12-27 14:16 ` [PATCH 5/6] video: s3c-fb: Hold runtime PM references when touching registers Mark Brown
@ 2011-12-27 14:16 ` Mark Brown
2011-12-28 23:55 ` Jingoo Han
2011-12-28 23:54 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Jingoo Han
5 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2011-12-27 14:16 UTC (permalink / raw)
To: Jingoo Han, Florian Tobias Schandinat
Cc: linux-fbdev, linux-kernel, Mark Brown
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 <broonie@opensource.wolfsonmicro.com>
---
drivers/video/s3c-fb.c | 22 ----------------------
1 files changed, 0 insertions(+), 22 deletions(-)
diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
index 84cf631..0c63b69 100644
--- a/drivers/video/s3c-fb.c
+++ b/drivers/video/s3c-fb.c
@@ -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
^ permalink raw reply related [flat|nested] 13+ messages in thread* RE: [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open
2011-12-27 14:16 ` [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open Mark Brown
@ 2011-12-28 23:55 ` Jingoo Han
0 siblings, 0 replies; 13+ messages in thread
From: Jingoo Han @ 2011-12-28 23:55 UTC (permalink / raw)
To: 'Mark Brown', 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel
> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Tuesday, December 27, 2011 11:16 PM
> To: Jingoo Han; Florian Tobias Schandinat
> Cc: linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Brown
> Subject: [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open
>
> 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 <broonie@opensource.wolfsonmicro.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/s3c-fb.c | 22 ----------------------
> 1 files changed, 0 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index 84cf631..0c63b69 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH 1/6] video: s3c-fb: Make runtime PM functional again
2011-12-27 14:16 ` [PATCH 1/6] video: s3c-fb: Make runtime PM functional again Mark Brown
` (4 preceding siblings ...)
2011-12-27 14:16 ` [PATCH 6/6] video: s3c-fb: Don't keep device runtime active when open Mark Brown
@ 2011-12-28 23:54 ` Jingoo Han
5 siblings, 0 replies; 13+ messages in thread
From: Jingoo Han @ 2011-12-28 23:54 UTC (permalink / raw)
To: 'Mark Brown', 'Florian Tobias Schandinat'
Cc: linux-fbdev, linux-kernel
> -----Original Message-----
> From: Mark Brown [mailto:broonie@opensource.wolfsonmicro.com]
> Sent: Tuesday, December 27, 2011 11:16 PM
> To: Jingoo Han; Florian Tobias Schandinat
> Cc: linux-fbdev@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Brown
> Subject: [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.
>
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
Acked-by: Jingoo Han <jg1.han@samsung.com>
> ---
> drivers/video/s3c-fb.c | 51 +++++++++++++++++++++++++++++------------------
> 1 files changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/video/s3c-fb.c b/drivers/video/s3c-fb.c
> index a0b3fd6..b1a75a0 100644
> --- a/drivers/video/s3c-fb.c
> +++ b/drivers/video/s3c-fb.c
> @@ -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
^ permalink raw reply [flat|nested] 13+ messages in thread