2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Pedro Ribeiro]

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

Hi,

commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
while aborting hibernation) introduced two new issues which were not
present in 2.6.33-rc7:

- every second resume from hibernate results in a blank screen
- the annoying flash at the end of atomic copy/restore during the
hibernate process is back (present in kernels < 2.6.33)

The first issue is serious, the second is just an annoyance.

Reverting this commit fixes both issues.

My platform is Lenovo T400, G45 chipset.

lspci is attached, please let me know if you need more info.

Regards,
Pedro

[-- Attachment #2: lspci --]
[-- Type: application/octet-stream, Size: 14058 bytes --]

00:00.0 Host bridge: Intel Corporation Mobile 4 Series Chipset Memory Controller Hub (rev 07)
	Subsystem: Lenovo Device 20e0
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
	Latency: 0
	Capabilities: <access denied>
	Kernel driver in use: agpgart-intel

00:02.0 VGA compatible controller: Intel Corporation Mobile 4 Series Chipset Integrated Graphics Controller (rev 07) (prog-if 00 [VGA controller])
	Subsystem: Lenovo Device 20e4
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 31
	Region 0: Memory at f4400000 (64-bit, non-prefetchable) [size=4M]
	Region 2: Memory at d0000000 (64-bit, prefetchable) [size=256M]
	Region 4: I/O ports at 1800 [size=8]
	Expansion ROM at <unassigned> [disabled]
	Capabilities: <access denied>
	Kernel driver in use: i915

00:02.1 Display controller: Intel Corporation Mobile 4 Series Chipset Integrated Graphics Controller (rev 07)
	Subsystem: Lenovo Device 20e4
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Region 0: Memory at f4200000 (64-bit, non-prefetchable) [size=1M]
	Capabilities: <access denied>

00:03.0 Communication controller: Intel Corporation Mobile 4 Series Chipset MEI Controller (rev 07)
	Subsystem: Lenovo Device 20e6
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 11
	Region 0: Memory at fc225800 (64-bit, non-prefetchable) [size=16]
	Capabilities: <access denied>

00:19.0 Ethernet controller: Intel Corporation 82567LF Gigabit Network Connection (rev 03)
	Subsystem: Lenovo Device 20ee
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 27
	Region 0: Memory at fc000000 (32-bit, non-prefetchable) [size=128K]
	Region 1: Memory at fc024000 (32-bit, non-prefetchable) [size=4K]
	Region 2: I/O ports at 1820 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: e1000e

00:1a.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #4 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo Device 20f0
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 20
	Region 4: I/O ports at 1840 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: uhci_hcd

00:1a.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #5 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo Device 20f0
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin B routed to IRQ 21
	Region 4: I/O ports at 1860 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: uhci_hcd

00:1a.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #6 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo Device 20f0
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin C routed to IRQ 22
	Region 4: I/O ports at 1880 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: uhci_hcd

00:1a.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI Controller #2 (rev 03) (prog-if 20 [EHCI])
	Subsystem: Lenovo Device 20f1
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin D routed to IRQ 23
	Region 0: Memory at fc225c00 (32-bit, non-prefetchable) [size=1K]
	Capabilities: <access denied>
	Kernel driver in use: ehci_hcd

00:1b.0 Audio device: Intel Corporation 82801I (ICH9 Family) HD Audio Controller (rev 03)
	Subsystem: Lenovo Device 20f2
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin B routed to IRQ 30
	Region 0: Memory at fc020000 (64-bit, non-prefetchable) [size=16K]
	Capabilities: <access denied>
	Kernel driver in use: HDA Intel

00:1c.0 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 1 (rev 03) (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Bus: primary=00, secondary=02, subordinate=02, sec-latency=0
	I/O behind bridge: 00002000-00002fff
	Memory behind bridge: c0000000-c01fffff
	Prefetchable memory behind bridge: 00000000c0200000-00000000c03fffff
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
	BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: <access denied>
	Kernel driver in use: pcieport

00:1c.1 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 2 (rev 03) (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Bus: primary=00, secondary=03, subordinate=03, sec-latency=0
	I/O behind bridge: 00008000-00008fff
	Memory behind bridge: f4300000-f43fffff
	Prefetchable memory behind bridge: 00000000c0400000-00000000c05fffff
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
	BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: <access denied>
	Kernel driver in use: pcieport

00:1c.4 PCI bridge: Intel Corporation 82801I (ICH9 Family) PCI Express Port 5 (rev 03) (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Bus: primary=00, secondary=0d, subordinate=14, sec-latency=0
	I/O behind bridge: 00003000-00003fff
	Memory behind bridge: fa000000-fbffffff
	Prefetchable memory behind bridge: 00000000f4100000-00000000f41fffff
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- <SERR- <PERR-
	BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: <access denied>
	Kernel driver in use: pcieport

00:1d.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #1 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo Device 20f0
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin A routed to IRQ 16
	Region 4: I/O ports at 18a0 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: uhci_hcd

00:1d.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #2 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo Device 20f0
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin B routed to IRQ 17
	Region 4: I/O ports at 18c0 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: uhci_hcd

00:1d.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI Controller #3 (rev 03) (prog-if 00 [UHCI])
	Subsystem: Lenovo Device 20f0
	Control: I/O+ Mem- BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin C routed to IRQ 18
	Region 4: I/O ports at 18e0 [size=32]
	Capabilities: <access denied>
	Kernel driver in use: uhci_hcd

00:1d.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI Controller #1 (rev 03) (prog-if 20 [EHCI])
	Subsystem: Lenovo Device 20f1
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin D routed to IRQ 19
	Region 0: Memory at fc226000 (32-bit, non-prefetchable) [size=1K]
	Capabilities: <access denied>
	Kernel driver in use: ehci_hcd

00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev 93) (prog-if 01 [Subtractive decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Bus: primary=00, secondary=15, subordinate=18, sec-latency=32
	I/O behind bridge: 00004000-00007fff
	Memory behind bridge: f4800000-f7ffffff
	Prefetchable memory behind bridge: 00000000f0000000-00000000f3ffffff
	Secondary status: 66MHz- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort+ <SERR- <PERR-
	BridgeCtl: Parity- SERR- NoISA+ VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: <access denied>

00:1f.0 ISA bridge: Intel Corporation ICH9M LPC Interface Controller (rev 03)
	Subsystem: Lenovo Device 20f6
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Capabilities: <access denied>

00:1f.2 SATA controller: Intel Corporation ICH9M/M-E SATA AHCI Controller (rev 03) (prog-if 01 [AHCI 1.0])
	Subsystem: Lenovo Device 20f8
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz+ UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0
	Interrupt: pin B routed to IRQ 28
	Region 0: I/O ports at 1818 [size=8]
	Region 1: I/O ports at 180c [size=4]
	Region 2: I/O ports at 1810 [size=8]
	Region 3: I/O ports at 1808 [size=4]
	Region 4: I/O ports at 1c00 [size=32]
	Region 5: Memory at fc225000 (32-bit, non-prefetchable) [size=2K]
	Capabilities: <access denied>
	Kernel driver in use: ahci

00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 03)
	Subsystem: Lenovo Device 20f9
	Control: I/O+ Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap- 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Interrupt: pin A routed to IRQ 23
	Region 0: Memory at fc226400 (64-bit, non-prefetchable) [size=256]
	Region 4: I/O ports at 1c20 [size=32]
	Kernel driver in use: i801_smbus

03:00.0 Network controller: Intel Corporation PRO/Wireless 5100 AGN [Shiloh] Network Connection
	Subsystem: Intel Corporation PRO/Wireless 5100AGN Network Connection
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 29
	Region 0: Memory at f4300000 (64-bit, non-prefetchable) [size=8K]
	Capabilities: <access denied>
	Kernel driver in use: iwlagn

15:00.0 CardBus bridge: Ricoh Co Ltd RL5c476 II (rev ba)
	Subsystem: Lenovo Device 20c6
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 32, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 16
	Region 0: Memory at f4800000 (32-bit, non-prefetchable) [size=4K]
	Bus: primary=15, secondary=16, subordinate=17, sec-latency=176
	Memory window 0: f0000000-f3fff000 (prefetchable)
	Memory window 1: c4000000-c7fff000 (prefetchable)
	I/O window 0: 00004000-000040ff
	I/O window 1: 00004400-000044ff
	BridgeCtl: Parity- SERR- ISA- VGA- MAbort- >Reset- 16bInt+ PostWrite+
	16-bit legacy interface ports at 0001

15:00.1 FireWire (IEEE 1394): Ricoh Co Ltd R5C832 IEEE 1394 Controller (rev 04) (prog-if 10 [OHCI])
	Subsystem: Lenovo Device 20c7
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=medium >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 32 (500ns min, 1000ns max), Cache Line Size: 64 bytes
	Interrupt: pin B routed to IRQ 17
	Region 0: Memory at f4801000 (32-bit, non-prefetchable) [size=2K]
	Capabilities: <access denied>

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Rafael J. Wysocki]

On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> Hi,
> 
> commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
> while aborting hibernation) introduced two new issues which were not
> present in 2.6.33-rc7:
> 
> - every second resume from hibernate results in a blank screen
> - the annoying flash at the end of atomic copy/restore during the
> hibernate process is back (present in kernels < 2.6.33)
> 
> The first issue is serious, the second is just an annoyance.

The second one is an expected price of fixing the aborted hibernation
regression.

The first one shouldn't happen, though.

I'll see if I can reproduce that locally.

Rafael

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Rafael J. Wysocki]

On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> > Hi,
> > 
> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
> > while aborting hibernation) introduced two new issues which were not
> > present in 2.6.33-rc7:
> > 
> > - every second resume from hibernate results in a blank screen
> > - the annoying flash at the end of atomic copy/restore during the
> > hibernate process is back (present in kernels < 2.6.33)
> > 
> > The first issue is serious, the second is just an annoyance.
> 
> The second one is an expected price of fixing the aborted hibernation
> regression.
> 
> The first one shouldn't happen, though.
> 
> I'll see if I can reproduce that locally.

