* Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 @ 2009-06-20 6:37 Igor Chernyshev 2009-06-22 16:42 ` Takashi Iwai 0 siblings, 1 reply; 10+ messages in thread From: Igor Chernyshev @ 2009-06-20 6:37 UTC (permalink / raw) To: alsa-devel Hi Alsa Developers! I've built a small HTPC and had to add suspend/resume support in ice1724 driver. There seem to be 3 existing bugs related to that: https://bugtrack.alsa-project.org/alsa-bug/view.php?id=4413 https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3748 https://bugtrack.alsa-project.org/alsa-bug/view.php?id=2314 Due to hardware (un)availability, I only enabled the fix for Audiotrak Prodigy HD2 card, which is installed in my HTPC. However, most of my code should be reusable in the future on other ice1724-based cards as well (as long as people add card-specific peices of code). The fix is currently based on ALSA 1.0.20 and works on my MythBuntu 9.04 HTPC (using 2.6.28-11 kernel). The following files have been modified: alsa-kernel/pci/ice1712/ice1712.h alsa-kernel/pci/ice1712/ice1724.c alsa-kernel/pci/ice1712/prodigy_hifi.c Now, I'm a complete newbie in Linux development and not sure I can safely rebuild latest Linux Kernel under MythBuntu, etc. Are there any resources for people like me to read? I want to provide the patch without destroying my computer a hunder times, but also make it more convenient and safer for ALSA maintainers. Thanks, Igor ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-20 6:37 Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 Igor Chernyshev @ 2009-06-22 16:42 ` Takashi Iwai 2009-06-22 19:46 ` Igor Chernyshev 0 siblings, 1 reply; 10+ messages in thread From: Takashi Iwai @ 2009-06-22 16:42 UTC (permalink / raw) To: Igor Chernyshev; +Cc: alsa-devel At Fri, 19 Jun 2009 23:37:16 -0700, Igor Chernyshev wrote: > > Hi Alsa Developers! > > I've built a small HTPC and had to add suspend/resume support in ice1724 > driver. There seem to be 3 existing bugs related to that: > > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=4413 > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3748 > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=2314 > > Due to hardware (un)availability, I only enabled the fix for Audiotrak > Prodigy HD2 card, which is installed in my HTPC. However, most of my code > should be reusable in the future on other ice1724-based cards as well (as > long as people add card-specific peices of code). The fix is currently based > on ALSA 1.0.20 and works on my MythBuntu 9.04 HTPC (using 2.6.28-11 kernel). > > The following files have been modified: > alsa-kernel/pci/ice1712/ice1712.h > alsa-kernel/pci/ice1712/ice1724.c > alsa-kernel/pci/ice1712/prodigy_hifi.c > > Now, I'm a complete newbie in Linux development and not sure I can safely > rebuild latest Linux Kernel under MythBuntu, etc. Are there any resources > for people like me to read? I want to provide the patch without destroying > my computer a hunder times, but also make it more convenient and safer for > ALSA maintainers. Well, I'd like to see the patch before anything else :) Could you just post your patch for review? thanks, Takashi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-22 16:42 ` Takashi Iwai @ 2009-06-22 19:46 ` Igor Chernyshev 2009-06-23 10:55 ` Takashi Iwai 0 siblings, 1 reply; 10+ messages in thread From: Igor Chernyshev @ 2009-06-22 19:46 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel Hi Takashi, here is the patch for: Supported suspend/resume in Audiotrak Prodigy HD2 Please, let me know if you want some special testing, patch formatting, etc on my part. So far I did suspend-resume with MythTV playing AVI and DVD through RCA. In original ALSA 1.0.20, MythTV playback upon resume would start ~20% faster than before suspend, volume would go to max (or above? given how awfully loud the noises were) and there would be ton of noises. After the change, resume gives clean playback in MythTV. I suspect that I may not have covered different scenarios (e.g. driver implies there may be 3 pcm's, ac97, and mic) in my testing. Thanks, Igor Here is the part I know about : : ))) Signed-off-by: Igor Chernyshev <igor.ch75+alsa at gmail.com> ----- 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-19 22:27:12.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 long 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-22 12:13:26.000000000 -0700 @@ -568,6 +568,10 @@ spin_unlock(&ice->reg_lock); break; + case SNDRV_PCM_TRIGGER_SUSPEND: + case SNDRV_PCM_TRIGGER_RESUME: + break; + default: return -EINVAL; } @@ -2266,6 +2270,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 +2419,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 +2449,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 +2632,104 @@ 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); + + if (ice->pcm) + snd_pcm_suspend_all(ice->pcm); + if (ice->pcm_pro) + snd_pcm_suspend_all(ice->pcm_pro); + if (ice->pcm_ds) + snd_pcm_suspend_all(ice->pcm_ds); + if (ice->ac97) + 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); + + spin_lock_irq(&ice->reg_lock); + if (ice->pm_saved_is_spdif_master) { + /* switching to external clock via SPDIF */ + ice->set_spdif_clock(ice); + } else { + /* internal on-card clock */ + spin_unlock_irq(&ice->reg_lock); + snd_vt1724_set_pro_rate(ice, ice->pro_rate_default, 1); + spin_lock_irq(&ice->reg_lock); + } + + 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)); + spin_unlock_irq(&ice->reg_lock); + + 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-22 12:06:28.000000000 -0700 @@ -1077,8 +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 */ AK4396_CTRL2, 0x02, @@ -1087,9 +1086,38 @@ 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; } On Mon, Jun 22, 2009 at 9:42 AM, Takashi Iwai <tiwai@suse.de> wrote: > > At Fri, 19 Jun 2009 23:37:16 -0700, > Igor Chernyshev wrote: > > > > Hi Alsa Developers! > > > > I've built a small HTPC and had to add suspend/resume support in ice1724 > > driver. There seem to be 3 existing bugs related to that: > > > > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=4413 > > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=3748 > > https://bugtrack.alsa-project.org/alsa-bug/view.php?id=2314 > > > > Due to hardware (un)availability, I only enabled the fix for Audiotrak > > Prodigy HD2 card, which is installed in my HTPC. However, most of my code > > should be reusable in the future on other ice1724-based cards as well (as > > long as people add card-specific peices of code). The fix is currently based > > on ALSA 1.0.20 and works on my MythBuntu 9.04 HTPC (using 2.6.28-11 kernel). > > > > The following files have been modified: > > alsa-kernel/pci/ice1712/ice1712.h > > alsa-kernel/pci/ice1712/ice1724.c > > alsa-kernel/pci/ice1712/prodigy_hifi.c > > > > Now, I'm a complete newbie in Linux development and not sure I can safely > > rebuild latest Linux Kernel under MythBuntu, etc. Are there any resources > > for people like me to read? I want to provide the patch without destroying > > my computer a hunder times, but also make it more convenient and safer for > > ALSA maintainers. > > Well, I'd like to see the patch before anything else :) > Could you just post your patch for review? > > > thanks, > > Takashi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-22 19:46 ` Igor Chernyshev @ 2009-06-23 10:55 ` Takashi Iwai 2009-06-23 22:50 ` Igor Chernyshev 0 siblings, 1 reply; 10+ messages in thread From: Takashi Iwai @ 2009-06-23 10:55 UTC (permalink / raw) To: Igor Chernyshev; +Cc: alsa-devel At Mon, 22 Jun 2009 12:46:48 -0700, Igor Chernyshev wrote: > > Hi Takashi, > > here is the patch for: > > Supported suspend/resume in Audiotrak Prodigy HD2 > > Please, let me know if you want some special testing, patch > formatting, etc on my part. So far I did suspend-resume with MythTV > playing AVI and DVD through RCA. In original ALSA 1.0.20, MythTV > playback upon resume would start ~20% faster than before suspend, > volume would go to max (or above? given how awfully loud the noises > were) and there would be ton of noises. After the change, resume gives > clean playback in MythTV. I suspect that I may not have covered > different scenarios (e.g. driver implies there may be 3 pcm's, ac97, > and mic) in my testing. > > Thanks, > Igor > > Here is the part I know about : : ))) > > Signed-off-by: Igor Chernyshev <igor.ch75+alsa at gmail.com> Thanks for the patch. Just looking through the changes, and it looks mostly OK. Small comments. > > 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-19 > 22:27:12.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 long pm_saved_route; This shouldn't be long but int. Long can be 64bit unnecessarily. > +#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-22 > 12:13:26.000000000 -0700 > @@ -568,6 +568,10 @@ > spin_unlock(&ice->reg_lock); > break; > > + case SNDRV_PCM_TRIGGER_SUSPEND: > + case SNDRV_PCM_TRIGGER_RESUME: > + break; At least, TRIGGER_SUSPEND neees to stop the PCM stream. > default: > return -EINVAL; > } > @@ -2266,6 +2270,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)); > + This is fine, but you need to remove __devinit from this function. > +#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); > + > + if (ice->pcm) > + snd_pcm_suspend_all(ice->pcm); > + if (ice->pcm_pro) > + snd_pcm_suspend_all(ice->pcm_pro); > + if (ice->pcm_ds) > + snd_pcm_suspend_all(ice->pcm_ds); > + if (ice->ac97) > + snd_ac97_suspend(ice->ac97); No need for NULL check here. All these check NULL by themselves. > + 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); > + > + spin_lock_irq(&ice->reg_lock); > + if (ice->pm_saved_is_spdif_master) { > + /* switching to external clock via SPDIF */ > + ice->set_spdif_clock(ice); > + } else { > + /* internal on-card clock */ > + 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... > + 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)); > + spin_unlock_irq(&ice->reg_lock); > + > + 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-22 > 12:06:28.000000000 -0700 > @@ -1077,8 +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 */ > AK4396_CTRL2, 0x02, > @@ -1087,9 +1086,38 @@ > 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; > } Last but not least, try to run $LINUX/scripts/checkpatch.pl to your patch once and fix the issues suggested there. thanks, Takashi ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-23 10:55 ` Takashi Iwai @ 2009-06-23 22:50 ` Igor Chernyshev 2009-06-24 10:13 ` Takashi Iwai 0 siblings, 1 reply; 10+ messages in thread From: Igor Chernyshev @ 2009-06-23 22:50 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel >> + 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; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-23 22:50 ` Igor Chernyshev @ 2009-06-24 10:13 ` Takashi Iwai 2009-06-25 6:15 ` Igor Chernyshev 0 siblings, 1 reply; 10+ messages in thread From: Takashi Iwai @ 2009-06-24 10:13 UTC (permalink / raw) To: Igor Chernyshev; +Cc: alsa-devel 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-24 10:13 ` Takashi Iwai @ 2009-06-25 6:15 ` Igor Chernyshev 2009-06-25 7:05 ` Takashi Iwai 0 siblings, 1 reply; 10+ messages in thread From: Igor Chernyshev @ 2009-06-25 6:15 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel 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@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; } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-25 6:15 ` Igor Chernyshev @ 2009-06-25 7:05 ` Takashi Iwai 2009-06-25 7:23 ` Igor Chernyshev 0 siblings, 1 reply; 10+ messages in thread From: Takashi Iwai @ 2009-06-25 7:05 UTC (permalink / raw) To: Igor Chernyshev; +Cc: alsa-devel At Wed, 24 Jun 2009 23:15:50 -0700, Igor Chernyshev wrote: > > Takashi, > > below is the same patch based on the latest snapshot. Your MUA (gmail?) broke the embedded patch by line-breaking, so it can't be applied. Either fix your MUA setup not to do that or use an attachment if it's difficult. thanks, Takashi > > Thanks, > Igor > > On Wed, Jun 24, 2009 at 3:13 AM, Takashi Iwai<tiwai@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; > } > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-25 7:05 ` Takashi Iwai @ 2009-06-25 7:23 ` Igor Chernyshev 2009-06-25 7:38 ` Takashi Iwai 0 siblings, 1 reply; 10+ messages in thread From: Igor Chernyshev @ 2009-06-25 7:23 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel [-- Attachment #1: Type: text/plain, Size: 12134 bytes --] On Thu, Jun 25, 2009 at 12:05 AM, Takashi Iwai<tiwai@suse.de> wrote: > At Wed, 24 Jun 2009 23:15:50 -0700, > Igor Chernyshev wrote: >> >> Takashi, >> >> below is the same patch based on the latest snapshot. > > Your MUA (gmail?) broke the embedded patch by line-breaking, so it > can't be applied. > > Either fix your MUA setup not to do that or use an attachment if it's > difficult. Strange... I was using Gmail Web interface in plain text mode for all the emails in this thread and it used to work. Anyway, I'm attaching the patch now. And just in case Gmail messes up again, I also uploaded patch here: http://www.rinacherry.com/patchfile2 Sorry for the problems. > > > thanks, > > Takashi > >> >> Thanks, >> Igor >> >> On Wed, Jun 24, 2009 at 3:13 AM, Takashi Iwai<tiwai@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; >> } >> > [-- Attachment #2: patchfile2 --] [-- Type: application/octet-stream, Size: 7074 bytes --] 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; } [-- Attachment #3: Type: text/plain, Size: 160 bytes --] _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 2009-06-25 7:23 ` Igor Chernyshev @ 2009-06-25 7:38 ` Takashi Iwai 0 siblings, 0 replies; 10+ messages in thread From: Takashi Iwai @ 2009-06-25 7:38 UTC (permalink / raw) To: Igor Chernyshev; +Cc: alsa-devel At Thu, 25 Jun 2009 00:23:24 -0700, Igor Chernyshev wrote: > > On Thu, Jun 25, 2009 at 12:05 AM, Takashi Iwai<tiwai@suse.de> wrote: > > At Wed, 24 Jun 2009 23:15:50 -0700, > > Igor Chernyshev wrote: > >> > >> Takashi, > >> > >> below is the same patch based on the latest snapshot. > > > > Your MUA (gmail?) broke the embedded patch by line-breaking, so it > > can't be applied. > > > > Either fix your MUA setup not to do that or use an attachment if it's > > difficult. > > Strange... I was using Gmail Web interface in plain text mode for all > the emails in this thread and it used to work. Anyway, I'm attaching > the patch now. And just in case Gmail messes up again, I also uploaded > patch here: http://www.rinacherry.com/patchfile2 Thanks, applied now. Takashi ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-06-25 7:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-06-20 6:37 Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2 Igor Chernyshev 2009-06-22 16:42 ` Takashi Iwai 2009-06-22 19:46 ` Igor Chernyshev 2009-06-23 10:55 ` Takashi Iwai 2009-06-23 22:50 ` Igor Chernyshev 2009-06-24 10:13 ` Takashi Iwai 2009-06-25 6:15 ` Igor Chernyshev 2009-06-25 7:05 ` Takashi Iwai 2009-06-25 7:23 ` Igor Chernyshev 2009-06-25 7:38 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.