[PATCH] video/omap2. dispc_mgr_enable needs runtime PM

[PATCH] video/omap2. dispc_mgr_enable needs runtime PM

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]



When dispc_mgr_enable is called during shutdown the device might
be asleep, which causes problems.  So ensure it is awake.

Signed-off-by: NeilBrown <neilb@suse.de>

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index 5c81533..75a767f 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -2052,12 +2052,14 @@ bool dispc_mgr_is_enabled(enum omap_channel channel)
 
 void dispc_mgr_enable(enum omap_channel channel, bool enable)
 {
+	dispc_runtime_get();
 	if (dispc_mgr_is_lcd(channel))
 		dispc_mgr_enable_lcd_out(channel, enable);
 	else if (channel == OMAP_DSS_CHANNEL_DIGIT)
 		dispc_mgr_enable_digit_out(enable);
 	else
 		BUG();
+	dispc_runtime_put();
 }
 
 void dispc_lcd_enable_signal_polarity(bool act_high)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] video/omap2. dispc_mgr_enable needs runtime PM

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 449 bytes --]

(dropping the Tony and Kevin, as they're probably not interested in
this)

On Fri, 2011-12-30 at 12:37 +1100, NeilBrown wrote:
> 
> When dispc_mgr_enable is called during shutdown the device might
> be asleep, which causes problems.  So ensure it is awake.

How does this problem happen? dispc_mgr_enable(channel, false) shouldn't
be called if the device is asleep, and thus dispc_mgr_enable() shouldn't
use dispc_runtime_get.

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] video/omap2. dispc_mgr_enable needs runtime PM

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

On Wed, 04 Jan 2012 09:50:56 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
wrote:

> (dropping the Tony and Kevin, as they're probably not interested in
> this)

Thanks.... takes a while to figure who cares about what :-)

> 
> On Fri, 2011-12-30 at 12:37 +1100, NeilBrown wrote:
> > 
> > When dispc_mgr_enable is called during shutdown the device might
> > be asleep, which causes problems.  So ensure it is awake.
> 
> How does this problem happen? dispc_mgr_enable(channel, false) shouldn't
> be called if the device is asleep, and thus dispc_mgr_enable() shouldn't
> use dispc_runtime_get.
> 
>  Tomi
> 

The stack trace shows:

[  101.764556] [<c0205b3c>] (dispc_mgr_enable+0x40/0x2e0) from [<c020c10c>] (dss_mgr_disable+0x14/0x20)
[  101.774078] [<c020c10c>] (dss_mgr_disable+0x14/0x20) from [<c020e500>] (omapdss_dpi_display_disable+0x1c/0x88)
[  101.784484] [<c020e500>] (omapdss_dpi_display_disable+0x1c/0x88) from [<c021efa4>] (td028ttec1_panel_suspend+0x1c/0x88)
[  101.795684] [<c021efa4>] (td028ttec1_panel_suspend+0x1c/0x88) from [<c021f03c>] (td028ttec1_panel_disable+0x2c/0x50)
[  101.806640] [<c021f03c>] (td028ttec1_panel_disable+0x2c/0x50) from [<c020ab6c>] (dss_disable_device+0x20/0x2c)
[  101.817047] [<c020ab6c>] (dss_disable_device+0x20/0x2c) from [<c024ad08>] (bus_for_each_dev+0x4c/0x8c)
[  101.826751] [<c024ad08>] (bus_for_each_dev+0x4c/0x8c) from [<c024ca0c>] (platform_drv_shutdown+0x1c/0x24)
[  101.836700] [<c024ca0c>] (platform_drv_shutdown+0x1c/0x24) from [<c0248164>] (device_shutdown+0xcc/0x10c)
[  101.846679] [<c0248164>] (device_shutdown+0xcc/0x10c) from [<c004c134>] (kernel_power_off+0x10/0x4c)
[  101.856170] [<c004c134>] (kernel_power_off+0x10/0x4c) from [<c004c420>] (sys_reboot+0x124/0x1e0)
[  101.865325] [<c004c420>] (sys_reboot+0x124/0x1e0) from [<c000e9c0>] (ret_fast_syscall+0x0/0x3c)


td028ttec1_panel_* are in a non-mainline driver that could well be buggy.
.....
yep, that looks likely.  The 'disable' routines of other panels only call
omapdss_dpi_display_disable() if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE).
This panel calls it unconditionally.  I guess that is the real bug? (and
putting an appropriate test in fixes it).

Thanks for your help.

(If/when I get this td028ttec driver cleaned up, would you be the one I send
it to?)

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] video/omap2. dispc_mgr_enable needs runtime PM

[Tomi Valkeinen]

[-- Attachment #1: Type: text/plain, Size: 839 bytes --]

On Thu, 2012-01-05 at 18:32 +1100, NeilBrown wrote:
> On Wed, 04 Jan 2012 09:50:56 +0200 Tomi Valkeinen <tomi.valkeinen@ti.com>
> wrote:
> 

> yep, that looks likely.  The 'disable' routines of other panels only call
> omapdss_dpi_display_disable() if (dssdev->state == OMAP_DSS_DISPLAY_ACTIVE).
> This panel calls it unconditionally.  I guess that is the real bug? (and
> putting an appropriate test in fixes it).

Yes, that's the bug then. The enable and disable calls must match.

> Thanks for your help.
> 
> (If/when I get this td028ttec driver cleaned up, would you be the one I send
> it to?)

Yep. However, I'm currently working on device tree support for omapdss,
which will affect all panel drivers, and I may reject any new panel
patches until the dev tree stuff is in. But please send anyway =).

 Tomi


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]