No, I can't.

Is the driver compiled directly into the kernel or modular?

Rafael

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Pedro Ribeiro]

2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
> On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
>> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>> > Hi,
>> >
>> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
>> > while aborting hibernation) introduced two new issues which were not
>> > present in 2.6.33-rc7:
>> >
>> > - every second resume from hibernate results in a blank screen
>> > - the annoying flash at the end of atomic copy/restore during the
>> > hibernate process is back (present in kernels < 2.6.33)
>> >
>> > The first issue is serious, the second is just an annoyance.
>>
>> The second one is an expected price of fixing the aborted hibernation
>> regression.
>>
>> The first one shouldn't happen, though.
>>
>> I'll see if I can reproduce that locally.
>
> No, I can't.
>
> Is the driver compiled directly into the kernel or modular?
>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

The driver is modular.
And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
a difference.

However, every since I reverted that commit I've done 10 test
hibernations and no hang so far.

Pedro

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Rafael J. Wysocki]

On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> 2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
> >> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> > Hi,
> >> >
> >> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
> >> > while aborting hibernation) introduced two new issues which were not
> >> > present in 2.6.33-rc7:
> >> >
> >> > - every second resume from hibernate results in a blank screen
> >> > - the annoying flash at the end of atomic copy/restore during the
> >> > hibernate process is back (present in kernels < 2.6.33)
> >> >
> >> > The first issue is serious, the second is just an annoyance.
> >>
> >> The second one is an expected price of fixing the aborted hibernation
> >> regression.
> >>
> >> The first one shouldn't happen, though.
> >>
> >> I'll see if I can reproduce that locally.
> >
> > No, I can't.
> >
> > Is the driver compiled directly into the kernel or modular?
> 
> The driver is modular.
> And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
> a difference.

