Message-ID: <20150226092646.7e20df30@notabene.brown>
Date:	Thu, 26 Feb 2015 09:26:46 +1100
From:	NeilBrown <neilb@...e.de>
To:	Tomi Valkeinen <tomi.valkeinen@...com>
Cc:	<linux-omap@...r.kernel.org>, <linux-fbdev@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>,
	GTA04 owners <gta04-owner@...delico.com>
Subject: Re: [PATCH] OMAP: DSS: DPI: disable vt-switch on suspend/resume.

On Wed, 25 Feb 2015 12:03:36 +0200 Tomi Valkeinen <tomi.valkeinen@...com>
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@...wn.name>
> > 
> > --
> > 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


Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists