[alsa-devel] Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2

Igor Chernyshev igor.ch75+alsa at gmail.com
Thu Jun 25 08:15:50 CEST 2009


Takashi,

below is the same patch based on the latest snapshot.

Thanks,
Igor

On Wed, Jun 24, 2009 at 3:13 AM, Takashi Iwai<tiwai at suse.de> wrote:
> At Tue, 23 Jun 2009 15:50:44 -0700,
> Igor Chernyshev wrote:
>>
>> >> +     unsigned long pm_saved_route;
>> >
>> > This shouldn't be long but int.  Long can be 64bit unnecessarily.
>>
>> Done. However, note that existing code uses results of
>> "inl(ICEMT1724(ice, ROUTE_PLAYBACK))" as both int and long.
>>
>> >> +     case SNDRV_PCM_TRIGGER_SUSPEND:
>> >> +     case SNDRV_PCM_TRIGGER_RESUME:
>> >> +             break;
>> >
>> > At least, TRIGGER_SUSPEND neees to stop the PCM stream.
>>
>> Consolidated with SNDRV_PCM_TRIGGER_STOP processing.
>>
>> >> +     outb(VT1724_MULTI_FIFO_ERR, ICEMT1724(ice, DMA_INT_MASK));
>> >> +
>> >
>> > This is fine, but you need to remove __devinit from this function.
>>
>> Done.
>>
>> >> +     if (ice->ac97)
>> >> +             snd_ac97_suspend(ice->ac97);
>> >
>> > No need for NULL check here.  All these check NULL by themselves.
>>
>> Removed all 4 checks (pcm's and ac97).
>>
>> >> +             spin_unlock_irq(&ice->reg_lock);
>> >> +             snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 1);
>> >> +             spin_lock_irq(&ice->reg_lock);
>> >> +     }
>> >
>> > These are basically no need to protect with spinlock, at least,
>> > in this function...
>>
>> Removed locking in "resume" only.
>>
>> > Last but not least, try to run $LINUX/scripts/checkpatch.pl to your
>> > patch once and fix the issues suggested there.
>>
>> Thanks for the pointer. Fixed 3 code style problems. It also complains
>> about missing "signed off" line, but I hope that line here will
>> suffice:
>>
>> Signed-off-by: Igor Chernyshev <igor.ch75+alsa at gmail.com>
>
> Thanks, this looks better.  But, I got some rejects when applying
> to the latest tree, and I guess your patch is based on 1.0.20 release.
>
> Please try to rebase to the latest alsa-driver snapshot and repost
> the patch, either against sound git tree or alsa-driver-snapshot
> tarball below:
>  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git
>  ftp://ftp.kernel.org/pub/linux/kernel/people/tiwai/snapshot/alsa-driver-snapshot.tar.gz
>
>
> thanks,
>
> Takashi
>

diff -uN alsa-driver.orig/alsa-kernel/pci/ice1712/ice1712.h
alsa-driver/alsa-kernel/pci/ice1712/ice1712.h
--- alsa-driver.orig/alsa-kernel/pci/ice1712/ice1712.h	2009-06-24
15:05:06.000000000 -0700
+++ alsa-driver/alsa-kernel/pci/ice1712/ice1712.h	2009-06-24
21:51:53.000000000 -0700
@@ -379,6 +379,15 @@
 	unsigned char (*set_mclk)(struct snd_ice1712 *ice, unsigned int rate);
 	void (*set_spdif_clock)(struct snd_ice1712 *ice);

+#ifdef CONFIG_PM
+	int (*pm_suspend)(struct snd_ice1712 *);
+	int (*pm_resume)(struct snd_ice1712 *);
+	int pm_suspend_enabled:1;
+	int pm_saved_is_spdif_master:1;
+	unsigned int pm_saved_spdif_ctrl;
+	unsigned char pm_saved_spdif_cfg;
+	unsigned int pm_saved_route;
+#endif
 };


diff -uN alsa-driver.orig/alsa-kernel/pci/ice1712/ice1724.c
alsa-driver/alsa-kernel/pci/ice1712/ice1724.c
--- alsa-driver.orig/alsa-kernel/pci/ice1712/ice1724.c	2009-06-24
15:05:06.000000000 -0700
+++ alsa-driver/alsa-kernel/pci/ice1712/ice1724.c	2009-06-24
22:13:25.000000000 -0700
@@ -560,6 +560,7 @@

 	case SNDRV_PCM_TRIGGER_START:
 	case SNDRV_PCM_TRIGGER_STOP:
