[alsa-devel] Patch for suspend/resume in ICE1724 for Audiotrak Prodigy HD2
Takashi Iwai
tiwai at suse.de
Thu Jun 25 09:05:47 CEST 2009
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 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