amd74xx crashes when resuming from STR

amd74xx crashes when resuming from STR

[Jason Lunz]

On my laptop, suspend-to-ram works for all drivers with the exception of
the amd74xx ide driver. And even then, it only has problems when
accessing a UDMA hard drive. I know this because the system can use STR
reliably when booted from a livecd, so long as nothing accesses the hard
disk.

I'm running amd64 2.6.17 untainted. The motherboard and ide chipset are
nvidia:

# lspci
00:00.0 Host bridge: nVidia Corporation nForce3 Host Bridge (rev a4)
00:01.0 ISA bridge: nVidia Corporation nForce3 LPC Bridge (rev a6)
00:01.1 SMBus: nVidia Corporation nForce3 SMBus (rev a4)
00:02.0 USB Controller: nVidia Corporation nForce3 USB 1.1 (rev a5)
00:02.1 USB Controller: nVidia Corporation nForce3 USB 1.1 (rev a5)
00:02.2 USB Controller: nVidia Corporation nForce3 USB 2.0 (rev a2)
00:06.0 Multimedia audio controller: nVidia Corporation nForce3 Audio (rev a2)
00:06.1 Modem: nVidia Corporation nForce3 Audio (rev a2)
00:08.0 IDE interface: nVidia Corporation nForce3 IDE (rev a5)
00:0a.0 PCI bridge: nVidia Corporation nForce3 PCI Bridge (rev a2)
00:0b.0 PCI bridge: nVidia Corporation nForce3 AGP Bridge (rev a4)
00:18.0 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] HyperTransport Technology Configuration
00:18.1 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Address Map
00:18.2 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] DRAM Controller
00:18.3 Host bridge: Advanced Micro Devices [AMD] K8 [Athlon64/Opteron] Miscellaneous Control
01:00.0 FireWire (IEEE 1394): Texas Instruments TSB43AB21 IEEE-1394a-2000 Controller (PHY/Link)
01:01.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139/8139C/8139C+ (rev 10)
01:02.0 Network controller: Broadcom Corporation BCM4306 802.11b/g Wireless LAN Controller (rev 03)
01:04.0 CardBus bridge: Texas Instruments PCI1620 PC Card Controller (rev 01)
01:04.1 CardBus bridge: Texas Instruments PCI1620 PC Card Controller (rev 01)
01:04.2 System peripheral: Texas Instruments PCI1620 Firmware Loading Function (rev 01)
0a:00.0 VGA compatible controller: nVidia Corporation NV17 [GeForce4 420 Go 32M] (rev a3)

I know that the amd74xx driver is definitely the problem, because STR
works reliably when using the ide-generic driver. But in that case
there's no DMA and the drive is painfully slow. And the crash is not
DMA-related, because the cdrom also uses DMA, yet it does trouble-free
suspend/resume under amd74xx.

Here's info from driver load, and /proc:

Uniform Multi-Platform E-IDE driver Revision: 7.00alpha2
ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx
NFORCE3-150: IDE controller at PCI slot 0000:00:08.0
NFORCE3-150: chipset revision 165
NFORCE3-150: not 100% native mode: will probe irqs later
NFORCE3-150: BIOS didn't set cable bits correctly. Enabling workaround.
NFORCE3-150: 0000:00:08.0 (rev a5) UDMA133 controller
    ide0: BM-DMA at 0x2080-0x2087, BIOS settings: hda:DMA, hdb:pio
    ide1: BM-DMA at 0x2088-0x208f, BIOS settings: hdc:DMA, hdd:pio
Probing IDE interface ide0...
hda: HITACHI_DK23DA-20, ATA DISK drive
isa bounce pool size: 16 pages
ide0 at 0x1f0-0x1f7,0x3f6 on irq 14
Probing IDE interface ide1...
hdc: HL-DT-STCD-RW/DVD DRIVE GCC-4241N, ATAPI CD/DVD-ROM drive
ide1 at 0x170-0x177,0x376 on irq 15
hda: max request size: 128KiB
hda: 39070080 sectors (20003 MB) w/2048KiB Cache, CHS=38760/16/63, UDMA(100)
hda: cache flushes supported
 hda: hda1 hda2 hda3

# cat /proc/ide/amd74xx
----------AMD BusMastering IDE Configuration----------------
Driver Version:                     2.13
South Bridge:                       0000:00:08.0
Revision:                           IDE 0xa5
Highest DMA rate:                   UDMA133
BM-DMA base:                        0x2080
PCI clock:                          33.3MHz
-----------------------Primary IDE-------Secondary IDE------
Prefetch Buffer:              yes                 yes
Post Write Buffer:            yes                 yes
Enabled:                      yes                 yes
Simplex only:                  no                  no
Cable Type:                   80w                 40w
-------------------drive0----drive1----drive2----drive3-----
Transfer Mode:       UDMA       PIO       DMA       PIO
Address Setup:       30ns      90ns      30ns      90ns
Cmd Active:          90ns      90ns      90ns      90ns
Cmd Recovery:        30ns      30ns      30ns      30ns
Data Active:         90ns     330ns      90ns     330ns
Data Recovery:       30ns     270ns      30ns     270ns
Cycle Time:          20ns     600ns     120ns     600ns
Transfer Rate:   99.9MB/s   3.3MB/s  16.6MB/s   3.3MB/s

Here's the crash that occurs post-resume, as captured by netconsole. I
compiled drivers/ide/ide-io.c with DEBUG_PM #defined:

netconsole: network logging started
Stopping tasks: ============================================|
hdc: start_power_step(step: 0)
hdc: completing PM request, suspend
hda: start_power_step(step: 0)
hda: complete_power_step(step: 0, stat: 50, err: 0)
hda: start_power_step(step: 1)
hda: complete_power_step(step: 1, stat: 50, err: 0)
hda: completing PM request, suspend
pnp: Device 00:0b disabled.
ACPI: PCI interrupt for device 0000:01:04.1 disabled
ACPI: PCI interrupt for device 0000:01:04.0 disabled
ACPI: PCI interrupt for device 0000:01:02.0 disabled

PCI: Enabling device 0000:01:02.0 (0000 -> 0002)
ACPI: PCI Interrupt 0000:01:02.0[A] -> Link [LNK3] -> GSI 17 (level, low) -> IRQ 21
PM: Writing back config space on device 0000:01:02.0 at offset f (was 100, writing 10b)
PM: Writing back config space on device 0000:01:02.0 at offset 4 (was 0, writing e0104000)
PM: Writing back config space on device 0000:01:02.0 at offset 3 (was 0, writing 4000)
PM: Writing back config space on device 0000:01:02.0 at offset 1 (was 2, writing 106)
PM: Writing back config space on device 0000:01:04.0 at offset f (was 34001ff, writing 5c0010b)
PM: Writing back config space on device 0000:01:04.0 at offset e (was 0, writing 34fc)
PM: Writing back config space on device 0000:01:04.0 at offset d (was 0, writing 3400)
PM: Writing back config space on device 0000:01:04.0 at offset c (was 0, writing 30fc)
PM: Writing back config space on device 0000:01:04.0 at offset b (was 0, writing 3000)
PM: Writing back config space on device 0000:01:04.0 at offset a (was 0, writing e07ff000)
PM: Writing back config space on device 0000:01:04.0 at offset 8 (was 0, writing 31fff000)
PM: Writing back config space on device 0000:01:04.0 at offset 6 (was 40000000, writing b0050201)
PM: Writing back config space on device 0000:01:04.0 at offset 3 (was 824008, writing 82a810)
PM: Writing back config space on device 0000:01:04.0 at offset 1 (was 2100107, writing 2100007)
ACPI: PCI Interrupt 0000:01:04.0[A] -> Link [LNK1] -> GSI 19 (level, low) -> IRQ 16
PM: Writing back config space on device 0000:01:04.1 at offset f (was 34002ff, writing 5c0020a)
PM: Writing back config space on device 0000:01:04.1 at offset e (was 0, writing 3cfc)
PM: Writing back config space on device 0000:01:04.1 at offset d (was 0, writing 3c00)
PM: Writing back config space on device 0000:01:04.1 at offset c (was 0, writing 38fc)
PM: Writing back config space on device 0000:01:04.1 at offset b (was 0, writing 3800)
PM: Writing back config space on device 0000:01:04.1 at offset a (was 0, writing e0fff000)
PM: Writing back config space on device 0000:01:04.1 at offset 8 (was 0, writing 33fff000)
PM: Writing back config space on device 0000:01:04.1 at offset 7 (was e1000000, writing 32000000)
PM: Writing back config space on device 0000:01:04.1 at offset 6 (was 40000000, writing b0090601)
PM: Writing back config space on device 0000:01:04.1 at offset 3 (was 824008, writing 82a810)
PM: Writing back config space on device 0000:01:04.1 at offset 1 (was 2100103, writing 2100007)
ACPI: PCI Interrupt 0000:01:04.1[B] -> Link [LNK2] -> GSI 18 (level, low) -> IRQ 17
PM: Writing back config space on device 0000:01:04.2 at offset 4 (was 1, writing 7401)
PM: Writing back config space on device 0000:01:04.2 at offset 3 (was 0, writing 4010)
PM: Writing back config space on device 0000:01:04.2 at offset 1 (was 2100000, writing 2100107)
PM: Writing back config space on device 0000:0a:00.0 at offset f (was 1050100, writing 105010b)
PM: Writing back config space on device 0000:0a:00.0 at offset 6 (was 8, writing f8000008)
PM: Writing back config space on device 0000:0a:00.0 at offset 5 (was 8, writing f0000008)
PM: Writing back config space on device 0000:0a:00.0 at offset 4 (was 0, writing e2000000)
PM: Writing back config space on device 0000:0a:00.0 at offset 3 (was 0, writing 4000)
PM: Writing back config space on device 0000:0a:00.0 at offset 1 (was 2b00000, writing 2b00007)
pnp: Res cnt 3
pnp: res cnt 3
pnp: Encode io
pnp: Encode io
pnp: Encode irq
pnp: Failed to activate device 00:08.
pnp: Res cnt 1
pnp: res cnt 1
pnp: Encode irq
pnp: Failed to activate device 00:09.
pnp: Res cnt 4
pnp: res cnt 4
pnp: Encode io
pnp: Encode io
pnp: Encode irq
pnp: Encode dma
pnp: Device 00:0b activated.
hda: Wakeup request inited, waiting for !BSY...
hda: start_power_step(step: 1000)
hda: complete_power_step(step: 1000, stat: 50, err: 0)
hda: start_power_step(step: 1001)
hda: completing PM request, resume
hdc: Wakeup request inited, waiting for !BSY...
hdc: start_power_step(step: 1000)
hdc: completing PM request, resume
Restarting tasks...
 done