It shouldn't in fact, although I'm not sure.

> However, every since I reverted that commit I've done 10 test
> hibernations and no hang so far.

First, please try if you can reproduce it with non-modular driver.

Second, please check if the appended patch helps.

Rafael

---
 drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
+++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
@@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
 
 static int i915_drm_freeze(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	pci_save_state(dev->pdev);
 
 	/* If KMS is active, we do the leavevt stuff here */
@@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
 
 	i915_save_state(dev);
 
-	return 0;
-}
-
-static void i915_drm_suspend(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
 	intel_opregion_free(dev, 1);
 
 	/* Modeset on resume, not lid events */
 	dev_priv->modeset_on_lid = 0;
+
+	return 0;
 }
 
 static int i915_suspend(struct drm_device *dev, pm_message_t state)
@@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
 	if (error)
 		return error;
 
-	i915_drm_suspend(dev);
-
 	if (state.event == PM_EVENT_SUSPEND) {
 		/* Shut down the device */
 		pci_disable_device(dev->pdev);
@@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int error = 0;
 
+	i915_restore_state(dev);
+
+	intel_opregion_init(dev, 1);
+
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		mutex_lock(&dev->struct_mutex);
@@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
 
 	pci_set_master(dev->pdev);
 
-	i915_restore_state(dev);
-
-	intel_opregion_init(dev, 1);
-
 	return i915_drm_thaw(dev);
 }
 
@@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
 	if (error)
 		return error;
 
-	i915_drm_suspend(drm_dev);
-
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
 
@@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	int error;
-
-	error = i915_drm_freeze(drm_dev);
-	if (!error)
-		i915_drm_suspend(drm_dev);
 
-	return error;
+	return i915_drm_freeze(drm_dev);
 }
 
 const struct dev_pm_ops i915_pm_ops = {

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Nigel Cunningham]

Hi Rafael.

