All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Add PM support to the rme96 driver
@ 2013-08-12 16:29 Knut Petersen
  2013-08-13  7:24 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Knut Petersen @ 2013-08-12 16:29 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: Takashi Iwai, alsa-devel

[-- Attachment #1: Type: text/plain, Size: 528 bytes --]

Hi everybody!

The attached patch adds PM support to the rme96 driver,
it should be applied after v2 of the patch adding pcm stream
synchronization support posted on 2013-08-08.

It does work perfectly here on an RME Digi96/8 PAD:

After connecting analog output and analog input executing

     #! /bin/sh
     #
     arecord  -D hw:0,0 -d 62 testout.wav  &
     aplay  -D hw:0,0 -d 60 testin.wav

produces an almost perfect recording even
when the test machine is suspended / hibernated
and resumed several times.

cu,
  Knut

[-- Attachment #2: 0001-alsa-rme96-Add-PM-support-to-the-rme96-driver.patch --]
[-- Type: text/x-patch, Size: 7199 bytes --]

>From 7305b056cbb4de562018483b91cf90c677df317f Mon Sep 17 00:00:00 2001
From: Knut Petersen <Knut_Petersen@t-online.de>
Date: Mon, 12 Aug 2013 18:00:30 +0200
Subject: [PATCH] alsa/rme96: Add PM support to the rme96 driver

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 35651c5..6e0a6e1 100644
--- a/sound/pci/rme96.c
+++ b/sound/pci/rme96.c
@@ -232,6 +232,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;
 
@@ -363,6 +370,9 @@ 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 |
+#ifdef CONFIG_PM
+			      SNDRV_PCM_INFO_RESUME |
+#endif
 			      SNDRV_PCM_INFO_INTERLEAVED |
 			      SNDRV_PCM_INFO_PAUSE),
 	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
@@ -393,6 +403,9 @@ 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 |
+#ifdef CONFIG_PM
+			      SNDRV_PCM_INFO_RESUME |
+#endif
 			      SNDRV_PCM_INFO_INTERLEAVED |
 			      SNDRV_PCM_INFO_PAUSE),
 	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
@@ -423,6 +436,9 @@ 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 |
+#ifdef CONFIG_PM
+			      SNDRV_PCM_INFO_RESUME |
+#endif
 			      SNDRV_PCM_INFO_INTERLEAVED |
 			      SNDRV_PCM_INFO_PAUSE),
 	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
@@ -449,6 +465,9 @@ 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 |
+#ifdef CONFIG_PM
+			      SNDRV_PCM_INFO_RESUME |
+#endif
 			      SNDRV_PCM_INFO_INTERLEAVED |
 			      SNDRV_PCM_INFO_PAUSE),
 	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
