Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
* Matthew Garrett <[EMAIL PROTECTED]> wrote: > We've got i915 suspend/resume now, which already fixes this for a > large number of users. Recent ATI is easy, now that we actually have > specs for ATOM. The nouveau guys are almost at the point where we can > do it for nvidia. That basically just leaves VIA. > > The other s2r issues are pretty much just driver bugs at this point. ok - sounds good :-) Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
> > Apart from those issues it looks fine to me. > > OK, please have a look at the modified patch below. Seems to work here after basic tests. ACK. (I discovered that -rc2 swsusp will not power down in some cases, but it was here before the patch, too...) Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
Apart from those issues it looks fine to me. OK, please have a look at the modified patch below. Seems to work here after basic tests. ACK. (I discovered that -rc2 swsusp will not power down in some cases, but it was here before the patch, too...) Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
* Matthew Garrett [EMAIL PROTECTED] wrote: We've got i915 suspend/resume now, which already fixes this for a large number of users. Recent ATI is easy, now that we actually have specs for ATOM. The nouveau guys are almost at the point where we can do it for nvidia. That basically just leaves VIA. The other s2r issues are pretty much just driver bugs at this point. ok - sounds good :-) Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Sat, Feb 23, 2008 at 10:07 AM, Linus Torvalds <[EMAIL PROTECTED]> wrote: > On Sat, 23 Feb 2008, Rafael J. Wysocki wrote: > > OK, please have a look at the modified patch below. > > All right, I'm fine with it. Now we just need to confirm that it works for > people.. Looks good. Applied Rafael patch on top of your latest git tree that has Jesse's i915 fix. No green screen. Tested with STD (platform), STR, and plain echo mem > /sys/power/state. Thanks, Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Sat, 23 Feb 2008, Rafael J. Wysocki wrote: > > In the revised patch below I redefined the PM_EVENT_* things as flags so > that I can "or" them and defined PM_EVENT_SLEEP in analogy with > CONFIG_PM_SLEEP. Ok, looks fine by me. > > Didn't you miss the apci_pci_choose_state() thing that also needs this > > extension? > > No, I didn't. That one operates on the ACPI D* states. Ok. I consider that just about the worst interface ever, but whatever... > OK, please have a look at the modified patch below. All right, I'm fine with it. Now we just need to confirm that it works for people.. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Saturday, 23 of February 2008, Linus Torvalds wrote:
>
> On Sat, 23 Feb 2008, Rafael J. Wysocki wrote:
>
> > --- linux-2.6.orig/drivers/char/drm/i915_drv.c
> > +++ linux-2.6/drivers/char/drm/i915_drv.c
> > @@ -222,6 +222,7 @@ static void i915_restore_vga(struct drm_
> >dev_priv->saveGR[0x18]);
> >
> > /* Attribute controller registers */
> > + inb(st01);
> > for (i = 0; i < 20; i++)
> > i915_write_ar(st01, i, dev_priv->saveAR[i], 0);
> > inb(st01); /* switch back to index mode */
>
> I'm doing this part separately, please drop it - it has nothing to do with
> the rest of the patch.
Dropped.
> I'd also suggest that you just add a helper function like
>
> int pm_event_powerdown(struct pm_message mesg)
> {
> return mesg.event >= PM_EVENT_SUSPEND;
> }
>
> or something, so that you can have code like
>
> if (pm_event_powerdown(mesg))
> pci_set_power_state(pdev, PCI_D3hot);
>
> instead of the test for EVENT_SUSPEND/HIBERNATE explicitly.
In the revised patch below I redefined the PM_EVENT_* things as flags so
that I can "or" them and defined PM_EVENT_SLEEP in analogy with
CONFIG_PM_SLEEP.
> Of course, the places that already do a switch-statement are much better
> kept that way and just add PM_EVENT_HIBERNATE to the list.
>
> > --- linux-2.6.orig/drivers/pci/pci.c
> > +++ linux-2.6/drivers/pci/pci.c
> > @@ -554,6 +554,7 @@ pci_power_t pci_choose_state(struct pci_
> > case PM_EVENT_PRETHAW:
> > /* REVISIT both freeze and pre-thaw "should" use D0 */
> > case PM_EVENT_SUSPEND:
> > + case PM_EVENT_HIBERNATE:
> > return PCI_D3hot;
>
> Didn't you miss the apci_pci_choose_state() thing that also needs this
> extension?
No, I didn't. That one operates on the ACPI D* states.
> > Index: linux-2.6/drivers/usb/host/u132-hcd.c
> > ===
> > --- linux-2.6.orig/drivers/usb/host/u132-hcd.c
> > +++ linux-2.6/drivers/usb/host/u132-hcd.c
> > @@ -3214,14 +3214,18 @@ static int u132_suspend(struct platform_
> > return -ESHUTDOWN;
> > } else {
> > int retval = 0;
> > -if (state.event == PM_EVENT_FREEZE) {
> > +
> > + switch (state.event) {
> > + case PM_EVENT_FREEZE:
> > retval = u132_bus_suspend(hcd);
> > -} else if (state.event == PM_EVENT_SUSPEND) {
> > + break;
> > + case PM_EVENT_SUSPEND:
> > + case PM_EVENT_HIBERNATE:
> > int ports = MAX_U132_PORTS;
> > while (ports-- > 0) {
> > port_power(u132, ports, 0);
> > }
> > -}
> > + break;
> > if (retval == 0)
> > pdev->dev.power.power_state = state;
> > return retval;
>
> Looks like a missing close-brace to me there - you removed the final '}'.
>
> Or am I blind?
No, you aren't. :-)
> Apart from those issues it looks fine to me.
OK, please have a look at the modified patch below.
Thanks,
Rafael
---
Documentation/power/devices.txt| 13 -
drivers/ata/ahci.c |2 +-
drivers/ata/ata_piix.c |2 +-
drivers/ata/libata-core.c |2 +-
drivers/ide/ppc/pmac.c |4 ++--
drivers/macintosh/mediabay.c |3 ++-
drivers/pci/pci.c |1 +
drivers/scsi/aic7xxx/aic79xx_osm_pci.c |2 +-
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c |2 +-
drivers/scsi/mesh.c|1 +
drivers/scsi/sd.c |3 +--
drivers/usb/host/sl811-hcd.c |1 +
drivers/usb/host/u132-hcd.c| 11 ---
drivers/video/chipsfb.c|2 +-
drivers/video/nvidia/nvidia.c |2 +-
include/linux/pm.h |9 -
kernel/power/disk.c|4 ++--
net/rfkill/rfkill.c|2 +-
18 files changed, 42 insertions(+), 24 deletions(-)
Index: linux-2.6/kernel/power/disk.c
===
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -391,7 +391,7 @@ int hibernation_platform_enter(void)
goto Close;
suspend_console();
- error = device_suspend(PMSG_SUSPEND);
+ error = device_suspend(PMSG_HIBERNATE);
if (error)
goto Resume_console;
@@ -404,7 +404,7 @@ int hibernation_platform_enter(void)
goto Finish;
local_irq_disable();
- error = device_power_down(PMSG_SUSPEND);
+ error = device_power_down(PMSG_HIBERNATE);
if (!error) {
hibernation_ops->enter();
/* We should never
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Sat, 23 Feb 2008, Rafael J. Wysocki wrote:
> --- linux-2.6.orig/drivers/char/drm/i915_drv.c
> +++ linux-2.6/drivers/char/drm/i915_drv.c
> @@ -222,6 +222,7 @@ static void i915_restore_vga(struct drm_
> dev_priv->saveGR[0x18]);
>
> /* Attribute controller registers */
> + inb(st01);
> for (i = 0; i < 20; i++)
> i915_write_ar(st01, i, dev_priv->saveAR[i], 0);
> inb(st01); /* switch back to index mode */
I'm doing this part separately, please drop it - it has nothing to do with
the rest of the patch.
I'd also suggest that you just add a helper function like
int pm_event_powerdown(struct pm_message mesg)
{
return mesg.event >= PM_EVENT_SUSPEND;
}
or something, so that you can have code like
if (pm_event_powerdown(mesg))
pci_set_power_state(pdev, PCI_D3hot);
instead of the test for EVENT_SUSPEND/HIBERNATE explicitly.
Of course, the places that already do a switch-statement are much better
kept that way and just add PM_EVENT_HIBERNATE to the list.
> --- linux-2.6.orig/drivers/pci/pci.c
> +++ linux-2.6/drivers/pci/pci.c
> @@ -554,6 +554,7 @@ pci_power_t pci_choose_state(struct pci_
> case PM_EVENT_PRETHAW:
> /* REVISIT both freeze and pre-thaw "should" use D0 */
> case PM_EVENT_SUSPEND:
> + case PM_EVENT_HIBERNATE:
> return PCI_D3hot;
Didn't you miss the apci_pci_choose_state() thing that also needs this
extension?
> Index: linux-2.6/drivers/usb/host/u132-hcd.c
> ===
> --- linux-2.6.orig/drivers/usb/host/u132-hcd.c
> +++ linux-2.6/drivers/usb/host/u132-hcd.c
> @@ -3214,14 +3214,18 @@ static int u132_suspend(struct platform_
> return -ESHUTDOWN;
> } else {
> int retval = 0;
> -if (state.event == PM_EVENT_FREEZE) {
> +
> + switch (state.event) {
> + case PM_EVENT_FREEZE:
> retval = u132_bus_suspend(hcd);
> -} else if (state.event == PM_EVENT_SUSPEND) {
> + break;
> + case PM_EVENT_SUSPEND:
> + case PM_EVENT_HIBERNATE:
> int ports = MAX_U132_PORTS;
> while (ports-- > 0) {
> port_power(u132, ports, 0);
> }
> -}
> + break;
> if (retval == 0)
> pdev->dev.power.power_state = state;
> return retval;
Looks like a missing close-brace to me there - you removed the final '}'.
Or am I blind?
Apart from those issues it looks fine to me.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Friday, 22 of February 2008, Jesse Barnes wrote:
> On Thursday, February 21, 2008 5:13 pm Jesse Barnes wrote:
> > On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
> > > On Friday, 22 of February 2008, Linus Torvalds wrote:
> > > > On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
> > > > > - if (state.event == PM_EVENT_SUSPEND) {
> > > > > + if (state.event == PM_EVENT_SUSPEND &&
> > > > > !in_hibernation_power_off())
> > > > > {
> > > >
> > > > I don't understand why hibernation just doesn't use a
> > > > PM_EVENT_HIBERNATE, and be done with it?
> > > >
> > > > Why should it be called PM_EVENT_SUSPEND when it isn't?
> > > >
> > > > Adding some external global variables is absolutely the wrong way to
> > > > fix this.
> > > >
> > > > It's not even like there are very many drivers who actually care about
> > > > "state.event" anyway: a 'git grep' returns just 35 users in the whole
> > > > tree, so if this was done this ugly way just to avoid double-chcking
> > > > the other cases that compare against PM_EVENT_SUSPEND, then it really
> > > > wasn't worth it.
> > >
> > > Please relax, we're debugging the thing right now and the patch doesn't
> > > even seem to help on the other affected box.
> >
> > Actually, looks like I forgot to reboot between tests (just rmmod'd &
> > modprobed i915), your patch actually does work.
> >
> > However, making new PM event messages might be a good thing anyway,
> > assuming Linus takes it for 2.6.25, since it should make the migration to
> > ->hibernate callbacks easier.
>
> Rafael, I'd actually prefer these changes to the i915 driver. One is to
> avoid
> the "green screen" problem and the other is to actually save state at
> hibernate time in case we don't do a POST coming out of S4 (probably not
> common but hey).
Below is a patch that introduces PM_EVENT_HIBERNATE as requested by Linus
and (hopefully) makes hibernation with i915 work correctly.
I must admit I don't feel very comfortable introduces PM_EVENT_HIBERNATE at
this point, since such changes tend to introduce unexpected issues all over the
place, but hopefully this time it won't break anything.
I have tested it on the nx6325.
Please review and tell me if it looks good.
Jesse and Jeff, please check if your boxes hibernate correctly with this patch
applied.
Thanks,
Rafael
---
Documentation/power/devices.txt| 13 -
drivers/ata/ahci.c |3 ++-
drivers/ata/ata_piix.c |3 ++-
drivers/ata/libata-core.c |2 +-
drivers/char/drm/i915_drv.c|4 +++-
drivers/ide/ppc/pmac.c |6 --
drivers/macintosh/mediabay.c |4 +++-
drivers/pci/pci.c |1 +
drivers/scsi/aic7xxx/aic79xx_osm_pci.c |2 +-
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c |2 +-
drivers/scsi/mesh.c|1 +
drivers/scsi/sd.c |5 +++--
drivers/usb/host/sl811-hcd.c |1 +
drivers/usb/host/u132-hcd.c| 10 +++---
drivers/video/chipsfb.c|3 ++-
drivers/video/nvidia/nvidia.c |3 ++-
include/linux/pm.h |5 +
kernel/power/disk.c|4 ++--
net/rfkill/rfkill.c|3 ++-
19 files changed, 51 insertions(+), 24 deletions(-)
Index: linux-2.6/kernel/power/disk.c
===
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -391,7 +391,7 @@ int hibernation_platform_enter(void)
goto Close;
suspend_console();
- error = device_suspend(PMSG_SUSPEND);
+ error = device_suspend(PMSG_HIBERNATE);
if (error)
goto Resume_console;
@@ -404,7 +404,7 @@ int hibernation_platform_enter(void)
goto Finish;
local_irq_disable();
- error = device_power_down(PMSG_SUSPEND);
+ error = device_power_down(PMSG_HIBERNATE);
if (!error) {
hibernation_ops->enter();
/* We should never get here */
Index: linux-2.6/include/linux/pm.h
===
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -143,6 +143,9 @@ typedef struct pm_message {
* the upcoming system state (such as PCI_D3hot), and enable
* wakeup events as appropriate.
*
+ * HIBERNATE Enter a low power device state appropriate for the hibernation
+ * state (eg. ACPI S4) and enable wakeup events as appropriate.
+ *
* FREEZE Quiesce operations so that a consistent image can be saved;
* but do NOT otherwise enter a low power device state, and do
* NOT emit system wakeup events.
@@ -167,10 +170,12 @@ typedef struct pm_message {
#define PM_EVENT_FREEZE 1
#define PM_EVENT_SUSPEND 2
#define PM_EVENT_PRETHAW 3
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 02:06:15PM +0100, Ingo Molnar wrote: > btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, > convenient /debug/s2r/whitelist append-API for distros (and testers) to > add more entries to the whitelist/blacklist? (for cases where the kernel > whitelist has not caught up yet) Which would eventually converge to > Utopia: s2ram that just works out of box. Because all of these video quirks are just workarounds for the fact that the kernel doesn't work properly. In general, you really don't want to call a real-mode video bios from the kernel, so punting it to userspace (and leaving the whitelisting there) is somewhat more straightforward. In addition, we can then extend the whitelist without requiring kernel upgrades. > ( Sorry about the strong words, while there's lots of good and positive > development lately i havent seen much change in this particular area > of s2ram in the past 1-2 years, and the whole chain is only as strong > as the weakest link - so someone finally has to deliver this message > to the cozy fire of s2r hackers while our testers and users are > standing out in the cold rain ... ) We've got i915 suspend/resume now, which already fixes this for a large number of users. Recent ATI is easy, now that we actually have specs for ATOM. The nouveau guys are almost at the point where we can do it for nvidia. That basically just leaves VIA. The other s2r issues are pretty much just driver bugs at this point. -- Matthew Garrett | [EMAIL PROTECTED] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, 22 Feb 2008, Ingo Molnar wrote: > > btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, > convenient /debug/s2r/whitelist append-API for distros (and testers) to > add more entries to the whitelist/blacklist? (for cases where the kernel > whitelist has not caught up yet) Which would eventually converge to > Utopia: s2ram that just works out of box. The big problem with that is - the people who know about the devices are usually not kernel people - the workarounds that the whitelist requires is quite often not a kernel workaround. In other words, the most common workarounds for the s2ram whitelist is usually to do things like running vbetool in user-level to do VGA register save/restore (VBE_POST and VGE_SAVE). Sure, the kernel could do that with usermodehelper etc, but s2ram also has those things as command line flags etc, so... Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Ingo Molnar wrote: > > * Matthew Garrett <[EMAIL PROTECTED]> wrote: > > > The s2ram command has a built-in whitelist used to set up video > > rePOSTing. If you want to test, reboot and do > > > > echo mem >/sys/power/state > > > > without i915 being loaded. If you get a console back on resume then > > the platform is reinitialising video for you, but otherwise it's your > > userspace. > > btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, > convenient /debug/s2r/whitelist append-API for distros (and testers) to > add more entries to the whitelist/blacklist? (for cases where the kernel > whitelist has not caught up yet) Which would eventually converge to > Utopia: s2ram that just works out of box. > > This would be a lot more flexible (people could even temporarily extend > the whitelist via rc.local if need to be, etc.), a lot more robust and a > lot more user friendly than the "dont use /sys/power/state, rely on some > user-space tool to work around bugs" approach. > > Really, i couldnt make the s2ram API/quirks situation much worse even if > i deliberately tried to design the whole code to be as hard to use and > as confusing as possible :-/ These types of half-kernel half-userspace > solutions usually result in constant finger-pointing and constant lag, > and they result in about the crappiest user experience that is possible > to achieve physically. > > ( Sorry about the strong words, while there's lots of good and positive > development lately i havent seen much change in this particular area > of s2ram in the past 1-2 years, and the whole chain is only as strong > as the weakest link - so someone finally has to deliver this message > to the cozy fire of s2r hackers while our testers and users are > standing out in the cold rain ... ) The problem with the whitelists is that they have to use quite a lot of data to reliably match the system. The s2ram whitelist is not 100% reliable, because it uses too little information to distinguish different versions of the same machine model, for example. Plus, in an ideal world, we should be able to match all possible working graphics/chipset/BIOS combinations and that would be quite a bit of a database. Also, there are some quirks that need to be run from the user land, AFAICS (eg. in an i86 real-mode compatible manner). IMO, whitelisting is not a solution. It's only a sort-of-working workaround and as such it shouldn't be put into the kernel. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
* Matthew Garrett <[EMAIL PROTECTED]> wrote: > The s2ram command has a built-in whitelist used to set up video > rePOSTing. If you want to test, reboot and do > > echo mem >/sys/power/state > > without i915 being loaded. If you get a console back on resume then > the platform is reinitialising video for you, but otherwise it's your > userspace. btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, convenient /debug/s2r/whitelist append-API for distros (and testers) to add more entries to the whitelist/blacklist? (for cases where the kernel whitelist has not caught up yet) Which would eventually converge to Utopia: s2ram that just works out of box. This would be a lot more flexible (people could even temporarily extend the whitelist via rc.local if need to be, etc.), a lot more robust and a lot more user friendly than the "dont use /sys/power/state, rely on some user-space tool to work around bugs" approach. Really, i couldnt make the s2ram API/quirks situation much worse even if i deliberately tried to design the whole code to be as hard to use and as confusing as possible :-/ These types of half-kernel half-userspace solutions usually result in constant finger-pointing and constant lag, and they result in about the crappiest user experience that is possible to achieve physically. ( Sorry about the strong words, while there's lots of good and positive development lately i havent seen much change in this particular area of s2ram in the past 1-2 years, and the whole chain is only as strong as the weakest link - so someone finally has to deliver this message to the cozy fire of s2r hackers while our testers and users are standing out in the cold rain ... ) Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 08:42:15AM +0800, Jeff Chua wrote: > On Fri, Feb 22, 2008 at 8:23 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > > > Your system (either your distro suspend/resume scripts or your platform) > > must > > be running the video BIOS at resume time, otherwise it would probably come > > back blank. > > But I don't think so, unless acpid is doing just that. In my > suspend/resume event script, it's just doing a simple s2ram (without > options), and exit after resume. The s2ram command has a built-in whitelist used to set up video rePOSTing. If you want to test, reboot and do echo mem >/sys/power/state without i915 being loaded. If you get a console back on resume then the platform is reinitialising video for you, but otherwise it's your userspace. -- Matthew Garrett | [EMAIL PROTECTED] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 08:42:15AM +0800, Jeff Chua wrote: On Fri, Feb 22, 2008 at 8:23 AM, Jesse Barnes [EMAIL PROTECTED] wrote: Your system (either your distro suspend/resume scripts or your platform) must be running the video BIOS at resume time, otherwise it would probably come back blank. But I don't think so, unless acpid is doing just that. In my suspend/resume event script, it's just doing a simple s2ram (without options), and exit after resume. The s2ram command has a built-in whitelist used to set up video rePOSTing. If you want to test, reboot and do echo mem /sys/power/state without i915 being loaded. If you get a console back on resume then the platform is reinitialising video for you, but otherwise it's your userspace. -- Matthew Garrett | [EMAIL PROTECTED] -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
* Matthew Garrett [EMAIL PROTECTED] wrote: The s2ram command has a built-in whitelist used to set up video rePOSTing. If you want to test, reboot and do echo mem /sys/power/state without i915 being loaded. If you get a console back on resume then the platform is reinitialising video for you, but otherwise it's your userspace. btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, convenient /debug/s2r/whitelist append-API for distros (and testers) to add more entries to the whitelist/blacklist? (for cases where the kernel whitelist has not caught up yet) Which would eventually converge to Utopia: s2ram that just works out of box. This would be a lot more flexible (people could even temporarily extend the whitelist via rc.local if need to be, etc.), a lot more robust and a lot more user friendly than the dont use /sys/power/state, rely on some user-space tool to work around bugs approach. Really, i couldnt make the s2ram API/quirks situation much worse even if i deliberately tried to design the whole code to be as hard to use and as confusing as possible :-/ These types of half-kernel half-userspace solutions usually result in constant finger-pointing and constant lag, and they result in about the crappiest user experience that is possible to achieve physically. ( Sorry about the strong words, while there's lots of good and positive development lately i havent seen much change in this particular area of s2ram in the past 1-2 years, and the whole chain is only as strong as the weakest link - so someone finally has to deliver this message to the cozy fire of s2r hackers while our testers and users are standing out in the cold rain ... ) Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Ingo Molnar wrote: * Matthew Garrett [EMAIL PROTECTED] wrote: The s2ram command has a built-in whitelist used to set up video rePOSTing. If you want to test, reboot and do echo mem /sys/power/state without i915 being loaded. If you get a console back on resume then the platform is reinitialising video for you, but otherwise it's your userspace. btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, convenient /debug/s2r/whitelist append-API for distros (and testers) to add more entries to the whitelist/blacklist? (for cases where the kernel whitelist has not caught up yet) Which would eventually converge to Utopia: s2ram that just works out of box. This would be a lot more flexible (people could even temporarily extend the whitelist via rc.local if need to be, etc.), a lot more robust and a lot more user friendly than the dont use /sys/power/state, rely on some user-space tool to work around bugs approach. Really, i couldnt make the s2ram API/quirks situation much worse even if i deliberately tried to design the whole code to be as hard to use and as confusing as possible :-/ These types of half-kernel half-userspace solutions usually result in constant finger-pointing and constant lag, and they result in about the crappiest user experience that is possible to achieve physically. ( Sorry about the strong words, while there's lots of good and positive development lately i havent seen much change in this particular area of s2ram in the past 1-2 years, and the whole chain is only as strong as the weakest link - so someone finally has to deliver this message to the cozy fire of s2r hackers while our testers and users are standing out in the cold rain ... ) The problem with the whitelists is that they have to use quite a lot of data to reliably match the system. The s2ram whitelist is not 100% reliable, because it uses too little information to distinguish different versions of the same machine model, for example. Plus, in an ideal world, we should be able to match all possible working graphics/chipset/BIOS combinations and that would be quite a bit of a database. Also, there are some quirks that need to be run from the user land, AFAICS (eg. in an i86 real-mode compatible manner). IMO, whitelisting is not a solution. It's only a sort-of-working workaround and as such it shouldn't be put into the kernel. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, 22 Feb 2008, Ingo Molnar wrote: btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, convenient /debug/s2r/whitelist append-API for distros (and testers) to add more entries to the whitelist/blacklist? (for cases where the kernel whitelist has not caught up yet) Which would eventually converge to Utopia: s2ram that just works out of box. The big problem with that is - the people who know about the devices are usually not kernel people - the workarounds that the whitelist requires is quite often not a kernel workaround. In other words, the most common workarounds for the s2ram whitelist is usually to do things like running vbetool in user-level to do VGA register save/restore (VBE_POST and VGE_SAVE). Sure, the kernel could do that with usermodehelper etc, but s2ram also has those things as command line flags etc, so... Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 02:06:15PM +0100, Ingo Molnar wrote: btw., why isnt there an in-kernel whitelist, with perhaps a dynamic, convenient /debug/s2r/whitelist append-API for distros (and testers) to add more entries to the whitelist/blacklist? (for cases where the kernel whitelist has not caught up yet) Which would eventually converge to Utopia: s2ram that just works out of box. Because all of these video quirks are just workarounds for the fact that the kernel doesn't work properly. In general, you really don't want to call a real-mode video bios from the kernel, so punting it to userspace (and leaving the whitelisting there) is somewhat more straightforward. In addition, we can then extend the whitelist without requiring kernel upgrades. ( Sorry about the strong words, while there's lots of good and positive development lately i havent seen much change in this particular area of s2ram in the past 1-2 years, and the whole chain is only as strong as the weakest link - so someone finally has to deliver this message to the cozy fire of s2r hackers while our testers and users are standing out in the cold rain ... ) We've got i915 suspend/resume now, which already fixes this for a large number of users. Recent ATI is easy, now that we actually have specs for ATOM. The nouveau guys are almost at the point where we can do it for nvidia. That basically just leaves VIA. The other s2r issues are pretty much just driver bugs at this point. -- Matthew Garrett | [EMAIL PROTECTED] -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Friday, 22 of February 2008, Jesse Barnes wrote:
On Thursday, February 21, 2008 5:13 pm Jesse Barnes wrote:
On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
On Friday, 22 of February 2008, Linus Torvalds wrote:
On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND
!in_hibernation_power_off())
{
I don't understand why hibernation just doesn't use a
PM_EVENT_HIBERNATE, and be done with it?
Why should it be called PM_EVENT_SUSPEND when it isn't?
Adding some external global variables is absolutely the wrong way to
fix this.
It's not even like there are very many drivers who actually care about
state.event anyway: a 'git grep' returns just 35 users in the whole
tree, so if this was done this ugly way just to avoid double-chcking
the other cases that compare against PM_EVENT_SUSPEND, then it really
wasn't worth it.
Please relax, we're debugging the thing right now and the patch doesn't
even seem to help on the other affected box.
Actually, looks like I forgot to reboot between tests (just rmmod'd
modprobed i915), your patch actually does work.
However, making new PM event messages might be a good thing anyway,
assuming Linus takes it for 2.6.25, since it should make the migration to
-hibernate callbacks easier.
Rafael, I'd actually prefer these changes to the i915 driver. One is to
avoid
the green screen problem and the other is to actually save state at
hibernate time in case we don't do a POST coming out of S4 (probably not
common but hey).
Below is a patch that introduces PM_EVENT_HIBERNATE as requested by Linus
and (hopefully) makes hibernation with i915 work correctly.
I must admit I don't feel very comfortable introduces PM_EVENT_HIBERNATE at
this point, since such changes tend to introduce unexpected issues all over the
place, but hopefully this time it won't break anything.
I have tested it on the nx6325.
Please review and tell me if it looks good.
Jesse and Jeff, please check if your boxes hibernate correctly with this patch
applied.
Thanks,
Rafael
---
Documentation/power/devices.txt| 13 -
drivers/ata/ahci.c |3 ++-
drivers/ata/ata_piix.c |3 ++-
drivers/ata/libata-core.c |2 +-
drivers/char/drm/i915_drv.c|4 +++-
drivers/ide/ppc/pmac.c |6 --
drivers/macintosh/mediabay.c |4 +++-
drivers/pci/pci.c |1 +
drivers/scsi/aic7xxx/aic79xx_osm_pci.c |2 +-
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c |2 +-
drivers/scsi/mesh.c|1 +
drivers/scsi/sd.c |5 +++--
drivers/usb/host/sl811-hcd.c |1 +
drivers/usb/host/u132-hcd.c| 10 +++---
drivers/video/chipsfb.c|3 ++-
drivers/video/nvidia/nvidia.c |3 ++-
include/linux/pm.h |5 +
kernel/power/disk.c|4 ++--
net/rfkill/rfkill.c|3 ++-
19 files changed, 51 insertions(+), 24 deletions(-)
Index: linux-2.6/kernel/power/disk.c
===
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -391,7 +391,7 @@ int hibernation_platform_enter(void)
goto Close;
suspend_console();
- error = device_suspend(PMSG_SUSPEND);
+ error = device_suspend(PMSG_HIBERNATE);
if (error)
goto Resume_console;
@@ -404,7 +404,7 @@ int hibernation_platform_enter(void)
goto Finish;
local_irq_disable();
- error = device_power_down(PMSG_SUSPEND);
+ error = device_power_down(PMSG_HIBERNATE);
if (!error) {
hibernation_ops-enter();
/* We should never get here */
Index: linux-2.6/include/linux/pm.h
===
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -143,6 +143,9 @@ typedef struct pm_message {
* the upcoming system state (such as PCI_D3hot), and enable
* wakeup events as appropriate.
*
+ * HIBERNATE Enter a low power device state appropriate for the hibernation
+ * state (eg. ACPI S4) and enable wakeup events as appropriate.
+ *
* FREEZE Quiesce operations so that a consistent image can be saved;
* but do NOT otherwise enter a low power device state, and do
* NOT emit system wakeup events.
@@ -167,10 +170,12 @@ typedef struct pm_message {
#define PM_EVENT_FREEZE 1
#define PM_EVENT_SUSPEND 2
#define PM_EVENT_PRETHAW 3
+#define PM_EVENT_HIBERNATE 4
#define PMSG_FREEZE((struct pm_message){ .event = PM_EVENT_FREEZE, })
#define PMSG_PRETHAW
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Sat, 23 Feb 2008, Rafael J. Wysocki wrote:
--- linux-2.6.orig/drivers/char/drm/i915_drv.c
+++ linux-2.6/drivers/char/drm/i915_drv.c
@@ -222,6 +222,7 @@ static void i915_restore_vga(struct drm_
dev_priv-saveGR[0x18]);
/* Attribute controller registers */
+ inb(st01);
for (i = 0; i 20; i++)
i915_write_ar(st01, i, dev_priv-saveAR[i], 0);
inb(st01); /* switch back to index mode */
I'm doing this part separately, please drop it - it has nothing to do with
the rest of the patch.
I'd also suggest that you just add a helper function like
int pm_event_powerdown(struct pm_message mesg)
{
return mesg.event = PM_EVENT_SUSPEND;
}
or something, so that you can have code like
if (pm_event_powerdown(mesg))
pci_set_power_state(pdev, PCI_D3hot);
instead of the test for EVENT_SUSPEND/HIBERNATE explicitly.
Of course, the places that already do a switch-statement are much better
kept that way and just add PM_EVENT_HIBERNATE to the list.
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -554,6 +554,7 @@ pci_power_t pci_choose_state(struct pci_
case PM_EVENT_PRETHAW:
/* REVISIT both freeze and pre-thaw should use D0 */
case PM_EVENT_SUSPEND:
+ case PM_EVENT_HIBERNATE:
return PCI_D3hot;
Didn't you miss the apci_pci_choose_state() thing that also needs this
extension?
Index: linux-2.6/drivers/usb/host/u132-hcd.c
===
--- linux-2.6.orig/drivers/usb/host/u132-hcd.c
+++ linux-2.6/drivers/usb/host/u132-hcd.c
@@ -3214,14 +3214,18 @@ static int u132_suspend(struct platform_
return -ESHUTDOWN;
} else {
int retval = 0;
-if (state.event == PM_EVENT_FREEZE) {
+
+ switch (state.event) {
+ case PM_EVENT_FREEZE:
retval = u132_bus_suspend(hcd);
-} else if (state.event == PM_EVENT_SUSPEND) {
+ break;
+ case PM_EVENT_SUSPEND:
+ case PM_EVENT_HIBERNATE:
int ports = MAX_U132_PORTS;
while (ports-- 0) {
port_power(u132, ports, 0);
}
-}
+ break;
if (retval == 0)
pdev-dev.power.power_state = state;
return retval;
Looks like a missing close-brace to me there - you removed the final '}'.
Or am I blind?
Apart from those issues it looks fine to me.
Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Saturday, 23 of February 2008, Linus Torvalds wrote:
On Sat, 23 Feb 2008, Rafael J. Wysocki wrote:
--- linux-2.6.orig/drivers/char/drm/i915_drv.c
+++ linux-2.6/drivers/char/drm/i915_drv.c
@@ -222,6 +222,7 @@ static void i915_restore_vga(struct drm_
dev_priv-saveGR[0x18]);
/* Attribute controller registers */
+ inb(st01);
for (i = 0; i 20; i++)
i915_write_ar(st01, i, dev_priv-saveAR[i], 0);
inb(st01); /* switch back to index mode */
I'm doing this part separately, please drop it - it has nothing to do with
the rest of the patch.
Dropped.
I'd also suggest that you just add a helper function like
int pm_event_powerdown(struct pm_message mesg)
{
return mesg.event = PM_EVENT_SUSPEND;
}
or something, so that you can have code like
if (pm_event_powerdown(mesg))
pci_set_power_state(pdev, PCI_D3hot);
instead of the test for EVENT_SUSPEND/HIBERNATE explicitly.
In the revised patch below I redefined the PM_EVENT_* things as flags so
that I can or them and defined PM_EVENT_SLEEP in analogy with
CONFIG_PM_SLEEP.
Of course, the places that already do a switch-statement are much better
kept that way and just add PM_EVENT_HIBERNATE to the list.
--- linux-2.6.orig/drivers/pci/pci.c
+++ linux-2.6/drivers/pci/pci.c
@@ -554,6 +554,7 @@ pci_power_t pci_choose_state(struct pci_
case PM_EVENT_PRETHAW:
/* REVISIT both freeze and pre-thaw should use D0 */
case PM_EVENT_SUSPEND:
+ case PM_EVENT_HIBERNATE:
return PCI_D3hot;
Didn't you miss the apci_pci_choose_state() thing that also needs this
extension?
No, I didn't. That one operates on the ACPI D* states.
Index: linux-2.6/drivers/usb/host/u132-hcd.c
===
--- linux-2.6.orig/drivers/usb/host/u132-hcd.c
+++ linux-2.6/drivers/usb/host/u132-hcd.c
@@ -3214,14 +3214,18 @@ static int u132_suspend(struct platform_
return -ESHUTDOWN;
} else {
int retval = 0;
-if (state.event == PM_EVENT_FREEZE) {
+
+ switch (state.event) {
+ case PM_EVENT_FREEZE:
retval = u132_bus_suspend(hcd);
-} else if (state.event == PM_EVENT_SUSPEND) {
+ break;
+ case PM_EVENT_SUSPEND:
+ case PM_EVENT_HIBERNATE:
int ports = MAX_U132_PORTS;
while (ports-- 0) {
port_power(u132, ports, 0);
}
-}
+ break;
if (retval == 0)
pdev-dev.power.power_state = state;
return retval;
Looks like a missing close-brace to me there - you removed the final '}'.
Or am I blind?
No, you aren't. :-)
Apart from those issues it looks fine to me.
OK, please have a look at the modified patch below.
Thanks,
Rafael
---
Documentation/power/devices.txt| 13 -
drivers/ata/ahci.c |2 +-
drivers/ata/ata_piix.c |2 +-
drivers/ata/libata-core.c |2 +-
drivers/ide/ppc/pmac.c |4 ++--
drivers/macintosh/mediabay.c |3 ++-
drivers/pci/pci.c |1 +
drivers/scsi/aic7xxx/aic79xx_osm_pci.c |2 +-
drivers/scsi/aic7xxx/aic7xxx_osm_pci.c |2 +-
drivers/scsi/mesh.c|1 +
drivers/scsi/sd.c |3 +--
drivers/usb/host/sl811-hcd.c |1 +
drivers/usb/host/u132-hcd.c| 11 ---
drivers/video/chipsfb.c|2 +-
drivers/video/nvidia/nvidia.c |2 +-
include/linux/pm.h |9 -
kernel/power/disk.c|4 ++--
net/rfkill/rfkill.c|2 +-
18 files changed, 42 insertions(+), 24 deletions(-)
Index: linux-2.6/kernel/power/disk.c
===
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -391,7 +391,7 @@ int hibernation_platform_enter(void)
goto Close;
suspend_console();
- error = device_suspend(PMSG_SUSPEND);
+ error = device_suspend(PMSG_HIBERNATE);
if (error)
goto Resume_console;
@@ -404,7 +404,7 @@ int hibernation_platform_enter(void)
goto Finish;
local_irq_disable();
- error = device_power_down(PMSG_SUSPEND);
+ error = device_power_down(PMSG_HIBERNATE);
if (!error) {
hibernation_ops-enter();
/* We should never get here */
Index: linux-2.6/include/linux/pm.h
===
---
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Sat, 23 Feb 2008, Rafael J. Wysocki wrote: In the revised patch below I redefined the PM_EVENT_* things as flags so that I can or them and defined PM_EVENT_SLEEP in analogy with CONFIG_PM_SLEEP. Ok, looks fine by me. Didn't you miss the apci_pci_choose_state() thing that also needs this extension? No, I didn't. That one operates on the ACPI D* states. Ok. I consider that just about the worst interface ever, but whatever... OK, please have a look at the modified patch below. All right, I'm fine with it. Now we just need to confirm that it works for people.. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: i915 hibernation patch (was: Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.)
On Sat, Feb 23, 2008 at 10:07 AM, Linus Torvalds [EMAIL PROTECTED] wrote: On Sat, 23 Feb 2008, Rafael J. Wysocki wrote: OK, please have a look at the modified patch below. All right, I'm fine with it. Now we just need to confirm that it works for people.. Looks good. Applied Rafael patch on top of your latest git tree that has Jesse's i915 fix. No green screen. Tested with STD (platform), STR, and plain echo mem /sys/power/state. Thanks, Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 5:28 pm Linus Torvalds wrote: > On Thu, 21 Feb 2008, Jesse Barnes wrote: > > So the advantage of the kernel suspend/resume hooks for the DRM layer is > > that the kernel video drivers can do full state save/restore (which X > > usually doesn't do, and isn't really designed to do), so that if your > > platform *doesn't* do it all, you'll still end up with a usable machine > > in the end. > > Well, I'm also hoping that eventually we could even just not do the VT > switch at all, and the kernel can treat X as "just another user process" > that it freezes. Hell yes. > At least from a mode setting standpoint. > > We'd still want to make sure that X repaints the screen if the contents > were lost, of course. And this is going to depend very intimately on the > type of graphics card and whether the video RAM is saved by STR or not - > for the Intel integrated graphics kind of situation, the video RAM will be > refreshed along with all the other memory, but for other cards we may end > up having to do the VT switch not so much for modesetting reasons as just > a way to get X to save and restore all the *other* state. Drivers supporting kernel modesetting will have to stuff their VRAM somewhere, yeah. Hopefully X won't have much to do with it though... > How close is the i915 driver from not having to even signal X? Or is that > just a pipedream of mine? It's there in the modesetting tree (though the requisite changes to avoid VT notification aren't done, it should all work fine). Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 5:13 pm Jesse Barnes wrote:
> On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
> > On Friday, 22 of February 2008, Linus Torvalds wrote:
> > > On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
> > > > - if (state.event == PM_EVENT_SUSPEND) {
> > > > + if (state.event == PM_EVENT_SUSPEND &&
> > > > !in_hibernation_power_off())
> > > > {
> > >
> > > I don't understand why hibernation just doesn't use a
> > > PM_EVENT_HIBERNATE, and be done with it?
> > >
> > > Why should it be called PM_EVENT_SUSPEND when it isn't?
> > >
> > > Adding some external global variables is absolutely the wrong way to
> > > fix this.
> > >
> > > It's not even like there are very many drivers who actually care about
> > > "state.event" anyway: a 'git grep' returns just 35 users in the whole
> > > tree, so if this was done this ugly way just to avoid double-chcking
> > > the other cases that compare against PM_EVENT_SUSPEND, then it really
> > > wasn't worth it.
> >
> > Please relax, we're debugging the thing right now and the patch doesn't
> > even seem to help on the other affected box.
>
> Actually, looks like I forgot to reboot between tests (just rmmod'd &
> modprobed i915), your patch actually does work.
>
> However, making new PM event messages might be a good thing anyway,
> assuming Linus takes it for 2.6.25, since it should make the migration to
> ->hibernate callbacks easier.
Rafael, I'd actually prefer these changes to the i915 driver. One is to avoid
the "green screen" problem and the other is to actually save state at
hibernate time in case we don't do a POST coming out of S4 (probably not
common but hey).
Jesse
Make sure hibernation works by not shutting down the video device during
hibernation power off. This is important because later stages of the
hibernation cycle end up touching the video device, which may cause a hang if
it was disabled early on. Also make sure the restoration correctly restores
the AR registers by flipping the ARX register into index mode before doing
anything.
Depends on Rafael's patch which exports hibernation state to drivers.
Signed-off-by: Jesse Barnes <[EMAIL PROTECTED]>
diff --git a/drivers/char/drm/i915_drv.c b/drivers/char/drm/i915_drv.c
index 35758a6..5e73869 100644
--- a/drivers/char/drm/i915_drv.c
+++ b/drivers/char/drm/i915_drv.c
@@ -27,6 +27,7 @@
*
*/
+#include
#include "drmP.h"
#include "drm.h"
#include "i915_drm.h"
@@ -222,6 +223,7 @@ static void i915_restore_vga(struct drm_device *dev)
dev_priv->saveGR[0x18]);
/* Attribute controller registers */
+ inb(st01);
for (i = 0; i < 20; i++)
i915_write_ar(st01, i, dev_priv->saveAR[i], 0);
inb(st01); /* switch back to index mode */
@@ -364,8 +366,8 @@ static int i915_suspend(struct drm_device *dev)
i915_save_vga(dev);
/* Shut down the device */
- pci_disable_device(dev->pdev);
- pci_set_power_state(dev->pdev, PCI_D3hot);
+ if (!in_hibernation_power_off())
+ pci_set_power_state(dev->pdev, PCI_D3hot);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thu, 21 Feb 2008, Jesse Barnes wrote: > > So the advantage of the kernel suspend/resume hooks for the DRM layer is that > the kernel video drivers can do full state save/restore (which X usually > doesn't do, and isn't really designed to do), so that if your platform > *doesn't* do it all, you'll still end up with a usable machine in the end. Well, I'm also hoping that eventually we could even just not do the VT switch at all, and the kernel can treat X as "just another user process" that it freezes. At least from a mode setting standpoint. We'd still want to make sure that X repaints the screen if the contents were lost, of course. And this is going to depend very intimately on the type of graphics card and whether the video RAM is saved by STR or not - for the Intel integrated graphics kind of situation, the video RAM will be refreshed along with all the other memory, but for other cards we may end up having to do the VT switch not so much for modesetting reasons as just a way to get X to save and restore all the *other* state. How close is the i915 driver from not having to even signal X? Or is that just a pipedream of mine? Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 9:02 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > The fact that you'd started running into problems since we merged this just > means your platform was taking care of it for you (lucky you) and that we > have some bugs in the hibernate code that we're just discovering. That's the main reason for moving to the X series. It's seems to work very well on Linux. Thanks, Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:52 pm Jeff Chua wrote: > On Fri, Feb 22, 2008 at 8:46 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > > Your s2ram script is doing your STD also? Seems counterintuitive. > > Anyway, some machines also re-POST the GPU on resume from S3; maybe yours > > is doing that. > > It's s2ram to do STR, not STD. Sorry for the confusion. But the key > point is there's no GREEN for STR. Mr Green only appear with STD. Ah, ok that makes sense. So typically, what you'd see at suspend time is this ugly call chain: 1) user requests suspend or hibernate 2) kernel kicks users off VT 3) X calls LeaveVT function of X driver 4) X driver restores whatever video state it felt like saving when it started up 5) kernel calls suspend methods 6) machine goes to sleep then on resume: 1) user requests wakeup 2) kernel calls resume methods 3) X takes back the VT, calling driver EnterVT methods 4) X driver EnterVT routine runs, doing whatever it wants 5) you're back to where you were (on a good day anyway) So, on your machine, I suspect your firmware is doing enough that X doesn't have to save/restore full video state around Enter/Leave VT (the same functions called at VT switch time when you press ctl-alt-fx), otherwise you'd be missing things like your backlight or text consoles. So the advantage of the kernel suspend/resume hooks for the DRM layer is that the kernel video drivers can do full state save/restore (which X usually doesn't do, and isn't really designed to do), so that if your platform *doesn't* do it all, you'll still end up with a usable machine in the end. The fact that you'd started running into problems since we merged this just means your platform was taking care of it for you (lucky you) and that we have some bugs in the hibernate code that we're just discovering. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
> On Friday, 22 of February 2008, Linus Torvalds wrote:
> > On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
> > > - if (state.event == PM_EVENT_SUSPEND) {
> > > + if (state.event == PM_EVENT_SUSPEND && !in_hibernation_power_off()) {
> >
> > I don't understand why hibernation just doesn't use a PM_EVENT_HIBERNATE,
> > and be done with it?
> >
> > Why should it be called PM_EVENT_SUSPEND when it isn't?
> >
> > Adding some external global variables is absolutely the wrong way to fix
> > this.
> >
> > It's not even like there are very many drivers who actually care about
> > "state.event" anyway: a 'git grep' returns just 35 users in the whole
> > tree, so if this was done this ugly way just to avoid double-chcking the
> > other cases that compare against PM_EVENT_SUSPEND, then it really wasn't
> > worth it.
>
> Please relax, we're debugging the thing right now and the patch doesn't
> even seem to help on the other affected box.
Actually, looks like I forgot to reboot between tests (just rmmod'd &
modprobed i915), your patch actually does work.
However, making new PM event messages might be a good thing anyway, assuming
Linus takes it for 2.6.25, since it should make the migration to ->hibernate
callbacks easier.
Jesse
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Jeff Chua wrote: > On Fri, Feb 22, 2008 at 8:42 AM, Jeff Chua <[EMAIL PROTECTED]> wrote: > > On Fri, Feb 22, 2008 at 8:31 AM, Rafael J. Wysocki <[EMAIL PROTECTED]> > > wrote: > > > > > Jeff, can you please test hibernation with the patch I've just sent to > > Jesse > > > (reproduced below for convenience)? > > > > Testing now. > > Great news. It works. STD (platform) does not hang at suspend. And the > annoying green is gone! STR still works. Great, thanks for testing. If Jesse confirms that it also works for him, I'll prepare a cleaner final fix tomorrow. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:42 AM, Jeff Chua <[EMAIL PROTECTED]> wrote: > On Fri, Feb 22, 2008 at 8:31 AM, Rafael J. Wysocki <[EMAIL PROTECTED]> wrote: > > > Jeff, can you please test hibernation with the patch I've just sent to > Jesse > > (reproduced below for convenience)? > > Testing now. Great news. It works. STD (platform) does not hang at suspend. And the annoying green is gone! STR still works. Thanks, Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Linus Torvalds wrote:
>
> On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
> >
> > - if (state.event == PM_EVENT_SUSPEND) {
> > + if (state.event == PM_EVENT_SUSPEND && !in_hibernation_power_off()) {
>
> I don't understand why hibernation just doesn't use a PM_EVENT_HIBERNATE,
> and be done with it?
>
> Why should it be called PM_EVENT_SUSPEND when it isn't?
>
> Adding some external global variables is absolutely the wrong way to fix
> this.
>
> It's not even like there are very many drivers who actually care about
> "state.event" anyway: a 'git grep' returns just 35 users in the whole
> tree, so if this was done this ugly way just to avoid double-chcking the
> other cases that compare against PM_EVENT_SUSPEND, then it really wasn't
> worth it.
Please relax, we're debugging the thing right now and the patch doesn't
even seem to help on the other affected box.
The issue appears to be more complicated than we initially thought.
Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:46 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > Your s2ram script is doing your STD also? Seems counterintuitive. Anyway, > some machines also re-POST the GPU on resume from S3; maybe yours is doing > that. It's s2ram to do STR, not STD. Sorry for the confusion. But the key point is there's no GREEN for STR. Mr Green only appear with STD. Thanks, Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
>
> - if (state.event == PM_EVENT_SUSPEND) {
> + if (state.event == PM_EVENT_SUSPEND && !in_hibernation_power_off()) {
I don't understand why hibernation just doesn't use a PM_EVENT_HIBERNATE,
and be done with it?
Why should it be called PM_EVENT_SUSPEND when it isn't?
Adding some external global variables is absolutely the wrong way to fix
this.
It's not even like there are very many drivers who actually care about
"state.event" anyway: a 'git grep' returns just 35 users in the whole
tree, so if this was done this ugly way just to avoid double-chcking the
other cases that compare against PM_EVENT_SUSPEND, then it really wasn't
worth it.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:42 pm Jeff Chua wrote: > On Fri, Feb 22, 2008 at 8:23 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > > Your system (either your distro suspend/resume scripts or your platform) > > must be running the video BIOS at resume time, otherwise it would > > probably come back blank. > > But I don't think so, unless acpid is doing just that. In my > suspend/resume event script, it's just doing a simple s2ram (without > options), and exit after resume. Your s2ram script is doing your STD also? Seems counterintuitive. Anyway, some machines also re-POST the GPU on resume from S3; maybe yours is doing that. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:31 AM, Rafael J. Wysocki <[EMAIL PROTECTED]> wrote: > Jeff, can you please test hibernation with the patch I've just sent to Jesse > (reproduced below for convenience)? Testing now. Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:23 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > Your system (either your distro suspend/resume scripts or your platform) must > be running the video BIOS at resume time, otherwise it would probably come > back blank. But I don't think so, unless acpid is doing just that. In my suspend/resume event script, it's just doing a simple s2ram (without options), and exit after resume. Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:20 pm Jeff Chua wrote: > On Fri, Feb 22, 2008 at 5:02 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > > On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote: > > > > > Let's try to narrow it down to what the interaction is. Are you > > > > > using something like acpi_sleep=s3_bios or similar? > > > > > > > > No. Not additional command line option except for resume=/dev/sda3 > > > > reboot=bios > > > > > > My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need > > > s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with > > > echo mem > /sys/power/state. > > > > Cool, glad to hear at least one success report. :) > > STR is always working on my X60s. No green screen, no hang. Both s2ram > and "echo mem > /sys/power/state". It's STD that's having problem. > > But strange thing is I could even restore console after STD even > without agp and i915 module loaded, so I don't know how the console > vga got saved and restored. Your system (either your distro suspend/resume scripts or your platform) must be running the video BIOS at resume time, otherwise it would probably come back blank. Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Jeff Chua wrote:
> On Fri, Feb 22, 2008 at 5:02 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote:
> > On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote:
> > > > > Let's try to narrow it down to what the interaction is. Are you using
> > > > > something like acpi_sleep=s3_bios or similar?
> > > >
> > > > No. Not additional command line option except for resume=/dev/sda3
> > > > reboot=bios
> > >
> > > My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need
> > > s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with
> > > echo mem > /sys/power/state.
> >
> > Cool, glad to hear at least one success report. :)
>
> STR is always working on my X60s. No green screen, no hang. Both s2ram
> and "echo mem > /sys/power/state". It's STD that's having problem.
>
> But strange thing is I could even restore console after STD even
> without agp and i915 module loaded, so I don't know how the console
> vga got saved and restored.
Jeff, can you please test hibernation with the patch I've just sent to Jesse
(reproduced below for convenience)?
Thanks,
Rafael
---
drivers/char/drm/i915_drv.c |5 +++--
include/linux/suspend.h |2 ++
kernel/power/disk.c | 10 +-
3 files changed, 14 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/char/drm/i915_drv.c
===
--- linux-2.6.orig/drivers/char/drm/i915_drv.c
+++ linux-2.6/drivers/char/drm/i915_drv.c
@@ -27,6 +27,7 @@
*
*/
+#include
#include "drmP.h"
#include "drm.h"
#include "i915_drm.h"
@@ -222,6 +223,7 @@ static void i915_restore_vga(struct drm_
dev_priv->saveGR[0x18]);
/* Attribute controller registers */
+ inb(st01); /* switch back to index mode */
for (i = 0; i < 20; i++)
i915_write_ar(st01, i, dev_priv->saveAR[i], 0);
inb(st01); /* switch back to index mode */
@@ -366,9 +368,8 @@ static int i915_suspend(struct drm_devic
i915_save_vga(dev);
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND && !in_hibernation_power_off()) {
/* Shut down the device */
- pci_disable_device(dev->pdev);
pci_set_power_state(dev->pdev, PCI_D3hot);
}
Index: linux-2.6/include/linux/suspend.h
===
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -209,6 +209,7 @@ extern unsigned long get_safe_page(gfp_t
extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
extern int hibernate(void);
+extern bool in_hibernation_power_off(void);
#else /* CONFIG_HIBERNATION */
static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
static inline void swsusp_set_page_free(struct page *p) {}
@@ -216,6 +217,7 @@ static inline void swsusp_unset_page_fre
static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
static inline int hibernate(void) { return -ENOSYS; }
+static inline bool in_hibernation_power_off(void) { return false; }
#endif /* CONFIG_HIBERNATION */
#ifdef CONFIG_PM_SLEEP
Index: linux-2.6/kernel/power/disk.c
===
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -24,7 +24,7 @@
#include "power.h"
-
+static bool entering_sleep_state;
static int noresume = 0;
static char resume_file[256] = CONFIG_PM_STD_PARTITION;
dev_t swsusp_resume_device;
@@ -381,6 +381,7 @@ int hibernation_platform_enter(void)
if (!hibernation_ops)
return -ENOSYS;
+ entering_sleep_state = true;
/*
* We have cancelled the power transition by running
* hibernation_ops->finish() before saving the image, so we should let
@@ -412,6 +413,7 @@ int hibernation_platform_enter(void)
}
local_irq_enable();
+ entering_sleep_state = false;
/*
* We don't need to reenable the nonboot CPUs or resume consoles, since
* the system is going to be halted anyway.
@@ -427,6 +429,12 @@ int hibernation_platform_enter(void)
return error;
}
+bool in_hibernation_power_off(void)
+{
+ return entering_sleep_state;
+}
+EXPORT_SYMBOL(in_hibernation_power_off);
+
/**
* power_down - Shut the machine down for hibernation.
*
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 5:02 AM, Jesse Barnes <[EMAIL PROTECTED]> wrote: > On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote: > > > > Let's try to narrow it down to what the interaction is. Are you using > > > > something like acpi_sleep=s3_bios or similar? > > > > > > No. Not additional command line option except for resume=/dev/sda3 > > > reboot=bios > > > > My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need > > s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with > > echo mem > /sys/power/state. > > Cool, glad to hear at least one success report. :) STR is always working on my X60s. No green screen, no hang. Both s2ram and "echo mem > /sys/power/state". It's STD that's having problem. But strange thing is I could even restore console after STD even without agp and i915 module loaded, so I don't know how the console vga got saved and restored. Thanks, Jeff. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote: > > > Let's try to narrow it down to what the interaction is. Are you using > > > something like acpi_sleep=s3_bios or similar? > > > > No. Not additional command line option except for resume=/dev/sda3 > > reboot=bios > > My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need > s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with > echo mem > /sys/power/state. Cool, glad to hear at least one success report. :) > This communication contains confidential information. It is for the > exclusive use of the intended addressee. If you are not the intended > addressee, please note that any form of distribution, copying or use of > this communication or the information in it is strictly prohibited by law. > If you have received this communication in error, please immediately notify > the sender by reply e-mail and destroy this message. Thank you for your > cooperation. Really? This contains confidential information? I'd better notify you and destroy this message now... :) Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thu, 2008-02-21 at 02:02 +0800, Jeff Chua wrote: > On Feb 21, 2008 1:52 AM, Linus Torvalds <[EMAIL PROTECTED]> wrote: > > > Ahh. You're using the BIOS to re-initialize your video, aren't you? > > I don't know. Just pure simple "s2ram" without any options. Well, as far as I know, s2ram could be doing vbe save/restore for you. Even without parameter, if your laptop is in the whitelist. > > Let's try to narrow it down to what the interaction is. Are you using > > something like acpi_sleep=s3_bios or similar? > > No. Not additional command line option except for resume=/dev/sda3 reboot=bios My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with echo mem > /sys/power/state. Romano I imagine this will be received as blasphemy, but if only ndiswrapper were not horribly broken, this will be my day-by-day kernel. I just hope ath5k will arrive to my chipset soon... -- La presente comunicación tiene carácter confidencial y es para el exclusivo uso del destinatario indicado en la misma. Si Ud. no es el destinatario indicado, le informamos que cualquier forma de distribución, reproducción o uso de esta comunicación y/o de la información contenida en la misma están estrictamente prohibidos por la ley. Si Ud. ha recibido esta comunicación por error, por favor, notifíquelo inmediatamente al remitente contestando a este mensaje y proceda a continuación a destruirlo. Gracias por su colaboración. This communication contains confidential information. It is for the exclusive use of the intended addressee. If you are not the intended addressee, please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited by law. If you have received this communication in error, please immediately notify the sender by reply e-mail and destroy this message. Thank you for your cooperation. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thu, 2008-02-21 at 02:02 +0800, Jeff Chua wrote: On Feb 21, 2008 1:52 AM, Linus Torvalds [EMAIL PROTECTED] wrote: Ahh. You're using the BIOS to re-initialize your video, aren't you? I don't know. Just pure simple s2ram without any options. Well, as far as I know, s2ram could be doing vbe save/restore for you. Even without parameter, if your laptop is in the whitelist. Let's try to narrow it down to what the interaction is. Are you using something like acpi_sleep=s3_bios or similar? No. Not additional command line option except for resume=/dev/sda3 reboot=bios My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with echo mem /sys/power/state. Romano I imagine this will be received as blasphemy, but if only ndiswrapper were not horribly broken, this will be my day-by-day kernel. I just hope ath5k will arrive to my chipset soon... -- La presente comunicación tiene carácter confidencial y es para el exclusivo uso del destinatario indicado en la misma. Si Ud. no es el destinatario indicado, le informamos que cualquier forma de distribución, reproducción o uso de esta comunicación y/o de la información contenida en la misma están estrictamente prohibidos por la ley. Si Ud. ha recibido esta comunicación por error, por favor, notifíquelo inmediatamente al remitente contestando a este mensaje y proceda a continuación a destruirlo. Gracias por su colaboración. This communication contains confidential information. It is for the exclusive use of the intended addressee. If you are not the intended addressee, please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited by law. If you have received this communication in error, please immediately notify the sender by reply e-mail and destroy this message. Thank you for your cooperation. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote: Let's try to narrow it down to what the interaction is. Are you using something like acpi_sleep=s3_bios or similar? No. Not additional command line option except for resume=/dev/sda3 reboot=bios My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with echo mem /sys/power/state. Cool, glad to hear at least one success report. :) This communication contains confidential information. It is for the exclusive use of the intended addressee. If you are not the intended addressee, please note that any form of distribution, copying or use of this communication or the information in it is strictly prohibited by law. If you have received this communication in error, please immediately notify the sender by reply e-mail and destroy this message. Thank you for your cooperation. Really? This contains confidential information? I'd better notify you and destroy this message now... :) Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 5:02 AM, Jesse Barnes [EMAIL PROTECTED] wrote: On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote: Let's try to narrow it down to what the interaction is. Are you using something like acpi_sleep=s3_bios or similar? No. Not additional command line option except for resume=/dev/sda3 reboot=bios My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with echo mem /sys/power/state. Cool, glad to hear at least one success report. :) STR is always working on my X60s. No green screen, no hang. Both s2ram and echo mem /sys/power/state. It's STD that's having problem. But strange thing is I could even restore console after STD even without agp and i915 module loaded, so I don't know how the console vga got saved and restored. Thanks, Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Jeff Chua wrote:
On Fri, Feb 22, 2008 at 5:02 AM, Jesse Barnes [EMAIL PROTECTED] wrote:
On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote:
Let's try to narrow it down to what the interaction is. Are you using
something like acpi_sleep=s3_bios or similar?
No. Not additional command line option except for resume=/dev/sda3
reboot=bios
My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need
s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with
echo mem /sys/power/state.
Cool, glad to hear at least one success report. :)
STR is always working on my X60s. No green screen, no hang. Both s2ram
and echo mem /sys/power/state. It's STD that's having problem.
But strange thing is I could even restore console after STD even
without agp and i915 module loaded, so I don't know how the console
vga got saved and restored.
Jeff, can you please test hibernation with the patch I've just sent to Jesse
(reproduced below for convenience)?
Thanks,
Rafael
---
drivers/char/drm/i915_drv.c |5 +++--
include/linux/suspend.h |2 ++
kernel/power/disk.c | 10 +-
3 files changed, 14 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/char/drm/i915_drv.c
===
--- linux-2.6.orig/drivers/char/drm/i915_drv.c
+++ linux-2.6/drivers/char/drm/i915_drv.c
@@ -27,6 +27,7 @@
*
*/
+#include linux/suspend.h
#include drmP.h
#include drm.h
#include i915_drm.h
@@ -222,6 +223,7 @@ static void i915_restore_vga(struct drm_
dev_priv-saveGR[0x18]);
/* Attribute controller registers */
+ inb(st01); /* switch back to index mode */
for (i = 0; i 20; i++)
i915_write_ar(st01, i, dev_priv-saveAR[i], 0);
inb(st01); /* switch back to index mode */
@@ -366,9 +368,8 @@ static int i915_suspend(struct drm_devic
i915_save_vga(dev);
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND !in_hibernation_power_off()) {
/* Shut down the device */
- pci_disable_device(dev-pdev);
pci_set_power_state(dev-pdev, PCI_D3hot);
}
Index: linux-2.6/include/linux/suspend.h
===
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -209,6 +209,7 @@ extern unsigned long get_safe_page(gfp_t
extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
extern int hibernate(void);
+extern bool in_hibernation_power_off(void);
#else /* CONFIG_HIBERNATION */
static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
static inline void swsusp_set_page_free(struct page *p) {}
@@ -216,6 +217,7 @@ static inline void swsusp_unset_page_fre
static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
static inline int hibernate(void) { return -ENOSYS; }
+static inline bool in_hibernation_power_off(void) { return false; }
#endif /* CONFIG_HIBERNATION */
#ifdef CONFIG_PM_SLEEP
Index: linux-2.6/kernel/power/disk.c
===
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -24,7 +24,7 @@
#include power.h
-
+static bool entering_sleep_state;
static int noresume = 0;
static char resume_file[256] = CONFIG_PM_STD_PARTITION;
dev_t swsusp_resume_device;
@@ -381,6 +381,7 @@ int hibernation_platform_enter(void)
if (!hibernation_ops)
return -ENOSYS;
+ entering_sleep_state = true;
/*
* We have cancelled the power transition by running
* hibernation_ops-finish() before saving the image, so we should let
@@ -412,6 +413,7 @@ int hibernation_platform_enter(void)
}
local_irq_enable();
+ entering_sleep_state = false;
/*
* We don't need to reenable the nonboot CPUs or resume consoles, since
* the system is going to be halted anyway.
@@ -427,6 +429,12 @@ int hibernation_platform_enter(void)
return error;
}
+bool in_hibernation_power_off(void)
+{
+ return entering_sleep_state;
+}
+EXPORT_SYMBOL(in_hibernation_power_off);
+
/**
* power_down - Shut the machine down for hibernation.
*
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:20 pm Jeff Chua wrote: On Fri, Feb 22, 2008 at 5:02 AM, Jesse Barnes [EMAIL PROTECTED] wrote: On Thursday, February 21, 2008 11:43 am Romano Giannetti wrote: Let's try to narrow it down to what the interaction is. Are you using something like acpi_sleep=s3_bios or similar? No. Not additional command line option except for resume=/dev/sda3 reboot=bios My laptop (a Toshiba satellite U305, intel 945GM chipset, used to need s2ram -f -p -m to STR correctly. In 2.6.25-rc2 I can simply STR with echo mem /sys/power/state. Cool, glad to hear at least one success report. :) STR is always working on my X60s. No green screen, no hang. Both s2ram and echo mem /sys/power/state. It's STD that's having problem. But strange thing is I could even restore console after STD even without agp and i915 module loaded, so I don't know how the console vga got saved and restored. Your system (either your distro suspend/resume scripts or your platform) must be running the video BIOS at resume time, otherwise it would probably come back blank. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:23 AM, Jesse Barnes [EMAIL PROTECTED] wrote: Your system (either your distro suspend/resume scripts or your platform) must be running the video BIOS at resume time, otherwise it would probably come back blank. But I don't think so, unless acpid is doing just that. In my suspend/resume event script, it's just doing a simple s2ram (without options), and exit after resume. Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:31 AM, Rafael J. Wysocki [EMAIL PROTECTED] wrote: Jeff, can you please test hibernation with the patch I've just sent to Jesse (reproduced below for convenience)? Testing now. Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND !in_hibernation_power_off()) {
I don't understand why hibernation just doesn't use a PM_EVENT_HIBERNATE,
and be done with it?
Why should it be called PM_EVENT_SUSPEND when it isn't?
Adding some external global variables is absolutely the wrong way to fix
this.
It's not even like there are very many drivers who actually care about
state.event anyway: a 'git grep' returns just 35 users in the whole
tree, so if this was done this ugly way just to avoid double-chcking the
other cases that compare against PM_EVENT_SUSPEND, then it really wasn't
worth it.
Linus
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:42 pm Jeff Chua wrote: On Fri, Feb 22, 2008 at 8:23 AM, Jesse Barnes [EMAIL PROTECTED] wrote: Your system (either your distro suspend/resume scripts or your platform) must be running the video BIOS at resume time, otherwise it would probably come back blank. But I don't think so, unless acpid is doing just that. In my suspend/resume event script, it's just doing a simple s2ram (without options), and exit after resume. Your s2ram script is doing your STD also? Seems counterintuitive. Anyway, some machines also re-POST the GPU on resume from S3; maybe yours is doing that. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:42 AM, Jeff Chua [EMAIL PROTECTED] wrote: On Fri, Feb 22, 2008 at 8:31 AM, Rafael J. Wysocki [EMAIL PROTECTED] wrote: Jeff, can you please test hibernation with the patch I've just sent to Jesse (reproduced below for convenience)? Testing now. Great news. It works. STD (platform) does not hang at suspend. And the annoying green is gone! STR still works. Thanks, Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Jeff Chua wrote: On Fri, Feb 22, 2008 at 8:42 AM, Jeff Chua [EMAIL PROTECTED] wrote: On Fri, Feb 22, 2008 at 8:31 AM, Rafael J. Wysocki [EMAIL PROTECTED] wrote: Jeff, can you please test hibernation with the patch I've just sent to Jesse (reproduced below for convenience)? Testing now. Great news. It works. STD (platform) does not hang at suspend. And the annoying green is gone! STR still works. Great, thanks for testing. If Jesse confirms that it also works for him, I'll prepare a cleaner final fix tomorrow. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
On Friday, 22 of February 2008, Linus Torvalds wrote:
On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND !in_hibernation_power_off()) {
I don't understand why hibernation just doesn't use a PM_EVENT_HIBERNATE,
and be done with it?
Why should it be called PM_EVENT_SUSPEND when it isn't?
Adding some external global variables is absolutely the wrong way to fix
this.
It's not even like there are very many drivers who actually care about
state.event anyway: a 'git grep' returns just 35 users in the whole
tree, so if this was done this ugly way just to avoid double-chcking the
other cases that compare against PM_EVENT_SUSPEND, then it really wasn't
worth it.
Please relax, we're debugging the thing right now and the patch doesn't
even seem to help on the other affected box.
Actually, looks like I forgot to reboot between tests (just rmmod'd
modprobed i915), your patch actually does work.
However, making new PM event messages might be a good thing anyway, assuming
Linus takes it for 2.6.25, since it should make the migration to -hibernate
callbacks easier.
Jesse
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 4:52 pm Jeff Chua wrote: On Fri, Feb 22, 2008 at 8:46 AM, Jesse Barnes [EMAIL PROTECTED] wrote: Your s2ram script is doing your STD also? Seems counterintuitive. Anyway, some machines also re-POST the GPU on resume from S3; maybe yours is doing that. It's s2ram to do STR, not STD. Sorry for the confusion. But the key point is there's no GREEN for STR. Mr Green only appear with STD. Ah, ok that makes sense. So typically, what you'd see at suspend time is this ugly call chain: 1) user requests suspend or hibernate 2) kernel kicks users off VT 3) X calls LeaveVT function of X driver 4) X driver restores whatever video state it felt like saving when it started up 5) kernel calls suspend methods 6) machine goes to sleep then on resume: 1) user requests wakeup 2) kernel calls resume methods 3) X takes back the VT, calling driver EnterVT methods 4) X driver EnterVT routine runs, doing whatever it wants 5) you're back to where you were (on a good day anyway) So, on your machine, I suspect your firmware is doing enough that X doesn't have to save/restore full video state around Enter/Leave VT (the same functions called at VT switch time when you press ctl-alt-fx), otherwise you'd be missing things like your backlight or text consoles. So the advantage of the kernel suspend/resume hooks for the DRM layer is that the kernel video drivers can do full state save/restore (which X usually doesn't do, and isn't really designed to do), so that if your platform *doesn't* do it all, you'll still end up with a usable machine in the end. The fact that you'd started running into problems since we merged this just means your platform was taking care of it for you (lucky you) and that we have some bugs in the hibernate code that we're just discovering. Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 8:46 AM, Jesse Barnes [EMAIL PROTECTED] wrote: Your s2ram script is doing your STD also? Seems counterintuitive. Anyway, some machines also re-POST the GPU on resume from S3; maybe yours is doing that. It's s2ram to do STR, not STD. Sorry for the confusion. But the key point is there's no GREEN for STR. Mr Green only appear with STD. Thanks, Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Friday, 22 of February 2008, Linus Torvalds wrote:
On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND !in_hibernation_power_off()) {
I don't understand why hibernation just doesn't use a PM_EVENT_HIBERNATE,
and be done with it?
Why should it be called PM_EVENT_SUSPEND when it isn't?
Adding some external global variables is absolutely the wrong way to fix
this.
It's not even like there are very many drivers who actually care about
state.event anyway: a 'git grep' returns just 35 users in the whole
tree, so if this was done this ugly way just to avoid double-chcking the
other cases that compare against PM_EVENT_SUSPEND, then it really wasn't
worth it.
Please relax, we're debugging the thing right now and the patch doesn't
even seem to help on the other affected box.
The issue appears to be more complicated than we initially thought.
Thanks,
Rafael
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Fri, Feb 22, 2008 at 9:02 AM, Jesse Barnes [EMAIL PROTECTED] wrote: The fact that you'd started running into problems since we merged this just means your platform was taking care of it for you (lucky you) and that we have some bugs in the hibernate code that we're just discovering. That's the main reason for moving to the X series. It's seems to work very well on Linux. Thanks, Jeff. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 5:13 pm Jesse Barnes wrote:
On Thursday, February 21, 2008 4:54 pm Rafael J. Wysocki wrote:
On Friday, 22 of February 2008, Linus Torvalds wrote:
On Fri, 22 Feb 2008, Rafael J. Wysocki wrote:
- if (state.event == PM_EVENT_SUSPEND) {
+ if (state.event == PM_EVENT_SUSPEND
!in_hibernation_power_off())
{
I don't understand why hibernation just doesn't use a
PM_EVENT_HIBERNATE, and be done with it?
Why should it be called PM_EVENT_SUSPEND when it isn't?
Adding some external global variables is absolutely the wrong way to
fix this.
It's not even like there are very many drivers who actually care about
state.event anyway: a 'git grep' returns just 35 users in the whole
tree, so if this was done this ugly way just to avoid double-chcking
the other cases that compare against PM_EVENT_SUSPEND, then it really
wasn't worth it.
Please relax, we're debugging the thing right now and the patch doesn't
even seem to help on the other affected box.
Actually, looks like I forgot to reboot between tests (just rmmod'd
modprobed i915), your patch actually does work.
However, making new PM event messages might be a good thing anyway,
assuming Linus takes it for 2.6.25, since it should make the migration to
-hibernate callbacks easier.
Rafael, I'd actually prefer these changes to the i915 driver. One is to avoid
the green screen problem and the other is to actually save state at
hibernate time in case we don't do a POST coming out of S4 (probably not
common but hey).
Jesse
Make sure hibernation works by not shutting down the video device during
hibernation power off. This is important because later stages of the
hibernation cycle end up touching the video device, which may cause a hang if
it was disabled early on. Also make sure the restoration correctly restores
the AR registers by flipping the ARX register into index mode before doing
anything.
Depends on Rafael's patch which exports hibernation state to drivers.
Signed-off-by: Jesse Barnes [EMAIL PROTECTED]
diff --git a/drivers/char/drm/i915_drv.c b/drivers/char/drm/i915_drv.c
index 35758a6..5e73869 100644
--- a/drivers/char/drm/i915_drv.c
+++ b/drivers/char/drm/i915_drv.c
@@ -27,6 +27,7 @@
*
*/
+#include linux/suspend.h
#include drmP.h
#include drm.h
#include i915_drm.h
@@ -222,6 +223,7 @@ static void i915_restore_vga(struct drm_device *dev)
dev_priv-saveGR[0x18]);
/* Attribute controller registers */
+ inb(st01);
for (i = 0; i 20; i++)
i915_write_ar(st01, i, dev_priv-saveAR[i], 0);
inb(st01); /* switch back to index mode */
@@ -364,8 +366,8 @@ static int i915_suspend(struct drm_device *dev)
i915_save_vga(dev);
/* Shut down the device */
- pci_disable_device(dev-pdev);
- pci_set_power_state(dev-pdev, PCI_D3hot);
+ if (!in_hibernation_power_off())
+ pci_set_power_state(dev-pdev, PCI_D3hot);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thu, 21 Feb 2008, Jesse Barnes wrote: So the advantage of the kernel suspend/resume hooks for the DRM layer is that the kernel video drivers can do full state save/restore (which X usually doesn't do, and isn't really designed to do), so that if your platform *doesn't* do it all, you'll still end up with a usable machine in the end. Well, I'm also hoping that eventually we could even just not do the VT switch at all, and the kernel can treat X as just another user process that it freezes. At least from a mode setting standpoint. We'd still want to make sure that X repaints the screen if the contents were lost, of course. And this is going to depend very intimately on the type of graphics card and whether the video RAM is saved by STR or not - for the Intel integrated graphics kind of situation, the video RAM will be refreshed along with all the other memory, but for other cards we may end up having to do the VT switch not so much for modesetting reasons as just a way to get X to save and restore all the *other* state. How close is the i915 driver from not having to even signal X? Or is that just a pipedream of mine? Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Suspend-devel] 2.6.25-rc2 System no longer powers off aftersuspend-to-disk. Screen becomes green.
On Thursday, February 21, 2008 5:28 pm Linus Torvalds wrote: On Thu, 21 Feb 2008, Jesse Barnes wrote: So the advantage of the kernel suspend/resume hooks for the DRM layer is that the kernel video drivers can do full state save/restore (which X usually doesn't do, and isn't really designed to do), so that if your platform *doesn't* do it all, you'll still end up with a usable machine in the end. Well, I'm also hoping that eventually we could even just not do the VT switch at all, and the kernel can treat X as just another user process that it freezes. Hell yes. At least from a mode setting standpoint. We'd still want to make sure that X repaints the screen if the contents were lost, of course. And this is going to depend very intimately on the type of graphics card and whether the video RAM is saved by STR or not - for the Intel integrated graphics kind of situation, the video RAM will be refreshed along with all the other memory, but for other cards we may end up having to do the VT switch not so much for modesetting reasons as just a way to get X to save and restore all the *other* state. Drivers supporting kernel modesetting will have to stuff their VRAM somewhere, yeah. Hopefully X won't have much to do with it though... How close is the i915 driver from not having to even signal X? Or is that just a pipedream of mine? It's there in the modesetting tree (though the requisite changes to avoid VT notification aren't done, it should all work fine). Jesse -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
