From f02f2f1bf9d154148325eb60d74bdf199022ea52 Mon Sep 17 00:00:00 2001 From: Len Baker Date: Sun, 19 Sep 2021 15:37:27 +0200 Subject: ALSA: usx2y: Prefer struct_size over open coded arithmetic As noted in the "Deprecated Interfaces, Language Features, Attributes, and Conventions" documentation [1], size calculations (especially multiplication) should not be performed in memory allocator (or similar) function arguments due to the risk of them overflowing. This could lead to values wrapping around and a smaller allocation being made than the caller was expecting. Using those allocations could lead to linear overflows of heap memory and other misbehaviors. In this case this is not actually dynamic size: all the operands involved in the calculation are constant values. However it is better to refactor this anyway, just to keep the open-coded math idiom out of code. So, use the struct_size() helper to do the arithmetic instead of the argument "size + size * count" in the kzalloc() function. Also, take the opportunity to refactor the declaration variables to make it more easy to read. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments Signed-off-by: Len Baker Link: https://lore.kernel.org/r/20210919133727.44694-1-len.baker@gmx.com Signed-off-by: Takashi Iwai --- sound/usb/usx2y/usbusx2yaudio.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/usx2y/usbusx2yaudio.c b/sound/usb/usx2y/usbusx2yaudio.c index c39cc6851e2d..cfc1ea53978d 100644 --- a/sound/usb/usx2y/usbusx2yaudio.c +++ b/sound/usb/usx2y/usbusx2yaudio.c @@ -668,14 +668,15 @@ static void i_usx2y_04int(struct urb *urb) static int usx2y_rate_set(struct usx2ydev *usx2y, int rate) { - int err = 0, i; - struct snd_usx2y_urb_seq *us = NULL; - int *usbdata = NULL; - const struct s_c2 *ra = rate == 48000 ? setrate_48000 : setrate_44100; + int err = 0, i; + struct snd_usx2y_urb_seq *us = NULL; + int *usbdata = NULL; + const struct s_c2 *ra = rate == 48000 ? setrate_48000 : setrate_44100; struct urb *urb; if (usx2y->rate != rate) { - us = kzalloc(sizeof(*us) + sizeof(struct urb *) * NOOF_SETRATE_URBS, GFP_KERNEL); + us = kzalloc(struct_size(us, urb, NOOF_SETRATE_URBS), + GFP_KERNEL); if (!us) { err = -ENOMEM; goto cleanup; -- cgit From 882e013a32ecdad7877bfb1669cd51ee052f3369 Mon Sep 17 00:00:00 2001 From: Geraldo Nascimento Date: Fri, 24 Sep 2021 23:33:51 -0300 Subject: ALSA: usb-audio: fix comment reference in __uac_clock_find_source snd_usb_find_clock_source and snd_usb_find_clock_selector are helper macros that look at an entity id and validate that this entity id is in fact a clock source or a clock selector. The present comments inside __uac_clock_find_source give the reader the impression we're looking for an entity id. We're looking for an entity id indeed, the clock source, but since __uac_clock_find_source is recursive, we're also looking *at* the entity ids, in the search for the one clock source. Fix the comment so we don't give readers a wrong idea. Signed-off-by: Geraldo Nascimento Link: https://lore.kernel.org/r/YU6Kj05oOqRmhJDf@geday Signed-off-by: Takashi Iwai --- sound/usb/clock.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 81d5ce07d548..7dd71d342443 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -271,7 +271,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, return -EINVAL; } - /* first, see if the ID we're looking for is a clock source already */ + /* first, see if the ID we're looking at is a clock source already */ source = snd_usb_find_clock_source(chip, entity_id, proto); if (source) { entity_id = GET_VAL(source, proto, bClockID); @@ -297,7 +297,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, goto find_source; } - /* the entity ID we are looking for is a selector. + /* the entity ID we are looking at is a selector. * find out what it currently selects */ ret = uac_clock_selector_get_val(chip, clock_id); if (ret < 0) { -- cgit From 4e7cf1fbb34ecb472c073980458cbe413afd4d64 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:36 +0200 Subject: ALSA: usb-audio: Restrict rates for the shared clocks When a single clock source is shared among several endpoints, we have to keep the same rate on all active endpoints as long as the clock is being used. For dealing with such a case, this patch adds one more check in the hw params constraint for the rate to take the shared clocks into account. The current rate is evaluated from the endpoint list that applies the same clock source. BugLink: https://bugzilla.suse.com/show_bug.cgi?id=1190418 Link: https://lore.kernel.org/r/20210929080844.11583-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 1 + sound/usb/endpoint.c | 21 +++++++++++++++++++++ sound/usb/endpoint.h | 1 + sound/usb/pcm.c | 9 +++++++++ 4 files changed, 32 insertions(+) (limited to 'sound/usb') diff --git a/sound/usb/card.h b/sound/usb/card.h index 5b19901f305a..3329ce710cb9 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -136,6 +136,7 @@ struct snd_usb_endpoint { unsigned int cur_period_frames; unsigned int cur_period_bytes; unsigned int cur_buffer_periods; + unsigned char cur_clock; spinlock_t lock; struct list_head list; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 533919a28856..29c4865966f5 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -722,6 +722,7 @@ snd_usb_endpoint_open(struct snd_usb_audio *chip, ep->cur_period_frames = params_period_size(params); ep->cur_period_bytes = ep->cur_period_frames * ep->cur_frame_bytes; ep->cur_buffer_periods = params_periods(params); + ep->cur_clock = fp->clock; if (ep->type == SND_USB_ENDPOINT_TYPE_SYNC) endpoint_set_syncinterval(chip, ep); @@ -833,6 +834,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, ep->altsetting = 0; ep->cur_audiofmt = NULL; ep->cur_rate = 0; + ep->cur_clock = 0; ep->iface_ref = NULL; usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num); } @@ -1340,6 +1342,25 @@ unlock: return err; } +/* get the current rate set to the given clock by any endpoint */ +int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock) +{ + struct snd_usb_endpoint *ep; + int rate = 0; + + if (!clock) + return 0; + mutex_lock(&chip->mutex); + list_for_each_entry(ep, &chip->ep_list, list) { + if (ep->cur_clock == clock && ep->cur_rate) { + rate = ep->cur_rate; + break; + } + } + mutex_unlock(&chip->mutex); + return rate; +} + /** * snd_usb_endpoint_start: start an snd_usb_endpoint * diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index a668f675b52b..a1099ec37e1c 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -19,6 +19,7 @@ void snd_usb_endpoint_close(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); int snd_usb_endpoint_configure(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep); +int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock); bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, struct snd_usb_endpoint *ep, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5dc9266180e3..19392117de9e 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -734,6 +734,7 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; + struct snd_usb_audio *chip = subs->stream->chip; const struct audioformat *fp; struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); unsigned int rmin, rmax, r; @@ -745,6 +746,14 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, list_for_each_entry(fp, &subs->fmt_list, list) { if (!hw_check_valid_format(subs, params, fp)) continue; + r = snd_usb_endpoint_get_clock_rate(chip, fp->clock); + if (r > 0) { + if (!snd_interval_test(it, r)) + continue; + rmin = min(rmin, r); + rmax = max(rmax, r); + continue; + } if (fp->rate_table && fp->nr_rates) { for (i = 0; i < fp->nr_rates; i++) { r = fp->rate_table[i]; -- cgit From 86a42ad07905110f82648853c0ea3434b4eab173 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:37 +0200 Subject: ALSA: usb-audio: Fix possible race at sync of urb completions USB-audio driver tries to sync with the clear of all pending URBs in wait_clear_urbs(), and it waits for all bits in active_mask getting cleared. This works fine for the normal operations, but when a stream is managed in the implicit feedback mode, there is still a very thin race window: namely, in snd_complete_usb(), the active_mask bit for the current URB is once cleared before re-submitted in queue_pending_output_urbs(). If wait_clear_urbs() is called during that period, it may pass the test and go forward even though there may be a still pending URB. For covering it, this patch adds a new counter to each endpoint to keep the number of in-flight URBs, and changes wait_clear_urbs() checking this number instead. The counter is decremented at the end of URB complete, hence the reference is kept as long as the URB complete is in process. Link: https://lore.kernel.org/r/20210929080844.11583-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 1 + sound/usb/endpoint.c | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) (limited to 'sound/usb') diff --git a/sound/usb/card.h b/sound/usb/card.h index 3329ce710cb9..746a765b2437 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -97,6 +97,7 @@ struct snd_usb_endpoint { unsigned int nominal_queue_size; /* total buffer sizes in URBs */ unsigned long active_mask; /* bitmask of active urbs */ unsigned long unlink_mask; /* bitmask of unlinked urbs */ + atomic_t submitted_urbs; /* currently submitted urbs */ char *syncbuf; /* sync buffer for all sync URBs */ dma_addr_t sync_dma; /* DMA address of syncbuf */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 29c4865966f5..06241568abf7 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -451,6 +451,7 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) } set_bit(ctx->index, &ep->active_mask); + atomic_inc(&ep->submitted_urbs); } } @@ -488,6 +489,7 @@ static void snd_complete_urb(struct urb *urb) clear_bit(ctx->index, &ep->active_mask); spin_unlock_irqrestore(&ep->lock, flags); queue_pending_output_urbs(ep); + atomic_dec(&ep->submitted_urbs); /* decrement at last */ return; } @@ -513,6 +515,7 @@ static void snd_complete_urb(struct urb *urb) exit_clear: clear_bit(ctx->index, &ep->active_mask); + atomic_dec(&ep->submitted_urbs); } /* @@ -596,6 +599,7 @@ int snd_usb_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type) ep->type = type; ep->ep_num = ep_num; INIT_LIST_HEAD(&ep->ready_playback_urbs); + atomic_set(&ep->submitted_urbs, 0); is_playback = ((ep_num & USB_ENDPOINT_DIR_MASK) == USB_DIR_OUT); ep_num &= USB_ENDPOINT_NUMBER_MASK; @@ -861,7 +865,7 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) return 0; do { - alive = bitmap_weight(&ep->active_mask, ep->nurbs); + alive = atomic_read(&ep->submitted_urbs); if (!alive) break; @@ -1441,6 +1445,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) goto __error; } set_bit(i, &ep->active_mask); + atomic_inc(&ep->submitted_urbs); } usb_audio_dbg(ep->chip, "%d URBs submitted for EP 0x%x\n", -- cgit From 9c9a3b9da891cc70405a544da6855700eddcbb71 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:38 +0200 Subject: ALSA: usb-audio: Rename early_playback_start flag with lowlatency_playback This is a preparation patch for the upcoming low-latency improvement changes. Rename early_playback_start flag with lowlatency_playback as it's more intuitive. The new flag is basically a reverse meaning. Along with the rename, factor out the code to set the flag to a function. This makes the complex condition checks simpler. Also, the same flag is introduced to snd_usb_endpoint, too, that is carried from the snd_usb_substream flag. Currently the endpoint flag isn't still referred, but will be used in later patches. Link: https://lore.kernel.org/r/20210929080844.11583-4-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 3 ++- sound/usb/endpoint.c | 4 ++++ sound/usb/pcm.c | 29 ++++++++++++++++++++--------- 3 files changed, 26 insertions(+), 10 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/card.h b/sound/usb/card.h index 746a765b2437..a00caa1db37e 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -126,6 +126,7 @@ struct snd_usb_endpoint { int skip_packets; /* quirks for devices to ignore the first n packets 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? */ /* for hw constraints */ @@ -190,7 +191,7 @@ struct snd_usb_substream { } dsd_dop; bool trigger_tstamp_pending_update; /* trigger timestamp being updated from initial estimate */ - bool early_playback_start; /* early start needed for playback? */ + bool lowlatency_playback; /* low-latency playback mode */ struct media_ctl *media_ctl; }; diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 06241568abf7..8e164d71d9ac 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -794,6 +794,10 @@ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, { ep->prepare_data_urb = prepare; ep->retire_data_urb = retire; + if (data_subs) + ep->lowlatency_playback = data_subs->lowlatency_playback; + else + ep->lowlatency_playback = false; WRITE_ONCE(ep->data_subs, data_subs); } diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 19392117de9e..4dd7f1c9e2af 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -581,6 +581,22 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) return 0; } +/* check whether early start is needed for playback stream */ +static int lowlatency_playback_available(struct snd_usb_substream *subs) +{ + struct snd_usb_audio *chip = subs->stream->chip; + + if (subs->direction == SNDRV_PCM_STREAM_CAPTURE) + return false; + /* disabled via module option? */ + if (!chip->lowlatency) + return false; + /* too short periods? */ + if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) + return false; + return true; +} + /* * prepare callback * @@ -614,13 +630,8 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) subs->period_elapsed_pending = 0; runtime->delay = 0; - /* check whether early start is needed for playback stream */ - subs->early_playback_start = - subs->direction == SNDRV_PCM_STREAM_PLAYBACK && - (!chip->lowlatency || - (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes)); - - if (subs->early_playback_start) + subs->lowlatency_playback = lowlatency_playback_available(subs); + if (!subs->lowlatency_playback) ret = start_endpoints(subs); unlock: @@ -1412,7 +1423,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->trigger_tstamp_pending_update = false; } - if (period_elapsed && !subs->running && !subs->early_playback_start) { + if (period_elapsed && !subs->running && subs->lowlatency_playback) { subs->period_elapsed_pending = 1; period_elapsed = 0; } @@ -1466,7 +1477,7 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea prepare_playback_urb, retire_playback_urb, subs); - if (!subs->early_playback_start && + if (subs->lowlatency_playback && cmd == SNDRV_PCM_TRIGGER_START) { err = start_endpoints(subs); if (err < 0) { -- cgit From e581f1cec4f899f788f6c9477f805b1d5fef25e2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:39 +0200 Subject: ALSA: usb-audio: Disable low-latency playback for free-wheel mode The free-wheel stream operation like dmix may not update the appl_ptr appropriately, and it doesn't fit with the low-latency playback mode. Disable the low-latency playback operation when the stream is set up in such a mode. Link: https://lore.kernel.org/r/20210929080844.11583-5-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 4dd7f1c9e2af..84b03a32ee23 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -582,7 +582,8 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) } /* check whether early start is needed for playback stream */ -static int lowlatency_playback_available(struct snd_usb_substream *subs) +static int lowlatency_playback_available(struct snd_pcm_runtime *runtime, + struct snd_usb_substream *subs) { struct snd_usb_audio *chip = subs->stream->chip; @@ -591,6 +592,9 @@ static int lowlatency_playback_available(struct snd_usb_substream *subs) /* disabled via module option? */ if (!chip->lowlatency) return false; + /* free-wheeling mode? (e.g. dmix) */ + if (runtime->stop_threshold > runtime->buffer_size) + return false; /* too short periods? */ if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) return false; @@ -630,7 +634,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) subs->period_elapsed_pending = 0; runtime->delay = 0; - subs->lowlatency_playback = lowlatency_playback_available(subs); + subs->lowlatency_playback = lowlatency_playback_available(runtime, subs); if (!subs->lowlatency_playback) ret = start_endpoints(subs); -- cgit From bceee75387554f682638e719d1ea60125ea78cea Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:40 +0200 Subject: ALSA: usb-audio: Disable low-latency mode for implicit feedback sync When a playback stream runs in the implicit feedback mode, its operation is passive and won't start unless the capture packet is received. This behavior contradicts with the low-latency playback mode, and we should turn off lowlatency_playback flag accordingly. In theory, we may take the low-latency mode when the playback-first quirk is set, but it still conflicts with the later operation with the fixed packet numbers, so it's disabled all together for now. Link: https://lore.kernel.org/r/20210929080844.11583-6-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'sound/usb') diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 84b03a32ee23..ec7eeb1b82b8 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -595,6 +595,9 @@ static int lowlatency_playback_available(struct snd_pcm_runtime *runtime, /* free-wheeling mode? (e.g. dmix) */ if (runtime->stop_threshold > runtime->buffer_size) return false; + /* implicit feedback mode has own operation mode */ + if (snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint)) + return false; /* too short periods? */ if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) return false; -- cgit From d215f63d49da9a8803af3e81acd6cad743686573 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:41 +0200 Subject: ALSA: usb-audio: Check available frames for the next packet size This is yet more preparation for the upcoming changes. Extend snd_usb_endpoint_next_packet_size() to check the available frames and return -EAGAIN if the next packet size is equal or exceeds the given size. This will be needed for avoiding XRUN during the low latency operation. As of this patch, avail=0 is passed, i.e. the check is skipped and no behavior change. Link: https://lore.kernel.org/r/20210929080844.11583-7-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 51 ++++++++++++++++++++++++++++++++++++--------------- sound/usb/endpoint.h | 3 ++- sound/usb/pcm.c | 2 +- 3 files changed, 39 insertions(+), 17 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 8e164d71d9ac..1f757a7eeafe 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -148,18 +148,23 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep) * This won't be used for implicit feedback which takes the packet size * returned from the sync source */ -static int slave_next_packet_size(struct snd_usb_endpoint *ep) +static int slave_next_packet_size(struct snd_usb_endpoint *ep, + unsigned int avail) { unsigned long flags; + unsigned int phase; int ret; if (ep->fill_max) return ep->maxframesize; spin_lock_irqsave(&ep->lock, flags); - ep->phase = (ep->phase & 0xffff) - + (ep->freqm << ep->datainterval); - ret = min(ep->phase >> 16, ep->maxframesize); + phase = (ep->phase & 0xffff) + (ep->freqm << ep->datainterval); + ret = min(phase >> 16, ep->maxframesize); + if (avail && ret >= avail) + ret = -EAGAIN; + else + ep->phase = phase; spin_unlock_irqrestore(&ep->lock, flags); return ret; @@ -169,20 +174,25 @@ static int slave_next_packet_size(struct snd_usb_endpoint *ep) * Return the number of samples to be sent in the next packet * for adaptive and synchronous endpoints */ -static int next_packet_size(struct snd_usb_endpoint *ep) +static int next_packet_size(struct snd_usb_endpoint *ep, unsigned int avail) { + unsigned int sample_accum; int ret; if (ep->fill_max) return ep->maxframesize; - ep->sample_accum += ep->sample_rem; - if (ep->sample_accum >= ep->pps) { - ep->sample_accum -= ep->pps; + sample_accum += ep->sample_rem; + if (sample_accum >= ep->pps) { + sample_accum -= ep->pps; ret = ep->packsize[1]; } else { ret = ep->packsize[0]; } + if (avail && ret >= avail) + ret = -EAGAIN; + else + ep->sample_accum = sample_accum; return ret; } @@ -190,16 +200,27 @@ static int next_packet_size(struct snd_usb_endpoint *ep) /* * snd_usb_endpoint_next_packet_size: Return the number of samples to be sent * in the next packet + * + * If the size is equal or exceeds @avail, don't proceed but return -EAGAIN + * Exception: @avail = 0 for skipping the check. */ int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *ctx, int idx) + struct snd_urb_ctx *ctx, int idx, + unsigned int avail) { - if (ctx->packet_size[idx]) - return ctx->packet_size[idx]; - else if (ep->sync_source) - return slave_next_packet_size(ep); + unsigned int packet; + + packet = ctx->packet_size[idx]; + if (packet) { + if (avail && packet >= avail) + return -EAGAIN; + return packet; + } + + if (ep->sync_source) + return slave_next_packet_size(ep, avail); else - return next_packet_size(ep); + return next_packet_size(ep, avail); } static void call_retire_callback(struct snd_usb_endpoint *ep, @@ -263,7 +284,7 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, unsigned int length; int counts; - counts = snd_usb_endpoint_next_packet_size(ep, ctx, i); + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, 0); length = counts * ep->stride; /* number of silent bytes */ offset = offs * ep->stride + extra * i; urb->iso_frame_desc[i].offset = offset; diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index a1099ec37e1c..1f1a72535a64 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -46,6 +46,7 @@ void snd_usb_endpoint_free_all(struct snd_usb_audio *chip); int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *ctx, int idx); + struct snd_urb_ctx *ctx, int idx, + unsigned int avail); #endif /* __USBAUDIO_ENDPOINT_H */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ec7eeb1b82b8..8ad48c35c559 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1365,7 +1365,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, spin_lock_irqsave(&subs->lock, flags); subs->frame_limit += ep->max_urb_frames; for (i = 0; i < ctx->packets; i++) { - counts = snd_usb_endpoint_next_packet_size(ep, ctx, i); + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, 0); /* set up descriptor */ urb->iso_frame_desc[i].offset = frames * stride; urb->iso_frame_desc[i].length = counts * stride; -- cgit From 0ef74366bc150dda4f53c546dfa6e8f7c707e087 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:42 +0200 Subject: ALSA: usb-audio: Add spinlock to stop_urbs() In theory, stop_urbs() may be called concurrently. Although we have the state check beforehand, it's safer to apply ep->lock during the critical list head manipulations. Link: https://lore.kernel.org/r/20210929080844.11583-8-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'sound/usb') diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 1f757a7eeafe..c32022672319 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -927,6 +927,7 @@ void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep) static int stop_urbs(struct snd_usb_endpoint *ep, bool force) { unsigned int i; + unsigned long flags; if (!force && atomic_read(&ep->running)) return -EBUSY; @@ -934,9 +935,11 @@ static int stop_urbs(struct snd_usb_endpoint *ep, bool force) if (!ep_state_update(ep, EP_STATE_RUNNING, EP_STATE_STOPPING)) return 0; + spin_lock_irqsave(&ep->lock, flags); INIT_LIST_HEAD(&ep->ready_playback_urbs); ep->next_packet_head = 0; ep->next_packet_queued = 0; + spin_unlock_irqrestore(&ep->lock, flags); for (i = 0; i < ep->nurbs; i++) { if (test_bit(i, &ep->active_mask)) { -- cgit From d5f871f89e21bb71827ea57bd484eedea85839a0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:43 +0200 Subject: ALSA: usb-audio: Improved lowlatency playback support This is another attempt to improve further the handling of playback stream in the low latency mode. The latest workaround in commit 4267c5a8f313 ("ALSA: usb-audio: Work around for XRUN with low latency playback") revealed that submitting URBs forcibly in advance may trigger XRUN easily. In the classical mode, this problem was avoided by practically delaying the submission of the actual data with the pre-submissions of silent data before triggering the stream start. But that is exactly what we want to avoid. Now, in this patch, instead of the previous workaround, we take a similar approach as used in the implicit feedback mode. The URBs are queued at the PCM trigger start like before, but we check whether the buffer has been already filled enough before each submission, and stop queuing if the data overcomes the threshold. The remaining URBs are kept in the ready list, and they will be retrieved in the URB complete callback of other (already queued) URBs. In the complete callback, we try to fill the data and submit as much as possible again. When there is no more available in-flight URBs that may handle the pending data, we'll check in PCM ack callback and submit and process URBs there in addition. In this way, the amount of in-flight URBs may vary dynamically and flexibly depending on the available data without hitting XRUN. The following things are changed to achieve the behavior above: * The endpoint prepare callback is changed to return an error code; when there is no enough data available, it may return -EAGAIN. Currently only prepare_playback_urb() returns the error. The evaluation of the available data is a bit messy here; we can't check with snd_pcm_avail() at the point of prepare callback (as runtime->status->hwptr hasn't been updated yet), hence we manually estimate the appl_ptr and compare with the internal hwptr_done to calculate the available frames. * snd_usb_endpoint_start() doesn't submit full URBs if the prepare callback returns -EAGAIN, and puts the remaining URBs to the ready list for the later submission. * snd_complete_urb() treats the URBs in the low-latency mode similarly like the implicit feedback mode, and submissions are done in (now exported) snd_usb_queue_pending_output_urbs(). * snd_usb_queue_pending_output_urbs() again checks the error value from the prepare callback. If it's -EAGAIN for the normal stream (i.e. not implicit feedback mode), we push it back to the ready list again. * PCM ack callback is introduced for the playback stream, and it calls snd_usb_queue_pending_output_urbs() if there is no in-flight URB while the stream is running. This corresponds to the case where the system needs the appl_ptr update for re-submitting a new URB. * snd_usb_queue_pending_output_urbs() and the prepare EP callback receive in_stream_lock argument, which is a bool flag indicating the call path from PCM ack. It's needed for avoiding the deadlock of snd_pcm_period_elapsed() calls. * Set the new SNDRV_PCM_INFO_EXPLICIT_SYNC flag when the new low-latency mode is deployed. This assures catching each applptr update even in the mmap mode. Fixes: 4267c5a8f313 ("ALSA: usb-audio: Work around for XRUN with low latency playback") Link: https://lore.kernel.org/r/20210929080844.11583-9-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/card.h | 6 +-- sound/usb/endpoint.c | 130 ++++++++++++++++++++++++++++++++++----------------- sound/usb/endpoint.h | 7 ++- sound/usb/pcm.c | 102 +++++++++++++++++++++++++++++++--------- 4 files changed, 177 insertions(+), 68 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/card.h b/sound/usb/card.h index a00caa1db37e..87f042d06ce0 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -74,8 +74,9 @@ struct snd_usb_endpoint { atomic_t state; /* running state */ - void (*prepare_data_urb) (struct snd_usb_substream *subs, - struct urb *urb); + int (*prepare_data_urb) (struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock); void (*retire_data_urb) (struct snd_usb_substream *subs, struct urb *urb); @@ -94,7 +95,6 @@ struct snd_usb_endpoint { struct list_head ready_playback_urbs; /* playback URB FIFO for implicit fb */ unsigned int nurbs; /* # urbs */ - unsigned int nominal_queue_size; /* total buffer sizes in URBs */ unsigned long active_mask; /* bitmask of active urbs */ unsigned long unlink_mask; /* bitmask of unlinked urbs */ atomic_t submitted_urbs; /* currently submitted urbs */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index c32022672319..0b336876e36d 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -307,8 +307,9 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, /* * Prepare a PLAYBACK urb for submission to the bus. */ -static void prepare_outbound_urb(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *ctx) +static int prepare_outbound_urb(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx, + bool in_stream_lock) { struct urb *urb = ctx->urb; unsigned char *cp = urb->transfer_buffer; @@ -320,9 +321,9 @@ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, case SND_USB_ENDPOINT_TYPE_DATA: data_subs = READ_ONCE(ep->data_subs); if (data_subs && ep->prepare_data_urb) - ep->prepare_data_urb(data_subs, urb); - else /* no data provider, so send silence */ - prepare_silent_urb(ep, ctx); + return ep->prepare_data_urb(data_subs, urb, in_stream_lock); + /* no data provider, so send silence */ + prepare_silent_urb(ep, ctx); break; case SND_USB_ENDPOINT_TYPE_SYNC: @@ -351,13 +352,14 @@ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, break; } + return 0; } /* * Prepare a CAPTURE or SYNC urb for submission to the bus. */ -static inline void prepare_inbound_urb(struct snd_usb_endpoint *ep, - struct snd_urb_ctx *urb_ctx) +static int prepare_inbound_urb(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *urb_ctx) { int i, offs; struct urb *urb = urb_ctx->urb; @@ -382,6 +384,7 @@ static inline void prepare_inbound_urb(struct snd_usb_endpoint *ep, urb->iso_frame_desc[0].offset = 0; break; } + return 0; } /* notify an error as XRUN to the assigned PCM data substream */ @@ -417,6 +420,16 @@ next_packet_fifo_dequeue(struct snd_usb_endpoint *ep) return p; } +static void push_back_to_ready_list(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx) +{ + unsigned long flags; + + spin_lock_irqsave(&ep->lock, flags); + list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); + spin_unlock_irqrestore(&ep->lock, flags); +} + /* * Send output urbs that have been prepared previously. URBs are dequeued * from ep->ready_playback_urbs and in case there aren't any available @@ -427,12 +440,14 @@ next_packet_fifo_dequeue(struct snd_usb_endpoint *ep) * is that host controllers don't guarantee the order in which they return * inbound and outbound packets to their submitters. * - * This function is only used for implicit feedback endpoints. For endpoints - * driven by dedicated sync endpoints, URBs are immediately re-submitted - * from their completion handler. + * This function is used both for implicit feedback endpoints and in low- + * latency playback mode. */ -static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) +void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, + bool in_stream_lock) { + bool implicit_fb = snd_usb_endpoint_implicit_feedback_sink(ep); + while (ep_state_running(ep)) { unsigned long flags; @@ -441,14 +456,14 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) int err, i; spin_lock_irqsave(&ep->lock, flags); - if (ep->next_packet_queued > 0 && + if ((!implicit_fb || ep->next_packet_queued > 0) && !list_empty(&ep->ready_playback_urbs)) { /* take URB out of FIFO */ ctx = list_first_entry(&ep->ready_playback_urbs, struct snd_urb_ctx, ready_list); list_del_init(&ctx->ready_list); - - packet = next_packet_fifo_dequeue(ep); + if (implicit_fb) + packet = next_packet_fifo_dequeue(ep); } spin_unlock_irqrestore(&ep->lock, flags); @@ -456,11 +471,24 @@ static void queue_pending_output_urbs(struct snd_usb_endpoint *ep) return; /* copy over the length information */ - for (i = 0; i < packet->packets; i++) - ctx->packet_size[i] = packet->packet_size[i]; + if (implicit_fb) { + for (i = 0; i < packet->packets; i++) + ctx->packet_size[i] = packet->packet_size[i]; + } /* call the data handler to fill in playback data */ - prepare_outbound_urb(ep, ctx); + err = prepare_outbound_urb(ep, ctx, in_stream_lock); + /* can be stopped during prepare callback */ + if (unlikely(!ep_state_running(ep))) + break; + if (err < 0) { + /* push back to ready list again for -EAGAIN */ + if (err == -EAGAIN) + push_back_to_ready_list(ep, ctx); + else + notify_xrun(ep); + return; + } err = usb_submit_urb(ctx->urb, GFP_ATOMIC); if (err < 0) { @@ -483,7 +511,6 @@ static void snd_complete_urb(struct urb *urb) { struct snd_urb_ctx *ctx = urb->context; struct snd_usb_endpoint *ep = ctx->ep; - unsigned long flags; int err; if (unlikely(urb->status == -ENOENT || /* unlinked */ @@ -504,17 +531,20 @@ static void snd_complete_urb(struct urb *urb) if (unlikely(!ep_state_running(ep))) goto exit_clear; - if (snd_usb_endpoint_implicit_feedback_sink(ep)) { - spin_lock_irqsave(&ep->lock, flags); - list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); + /* in low-latency and implicit-feedback modes, push back the + * URB to ready list at first, then process as much as possible + */ + if (ep->lowlatency_playback || + snd_usb_endpoint_implicit_feedback_sink(ep)) { + push_back_to_ready_list(ep, ctx); clear_bit(ctx->index, &ep->active_mask); - spin_unlock_irqrestore(&ep->lock, flags); - queue_pending_output_urbs(ep); + snd_usb_queue_pending_output_urbs(ep, false); atomic_dec(&ep->submitted_urbs); /* decrement at last */ return; } - prepare_outbound_urb(ep, ctx); + /* in non-lowlatency mode, no error handling for prepare */ + prepare_outbound_urb(ep, ctx, false); /* can be stopped during prepare callback */ if (unlikely(!ep_state_running(ep))) goto exit_clear; @@ -807,8 +837,9 @@ void snd_usb_endpoint_set_sync(struct snd_usb_audio *chip, * Pass NULL to deactivate each callback. */ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, - void (*prepare)(struct snd_usb_substream *subs, - struct urb *urb), + int (*prepare)(struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock), void (*retire)(struct snd_usb_substream *subs, struct urb *urb), struct snd_usb_substream *data_subs) @@ -1166,10 +1197,6 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep) INIT_LIST_HEAD(&u->ready_list); } - /* total buffer bytes of all URBs plus the next queue; - * referred in pcm.c - */ - ep->nominal_queue_size = maxsize * urb_packs * (ep->nurbs + 1); return 0; out_of_memory: @@ -1408,6 +1435,7 @@ int snd_usb_endpoint_get_clock_rate(struct snd_usb_audio *chip, int clock) */ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) { + bool is_playback = usb_pipeout(ep->pipe); int err; unsigned int i; @@ -1444,13 +1472,9 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (snd_usb_endpoint_implicit_feedback_sink(ep) && !(ep->chip->quirk_flags & QUIRK_FLAG_PLAYBACK_FIRST)) { - for (i = 0; i < ep->nurbs; i++) { - struct snd_urb_ctx *ctx = ep->urb + i; - list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs); - } - usb_audio_dbg(ep->chip, "No URB submission due to implicit fb sync\n"); - return 0; + i = 0; + goto fill_rest; } for (i = 0; i < ep->nurbs; i++) { @@ -1459,10 +1483,18 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (snd_BUG_ON(!urb)) goto __error; - if (usb_pipeout(ep->pipe)) { - prepare_outbound_urb(ep, urb->context); - } else { - prepare_inbound_urb(ep, urb->context); + if (is_playback) + err = prepare_outbound_urb(ep, urb->context, true); + else + err = prepare_inbound_urb(ep, urb->context); + if (err < 0) { + /* stop filling at applptr */ + if (err == -EAGAIN) + break; + usb_audio_dbg(ep->chip, + "EP 0x%x: failed to prepare urb: %d\n", + ep->ep_num, err); + goto __error; } err = usb_submit_urb(urb, GFP_ATOMIC); @@ -1476,8 +1508,22 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) atomic_inc(&ep->submitted_urbs); } + if (!i) { + usb_audio_dbg(ep->chip, "XRUN at starting EP 0x%x\n", + ep->ep_num); + goto __error; + } + usb_audio_dbg(ep->chip, "%d URBs submitted for EP 0x%x\n", - ep->nurbs, ep->ep_num); + i, ep->ep_num); + + fill_rest: + /* put the remaining URBs to ready list */ + if (is_playback) { + for (; i < ep->nurbs; i++) + push_back_to_ready_list(ep, ep->urb + i); + } + return 0; __error: @@ -1629,7 +1675,7 @@ static void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, } spin_unlock_irqrestore(&ep->lock, flags); - queue_pending_output_urbs(ep); + snd_usb_queue_pending_output_urbs(ep, false); return; } diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 1f1a72535a64..6895d50d14d1 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -30,8 +30,9 @@ void snd_usb_endpoint_set_sync(struct snd_usb_audio *chip, struct snd_usb_endpoint *data_ep, struct snd_usb_endpoint *sync_ep); void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, - void (*prepare)(struct snd_usb_substream *subs, - struct urb *urb), + int (*prepare)(struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock), void (*retire)(struct snd_usb_substream *subs, struct urb *urb), struct snd_usb_substream *data_subs); @@ -48,5 +49,7 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, struct snd_urb_ctx *ctx, int idx, unsigned int avail); +void snd_usb_queue_pending_output_urbs(struct snd_usb_endpoint *ep, + bool in_stream_lock); #endif /* __USBAUDIO_ENDPOINT_H */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 8ad48c35c559..d5a14e5b9ad3 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -598,9 +598,6 @@ static int lowlatency_playback_available(struct snd_pcm_runtime *runtime, /* implicit feedback mode has own operation mode */ if (snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint)) return false; - /* too short periods? */ - if (subs->data_endpoint->nominal_queue_size >= subs->buffer_bytes) - return false; return true; } @@ -1095,6 +1092,10 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream) int ret; runtime->hw = snd_usb_hardware; + /* need an explicit sync to catch applptr update in low-latency mode */ + if (direction == SNDRV_PCM_STREAM_PLAYBACK && + as->chip->lowlatency) + runtime->hw.info |= SNDRV_PCM_INFO_EXPLICIT_SYNC; runtime->private_data = subs; subs->pcm_substream = substream; /* runtime PM is also done there */ @@ -1347,44 +1348,66 @@ static unsigned int copy_to_urb_quirk(struct snd_usb_substream *subs, return bytes; } -static void prepare_playback_urb(struct snd_usb_substream *subs, - struct urb *urb) +static int prepare_playback_urb(struct snd_usb_substream *subs, + struct urb *urb, + bool in_stream_lock) { struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime; struct snd_usb_endpoint *ep = subs->data_endpoint; struct snd_urb_ctx *ctx = urb->context; - unsigned int counts, frames, bytes; + unsigned int frames, bytes; + int counts; + unsigned int transfer_done, frame_limit, avail = 0; int i, stride, period_elapsed = 0; unsigned long flags; + int err = 0; stride = ep->stride; frames = 0; ctx->queued = 0; urb->number_of_packets = 0; + spin_lock_irqsave(&subs->lock, flags); - subs->frame_limit += ep->max_urb_frames; + frame_limit = subs->frame_limit + ep->max_urb_frames; + transfer_done = subs->transfer_done; + + if (subs->lowlatency_playback && + runtime->status->state != SNDRV_PCM_STATE_DRAINING) { + unsigned int hwptr = subs->hwptr_done / stride; + + /* calculate the byte offset-in-buffer of the appl_ptr */ + avail = (runtime->control->appl_ptr - runtime->hw_ptr_base) + % runtime->buffer_size; + if (avail <= hwptr) + avail += runtime->buffer_size; + avail -= hwptr; + } + for (i = 0; i < ctx->packets; i++) { - counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, 0); + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i, avail); + if (counts < 0) + break; /* set up descriptor */ urb->iso_frame_desc[i].offset = frames * stride; urb->iso_frame_desc[i].length = counts * stride; frames += counts; + avail -= counts; urb->number_of_packets++; - subs->transfer_done += counts; - if (subs->transfer_done >= runtime->period_size) { - subs->transfer_done -= runtime->period_size; - subs->frame_limit = 0; + transfer_done += counts; + if (transfer_done >= runtime->period_size) { + transfer_done -= runtime->period_size; + frame_limit = 0; period_elapsed = 1; if (subs->fmt_type == UAC_FORMAT_TYPE_II) { - if (subs->transfer_done > 0) { + if (transfer_done > 0) { /* FIXME: fill-max mode is not * supported yet */ - frames -= subs->transfer_done; - counts -= subs->transfer_done; + frames -= transfer_done; + counts -= transfer_done; urb->iso_frame_desc[i].length = counts * stride; - subs->transfer_done = 0; + transfer_done = 0; } i++; if (i < ctx->packets) { @@ -1398,13 +1421,19 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, } } /* finish at the period boundary or after enough frames */ - if ((period_elapsed || - subs->transfer_done >= subs->frame_limit) && + if ((period_elapsed || transfer_done >= frame_limit) && !snd_usb_endpoint_implicit_feedback_sink(ep)) break; } - bytes = frames * stride; + if (!frames) { + err = -EAGAIN; + goto unlock; + } + + bytes = frames * stride; + subs->transfer_done = transfer_done; + subs->frame_limit = frame_limit; if (unlikely(ep->cur_format == SNDRV_PCM_FORMAT_DSD_U16_LE && subs->cur_audiofmt->dsd_dop)) { fill_playback_urb_dsd_dop(subs, urb, bytes); @@ -1434,10 +1463,19 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->period_elapsed_pending = 1; period_elapsed = 0; } + + unlock: spin_unlock_irqrestore(&subs->lock, flags); + if (err < 0) + return err; urb->transfer_buffer_length = bytes; - if (period_elapsed) - snd_pcm_period_elapsed(subs->pcm_substream); + if (period_elapsed) { + if (in_stream_lock) + snd_pcm_period_elapsed_under_stream_lock(subs->pcm_substream); + else + snd_pcm_period_elapsed(subs->pcm_substream); + } + return 0; } /* @@ -1469,6 +1507,27 @@ static void retire_playback_urb(struct snd_usb_substream *subs, snd_pcm_period_elapsed(subs->pcm_substream); } +/* PCM ack callback for the playback stream; + * this plays a role only when the stream is running in low-latency mode. + */ +static int snd_usb_pcm_playback_ack(struct snd_pcm_substream *substream) +{ + struct snd_usb_substream *subs = substream->runtime->private_data; + struct snd_usb_endpoint *ep; + + if (!subs->lowlatency_playback || !subs->running) + return 0; + ep = subs->data_endpoint; + if (!ep) + return 0; + /* When no more in-flight URBs available, try to process the pending + * outputs here + */ + if (!ep->active_mask) + snd_usb_queue_pending_output_urbs(ep, true); + return 0; +} + static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int cmd) { @@ -1572,6 +1631,7 @@ static const struct snd_pcm_ops snd_usb_playback_ops = { .trigger = snd_usb_substream_playback_trigger, .sync_stop = snd_usb_pcm_sync_stop, .pointer = snd_usb_pcm_pointer, + .ack = snd_usb_pcm_playback_ack, }; static const struct snd_pcm_ops snd_usb_capture_ops = { -- cgit From 813a17cab9b708bbb1e0db8902e19857b57196ec Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 29 Sep 2021 10:08:44 +0200 Subject: ALSA: usb-audio: Avoid killing in-flight URBs during draining While draining a stream, ALSA PCM core stops the stream by issuing snd_pcm_stop() after all data has been sent out. And, at PCM trigger stop, currently USB-audio driver kills the in-flight URBs explicitly, then at sync-stop ops, sync with the finish of all remaining URBs. This might result in a drop of the drained samples as most of USB-audio devices / hosts allow relatively long in-flight samples (as a sort of FIFO). For avoiding the trimming, this patch changes the stream-stop behavior during PCM draining state. Under that condition, the pending URBs won't be killed. The leftover in-flight URBs are caught by the sync-stop operation that shall be performed after the trigger-stop operation. Link: https://lore.kernel.org/r/20210929080844.11583-10-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 14 +++++++++----- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 16 ++++++++-------- 3 files changed, 18 insertions(+), 14 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0b336876e36d..42c0d2db8ba8 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -955,7 +955,7 @@ void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep) * * This function moves the EP to STOPPING state if it's being RUNNING. */ -static int stop_urbs(struct snd_usb_endpoint *ep, bool force) +static int stop_urbs(struct snd_usb_endpoint *ep, bool force, bool keep_pending) { unsigned int i; unsigned long flags; @@ -972,6 +972,9 @@ static int stop_urbs(struct snd_usb_endpoint *ep, bool force) ep->next_packet_queued = 0; spin_unlock_irqrestore(&ep->lock, flags); + if (keep_pending) + return 0; + for (i = 0; i < ep->nurbs; i++) { if (test_bit(i, &ep->active_mask)) { if (!test_and_set_bit(i, &ep->unlink_mask)) { @@ -995,7 +998,7 @@ static int release_urbs(struct snd_usb_endpoint *ep, bool force) snd_usb_endpoint_set_callback(ep, NULL, NULL, NULL); /* stop and unlink urbs */ - err = stop_urbs(ep, force); + err = stop_urbs(ep, force, false); if (err) return err; @@ -1527,7 +1530,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) return 0; __error: - snd_usb_endpoint_stop(ep); + snd_usb_endpoint_stop(ep, false); return -EPIPE; } @@ -1535,6 +1538,7 @@ __error: * snd_usb_endpoint_stop: stop an snd_usb_endpoint * * @ep: the endpoint to stop (may be NULL) + * @keep_pending: keep in-flight URBs * * A call to this function will decrement the running count of the endpoint. * In case the last user has requested the endpoint stop, the URBs will @@ -1545,7 +1549,7 @@ __error: * The caller needs to synchronize the pending stop operation via * snd_usb_endpoint_sync_pending_stop(). */ -void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) +void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending) { if (!ep) return; @@ -1560,7 +1564,7 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) if (!atomic_dec_return(&ep->running)) { if (ep->sync_source) WRITE_ONCE(ep->sync_source->sync_sink, NULL); - stop_urbs(ep, false); + stop_urbs(ep, false, keep_pending); } } diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6895d50d14d1..6a9af04cf175 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -38,7 +38,7 @@ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, struct snd_usb_substream *data_subs); int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); -void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep); +void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, bool keep_pending); void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep); int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index d5a14e5b9ad3..f09c7380a923 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -219,16 +219,16 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, return 0; } -static bool stop_endpoints(struct snd_usb_substream *subs) +static bool stop_endpoints(struct snd_usb_substream *subs, bool keep_pending) { bool stopped = 0; if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { - snd_usb_endpoint_stop(subs->sync_endpoint); + snd_usb_endpoint_stop(subs->sync_endpoint, keep_pending); stopped = true; } if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) { - snd_usb_endpoint_stop(subs->data_endpoint); + snd_usb_endpoint_stop(subs->data_endpoint, keep_pending); stopped = true; } return stopped; @@ -261,7 +261,7 @@ static int start_endpoints(struct snd_usb_substream *subs) return 0; error: - stop_endpoints(subs); + stop_endpoints(subs, false); return err; } @@ -437,7 +437,7 @@ static int configure_endpoints(struct snd_usb_audio *chip, if (subs->data_endpoint->need_setup) { /* stop any running stream beforehand */ - if (stop_endpoints(subs)) + if (stop_endpoints(subs, false)) sync_pending_stops(subs); err = snd_usb_endpoint_configure(chip, subs->data_endpoint); if (err < 0) @@ -572,7 +572,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) subs->cur_audiofmt = NULL; mutex_unlock(&chip->mutex); if (!snd_usb_lock_shutdown(chip)) { - if (stop_endpoints(subs)) + if (stop_endpoints(subs, false)) sync_pending_stops(subs); close_endpoints(chip, subs); snd_usb_unlock_shutdown(chip); @@ -1559,7 +1559,7 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea return 0; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - stop_endpoints(subs); + stop_endpoints(subs, substream->runtime->status->state == SNDRV_PCM_STATE_DRAINING); snd_usb_endpoint_set_callback(subs->data_endpoint, NULL, NULL, NULL); subs->running = 0; @@ -1607,7 +1607,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream return 0; case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_STOP: - stop_endpoints(subs); + stop_endpoints(subs, false); fallthrough; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: snd_usb_endpoint_set_callback(subs->data_endpoint, -- cgit From 28c369e60827f706cef4604a3e2848198f25bd26 Mon Sep 17 00:00:00 2001 From: Geraldo Nascimento Date: Thu, 30 Sep 2021 16:40:14 -0300 Subject: ALSA: usb-audio: disable implicit feedback sync for Behringer UFX1204 and UFX1604 Behringer UFX1204 and UFX1604 have Synchronous endpoints to which current ALSA code applies implicit feedback sync as if they were Asynchronous endpoints. This breaks UAC compliance and is unneeded. The commit 5e35dc0338d85ccebacf3f77eca1e5dea73155e8 and subsequent 1a15718b41df026cffd0e42cfdc38a1384ce19f9 were meant to clear up noise. Unfortunately, noise persisted for those using higher sample rates and this was only solved by commit d2e8f641257d0d3af6e45d6ac2d6f9d56b8ea964 Since there are no more reports of noise, let's get rid of the implicit-fb quirks breaking UAC compliance. Signed-off-by: Geraldo Nascimento Link: https://lore.kernel.org/r/YVYSnoQ7nxLXT0Dq@geday Signed-off-by: Takashi Iwai --- sound/usb/implicit.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 23767a14d126..70319c822c10 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -54,8 +54,6 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = { /* Fixed EP */ /* FIXME: check the availability of generic matching */ - IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer UFX1604 */ - IMPLICIT_FB_FIXED_DEV(0x1397, 0x0002, 0x81, 1), /* Behringer UFX1204 */ IMPLICIT_FB_FIXED_DEV(0x2466, 0x8010, 0x81, 2), /* Fractal Audio Axe-Fx III */ IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0001, 0x81, 2), /* Solid State Logic SSL2 */ IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0002, 0x81, 2), /* Solid State Logic SSL2+ */ -- cgit From 23939115be181bc5dbc33aa8471adcdbffa28910 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 1 Oct 2021 12:54:25 +0200 Subject: ALSA: usb-audio: Fix packet size calculation regression The commit d215f63d49da ("ALSA: usb-audio: Check available frames for the next packet size") introduced the available frame size check, but the conversion forgot to initialize the temporary variable properly, and it resulted in a bogus calculation. This patch fixes it. Fixes: d215f63d49da ("ALSA: usb-audio: Check available frames for the next packet size") Reported-by: Colin Ian King Link: https://lore.kernel.org/r/20211001104417.14291-1-colin.king@canonical.com Link: https://lore.kernel.org/r/20211001105425.16191-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/usb') diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 42c0d2db8ba8..743b8287cfcd 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -182,7 +182,7 @@ static int next_packet_size(struct snd_usb_endpoint *ep, unsigned int avail) if (ep->fill_max) return ep->maxframesize; - sample_accum += ep->sample_rem; + sample_accum = ep->sample_accum + ep->sample_rem; if (sample_accum >= ep->pps) { sample_accum -= ep->pps; ret = ep->packsize[1]; -- cgit From 59d7f5f6ddbc23f6ca4a0523c85dc18f027c80d9 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 7 Oct 2021 10:35:28 +0200 Subject: ALSA: usb-audio: Pass JOINT_DUPLEX info flag for implicit fb streams When a stream is in the implicit feedback mode, it's more or less tied with a capture stream. Passing SNDRV_PCM_INFO_JOINT_DUPLEX may help for user-space to understand the situation. Link: https://lore.kernel.org/r/20211007083528.4184-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'sound/usb') diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index f09c7380a923..3bb095a3c9b6 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1080,6 +1080,13 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre if (err < 0) return err; + list_for_each_entry(fp, &subs->fmt_list, list) { + if (fp->implicit_fb) { + runtime->hw.info |= SNDRV_PCM_INFO_JOINT_DUPLEX; + break; + } + } + return 0; } -- cgit From 6d27788160362a7ee6c0d317636fe4b1ddbe59a7 Mon Sep 17 00:00:00 2001 From: William Overton Date: Sun, 10 Oct 2021 15:58:41 +0100 Subject: ALSA: usb-audio: Add support for the Pioneer DJM 750MK2 Mixer/Soundcard The kernel already has support for very similar Pioneer djm products and this work is based on that. Added device to quirks-table.h and added control info to mixer_quirks.c. Tested on my hardware and all working. Signed-off-by: William Overton Link: https://lore.kernel.org/r/20211010145841.11907-1-willovertonuk@gmail.com Signed-off-by: Takashi Iwai --- sound/usb/mixer_quirks.c | 34 ++++++++++++++++++++++++++++ sound/usb/quirks-table.h | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+) (limited to 'sound/usb') diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 46082dc57be0..d489c1de3bae 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -2795,6 +2795,7 @@ static int snd_bbfpro_controls_create(struct usb_mixer_interface *mixer) #define SND_DJM_750_IDX 0x1 #define SND_DJM_850_IDX 0x2 #define SND_DJM_900NXS2_IDX 0x3 +#define SND_DJM_750MK2_IDX 0x4 #define SND_DJM_CTL(_name, suffix, _default_value, _windex) { \ @@ -2984,10 +2985,40 @@ static const struct snd_djm_ctl snd_djm_ctls_900nxs2[] = { SND_DJM_CTL("Ch5 Input", 900nxs2_cap5, 3, SND_DJM_WINDEX_CAP) }; +// DJM-750MK2 +static const u16 snd_djm_opts_750mk2_cap1[] = { + 0x0100, 0x0102, 0x0103, 0x0106, 0x0107, 0x0108, 0x0109, 0x010a }; +static const u16 snd_djm_opts_750mk2_cap2[] = { + 0x0200, 0x0202, 0x0203, 0x0206, 0x0207, 0x0208, 0x0209, 0x020a }; +static const u16 snd_djm_opts_750mk2_cap3[] = { + 0x0300, 0x0302, 0x0303, 0x0306, 0x0307, 0x0308, 0x0309, 0x030a }; +static const u16 snd_djm_opts_750mk2_cap4[] = { + 0x0400, 0x0402, 0x0403, 0x0406, 0x0407, 0x0408, 0x0409, 0x040a }; +static const u16 snd_djm_opts_750mk2_cap5[] = { + 0x0507, 0x0508, 0x0509, 0x050a, 0x0511, 0x0512, 0x0513, 0x0514 }; + +static const u16 snd_djm_opts_750mk2_pb1[] = { 0x0100, 0x0101, 0x0104 }; +static const u16 snd_djm_opts_750mk2_pb2[] = { 0x0200, 0x0201, 0x0204 }; +static const u16 snd_djm_opts_750mk2_pb3[] = { 0x0300, 0x0301, 0x0304 }; + + +static const struct snd_djm_ctl snd_djm_ctls_750mk2[] = { + SND_DJM_CTL("Capture Level", cap_level, 0, SND_DJM_WINDEX_CAPLVL), + SND_DJM_CTL("Ch1 Input", 750mk2_cap1, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch2 Input", 750mk2_cap2, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch3 Input", 750mk2_cap3, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch4 Input", 750mk2_cap4, 2, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch5 Input", 750mk2_cap5, 3, SND_DJM_WINDEX_CAP), + SND_DJM_CTL("Ch1 Output", 750mk2_pb1, 0, SND_DJM_WINDEX_PB), + SND_DJM_CTL("Ch2 Output", 750mk2_pb2, 1, SND_DJM_WINDEX_PB), + SND_DJM_CTL("Ch3 Output", 750mk2_pb3, 2, SND_DJM_WINDEX_PB) +}; + static const struct snd_djm_device snd_djm_devices[] = { SND_DJM_DEVICE(250mk2), SND_DJM_DEVICE(750), + SND_DJM_DEVICE(750mk2), SND_DJM_DEVICE(850), SND_DJM_DEVICE(900nxs2) }; @@ -3235,6 +3266,9 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) case USB_ID(0x08e4, 0x017f): /* Pioneer DJ DJM-750 */ err = snd_djm_controls_create(mixer, SND_DJM_750_IDX); break; + case USB_ID(0x2b73, 0x001b): /* Pioneer DJ DJM-750MK2 */ + err = snd_djm_controls_create(mixer, SND_DJM_750MK2_IDX); + break; case USB_ID(0x08e4, 0x0163): /* Pioneer DJ DJM-850 */ err = snd_djm_controls_create(mixer, SND_DJM_850_IDX); break; diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index e03043f7dad3..bc0116273e94 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3850,6 +3850,64 @@ YAMAHA_DEVICE(0x7010, "UB99"), } } }, +{ + /* + * Pioneer DJ DJM-750MK2 + * 10 channels playback & 12 channels capture @ 48kHz S24LE + */ + USB_DEVICE_VENDOR_SPEC(0x2b73, 0x001b), + .driver_info = (unsigned long) &(const struct snd_usb_audio_quirk) { + .ifnum = QUIRK_ANY_INTERFACE, + .type = QUIRK_COMPOSITE, + .data = (const struct snd_usb_audio_quirk[]) { + { + .ifnum = 0, + .type = QUIRK_AUDIO_FIXED_ENDPOINT, + .data = &(const struct audioformat) { + .formats = SNDRV_PCM_FMTBIT_S24_3LE, + .channels = 10, + .iface = 0, + .altsetting = 1, + .altset_idx = 1, + .endpoint = 0x01, + .ep_attr = USB_ENDPOINT_XFER_ISOC| + USB_ENDPOINT_SYNC_ASYNC, + .rates = SNDRV_PCM_RATE_48000, + .rate_min = 48000, + .rate_max = 48000, + .nr_rates = 1, + .rate_table = (unsigned int[]) { + 48000 + } + } + }, + { + .ifnum = 0, + .type = QUIRK_AUDIO_FIXED_ENDPOINT, + .data = &(const struct audioformat) { + .formats = SNDRV_PCM_FMTBIT_S24_3LE, + .channels = 12, + .iface = 0, + .altsetting = 1, + .altset_idx = 1, + .endpoint = 0x82, + .ep_idx = 1, + .ep_attr = USB_ENDPOINT_XFER_ISOC| + USB_ENDPOINT_SYNC_ASYNC| + USB_ENDPOINT_USAGE_IMPLICIT_FB, + .rates = SNDRV_PCM_RATE_48000, + .rate_min = 48000, + .rate_max = 48000, + .nr_rates = 1, + .rate_table = (unsigned int[]) { 48000 } + } + }, + { + .ifnum = -1 + } + } + } +}, { /* * Pioneer DJ DJM-850 -- cgit From 53451b6da8271905941eb1eb369db152c4bd92f2 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 11 Oct 2021 12:36:50 +0200 Subject: ALSA: usb-audio: Less restriction for low-latency playback mode The recent support for the improved low-latency playback mode applied the SNDRV_PCM_INFO_EXPLICIT_SYNC flag for the target streams, but this was a slight overkill. The use of the flag above disables effectively both PCM status and control mmaps, while basically what we want to track is only about the appl_ptr update. For less restriction, use a more proper flag, SNDRV_PCM_INFO_SYNC_APPLPTR instead, which disables only the control mmap. Fixes: d5f871f89e21 ("ALSA: usb-audio: Improved lowlatency playback support") Link: https://lore.kernel.org/r/20211011103650.10182-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/pcm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/usb') diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 3bb095a3c9b6..95ec8eec1bb0 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1102,7 +1102,7 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream) /* need an explicit sync to catch applptr update in low-latency mode */ if (direction == SNDRV_PCM_STREAM_PLAYBACK && as->chip->lowlatency) - runtime->hw.info |= SNDRV_PCM_INFO_EXPLICIT_SYNC; + runtime->hw.info |= SNDRV_PCM_INFO_SYNC_APPLPTR; runtime->private_data = subs; subs->pcm_substream = substream; /* runtime PM is also done there */ -- cgit From ac9b019d07ee1666e87f6832b1bde7e71618c4b0 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 15:06:34 +0200 Subject: ALSA: usb-audio: Downgrade error message in get_ctl_value_v2() The error message in get_ctl_value_v2() (for UAC2/3) is shown via KERN_ERR level but it was intended to be rather a debug message as seen in get_ctl_value_v1() (for UAC1). This patch downgrade the printk level. Tested-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20211014130636.17860-2-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'sound/usb') diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index a2ce535df14b..a20af9243155 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -373,7 +373,7 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, if (ret < 0) { error: - usb_audio_err(chip, + usb_audio_dbg(chip, "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n", request, validx, idx, cval->val_type); return ret; -- cgit From 509975c7789f84b4d98e06fe2db51ee4ec02eef5 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 15:06:35 +0200 Subject: ALSA: usb-audio: Drop superfluous error message after disconnection The error from snd_usb_lock_shutdown() indicates that the device is disconnected, hence it makes no sense to show any further control message error in get_ctl_value_v2(). Return the error directly instead. Tested-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20211014130636.17860-3-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index a20af9243155..60d394361a43 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -361,9 +361,8 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, memset(buf, 0, sizeof(buf)); - ret = snd_usb_lock_shutdown(chip) ? -EIO : 0; - if (ret) - goto error; + if (snd_usb_lock_shutdown(chip)) + return -EIO; idx = mixer_ctrl_intf(cval->head.mixer) | (cval->head.id << 8); ret = snd_usb_ctl_msg(chip->dev, usb_rcvctrlpipe(chip->dev, 0), bRequest, @@ -372,7 +371,6 @@ static int get_ctl_value_v2(struct usb_mixer_elem_info *cval, int request, snd_usb_unlock_shutdown(chip); if (ret < 0) { -error: usb_audio_dbg(chip, "cannot get ctl value: req = %#x, wValue = %#x, wIndex = %#x, type = %d\n", request, validx, idx, cval->val_type); -- cgit From b96681bd58276e1c7ca4ca37bbaab9f8f1738d61 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 14 Oct 2021 15:06:36 +0200 Subject: ALSA: usb-audio: Initialize every feature unit once at probe time So far we used to read the current value of the mixer element dynamically at the first access, and the error from a GET_CUR message is treated as a fatal error (unless QUIRK_IGNORE_CTL_ERROR is set). It's rather inconvenient, as most of GET_CUR errors are no fatal, and we can continue operation with assumption of some fixed value. This patch makes the USB-audio driver to change the behavior at probe time; now it tries to initialize the current value of each mixer element that is built from a feature unit (those for typically for mixer volumes and switches). When a read failure happens, it tries to set the known minimum value. After that point, a cached value is used always, hence we won't hit GET_CUR message error any longer. The error from GET_CUR message is still shown as a warning normally, but only once at the probe time, and it'll keep operating. If the message is confirmed to be harmless, it can be shut up by QUIRK_IGNORE_CTL_ERROR quirk flag, too. Tested-by: Greg Kroah-Hartman Link: https://lore.kernel.org/r/20211014130636.17860-4-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/usb/mixer.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) (limited to 'sound/usb') diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 60d394361a43..4555de9830c5 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -1199,12 +1199,32 @@ static void volume_control_quirks(struct usb_mixer_elem_info *cval, } } +/* forcibly initialize the current mixer value; if GET_CUR fails, set to + * the minimum as default + */ +static void init_cur_mix_raw(struct usb_mixer_elem_info *cval, int ch, int idx) +{ + int val, err; + + err = snd_usb_get_cur_mix_value(cval, ch, idx, &val); + if (!err) + return; + if (!cval->head.mixer->ignore_ctl_error) + usb_audio_warn(cval->head.mixer->chip, + "%d:%d: failed to get current value for ch %d (%d)\n", + cval->head.id, mixer_ctrl_intf(cval->head.mixer), + ch, err); + snd_usb_set_cur_mix_value(cval, ch, idx, cval->min); +} + /* * retrieve the minimum and maximum values for the specified control */ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval, int default_min, struct snd_kcontrol *kctl) { + int i, idx; + /* for failsafe */ cval->min = default_min; cval->max = cval->min + 1; @@ -1217,7 +1237,6 @@ static int get_min_max_with_quirks(struct usb_mixer_elem_info *cval, } else { int minchn = 0; if (cval->cmask) { - int i; for (i = 0; i < MAX_CHANNELS; i++) if (cval->cmask & (1 << i)) { minchn = i + 1; @@ -1318,6 +1337,19 @@ no_res_check: } } + /* initialize all elements */ + if (!cval->cmask) { + init_cur_mix_raw(cval, 0, 0); + } else { + idx = 0; + for (i = 0; i < MAX_CHANNELS; i++) { + if (cval->cmask & (1 << i)) { + init_cur_mix_raw(cval, i + 1, idx); + idx++; + } + } + } + return 0; } -- cgit From b97053df0f04747c3c1e021ecbe99db675342954 Mon Sep 17 00:00:00 2001 From: Chengfeng Ye Date: Sun, 24 Oct 2021 04:17:36 -0700 Subject: ALSA: usb-audio: fix null pointer dereference on pointer cs_desc The pointer cs_desc return from snd_usb_find_clock_source could be null, so there is a potential null pointer dereference issue. Fix this by adding a null check before dereference. Signed-off-by: Chengfeng Ye Link: https://lore.kernel.org/r/20211024111736.11342-1-cyeaa@connect.ust.hk Signed-off-by: Takashi Iwai --- sound/usb/clock.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'sound/usb') diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 7dd71d342443..4dfe76416794 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -496,6 +496,10 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip, union uac23_clock_source_desc *cs_desc; cs_desc = snd_usb_find_clock_source(chip, clock, fmt->protocol); + + if (!cs_desc) + return 0; + if (fmt->protocol == UAC_VERSION_3) bmControls = le32_to_cpu(cs_desc->v3.bmControls); else -- cgit From 9b371c6cc37f954360989eec41c2ddc5a6b83917 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 25 Oct 2021 14:11:41 +0200 Subject: ALSA: 6fire: fix control and bulk message timeouts USB control and bulk message timeouts are specified in milliseconds and should specifically not vary with CONFIG_HZ. Fixes: c6d43ba816d1 ("ALSA: usb/6fire - Driver for TerraTec DMX 6Fire USB") Cc: stable@vger.kernel.org # 2.6.39 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211025121142.6531-2-johan@kernel.org Signed-off-by: Takashi Iwai --- sound/usb/6fire/comm.c | 2 +- sound/usb/6fire/firmware.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/6fire/comm.c b/sound/usb/6fire/comm.c index 43a2a62d66f7..49629d4bb327 100644 --- a/sound/usb/6fire/comm.c +++ b/sound/usb/6fire/comm.c @@ -95,7 +95,7 @@ static int usb6fire_comm_send_buffer(u8 *buffer, struct usb_device *dev) int actual_len; ret = usb_interrupt_msg(dev, usb_sndintpipe(dev, COMM_EP), - buffer, buffer[1] + 2, &actual_len, HZ); + buffer, buffer[1] + 2, &actual_len, 1000); if (ret < 0) return ret; else if (actual_len != buffer[1] + 2) diff --git a/sound/usb/6fire/firmware.c b/sound/usb/6fire/firmware.c index 8981e61f2da4..c51abc54d2f8 100644 --- a/sound/usb/6fire/firmware.c +++ b/sound/usb/6fire/firmware.c @@ -160,7 +160,7 @@ static int usb6fire_fw_ezusb_write(struct usb_device *device, { return usb_control_msg_send(device, 0, type, USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - value, 0, data, len, HZ, GFP_KERNEL); + value, 0, data, len, 1000, GFP_KERNEL); } static int usb6fire_fw_ezusb_read(struct usb_device *device, @@ -168,7 +168,7 @@ static int usb6fire_fw_ezusb_read(struct usb_device *device, { return usb_control_msg_recv(device, 0, type, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE, - value, 0, data, len, HZ, GFP_KERNEL); + value, 0, data, len, 1000, GFP_KERNEL); } static int usb6fire_fw_fpga_write(struct usb_device *device, @@ -178,7 +178,7 @@ static int usb6fire_fw_fpga_write(struct usb_device *device, int ret; ret = usb_bulk_msg(device, usb_sndbulkpipe(device, FPGA_EP), data, len, - &actual_len, HZ); + &actual_len, 1000); if (ret < 0) return ret; else if (actual_len != len) -- cgit From f4000b58b64344871d7b27c05e73932f137cfef6 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 25 Oct 2021 14:11:42 +0200 Subject: ALSA: line6: fix control and interrupt message timeouts USB control and interrupt message timeouts are specified in milliseconds and should specifically not vary with CONFIG_HZ. Fixes: 705ececd1c60 ("Staging: add line6 usb driver") Cc: stable@vger.kernel.org # 2.6.30 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211025121142.6531-3-johan@kernel.org Signed-off-by: Takashi Iwai --- sound/usb/line6/driver.c | 14 +++++++------- sound/usb/line6/driver.h | 2 +- sound/usb/line6/podhd.c | 6 +++--- sound/usb/line6/toneport.c | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/line6/driver.c b/sound/usb/line6/driver.c index 9602929b7de9..59faa5a9a714 100644 --- a/sound/usb/line6/driver.c +++ b/sound/usb/line6/driver.c @@ -113,12 +113,12 @@ int line6_send_raw_message(struct usb_line6 *line6, const char *buffer, retval = usb_interrupt_msg(line6->usbdev, usb_sndintpipe(line6->usbdev, properties->ep_ctrl_w), (char *)frag_buf, frag_size, - &partial, LINE6_TIMEOUT * HZ); + &partial, LINE6_TIMEOUT); } else { retval = usb_bulk_msg(line6->usbdev, usb_sndbulkpipe(line6->usbdev, properties->ep_ctrl_w), (char *)frag_buf, frag_size, - &partial, LINE6_TIMEOUT * HZ); + &partial, LINE6_TIMEOUT); } if (retval) { @@ -347,7 +347,7 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, (datalen << 8) | 0x21, address, NULL, 0, - LINE6_TIMEOUT * HZ, GFP_KERNEL); + LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, "read request failed (error %d)\n", ret); goto exit; @@ -360,7 +360,7 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, 0x0012, 0x0000, &len, 1, - LINE6_TIMEOUT * HZ, GFP_KERNEL); + LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, "receive length failed (error %d)\n", ret); @@ -387,7 +387,7 @@ int line6_read_data(struct usb_line6 *line6, unsigned address, void *data, /* receive the result: */ ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, - 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT * HZ, + 0x0013, 0x0000, data, datalen, LINE6_TIMEOUT, GFP_KERNEL); if (ret) dev_err(line6->ifcdev, "read failed (error %d)\n", ret); @@ -417,7 +417,7 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, - 0x0022, address, data, datalen, LINE6_TIMEOUT * HZ, + 0x0022, address, data, datalen, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, @@ -430,7 +430,7 @@ int line6_write_data(struct usb_line6 *line6, unsigned address, void *data, ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, - 0x0012, 0x0000, status, 1, LINE6_TIMEOUT * HZ, + 0x0012, 0x0000, status, 1, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(line6->ifcdev, diff --git a/sound/usb/line6/driver.h b/sound/usb/line6/driver.h index 71d3da1db8c8..ecf3a2b39c7e 100644 --- a/sound/usb/line6/driver.h +++ b/sound/usb/line6/driver.h @@ -27,7 +27,7 @@ #define LINE6_FALLBACK_INTERVAL 10 #define LINE6_FALLBACK_MAXPACKETSIZE 16 -#define LINE6_TIMEOUT 1 +#define LINE6_TIMEOUT 1000 #define LINE6_BUFSIZE_LISTEN 64 #define LINE6_MIDI_MESSAGE_MAXLEN 256 diff --git a/sound/usb/line6/podhd.c b/sound/usb/line6/podhd.c index 28794a35949d..b24bc82f89e3 100644 --- a/sound/usb/line6/podhd.c +++ b/sound/usb/line6/podhd.c @@ -190,7 +190,7 @@ static int podhd_dev_start(struct usb_line6_podhd *pod) ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, 0x11, 0, - NULL, 0, LINE6_TIMEOUT * HZ, GFP_KERNEL); + NULL, 0, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(pod->line6.ifcdev, "read request failed (error %d)\n", ret); goto exit; @@ -200,7 +200,7 @@ static int podhd_dev_start(struct usb_line6_podhd *pod) ret = usb_control_msg_recv(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN, 0x11, 0x0, - init_bytes, 3, LINE6_TIMEOUT * HZ, GFP_KERNEL); + init_bytes, 3, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { dev_err(pod->line6.ifcdev, "receive length failed (error %d)\n", ret); @@ -220,7 +220,7 @@ static int podhd_dev_start(struct usb_line6_podhd *pod) USB_REQ_SET_FEATURE, USB_TYPE_STANDARD | USB_RECIP_DEVICE | USB_DIR_OUT, 1, 0, - NULL, 0, LINE6_TIMEOUT * HZ, GFP_KERNEL); + NULL, 0, LINE6_TIMEOUT, GFP_KERNEL); exit: return ret; } diff --git a/sound/usb/line6/toneport.c b/sound/usb/line6/toneport.c index 4e5693c97aa4..e33df58740a9 100644 --- a/sound/usb/line6/toneport.c +++ b/sound/usb/line6/toneport.c @@ -128,7 +128,7 @@ static int toneport_send_cmd(struct usb_device *usbdev, int cmd1, int cmd2) ret = usb_control_msg_send(usbdev, 0, 0x67, USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_OUT, - cmd1, cmd2, NULL, 0, LINE6_TIMEOUT * HZ, + cmd1, cmd2, NULL, 0, LINE6_TIMEOUT, GFP_KERNEL); if (ret) { -- cgit From 55f261b73a7e1cb254577c3536cef8f415de220a Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Tue, 26 Oct 2021 11:54:01 +0200 Subject: ALSA: ua101: fix division by zero at probe Add the missing endpoint max-packet sanity check to probe() to avoid division by zero in alloc_stream_buffers() in case a malicious device has broken descriptors (or when doing descriptor fuzz testing). Note that USB core will reject URBs submitted for endpoints with zero wMaxPacketSize but that drivers doing packet-size calculations still need to handle this (cf. commit 2548288b4fb0 ("USB: Fix: Don't skip endpoint descriptors with maxpacket=0")). Fixes: 63978ab3e3e9 ("sound: add Edirol UA-101 support") Cc: stable@vger.kernel.org # 2.6.34 Signed-off-by: Johan Hovold Link: https://lore.kernel.org/r/20211026095401.26522-1-johan@kernel.org Signed-off-by: Takashi Iwai --- sound/usb/misc/ua101.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sound/usb') diff --git a/sound/usb/misc/ua101.c b/sound/usb/misc/ua101.c index 5834d1dc317e..4f6b20ed29dd 100644 --- a/sound/usb/misc/ua101.c +++ b/sound/usb/misc/ua101.c @@ -1000,7 +1000,7 @@ static int detect_usb_format(struct ua101 *ua) fmt_playback->bSubframeSize * ua->playback.channels; epd = &ua->intf[INTF_CAPTURE]->altsetting[1].endpoint[0].desc; - if (!usb_endpoint_is_isoc_in(epd)) { + if (!usb_endpoint_is_isoc_in(epd) || usb_endpoint_maxp(epd) == 0) { dev_err(&ua->dev->dev, "invalid capture endpoint\n"); return -ENXIO; } @@ -1008,7 +1008,7 @@ static int detect_usb_format(struct ua101 *ua) ua->capture.max_packet_bytes = usb_endpoint_maxp(epd); epd = &ua->intf[INTF_PLAYBACK]->altsetting[1].endpoint[0].desc; - if (!usb_endpoint_is_isoc_out(epd)) { + if (!usb_endpoint_is_isoc_out(epd) || usb_endpoint_maxp(epd) == 0) { dev_err(&ua->dev->dev, "invalid playback endpoint\n"); return -ENXIO; } -- cgit