+	case SNDRV_PCM_TRIGGER_SUSPEND:
 		spin_lock(&ice->reg_lock);
 		old = inb(ICEMT1724(ice, DMA_CONTROL));
 		if (cmd == SNDRV_PCM_TRIGGER_START)
@@ -570,6 +571,10 @@
 		spin_unlock(&ice->reg_lock);
 		break;

+	case SNDRV_PCM_TRIGGER_RESUME:
+		/* apps will have to restart stream */
+		break;
+
 	default:
 		return -EINVAL;
 	}
@@ -2272,7 +2277,7 @@
 	msleep(10);
 }

-static int __devinit snd_vt1724_chip_init(struct snd_ice1712 *ice)
+static int snd_vt1724_chip_init(struct snd_ice1712 *ice)
 {
 	outb(ice->eeprom.data[ICE_EEP2_SYSCONF], ICEREG1724(ice, SYS_CFG));
 	outb(ice->eeprom.data[ICE_EEP2_ACLINK], ICEREG1724(ice, AC97_CFG));
@@ -2287,6 +2292,14 @@

 	outb(0, ICEREG1724(ice, POWERDOWN));

+	/* MPU_RX and TX irq masks are cleared later dynamically */
+	outb(VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX , ICEREG1724(ice, IRQMASK));
+
+	/* don't handle FIFO overrun/underruns (just yet),
+	 * since they cause machine lockups
+	 */
+	outb(VT1724_MULTI_FIFO_ERR, ICEMT1724(ice, DMA_INT_MASK));
+
 	return 0;
 }

@@ -2431,6 +2444,8 @@
 	snd_vt1724_proc_init(ice);
 	synchronize_irq(pci->irq);

+	card->private_data = ice;
+
 	err = pci_request_regions(pci, "ICE1724");
 	if (err < 0) {
 		kfree(ice);
@@ -2459,14 +2474,6 @@
 		return -EIO;
 	}

-	/* MPU_RX and TX irq masks are cleared later dynamically */
-	outb(VT1724_IRQ_MPU_RX | VT1724_IRQ_MPU_TX , ICEREG1724(ice, IRQMASK));
-
-	/* don't handle FIFO overrun/underruns (just yet),
-	 * since they cause machine lockups
-	 */
-	outb(VT1724_MULTI_FIFO_ERR, ICEMT1724(ice, DMA_INT_MASK));
-
 	err = snd_device_new(card, SNDRV_DEV_LOWLEVEL, ice, &ops);
 	if (err < 0) {
 		snd_vt1724_free(ice);
@@ -2650,11 +2657,96 @@
 	pci_set_drvdata(pci, NULL);
 }

+#ifdef CONFIG_PM
+static int snd_vt1724_suspend(struct pci_dev *pci, pm_message_t state)
+{
+	struct snd_card *card = pci_get_drvdata(pci);
+	struct snd_ice1712 *ice = card->private_data;
+
+	if (!ice->pm_suspend_enabled)
+		return 0;
+
+	snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
+
+	snd_pcm_suspend_all(ice->pcm);
+	snd_pcm_suspend_all(ice->pcm_pro);
+	snd_pcm_suspend_all(ice->pcm_ds);
+	snd_ac97_suspend(ice->ac97);
+
+	spin_lock_irq(&ice->reg_lock);
+	ice->pm_saved_is_spdif_master = ice->is_spdif_master(ice);
+	ice->pm_saved_spdif_ctrl = inw(ICEMT1724(ice, SPDIF_CTRL));
+	ice->pm_saved_spdif_cfg = inb(ICEREG1724(ice, SPDIF_CFG));
+	ice->pm_saved_route = inl(ICEMT1724(ice, ROUTE_PLAYBACK));
+	spin_unlock_irq(&ice->reg_lock);
+
+	if (ice->pm_suspend)
+		ice->pm_suspend(ice);
+
+	pci_disable_device(pci);
+	pci_save_state(pci);
+	pci_set_power_state(pci, pci_choose_state(pci, state));
+	return 0;
+}
+
+static int snd_vt1724_resume(struct pci_dev *pci)
+{
+	struct snd_card *card = pci_get_drvdata(pci);
+	struct snd_ice1712 *ice = card->private_data;
+
+	if (!ice->pm_suspend_enabled)
+		return 0;
+
+	pci_set_power_state(pci, PCI_D0);
+	pci_restore_state(pci);
+
+	if (pci_enable_device(pci) < 0) {
+		snd_card_disconnect(card);
+		return -EIO;
+	}
+
+	pci_set_master(pci);
+
+	snd_vt1724_chip_reset(ice);
+
+	if (snd_vt1724_chip_init(ice) < 0) {
+		snd_card_disconnect(card);
+		return -EIO;
+	}
+
+	if (ice->pm_resume)
+		ice->pm_resume(ice);
+
+	if (ice->pm_saved_is_spdif_master) {
+		/* switching to external clock via SPDIF */
+		ice->set_spdif_clock(ice);
+	} else {
+		/* internal on-card clock */
+		snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 1);
+	}
+
+	update_spdif_bits(ice, ice->pm_saved_spdif_ctrl);
+
+	outb(ice->pm_saved_spdif_cfg, ICEREG1724(ice, SPDIF_CFG));
+	outl(ice->pm_saved_route, ICEMT1724(ice, ROUTE_PLAYBACK));
+
+	if (ice->ac97)
+		snd_ac97_resume(ice->ac97);
+
+	snd_power_change_state(card, SNDRV_CTL_POWER_D0);
+	return 0;
+}
+#endif
+
 static struct pci_driver driver = {
 	.name = "ICE1724",
 	.id_table = snd_vt1724_ids,
 	.probe = snd_vt1724_probe,
 	.remove = __devexit_p(snd_vt1724_remove),
+#ifdef CONFIG_PM
+	.suspend = snd_vt1724_suspend,
+	.resume = snd_vt1724_resume,
+#endif
 };

 static int __init alsa_card_ice1724_init(void)