hda: dma_timer_expiry: dma status == 0x21
hda: DMA timeout error

HARDWARE ERROR
CPU 0: Machine Check Exception:                4 Bank 4: b200000000070f0f
TSC 1370e9bdb9
This is not a software problem!
Run through mcelog --ascii to decode and contact your hardware vendor
Kernel panic - not syncing: Machine check

Does this driver need special handling? I notice one other driver in
drivers/ide/pci, sc1200, implements its own pci_driver->suspend() and
pci_driver->resume() hooks. Maybe similar methods are needed in this
case?

thanks,

Jason

Re: [linux-pm] amd74xx crashes when resuming from STR

[Pavel Machek]

Hi!

> On my laptop, suspend-to-ram works for all drivers with the exception of
> the amd74xx ide driver. And even then, it only has problems when
> accessing a UDMA hard drive. I know this because the system can use STR
> reliably when booted from a livecd, so long as nothing accesses the hard
> disk.

I guess you need to ask on linux-ide mainling lists?

Ahha, you are cross-posting there, already.

What happens if you rmmod amd74xx and insmod it after resume?
							Pavel

-- 
Thanks for all the (sleeping) penguins.

Re: amd74xx crashes when resuming from STR

[Pavel Machek]

Hi!

> On my laptop, suspend-to-ram works for all drivers with the exception of
> the amd74xx ide driver. And even then, it only has problems when
> accessing a UDMA hard drive. I know this because the system can use STR
> reliably when booted from a livecd, so long as nothing accesses the hard
> disk.

I guess you need to ask on linux-ide mainling lists?

Ahha, you are cross-posting there, already.

What happens if you rmmod amd74xx and insmod it after resume?
							Pavel

-- 
Thanks for all the (sleeping) penguins.

Re: [linux-pm] amd74xx crashes when resuming from STR

[Vojtech Pavlik]

On Thu, Jan 01, 1970 at 12:37:16AM +0000, Pavel Machek wrote:
> Hi!
> 
> > On my laptop, suspend-to-ram works for all drivers with the exception of
> > the amd74xx ide driver. And even then, it only has problems when
> > accessing a UDMA hard drive. I know this because the system can use STR
> > reliably when booted from a livecd, so long as nothing accesses the hard
> > disk.
> 
> I guess you need to ask on linux-ide mainling lists?
> 
> Ahha, you are cross-posting there, already.
> 
> What happens if you rmmod amd74xx and insmod it after resume?
 
amd74xx relies on the BIOS to have set up a significant part of the
controller correctly. This likely isn't true after S3 resume. It might
be necessary to save & restore the whole PCI config space of the device.

-- 
Vojtech Pavlik
Director SuSE Labs

[patch, rft] amd74xx: implement suspend-to-ram

[Jason Lunz]

The amd74xx driver needs to reprogram each drive's PIO timings as well
as the DMA timings on resume from s2ram.  Otherwise, my
nforce3-150-based laptop hangs hard when ide_start_power_step() calls
drive->hwif->ide_dma_check(drive).

Suspend/resume from ram now works with the disk and the cdrom under
load, both with and without DMA enabled.

Signed-off-by: Jason Lunz <lunz@falooley.org>
---

I'm hardcoding a maximum of 2 ide channels, but other aspects of this
driver (like the amd_80w global) are already doing that.

DMA is re-enabled on resume even if it wasn't on at suspend, but that
doesn't look unusual in drivers/ide/pci.

 drivers/ide/pci/amd74xx.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

Index: linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
===================================================================
--- linux-2.6.18-rc2-git1.orig/drivers/ide/pci/amd74xx.c
+++ linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
@@ -83,6 +83,8 @@
 static ide_pci_device_t *amd_chipset;
 static unsigned int amd_80w;
 static unsigned int amd_clock;
+#define AMD_MAX_CHANNELS	(2)
+static ide_hwif_t *amd_hwifs[AMD_MAX_CHANNELS];
 
 static char *amd_dma[] = { "MWDMA16", "UDMA33", "UDMA66", "UDMA100", "UDMA133" };
 static unsigned char amd_cyc2udma[] = { 6, 6, 5, 4, 0, 1, 1, 2, 2, 3, 3, 3, 3, 3, 3, 7 };
@@ -416,6 +418,8 @@
 {
 	int i;
 
+	amd_hwifs[hwif->channel] = hwif;
+
 	if (hwif->irq == 0) /* 0 is bogus but will do for now */
 		hwif->irq = pci_get_legacy_ide_irq(hwif->pci_dev, hwif->channel);
 
@@ -494,6 +498,53 @@
 	/* 19 */ DECLARE_AMD_DEV("AMD5536"),
 };
 
+static int amd74xx_suspend(struct pci_dev *dev, pm_message_t state)
+{
+	pci_save_state(dev);
+
+	// PM_EVENT_SUSPEND means s2ram. otherwise, the disk behind this device
+	// might hold the s2disk image, and we can't disable the disk
+	// controller until we've used it to write that out.
+	if(state.event == PM_EVENT_SUSPEND) {
+		pci_disable_device(dev);
+		pci_set_power_state(dev, pci_choose_state(dev, state));
+	}
+	return 0;
+}
+
+static int amd74xx_resume(struct pci_dev *dev)
+{
+	int retval = 0;
+	int i;
+
+	pci_set_power_state(dev, PCI_D0);
+	retval = pci_enable_device(dev);
+	pci_restore_state(dev);
+
+	for (i = 0; i < AMD_MAX_CHANNELS; i++) {
+		int d;
+
+		if (!amd_hwifs[i])
+			continue;
+
+		for (d = 0; d < MAX_DRIVES; ++d) {
+			ide_drive_t *drive = &amd_hwifs[i]->drives[d];
+			if (drive->present && !__ide_dma_bad_drive(drive)) {
+				/* this is the primary reason this driver needs
+				 * a suspend()/resume() implementation at all.
+				 * Calling amd74xx_ide_dma_check() without also
+				 * calling amd74xx_tune_drive() hangs my
+				 * nforce3-150 system.  ide-disk.c will do just
+				 * that later if we're resuming from s2ram. */
+				amd_hwifs[i]->tuneproc(drive, 255);
+				amd_hwifs[i]->ide_dma_check(drive);
+			}
+		}
+	}
+
+	return retval;
+}
+
 static int __devinit amd74xx_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	amd_chipset = amd74xx_chipsets + id->driver_data;
@@ -539,6 +590,8 @@
 	.name		= "AMD_IDE",
 	.id_table	= amd74xx_pci_tbl,
 	.probe		= amd74xx_probe,
+	.suspend	= amd74xx_suspend,
+	.resume		= amd74xx_resume,
 };
 
 static int amd74xx_ide_init(void)

Re: [patch, rft] amd74xx: implement suspend-to-ram

[Pavel Machek]

Hi!

I do not have the affected hardware, sorry.

> Index: linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
> ===================================================================
> --- linux-2.6.18-rc2-git1.orig/drivers/ide/pci/amd74xx.c
> +++ linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
> @@ -494,6 +498,53 @@
>  	/* 19 */ DECLARE_AMD_DEV("AMD5536"),
>  };
>  
> +static int amd74xx_suspend(struct pci_dev *dev, pm_message_t state)
> +{
> +	pci_save_state(dev);
> +
> +	// PM_EVENT_SUSPEND means s2ram. otherwise, the disk behind this device
> +	// might hold the s2disk image, and we can't disable the disk
> +	// controller until we've used it to write that out.

C-style comments, please.

> +	if(state.event == PM_EVENT_SUSPEND) {

"if (". Ouch and please don't do this. Suspend-to-disk will call
amd74xx_resume when it needs to talk to the disk.

> +		pci_disable_device(dev);
> +		pci_set_power_state(dev, pci_choose_state(dev, state));
> +	}
> +	return 0;
> +}


								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[David Brownell]

On Tuesday 25 July 2006 4:09 pm, Pavel Machek wrote:

> 
> > +	if(state.event == PM_EVENT_SUSPEND) {
> 
> "if (". Ouch 

He already got that feedback, along with the fact that PM_EVENT_SUSPEND
gets called in _all_ suspend sequences not just "standby" and STR.  That
comment just needs to be removed.  :)


> and please don't do this. Suspend-to-disk will call 
> amd74xx_resume when it needs to talk to the disk.

I don't understand the rest of this comment.  That code looks
correct (other than calling the goofy/broken pci_choose_state thing);
PM_EVENT_FREEZE and PM_EVENT_PRETHAW should not disable the device.

So what "this" are you suggesting he not do ... and why?  


> > +		pci_disable_device(dev);
> > +		pci_set_power_state(dev, pci_choose_state(dev, state));
> > +	}
> > +	return 0;
> > +}
> 
> 

[patch v2] amd74xx: fix hang on resume from ram

[Jason Lunz]

The amd74xx driver needs to reprogram each drive's PIO timings as well
as the DMA timings on resume from s2ram.  Otherwise, my
nforce3-150-based laptop hangs hard when ide_start_power_step() calls
drive->hwif->ide_dma_check(drive).

Suspend/resume from ram now works with the disk and the cdrom under
load, both with and without DMA enabled.

Signed-off-by: Jason Lunz <lunz@falooley.org>
---

I've incorporated improvements helpfully suggested by David Brownell and
Pavel Machek.

I'm hardcoding a maximum of 2 ide channels, but other aspects of this
driver (like the amd_80w global) are already doing that.

DMA is re-enabled on resume even if it wasn't on at suspend, but that
doesn't look unusual in drivers/ide/pci.

 drivers/ide/pci/amd74xx.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

Index: linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
===================================================================
--- linux-2.6.18-rc2-git1.orig/drivers/ide/pci/amd74xx.c
+++ linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
@@ -83,6 +83,8 @@
 static ide_pci_device_t *amd_chipset;
 static unsigned int amd_80w;
 static unsigned int amd_clock;
+#define AMD_MAX_CHANNELS	(2)
+static ide_hwif_t *amd_hwifs[AMD_MAX_CHANNELS];
 
 static char *amd_dma[] = { "MWDMA16", "UDMA33", "UDMA66", "UDMA100", "UDMA133" };
 static unsigned char amd_cyc2udma[] = { 6, 6, 5, 4, 0, 1, 1, 2, 2, 3, 3, 3, 3, 3, 3, 7 };
@@ -416,6 +418,8 @@
 {
 	int i;
 
+	amd_hwifs[hwif->channel] = hwif;
+
 	if (hwif->irq == 0) /* 0 is bogus but will do for now */
 		hwif->irq = pci_get_legacy_ide_irq(hwif->pci_dev, hwif->channel);
 
@@ -494,6 +498,58 @@
 	/* 19 */ DECLARE_AMD_DEV("AMD5536"),
 };
 
+#ifdef	CONFIG_PM
+
+static int amd74xx_suspend(struct pci_dev *dev, pm_message_t state)
+{
+	pci_save_state(dev);
+
+	if (state.event == PM_EVENT_SUSPEND) {
+		pci_disable_device(dev);
+		pci_set_power_state(dev, PCI_D3hot);
+	}
+	return 0;
+}
+
+static int amd74xx_resume(struct pci_dev *dev)
+{
+	int retval = 0;
+	int i;
+
+	pci_set_power_state(dev, PCI_D0);
+	retval = pci_enable_device(dev);
+	pci_restore_state(dev);
+
+	for (i = 0; i < AMD_MAX_CHANNELS; i++) {
+		int d;
+
+		if (!amd_hwifs[i])
+			continue;
+
+		for (d = 0; d < MAX_DRIVES; ++d) {
+			ide_drive_t *drive = &amd_hwifs[i]->drives[d];
+			if (drive->present && !__ide_dma_bad_drive(drive)) {
+				/* this is the primary reason this driver needs
+				 * a suspend()/resume() implementation at all.
+				 * Calling amd74xx_ide_dma_check() without also
+				 * calling amd74xx_tune_drive() hangs my
+				 * nforce3-150 system.  ide-io.c will do just
+				 * that later if we're resuming from s2ram.
+				 */
+				amd_hwifs[i]->tuneproc(drive, 255);
+				amd_hwifs[i]->ide_dma_check(drive);
+			}
+		}
+	}
+
+	return retval;
+}
+
+#else	/* !CONFIG_PM */
+#define amd74xx_suspend
+#define amd74xx_resume
+#endif	/* !CONFIG_PM */
+
 static int __devinit amd74xx_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	amd_chipset = amd74xx_chipsets + id->driver_data;
@@ -539,6 +595,8 @@
 	.name		= "AMD_IDE",
 	.id_table	= amd74xx_pci_tbl,
 	.probe		= amd74xx_probe,
+	.suspend	= amd74xx_suspend,
+	.resume		= amd74xx_resume,
 };
 
 static int amd74xx_ide_init(void)

[patch v3] amd74xx: fix hang on resume from ram

[Jason Lunz]

The amd74xx driver needs to reprogram each drive's PIO timings as well
as the DMA timings on resume from s2ram.  Otherwise, my
nforce3-150-based laptop hangs hard when ide_start_power_step() calls
drive->hwif->ide_dma_check(drive).

Suspend/resume from ram now works with the disk and the cdrom under
load, both with and without DMA enabled.

Signed-off-by: Jason Lunz <lunz@falooley.org>
---

I'm hardcoding a maximum of 2 ide channels, but other aspects of this
driver (like the amd_80w global) are already doing that.

DMA is re-enabled on resume even if it wasn't on at suspend, but that
doesn't look unusual in drivers/ide/pci.

I've incorporated improvements helpfully suggested by David Brownell and
Pavel Machek.

Now compiles with !CONFIG_PM.

 drivers/ide/pci/amd74xx.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Index: linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
===================================================================
--- linux-2.6.18-rc2-git1.orig/drivers/ide/pci/amd74xx.c
+++ linux-2.6.18-rc2-git1/drivers/ide/pci/amd74xx.c
@@ -83,6 +83,8 @@
 static ide_pci_device_t *amd_chipset;
 static unsigned int amd_80w;
 static unsigned int amd_clock;
+#define AMD_MAX_CHANNELS	(2)
+static ide_hwif_t *amd_hwifs[AMD_MAX_CHANNELS];
 
 static char *amd_dma[] = { "MWDMA16", "UDMA33", "UDMA66", "UDMA100", "UDMA133" };
 static unsigned char amd_cyc2udma[] = { 6, 6, 5, 4, 0, 1, 1, 2, 2, 3, 3, 3, 3, 3, 3, 7 };
@@ -416,6 +418,8 @@
 {
 	int i;
 
+	amd_hwifs[hwif->channel] = hwif;
+
 	if (hwif->irq == 0) /* 0 is bogus but will do for now */
 		hwif->irq = pci_get_legacy_ide_irq(hwif->pci_dev, hwif->channel);
 
@@ -494,6 +498,55 @@
 	/* 19 */ DECLARE_AMD_DEV("AMD5536"),
 };
 
+#ifdef	CONFIG_PM
+
+static int amd74xx_suspend(struct pci_dev *dev, pm_message_t state)
+{
+	pci_save_state(dev);
+
+	if (state.event == PM_EVENT_SUSPEND) {
+		pci_disable_device(dev);
+		pci_set_power_state(dev, PCI_D3hot);
+	}
+	return 0;
+}
+
+static int amd74xx_resume(struct pci_dev *dev)
+{
+	int retval = 0;
+	int i;
+
+	pci_set_power_state(dev, PCI_D0);
+	retval = pci_enable_device(dev);
+	pci_restore_state(dev);
+
+	for (i = 0; i < AMD_MAX_CHANNELS; i++) {
+		int d;
+
+		if (!amd_hwifs[i])
+			continue;
+
+		for (d = 0; d < MAX_DRIVES; ++d) {
+			ide_drive_t *drive = &amd_hwifs[i]->drives[d];
+			if (drive->present && !__ide_dma_bad_drive(drive)) {
+				/* this is the primary reason this driver needs
+				 * a suspend()/resume() implementation at all.
+				 * Calling amd74xx_ide_dma_check() without also
+				 * calling amd74xx_tune_drive() hangs my
+				 * nforce3-150 system.  ide-io.c will do just
+				 * that later if we're resuming from s2ram.
+				 */
+				amd_hwifs[i]->tuneproc(drive, 255);
+				amd_hwifs[i]->ide_dma_check(drive);
+			}
+		}
+	}
+
+	return retval;
+}
+
+#endif	/* !CONFIG_PM */
+
 static int __devinit amd74xx_probe(struct pci_dev *dev, const struct pci_device_id *id)
 {
 	amd_chipset = amd74xx_chipsets + id->driver_data;
@@ -539,6 +592,10 @@
 	.name		= "AMD_IDE",
 	.id_table	= amd74xx_pci_tbl,
 	.probe		= amd74xx_probe,
+#ifdef CONFIG_PM
+	.suspend	= amd74xx_suspend,
+	.resume		= amd74xx_resume,
+#endif
 };
 
 static int amd74xx_ide_init(void)

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[Pavel Machek]

Hi!

> > and please don't do this. Suspend-to-disk will call 
> > amd74xx_resume when it needs to talk to the disk.
> 
> I don't understand the rest of this comment.  That code looks
> correct (other than calling the goofy/broken pci_choose_state thing);
> PM_EVENT_FREEZE and PM_EVENT_PRETHAW should not disable the device.

Well, I'd prefer PM_EVENT_FREEZE and PM_EVENT_SUSPEND to be the same
code (i.e. remove the if() and just do 

> > > +		pci_disable_device(dev);
> > > +		pci_set_power_state(dev, pci_choose_state(dev, state));

unconditionaly). It should also work, and not really be slower.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[David Brownell]

On Wednesday 26 July 2006 2:02 am, Pavel Machek wrote:
> Hi!
> 
> > > and please don't do this. Suspend-to-disk will call 
> > > amd74xx_resume when it needs to talk to the disk.
> > 
> > I don't understand the rest of this comment.  That code looks
> > correct (other than calling the goofy/broken pci_choose_state thing);
> > PM_EVENT_FREEZE and PM_EVENT_PRETHAW should not disable the device.
> 
> Well, I'd prefer PM_EVENT_FREEZE and PM_EVENT_SUSPEND to be the same
> code (i.e. remove the if() and just do 
> 
> > > > +		pci_disable_device(dev);
> > > > +		pci_set_power_state(dev, pci_choose_state(dev, state));
> 
> unconditionaly). It should also work, and not really be slower.

Oh, I see.  But wouldn't that cause FREEZE to spin down the drives?

Re: [patch v3] amd74xx: fix hang on resume from ram

[David Brownell]

Your v3 patch worked for me on an NF3 system, modulo video issues
I've yet to track down.  It looks like that was otherwise the "last"
bug blocking S3 from working on that box.

I can't say the same for an NF2 system, unfortunately.  :(

- Dave

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[Pavel Machek]

