Commit 813a17ca authored by Takashi Iwai's avatar Takashi Iwai
Browse files

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: default avatarTakashi Iwai <tiwai@suse.de>
parent d5f871f8

sound/usb/endpoint.c

+9 −5
Original line number Diff line number Diff line
@@ -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 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) 
 * 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 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) 
 * 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);

	}

}

sound/usb/endpoint.h

+1 −1
Original line number Diff line number Diff line
@@ -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);

sound/usb/pcm.c

+8 −8
Original line number Diff line number Diff line
@@ -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,