@@ -1387,6 +1406,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) {
@@ -1402,6 +1422,7 @@ snd_rme96_playback_trigger(struct snd_pcm_substream *substream,
 		}
 		break;
 
+	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (!RME96_ISPLAYING(rme96)) {
 			snd_rme96_trigger(rme96, sync ? RME96_RESUME_BOTH : RME96_RESUME_PLAYBACK);
@@ -1441,6 +1462,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 +1478,7 @@ snd_rme96_capture_trigger(struct snd_pcm_substream *substream,
 		}
 		break;
 
+	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		if (!RME96_ISRECORDING(rme96)) {
 			snd_rme96_trigger(rme96, sync ? RME96_RESUME_BOTH : RME96_RESUME_CAPTURE);
@@ -1556,6 +1579,14 @@ snd_rme96_free(void *private_data)
 		pci_release_regions(rme96->pci);
 		rme96->port = 0;
 	}
+#ifdef CONFIG_PM
+	if (rme96->playback_suspend_buffer) {  
+		kfree(rme96->playback_suspend_buffer);
+	}
+	if (rme96->capture_suspend_buffer) {  
+		kfree(rme96->capture_suspend_buffer);
+	}
+#endif
 	pci_disable_device(rme96->pci);
 }
 
@@ -2354,6 +2385,76 @@ 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)) {
+		/* msleep(1) is the  documented AD1852 minimum */
+		msleep(3);
+		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 +2491,19 @@ snd_rme96_probe(struct pci_dev *pci,
 		return err;
 	}
 	
+#ifdef CONFIG_PM
+	rme96->playback_suspend_buffer = kmalloc(RME96_BUFFER_SIZE, GFP_KERNEL);
+	if (!rme96->playback_suspend_buffer) {
+	snd_printk(KERN_ERR "Failed to allocate playback suspend buffer!\n");
+		return -ENOMEM;
+	}
+	rme96->capture_suspend_buffer = kmalloc(RME96_BUFFER_SIZE, GFP_KERNEL);
+	if (!rme96->capture_suspend_buffer) {
+		snd_printk(KERN_ERR "Failed to allocate capture suspend buffer!\n");
+	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


[-- Attachment #3: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] Add PM support to the rme96 driver
  2013-08-12 16:29 [PATCH] Add PM support to the rme96 driver Knut Petersen
@ 2013-08-13  7:24 ` Takashi Iwai
  2013-08-13  9:02   ` Knut Petersen
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2013-08-13  7:24 UTC (permalink / raw)
  To: Knut Petersen; +Cc: alsa-devel

At Mon, 12 Aug 2013 18:29:44 +0200,
Knut Petersen wrote:
> 
> >From 7305b056cbb4de562018483b91cf90c677df317f Mon Sep 17 00:00:00 2001
> From: Knut Petersen <Knut_Petersen@t-online.de>
> Date: Mon, 12 Aug 2013 18:00:30 +0200
> Subject: [PATCH] alsa/rme96: Add PM support to the rme96 driver

Isn't there any other information you'd like to add?
Put it here.

> 
> 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 35651c5..6e0a6e1 100644
> --- a/sound/pci/rme96.c
> +++ b/sound/pci/rme96.c
> @@ -232,6 +232,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;
>  
> @@ -363,6 +370,9 @@ 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 |
> +#ifdef CONFIG_PM
> +			      SNDRV_PCM_INFO_RESUME |
> +#endif

No need for ifdef.

>  			      SNDRV_PCM_INFO_INTERLEAVED |
>  			      SNDRV_PCM_INFO_PAUSE),
>  	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
> @@ -393,6 +403,9 @@ 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 |
> +#ifdef CONFIG_PM
> +			      SNDRV_PCM_INFO_RESUME |
> +#endif
>  			      SNDRV_PCM_INFO_INTERLEAVED |
>  			      SNDRV_PCM_INFO_PAUSE),
>  	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
> @@ -423,6 +436,9 @@ 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 |
> +#ifdef CONFIG_PM
> +			      SNDRV_PCM_INFO_RESUME |
> +#endif
>  			      SNDRV_PCM_INFO_INTERLEAVED |
>  			      SNDRV_PCM_INFO_PAUSE),
>  	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
> @@ -449,6 +465,9 @@ 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 |
> +#ifdef CONFIG_PM
> +			      SNDRV_PCM_INFO_RESUME |
> +#endif
>  			      SNDRV_PCM_INFO_INTERLEAVED |
>  			      SNDRV_PCM_INFO_PAUSE),
>  	.formats =	     (SNDRV_PCM_FMTBIT_S16_LE |
> @@ -1387,6 +1406,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) {
> @@ -1402,6 +1422,7 @@ snd_rme96_playback_trigger(struct snd_pcm_substream *substream,
>  		}
>  		break;
>  
> +	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>  		if (!RME96_ISPLAYING(rme96)) {
>  			snd_rme96_trigger(rme96, sync ? RME96_RESUME_BOTH : RME96_RESUME_PLAYBACK);
> @@ -1441,6 +1462,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 +1478,7 @@ snd_rme96_capture_trigger(struct snd_pcm_substream *substream,
>  		}
>  		break;
>  
> +	case SNDRV_PCM_TRIGGER_RESUME:
>  	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
>  		if (!RME96_ISRECORDING(rme96)) {
>  			snd_rme96_trigger(rme96, sync ? RME96_RESUME_BOTH : RME96_RESUME_CAPTURE);
> @@ -1556,6 +1579,14 @@ snd_rme96_free(void *private_data)
>  		pci_release_regions(rme96->pci);
>  		rme96->port = 0;
>  	}
> +#ifdef CONFIG_PM
> +	if (rme96->playback_suspend_buffer) {  
> +		kfree(rme96->playback_suspend_buffer);
> +	}
> +	if (rme96->capture_suspend_buffer) {  
> +		kfree(rme96->capture_suspend_buffer);
> +	}
> +#endif
>  	pci_disable_device(rme96->pci);
>  }
>  
> @@ -2354,6 +2385,76 @@ 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);

