>> + 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>
Here is the updated patch file:
---------------------------------------------
diff -uN alsa-driver-1.0.20.orig/alsa-kernel/pci/ice1712/ice1712.h
alsa-driver-1.0.20/alsa-kernel/pci/ice1712/ice1712.h
--- alsa-driver-1.0.20.orig/alsa-kernel/pci/ice1712/ice1712.h 2009-05-06
00:06:04.000000000 -0700
+++ alsa-driver-1.0.20/alsa-kernel/pci/ice1712/ice1712.h 2009-06-23
14:20:56.000000000 -0700
@@ -378,6 +378,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-1.0.20.orig/alsa-kernel/pci/ice1712/ice1724.c
alsa-driver-1.0.20/alsa-kernel/pci/ice1712/ice1724.c
--- alsa-driver-1.0.20.orig/alsa-kernel/pci/ice1712/ice1724.c 2009-05-06
00:06:04.000000000 -0700
+++ alsa-driver-1.0.20/alsa-kernel/pci/ice1712/ice1724.c 2009-06-23
14:47:23.000000000 -0700
@@ -558,6 +558,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)
@@ -568,6 +569,10 @@
spin_unlock(&ice->reg_lock);
break;
+ case SNDRV_PCM_TRIGGER_RESUME:
+ /* apps will have to restart stream */
+ break;
+
default:
return -EINVAL;
}
@@ -2251,7 +2256,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));
@@ -2266,6 +2271,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;
}
@@ -2407,6 +2420,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);
@@ -2435,14 +2450,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);
@@ -2626,11 +2633,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-1.0.20.orig/alsa-kernel/pci/ice1712/prodigy_hifi.c
alsa-driver-1.0.20/alsa-kernel/pci/ice1712/prodigy_hifi.c
--- alsa-driver-1.0.20.orig/alsa-kernel/pci/ice1712/prodigy_hifi.c 2009-05-06
00:06:04.000000000 -0700
+++ alsa-driver-1.0.20/alsa-kernel/pci/ice1712/prodigy_hifi.c 2009-06-23
14:58:35.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;
}
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
