[PATCH 0/4] ALSA: usb-audio: More EP-related fixes
This is a series of small fixes for USB-audio endpoint management after the latest change to split prepare/hw_params.
Takashi
===
Takashi Iwai (4): ALSA: usb-audio: Avoid unnecessary interface change at EP close ALSA: usb-audio: Apply mutex around snd_usb_endpoint_set_params() ALSA: usb-audio: Correct the return code from snd_usb_endpoint_set_params() ALSA: usb-audio: Avoid superfluous endpoint setup
sound/usb/card.h | 3 ++- sound/usb/endpoint.c | 32 +++++++++++++++++++++++++------- 2 files changed, 27 insertions(+), 8 deletions(-)
We toggle USB interface at PCM prepare and reset at close. When the PCM isn't prepared, resetting again makes little sense. Check the current altset and avoid unnecessary interface reset at EP close.
Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 48a3843a08f1..f21acbc9f4f4 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -32,6 +32,7 @@ struct snd_usb_iface_ref { unsigned char iface; bool need_setup; int opened; + int altset; struct list_head list; };
@@ -899,6 +900,9 @@ static int endpoint_set_interface(struct snd_usb_audio *chip, int altset = set ? ep->altsetting : 0; int err;
+ if (ep->iface_ref->altset == altset) + return 0; + usb_audio_dbg(chip, "Setting usb interface %d:%d for EP 0x%x\n", ep->iface, altset, ep->ep_num); err = usb_set_interface(chip->dev, ep->iface, altset); @@ -910,6 +914,7 @@ static int endpoint_set_interface(struct snd_usb_audio *chip,
if (chip->quirk_flags & QUIRK_FLAG_IFACE_DELAY) msleep(50); + ep->iface_ref->altset = altset; return 0; }
The protection with chip->mutex was lost after splitting snd_usb_endpoint_set_params() and snd_usb_endpoint_prepare(). Apply the same mutex again to the former function.
Fixes: 2be79d586454 ("ALSA: usb-audio: Split endpoint setups for hw_params and prepare (take#2)") Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index f21acbc9f4f4..da378e565ef8 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -1337,10 +1337,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, const struct audioformat *fmt = ep->cur_audiofmt; int err;
+ mutex_lock(&chip->mutex); /* release old buffers, if any */ err = release_urbs(ep, false); if (err < 0) - return err; + goto unlock;
ep->datainterval = fmt->datainterval; ep->maxpacksize = fmt->maxpacksize; @@ -1378,13 +1379,16 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, usb_audio_dbg(chip, "Set up %d URBS, ret=%d\n", ep->nurbs, err);
if (err < 0) - return err; + goto unlock;
/* some unit conversions in runtime */ ep->maxframesize = ep->maxpacksize / ep->cur_frame_bytes; ep->curframesize = ep->curpacksize / ep->cur_frame_bytes;
- return update_clock_ref_rate(chip, ep); + err = update_clock_ref_rate(chip, ep); + unlock: + mutex_unlock(&chip->mutex); + return err; }
static int init_sample_rate(struct snd_usb_audio *chip,
snd_usb_endpoint_set_params() should return zero for a success, but currently it returns the sample rate. Correct it.
Fixes: 2be79d586454 ("ALSA: usb-audio: Split endpoint setups for hw_params and prepare (take#2)") Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index da378e565ef8..44cce6cec9da 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -1386,6 +1386,8 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, ep->curframesize = ep->curpacksize / ep->cur_frame_bytes;
err = update_clock_ref_rate(chip, ep); + if (err >= 0) + err = 0; unlock: mutex_unlock(&chip->mutex); return err;
After splitting to snd_usb_endpoint_set_params() and *_prepare(), the skip of each function should be checked with different flags, while we still use ep->need_setup as the single one. Introduce ep->need_prepare for indicating the need of prepare, and also add the missing check of ep->need_setup at the set_params.
Fixes: 2be79d586454 ("ALSA: usb-audio: Split endpoint setups for hw_params and prepare (take#2)") Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.h | 3 ++- sound/usb/endpoint.c | 17 ++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/sound/usb/card.h b/sound/usb/card.h index ca75f2206170..40061550105a 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -129,7 +129,8 @@ struct snd_usb_endpoint { in a stream */ bool implicit_fb_sync; /* syncs with implicit feedback */ bool lowlatency_playback; /* low-latency playback mode */ - bool need_setup; /* (re-)need for configure? */ + bool need_setup; /* (re-)need for hw_params? */ + bool need_prepare; /* (re-)need for prepare? */
/* for hw constraints */ const struct audioformat *cur_audiofmt; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 44cce6cec9da..d0b8d61d1d22 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -824,6 +824,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip,
ep->implicit_fb_sync = fp->implicit_fb; ep->need_setup = true; + ep->need_prepare = true;
usb_audio_dbg(chip, " channels=%d, rate=%d, format=%s, period_bytes=%d, periods=%d, implicit_fb=%d\n", ep->cur_channels, ep->cur_rate, @@ -952,7 +953,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, /* Prepare for suspening EP, called from the main suspend handler */ void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep) { - ep->need_setup = true; + ep->need_prepare = true; if (ep->iface_ref) ep->iface_ref->need_setup = true; if (ep->clock_ref) @@ -1335,9 +1336,12 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep) { const struct audioformat *fmt = ep->cur_audiofmt; - int err; + int err = 0;
mutex_lock(&chip->mutex); + if (!ep->need_setup) + goto unlock; + /* release old buffers, if any */ err = release_urbs(ep, false); if (err < 0) @@ -1386,8 +1390,11 @@ int snd_usb_endpoint_set_params(struct snd_usb_audio *chip, ep->curframesize = ep->curpacksize / ep->cur_frame_bytes;
err = update_clock_ref_rate(chip, ep); - if (err >= 0) + if (err >= 0) { + ep->need_setup = false; err = 0; + } + unlock: mutex_unlock(&chip->mutex); return err; @@ -1437,7 +1444,7 @@ int snd_usb_endpoint_prepare(struct snd_usb_audio *chip, mutex_lock(&chip->mutex); if (WARN_ON(!ep->iface_ref)) goto unlock; - if (!ep->need_setup) + if (!ep->need_prepare) goto unlock;
/* If the interface has been already set up, just set EP parameters */ @@ -1491,7 +1498,7 @@ int snd_usb_endpoint_prepare(struct snd_usb_audio *chip, ep->iface_ref->need_setup = false;
done: - ep->need_setup = false; + ep->need_prepare = false; err = 1;
unlock:
participants (1)
-
Takashi Iwai