* [PATCH 1/2] OMAPDSS: Use PM notifiers for system suspend
2012-07-04 12:55 [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc Archit Taneja
@ 2012-07-04 12:55 ` Archit Taneja
2012-07-04 12:55 ` [PATCH 2/2] OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n Archit Taneja
2012-07-08 22:10 ` [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc Florian Tobias Schandinat
2 siblings, 0 replies; 5+ messages in thread
From: Archit Taneja @ 2012-07-04 12:55 UTC (permalink / raw)
To: FlorianSchandinat
Cc: tomi.valkeinen, jaswinder.singh, jw, linux-fbdev, linux-omap
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
The current way how omapdss handles system suspend and resume is that
omapdss device (a platform device, which is not part of the device
hierarchy of the DSS HW devices, like DISPC and DSI, or panels.) uses
the suspend and resume callbacks from platform_driver to handle system
suspend. It does this by disabling all enabled panels on suspend, and
resuming the previously disabled panels on resume.
This presents a few problems.
One is that as omapdss device is not related to the panel devices or the
DSS HW devices, there's no ordering in the suspend process. This means
that suspend could be first ran for DSS HW devices and panels, and only
then for omapdss device. Currently this is not a problem, as DSS HW
devices and panels do not handle suspend.
Another, more pressing problem, is that when suspending or resuming, the
runtime PM functions return -EACCES as runtime PM is disabled during
system suspend. This causes the driver to print warnings, and operations
to fail as they think that they failed to bring up the HW.
This patch changes the omapdss suspend handling to use PM notifiers,
which are called before suspend and after resume. This way we have a
normally functioning system when we are suspending and resuming the
panels.
This patch, I believe, creates a problem that somebody could enable or
disable a panel between PM_SUSPEND_PREPARE and the system suspend, and
similarly the other way around in resume. I choose to ignore the problem
for now, as it sounds rather unlikely, and if it happens, it's not
fatal.
In the long run the system suspend handling of omapdss and panels should
be thought out properly. The current approach feels rather hacky.
Perhaps the panel drivers should handle system suspend, or the users of
omapdss (omapfb, omapdrm) should handle system suspend.
Note that after this patch we could probably revert
0eaf9f52e94f756147dbfe1faf1f77a02378dbf9 (OMAPDSS: use sync versions of
pm_runtime_put). But as I said, this patch may be temporary, so let's
leave the sync version still in place.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Jassi Brar <jaswinder.singh@linaro.org>
Tested-by: Jassi Brar <jaswinder.singh@linaro.org>
Tested-by: Joe Woodward <jw@terrafix.co.uk>
---
drivers/video/omap2/dss/core.c | 45 ++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index 5066eee..c35a248 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -32,6 +32,7 @@
#include <linux/io.h>
#include <linux/device.h>
#include <linux/regulator/consumer.h>
+#include <linux/suspend.h>
#include <video/omapdss.h>
@@ -201,6 +202,30 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
#endif /* CONFIG_DEBUG_FS && CONFIG_OMAP2_DSS_DEBUG_SUPPORT */
/* PLATFORM DEVICE */
+static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void *d)
+{
+ DSSDBG("pm notif %lu\n", v);
+
+ switch (v)
+ {
+ case PM_SUSPEND_PREPARE:
+ DSSDBG("suspending displays\n");
+ return dss_suspend_all_devices();
+
+ case PM_POST_SUSPEND:
+ DSSDBG("resuming displays\n");
+ return dss_resume_all_devices();
+
+ default:
+ return 0;
+ }
+}
+
+static struct notifier_block omap_dss_pm_notif_block +{
+ .notifier_call = omap_dss_pm_notif,
+};
+
static int __init omap_dss_probe(struct platform_device *pdev)
{
struct omap_dss_board_info *pdata = pdev->dev.platform_data;
@@ -224,6 +249,8 @@ static int __init omap_dss_probe(struct platform_device *pdev)
else if (pdata->default_device)
core.default_display_name = pdata->default_device->name;
+ register_pm_notifier(&omap_dss_pm_notif_block);
+
return 0;
err_debugfs:
@@ -233,6 +260,8 @@ err_debugfs:
static int omap_dss_remove(struct platform_device *pdev)
{
+ unregister_pm_notifier(&omap_dss_pm_notif_block);
+
dss_uninitialize_debugfs();
dss_uninit_overlays(pdev);
@@ -247,25 +276,9 @@ static void omap_dss_shutdown(struct platform_device *pdev)
dss_disable_all_devices();
}
-static int omap_dss_suspend(struct platform_device *pdev, pm_message_t state)
-{
- DSSDBG("suspend %d\n", state.event);
-
- return dss_suspend_all_devices();
-}
-
-static int omap_dss_resume(struct platform_device *pdev)
-{
- DSSDBG("resume\n");
-
- return dss_resume_all_devices();
-}
-
static struct platform_driver omap_dss_driver = {
.remove = omap_dss_remove,
.shutdown = omap_dss_shutdown,
- .suspend = omap_dss_suspend,
- .resume = omap_dss_resume,
.driver = {
.name = "omapdss",
.owner = THIS_MODULE,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n
2012-07-04 12:55 [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc Archit Taneja
2012-07-04 12:55 ` [PATCH 1/2] OMAPDSS: Use PM notifiers for system suspend Archit Taneja
@ 2012-07-04 12:55 ` Archit Taneja
2012-07-08 22:10 ` [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc Florian Tobias Schandinat
2 siblings, 0 replies; 5+ messages in thread
From: Archit Taneja @ 2012-07-04 12:55 UTC (permalink / raw)
To: FlorianSchandinat
Cc: tomi.valkeinen, jaswinder.singh, jw, linux-fbdev, linux-omap,
Grazvydas Ignotas
From: Tomi Valkeinen <tomi.valkeinen@ti.com>
If runtime PM is not enabled in the kernel config, pm_runtime_get_sync()
will always return 1 and pm_runtime_put_sync() will always return
-ENOSYS. pm_runtime_get_sync() returning 1 presents no problem to the
driver, but -ENOSYS from pm_runtime_put_sync() causes the driver to
print a warning.
One option would be to ignore errors returned by pm_runtime_put_sync()
totally, as they only say that the call was unable to put the hardware
into suspend mode.
However, I chose to ignore the returned -ENOSYS explicitly, and print a
warning for other errors, as I think we should get notified if the HW
failed to go to suspend properly.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Jassi Brar <jaswinder.singh@linaro.org>
Cc: Grazvydas Ignotas <notasas@gmail.com>
---
drivers/video/omap2/dss/dispc.c | 2 +-
drivers/video/omap2/dss/dsi.c | 2 +-
drivers/video/omap2/dss/dss.c | 2 +-
drivers/video/omap2/dss/hdmi.c | 2 +-
drivers/video/omap2/dss/rfbi.c | 2 +-
drivers/video/omap2/dss/venc.c | 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 4749ac3..397d4ee 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -384,7 +384,7 @@ void dispc_runtime_put(void)
DSSDBG("dispc_runtime_put\n");
r = pm_runtime_put_sync(&dispc.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
static inline bool dispc_mgr_is_lcd(enum omap_channel channel)
diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c
index ca8382d..14ce8cc 100644
--- a/drivers/video/omap2/dss/dsi.c
+++ b/drivers/video/omap2/dss/dsi.c
@@ -1075,7 +1075,7 @@ void dsi_runtime_put(struct platform_device *dsidev)
DSSDBG("dsi_runtime_put\n");
r = pm_runtime_put_sync(&dsi->pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
/* source clock for DSI PLL. this could also be PCLKFREE */
diff --git a/drivers/video/omap2/dss/dss.c b/drivers/video/omap2/dss/dss.c
index 7706323..d2b5719 100644
--- a/drivers/video/omap2/dss/dss.c
+++ b/drivers/video/omap2/dss/dss.c
@@ -731,7 +731,7 @@ static void dss_runtime_put(void)
DSSDBG("dss_runtime_put\n");
r = pm_runtime_put_sync(&dss.pdev->dev);
- WARN_ON(r < 0 && r != -EBUSY);
+ WARN_ON(r < 0 && r != -ENOSYS && r != -EBUSY);
}
/* DEBUGFS */
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 8195c71..26a2430 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -138,7 +138,7 @@ static void hdmi_runtime_put(void)
DSSDBG("hdmi_runtime_put\n");
r = pm_runtime_put_sync(&hdmi.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
static int __init hdmi_init_display(struct omap_dss_device *dssdev)
diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c
index 3d8c206..7985fa1 100644
--- a/drivers/video/omap2/dss/rfbi.c
+++ b/drivers/video/omap2/dss/rfbi.c
@@ -141,7 +141,7 @@ static void rfbi_runtime_put(void)
DSSDBG("rfbi_runtime_put\n");
r = pm_runtime_put_sync(&rfbi.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
void rfbi_bus_lock(void)
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 2b89739..3907c8b 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -402,7 +402,7 @@ static void venc_runtime_put(void)
DSSDBG("venc_runtime_put\n");
r = pm_runtime_put_sync(&venc.pdev->dev);
- WARN_ON(r < 0);
+ WARN_ON(r < 0 && r != -ENOSYS);
}
static const struct venc_config *venc_timings_to_config(
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc
2012-07-04 12:55 [PATCH 0/2] OMAPDSS: PM runtime fixes for 3.5-rc Archit Taneja
2012-07-04 12:55 ` [PATCH 1/2] OMAPDSS: Use PM notifiers for system suspend Archit Taneja
2012-07-04 12:55 ` [PATCH 2/2] OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n Archit Taneja
@ 2012-07-08 22:10 ` Florian Tobias Schandinat
2012-07-09 7:59 ` Archit Taneja
2 siblings, 1 reply; 5+ messages in thread
From: Florian Tobias Schandinat @ 2012-07-08 22:10 UTC (permalink / raw)
To: Archit Taneja
Cc: tomi.valkeinen, jaswinder.singh, jw, linux-fbdev, linux-omap
Hi Archit,
On 07/04/2012 12:43 PM, Archit Taneja wrote:
> Hi Florian,
>
> These are 2 fixes which Tomi had intended to push for the coming rcs. He is
> currently on vacation and asked me to post the patches on his behalf.
>
> The first patch ensures that system suspend doesn't break on OMAP if DSS is
> enabled. The second patch prevents some warning backtraces when the kernel is
> not built with PM runtime.
>
> Could you please queue these for the upcoming rc?
as you sent me those patches you should have added your Signed-off-by as
well, I added it.
Also the first patch of this series contained 2 coding style issues
related to braces that checkpatch complained about and also differed
from the coding style used in those files, I fixed those as well.
I applied both patches.
Thanks,
Florian Tobias Schandinat
>
> Thanks,
> Archit
>
> Tomi Valkeinen (2):
> OMAPDSS: Use PM notifiers for system suspend
> OMAPDSS: fix warnings if CONFIG_PM_RUNTIME=n
>
> drivers/video/omap2/dss/core.c | 45 +++++++++++++++++++++++++--------------
> drivers/video/omap2/dss/dispc.c | 2 +-
> drivers/video/omap2/dss/dsi.c | 2 +-
> drivers/video/omap2/dss/dss.c | 2 +-
> drivers/video/omap2/dss/hdmi.c | 2 +-
> drivers/video/omap2/dss/rfbi.c | 2 +-
> drivers/video/omap2/dss/venc.c | 2 +-
> 7 files changed, 35 insertions(+), 22 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread