amd74xx: implement suspend-to-ram

From: Jason Lunz <[email protected]>

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.

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 |   57 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Index: 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)

Ar Gwe, 2006-07-28 am 16:46 +0200, ysgrifennodd Rafael J. Wysocki:

NAK

This beings in the IDE power step code. You should do that as a step before the win_idleimmediate I suspect. Theory is right, diagnosis is right, implementation is in the wrong place.

You'll make a lot more people happy by fixing it in ide-io

Alan

OK, I'll see about moving it there. Will this still be controller-specific, or are you suggesting this is something ide ought to do globally?

I poked around in ide-io.c a little while writing the patch, but my assumption so far has been that the core ide suspend is OK wrt s2ram, since I never hear IDE cited as the reason for s2ram failure. Usually it's ACPI or video problems.

I'm beginning to suspect that the real problem is that everyone who experiences ide-related hangs when attempting s2ram switches to s2disk, which sidesteps ide problems since the ide controller stays alive throughout the process and gets reinitialized on resume. So I guess ide/s2ram problems could be more common than is widely known.

Jason

Actually I had exactly your issue on an ICH6 and ended up working around it by moving to Alan's latest libata code.

0000:00:1f.1 IDE interface: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) IDE Controller (rev 03)

If you were to patch up IDE I'd be happy to run up a couple of test kernels and test it out on an Intel chipset also.

Brad

"Human beings, who are almost unique in having the ability
to learn from the experience of others, are also remarkable
for their apparent disinclination to do so." -- Douglas Adams

Ar Gwe, 2006-07-28 am 13:13 -0400, ysgrifennodd Jason Lunz:

It should be done globally. In many cases the chips start up from power on configured for PIO 0 so that side happens to work, but not all chips do this as you've found out. Setting the PIO side correctly is a fix even if its not a bug people hit a lot.

Alan

It's actually incorrect to just "restore" the previous timings. In many case, the disk will have been reset too. The safe thing is to force the controller in PIO0 mode at resume, and the best place to do that is from the controller's own resume(), as I do on pmac. It's the parent of the hwif and thus will be resumed before the later.

Later on, the state machine will redo the tuning to get to a better speed, including sending the right commands to also reconfigure the drive.

Ben.

Add a step to the IDE PM state machine that reprograms disk PIO timings as the first step on resume. This prevents ide deadlock on resume-from-ram on my nforce3-based laptop.

An earlier implementation was written entirely within the amd74xx ide driver, but Alan helpfully pointed out that this is the correct thing to do globally. Still, I'm only calling hwif->tuneproc() for disks, based on two things:

- The existing state machine is already passed over for non-disk drives
- Previous testing on my laptop shows that the hangs are related only to the disk - suspend/resume from a livecd showed that there's no need for this on the cdrom.

---

Alan: I'm doing tuneproc() before WIN_IDLEIMMEDIATE as you said. It works. I'd otherwise have done it after, based solely on the name - thanks for the suggestion.

David, Rafael: It would be helpful if you could verify that this patch cures your ide-relate hangs on s2ram as well (with my previous patch to amd74xx unapplied).

Brad: thanks for volunteering to test! You'll be the lucky first person to try this on a non-nforce ide chipset. good luck.

Please be careful - I know next to nothing about IDE; I just know that this make s2ram work on my laptop. This patch means altering ide resume for all systems where it currently works already. I suppose there could be regressions.

 drivers/ide/ide-io.c |   25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

Index: linux-2.6.18-rc2-git6/drivers/ide/ide-io.c
===================================================================
@@ -135,7 +135,8 @@
 	ide_pm_flush_cache	= ide_pm_state_start_suspend,
 	idedisk_pm_standby,
 
-	idedisk_pm_idle		= ide_pm_state_start_resume,
+	idedisk_pm_restore_pio	= ide_pm_state_start_resume,
+	idedisk_pm_idle,
 	ide_pm_restore_dma,
 };
 

@@ -156,7 +157,10 @@
 	case idedisk_pm_standby:	/* Suspend step 2 (standby) complete */
 		pm->pm_step = ide_pm_state_completed;
 		break;
-	case idedisk_pm_idle:		/* Resume step 1 (idle) complete */
+	case idedisk_pm_restore_pio:	/* Resume step 1 complete */
+		pm->pm_step = idedisk_pm_idle;
+		break;
+	case idedisk_pm_idle:		/* Resume step 2 (idle) complete */
 		pm->pm_step = ide_pm_restore_dma;
 		break;
 	}

@@ -170,8 +174,11 @@
 	memset(args, 0, sizeof(*args));
 
 	if (drive->media != ide_disk) {
-		/* skip idedisk_pm_idle for ATAPI devices */
-		if (pm->pm_step == idedisk_pm_idle)
+		/*
+		 * skip idedisk_pm_restore_pio and idedisk_pm_idle for ATAPI
+		 * devices
+		 */
+		if (pm->pm_step == idedisk_pm_restore_pio)
 			pm->pm_step = ide_pm_restore_dma;
 	}
 

@@ -198,13 +205,19 @@
 		args->handler	   = &task_no_data_intr;
 		return do_rw_taskfile(drive, args);
 
-	case idedisk_pm_idle:		/* Resume step 1 (idle) */
+	case idedisk_pm_restore_pio:	/* Resume step 1 (restore PIO) */
+		if (drive->hwif->tuneproc != NULL)
+			drive->hwif->tuneproc(drive, 255);
+		ide_complete_power_step(drive, rq, 0, 0);
+		return ide_stopped;
+
+	case idedisk_pm_idle:		/* Resume step 2 (idle) */
 		args->tfRegister[IDE_COMMAND_OFFSET] = WIN_IDLEIMMEDIATE;
 		args->command_type = IDE_DRIVE_TASK_NO_DATA;
 		args->handler = task_no_data_intr;
 		return do_rw_taskfile(drive, args);
 
-	case ide_pm_restore_dma:	/* Resume step 2 (restore DMA) */
+	case ide_pm_restore_dma:	/* Resume step 3 (restore DMA) */
 		/*
 		 * Right now, all we do is call hwif->ide_dma_check(drive),
 		 * we could be smarter and check for current xfer_speed

<snip>

No change to the behaviour here.. It does not hurt, but it does not help either.
I have not been able to get logs out of this machine yet while it's resuming. (no serial port and don't have another machine handy to test with netconsole)

Hard disk locks up for 30 secs and then resumes with a dropped interrupt message and I get a console back but all hard disk access result in timeouts. (I'm testing from a complete system initramfs)

As I said, with 2.6.18-rc2 and the libata patches from -mm1 it takes about 10 seconds to come back, but resumes reliably.

Brad

"Human beings, who are almost unique in having the ability
to learn from the experience of others, are also remarkable
for their apparent disinclination to do so." -- Douglas Adams

Ar Sad, 2006-07-29 am 19:34 -0400, ysgrifennodd Jason Lunz:

CDs should in theory also need the PIO restore.

Otherwise looks excellent