[ALSA] cs5535audio: fix PRD register save/restore power management race
authorAndres Salomon <dilinger@debian.org>
Mon, 3 Sep 2007 13:42:16 +0000 (15:42 +0200)
committerJaroslav Kysela <perex@perex.cz>
Tue, 16 Oct 2007 13:59:52 +0000 (15:59 +0200)
In the suspend path, we currently save the PRD registers and then disable DMA.
This is racy; the sound hardware might update the PRD register as it finishes
processing some DMA pages between when we've saved the PRD registers and
when DMA actually gets disabled.  Furthermore, we actively check whether or
not DMA is enabled before saving PRD registers; there's no reason to do that,
as the PRD registers should not update when we twiddle the ACC_BM[x]_CMD
register(s).  Worst case, we save the PRD registers twice; even powering
down the ACC shouldn't mess with the PRD registers (according to the 5536
data sheet, section 5.3.7.4, power-down procedure).  This patch reworks
all that to first disable DMA, and then save PRD registers.

Signed-off-by: Andres Salomon <dilinger@debian.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Jaroslav Kysela <perex@suse.cz>
sound/pci/cs5535audio/cs5535audio.h
sound/pci/cs5535audio/cs5535audio_pcm.c
sound/pci/cs5535audio/cs5535audio_pm.c

index 4fd1f31a6cf9666606c854b3b7931ea1c48a85a6..c7a20446703727652de01ed52aa7e36f3a6d76ff 100644 (file)
@@ -106,7 +106,6 @@ struct cs5535audio_dma {
        struct snd_pcm_substream *substream;
        unsigned int buf_addr, buf_bytes;
        unsigned int period_bytes, periods;
-       int suspended;
        u32 saved_prd;
 };
 
index 9a1e87fd481577d7d8d2ce97748354358cbbef5c..21df0634af32a2be83133a4375e0810b6d801455 100644 (file)
@@ -297,14 +297,12 @@ static int snd_cs5535audio_trigger(struct snd_pcm_substream *substream, int cmd)
                break;
        case SNDRV_PCM_TRIGGER_RESUME:
                dma->ops->enable_dma(cs5535au);
-               dma->suspended = 0;
                break;
        case SNDRV_PCM_TRIGGER_STOP:
                dma->ops->disable_dma(cs5535au);
                break;
        case SNDRV_PCM_TRIGGER_SUSPEND:
                dma->ops->disable_dma(cs5535au);
-               dma->suspended = 1;
                break;
        default:
                snd_printk(KERN_ERR "unhandled trigger\n");
index 3e4d198a4502dca0fc7def2652fe55369e9cfb4e..9a4e84aa3e048bab85f7be535a6818e43e323a22 100644 (file)
@@ -64,13 +64,13 @@ int snd_cs5535audio_suspend(struct pci_dev *pci, pm_message_t state)
        int i;
 
        snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
+       snd_pcm_suspend_all(cs5535au->pcm);
+       snd_ac97_suspend(cs5535au->ac97);
        for (i = 0; i < NUM_CS5535AUDIO_DMAS; i++) {
                struct cs5535audio_dma *dma = &cs5535au->dmas[i];
-               if (dma && dma->substream && !dma->suspended) 
+               if (dma && dma->substream)
                        dma->saved_prd = dma->ops->read_prd(cs5535au);
        }
-       snd_pcm_suspend_all(cs5535au->pcm);
-       snd_ac97_suspend(cs5535au->ac97);
        /* save important regs, then disable aclink in hw */
        snd_cs5535audio_stop_hardware(cs5535au);
 
@@ -112,17 +112,17 @@ int snd_cs5535audio_resume(struct pci_dev *pci)
        if (!timeout)
                snd_printk(KERN_ERR "Failure getting AC Link ready\n");
 
-       /* we depend on ac97 to perform the codec power up */
-       snd_ac97_resume(cs5535au->ac97);
        /* set up rate regs, dma. actual initiation is done in trig */
        for (i = 0; i < NUM_CS5535AUDIO_DMAS; i++) {
                struct cs5535audio_dma *dma = &cs5535au->dmas[i];
-               if (dma && dma->substream && dma->suspended) {
+               if (dma && dma->substream) {
                        dma->substream->ops->prepare(dma->substream);
                        dma->ops->setup_prd(cs5535au, dma->saved_prd);
                }
        }
-               
+
+       /* we depend on ac97 to perform the codec power up */
+       snd_ac97_resume(cs5535au->ac97);
        snd_power_change_state(card, SNDRV_CTL_POWER_D0);
 
        return 0;