At Wed, 21 Aug 2013 09:44:22 +0200, Knut Petersen wrote:
[1 <text/plain; ISO-8859-1 (7bit)>] On 15.08.2013 10:37, Takashi Iwai wrote:
It'd be much simpler to just let the driver aborting the problem in the error of memory allocations.
Ok, so here is v3:
- Add PM,
- abort modprobe if vmalloc() of suspend buffers fail,
- do not leak memory in that case.
Actually this triggers the double free. It's freed in snd_rme96_free() via snd_card_free().
I applied the patch with removal of this double vfree call.
thanks,
Takashi
cu, Knut [2 0001-alsa-rme96-Add-PM-support-v3.patch <text/x-patch (7bit)>]
From d26114c1dc66c1396055f5cd44150fa66604a739 Mon Sep 17 00:00:00 2001
From: Knut Petersen Knut_Petersen@t-online.de Date: Wed, 21 Aug 2013 09:18:54 +0200 Subject: [PATCH] alsa/rme96: Add PM support v3
Without proper power management handling, the first use of a Digi96/8 anytime after a suspend / resume cycle will start playback with distortions.
v3: Abort if vmalloc() of suspend buffers fail, but do not leak memory in that case.
Signed-off-by: Knut Petersen Knut_Petersen@t-online.de
sound/pci/rme96.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+)
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c index 3eb0bdd..2acf565 100644 --- a/sound/pci/rme96.c +++ b/sound/pci/rme96.c @@ -239,6 +239,13 @@ struct rme96 {
u8 rev; /* card revision number */
+#ifdef CONFIG_PM
- u32 playback_pointer;
- u32 capture_pointer;
- void *playback_suspend_buffer;
- void *capture_suspend_buffer;
+#endif
- struct snd_pcm_substream *playback_substream; struct snd_pcm_substream *capture_substream;
@@ -370,6 +377,7 @@ static struct snd_pcm_hardware snd_rme96_playback_spdif_info = .info = (SNDRV_PCM_INFO_MMAP_IOMEM | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_SYNC_START |
SNDRV_PCM_INFO_RESUME | SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_PAUSE),@@ -400,6 +408,7 @@ static struct snd_pcm_hardware snd_rme96_capture_spdif_info = .info = (SNDRV_PCM_INFO_MMAP_IOMEM | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_SYNC_START |
SNDRV_PCM_INFO_RESUME | SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_PAUSE),@@ -430,6 +439,7 @@ static struct snd_pcm_hardware snd_rme96_playback_adat_info = .info = (SNDRV_PCM_INFO_MMAP_IOMEM | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_SYNC_START |
SNDRV_PCM_INFO_RESUME | SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_PAUSE),@@ -456,6 +466,7 @@ static struct snd_pcm_hardware snd_rme96_capture_adat_info = .info = (SNDRV_PCM_INFO_MMAP_IOMEM | SNDRV_PCM_INFO_MMAP_VALID | SNDRV_PCM_INFO_SYNC_START |
SNDRV_PCM_INFO_RESUME | SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_PAUSE),@@ -1386,6 +1397,7 @@ snd_rme96_playback_trigger(struct snd_pcm_substream *substream, } break;
- case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: if (RME96_ISPLAYING(rme96)) { if (substream != rme96->playback_substream)
@@ -1401,6 +1413,7 @@ snd_rme96_playback_trigger(struct snd_pcm_substream *substream, : RME96_STOP_PLAYBACK); break;
- case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (!RME96_ISPLAYING(rme96)) snd_rme96_trigger(rme96, sync ? RME96_RESUME_BOTH
@@ -1441,6 +1454,7 @@ snd_rme96_capture_trigger(struct snd_pcm_substream *substream, } break;
- case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: if (RME96_ISRECORDING(rme96)) { if (substream != rme96->capture_substream)
@@ -1456,6 +1470,7 @@ snd_rme96_capture_trigger(struct snd_pcm_substream *substream, : RME96_STOP_CAPTURE); break;
- case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: if (!RME96_ISRECORDING(rme96)) snd_rme96_trigger(rme96, sync ? RME96_RESUME_BOTH
@@ -1556,6 +1571,10 @@ snd_rme96_free(void *private_data) pci_release_regions(rme96->pci); rme96->port = 0; } +#ifdef CONFIG_PM
- vfree(rme96->playback_suspend_buffer);
- vfree(rme96->capture_suspend_buffer);
+#endif pci_disable_device(rme96->pci); }
@@ -2354,6 +2373,83 @@ snd_rme96_create_switches(struct snd_card *card,
- Card initialisation
*/
+#ifdef CONFIG_PM
+static int +snd_rme96_suspend(struct pci_dev *pci,
pm_message_t state)+{
- struct snd_card *card = pci_get_drvdata(pci);
- struct rme96 *rme96 = card->private_data;
- snd_power_change_state(card, SNDRV_CTL_POWER_D3hot);
- snd_pcm_suspend(rme96->playback_substream);
- snd_pcm_suspend(rme96->capture_substream);
- /* save capture & playback pointers */
- rme96->playback_pointer = readl(rme96->iobase + RME96_IO_GET_PLAY_POS)
& RME96_RCR_AUDIO_ADDR_MASK;- rme96->capture_pointer = readl(rme96->iobase + RME96_IO_GET_REC_POS)
& RME96_RCR_AUDIO_ADDR_MASK;- /* save playback and capture buffers */
- memcpy_fromio(rme96->playback_suspend_buffer,
rme96->iobase + RME96_IO_PLAY_BUFFER, RME96_BUFFER_SIZE);- memcpy_fromio(rme96->capture_suspend_buffer,
rme96->iobase + RME96_IO_REC_BUFFER, RME96_BUFFER_SIZE);- /* disable the DAC */
- rme96->areg &= ~RME96_AR_DAC_EN;
- writel(rme96->areg, rme96->iobase + RME96_IO_ADDITIONAL_REG);
- pci_disable_device(pci);
- pci_save_state(pci);
- return 0;
+}
+static int +snd_rme96_resume(struct pci_dev *pci) +{
- struct snd_card *card = pci_get_drvdata(pci);
- struct rme96 *rme96 = card->private_data;
- pci_restore_state(pci);
- pci_enable_device(pci);
- /* reset playback and record buffer pointers */
- writel(0, rme96->iobase + RME96_IO_SET_PLAY_POS
+ rme96->playback_pointer);- writel(0, rme96->iobase + RME96_IO_SET_REC_POS
+ rme96->capture_pointer);- /* restore playback and capture buffers */
- memcpy_toio(rme96->iobase + RME96_IO_PLAY_BUFFER,
rme96->playback_suspend_buffer, RME96_BUFFER_SIZE);- memcpy_toio(rme96->iobase + RME96_IO_REC_BUFFER,
rme96->capture_suspend_buffer, RME96_BUFFER_SIZE);- /* reset the ADC */
- writel(rme96->areg | RME96_AR_PD2,
rme96->iobase + RME96_IO_ADDITIONAL_REG);- writel(rme96->areg, rme96->iobase + RME96_IO_ADDITIONAL_REG);
- /* reset and enable DAC, restore analog volume */
- snd_rme96_reset_dac(rme96);
- rme96->areg |= RME96_AR_DAC_EN;
- writel(rme96->areg, rme96->iobase + RME96_IO_ADDITIONAL_REG);
- if (RME96_HAS_ANALOG_OUT(rme96)) {
usleep_range(3000, 10000);snd_rme96_apply_dac_volume(rme96);- }
- snd_power_change_state(card, SNDRV_CTL_POWER_D0);
- return 0;
+}
+#endif
static void snd_rme96_card_free(struct snd_card *card) { snd_rme96_free(card->private_data); @@ -2390,6 +2486,24 @@ snd_rme96_probe(struct pci_dev *pci, return err; }
+#ifdef CONFIG_PM
- rme96->playback_suspend_buffer = vmalloc(RME96_BUFFER_SIZE);
- if (!rme96->playback_suspend_buffer) {
snd_printk(KERN_ERR"Failed to allocate playback suspend buffer!\n");snd_card_free(card);return -ENOMEM;- }
- rme96->capture_suspend_buffer = vmalloc(RME96_BUFFER_SIZE);
- if (!rme96->capture_suspend_buffer) {
snd_printk(KERN_ERR"Failed to allocate capture suspend buffer!\n");vfree(rme96->playback_suspend_buffer);snd_card_free(card);return -ENOMEM;- }
+#endif
- strcpy(card->driver, "Digi96"); switch (rme96->pci->device) { case PCI_DEVICE_ID_RME_DIGI96:
@@ -2433,6 +2547,10 @@ static struct pci_driver rme96_driver = { .id_table = snd_rme96_ids, .probe = snd_rme96_probe, .remove = snd_rme96_remove, +#ifdef CONFIG_PM
- .suspend = snd_rme96_suspend,
- .resume = snd_rme96_resume,
+#endif };
module_pci_driver(rme96_driver);
1.8.1.4