Doesn't it go to PCI D3 state?


> +
> +	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)) {
> +		/* msleep(1) is the  documented AD1852 minimum */
> +		msleep(3);
> +		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 +2491,19 @@ snd_rme96_probe(struct pci_dev *pci,
>  		return err;
>  	}
>  	
> +#ifdef CONFIG_PM
> +	rme96->playback_suspend_buffer = kmalloc(RME96_BUFFER_SIZE, GFP_KERNEL);
> +	if (!rme96->playback_suspend_buffer) {
> +	snd_printk(KERN_ERR "Failed to allocate playback suspend buffer!\n");
> +		return -ENOMEM;
> +	}
> +	rme96->capture_suspend_buffer = kmalloc(RME96_BUFFER_SIZE, GFP_KERNEL);
> +	if (!rme96->capture_suspend_buffer) {
> +		snd_printk(KERN_ERR "Failed to allocate capture suspend buffer!\n");
> +	return -ENOMEM;

Better to use vmalloc() for 64k buffer.

Other than the above, please fix the coding-style issues.


thanks,

Takashi


> +	}
> +#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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] Add PM support to the rme96 driver
  2013-08-13  7:24 ` Takashi Iwai
@ 2013-08-13  9:02   ` Knut Petersen
  0 siblings, 0 replies; 3+ messages in thread
From: Knut Petersen @ 2013-08-13  9:02 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel

On 13.08.2013 09:24, Takashi Iwai wrote:
> At Mon, 12 Aug 2013 18:29:44 +0200,
> Knut Petersen wrote:
>> >From 7305b056cbb4de562018483b91cf90c677df317f Mon Sep 17 00:00:00 2001
>> From: Knut Petersen <Knut_Petersen@t-online.de>
>> Date: Mon, 12 Aug 2013 18:00:30 +0200
>> Subject: [PATCH] alsa/rme96: Add PM support to the rme96 driver
> Isn't there any other information you'd like to add?
> Put it here.

Ok, I`ll try to be a bit more verbose. Without the hardware reinitialization added,
the first use of this card anytime after a suspend / resume cycle will start with
distortions.

>   			      SNDRV_PCM_INFO_SYNC_START |
> +#ifdef CONFIG_PM
> +			      SNDRV_PCM_INFO_RESUME |
> +#endif
> No need for ifdef.

ACK.


> +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);
[...]
> +
> +	pci_disable_device(pci);
> +	pci_save_state(pci);
> Doesn't it go to PCI D3 state?

lspci -vv

05:04.0 Multimedia audio controller: Xilinx Corporation RME Digi96/8 Pad (rev 04)
         Control: I/O- Mem+ BusMaster- SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
         Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=slow >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
         Interrupt: pin A routed to IRQ 16
         Region 0: Memory at d0000000 (32-bit, non-prefetchable) [size=16M]
         Kernel driver in use: snd_rme96

This is pretty old hardware, the Digi96 series was introduced to the market in October 1998.
There is no support for any power management, and the Digi96 cards do not act as bus master.
No, the hardware will definitely not enter PCI D3 state.

>> +#ifdef CONFIG_PM
>> +	rme96->playback_suspend_buffer = kmalloc(RME96_BUFFER_SIZE, GFP_KERNEL);
>> +	if (!rme96->playback_suspend_buffer) {
>> +	snd_printk(KERN_ERR "Failed to allocate playback suspend buffer!\n");
>> +		return -ENOMEM;
>> +	}
>> +	rme96->capture_suspend_buffer = kmalloc(RME96_BUFFER_SIZE, GFP_KERNEL);
>> +	if (!rme96->capture_suspend_buffer) {
>> +		snd_printk(KERN_ERR "Failed to allocate capture suspend buffer!\n");
>> +	return -ENOMEM;
> Better to use vmalloc() for 64k buffer.
ACK

> Other than the above, please fix the coding-style issues.

ok, cu,
  knut

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-08-13  9:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-12 16:29 [PATCH] Add PM support to the rme96 driver Knut Petersen
2013-08-13  7:24 ` Takashi Iwai
2013-08-13  9:02   ` Knut Petersen

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.