On Wed 2006-07-26 17:29:33, David Brownell wrote:
> On Wednesday 26 July 2006 2:02 am, Pavel Machek wrote:
> > Hi!
> > 
> > > > and please don't do this. Suspend-to-disk will call 
> > > > amd74xx_resume when it needs to talk to the disk.
> > > 
> > > I don't understand the rest of this comment.  That code looks
> > > correct (other than calling the goofy/broken pci_choose_state thing);
> > > PM_EVENT_FREEZE and PM_EVENT_PRETHAW should not disable the device.
> > 
> > Well, I'd prefer PM_EVENT_FREEZE and PM_EVENT_SUSPEND to be the same
> > code (i.e. remove the if() and just do 
> > 
> > > > > +		pci_disable_device(dev);
> > > > > +		pci_set_power_state(dev, pci_choose_state(dev, state));
> > 
> > unconditionaly). It should also work, and not really be slower.
> 
> Oh, I see.  But wouldn't that cause FREEZE to spin down the drives?

I do not think so. If it will, I'm obviously wrong and original
version is okay.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[Jason Lunz]

On Thu, Jul 27, 2006 at 10:49:39PM +0200, Pavel Machek wrote:
> > Oh, I see.  But wouldn't that cause FREEZE to spin down the drives?
> 
> I do not think so. If it will, I'm obviously wrong and original
> version is okay.

I think it's the ide bus shutdown methods that do power management on
the drives. But here we're talking about the pci ide controller.

At any rate, not having the if() means my machine freezes during the
suspend phase of STD.

Jason

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[Pavel Machek]

On Thu 2006-07-27 18:43:12, Jason Lunz wrote:
> On Thu, Jul 27, 2006 at 10:49:39PM +0200, Pavel Machek wrote:
> > > Oh, I see.  But wouldn't that cause FREEZE to spin down the drives?
> > 
> > I do not think so. If it will, I'm obviously wrong and original
> > version is okay.
> 
> I think it's the ide bus shutdown methods that do power management on
> the drives. But here we're talking about the pci ide controller.
> 
> At any rate, not having the if() means my machine freezes during the
> suspend phase of STD.

I'd still like to understand why it freezes (it should not), but I
guess you have my ACK on that patch.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[Rafael J. Wysocki]

On Friday 28 July 2006 00:51, Pavel Machek wrote:
> On Thu 2006-07-27 18:43:12, Jason Lunz wrote:
> > On Thu, Jul 27, 2006 at 10:49:39PM +0200, Pavel Machek wrote:
> > > > Oh, I see.  But wouldn't that cause FREEZE to spin down the drives?
> > > 
> > > I do not think so. If it will, I'm obviously wrong and original
> > > version is okay.
> > 
> > I think it's the ide bus shutdown methods that do power management on
> > the drives. But here we're talking about the pci ide controller.
> > 
> > At any rate, not having the if() means my machine freezes during the
> > suspend phase of STD.
> 
> I'd still like to understand why it freezes (it should not), but I
> guess you have my ACK on that patch.

So I guess it needs to be forwarded to Andrew.  Should I?

It helps to resume my box from RAM, btw.  Now only the graphics doesn't
wake up.

Rafael

Re: [linux-pm] [patch, rft] amd74xx: implement suspend-to-ram

[Vojtech Pavlik]

On Fri, Jul 28, 2006 at 11:15:01AM +0200, Rafael J. Wysocki wrote:
> On Friday 28 July 2006 00:51, Pavel Machek wrote:
> > On Thu 2006-07-27 18:43:12, Jason Lunz wrote:
> > > On Thu, Jul 27, 2006 at 10:49:39PM +0200, Pavel Machek wrote:
> > > > > Oh, I see.  But wouldn't that cause FREEZE to spin down the drives?
> > > > 
> > > > I do not think so. If it will, I'm obviously wrong and original
> > > > version is okay.
> > > 
> > > I think it's the ide bus shutdown methods that do power management on
> > > the drives. But here we're talking about the pci ide controller.
> > > 
> > > At any rate, not having the if() means my machine freezes during the
> > > suspend phase of STD.
> > 
> > I'd still like to understand why it freezes (it should not), but I
> > guess you have my ACK on that patch.
> 
> So I guess it needs to be forwarded to Andrew.  Should I?

Yes, please do.

> 
> It helps to resume my box from RAM, btw.  Now only the graphics doesn't
> wake up.
> 
> Rafael
> 
> 

-- 
Vojtech Pavlik
Director SuSE Labs