diff -uN alsa-driver.orig/alsa-kernel/pci/ice1712/prodigy_hifi.c
alsa-driver/alsa-kernel/pci/ice1712/prodigy_hifi.c
--- alsa-driver.orig/alsa-kernel/pci/ice1712/prodigy_hifi.c	2009-06-24
15:05:06.000000000 -0700
+++ alsa-driver/alsa-kernel/pci/ice1712/prodigy_hifi.c	2009-06-24
22:00:04.000000000 -0700
@@ -1077,7 +1077,7 @@
 /*
  * initialize the chip
  */
-static int __devinit prodigy_hd2_init(struct snd_ice1712 *ice)
+static void ak4396_init(struct snd_ice1712 *ice)
 {
 	static unsigned short ak4396_inits[] = {
 		AK4396_CTRL1,	   0x87,   /* I2S Normal Mode, 24 bit */
@@ -1087,9 +1087,37 @@
 		AK4396_RCH_ATT,	 0x00,
 	};

-	struct prodigy_hifi_spec *spec;
 	unsigned int i;

+	/* initialize ak4396 codec */
+	/* reset codec */
+	ak4396_write(ice, AK4396_CTRL1, 0x86);
+	msleep(100);
+	ak4396_write(ice, AK4396_CTRL1, 0x87);
+
+	for (i = 0; i < ARRAY_SIZE(ak4396_inits); i += 2)
+		ak4396_write(ice, ak4396_inits[i], ak4396_inits[i+1]);
+}
+
+#ifdef CONFIG_PM
+static int __devinit prodigy_hd2_resume(struct snd_ice1712 *ice)
+{
+	/* initialize ak4396 codec and restore previous mixer volumes */
+	struct prodigy_hifi_spec *spec = ice->spec;
+	int i;
+	mutex_lock(&ice->gpio_mutex);
+	ak4396_init(ice);
+	for (i = 0; i < 2; i++)
+		ak4396_write(ice, AK4396_LCH_ATT + i, spec->vol[i] & 0xff);
+	mutex_unlock(&ice->gpio_mutex);
+	return 0;
+}
+#endif
+
+static int __devinit prodigy_hd2_init(struct snd_ice1712 *ice)
+{
+	struct prodigy_hifi_spec *spec;
+
 	ice->vt1720 = 0;
 	ice->vt1724 = 1;

@@ -1112,14 +1140,12 @@
 		return -ENOMEM;
 	ice->spec = spec;

-	/* initialize ak4396 codec */
-	/* reset codec */
-	ak4396_write(ice, AK4396_CTRL1, 0x86);
-	msleep(100);
-	ak4396_write(ice, AK4396_CTRL1, 0x87);
-			
-	for (i = 0; i < ARRAY_SIZE(ak4396_inits); i += 2)
-		ak4396_write(ice, ak4396_inits[i], ak4396_inits[i+1]);
+#ifdef CONFIG_PM
+	ice->pm_resume = &prodigy_hd2_resume;
+	ice->pm_suspend_enabled = 1;
+#endif
+
+	ak4396_init(ice);

 	return 0;
 }


More information about the Alsa-devel mailing list