On Wed, 25 Feb 2015 12:03:36 +0200 Tomi Valkeinen <tomi.valkeinen@xxxxxx> wrote: > On 25/02/15 11:37, NeilBrown wrote: > > > > These devices do not need to return to non-graphic console > > for suspend, so disable that option. > > This means there is less work to do in the suspend/resume cycle, > > making it smoother and cheaper. > > > > > > Signed-off-by: NeilBrown <neil@xxxxxxxxxx> > > > > -- > > Hi Tomi, > > I wonder if you would consider this patch too. It works for me and > > removes pointless vt switching. I assume no-one would need or want that > > switching on suspend/resume but I guess I cannot be certain. > > > > Maybe a module-parameter could be used if there was any uncertainty. > > I was just yesterday picking patches from TI's kernel on top of > mainline, and there's a similar one for omapfb and for omapdrm. Here's > for omapfb: > > http://git.ti.com/android-sdk/kernel-video/commit/5915d8744c927307987b12b14bc6aa37b3bac05c?format=patch > > The patch in TI's kernel claims it's needed to make resume work. You're > saying it just optimizes suspend/resume. I hadn't picked this up > earlier, because I didn't understand why pm_set_vt_switch() would be > needed to make resume work. And never had time to study it, so I still > don't know what it's about... > > Looking at the drivers/tty/vt/vt_ioctl.c, it sounds to me that we should > always do pm_set_vt_switch(0), as omapdss restores everything just fine > on resume. Although it makes me wonder how it works if there are two > display controllers, one needing the switch and the other not... I wondered that too. It would seem to make more sense for pm_set_vt_switch(0) to be the default, that drivers which need the textmode switch should call (1). But I suspect there are "historical reasons" for the current situation. My experience is that suspend works just find without this setting, and I don't even notice the vt switch, unless I have DEBUG_DRIVERS which slows suspend/resume down so much (writing lots of test to a serial console) that the transition is noticeable. I made the patch because I think it is the right thing to do, rather than because it fixed a particular symptom. I suspect that if suspend doesn't work without this patch, then something very different is being done in user-space. Maybe some other display manager, not X. Maybe the X server is running on vt1 rather than vt7. > > Your patch does the call in a bit odd place, so I'll be queuing the > patches for omapfb and omapdrm. Or actually, maybe the call could be > done in one place in omapdss driver, as you do, but in omap_dsshw_probe(). I just figured that it had to be in some 'probe' function somewhere - I have no opinion about which one (maybe all?-). I'm perfectly happy with it appearing anywhere that affects omap dss devices. One thing I was reminded while testing and may as well mention: I usually blank my display before suspending (using FBIOBLANK/FB_BLANK_POWERDOWN ioctls). However if I don't then I get a warning: [ 87.261077] WARNING: CPU: 0 PID: 1901 at ../drivers/video/fbdev/omap2/dss/dispc.c:558 dispc_mgr_go+0x20/0x8c() [ 87.261138] Modules linked in: ipv6 usb_f_ecm g_ether usb_f_rndis u_ether libcomposite configfs hso w1_bq27000 libertas_sdio libertas cfg80211 omap_hdq itg3200 ehci_omap ehci_hcd uio_pdrv_genirq uio [ 87.261169] CPU: 0 PID: 1901 Comm: Xorg Not tainted 4.0.0-rc1-gta04+ #538 [ 87.261169] Hardware name: Generic OMAP36xx (Flattened Device Tree) [ 87.261199] [<c00137e0>] (unwind_backtrace) from [<c00113a4>] (show_stack+0x10/0x14) [ 87.261230] [<c00113a4>] (show_stack) from [<c0033938>] (warn_slowpath_common+0x80/0xa8) [ 87.261260] [<c0033938>] (warn_slowpath_common) from [<c0033978>] (warn_slowpath_null+0x18/0x1c) [ 87.261291] [<c0033978>] (warn_slowpath_null) from [<c0232538>] (dispc_mgr_go+0x20/0x8c) [ 87.261291] [<c0232538>] (dispc_mgr_go) from [<c023e30c>] (dss_set_go_bits+0xd4/0x100) [ 87.261322] [<c023e30c>] (dss_set_go_bits) from [<c023f308>] (omap_dss_mgr_apply+0x16c/0x1b8) [ 87.261352] [<c023f308>] (omap_dss_mgr_apply) from [<c0250790>] (omapfb_apply_changes+0x428/0x488) [ 87.261352] [<c0250790>] (omapfb_apply_changes) from [<c0251024>] (omapfb_set_par+0x74/0xb0) [ 87.261383] [<c0251024>] (omapfb_set_par) from [<c02281a4>] (fb_set_var+0x250/0x330) [ 87.261413] [<c02281a4>] (fb_set_var) from [<c021fa24>] (fbcon_blank+0x6c/0x260) [ 87.261444] [<c021fa24>] (fbcon_blank) from [<c0275464>] (do_unblank_screen+0xf8/0x19c) [ 87.261474] [<c0275464>] (do_unblank_screen) from [<c026c268>] (complete_change_console+0x50/0xcc) [ 87.261474] [<c026c268>] (complete_change_console) from [<c026ccd8>] (vt_ioctl+0x9f4/0x1238) [ 87.261505] [<c026ccd8>] (vt_ioctl) from [<c02621e8>] (tty_ioctl+0xc48/0xcac) [ 87.261535] [<c02621e8>] (tty_ioctl) from [<c00dc344>] (do_vfs_ioctl+0x5b0/0x61c) [ 87.261566] [<c00dc344>] (do_vfs_ioctl) from [<c00dc3e4>] (SyS_ioctl+0x34/0x58) [ 87.261566] [<c00dc3e4>] (SyS_ioctl) from [<c000ea40>] (ret_fast_syscall+0x0/0x34) I think the Xserver is responding to a 'suspend' notification and is blanking the screen, maybe at an awkward time. I haven't looked into further details yet. I don't really expect you to, but I thought I would mention it. Thanks, NeilBrown
Attachment:
pgpPffrDllh9s.pgp
Description: OpenPGP digital signature