Rafael J. Wysocki wrote:
> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>> 2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
>>> On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
>>>> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>>>>> Hi,
>>>>>
>>>>> commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
>>>>> while aborting hibernation) introduced two new issues which were not
>>>>> present in 2.6.33-rc7:
>>>>>
>>>>> - every second resume from hibernate results in a blank screen
>>>>> - the annoying flash at the end of atomic copy/restore during the
>>>>> hibernate process is back (present in kernels < 2.6.33)
>>>>>
>>>>> The first issue is serious, the second is just an annoyance.
>>>> The second one is an expected price of fixing the aborted hibernation
>>>> regression.
>>>>
>>>> The first one shouldn't happen, though.
>>>>
>>>> I'll see if I can reproduce that locally.
>>> No, I can't.
>>>
>>> Is the driver compiled directly into the kernel or modular?
>> The driver is modular.
>> And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
>> a difference.
> 
> It shouldn't in fact, although I'm not sure.

You're right. It shouldn't. I'm using exactly the same driver model
calls in exactly the same order, and not changing driver code at all.

Regards,

Nigel

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Tino Keitel]

On Wed, Feb 17, 2010 at 23:20:57 +0100, Rafael J. Wysocki wrote:

[...]

> First, please try if you can reproduce it with non-modular driver.

I got hangs at resume on my ThinkPad X61s with i965 graphics and
non-modular i915 driver. I got the hangs after suspend to RAM and
suspend to disk using TuxOnIce, but not every time. I reverted the 
commit metioned by Pedro and wasn't able to reproduce the hang after
several suspend cycles.

I'll also try the attached patch.

Regards,
Tino

Re: 2.6.33-rc8 regression on i915: resume from hibernate locks up every 2nd time

[Pedro Ribeiro]

