[alsa-devel] [PATCH v2 0/3] Allow suspend/resume with USB audio.

[Takashi Iwai]

At Tue, 18 Sep 2012 09:49:45 -0700,
Dylan Reid wrote:
> 
> Problem being addressed:
> shell one$ aplay -Dhw:<usbcard> file.wav
> shell two$ echo mem > /sys/power/state
> After resume, aplay attempts resume, then restart, then after a timeout
> gets a write error and no audio will be played until it is restarted.
> 
> During suspend/resume, if the USB port remains powered the audio device
> will still be present upon resume.  The interface and endpoint have to
> be reconfigured however.  These patches move the configuration from
> hw_params to prepare causing re-configuration during resume.
> 
> Is this the right way to fix this?  Or is there an easier way I am
> missing?
> 
> Thanks for looking.
> 
> v2: fix unused variable warnings.

I think the patches are OK.  If no big objection comes up, I'm going
to apply it tomorrow.

Meanwhile I found a problem in my patch.  The re-setup might be needed
if the app changes hw_params again.  So, the flag has to be set in
hw_params callback instead.  The revised patch is below.


thanks,

Takashi

===

From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH v2] ALSA: usb-audio: Avoid unnecessary EP setups in prepare

The recent fix for USB suspend breakage moved the code to set up EP
from hw_params to prepare, but it means also the EP setup might be
called multiple times unnecessarily because the prepare callback can
be called multiple times without starting the stream (e.g. OSS
emulation).

This patch adds a new flag to struct snd_usb_substream indicating
whether the setup of EP is required, and do it only when necessary,
i.e. right after hw_params or suspend.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
v1->v2: set the flag in hw_params callback

 sound/usb/card.c |  2 ++
 sound/usb/card.h |  1 +
 sound/usb/pcm.c  | 10 +++++++---
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 4a469f0..561bb74 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -646,6 +646,8 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 			list_for_each(p, &chip->pcm_list) {
 				as = list_entry(p, struct snd_usb_stream, list);
 				snd_pcm_suspend_all(as->pcm);
+				as->substream[0].need_setup_ep =
+					as->substream[1].need_setup_ep = true;
 			}
  		}
 	} else {
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 6cc883c..afa4f9e 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -125,6 +125,7 @@ struct snd_usb_substream {
 	struct snd_usb_endpoint *data_endpoint;
 	struct snd_usb_endpoint *sync_endpoint;
 	unsigned long flags;
+	bool need_setup_ep;		/* (re)configure EP at prepare? */
 
 	u64 formats;			/* format bitmasks (all or'ed) */
 	unsigned int num_formats;		/* number of supported audio formats (list) */
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index ae783d4..55e19e1 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -510,6 +510,7 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 
 	subs->interface = fmt->iface;
 	subs->altset_idx = fmt->altset_idx;
+	subs->need_setup_ep = true;
 
 	return 0;
 }
@@ -568,9 +569,12 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	if (ret < 0)
 		return ret;
 
-	ret = configure_endpoint(subs);
-	if (ret < 0)
-		return ret;
+	if (subs->need_setup_ep) {
+		ret = configure_endpoint(subs);
+		if (ret < 0)
+			return ret;
+		subs->need_setup_ep = false;
+	}
 
 	/* some unit conversions in runtime */
 	subs->data_endpoint->maxframesize =
-- 
1.7.11.5