On 17 February 2010 22:20, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>> 2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
>> > On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
>> >> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
>> >> > Hi,
>> >> >
>> >> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
>> >> > while aborting hibernation) introduced two new issues which were not
>> >> > present in 2.6.33-rc7:
>> >> >
>> >> > - every second resume from hibernate results in a blank screen
>> >> > - the annoying flash at the end of atomic copy/restore during the
>> >> > hibernate process is back (present in kernels < 2.6.33)
>> >> >
>> >> > The first issue is serious, the second is just an annoyance.
>> >>
>> >> The second one is an expected price of fixing the aborted hibernation
>> >> regression.
>> >>
>> >> The first one shouldn't happen, though.
>> >>
>> >> I'll see if I can reproduce that locally.
>> >
>> > No, I can't.
>> >
>> > Is the driver compiled directly into the kernel or modular?
>>
>> The driver is modular.
>> And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
>> a difference.
>
> It shouldn't in fact, although I'm not sure.
>
>> However, every since I reverted that commit I've done 10 test
>> hibernations and no hang so far.
>
> First, please try if you can reproduce it with non-modular driver.
>
> Second, please check if the appended patch helps.
>
> Rafael
>
> ---
>  drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
>  1 file changed, 9 insertions(+), 21 deletions(-)
>
> Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> ===================================================================
> --- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
> +++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> @@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>
>  static int i915_drm_freeze(struct drm_device *dev)
>  {
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +
>        pci_save_state(dev->pdev);
>
>        /* If KMS is active, we do the leavevt stuff here */
> @@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
>
>        i915_save_state(dev);
>
> -       return 0;
> -}
> -
> -static void i915_drm_suspend(struct drm_device *dev)
> -{
> -       struct drm_i915_private *dev_priv = dev->dev_private;
> -
>        intel_opregion_free(dev, 1);
>
>        /* Modeset on resume, not lid events */
>        dev_priv->modeset_on_lid = 0;
> +
> +       return 0;
>  }
>
>  static int i915_suspend(struct drm_device *dev, pm_message_t state)
> @@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
>        if (error)
>                return error;
>
> -       i915_drm_suspend(dev);
> -
>        if (state.event == PM_EVENT_SUSPEND) {
>                /* Shut down the device */
>                pci_disable_device(dev->pdev);
> @@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
>        struct drm_i915_private *dev_priv = dev->dev_private;
>        int error = 0;
>
> +       i915_restore_state(dev);
> +
> +       intel_opregion_init(dev, 1);
> +
>        /* KMS EnterVT equivalent */
>        if (drm_core_check_feature(dev, DRIVER_MODESET)) {
>                mutex_lock(&dev->struct_mutex);
> @@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
>
>        pci_set_master(dev->pdev);
>
> -       i915_restore_state(dev);
> -
> -       intel_opregion_init(dev, 1);
> -
>        return i915_drm_thaw(dev);
>  }
>
> @@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
>        if (error)
>                return error;
>
> -       i915_drm_suspend(drm_dev);
> -
>        pci_disable_device(pdev);
>        pci_set_power_state(pdev, PCI_D3hot);
>
> @@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
>  {
>        struct pci_dev *pdev = to_pci_dev(dev);
>        struct drm_device *drm_dev = pci_get_drvdata(pdev);
> -       int error;
> -
> -       error = i915_drm_freeze(drm_dev);
> -       if (!error)
> -               i915_drm_suspend(drm_dev);
>
> -       return error;
> +       return i915_drm_freeze(drm_dev);
>  }
>
>  const struct dev_pm_ops i915_pm_ops = {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

The patch fixes this issue for me.

Thanks for your help.

Pedro

[PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Rafael J. Wysocki]

On Thursday 18 February 2010, Pedro Ribeiro wrote:
> On 17 February 2010 22:20, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> 2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
> >> > On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
> >> >> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> >> > Hi,
> >> >> >
> >> >> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
> >> >> > while aborting hibernation) introduced two new issues which were not
> >> >> > present in 2.6.33-rc7:
> >> >> >
> >> >> > - every second resume from hibernate results in a blank screen
> >> >> > - the annoying flash at the end of atomic copy/restore during the
> >> >> > hibernate process is back (present in kernels < 2.6.33)
> >> >> >
> >> >> > The first issue is serious, the second is just an annoyance.
> >> >>
> >> >> The second one is an expected price of fixing the aborted hibernation
> >> >> regression.
> >> >>
> >> >> The first one shouldn't happen, though.
> >> >>
> >> >> I'll see if I can reproduce that locally.
> >> >
> >> > No, I can't.
> >> >
> >> > Is the driver compiled directly into the kernel or modular?
> >>
> >> The driver is modular.
> >> And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
> >> a difference.
> >
> > It shouldn't in fact, although I'm not sure.
> >
> >> However, every since I reverted that commit I've done 10 test
> >> hibernations and no hang so far.
> >
> > First, please try if you can reproduce it with non-modular driver.
> >
> > Second, please check if the appended patch helps.
> >
> > Rafael
> >
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
> >  1 file changed, 9 insertions(+), 21 deletions(-)
> >
> > Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
> > +++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> > @@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
> >
> >  static int i915_drm_freeze(struct drm_device *dev)
> >  {
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> >        pci_save_state(dev->pdev);
> >
> >        /* If KMS is active, we do the leavevt stuff here */
> > @@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
> >
> >        i915_save_state(dev);
> >
> > -       return 0;
> > -}
> > -
> > -static void i915_drm_suspend(struct drm_device *dev)
> > -{
> > -       struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> >        intel_opregion_free(dev, 1);
> >
> >        /* Modeset on resume, not lid events */
> >        dev_priv->modeset_on_lid = 0;
> > +
> > +       return 0;
> >  }
> >
> >  static int i915_suspend(struct drm_device *dev, pm_message_t state)
> > @@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
> >        if (error)
> >                return error;
> >
> > -       i915_drm_suspend(dev);
> > -
> >        if (state.event == PM_EVENT_SUSPEND) {
> >                /* Shut down the device */
> >                pci_disable_device(dev->pdev);
> > @@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
> >        struct drm_i915_private *dev_priv = dev->dev_private;
> >        int error = 0;
> >
> > +       i915_restore_state(dev);
> > +
> > +       intel_opregion_init(dev, 1);
> > +
> >        /* KMS EnterVT equivalent */
> >        if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> >                mutex_lock(&dev->struct_mutex);
> > @@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
> >
> >        pci_set_master(dev->pdev);
> >
> > -       i915_restore_state(dev);
> > -
> > -       intel_opregion_init(dev, 1);
> > -
> >        return i915_drm_thaw(dev);
> >  }
> >
> > @@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
> >        if (error)
> >                return error;
> >
> > -       i915_drm_suspend(drm_dev);
> > -
> >        pci_disable_device(pdev);
> >        pci_set_power_state(pdev, PCI_D3hot);
> >
> > @@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
> >  {
> >        struct pci_dev *pdev = to_pci_dev(dev);
> >        struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > -       int error;
> > -
> > -       error = i915_drm_freeze(drm_dev);
> > -       if (!error)
> > -               i915_drm_suspend(drm_dev);
> >
> > -       return error;
> > +       return i915_drm_freeze(drm_dev);
> >  }
> >
> >  const struct dev_pm_ops i915_pm_ops = {
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
> 
> The patch fixes this issue for me.
> 
> Thanks for your help.

OK, thanks for testing, let's submit it appropriately.

Jesse, Eric, the appended patch fixes a very recent hibernate regression in
i915, please push it to Linus ASAP.

Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: i915 / PM: Fix hibernate regression caused by suspend/resume splitting

Commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
while aborting hibernation) attempted to fix a regression introduced
by commit cbda12d77ea590082edb6d30bd342a67ebc459e0 (drm/i915:
implement new pm ops for i915), but it went too far trying to split
the freeze/suspend and resume/thaw parts of the code.  As a result,
it introduced another regression, which only is visible on some systems.

Fix the problem by merging i915_drm_suspend() with
i915_drm_freeze() and moving some code from i915_resume()
into i915_drm_thaw(), so that intel_opregion_free() and
intel_opregion_init() are also executed in the freeze and thaw code
paths, respectively.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reported-and-tested-by: Pedro Ribeiro <pedrib@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
+++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
@@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
 
 static int i915_drm_freeze(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	pci_save_state(dev->pdev);
 
 	/* If KMS is active, we do the leavevt stuff here */
@@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
 
 	i915_save_state(dev);
 
-	return 0;
-}
-
-static void i915_drm_suspend(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
 	intel_opregion_free(dev, 1);
 
 	/* Modeset on resume, not lid events */
 	dev_priv->modeset_on_lid = 0;
+
+	return 0;
 }
 
 static int i915_suspend(struct drm_device *dev, pm_message_t state)
@@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
 	if (error)
 		return error;
 
-	i915_drm_suspend(dev);
-
 	if (state.event == PM_EVENT_SUSPEND) {
 		/* Shut down the device */
 		pci_disable_device(dev->pdev);
@@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int error = 0;
 
+	i915_restore_state(dev);
+
+	intel_opregion_init(dev, 1);
+
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		mutex_lock(&dev->struct_mutex);
@@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
 
 	pci_set_master(dev->pdev);
 
-	i915_restore_state(dev);
-
-	intel_opregion_init(dev, 1);
-
 	return i915_drm_thaw(dev);
 }
 
@@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
 	if (error)
 		return error;
 
-	i915_drm_suspend(drm_dev);
-
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
 
@@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	int error;
-
-	error = i915_drm_freeze(drm_dev);
-	if (!error)
-		i915_drm_suspend(drm_dev);
 
-	return error;
+	return i915_drm_freeze(drm_dev);
 }
 
 const struct dev_pm_ops i915_pm_ops = {

[PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Rafael J. Wysocki]

On Thursday 18 February 2010, Pedro Ribeiro wrote:
> On 17 February 2010 22:20, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> 2010/2/17 Rafael J. Wysocki <rjw@sisk.pl>:
> >> > On Wednesday 17 February 2010, Rafael J. Wysocki wrote:
> >> >> On Wednesday 17 February 2010, Pedro Ribeiro wrote:
> >> >> > Hi,
> >> >> >
> >> >> > commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
> >> >> > while aborting hibernation) introduced two new issues which were not
> >> >> > present in 2.6.33-rc7:
> >> >> >
> >> >> > - every second resume from hibernate results in a blank screen
> >> >> > - the annoying flash at the end of atomic copy/restore during the
> >> >> > hibernate process is back (present in kernels < 2.6.33)
> >> >> >
> >> >> > The first issue is serious, the second is just an annoyance.
> >> >>
> >> >> The second one is an expected price of fixing the aborted hibernation
> >> >> regression.
> >> >>
> >> >> The first one shouldn't happen, though.
> >> >>
> >> >> I'll see if I can reproduce that locally.
> >> >
> >> > No, I can't.
> >> >
> >> > Is the driver compiled directly into the kernel or modular?
> >>
> >> The driver is modular.
> >> And sorry, I forgot to tell you I'm using TuxOnIce, it certainly makes
> >> a difference.
> >
> > It shouldn't in fact, although I'm not sure.
> >
> >> However, every since I reverted that commit I've done 10 test
> >> hibernations and no hang so far.
> >
> > First, please try if you can reproduce it with non-modular driver.
> >
> > Second, please check if the appended patch helps.
> >
> > Rafael
> >
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
> >  1 file changed, 9 insertions(+), 21 deletions(-)
> >
> > Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
> > +++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
> > @@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
> >
> >  static int i915_drm_freeze(struct drm_device *dev)
> >  {
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> >        pci_save_state(dev->pdev);
> >
> >        /* If KMS is active, we do the leavevt stuff here */
> > @@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
> >
> >        i915_save_state(dev);
> >
> > -       return 0;
> > -}
> > -
> > -static void i915_drm_suspend(struct drm_device *dev)
> > -{
> > -       struct drm_i915_private *dev_priv = dev->dev_private;
> > -
> >        intel_opregion_free(dev, 1);
> >
> >        /* Modeset on resume, not lid events */
> >        dev_priv->modeset_on_lid = 0;
> > +
> > +       return 0;
> >  }
> >
> >  static int i915_suspend(struct drm_device *dev, pm_message_t state)
> > @@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
> >        if (error)
> >                return error;
> >
> > -       i915_drm_suspend(dev);
> > -
> >        if (state.event == PM_EVENT_SUSPEND) {
> >                /* Shut down the device */
> >                pci_disable_device(dev->pdev);
> > @@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
> >        struct drm_i915_private *dev_priv = dev->dev_private;
> >        int error = 0;
> >
> > +       i915_restore_state(dev);
> > +
> > +       intel_opregion_init(dev, 1);
> > +
> >        /* KMS EnterVT equivalent */
> >        if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> >                mutex_lock(&dev->struct_mutex);
> > @@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
> >
> >        pci_set_master(dev->pdev);
> >
> > -       i915_restore_state(dev);
> > -
> > -       intel_opregion_init(dev, 1);
> > -
> >        return i915_drm_thaw(dev);
> >  }
> >
> > @@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
> >        if (error)
> >                return error;
> >
> > -       i915_drm_suspend(drm_dev);
> > -
> >        pci_disable_device(pdev);
> >        pci_set_power_state(pdev, PCI_D3hot);
> >
> > @@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
> >  {
> >        struct pci_dev *pdev = to_pci_dev(dev);
> >        struct drm_device *drm_dev = pci_get_drvdata(pdev);
> > -       int error;
> > -
> > -       error = i915_drm_freeze(drm_dev);
> > -       if (!error)
> > -               i915_drm_suspend(drm_dev);
> >
> > -       return error;
> > +       return i915_drm_freeze(drm_dev);
> >  }
> >
> >  const struct dev_pm_ops i915_pm_ops = {
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> >
> 
> The patch fixes this issue for me.
> 
> Thanks for your help.

OK, thanks for testing, let's submit it appropriately.

Jesse, Eric, the appended patch fixes a very recent hibernate regression in
i915, please push it to Linus ASAP.

Rafael

---
From: Rafael J. Wysocki <rjw@sisk.pl>
Subject: i915 / PM: Fix hibernate regression caused by suspend/resume splitting

Commit 84b79f8d2882b0a84330c04839ed4d3cefd2ff77 (drm/i915: Fix crash
while aborting hibernation) attempted to fix a regression introduced
by commit cbda12d77ea590082edb6d30bd342a67ebc459e0 (drm/i915:
implement new pm ops for i915), but it went too far trying to split
the freeze/suspend and resume/thaw parts of the code.  As a result,
it introduced another regression, which only is visible on some systems.

Fix the problem by merging i915_drm_suspend() with
i915_drm_freeze() and moving some code from i915_resume()
into i915_drm_thaw(), so that intel_opregion_free() and
intel_opregion_init() are also executed in the freeze and thaw code
paths, respectively.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reported-and-tested-by: Pedro Ribeiro <pedrib@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c |   30 +++++++++---------------------
 1 file changed, 9 insertions(+), 21 deletions(-)

Index: linux-2.6/drivers/gpu/drm/i915/i915_drv.c
===================================================================
--- linux-2.6.orig/drivers/gpu/drm/i915/i915_drv.c
+++ linux-2.6/drivers/gpu/drm/i915/i915_drv.c
@@ -177,6 +177,8 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
 
 static int i915_drm_freeze(struct drm_device *dev)
 {
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
 	pci_save_state(dev->pdev);
 
 	/* If KMS is active, we do the leavevt stuff here */
@@ -192,17 +194,12 @@ static int i915_drm_freeze(struct drm_de
 
 	i915_save_state(dev);
 
-	return 0;
-}
-
-static void i915_drm_suspend(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
 	intel_opregion_free(dev, 1);
 
 	/* Modeset on resume, not lid events */
 	dev_priv->modeset_on_lid = 0;
+
+	return 0;
 }
 
 static int i915_suspend(struct drm_device *dev, pm_message_t state)
@@ -222,8 +219,6 @@ static int i915_suspend(struct drm_devic
 	if (error)
 		return error;
 
-	i915_drm_suspend(dev);
-
 	if (state.event == PM_EVENT_SUSPEND) {
 		/* Shut down the device */
 		pci_disable_device(dev->pdev);
@@ -238,6 +233,10 @@ static int i915_drm_thaw(struct drm_devi
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int error = 0;
 
+	i915_restore_state(dev);
+
+	intel_opregion_init(dev, 1);
+
 	/* KMS EnterVT equivalent */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
 		mutex_lock(&dev->struct_mutex);
@@ -264,10 +263,6 @@ static int i915_resume(struct drm_device
 
 	pci_set_master(dev->pdev);
 
-	i915_restore_state(dev);
-
-	intel_opregion_init(dev, 1);
-
 	return i915_drm_thaw(dev);
 }
 
@@ -424,8 +419,6 @@ static int i915_pm_suspend(struct device
 	if (error)
 		return error;
 
-	i915_drm_suspend(drm_dev);
-
 	pci_disable_device(pdev);
 	pci_set_power_state(pdev, PCI_D3hot);
 
@@ -465,13 +458,8 @@ static int i915_pm_poweroff(struct devic
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
 	struct drm_device *drm_dev = pci_get_drvdata(pdev);
-	int error;
-
-	error = i915_drm_freeze(drm_dev);
-	if (!error)
-		i915_drm_suspend(drm_dev);
 
-	return error;
+	return i915_drm_freeze(drm_dev);
 }
 
 const struct dev_pm_ops i915_pm_ops = {

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Tino Keitel]

On Thu, Feb 18, 2010 at 23:06:27 +0100, Rafael J. Wysocki wrote:

[...]

> Jesse, Eric, the appended patch fixes a very recent hibernate regression in
> i915, please push it to Linus ASAP.

FYI: I got no resume failure during normal usage after 9 suspend to RAM
and 8 suspend to disk cycles (using both suspend types alternately)
with the patch applied.

Regards,
Tino

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Tino Keitel]

On Thu, Feb 18, 2010 at 23:06:27 +0100, Rafael J. Wysocki wrote:

[...]

> Jesse, Eric, the appended patch fixes a very recent hibernate regression in
> i915, please push it to Linus ASAP.

FYI: I got no resume failure during normal usage after 9 suspend to RAM
and 8 suspend to disk cycles (using both suspend types alternately)
with the patch applied.

Regards,
Tino

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Linus Torvalds]

Jesse, Eric, what's the status of this patch? 

Should I take it directly (I'd really like an Ack) or is it queued up in 
some tree waiting for me to pull? Or is there some reason not to have it, 
despite it fixing things for some people?

		Linus

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Linus Torvalds]

Jesse, Eric, what's the status of this patch? 

Should I take it directly (I'd really like an Ack) or is it queued up in 
some tree waiting for me to pull? Or is there some reason not to have it, 
despite it fixing things for some people?

		Linus

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Jesse Barnes]

On Mon, 22 Feb 2010 07:47:53 -0800 (PST)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> Jesse, Eric, what's the status of this patch? 
> 
> Should I take it directly (I'd really like an Ack) or is it queued up in 
> some tree waiting for me to pull? Or is there some reason not to have it, 
> despite it fixing things for some people?

Hm, I thought Eric had applied that to his for-linus branch, but it
looks like that branch is empty now.  Maybe Rafael posted an update
that Eric missed and so you just have a partial fix in your tree?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Jesse Barnes]

On Mon, 22 Feb 2010 07:47:53 -0800 (PST)
Linus Torvalds <torvalds@linux-foundation.org> wrote:

> 
> Jesse, Eric, what's the status of this patch? 
> 
> Should I take it directly (I'd really like an Ack) or is it queued up in 
> some tree waiting for me to pull? Or is there some reason not to have it, 
> despite it fixing things for some people?

Hm, I thought Eric had applied that to his for-linus branch, but it
looks like that branch is empty now.  Maybe Rafael posted an update
that Eric missed and so you just have a partial fix in your tree?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Eric Anholt]

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

On Mon, 22 Feb 2010 07:47:53 -0800 (PST), Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> Jesse, Eric, what's the status of this patch? 
> 
> Should I take it directly (I'd really like an Ack) or is it queued up in 
> some tree waiting for me to pull? Or is there some reason not to have it, 
> despite it fixing things for some people?

Acked-by: Eric Anholt <eric@anholt.net>

I just reviewed the rest of my tag:todo and I don't think I have
anything else I want to pull request for this kernel (finally), so if
you could just grab this patch it would be great.

(Well, there's also "quit trying to _LID on 855 because it's always a
pack of lies", but I'd rather do it as a backport later in case someone
screams)

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] i915 / PM: Fix hibernate regression caused by suspend/resume splitting

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 744 bytes --]

On Mon, 22 Feb 2010 07:47:53 -0800 (PST), Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> Jesse, Eric, what's the status of this patch? 
> 
> Should I take it directly (I'd really like an Ack) or is it queued up in 
> some tree waiting for me to pull? Or is there some reason not to have it, 
> despite it fixing things for some people?

Acked-by: Eric Anholt <eric@anholt.net>

I just reviewed the rest of my tag:todo and I don't think I have
anything else I want to pull request for this kernel (finally), so if
you could just grab this patch it would be great.

(Well, there's also "quit trying to _LID on 855 because it's always a
pack of lies", but I'd rather do it as a backport later in case someone
screams)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]