[PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack]

It's been a while since the last round of patches for this topic, so
here is a new version.

In contrast to earlier versions, I did another bunch of cleanips and
can now use input and output simultaniously.

The patches that follow are structured in a way that should make
reviews easier, as they first implement a new streaming model while
leaving the old one working, and then switch to it in a separate
patch. This way, the series is also fully bisectable.

The problems that remain is a regression in handling data streams
with feedback endpoints in full duplex mode and a hard lockup when
changing audio parameters while two streams are running. I guess the
latter is caused by missing locking.

I didn't sign-off the patches on purpose, as I would really like to
get them reviewed before they go in. Can people have a look and state
whether the whole idea is at all sane?


Thanks a lot,
Daniel


Daniel Mack (5):
  ALSA: snd-usb: implement new endpoint streaming model
  ALSA: snd-usb: switch over to new endpoint streaming logic
  ALSA: snd-usb: remove old streaming logic
  ALSA: snd-usb: set MAX_URBS to 16
  ALSA: snd-usb: add support for implicit feedback

 sound/usb/card.c     |    6 +-
 sound/usb/card.h     |   56 ++-
 sound/usb/endpoint.c | 1347 ++++++++++++++++++++++++--------------------------
 sound/usb/endpoint.h |   31 +-
 sound/usb/pcm.c      |  412 +++++++++++++---
 sound/usb/stream.c   |   31 +-
 sound/usb/usbaudio.h |    1 +
 7 files changed, 1081 insertions(+), 803 deletions(-)

-- 
1.7.5.4

[PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Daniel Mack]

In order to split changes properly, this patch only adds the new
implementation but leaves the old one around, so the rest of the driver
does not behave differently. The switch to actually use the new thing is
submitted separately.
---
 sound/usb/card.h     |   49 +++
 sound/usb/endpoint.c |  818 +++++++++++++++++++++++++++++++++++++++++++++++++-
 sound/usb/endpoint.h |   25 ++
 sound/usb/usbaudio.h |    1 +
 4 files changed, 882 insertions(+), 11 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index a39edcc..8662702 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -29,13 +29,16 @@ struct audioformat {
 };
 
 struct snd_usb_substream;
+struct snd_usb_endpoint;
 
 struct snd_urb_ctx {
 	struct urb *urb;
 	unsigned int buffer_size;	/* size of data buffer, if data URB */
 	struct snd_usb_substream *subs;
+	struct snd_usb_endpoint *ep;
 	int index;	/* index for urb array */
 	int packets;	/* number of packets per urb */
+	int packet_size[MAX_PACKS_HS]; /* size of packets for next submission */
 };
 
 struct snd_urb_ops {
@@ -45,6 +48,52 @@ struct snd_urb_ops {
 	int (*retire_sync)(struct snd_usb_substream *subs, struct snd_pcm_runtime *runtime, struct urb *u);
 };
 
+struct snd_usb_endpoint {
+	struct snd_usb_audio *chip;
+
+	int use_count;
+	int ep_num;     /* the referenced endpoint number */
+	int type;	/* SND_USB_ENDPOINT_TYPE_* */
+	int activated;
+
+	void (* prepare_data_urb) (struct snd_usb_substream *subs,
+				   struct urb *urb);
+	void (* retire_data_urb) (struct snd_usb_substream *subs,
+				  struct urb *urb);
+
+	struct snd_usb_substream *data_subs;
+	struct snd_usb_endpoint *sync_master;
+	struct snd_usb_endpoint *sync_slave;
+
+	struct snd_urb_ctx urb[MAX_URBS];
+	unsigned int nurbs;			/* # urbs */
+	unsigned long active_mask;		/* bitmask of active urbs */
+	unsigned long unlink_mask;		/* bitmask of unlinked urbs */
+	char *syncbuf;			     /* sync buffer for all sync URBs */
+	dma_addr_t sync_dma;		      /* DMA address of syncbuf */
+
+	unsigned int pipe;      /* the data i/o pipe */
+	unsigned int freqn;     /* nominal sampling rate in fs/fps in Q16.16 format */
+	unsigned int freqm;      /* momentary sampling rate in fs/fps in Q16.16 format */
+	int	   freqshift;  /* how much to shift the feedback value to get Q16.16 */
+	unsigned int freqmax;    /* maximum sampling rate, used for buffer management */
+	unsigned int phase;      /* phase accumulator */
+	unsigned int maxpacksize;	/* max packet size in bytes */
+	unsigned int maxframesize;      /* max packet size in frames */
+	unsigned int curpacksize;	/* current packet size in bytes (for capture) */
+	unsigned int curframesize;      /* current packet size in frames (for capture) */
+	unsigned int syncmaxsize;	/* sync endpoint packet size */
+	unsigned int fill_max: 1;	/* fill max packet size always */
+	unsigned int datainterval;      /* log_2 of data packet interval */
+	unsigned int syncinterval;  /* P for adaptive mode, 0 otherwise */
+	unsigned char silence_value;
+	unsigned int stride;
+	int iface, alt_idx;
+
+	spinlock_t lock;
+	struct list_head list;
+};
+
 struct snd_usb_substream {
 	struct snd_usb_stream *stream;
 	struct usb_device *dev;
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 81c6ede..03a67f2 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -19,9 +19,11 @@
 #include <linux/init.h>
 #include <linux/usb.h>
 #include <linux/usb/audio.h>
+#include <linux/slab.h>
 
 #include <sound/core.h>
 #include <sound/pcm.h>
+#include <sound/pcm_params.h>
 
 #include "usbaudio.h"
 #include "helper.h"
@@ -50,7 +52,7 @@ static inline unsigned get_usb_high_speed_rate(unsigned int rate)
 /*
  * unlink active urbs.
  */
-static int deactivate_urbs(struct snd_usb_substream *subs, int force, int can_sleep)
+static int deactivate_urbs_old(struct snd_usb_substream *subs, int force, int can_sleep)
 {
 	struct snd_usb_audio *chip = subs->stream->chip;
 	unsigned int i;
@@ -112,7 +114,7 @@ static void release_urb_ctx(struct snd_urb_ctx *u)
 /*
  *  wait until all urbs are processed.
  */
-static int wait_clear_urbs(struct snd_usb_substream *subs)
+static int wait_clear_urbs_old(struct snd_usb_substream *subs)
 {
 	unsigned long end_time = jiffies + msecs_to_jiffies(1000);
 	unsigned int i;
@@ -147,8 +149,8 @@ void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force)
 	int i;
 
 	/* stop urbs (to be sure) */
-	deactivate_urbs(subs, force, 1);
-	wait_clear_urbs(subs);
+	deactivate_urbs_old(subs, force, 1);
+	wait_clear_urbs_old(subs);
 
 	for (i = 0; i < MAX_URBS; i++)
 		release_urb_ctx(&subs->dataurb[i]);
@@ -163,7 +165,7 @@ void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force)
 /*
  * complete callback from data urb
  */
-static void snd_complete_urb(struct urb *urb)
+static void snd_complete_urb_old(struct urb *urb)
 {
 	struct snd_urb_ctx *ctx = urb->context;
 	struct snd_usb_substream *subs = ctx->subs;
@@ -317,7 +319,7 @@ int snd_usb_init_substream_urbs(struct snd_usb_substream *subs,
 		u->urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
 		u->urb->interval = 1 << subs->datainterval;
 		u->urb->context = u;
-		u->urb->complete = snd_complete_urb;
+		u->urb->complete = snd_complete_urb_old;
 	}
 
 	if (subs->syncpipe) {
@@ -855,7 +857,7 @@ static int start_urbs(struct snd_usb_substream *subs, struct snd_pcm_runtime *ru
 
  __error:
 	// snd_pcm_stop(subs->pcm_substream, SNDRV_PCM_STATE_XRUN);
-	deactivate_urbs(subs, 0, 0);
+	deactivate_urbs_old(subs, 0, 0);
 	return -EPIPE;
 }
 
@@ -916,7 +918,7 @@ int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int
 		subs->ops.prepare = prepare_playback_urb;
 		return 0;
 	case SNDRV_PCM_TRIGGER_STOP:
-		return deactivate_urbs(subs, 0, 0);
+		return deactivate_urbs_old(subs, 0, 0);
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		subs->ops.prepare = prepare_nodata_playback_urb;
 		return 0;
@@ -934,7 +936,7 @@ int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int c
 		subs->ops.retire = retire_capture_urb;
 		return start_urbs(subs, substream->runtime);
 	case SNDRV_PCM_TRIGGER_STOP:
-		return deactivate_urbs(subs, 0, 0);
+		return deactivate_urbs_old(subs, 0, 0);
 	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
 		subs->ops.retire = retire_paused_capture_urb;
 		return 0;
@@ -950,8 +952,8 @@ int snd_usb_substream_prepare(struct snd_usb_substream *subs,
 			      struct snd_pcm_runtime *runtime)
 {
 	/* clear urbs (to be sure) */
-	deactivate_urbs(subs, 0, 1);
-	wait_clear_urbs(subs);
+	deactivate_urbs_old(subs, 0, 1);
+	wait_clear_urbs_old(subs);
 
 	/* for playback, submit the URBs now; otherwise, the first hwptr_done
 	 * updates for all URBs would happen at the same time when starting */
@@ -963,3 +965,797 @@ int snd_usb_substream_prepare(struct snd_usb_substream *subs,
 	return 0;
 }
 
+int snd_usb_endpoint_implict_feedback_sink(struct snd_usb_endpoint *ep)
+{
+	return  ep->sync_master &&
+		ep->sync_master->type == SND_USB_ENDPOINT_TYPE_DATA &&
+		ep->type == SND_USB_ENDPOINT_TYPE_DATA &&
+		usb_pipeout(ep->pipe);
+}
+
+/* determine the number of frames in the next packet */
+static int next_packet_size(struct snd_usb_endpoint *ep)
+{
+	if (ep->fill_max)
+		return ep->maxframesize;
+	else {
+		ep->phase = (ep->phase & 0xffff)
+			+ (ep->freqm << ep->datainterval);
+		return min(ep->phase >> 16, ep->maxframesize);
+	}
+}
+
+static void retire_outbound_urb(struct snd_usb_endpoint *ep,
+				struct snd_urb_ctx *urb_ctx)
+{
+	if (ep->retire_data_urb)
+		ep->retire_data_urb(ep->data_subs, urb_ctx->urb);
+}
+
+static void retire_inbound_urb(struct snd_usb_endpoint *ep,
+			       struct snd_urb_ctx *urb_ctx)
+{
+	struct urb *urb = urb_ctx->urb;
+
+	if (ep->sync_slave)
+		snd_usb_handle_sync_urb(ep->sync_slave, ep, urb);
+
+	if (ep->retire_data_urb)
+		ep->retire_data_urb(ep->data_subs, urb);
+}
+
+static void prepare_outbound_urb_sizes(struct snd_usb_endpoint *ep,
+				       struct snd_urb_ctx *ctx)
+{
+	int i;
+
+	for (i = 0; i < ctx->packets; ++i)
+		ctx->packet_size[i] = next_packet_size(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)
+{
+	int i;
+	struct urb *urb = ctx->urb;
+	unsigned char *cp = urb->transfer_buffer;
+
+	urb->dev = ep->chip->dev; /* we need to set this at each time */
+
+	switch (ep->type) {
+	case SND_USB_ENDPOINT_TYPE_DATA:
+		if (ep->prepare_data_urb) {
+			ep->prepare_data_urb(ep->data_subs, urb);
+		} else {
+			/* no data provider, so send silence */
+			unsigned int offs = 0;
+			for (i = 0; i < ctx->packets; ++i) {
+				int counts = ctx->packet_size[i];
+				urb->iso_frame_desc[i].offset = offs * ep->stride;
+				urb->iso_frame_desc[i].length = counts * ep->stride;
+				offs += counts;
+			}
+
+			urb->number_of_packets = ctx->packets;
+			urb->transfer_buffer_length = offs * ep->stride;
+			memset(urb->transfer_buffer, ep->silence_value,
+			       offs * ep->stride);
+		}
+		break;
+
+	case SND_USB_ENDPOINT_TYPE_SYNC:
+		if (snd_usb_get_speed(ep->chip->dev) >= USB_SPEED_HIGH) {
+			/*
+			 * fill the length and offset of each urb descriptor.
+			 * the fixed 12.13 frequency is passed as 16.16 through the pipe.
+			 */
+			urb->iso_frame_desc[0].length = 4;
+			urb->iso_frame_desc[0].offset = 0;
+			cp[0] = ep->freqn;
+			cp[1] = ep->freqn >> 8;
+			cp[2] = ep->freqn >> 16;
+			cp[3] = ep->freqn >> 24;
+		} else {
+			/*
+			 * fill the length and offset of each urb descriptor.
+			 * the fixed 10.14 frequency is passed through the pipe.
+			 */
+			urb->iso_frame_desc[0].length = 3;
+			urb->iso_frame_desc[0].offset = 0;
+			cp[0] = ep->freqn >> 2;
+			cp[1] = ep->freqn >> 10;
+			cp[2] = ep->freqn >> 18;
+		}
+
+		break;
+	}
+}
+
+/*
+ * 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)
+{
+	int i, offs;
+	struct urb *urb = urb_ctx->urb;
+
+	urb->dev = ep->chip->dev; /* we need to set this at each time */
+
+	switch (ep->type) {
+	case SND_USB_ENDPOINT_TYPE_DATA:
+		offs = 0;
+		for (i = 0; i < urb_ctx->packets; i++) {
+			urb->iso_frame_desc[i].offset = offs;
+			urb->iso_frame_desc[i].length = ep->curpacksize;
+			offs += ep->curpacksize;
+		}
+
+		urb->transfer_buffer_length = offs;
+		urb->number_of_packets = urb_ctx->packets;
+		break;
+
+	case SND_USB_ENDPOINT_TYPE_SYNC:
+		urb->iso_frame_desc[0].length = min(4u, ep->syncmaxsize);
+		urb->iso_frame_desc[0].offset = 0;
+		break;
+	}
+}
+
+/*
+ * complete callback for urbs
+ */
+static void snd_complete_urb(struct urb *urb)
+{
+	struct snd_urb_ctx *ctx = urb->context;
+	struct snd_usb_endpoint *ep = ctx->ep;
+	int err;
+
+	if (ep->use_count == 0)
+		goto exit_clear;
+
+	if (usb_pipeout(ep->pipe)) {
+		retire_outbound_urb(ep, ctx);
+		if (ep->use_count == 0) /* can be stopped during retire callback */
+			goto exit_clear;
+
+		if (snd_usb_endpoint_implict_feedback_sink(ep))
+			goto exit_clear;
+
+		prepare_outbound_urb_sizes(ep, ctx);
+		prepare_outbound_urb(ep, ctx);
+	} else {
+		retire_inbound_urb(ep, ctx);
+		if (ep->use_count == 0) /* can be stopped during retire callback */
+			goto exit_clear;
+
+		prepare_inbound_urb(ep, ctx);
+	}
+
+	err = usb_submit_urb(urb, GFP_ATOMIC);
+	if (err == 0)
+		return;
+
+	snd_printk(KERN_ERR "cannot submit urb (err = %d)\n", err);
+	//snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
+
+exit_clear:
+	clear_bit(ctx->index, &ep->active_mask);
+}
+
+struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
+					      struct usb_host_interface *alts,
+					      int ep_num, int direction, int type)
+{
+	struct list_head *p;
+	struct snd_usb_endpoint *ep;
+	int is_playback = direction == SNDRV_PCM_STREAM_PLAYBACK;
+
+	list_for_each(p, &chip->ep_list) {
+		ep = list_entry(p, struct snd_usb_endpoint, list);
+		if (ep->ep_num == ep_num &&
+		    ep->iface == alts->desc.bInterfaceNumber &&
+		    ep->alt_idx == alts->desc.bAlternateSetting) {
+			snd_printk("Re-using EP %x in iface %d,%d @%p\n",
+					ep_num, ep->iface, ep->alt_idx, ep);
+			return ep;
+		}
+	}
+
+	snd_printk(KERN_INFO "Creating new %s %s endpoint #%x\n",
+		   is_playback ? "playback" : "capture",
+		   type == SND_USB_ENDPOINT_TYPE_DATA ? "data" : "sync",
+		   ep_num);
+
+	ep = kzalloc(sizeof(*ep), GFP_KERNEL);
+	if (!ep)
+		return NULL;
+
+	spin_lock_init(&ep->lock);
+
+	ep->chip = chip;
+	ep->type = type;
+	ep->ep_num = ep_num;
+	ep->iface = alts->desc.bInterfaceNumber;
+	ep->alt_idx = alts->desc.bAlternateSetting;
+
+	/* select the alt setting once so the endpoints become valid */
+	snd_printk("%s() calling usb_set_interface()\n", __func__);
+	usb_set_interface(ep->chip->dev, ep->iface, ep->alt_idx);
+
+	ep_num &= USB_ENDPOINT_NUMBER_MASK;
+
+	if (is_playback)
+		ep->pipe = usb_sndisocpipe(chip->dev, ep_num);
+	else
+		ep->pipe = usb_rcvisocpipe(chip->dev, ep_num);
+
+	if (type == SND_USB_ENDPOINT_TYPE_SYNC) {
+		if (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
+		    get_endpoint(alts, 1)->bRefresh >= 1 &&
+		    get_endpoint(alts, 1)->bRefresh <= 9)
+			ep->syncinterval = get_endpoint(alts, 1)->bRefresh;
+		else if (snd_usb_get_speed(chip->dev) == USB_SPEED_FULL)
+			ep->syncinterval = 1;
+		else if (get_endpoint(alts, 1)->bInterval >= 1 &&
+			 get_endpoint(alts, 1)->bInterval <= 16)
+			ep->syncinterval = get_endpoint(alts, 1)->bInterval - 1;
+		else
+			ep->syncinterval = 3;
+
+		ep->syncmaxsize = le16_to_cpu(get_endpoint(alts, 1)->wMaxPacketSize);
+	}
+
+	list_add_tail(&ep->list, &chip->ep_list);
+
+	return ep;
+}
+
+/*
+ *  wait until all urbs are processed.
+ */
+static int wait_clear_urbs(struct snd_usb_endpoint *ep)
+{
+	unsigned long end_time = jiffies + msecs_to_jiffies(1000);
+	unsigned int i;
+	int alive;
+
+	do {
+		alive = 0;
+		for (i = 0; i < ep->nurbs; i++)
+			if (test_bit(i, &ep->active_mask))
+				alive++;
+
+		if (! alive)
+			break;
+
+		schedule_timeout_uninterruptible(1);
+	} while (time_before(jiffies, end_time));
+
+	if (alive)
+		snd_printk(KERN_ERR "timeout: still %d active urbs on EP #%x\n",
+					alive, ep->ep_num);
+
+	return 0;
+}
+
+/*
+ * unlink active urbs.
+ */
+static int deactivate_urbs(struct snd_usb_endpoint *ep, int force, int can_sleep)
+{
+	unsigned int i;
+	int async;
+
+	if (!force && ep->chip->shutdown) /* to be sure... */
+		return -EBADFD;
+
+	async = !can_sleep && ep->chip->async_unlink;
+
+	if (!async && in_interrupt())
+		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)) {
+				struct urb *u = ep->urb[i].urb;
+				if (async)
+					usb_unlink_urb(u);
+				else
+					usb_kill_urb(u);
+			}
+		}
+	}
+
+	return 0;
+}
+
+/*
+ * release an endpoint's urbs
+ */
+static void release_urbs(struct snd_usb_endpoint *ep, int force)
+{
+	int i;
+
+	/* route incoming urbs to nirvana */
+	ep->retire_data_urb = NULL;
+	ep->prepare_data_urb = NULL;
+
+	/* stop urbs */
+	deactivate_urbs(ep, force, 1);
+	wait_clear_urbs(ep);
+
+	for (i = 0; i < ep->nurbs; i++)
+		release_urb_ctx(&ep->urb[i]);
+
+	usb_free_coherent(ep->chip->dev, SYNC_URBS * 4,
+			  ep->syncbuf, ep->sync_dma);
+
+	ep->syncbuf = NULL;
+	ep->nurbs = 0;
+}
+
+static int data_ep_set_params(struct snd_usb_endpoint *ep,
+			      struct snd_pcm_hw_params *hw_params,
+			      struct audioformat *fmt,
+			      struct snd_usb_endpoint *sync_ep)
+{
+	unsigned int maxsize, i, urb_packs, total_packs, packs_per_ms;
+	int period_bytes = params_period_bytes(hw_params);
+	int format = params_format(hw_params);
+	int is_playback = usb_pipeout(ep->pipe);
+	int frame_bits = snd_pcm_format_physical_width(params_format(hw_params)) *
+							params_channels(hw_params);
+
+	ep->datainterval = fmt->datainterval;
+	ep->stride = frame_bits >> 3;
+	ep->silence_value = format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0;
+
+	/* calculate max. frequency */
+	if (ep->maxpacksize) {
+		/* whatever fits into a max. size packet */
+		maxsize = ep->maxpacksize;
+		ep->freqmax = (maxsize / (frame_bits >> 3))
+				<< (16 - ep->datainterval);
+	} else {
+		/* no max. packet size: just take 25% higher than nominal */
+		ep->freqmax = ep->freqn + (ep->freqn >> 2);
+		maxsize = ((ep->freqmax + 0xffff) * (frame_bits >> 3))
+				>> (16 - ep->datainterval);
+	}
+
+	if (ep->fill_max)
+		ep->curpacksize = ep->maxpacksize;
+	else
+		ep->curpacksize = maxsize;
+
+	if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL)
+		packs_per_ms = 8 >> ep->datainterval;
+	else
+		packs_per_ms = 1;
+
+	if (is_playback && !snd_usb_endpoint_implict_feedback_sink(ep)) {
+		urb_packs = max(ep->chip->nrpacks, 1);
+		urb_packs = min(urb_packs, (unsigned int) MAX_PACKS);
+	} else {
+		urb_packs = 1;
+	}
+
+	urb_packs *= packs_per_ms;
+
+	if (sync_ep && !snd_usb_endpoint_implict_feedback_sink(ep))
+		urb_packs = min(urb_packs, 1U << sync_ep->syncinterval);
+
+	/* decide how many packets to be used */
+	if (is_playback && !snd_usb_endpoint_implict_feedback_sink(ep)) {
+		unsigned int minsize, maxpacks;
+		/* determine how small a packet can be */
+		minsize = (ep->freqn >> (16 - ep->datainterval))
+			  * (frame_bits >> 3);
+		/* with sync from device, assume it can be 12% lower */
+		if (sync_ep)
+			minsize -= minsize >> 3;
+		minsize = max(minsize, 1u);
+		total_packs = (period_bytes + minsize - 1) / minsize;
+		/* we need at least two URBs for queueing */
+		if (total_packs < 2) {
+			total_packs = 2;
+		} else {
+			/* and we don't want too long a queue either */
+			maxpacks = max(MAX_QUEUE * packs_per_ms, urb_packs * 2);
+			total_packs = min(total_packs, maxpacks);
+		}
+	} else {
+		while (urb_packs > 1 && urb_packs * maxsize >= period_bytes)
+			urb_packs >>= 1;
+		total_packs = MAX_URBS * urb_packs;
+	}
+
+	snd_printk("is_playback? %d impf %d, total_packs %d urb_packs %d\n",
+		is_playback, snd_usb_endpoint_implict_feedback_sink(ep), total_packs, urb_packs);
+
+	ep->nurbs = (total_packs + urb_packs - 1) / urb_packs;
+	if (ep->nurbs > MAX_URBS) {
+		/* too much... */
+		ep->nurbs = MAX_URBS;
+		total_packs = MAX_URBS * urb_packs;
+	} else if (ep->nurbs < 2) {
+		/* too little - we need at least two packets
+		 * to ensure contiguous playback/capture
+		 */
+		ep->nurbs = 2;
+	}
+
+	/* allocate and initialize data urbs */
+	for (i = 0; i < ep->nurbs; i++) {
+		struct snd_urb_ctx *u = &ep->urb[i];
+		u->index = i;
+		u->ep = ep;
+		u->packets = (i + 1) * total_packs / ep->nurbs
+			- i * total_packs / ep->nurbs;
+		u->buffer_size = maxsize * u->packets;
+		snd_printk(" ep %p:: buffer_size %d maxsize %d packets %d\n", ep, u->buffer_size, maxsize, u->packets);
+		if (fmt->fmt_type == UAC_FORMAT_TYPE_II)
+			u->packets++; /* for transfer delimiter */
+		u->urb = usb_alloc_urb(u->packets, GFP_KERNEL);
+		if (!u->urb)
+			goto out_of_memory;
+
+		u->urb->transfer_buffer =
+			usb_alloc_coherent(ep->chip->dev, u->buffer_size,
+					   GFP_KERNEL, &u->urb->transfer_dma);
+		if (!u->urb->transfer_buffer)
+			goto out_of_memory;
+		u->urb->pipe = ep->pipe;
+		u->urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
+		u->urb->interval = 1 << ep->datainterval;
+		u->urb->context = u;
+		u->urb->complete = snd_complete_urb;
+	}
+
+	return 0;
+
+out_of_memory:
+	release_urbs(ep, 0);
+	return -ENOMEM;
+}
+
+static int sync_ep_set_params(struct snd_usb_endpoint *ep,
+			      struct snd_pcm_hw_params *hw_params,
+			      struct audioformat *fmt)
+{
+	int i;
+
+	ep->syncbuf = usb_alloc_coherent(ep->chip->dev, SYNC_URBS * 4,
+					 GFP_KERNEL, &ep->sync_dma);
+	if (!ep->syncbuf)
+		goto out_of_memory;
+
+	for (i = 0; i < SYNC_URBS; i++) {
+		struct snd_urb_ctx *u = &ep->urb[i];
+		u->index = i;
+		u->ep = ep;
+		u->packets = 1;
+		u->urb = usb_alloc_urb(1, GFP_KERNEL);
+		if (!u->urb)
+			goto out_of_memory;
+		u->urb->transfer_buffer = ep->syncbuf + i * 4;
+		u->urb->transfer_dma = ep->sync_dma + i * 4;
+		u->urb->transfer_buffer_length = 4;
+		u->urb->pipe = ep->pipe;
+		u->urb->transfer_flags = URB_ISO_ASAP |
+					 URB_NO_TRANSFER_DMA_MAP;
+		u->urb->number_of_packets = 1;
+		u->urb->interval = 1 << ep->syncinterval;
+		u->urb->context = u;
+		u->urb->complete = snd_complete_urb;
+	}
+
+	ep->nurbs = SYNC_URBS;
+
+	return 0;
+
+out_of_memory:
+	release_urbs(ep, 0);
+	return -ENOMEM;
+}
+
+int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
+				struct snd_pcm_hw_params *hw_params,
+				struct audioformat *fmt,
+				struct snd_usb_endpoint *sync_ep)
+{
+	int err;
+
+	if (ep->use_count > 0) {
+		snd_printk(KERN_WARNING "Unable to change format on ep #%x: already in use\n",
+			   ep->ep_num);
+		return -EBUSY;
+	}
+
+	ep->datainterval = fmt->datainterval;
+	ep->maxpacksize = fmt->maxpacksize;
+	ep->fill_max = fmt->attributes & UAC_EP_CS_ATTR_FILL_MAX;
+
+	if (snd_usb_get_speed(ep->chip->dev) == USB_SPEED_FULL)
+		ep->freqn = get_usb_full_speed_rate(params_rate(hw_params));
+	else
+		ep->freqn = get_usb_high_speed_rate(params_rate(hw_params));
+
+	/* calculate the frequency in 16.16 format */
+	ep->freqm = ep->freqn;
+	ep->freqshift = INT_MIN;
+
+	ep->phase = 0;
+
+	switch (ep->type) {
+	case  SND_USB_ENDPOINT_TYPE_DATA:
+		err = data_ep_set_params(ep, hw_params, fmt, sync_ep);
+		break;
+	case  SND_USB_ENDPOINT_TYPE_SYNC:
+		err = sync_ep_set_params(ep, hw_params, fmt);
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	snd_printk("Setting params for ep #%x (type %d, %d urbs) -> %d\n",
+		   ep->ep_num, ep->type, ep->nurbs, err);
+
+	return err;
+}
+
+int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
+{
+	int err;
+	unsigned int i;
+
+	if (ep->chip->shutdown)
+		return -EBADFD;
+
+	/* already running? */
+	if (++ep->use_count != 1)
+		return 0;
+
+	ep->phase = 0;
+	ep->active_mask = 0;
+	ep->unlink_mask = 0;
+
+	/*
+	 * If this endpoint has a data endpoint as implicit feedback source,
+	 * don't start the urbs here. Instead, wait for the record urbs to
+	 * arrive and queue from that context.
+	 */
+	if (snd_usb_endpoint_implict_feedback_sink(ep))
+		return 0;
+
+	for (i = 0; i < ep->nurbs; i++) {
+		struct urb *urb = ep->urb[i].urb;
+
+		if (snd_BUG_ON(!urb))
+			return -EINVAL;
+
+		if (usb_pipeout(ep->pipe)) {
+			prepare_outbound_urb_sizes(ep, urb->context);
+			prepare_outbound_urb(ep, urb->context);
+		} else {
+			prepare_inbound_urb(ep, urb->context);
+		}
+
+		err = usb_submit_urb(urb, GFP_ATOMIC);
+		snd_printk(KERN_ERR "Submmiting urb %d/%d (in? %d buf %p len %d) ret (%d)\n",
+				i, ep->nurbs, usb_pipein(ep->pipe),
+				ep->urb[i].urb->transfer_buffer,
+				ep->urb[i].urb->transfer_buffer_length,
+				err);
+		if (err < 0) {
+			snd_printk(KERN_ERR "cannot submit urb %d, error %d: %s\n",
+				   i, err, usb_error_string(err));
+			goto __error;
+		}
+		set_bit(i, &ep->active_mask);
+	}
+
+	return 0;
+
+__error:
+	ep->use_count--;
+	deactivate_urbs(ep, 0, 0);
+	return -EPIPE;
+}
+
+void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, int force, int can_sleep)
+{
+	if (!ep)
+		return;
+
+	if (snd_BUG_ON(ep->use_count == 0))
+		return;
+
+	if (--ep->use_count == 0) {
+		ep->sync_slave = NULL;
+		ep->retire_data_urb = NULL;
+		ep->prepare_data_urb = NULL;
+		ep->sync_master = NULL;
+		deactivate_urbs(ep, force, can_sleep);
+	}
+}
+
+void snd_usb_endpoint_activate(struct snd_usb_endpoint *ep)
+{
+	if (ep && ep->use_count == 0 && !ep->chip->shutdown && !ep->activated) {
+		snd_printk("%s() calling usb_set_interface(%d,%d)\n", __func__, ep->iface, ep->alt_idx);
+		usb_set_interface(ep->chip->dev, ep->iface, ep->alt_idx);
+		ep->activated = 1;
+	}
+}
+
+void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)
+{
+	if (ep && ep->use_count == 0 && !ep->chip->shutdown && ep->activated) {
+		snd_printk("%s() calling usb_set_interface(%d,0)\n", __func__, ep->iface);
+		usb_set_interface(ep->chip->dev, ep->iface, 0);
+		ep->activated = 0;
+	}
+}
+
+void snd_usb_endpoint_free(struct list_head *head)
+{
+	struct snd_usb_endpoint *ep;
+
+	ep = list_entry(head, struct snd_usb_endpoint, list);
+	deactivate_urbs(ep, 1, 1);
+	release_urbs(ep, 1);
+}
+
+/*
+ * process after playback sync complete
+ *
+ * Full speed devices report feedback values in 10.14 format as samples per
+ * frame, high speed devices in 16.16 format as samples per microframe.
+ * Because the Audio Class 1 spec was written before USB 2.0, many high speed
+ * devices use a wrong interpretation, some others use an entirely different
+ * format.  Therefore, we cannot predict what format any particular device uses
+ * and must detect it automatically.
+ */
+void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
+			     struct snd_usb_endpoint *sender,
+			     const struct urb *urb)
+{
+	int shift;
+	unsigned int f;
+	unsigned long flags;
+
+	snd_BUG_ON(ep == sender);
+
+	if (snd_usb_endpoint_implict_feedback_sink(ep) && ep->use_count > 0) {
+		/* implicit feedback case */
+		int i, err, bytes = 0;
+		struct urb *out_urb = NULL;
+		struct snd_urb_ctx *in_ctx, *out_ctx;
+
+		in_ctx = urb->context;
+
+		/* Count overall packet size */
+		for (i = 0; i < in_ctx->packets; i++)
+			if (urb->iso_frame_desc[i].status == 0)
+				bytes += urb->iso_frame_desc[i].actual_length;
+
+		/*
+		 * skip empty packets. At least M-Audio's Fast Track Ultra stops
+		 * streaming once it received a 0-byte OUT URB
+		 */
+		if (bytes == 0)
+			return;
+
+		for (i = 0; i < ep->nurbs; i++)
+			if (!test_and_set_bit(i, &ep->active_mask)) {
+				out_urb = ep->urb[i].urb;
+				break;
+			}
+
+		if (!out_urb) {
+			snd_printk(KERN_ERR "Unable to find an urb for playback (nurbs %d)\n",
+				   ep->nurbs);
+			return;
+		}
+
+		out_ctx = out_urb->context;
+		out_ctx->packets = in_ctx->packets;
+
+		/*
+		 * Iterate through the inbound packet and prepare the lengths
+		 * for the output packet. The OUT packet we are about to send
+		 * will have the same amount of payload than the IN packet we
+		 * just received.
+		 */
+
+		for (i = 0; i < in_ctx->packets; i++) {
+			if (urb->iso_frame_desc[i].status == 0)
+				out_ctx->packet_size[i] =
+					urb->iso_frame_desc[i].actual_length / ep->stride;
+			else
+				out_ctx->packet_size[i] = 0;
+		}
+
+		prepare_outbound_urb(ep, out_ctx);
+
+		for (i = 0; i < in_ctx->packets; i++) {
+			if (urb->iso_frame_desc[i].status != 0)
+				continue;
+
+			if (urb->iso_frame_desc[i].actual_length !=
+			    out_urb->iso_frame_desc[i].length)
+				snd_printk(" index %d: in %d, out %d    (%d) stride %d\n", i,
+					urb->iso_frame_desc[i].actual_length,
+					out_urb->iso_frame_desc[i].length,
+					out_ctx->packet_size[i], ep->stride);
+		}
+
+
+		err = usb_submit_urb(out_urb, GFP_ATOMIC);
+		if (err < 0) {
+			snd_printk(KERN_ERR "Unable to submit urb #%d: %d (urb %p)\n",
+				   out_ctx->index, err, out_urb);
+			clear_bit(out_ctx->index, &ep->active_mask);
+		}
+
+		return;
+	}
+
+	if (urb->iso_frame_desc[0].status != 0 ||
+	    urb->iso_frame_desc[0].actual_length < 3)
+		return;
+
+	f = le32_to_cpup(urb->transfer_buffer);
+	if (urb->iso_frame_desc[0].actual_length == 3)
+		f &= 0x00ffffff;
+	else
+		f &= 0x0fffffff;
+
+	if (f == 0)
+		return;
+
+	if (unlikely(ep->freqshift == INT_MIN)) {
+		/*
+		 * The first time we see a feedback value, determine its format
+		 * by shifting it left or right until it matches the nominal
+		 * frequency value.  This assumes that the feedback does not
+		 * differ from the nominal value more than +50% or -25%.
+		 */
+		shift = 0;
+		while (f < ep->freqn - ep->freqn / 4) {
+			f <<= 1;
+			shift++;
+		}
+		while (f > ep->freqn + ep->freqn / 2) {
+			f >>= 1;
+			shift--;
+		}
+		ep->freqshift = shift;
+	}
+	else if (ep->freqshift >= 0)
+		f <<= ep->freqshift;
+	else
+		f >>= -ep->freqshift;
+
+	if (likely(f >= ep->freqn - ep->freqn / 8 && f <= ep->freqmax)) {
+		/*
+		 * If the frequency looks valid, set it.
+		 * This value is referred to in prepare_playback_urb().
+		 */
+		spin_lock_irqsave(&ep->lock, flags);
+		ep->freqm = f;
+		spin_unlock_irqrestore(&ep->lock, flags);
+	} else {
+		/*
+		 * Out of range; maybe the shift value is wrong.
+		 * Reset it so that we autodetect again the next time.
+		 */
+		ep->freqshift = INT_MIN;
+	}
+}
+
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index 88eb63a..6819b83 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -18,4 +18,29 @@ int snd_usb_substream_prepare(struct snd_usb_substream *subs,
 int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int cmd);
 int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd);
 
+
+#define SND_USB_ENDPOINT_TYPE_DATA     0
+#define SND_USB_ENDPOINT_TYPE_SYNC     1
+
+struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
+					      struct usb_host_interface *alts,
+					      int ep_num, int direction, int type);
+
+int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
+				struct snd_pcm_hw_params *hw_params,
+				struct audioformat *fmt,
+				struct snd_usb_endpoint *sync_ep);
+
+int snd_usb_endpoint_start(struct snd_usb_endpoint *ep);
+void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep, int force, int can_sleep);
+void snd_usb_endpoint_activate(struct snd_usb_endpoint *ep);
+void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep);
+void snd_usb_endpoint_free(struct list_head *head);
+
+int snd_usb_endpoint_implict_feedback_sink(struct snd_usb_endpoint *ep);
+
+void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
+			     struct snd_usb_endpoint *sender,
+			     const struct urb *urb);
+
 #endif /* __USBAUDIO_ENDPOINT_H */
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 3e2b035..f1f4b6f 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -46,6 +46,7 @@ struct snd_usb_audio {
 	int num_suspended_intf;
 
 	struct list_head pcm_list;	/* list of pcm streams */
+	struct list_head ep_list;	/* list of audio-related endpoints */
 	int pcm_devs;
 
 	struct list_head midi_list;	/* list of midi interfaces */
-- 
1.7.5.4

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Clemens Ladisch]

Daniel Mack wrote:
> +++ b/sound/usb/card.h
> +struct snd_usb_endpoint {
>  	
> +	struct snd_usb_endpoint *sync_slave;

In theory, there could be more than one slave.

> +struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
>  	...
> +	/* select the alt setting once so the endpoints become valid */
> +	snd_printk("%s() calling usb_set_interface()\n", __func__);
> +	usb_set_interface(ep->chip->dev, ep->iface, ep->alt_idx);

This needs error handling.

> +void snd_usb_endpoint_activate(struct snd_usb_endpoint *ep)
> +{
> +		usb_set_interface(ep->chip->dev, ep->iface, ep->alt_idx);

This too.

> +void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
> +{
> +		for (i = 0; i < in_ctx->packets; i++) {
> +			if (urb->iso_frame_desc[i].status == 0)
> +				out_ctx->packet_size[i] =
> +					urb->iso_frame_desc[i].actual_length / ep->stride;
> +			else
> +				out_ctx->packet_size[i] = 0;

When there is any error in any received packet (status != 0), we do not
know how many frames to send.  Sending zero frames is certain to be
wrong and will break synchronization between the two streams.

As far as I can see, the only way to handle such errors is to stop the
stream(s) and the PCM device(s).


Regards,
Clemens

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Daniel Mack]

On 11/01/2011 09:57 PM, Clemens Ladisch wrote:
> Daniel Mack wrote:
>> +++ b/sound/usb/card.h
>> +struct snd_usb_endpoint {
>>   	
>> +	struct snd_usb_endpoint *sync_slave;
>
> In theory, there could be more than one slave.

Yes, I thought about this too, but for now, I'd like to leave it to a 
later patch to implement this. This patch set already incorporated too 
many changes.


Daniel

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Clemens Ladisch]

Daniel Mack wrote:
> +void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
> +{
> +	if (snd_usb_endpoint_implict_feedback_sink(ep) && ep->use_count > 0) {
>  		...
> +		for (i = 0; i < ep->nurbs; i++)
> +			if (!test_and_set_bit(i, &ep->active_mask)) {
> +				out_urb = ep->urb[i].urb;
> +				break;
> +			}
> +
> +		if (!out_urb) {
> +			snd_printk(KERN_ERR "Unable to find an urb for playback (nurbs %d)\n",
> +				   ep->nurbs);

AFAICS the driver tries for all URBS to be queued at all times.
So if the HC driver completes a sync URB before the corresponding
playback URB, it may be possible for this error to happen.


Regards,
Clemens

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Daniel Mack]

On 11/02/2011 11:26 AM, Clemens Ladisch wrote:
> Daniel Mack wrote:
>> +void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
>> +{
>> +	if (snd_usb_endpoint_implict_feedback_sink(ep)&&  ep->use_count>  0) {
>>   		...
>> +		for (i = 0; i<  ep->nurbs; i++)
>> +			if (!test_and_set_bit(i,&ep->active_mask)) {
>> +				out_urb = ep->urb[i].urb;
>> +				break;
>> +			}
>> +
>> +		if (!out_urb) {
>> +			snd_printk(KERN_ERR "Unable to find an urb for playback (nurbs %d)\n",
>> +				   ep->nurbs);
>
> AFAICS the driver tries for all URBS to be queued at all times.

Hmm, no, not for the "implicit feedback" case. In this mode, we don't 
queue any output urbs on startup but wait for the capture urbs to arrive 
and then queue from there. So this should be ok, right?


Daniel

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Clemens Ladisch]

Daniel Mack wrote:
> On 11/02/2011 11:26 AM, Clemens Ladisch wrote:
>> Daniel Mack wrote:
>>> +void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
>>>  		...
>>> +		snd_printk(KERN_ERR "Unable to find an urb for playback (nurbs %d)\n",
>>> +			   ep->nurbs);
>>
>> AFAICS the driver tries for all URBS to be queued at all times.
> 
> Hmm, no, not for the "implicit feedback" case. In this mode, we don't
> queue any output urbs on startup but wait for the capture urbs to arrive
> and then queue from there. So this should be ok, right?

Example with queue length = 2:

1) startup:
   driver queues both capture URBs
2) 1st capture URB completes:
   driver queues 1st playback URB and requeues 1st capture URB
3) 2nd capture URB completes:
   driver queues 2nd playback URB and requeues 2nd capture URB
   (all URBs are now queued)
4) 1st capture URB completes:
   1st playback URB is still busy
   (The playback URB might have been scheduled for a later frame, and
   even for the same frame, there is no guarantee that the completions
   for different endpoints happen in the same order as the queueing.)


Regards,
Clemens

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Daniel Mack]

On 11/02/2011 04:49 PM, Clemens Ladisch wrote:
> Example with queue length = 2:
>
> 1) startup:
>     driver queues both capture URBs
> 2) 1st capture URB completes:
>     driver queues 1st playback URB and requeues 1st capture URB
> 3) 2nd capture URB completes:
>     driver queues 2nd playback URB and requeues 2nd capture URB
>     (all URBs are now queued)
> 4) 1st capture URB completes:
>     1st playback URB is still busy
>     (The playback URB might have been scheduled for a later frame, and
>     even for the same frame, there is no guarantee that the completions
>     for different endpoints happen in the same order as the queueing.)

I see. However, this really never happened in my tests so far. What 
would be a way to fix this? Queue less capture urbs than available slots 
for playback urbs?


Daniel

Re: [PATCH 1/5] ALSA: snd-usb: implement new endpoint streaming model

[Clemens Ladisch]

Daniel Mack wrote:
> On 11/02/2011 04:49 PM, Clemens Ladisch wrote:
> > Example with queue length = 2:
> >
> > 1) startup:
> >     driver queues both capture URBs
> > 2) 1st capture URB completes:
> >     driver queues 1st playback URB and requeues 1st capture URB
> > 3) 2nd capture URB completes:
> >     driver queues 2nd playback URB and requeues 2nd capture URB
> >     (all URBs are now queued)
> > 4) 1st capture URB completes:
> >     1st playback URB is still busy
> >     (The playback URB might have been scheduled for a later frame, and
> >     even for the same frame, there is no guarantee that the completions
> >     for different endpoints happen in the same order as the queueing.)
> 
> I see. However, this really never happened in my tests so far.

This depends on how the HC drivers organize their internal data
structures.  (Most HCs have one global interrupt handler and check the
endpoint queues in the order in which the endpoints are listed in the
device's descriptors, or by number.  However, xHCI allows multiple
interrupts, so completion can arrive in random order, or even
concurrently.)

> What would be a way to fix this? Queue less capture urbs than available
> slots for playback urbs?

There is still that delay in the first playback URB (IIRC EHCI uses
10 ms), and there is no guaranteed upper bound on interrupt latencies.

Generally speaking, a playback URB can be submitted only when both
itself and the corresponding capture URB have been completed.

In my misc/ua101.c driver, I store 'free' playback URBs and capture
frame counts in FIFOs, and submit playback URBs from a tasklet that is
triggered from both completion handlers.


Regards,
Clemens

[PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Daniel Mack]

---
 sound/usb/card.c     |    6 +-
 sound/usb/card.h     |    5 +
 sound/usb/endpoint.c |   40 -----
 sound/usb/endpoint.h |    3 -
 sound/usb/pcm.c      |  402 +++++++++++++++++++++++++++++++++++++++++---------
 sound/usb/stream.c   |   31 ++++-
 6 files changed, 369 insertions(+), 118 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index 05c1aae..21f6359 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -130,7 +130,6 @@ static void snd_usb_stream_disconnect(struct list_head *head)
 		subs = &as->substream[idx];
 		if (!subs->num_formats)
 			continue;
-		snd_usb_release_substream_urbs(subs, 1);
 		subs->interface = -1;
 	}
 }
@@ -347,6 +346,7 @@ static int snd_usb_audio_create(struct usb_device *dev, int idx,
 	chip->usb_id = USB_ID(le16_to_cpu(dev->descriptor.idVendor),
 			      le16_to_cpu(dev->descriptor.idProduct));
 	INIT_LIST_HEAD(&chip->pcm_list);
+	INIT_LIST_HEAD(&chip->ep_list);
 	INIT_LIST_HEAD(&chip->midi_list);
 	INIT_LIST_HEAD(&chip->mixer_list);
 
@@ -564,6 +564,10 @@ static void snd_usb_audio_disconnect(struct usb_device *dev,
 		list_for_each(p, &chip->pcm_list) {
 			snd_usb_stream_disconnect(p);
 		}
+		/* release the endpoint resources */
+		list_for_each(p, &chip->ep_list) {
+			snd_usb_endpoint_free(p);
+		}
 		/* release the midi resources */
 		list_for_each(p, &chip->midi_list) {
 			snd_usbmidi_disconnect(p);
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 8662702..7fb8778 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -135,6 +135,11 @@ struct snd_usb_substream {
 	struct snd_urb_ctx syncurb[SYNC_URBS];	/* sync urb table */
 	char *syncbuf;				/* sync buffer for all sync URBs */
 	dma_addr_t sync_dma;			/* DMA address of syncbuf */
+	/* data and sync endpoints for this stream */
+	struct snd_usb_endpoint *data_endpoint;
+	struct snd_usb_endpoint *sync_endpoint;
+	unsigned int data_endpoint_started:1;
+	unsigned int sync_endpoint_started:1;
 
 	u64 formats;			/* format bitmasks (all or'ed) */
 	unsigned int num_formats;		/* number of supported audio formats (list) */
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 03a67f2..e0872a0 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -908,46 +908,6 @@ void snd_usb_init_substream(struct snd_usb_stream *as,
 	subs->fmt_type = fp->fmt_type;
 }
 
-int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-	struct snd_usb_substream *subs = substream->runtime->private_data;
-
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		subs->ops.prepare = prepare_playback_urb;
-		return 0;
-	case SNDRV_PCM_TRIGGER_STOP:
-		return deactivate_urbs_old(subs, 0, 0);
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		subs->ops.prepare = prepare_nodata_playback_urb;
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
-int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd)
-{
-	struct snd_usb_substream *subs = substream->runtime->private_data;
-
-	switch (cmd) {
-	case SNDRV_PCM_TRIGGER_START:
-		subs->ops.retire = retire_capture_urb;
-		return start_urbs(subs, substream->runtime);
-	case SNDRV_PCM_TRIGGER_STOP:
-		return deactivate_urbs_old(subs, 0, 0);
-	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
-		subs->ops.retire = retire_paused_capture_urb;
-		return 0;
-	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
-		subs->ops.retire = retire_capture_urb;
-		return 0;
-	}
-
-	return -EINVAL;
-}
-
 int snd_usb_substream_prepare(struct snd_usb_substream *subs,
 			      struct snd_pcm_runtime *runtime)
 {
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index 6819b83..f9d3843 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -15,9 +15,6 @@ void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force);
 int snd_usb_substream_prepare(struct snd_usb_substream *subs,
 			      struct snd_pcm_runtime *runtime);
 
-int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream, int cmd);
-int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd);
-
 
 #define SND_USB_ENDPOINT_TYPE_DATA     0
 #define SND_USB_ENDPOINT_TYPE_SYNC     1
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 0220b0f..af21d38 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -208,6 +208,71 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 	}
 }
 
+static int start_endpoints(struct snd_usb_substream *subs)
+{
+	int err;
+
+	if (!subs->data_endpoint)
+		return -EINVAL;
+
+	if (!subs->data_endpoint_started) {
+		struct snd_usb_endpoint *ep = subs->data_endpoint;
+
+		snd_printk("Starting data EP @%p\n", ep);
+
+		ep->data_subs = subs;
+
+		err = snd_usb_endpoint_start(ep);
+		if (err < 0)
+			return err;
+
+		subs->data_endpoint_started = 1;
+	}
+
+	if (!subs->sync_endpoint_started && subs->sync_endpoint) {
+		struct snd_usb_endpoint *ep = subs->sync_endpoint;
+
+		snd_printk("Starting sync EP @%p\n", ep);
+
+		ep->sync_slave = subs->data_endpoint;
+		err = snd_usb_endpoint_start(ep);
+		if (err < 0)
+			return err;
+
+		subs->sync_endpoint_started = 1;
+	}
+
+	return 0;
+}
+
+static void stop_endpoints(struct snd_usb_substream *subs, int force, int can_sleep)
+{
+	snd_printk("%s(%p,%d,%d)\n", __func__, subs, force, can_sleep);
+
+	if (subs->sync_endpoint_started)
+		snd_usb_endpoint_stop(subs->sync_endpoint, force, can_sleep);
+
+	if (subs->data_endpoint_started)
+		snd_usb_endpoint_stop(subs->data_endpoint, force, can_sleep);
+
+	subs->sync_endpoint_started = 0;
+	subs->data_endpoint_started = 0;
+}
+
+static void activate_endpoints(struct snd_usb_substream *subs)
+{
+	snd_printk("%s(%p)\n", __func__, subs);
+	snd_usb_endpoint_activate(subs->sync_endpoint);
+	snd_usb_endpoint_activate(subs->data_endpoint);
+}
+
+static void deactivate_endpoints(struct snd_usb_substream *subs)
+{
+	snd_printk("%s(%p)\n", __func__, subs);
+	snd_usb_endpoint_deactivate(subs->sync_endpoint);
+	snd_usb_endpoint_deactivate(subs->data_endpoint);
+}
+
 /*
  * find a matching format and set up the interface
  */
@@ -232,40 +297,11 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 	if (fmt == subs->cur_audiofmt)
 		return 0;
 
-	/* close the old interface */
-	if (subs->interface >= 0 && subs->interface != fmt->iface) {
-		if (usb_set_interface(subs->dev, subs->interface, 0) < 0) {
-			snd_printk(KERN_ERR "%d:%d:%d: return to setting 0 failed\n",
-				dev->devnum, fmt->iface, fmt->altsetting);
-			return -EIO;
-		}
-		subs->interface = -1;
-		subs->altset_idx = 0;
-	}
-
-	/* set interface */
-	if (subs->interface != fmt->iface || subs->altset_idx != fmt->altset_idx) {
-		if (usb_set_interface(dev, fmt->iface, fmt->altsetting) < 0) {
-			snd_printk(KERN_ERR "%d:%d:%d: usb_set_interface failed\n",
-				   dev->devnum, fmt->iface, fmt->altsetting);
-			return -EIO;
-		}
-		snd_printdd(KERN_INFO "setting usb interface %d:%d\n", fmt->iface, fmt->altsetting);
-		subs->interface = fmt->iface;
-		subs->altset_idx = fmt->altset_idx;
-	}
-
-	/* create a data pipe */
-	ep = fmt->endpoint & USB_ENDPOINT_NUMBER_MASK;
-	if (is_playback)
-		subs->datapipe = usb_sndisocpipe(dev, ep);
-	else
-		subs->datapipe = usb_rcvisocpipe(dev, ep);
-	subs->datainterval = fmt->datainterval;
-	subs->syncpipe = subs->syncinterval = 0;
-	subs->maxpacksize = fmt->maxpacksize;
-	subs->syncmaxsize = 0;
-	subs->fill_max = 0;
+	subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip,
+						   alts, fmt->endpoint, subs->direction,
+						   SND_USB_ENDPOINT_TYPE_DATA);
+	if (!subs->data_endpoint)
+		return -EINVAL;
 
 	/* we need a sync pipe in async OUT or adaptive IN mode */
 	/* check the number of EP, since some devices have broken
@@ -276,6 +312,15 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
 	     (! is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
 	    altsd->bNumEndpoints >= 2) {
+		switch (subs->stream->chip->usb_id) {
+		case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
+		case USB_ID(0x0763, 0x2081):
+			ep = 0x81;
+			iface = usb_ifnum_to_if(dev, 2);
+			alts = &iface->altsetting[1];
+			goto add_sync_ep;
+		}
+
 		/* check sync-pipe endpoint */
 		/* ... and check descriptor size before accessing bSynchAddress
 		   because there is a version of the SB Audigy 2 NX firmware lacking
@@ -295,28 +340,16 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 				   dev->devnum, fmt->iface, fmt->altsetting);
 			return -EINVAL;
 		}
-		ep &= USB_ENDPOINT_NUMBER_MASK;
-		if (is_playback)
-			subs->syncpipe = usb_rcvisocpipe(dev, ep);
-		else
-			subs->syncpipe = usb_sndisocpipe(dev, ep);
-		if (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
-		    get_endpoint(alts, 1)->bRefresh >= 1 &&
-		    get_endpoint(alts, 1)->bRefresh <= 9)
-			subs->syncinterval = get_endpoint(alts, 1)->bRefresh;
-		else if (snd_usb_get_speed(subs->dev) == USB_SPEED_FULL)
-			subs->syncinterval = 1;
-		else if (get_endpoint(alts, 1)->bInterval >= 1 &&
-			 get_endpoint(alts, 1)->bInterval <= 16)
-			subs->syncinterval = get_endpoint(alts, 1)->bInterval - 1;
-		else
-			subs->syncinterval = 3;
-		subs->syncmaxsize = le16_to_cpu(get_endpoint(alts, 1)->wMaxPacketSize);
-	}
-
-	/* always fill max packet size */
-	if (fmt->attributes & UAC_EP_CS_ATTR_FILL_MAX)
-		subs->fill_max = 1;
+add_sync_ep:
+		subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip,
+							   alts, ep, !subs->direction,
+							   SND_USB_ENDPOINT_TYPE_SYNC);
+
+		if (!subs->sync_endpoint)
+			return -EINVAL;
+
+		subs->data_endpoint->sync_master = subs->sync_endpoint;
+	}
 
 	if ((err = snd_usb_init_pitch(subs->stream->chip, subs->interface, alts, fmt)) < 0)
 		return err;
@@ -390,15 +423,22 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	if (changed) {
 		mutex_lock(&subs->stream->chip->shutdown_mutex);
 		/* format changed */
-		snd_usb_release_substream_urbs(subs, 0);
-		/* influenced: period_bytes, channels, rate, format, */
-		ret = snd_usb_init_substream_urbs(subs, params_period_bytes(hw_params),
-						  params_rate(hw_params),
-						  snd_pcm_format_physical_width(params_format(hw_params)) *
-							params_channels(hw_params));
+		snd_printk("%s(): format changed\n", __func__);
+		stop_endpoints(subs, 0, 1);
+
+		snd_printk(" -- data %p\n", subs->data_endpoint);
+		snd_printk(" -- sync %p\n", subs->sync_endpoint);
+
+		ret = snd_usb_endpoint_set_params(subs->data_endpoint, hw_params, fmt,
+						  subs->sync_endpoint);
+		if (ret == 0 && subs->sync_endpoint)
+			snd_usb_endpoint_set_params(subs->sync_endpoint,
+						    hw_params, fmt, NULL);
 		mutex_unlock(&subs->stream->chip->shutdown_mutex);
 	}
 
+	snd_printk(KERN_ERR "Returning from %s() with %d\n", __func__, ret);
+
 	return ret;
 }
 
@@ -415,7 +455,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 	subs->cur_rate = 0;
 	subs->period_bytes = 0;
 	mutex_lock(&subs->stream->chip->shutdown_mutex);
-	snd_usb_release_substream_urbs(subs, 0);
+	stop_endpoints(subs, 0, 1);
 	mutex_unlock(&subs->stream->chip->shutdown_mutex);
 	return snd_pcm_lib_free_vmalloc_buffer(substream);
 }
@@ -435,19 +475,30 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 		return -ENXIO;
 	}
 
+	if (snd_BUG_ON(!subs->data_endpoint))
+		return -EIO;
+
 	/* some unit conversions in runtime */
-	subs->maxframesize = bytes_to_frames(runtime, subs->maxpacksize);
-	subs->curframesize = bytes_to_frames(runtime, subs->curpacksize);
+	subs->data_endpoint->maxframesize =
+		bytes_to_frames(runtime, subs->data_endpoint->maxpacksize);
+	subs->data_endpoint->curframesize =
+		bytes_to_frames(runtime, subs->data_endpoint->curpacksize);
 
 	/* reset the pointer */
 	subs->hwptr_done = 0;
 	subs->transfer_done = 0;
-	subs->phase = 0;
 	subs->last_delay = 0;
 	subs->last_frame_number = 0;
 	runtime->delay = 0;
 
-	return snd_usb_substream_prepare(subs, runtime);
+	activate_endpoints(subs);
+
+	/* for playback, submit the URBs now; otherwise, the first hwptr_done
+	 * updates for all URBs would happen at the same time when starting */
+	if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK)
+		return start_endpoints(subs);
+
+	return 0;
 }
 
 static struct snd_pcm_hardware snd_usb_hardware =
@@ -843,15 +894,168 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream, int direction)
 	struct snd_usb_stream *as = snd_pcm_substream_chip(substream);
 	struct snd_usb_substream *subs = &as->substream[direction];
 
-	if (!as->chip->shutdown && subs->interface >= 0) {
-		usb_set_interface(subs->dev, subs->interface, 0);
-		subs->interface = -1;
-	}
+	stop_endpoints(subs, 1, 1);
+	deactivate_endpoints(subs);
 	subs->pcm_substream = NULL;
 	snd_usb_autosuspend(subs->stream->chip);
 	return 0;
 }
 
+/* Since a URB can handle only a single linear buffer, we must use double
+ * buffering when the data to be transferred overflows the buffer boundary.
+ * To avoid inconsistencies when updating hwptr_done, we use double buffering
+ * for all URBs.
+ */
+static void retire_capture_urb(struct snd_usb_substream *subs,
+			       struct urb *urb)
+{
+	struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime;
+	unsigned int stride, frames, bytes, oldptr;
+	int i, period_elapsed = 0;
+	unsigned long flags;
+	unsigned char *cp;
+
+	stride = runtime->frame_bits >> 3;
+
+	for (i = 0; i < urb->number_of_packets; i++) {
+		cp = (unsigned char *)urb->transfer_buffer + urb->iso_frame_desc[i].offset;
+		if (urb->iso_frame_desc[i].status) {
+			snd_printd(KERN_ERR "frame %d active: %d\n", i, urb->iso_frame_desc[i].status);
+			// continue;
+		}
+		bytes = urb->iso_frame_desc[i].actual_length;
+		frames = bytes / stride;
+		if (!subs->txfr_quirk)
+			bytes = frames * stride;
+		if (bytes % (runtime->sample_bits >> 3) != 0) {
+#ifdef CONFIG_SND_DEBUG_VERBOSE
+			int oldbytes = bytes;
+#endif
+			bytes = frames * stride;
+			snd_printdd(KERN_ERR "Corrected urb data len. %d->%d\n",
+							oldbytes, bytes);
+		}
+		/* update the current pointer */
+		spin_lock_irqsave(&subs->lock, flags);
+		oldptr = subs->hwptr_done;
+		subs->hwptr_done += bytes;
+		if (subs->hwptr_done >= runtime->buffer_size * stride)
+			subs->hwptr_done -= runtime->buffer_size * stride;
+		frames = (bytes + (oldptr % stride)) / stride;
+		subs->transfer_done += frames;
+		if (subs->transfer_done >= runtime->period_size) {
+			subs->transfer_done -= runtime->period_size;
+			period_elapsed = 1;
+		}
+		spin_unlock_irqrestore(&subs->lock, flags);
+		/* copy a data chunk */
+		if (oldptr + bytes > runtime->buffer_size * stride) {
+			unsigned int bytes1 =
+					runtime->buffer_size * stride - oldptr;
+			memcpy(runtime->dma_area + oldptr, cp, bytes1);
+			memcpy(runtime->dma_area, cp + bytes1, bytes - bytes1);
+		} else {
+			memcpy(runtime->dma_area + oldptr, cp, bytes);
+		}
+	}
+
+	if (period_elapsed)
+		snd_pcm_period_elapsed(subs->pcm_substream);
+}
+
+static void prepare_playback_urb(struct snd_usb_substream *subs,
+				 struct urb *urb)
+{
+	struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime;
+	struct snd_urb_ctx *ctx = urb->context;
+	unsigned int counts, frames, bytes;
+	int i, stride, period_elapsed = 0;
+	unsigned long flags;
+
+	stride = runtime->frame_bits >> 3;
+
+	frames = 0;
+	urb->number_of_packets = 0;
+	spin_lock_irqsave(&subs->lock, flags);
+	for (i = 0; i < ctx->packets; i++) {
+		counts = ctx->packet_size[i];
+		/* set up descriptor */
+		urb->iso_frame_desc[i].offset = frames * stride;
+		urb->iso_frame_desc[i].length = counts * stride;
+		frames += counts;
+		urb->number_of_packets++;
+		subs->transfer_done += counts;
+		if (subs->transfer_done >= runtime->period_size) {
+			subs->transfer_done -= runtime->period_size;
+			period_elapsed = 1;
+			if (subs->fmt_type == UAC_FORMAT_TYPE_II) {
+				if (subs->transfer_done > 0) {
+					/* FIXME: fill-max mode is not
+					 * supported yet */
+					frames -= subs->transfer_done;
+					counts -= subs->transfer_done;
+					urb->iso_frame_desc[i].length =
+						counts * stride;
+					subs->transfer_done = 0;
+				}
+				i++;
+				if (i < ctx->packets) {
+					/* add a transfer delimiter */
+					urb->iso_frame_desc[i].offset =
+						frames * stride;
+					urb->iso_frame_desc[i].length = 0;
+					urb->number_of_packets++;
+				}
+				break;
+			}
+		}
+		if (period_elapsed &&
+		    !snd_usb_endpoint_implict_feedback_sink(subs->data_endpoint)) /* finish at the period boundary */
+			break;
+	}
+	bytes = frames * stride;
+	if (subs->hwptr_done + bytes > runtime->buffer_size * stride) {
+		/* err, the transferred area goes over buffer boundary. */
+		unsigned int bytes1 =
+			runtime->buffer_size * stride - subs->hwptr_done;
+		memcpy(urb->transfer_buffer,
+		       runtime->dma_area + subs->hwptr_done, bytes1);
+		memcpy(urb->transfer_buffer + bytes1,
+		       runtime->dma_area, bytes - bytes1);
+	} else {
+		memcpy(urb->transfer_buffer,
+		       runtime->dma_area + subs->hwptr_done, bytes);
+	}
+	subs->hwptr_done += bytes;
+	if (subs->hwptr_done >= runtime->buffer_size * stride)
+		subs->hwptr_done -= runtime->buffer_size * stride;
+	runtime->delay += frames;
+	spin_unlock_irqrestore(&subs->lock, flags);
+	urb->transfer_buffer_length = bytes;
+	if (period_elapsed)
+		snd_pcm_period_elapsed(subs->pcm_substream);
+}
+
+/*
+ * process after playback data complete
+ * - decrease the delay count again
+ */
+static void retire_playback_urb(struct snd_usb_substream *subs,
+			       struct urb *urb)
+{
+	unsigned long flags;
+	struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime;
+	int stride = runtime->frame_bits >> 3;
+	int processed = urb->transfer_buffer_length / stride;
+
+	spin_lock_irqsave(&subs->lock, flags);
+	if (processed > runtime->delay)
+		runtime->delay = 0;
+	else
+		runtime->delay -= processed;
+	spin_unlock_irqrestore(&subs->lock, flags);
+}
+
 static int snd_usb_playback_open(struct snd_pcm_substream *substream)
 {
 	return snd_usb_pcm_open(substream, SNDRV_PCM_STREAM_PLAYBACK);
@@ -872,6 +1076,62 @@ static int snd_usb_capture_close(struct snd_pcm_substream *substream)
 	return snd_usb_pcm_close(substream, SNDRV_PCM_STREAM_CAPTURE);
 }
 
+static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substream,
+					      int cmd)
+{
+	struct snd_usb_substream *subs = substream->runtime->private_data;
+
+	snd_printk("%s(): cmd %d\n", __func__, cmd);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		snd_printk("START!\n");
+		subs->data_endpoint->prepare_data_urb = prepare_playback_urb;
+		subs->data_endpoint->retire_data_urb = retire_playback_urb;
+		return 0;
+	case SNDRV_PCM_TRIGGER_STOP:
+		stop_endpoints(subs, 0, 0);
+		return 0;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		snd_printk("PAUSE!\n");
+		subs->data_endpoint->prepare_data_urb = NULL;
+		subs->data_endpoint->retire_data_urb = NULL;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
+int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	int err;
+	struct snd_usb_substream *subs = substream->runtime->private_data;
+
+	snd_printk("%s(): cmd %d\n", __func__, cmd);
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		err = start_endpoints(subs);
+		if (err < 0)
+			return err;
+
+		subs->data_endpoint->retire_data_urb = retire_capture_urb;
+		return 0;
+	case SNDRV_PCM_TRIGGER_STOP:
+		stop_endpoints(subs, 0, 0);
+		return 0;
+	case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
+		subs->data_endpoint->retire_data_urb = NULL;
+		return 0;
+	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
+		subs->data_endpoint->retire_data_urb = retire_capture_urb;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 static struct snd_pcm_ops snd_usb_playback_ops = {
 	.open =		snd_usb_playback_open,
 	.close =	snd_usb_playback_close,
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index 5ff8010..6b7d7a2 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -73,6 +73,31 @@ static void snd_usb_audio_pcm_free(struct snd_pcm *pcm)
 	}
 }
 
+/*
+ * initialize the substream instance.
+ */
+
+static void snd_usb_init_substream(struct snd_usb_stream *as,
+				   int stream,
+				   struct audioformat *fp)
+{
+	struct snd_usb_substream *subs = &as->substream[stream];
+
+	INIT_LIST_HEAD(&subs->fmt_list);
+	spin_lock_init(&subs->lock);
+
+	subs->stream = as;
+	subs->direction = stream;
+	subs->dev = as->chip->dev;
+	subs->txfr_quirk = as->chip->txfr_quirk;
+
+	snd_usb_set_pcm_ops(as->pcm, stream);
+
+	list_add_tail(&fp->list, &subs->fmt_list);
+	subs->formats |= fp->formats;
+	subs->num_formats++;
+	subs->fmt_type = fp->fmt_type;
+}
 
 /*
  * add this endpoint to the chip instance.
@@ -94,9 +119,9 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		if (as->fmt_type != fp->fmt_type)
 			continue;
 		subs = &as->substream[stream];
-		if (!subs->endpoint)
+		if (!subs->data_endpoint)
 			continue;
-		if (subs->endpoint == fp->endpoint) {
+		if (subs->data_endpoint->ep_num == fp->endpoint) {
 			list_add_tail(&fp->list, &subs->fmt_list);
 			subs->num_formats++;
 			subs->formats |= fp->formats;
@@ -109,7 +134,7 @@ int snd_usb_add_audio_stream(struct snd_usb_audio *chip,
 		if (as->fmt_type != fp->fmt_type)
 			continue;
 		subs = &as->substream[stream];
-		if (subs->endpoint)
+		if (subs->data_endpoint)
 			continue;
 		err = snd_pcm_new_stream(as->pcm, stream, 1);
 		if (err < 0)
-- 
1.7.5.4

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Clemens Ladisch]

On 10/31/2011 01:10 PM, Daniel Mack wrote:
> @@ -276,6 +312,15 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
>  	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
>  	     (! is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
>  	    altsd->bNumEndpoints >= 2) {
> +		switch (subs->stream->chip->usb_id) {
> +		case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
> +		case USB_ID(0x0763, 0x2081):
> +			ep = 0x81;
> +			iface = usb_ifnum_to_if(dev, 2);
> +			alts = &iface->altsetting[1];

Better check that the device actually has these descriptors.

> +int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd)
> +{
> +	case SNDRV_PCM_TRIGGER_START:
> +		err = start_endpoints(subs);

The trigger callback should execute very quickly; the initial URB
submission should be handled in the prepare callback.


Regards,
Clemens

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Takashi Iwai]

At Tue, 01 Nov 2011 21:57:30 +0100,
Clemens Ladisch wrote:
> 
> On 10/31/2011 01:10 PM, Daniel Mack wrote:
> > @@ -276,6 +312,15 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
> >  	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
> >  	     (! is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
> >  	    altsd->bNumEndpoints >= 2) {
> > +		switch (subs->stream->chip->usb_id) {
> > +		case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
> > +		case USB_ID(0x0763, 0x2081):
> > +			ep = 0x81;
> > +			iface = usb_ifnum_to_if(dev, 2);
> > +			alts = &iface->altsetting[1];
> 
> Better check that the device actually has these descriptors.
> 
> > +int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd)
> > +{
> > +	case SNDRV_PCM_TRIGGER_START:
> > +		err = start_endpoints(subs);
> 
> The trigger callback should execute very quickly; the initial URB
> submission should be handled in the prepare callback.

Indeed this is a PITA since long time ago...  In general, it's wrong
to do it in prepare callback.  But we had to do just as a workaround.

Clemens, I thought you wanted to implement some framework for such
devices?


thanks,

Takashi

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Clemens Ladisch]

Takashi Iwai wrote:
> Clemens, I thought you wanted to implement some framework for such
> devices?

Yes, but I haven't yet found time for an actual _implementation_.
I would be reusing most of the streaming code anyway, so it doesn't
hurt to implement implicit feedback first.


Regards,
Clemens

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Takashi Iwai]

At Wed, 02 Nov 2011 17:18:20 +0100,
Clemens Ladisch wrote:
> 
> Takashi Iwai wrote:
> > Clemens, I thought you wanted to implement some framework for such
> > devices?
> 
> Yes, but I haven't yet found time for an actual _implementation_.

Yeah, so I wrote you "wanted" :)

> I would be reusing most of the streaming code anyway, so it doesn't
> hurt to implement implicit feedback first.

Agreed.


Takashi

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Torsten Schenk]

Hello,

I'm following the discussion about Daniel's patch because I'm at a similar point
with the 6fire driver - separating urb handling from alsa substream handling
and pcm callbacks. One comment catched my attention:

>> The trigger callback should execute very quickly; the initial URB 
>> submission should be handled in the prepare callback. 
> 
>Indeed this is a PITA since long time ago... In general, it's wrong 
>to do it in prepare callback. But we had to do just as a workaround. 
> 

It did because I always thought that would be the right place to start the
transfer. If it is not, what is the prepare callback supposed to do (at least
in the usb case)? Is it simply a check routine for the hw_params?

I also implemented copying data alsa<->urb in a tasklet. This tasklet
might actually also be a good choice for starting urb transfer if that should
be done in the trigger callback. If so, I'll change the 6fire driver
accordingly.

On the other side, the transfer would not necessarily have been started
when trigger finishes - one advantage of doing it in the prepare callback.
Does that matter for the alsa system?

Another question came into my mind: does alsa allow changing the hw_params
after prepare has been called?

Greets,
Torsten

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Daniel Mack]

Hi Torsten,

On 11/02/2011 11:01 PM, Torsten Schenk wrote:
> I'm following the discussion about Daniel's patch because I'm at a similar point
> with the 6fire driver - separating urb handling from alsa substream handling
> and pcm callbacks. One comment catched my attention:

You might be better off waiting for this series to land. Maybe we can
move the code that emerges here to a library that more drivers can use
eventually (including yours, ua101 and also snd-usb-caiaq).


Daniel

Re: [PATCH 2/5] ALSA: snd-usb: switch over to new endpoint streaming logic

[Daniel Mack]

Hi Clemens and Takashi,

I'm back on working on (and hopefully finishing) this series now. One
question that remains is ...

On 11/01/2011 09:57 PM, Clemens Ladisch wrote:
> On 10/31/2011 01:10 PM, Daniel Mack wrote:
>> @@ -276,6 +312,15 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
>>  	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
>>  	     (! is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
>>  	    altsd->bNumEndpoints >= 2) {
>> +		switch (subs->stream->chip->usb_id) {
>> +		case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
>> +		case USB_ID(0x0763, 0x2081):
>> +			ep = 0x81;
>> +			iface = usb_ifnum_to_if(dev, 2);
>> +			alts = &iface->altsetting[1];
> 
> Better check that the device actually has these descriptors.
> 
>> +int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream, int cmd)
>> +{
>> +	case SNDRV_PCM_TRIGGER_START:
>> +		err = start_endpoints(subs);
> 
> The trigger callback should execute very quickly; the initial URB
> submission should be handled in the prepare callback.

This is actually the way the current implementation does it as well. And
there seems to be a reason why the inital submission of urbs is done
differently for the two directions. In the prepare callback, there's
also a comment that says

        /* for playback, submit the URBs now; otherwise, the first
hwptr_done
         * updates for all URBs would happen at the same time when
starting */
        if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK) {
                subs->ops.prepare = prepare_nodata_playback_urb;
                return start_urbs(subs, runtime);
        }

I'm not sure whether my patch set should touch this implementation
detail for now.


Daniel

[PATCH 3/5] ALSA: snd-usb: remove old streaming logic

[Daniel Mack]

---
 sound/usb/endpoint.c |  917 +++-----------------------------------------------
 sound/usb/endpoint.h |   15 -
 2 files changed, 41 insertions(+), 891 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index e0872a0..cceb2ff 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -49,882 +49,6 @@ static inline unsigned get_usb_high_speed_rate(unsigned int rate)
 	return ((rate << 10) + 62) / 125;
 }
 
-/*
- * unlink active urbs.
- */
-static int deactivate_urbs_old(struct snd_usb_substream *subs, int force, int can_sleep)
-{
-	struct snd_usb_audio *chip = subs->stream->chip;
-	unsigned int i;
-	int async;
-
-	subs->running = 0;
-
-	if (!force && subs->stream->chip->shutdown) /* to be sure... */
-		return -EBADFD;
-
-	async = !can_sleep && chip->async_unlink;
-
-	if (!async && in_interrupt())
-		return 0;
-
-	for (i = 0; i < subs->nurbs; i++) {
-		if (test_bit(i, &subs->active_mask)) {
-			if (!test_and_set_bit(i, &subs->unlink_mask)) {
-				struct urb *u = subs->dataurb[i].urb;
-				if (async)
-					usb_unlink_urb(u);
-				else
-					usb_kill_urb(u);
-			}
-		}
-	}
-	if (subs->syncpipe) {
-		for (i = 0; i < SYNC_URBS; i++) {
-			if (test_bit(i+16, &subs->active_mask)) {
-				if (!test_and_set_bit(i+16, &subs->unlink_mask)) {
-					struct urb *u = subs->syncurb[i].urb;
-					if (async)
-						usb_unlink_urb(u);
-					else
-						usb_kill_urb(u);
-				}
-			}
-		}
-	}
-	return 0;
-}
-
-
-/*
- * release a urb data
- */
-static void release_urb_ctx(struct snd_urb_ctx *u)
-{
-	if (u->urb) {
-		if (u->buffer_size)
-			usb_free_coherent(u->subs->dev, u->buffer_size,
-					u->urb->transfer_buffer,
-					u->urb->transfer_dma);
-		usb_free_urb(u->urb);
-		u->urb = NULL;
-	}
-}
-
-/*
- *  wait until all urbs are processed.
- */
-static int wait_clear_urbs_old(struct snd_usb_substream *subs)
-{
-	unsigned long end_time = jiffies + msecs_to_jiffies(1000);
-	unsigned int i;
-	int alive;
-
-	do {
-		alive = 0;
-		for (i = 0; i < subs->nurbs; i++) {
-			if (test_bit(i, &subs->active_mask))
-				alive++;
-		}
-		if (subs->syncpipe) {
-			for (i = 0; i < SYNC_URBS; i++) {
-				if (test_bit(i + 16, &subs->active_mask))
-					alive++;
-			}
-		}
-		if (! alive)
-			break;
-		schedule_timeout_uninterruptible(1);
-	} while (time_before(jiffies, end_time));
-	if (alive)
-		snd_printk(KERN_ERR "timeout: still %d active urbs..\n", alive);
-	return 0;
-}
-
-/*
- * release a substream
- */
-void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force)
-{
-	int i;
-
-	/* stop urbs (to be sure) */
-	deactivate_urbs_old(subs, force, 1);
-	wait_clear_urbs_old(subs);
-
-	for (i = 0; i < MAX_URBS; i++)
-		release_urb_ctx(&subs->dataurb[i]);
-	for (i = 0; i < SYNC_URBS; i++)
-		release_urb_ctx(&subs->syncurb[i]);
-	usb_free_coherent(subs->dev, SYNC_URBS * 4,
-			subs->syncbuf, subs->sync_dma);
-	subs->syncbuf = NULL;
-	subs->nurbs = 0;
-}
-
-/*
- * complete callback from data urb
- */
-static void snd_complete_urb_old(struct urb *urb)
-{
-	struct snd_urb_ctx *ctx = urb->context;
-	struct snd_usb_substream *subs = ctx->subs;
-	struct snd_pcm_substream *substream = ctx->subs->pcm_substream;
-	int err = 0;
-
-	if ((subs->running && subs->ops.retire(subs, substream->runtime, urb)) ||
-	    !subs->running || /* can be stopped during retire callback */
-	    (err = subs->ops.prepare(subs, substream->runtime, urb)) < 0 ||
-	    (err = usb_submit_urb(urb, GFP_ATOMIC)) < 0) {
-		clear_bit(ctx->index, &subs->active_mask);
-		if (err < 0) {
-			snd_printd(KERN_ERR "cannot submit urb (err = %d)\n", err);
-			snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
-		}
-	}
-}
-
-
-/*
- * complete callback from sync urb
- */
-static void snd_complete_sync_urb(struct urb *urb)
-{
-	struct snd_urb_ctx *ctx = urb->context;
-	struct snd_usb_substream *subs = ctx->subs;
-	struct snd_pcm_substream *substream = ctx->subs->pcm_substream;
-	int err = 0;
-
-	if ((subs->running && subs->ops.retire_sync(subs, substream->runtime, urb)) ||
-	    !subs->running || /* can be stopped during retire callback */
-	    (err = subs->ops.prepare_sync(subs, substream->runtime, urb)) < 0 ||
-	    (err = usb_submit_urb(urb, GFP_ATOMIC)) < 0) {
-		clear_bit(ctx->index + 16, &subs->active_mask);
-		if (err < 0) {
-			snd_printd(KERN_ERR "cannot submit sync urb (err = %d)\n", err);
-			snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN);
-		}
-	}
-}
-
-
-/*
- * initialize a substream for plaback/capture
- */
-int snd_usb_init_substream_urbs(struct snd_usb_substream *subs,
-				unsigned int period_bytes,
-				unsigned int rate,
-				unsigned int frame_bits)
-{
-	unsigned int maxsize, i;
-	int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK;
-	unsigned int urb_packs, total_packs, packs_per_ms;
-	struct snd_usb_audio *chip = subs->stream->chip;
-
-	/* calculate the frequency in 16.16 format */
-	if (snd_usb_get_speed(subs->dev) == USB_SPEED_FULL)
-		subs->freqn = get_usb_full_speed_rate(rate);
-	else
-		subs->freqn = get_usb_high_speed_rate(rate);
-	subs->freqm = subs->freqn;
-	subs->freqshift = INT_MIN;
-	/* calculate max. frequency */
-	if (subs->maxpacksize) {
-		/* whatever fits into a max. size packet */
-		maxsize = subs->maxpacksize;
-		subs->freqmax = (maxsize / (frame_bits >> 3))
-				<< (16 - subs->datainterval);
-	} else {
-		/* no max. packet size: just take 25% higher than nominal */
-		subs->freqmax = subs->freqn + (subs->freqn >> 2);
-		maxsize = ((subs->freqmax + 0xffff) * (frame_bits >> 3))
-				>> (16 - subs->datainterval);
-	}
-	subs->phase = 0;
-
-	if (subs->fill_max)
-		subs->curpacksize = subs->maxpacksize;
-	else
-		subs->curpacksize = maxsize;
-
-	if (snd_usb_get_speed(subs->dev) != USB_SPEED_FULL)
-		packs_per_ms = 8 >> subs->datainterval;
-	else
-		packs_per_ms = 1;
-
-	if (is_playback) {
-		urb_packs = max(chip->nrpacks, 1);
-		urb_packs = min(urb_packs, (unsigned int)MAX_PACKS);
-	} else
-		urb_packs = 1;
-	urb_packs *= packs_per_ms;
-	if (subs->syncpipe)
-		urb_packs = min(urb_packs, 1U << subs->syncinterval);
-
-	/* decide how many packets to be used */
-	if (is_playback) {
-		unsigned int minsize, maxpacks;
-		/* determine how small a packet can be */
-		minsize = (subs->freqn >> (16 - subs->datainterval))
-			  * (frame_bits >> 3);
-		/* with sync from device, assume it can be 12% lower */
-		if (subs->syncpipe)
-			minsize -= minsize >> 3;
-		minsize = max(minsize, 1u);
-		total_packs = (period_bytes + minsize - 1) / minsize;
-		/* we need at least two URBs for queueing */
-		if (total_packs < 2) {
-			total_packs = 2;
-		} else {
-			/* and we don't want too long a queue either */
-			maxpacks = max(MAX_QUEUE * packs_per_ms, urb_packs * 2);
-			total_packs = min(total_packs, maxpacks);
-		}
-	} else {
-		while (urb_packs > 1 && urb_packs * maxsize >= period_bytes)
-			urb_packs >>= 1;
-		total_packs = MAX_URBS * urb_packs;
-	}
-	subs->nurbs = (total_packs + urb_packs - 1) / urb_packs;
-	if (subs->nurbs > MAX_URBS) {
-		/* too much... */
-		subs->nurbs = MAX_URBS;
-		total_packs = MAX_URBS * urb_packs;
-	} else if (subs->nurbs < 2) {
-		/* too little - we need at least two packets
-		 * to ensure contiguous playback/capture
-		 */
-		subs->nurbs = 2;
-	}
-
-	/* allocate and initialize data urbs */
-	for (i = 0; i < subs->nurbs; i++) {
-		struct snd_urb_ctx *u = &subs->dataurb[i];
-		u->index = i;
-		u->subs = subs;
-		u->packets = (i + 1) * total_packs / subs->nurbs
-			- i * total_packs / subs->nurbs;
-		u->buffer_size = maxsize * u->packets;
-		if (subs->fmt_type == UAC_FORMAT_TYPE_II)
-			u->packets++; /* for transfer delimiter */
-		u->urb = usb_alloc_urb(u->packets, GFP_KERNEL);
-		if (!u->urb)
-			goto out_of_memory;
-		u->urb->transfer_buffer =
-			usb_alloc_coherent(subs->dev, u->buffer_size,
-					   GFP_KERNEL, &u->urb->transfer_dma);
-		if (!u->urb->transfer_buffer)
-			goto out_of_memory;
-		u->urb->pipe = subs->datapipe;
-		u->urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
-		u->urb->interval = 1 << subs->datainterval;
-		u->urb->context = u;
-		u->urb->complete = snd_complete_urb_old;
-	}
-
-	if (subs->syncpipe) {
-		/* allocate and initialize sync urbs */
-		subs->syncbuf = usb_alloc_coherent(subs->dev, SYNC_URBS * 4,
-						 GFP_KERNEL, &subs->sync_dma);
-		if (!subs->syncbuf)
-			goto out_of_memory;
-		for (i = 0; i < SYNC_URBS; i++) {
-			struct snd_urb_ctx *u = &subs->syncurb[i];
-			u->index = i;
-			u->subs = subs;
-			u->packets = 1;
-			u->urb = usb_alloc_urb(1, GFP_KERNEL);
-			if (!u->urb)
-				goto out_of_memory;
-			u->urb->transfer_buffer = subs->syncbuf + i * 4;
-			u->urb->transfer_dma = subs->sync_dma + i * 4;
-			u->urb->transfer_buffer_length = 4;
-			u->urb->pipe = subs->syncpipe;
-			u->urb->transfer_flags = URB_ISO_ASAP |
-						 URB_NO_TRANSFER_DMA_MAP;
-			u->urb->number_of_packets = 1;
-			u->urb->interval = 1 << subs->syncinterval;
-			u->urb->context = u;
-			u->urb->complete = snd_complete_sync_urb;
-		}
-	}
-	return 0;
-
-out_of_memory:
-	snd_usb_release_substream_urbs(subs, 0);
-	return -ENOMEM;
-}
-
-/*
- * prepare urb for full speed capture sync pipe
- *
- * fill the length and offset of each urb descriptor.
- * the fixed 10.14 frequency is passed through the pipe.
- */
-static int prepare_capture_sync_urb(struct snd_usb_substream *subs,
-				    struct snd_pcm_runtime *runtime,
-				    struct urb *urb)
-{
-	unsigned char *cp = urb->transfer_buffer;
-	struct snd_urb_ctx *ctx = urb->context;
-
-	urb->dev = ctx->subs->dev; /* we need to set this at each time */
-	urb->iso_frame_desc[0].length = 3;
-	urb->iso_frame_desc[0].offset = 0;
-	cp[0] = subs->freqn >> 2;
-	cp[1] = subs->freqn >> 10;
-	cp[2] = subs->freqn >> 18;
-	return 0;
-}
-
-/*
- * prepare urb for high speed capture sync pipe
- *
- * fill the length and offset of each urb descriptor.
- * the fixed 12.13 frequency is passed as 16.16 through the pipe.
- */
-static int prepare_capture_sync_urb_hs(struct snd_usb_substream *subs,
-				       struct snd_pcm_runtime *runtime,
-				       struct urb *urb)
-{
-	unsigned char *cp = urb->transfer_buffer;
-	struct snd_urb_ctx *ctx = urb->context;
-
-	urb->dev = ctx->subs->dev; /* we need to set this at each time */
-	urb->iso_frame_desc[0].length = 4;
-	urb->iso_frame_desc[0].offset = 0;
-	cp[0] = subs->freqn;
-	cp[1] = subs->freqn >> 8;
-	cp[2] = subs->freqn >> 16;
-	cp[3] = subs->freqn >> 24;
-	return 0;
-}
-
-/*
- * process after capture sync complete
- * - nothing to do
- */
-static int retire_capture_sync_urb(struct snd_usb_substream *subs,
-				   struct snd_pcm_runtime *runtime,
-				   struct urb *urb)
-{
-	return 0;
-}
-
-/*
- * prepare urb for capture data pipe
- *
- * fill the offset and length of each descriptor.
- *
- * we use a temporary buffer to write the captured data.
- * since the length of written data is determined by host, we cannot
- * write onto the pcm buffer directly...  the data is thus copied
- * later at complete callback to the global buffer.
- */
-static int prepare_capture_urb(struct snd_usb_substream *subs,
-			       struct snd_pcm_runtime *runtime,
-			       struct urb *urb)
-{
-	int i, offs;
-	struct snd_urb_ctx *ctx = urb->context;
-
-	offs = 0;
-	urb->dev = ctx->subs->dev; /* we need to set this at each time */
-	for (i = 0; i < ctx->packets; i++) {
-		urb->iso_frame_desc[i].offset = offs;
-		urb->iso_frame_desc[i].length = subs->curpacksize;
-		offs += subs->curpacksize;
-	}
-	urb->transfer_buffer_length = offs;
-	urb->number_of_packets = ctx->packets;
-	return 0;
-}
-
-/*
- * process after capture complete
- *
- * copy the data from each desctiptor to the pcm buffer, and
- * update the current position.
- */
-static int retire_capture_urb(struct snd_usb_substream *subs,
-			      struct snd_pcm_runtime *runtime,
-			      struct urb *urb)
-{
-	unsigned long flags;
-	unsigned char *cp;
-	int i;
-	unsigned int stride, frames, bytes, oldptr;
-	int period_elapsed = 0;
-
-	stride = runtime->frame_bits >> 3;
-
-	for (i = 0; i < urb->number_of_packets; i++) {
-		cp = (unsigned char *)urb->transfer_buffer + urb->iso_frame_desc[i].offset;
-		if (urb->iso_frame_desc[i].status) {
-			snd_printd(KERN_ERR "frame %d active: %d\n", i, urb->iso_frame_desc[i].status);
-			// continue;
-		}
-		bytes = urb->iso_frame_desc[i].actual_length;
-		frames = bytes / stride;
-		if (!subs->txfr_quirk)
-			bytes = frames * stride;
-		if (bytes % (runtime->sample_bits >> 3) != 0) {
-#ifdef CONFIG_SND_DEBUG_VERBOSE
-			int oldbytes = bytes;
-#endif
-			bytes = frames * stride;
-			snd_printdd(KERN_ERR "Corrected urb data len. %d->%d\n",
-							oldbytes, bytes);
-		}
-		/* update the current pointer */
-		spin_lock_irqsave(&subs->lock, flags);
-		oldptr = subs->hwptr_done;
-		subs->hwptr_done += bytes;
-		if (subs->hwptr_done >= runtime->buffer_size * stride)
-			subs->hwptr_done -= runtime->buffer_size * stride;
-		frames = (bytes + (oldptr % stride)) / stride;
-		subs->transfer_done += frames;
-		if (subs->transfer_done >= runtime->period_size) {
-			subs->transfer_done -= runtime->period_size;
-			period_elapsed = 1;
-		}
-		spin_unlock_irqrestore(&subs->lock, flags);
-		/* copy a data chunk */
-		if (oldptr + bytes > runtime->buffer_size * stride) {
-			unsigned int bytes1 =
-					runtime->buffer_size * stride - oldptr;
-			memcpy(runtime->dma_area + oldptr, cp, bytes1);
-			memcpy(runtime->dma_area, cp + bytes1, bytes - bytes1);
-		} else {
-			memcpy(runtime->dma_area + oldptr, cp, bytes);
-		}
-	}
-	if (period_elapsed)
-		snd_pcm_period_elapsed(subs->pcm_substream);
-	return 0;
-}
-
-/*
- * Process after capture complete when paused.  Nothing to do.
- */
-static int retire_paused_capture_urb(struct snd_usb_substream *subs,
-				     struct snd_pcm_runtime *runtime,
-				     struct urb *urb)
-{
-	return 0;
-}
-
-
-/*
- * prepare urb for playback sync pipe
- *
- * set up the offset and length to receive the current frequency.
- */
-static int prepare_playback_sync_urb(struct snd_usb_substream *subs,
-				     struct snd_pcm_runtime *runtime,
-				     struct urb *urb)
-{
-	struct snd_urb_ctx *ctx = urb->context;
-
-	urb->dev = ctx->subs->dev; /* we need to set this at each time */
-	urb->iso_frame_desc[0].length = min(4u, ctx->subs->syncmaxsize);
-	urb->iso_frame_desc[0].offset = 0;
-	return 0;
-}
-
-/*
- * process after playback sync complete
- *
- * Full speed devices report feedback values in 10.14 format as samples per
- * frame, high speed devices in 16.16 format as samples per microframe.
- * Because the Audio Class 1 spec was written before USB 2.0, many high speed
- * devices use a wrong interpretation, some others use an entirely different
- * format.  Therefore, we cannot predict what format any particular device uses
- * and must detect it automatically.
- */
-static int retire_playback_sync_urb(struct snd_usb_substream *subs,
-				    struct snd_pcm_runtime *runtime,
-				    struct urb *urb)
-{
-	unsigned int f;
-	int shift;
-	unsigned long flags;
-
-	if (urb->iso_frame_desc[0].status != 0 ||
-	    urb->iso_frame_desc[0].actual_length < 3)
-		return 0;
-
-	f = le32_to_cpup(urb->transfer_buffer);
-	if (urb->iso_frame_desc[0].actual_length == 3)
-		f &= 0x00ffffff;
-	else
-		f &= 0x0fffffff;
-	if (f == 0)
-		return 0;
-
-	if (unlikely(subs->freqshift == INT_MIN)) {
-		/*
-		 * The first time we see a feedback value, determine its format
-		 * by shifting it left or right until it matches the nominal
-		 * frequency value.  This assumes that the feedback does not
-		 * differ from the nominal value more than +50% or -25%.
-		 */
-		shift = 0;
-		while (f < subs->freqn - subs->freqn / 4) {
-			f <<= 1;
-			shift++;
-		}
-		while (f > subs->freqn + subs->freqn / 2) {
-			f >>= 1;
-			shift--;
-		}
-		subs->freqshift = shift;
-	}
-	else if (subs->freqshift >= 0)
-		f <<= subs->freqshift;
-	else
-		f >>= -subs->freqshift;
-
-	if (likely(f >= subs->freqn - subs->freqn / 8 && f <= subs->freqmax)) {
-		/*
-		 * If the frequency looks valid, set it.
-		 * This value is referred to in prepare_playback_urb().
-		 */
-		spin_lock_irqsave(&subs->lock, flags);
-		subs->freqm = f;
-		spin_unlock_irqrestore(&subs->lock, flags);
-	} else {
-		/*
-		 * Out of range; maybe the shift value is wrong.
-		 * Reset it so that we autodetect again the next time.
-		 */
-		subs->freqshift = INT_MIN;
-	}
-
-	return 0;
-}
-
-/* determine the number of frames in the next packet */
-static int snd_usb_audio_next_packet_size(struct snd_usb_substream *subs)
-{
-	if (subs->fill_max)
-		return subs->maxframesize;
-	else {
-		subs->phase = (subs->phase & 0xffff)
-			+ (subs->freqm << subs->datainterval);
-		return min(subs->phase >> 16, subs->maxframesize);
-	}
-}
-
-/*
- * Prepare urb for streaming before playback starts or when paused.
- *
- * We don't have any data, so we send silence.
- */
-static int prepare_nodata_playback_urb(struct snd_usb_substream *subs,
-				       struct snd_pcm_runtime *runtime,
-				       struct urb *urb)
-{
-	unsigned int i, offs, counts;
-	struct snd_urb_ctx *ctx = urb->context;
-	int stride = runtime->frame_bits >> 3;
-
-	offs = 0;
-	urb->dev = ctx->subs->dev;
-	for (i = 0; i < ctx->packets; ++i) {
-		counts = snd_usb_audio_next_packet_size(subs);
-		urb->iso_frame_desc[i].offset = offs * stride;
-		urb->iso_frame_desc[i].length = counts * stride;
-		offs += counts;
-	}
-	urb->number_of_packets = ctx->packets;
-	urb->transfer_buffer_length = offs * stride;
-	memset(urb->transfer_buffer,
-	       runtime->format == SNDRV_PCM_FORMAT_U8 ? 0x80 : 0,
-	       offs * stride);
-	return 0;
-}
-
-/*
- * prepare urb for playback data pipe
- *
- * Since a URB can handle only a single linear buffer, we must use double
- * buffering when the data to be transferred overflows the buffer boundary.
- * To avoid inconsistencies when updating hwptr_done, we use double buffering
- * for all URBs.
- */
-static int prepare_playback_urb(struct snd_usb_substream *subs,
-				struct snd_pcm_runtime *runtime,
-				struct urb *urb)
-{
-	int i, stride;
-	unsigned int counts, frames, bytes;
-	unsigned long flags;
-	int period_elapsed = 0;
-	struct snd_urb_ctx *ctx = urb->context;
-
-	stride = runtime->frame_bits >> 3;
-
-	frames = 0;
-	urb->dev = ctx->subs->dev; /* we need to set this at each time */
-	urb->number_of_packets = 0;
-	spin_lock_irqsave(&subs->lock, flags);
-	for (i = 0; i < ctx->packets; i++) {
-		counts = snd_usb_audio_next_packet_size(subs);
-		/* set up descriptor */
-		urb->iso_frame_desc[i].offset = frames * stride;
-		urb->iso_frame_desc[i].length = counts * stride;
-		frames += counts;
-		urb->number_of_packets++;
-		subs->transfer_done += counts;
-		if (subs->transfer_done >= runtime->period_size) {
-			subs->transfer_done -= runtime->period_size;
-			period_elapsed = 1;
-			if (subs->fmt_type == UAC_FORMAT_TYPE_II) {
-				if (subs->transfer_done > 0) {
-					/* FIXME: fill-max mode is not
-					 * supported yet */
-					frames -= subs->transfer_done;
-					counts -= subs->transfer_done;
-					urb->iso_frame_desc[i].length =
-						counts * stride;
-					subs->transfer_done = 0;
-				}
-				i++;
-				if (i < ctx->packets) {
-					/* add a transfer delimiter */
-					urb->iso_frame_desc[i].offset =
-						frames * stride;
-					urb->iso_frame_desc[i].length = 0;
-					urb->number_of_packets++;
-				}
-				break;
-			}
-		}
-		if (period_elapsed) /* finish at the period boundary */
-			break;
-	}
-	bytes = frames * stride;
-	if (subs->hwptr_done + bytes > runtime->buffer_size * stride) {
-		/* err, the transferred area goes over buffer boundary. */
-		unsigned int bytes1 =
-			runtime->buffer_size * stride - subs->hwptr_done;
-		memcpy(urb->transfer_buffer,
-		       runtime->dma_area + subs->hwptr_done, bytes1);
-		memcpy(urb->transfer_buffer + bytes1,
-		       runtime->dma_area, bytes - bytes1);
-	} else {
-		memcpy(urb->transfer_buffer,
-		       runtime->dma_area + subs->hwptr_done, bytes);
-	}
-	subs->hwptr_done += bytes;
-	if (subs->hwptr_done >= runtime->buffer_size * stride)
-		subs->hwptr_done -= runtime->buffer_size * stride;
-
-	/* update delay with exact number of samples queued */
-	runtime->delay = subs->last_delay;
-	runtime->delay += frames;
-	subs->last_delay = runtime->delay;
-
-	/* realign last_frame_number */
-	subs->last_frame_number = usb_get_current_frame_number(subs->dev);
-	subs->last_frame_number &= 0xFF; /* keep 8 LSBs */
-
-	spin_unlock_irqrestore(&subs->lock, flags);
-	urb->transfer_buffer_length = bytes;
-	if (period_elapsed)
-		snd_pcm_period_elapsed(subs->pcm_substream);
-	return 0;
-}
-
-/*
- * process after playback data complete
- * - decrease the delay count again
- */
-static int retire_playback_urb(struct snd_usb_substream *subs,
-			       struct snd_pcm_runtime *runtime,
-			       struct urb *urb)
-{
-	unsigned long flags;
-	int stride = runtime->frame_bits >> 3;
-	int processed = urb->transfer_buffer_length / stride;
-	int est_delay;
-
-	spin_lock_irqsave(&subs->lock, flags);
-
-	est_delay = snd_usb_pcm_delay(subs, runtime->rate);
-	/* update delay with exact number of samples played */
-	if (processed > subs->last_delay)
-		subs->last_delay = 0;
-	else
-		subs->last_delay -= processed;
-	runtime->delay = subs->last_delay;
-
-	/*
-	 * Report when delay estimate is off by more than 2ms.
-	 * The error should be lower than 2ms since the estimate relies
-	 * on two reads of a counter updated every ms.
-	 */
-	if (abs(est_delay - subs->last_delay) * 1000 > runtime->rate * 2)
-		snd_printk(KERN_DEBUG "delay: estimated %d, actual %d\n",
-			est_delay, subs->last_delay);
-
-	spin_unlock_irqrestore(&subs->lock, flags);
-	return 0;
-}
-
-static const char *usb_error_string(int err)
-{
-	switch (err) {
-	case -ENODEV:
-		return "no device";
-	case -ENOENT:
-		return "endpoint not enabled";
-	case -EPIPE:
-		return "endpoint stalled";
-	case -ENOSPC:
-		return "not enough bandwidth";
-	case -ESHUTDOWN:
-		return "device disabled";
-	case -EHOSTUNREACH:
-		return "device suspended";
-	case -EINVAL:
-	case -EAGAIN:
-	case -EFBIG:
-	case -EMSGSIZE:
-		return "internal error";
-	default:
-		return "unknown error";
-	}
-}
-
-/*
- * set up and start data/sync urbs
- */
-static int start_urbs(struct snd_usb_substream *subs, struct snd_pcm_runtime *runtime)
-{
-	unsigned int i;
-	int err;
-
-	if (subs->stream->chip->shutdown)
-		return -EBADFD;
-
-	for (i = 0; i < subs->nurbs; i++) {
-		if (snd_BUG_ON(!subs->dataurb[i].urb))
-			return -EINVAL;
-		if (subs->ops.prepare(subs, runtime, subs->dataurb[i].urb) < 0) {
-			snd_printk(KERN_ERR "cannot prepare datapipe for urb %d\n", i);
-			goto __error;
-		}
-	}
-	if (subs->syncpipe) {
-		for (i = 0; i < SYNC_URBS; i++) {
-			if (snd_BUG_ON(!subs->syncurb[i].urb))
-				return -EINVAL;
-			if (subs->ops.prepare_sync(subs, runtime, subs->syncurb[i].urb) < 0) {
-				snd_printk(KERN_ERR "cannot prepare syncpipe for urb %d\n", i);
-				goto __error;
-			}
-		}
-	}
-
-	subs->active_mask = 0;
-	subs->unlink_mask = 0;
-	subs->running = 1;
-	for (i = 0; i < subs->nurbs; i++) {
-		err = usb_submit_urb(subs->dataurb[i].urb, GFP_ATOMIC);
-		if (err < 0) {
-			snd_printk(KERN_ERR "cannot submit datapipe "
-				   "for urb %d, error %d: %s\n",
-				   i, err, usb_error_string(err));
-			goto __error;
-		}
-		set_bit(i, &subs->active_mask);
-	}
-	if (subs->syncpipe) {
-		for (i = 0; i < SYNC_URBS; i++) {
-			err = usb_submit_urb(subs->syncurb[i].urb, GFP_ATOMIC);
-			if (err < 0) {
-				snd_printk(KERN_ERR "cannot submit syncpipe "
-					   "for urb %d, error %d: %s\n",
-					   i, err, usb_error_string(err));
-				goto __error;
-			}
-			set_bit(i + 16, &subs->active_mask);
-		}
-	}
-	return 0;
-
- __error:
-	// snd_pcm_stop(subs->pcm_substream, SNDRV_PCM_STATE_XRUN);
-	deactivate_urbs_old(subs, 0, 0);
-	return -EPIPE;
-}
-
-
-/*
- */
-static struct snd_urb_ops audio_urb_ops[2] = {
-	{
-		.prepare =	prepare_nodata_playback_urb,
-		.retire =	retire_playback_urb,
-		.prepare_sync =	prepare_playback_sync_urb,
-		.retire_sync =	retire_playback_sync_urb,
-	},
-	{
-		.prepare =	prepare_capture_urb,
-		.retire =	retire_capture_urb,
-		.prepare_sync =	prepare_capture_sync_urb,
-		.retire_sync =	retire_capture_sync_urb,
-	},
-};
-
-/*
- * initialize the substream instance.
- */
-
-void snd_usb_init_substream(struct snd_usb_stream *as,
-			    int stream, struct audioformat *fp)
-{
-	struct snd_usb_substream *subs = &as->substream[stream];
-
-	INIT_LIST_HEAD(&subs->fmt_list);
-	spin_lock_init(&subs->lock);
-
-	subs->stream = as;
-	subs->direction = stream;
-	subs->dev = as->chip->dev;
-	subs->txfr_quirk = as->chip->txfr_quirk;
-	subs->ops = audio_urb_ops[stream];
-	if (snd_usb_get_speed(subs->dev) >= USB_SPEED_HIGH)
-		subs->ops.prepare_sync = prepare_capture_sync_urb_hs;
-
-	snd_usb_set_pcm_ops(as->pcm, stream);
-
-	list_add_tail(&fp->list, &subs->fmt_list);
-	subs->formats |= fp->formats;
-	subs->endpoint = fp->endpoint;
-	subs->num_formats++;
-	subs->fmt_type = fp->fmt_type;
-}
-
-int snd_usb_substream_prepare(struct snd_usb_substream *subs,
-			      struct snd_pcm_runtime *runtime)
-{
-	/* clear urbs (to be sure) */
-	deactivate_urbs_old(subs, 0, 1);
-	wait_clear_urbs_old(subs);
-
-	/* for playback, submit the URBs now; otherwise, the first hwptr_done
-	 * updates for all URBs would happen at the same time when starting */
-	if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK) {
-		subs->ops.prepare = prepare_nodata_playback_urb;
-		return start_urbs(subs, runtime);
-	}
-
-	return 0;
-}
-
 int snd_usb_endpoint_implict_feedback_sink(struct snd_usb_endpoint *ep)
 {
 	return  ep->sync_master &&
@@ -1174,6 +298,47 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip,
 	return ep;
 }
 
+static const char *usb_error_string(int err)
+{
+	switch (err) {
+	case -ENODEV:
+		return "no device";
+	case -ENOENT:
+		return "endpoint not enabled";
+	case -EPIPE:
+		return "endpoint stalled";
+	case -ENOSPC:
+		return "not enough bandwidth";
+	case -ESHUTDOWN:
+		return "device disabled";
+	case -EHOSTUNREACH:
+		return "device suspended";
+	case -EINVAL:
+	case -EAGAIN:
+	case -EFBIG:
+	case -EMSGSIZE:
+		return "internal error";
+	default:
+		return "unknown error";
+	}
+}
+
+/*
+ * release a urb data
+ */
+static void release_urb_ctx(struct snd_urb_ctx *u)
+{
+	if (!u->urb)
+		return;
+
+	if (u->buffer_size)
+		usb_free_coherent(u->ep->chip->dev, u->buffer_size,
+				  u->urb->transfer_buffer,
+				  u->urb->transfer_dma);
+	usb_free_urb(u->urb);
+	u->urb = NULL;
+}
+
 /*
  *  wait until all urbs are processed.
  */
diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h
index f9d3843..e499050 100644
--- a/sound/usb/endpoint.h
+++ b/sound/usb/endpoint.h
@@ -1,21 +1,6 @@
 #ifndef __USBAUDIO_ENDPOINT_H
 #define __USBAUDIO_ENDPOINT_H
 
-void snd_usb_init_substream(struct snd_usb_stream *as,
-			    int stream,
-			    struct audioformat *fp);
-
-int snd_usb_init_substream_urbs(struct snd_usb_substream *subs,
-				unsigned int period_bytes,
-				unsigned int rate,
-				unsigned int frame_bits);
-
-void snd_usb_release_substream_urbs(struct snd_usb_substream *subs, int force);
-
-int snd_usb_substream_prepare(struct snd_usb_substream *subs,
-			      struct snd_pcm_runtime *runtime);
-
-
 #define SND_USB_ENDPOINT_TYPE_DATA     0
 #define SND_USB_ENDPOINT_TYPE_SYNC     1
 
-- 
1.7.5.4

[PATCH 4/5] ALSA: snd-usb: set MAX_URBS to 16

[Daniel Mack]

This is needed for implicity feedback data streams.
---
 sound/usb/card.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h
index 7fb8778..0a5515a 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -3,7 +3,7 @@
 
 #define MAX_PACKS	20
 #define MAX_PACKS_HS	(MAX_PACKS * 8)	/* in high speed mode */
-#define MAX_URBS	8
+#define MAX_URBS	16
 #define SYNC_URBS	4	/* always four urbs for sync */
 #define MAX_QUEUE	24	/* try not to exceed this queue length, in ms */
 
-- 
1.7.5.4

Re: [PATCH 4/5] ALSA: snd-usb: set MAX_URBS to 16

[Clemens Ladisch]

Daniel Mack wrote:
> This is needed for implicity feedback data streams.

Why?

> -#define MAX_URBS	8
> +#define MAX_URBS	16


Regards,
Clemens

[PATCH 5/5] ALSA: snd-usb: add support for implicit feedback

[Daniel Mack]

---
 sound/usb/pcm.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index af21d38..e88abfa 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -312,9 +312,12 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 	if (((is_playback && attr == USB_ENDPOINT_SYNC_ASYNC) ||
 	     (! is_playback && attr == USB_ENDPOINT_SYNC_ADAPTIVE)) &&
 	    altsd->bNumEndpoints >= 2) {
+		int implicit_fb = (get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_USAGE_MASK)
+				== USB_ENDPOINT_USAGE_IMPLICIT_FB;
 		switch (subs->stream->chip->usb_id) {
 		case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */
 		case USB_ID(0x0763, 0x2081):
+			implicit_fb = 1;
 			ep = 0x81;
 			iface = usb_ifnum_to_if(dev, 2);
 			alts = &iface->altsetting[1];
@@ -327,7 +330,8 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 		   the audio fields in the endpoint descriptors */
 		if ((get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != 0x01 ||
 		    (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
-		     get_endpoint(alts, 1)->bSynchAddress != 0)) {
+		     get_endpoint(alts, 1)->bSynchAddress != 0 &&
+		     !implicit_fb)) {
 			snd_printk(KERN_ERR "%d:%d:%d : invalid synch pipe\n",
 				   dev->devnum, fmt->iface, fmt->altsetting);
 			return -EINVAL;
@@ -335,7 +339,8 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 		ep = get_endpoint(alts, 1)->bEndpointAddress;
 		if (get_endpoint(alts, 0)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
 		    (( is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress | USB_DIR_IN)) ||
-		     (!is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress & ~USB_DIR_IN)))) {
+		     (!is_playback && ep != (unsigned int)(get_endpoint(alts, 0)->bSynchAddress & ~USB_DIR_IN)) ||
+		     ( is_playback && !implicit_fb))) {
 			snd_printk(KERN_ERR "%d:%d:%d : invalid synch pipe\n",
 				   dev->devnum, fmt->iface, fmt->altsetting);
 			return -EINVAL;
@@ -343,8 +348,9 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 add_sync_ep:
 		subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip,
 							   alts, ep, !subs->direction,
-							   SND_USB_ENDPOINT_TYPE_SYNC);
-
+							   implicit_fb ?
+								SND_USB_ENDPOINT_TYPE_DATA :
+								SND_USB_ENDPOINT_TYPE_SYNC);
 		if (!subs->sync_endpoint)
 			return -EINVAL;
 
-- 
1.7.5.4

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack]

On 10/31/2011 01:10 PM, Daniel Mack wrote:
> I didn't sign-off the patches on purpose, as I would really like to
> get them reviewed before they go in. Can people have a look and state
> whether the whole idea is at all sane?

To briefly state the idea as such: the new implementation defines a 
model (snd_usb_endpoint) that handles everything that is related to an 
USB endpoint and its streaming. There are functions to activate and 
deactivate an endpoint (which call usb_set_interface()), and to start 
and stop its URBs. It also has function pointers to be called when data 
was received or is about to be sent, and pointer to a sync slave 
(another snd_usb_endpoint) that is informed when data has been received. 
A snd_usb_endpoint knows about its state and implements a refcounting, 
so only the first user will actually start the URBs and only the last 
one to stop it will tear them down again.

With this sort of abstraction, the actual streaming is decoupled from 
the pcm handling, which makes the "implicit feedback" mechanisms easy to 
implement. All the code that actually handles the payload of a stream's 
packets is now implemented in pcm.c, which is were it belongs to.

But I'm sure there are some unresolved corner cases which need attention.


Daniel

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Aurélien Leblond]

Hi Daniel,

Brilliant!

I have a question though, related to how you guys in Alsa work and
what processes you follow.

Are you looking for testers already?

Or more for some kind of approval first from some other Alsa gurus to
get the code accepted?

Aurélien

On Mon, Oct 31, 2011 at 12:38 PM, Daniel Mack <zonque@gmail.com> wrote:
> On 10/31/2011 01:10 PM, Daniel Mack wrote:
>>
>> I didn't sign-off the patches on purpose, as I would really like to
>> get them reviewed before they go in. Can people have a look and state
>> whether the whole idea is at all sane?
>
> To briefly state the idea as such: the new implementation defines a model
> (snd_usb_endpoint) that handles everything that is related to an USB
> endpoint and its streaming. There are functions to activate and deactivate
> an endpoint (which call usb_set_interface()), and to start and stop its
> URBs. It also has function pointers to be called when data was received or
> is about to be sent, and pointer to a sync slave (another snd_usb_endpoint)
> that is informed when data has been received. A snd_usb_endpoint knows about
> its state and implements a refcounting, so only the first user will actually
> start the URBs and only the last one to stop it will tear them down again.
>
> With this sort of abstraction, the actual streaming is decoupled from the
> pcm handling, which makes the "implicit feedback" mechanisms easy to
> implement. All the code that actually handles the payload of a stream's
> packets is now implemented in pcm.c, which is were it belongs to.
>
> But I'm sure there are some unresolved corner cases which need attention.
>
>
> Daniel
>

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack]

On 01.11.11 09:50, Aurélien Leblond wrote:
> Hi Daniel,
> 
> Brilliant!
> 
> I have a question though, related to how you guys in Alsa work and
> what processes you follow.
> 
> Are you looking for testers already?
> 
> Or more for some kind of approval first from some other Alsa gurus to
> get the code accepted?

No, go ahead and test. Be warned that your machine might still lock up,
but any kind of feedback is certainly valuable.

Thanks,
Daniel

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Felix Homann]

Hi Daniel,

that's great news indeed!

> No, go ahead and test. Be warned that your machine might still lock up,
> but any kind of feedback is certainly valuable.

Would you mind sending a single diff as an attachment for us simple testers?
(I'm reading this list in a web interface only and had several
problems with copy and pasting inlined patches in the past.)

Thanks,

Felix

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack]

On 11/01/2011 10:40 AM, Felix Homann wrote:
> Hi Daniel,
>
> that's great news indeed!
>
>> No, go ahead and test. Be warned that your machine might still lock up,
>> but any kind of feedback is certainly valuable.
>
> Would you mind sending a single diff as an attachment for us simple testers?
> (I'm reading this list in a web interface only and had several
> problems with copy and pasting inlined patches in the past.)

Here you go:

   https://gist.github.com/1168715


Daniel

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Felix Homann]

2011/11/1 Daniel Mack <zonque@gmail.com>:

> Here you go:
>
>  https://gist.github.com/1168715

Thank you!

(git.alsa-project.org seems to be down at the moment. I'll give it a
try when I can pull a current alsa tree.)

--Felix

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack]

On 11/01/2011 11:39 AM, Felix Homann wrote:
> 2011/11/1 Daniel Mack<zonque@gmail.com>:
>
>> Here you go:
>>
>>   https://gist.github.com/1168715
>
> Thank you!
>
> (git.alsa-project.org seems to be down at the moment. I'll give it a
> try when I can pull a current alsa tree.)

The ALSA master is currently located at GitHub:

   git://github.com/tiwai/sound.git


Daniel

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Grant Diffey]

On Tue, Nov 1, 2011 at 11:11 PM, Daniel Mack <zonque@gmail.com> wrote:

> On 11/01/2011 11:39 AM, Felix Homann wrote:
>
>> 2011/11/1 Daniel Mack<zonque@gmail.com>:
>>
>>  Here you go:
>>>
>>>  https://gist.github.com/**1168715 <https://gist.github.com/1168715>
>>>
>>
>> Thank you!
>>
>> (git.alsa-project.org seems to be down at the moment. I'll give it a
>> try when I can pull a current alsa tree.)
>>
>
> The ALSA master is currently located at GitHub:
>
>  git://github.com/tiwai/sound.**git <http://github.com/tiwai/sound.git>
>
>
> Daniel
>
>
ah so not the alsa-drivers tree at git.kernel.org?

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Takashi Iwai]

At Tue, 01 Nov 2011 13:11:32 +0100,
Daniel Mack wrote:
> 
> On 11/01/2011 11:39 AM, Felix Homann wrote:
> > 2011/11/1 Daniel Mack<zonque@gmail.com>:
> >
> >> Here you go:
> >>
> >>   https://gist.github.com/1168715
> >
> > Thank you!
> >
> > (git.alsa-project.org seems to be down at the moment. I'll give it a
> > try when I can pull a current alsa tree.)
> 
> The ALSA master is currently located at GitHub:
> 
>    git://github.com/tiwai/sound.git

It's now back to kernel.org.

  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git

github is still mirrored, but might disappear sometime in future.

(sorry I forgot to announce in alsa-devel ML.)


Takashi

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Takashi Iwai]

At Mon, 31 Oct 2011 13:38:20 +0100,
Daniel Mack wrote:
> 
> On 10/31/2011 01:10 PM, Daniel Mack wrote:
> > I didn't sign-off the patches on purpose, as I would really like to
> > get them reviewed before they go in. Can people have a look and state
> > whether the whole idea is at all sane?
> 
> To briefly state the idea as such: the new implementation defines a 
> model (snd_usb_endpoint) that handles everything that is related to an 
> USB endpoint and its streaming. There are functions to activate and 
> deactivate an endpoint (which call usb_set_interface()), and to start 
> and stop its URBs. It also has function pointers to be called when data 
> was received or is about to be sent, and pointer to a sync slave 
> (another snd_usb_endpoint) that is informed when data has been received. 
> A snd_usb_endpoint knows about its state and implements a refcounting, 
> so only the first user will actually start the URBs and only the last 
> one to stop it will tear them down again.
> 
> With this sort of abstraction, the actual streaming is decoupled from 
> the pcm handling, which makes the "implicit feedback" mechanisms easy to 
> implement. All the code that actually handles the payload of a stream's 
> packets is now implemented in pcm.c, which is were it belongs to.
> 
> But I'm sure there are some unresolved corner cases which need attention.

The overall design looks good to me.  Nice work!
A few nitpicking:

- No need for check of activated flags at starting streams?
- Better to clear subs->data_endpoint & co at closing.
- There might be unbalances when deactivate_endpoints() isn't called
  in prepare callback before activate_endpoints().  The apps may call
  like
	open -> hw_params -> prepare -> hw_params -> prepare -> trigger
  and
	trigger (stop) -> prepare -> trigger (start)
  So, be careful about refcounting and active flags.

And, yes, the protection for snd_usb_add_endpoint() would be really
needed for concurrent access.  The card-global mutex would be enough.


thanks,

Takashi

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Felix Homann]

On my machine building from alsa-driver/alsa-kernel fails.

I've got my alsa-kernel from here:

git clone git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
alsa-kernel

alsa-driver is this one:

git clone git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/alsa-driver-build.git
alsa-driver

After successfully patching alsa-kernel the build fails:

./gitcompile  --with-oss=no --with-pcm-oss-plugins=no
--with-debug=full --enable-dynamic-minors --with-moddir=updates

[.....]

copying file alsa-kernel/usb/endpoint.cpatching file endpoint.cHunk #2
FAILED at 72.Hunk #3 FAILED at 84.Hunk #4 succeeded at 194 with fuzz 2
(offset 29 lines).Hunk #5 FAILED at 192.Hunk #6 FAILED at 341.4 out of
6 hunks FAILED -- saving rejects to file endpoint.c.rej

Here's endpoint.c.rej:

--- ../alsa-kernel/usb/endpoint.c       2011-09-14 13:57:01.622042886 +0200
+++ endpoint.c  2011-09-14 13:57:20.178761685 +0200
@@ -72,9 +74,12 @@
                if (test_bit(i, &subs->active_mask)) {
                        if (!test_and_set_bit(i, &subs->unlink_mask)) {
                                struct urb *u = subs->dataurb[i].urb;
-                               if (async)
+                               if (async) {
+#ifdef URB_ASYNC_UNLINK
+                                       u->transfer_flags |= URB_ASYNC_UNLINK;
+#endif
                                        usb_unlink_urb(u);
-                               else
+                               } else
                                        usb_kill_urb(u);
                        }
                }
@@ -84,9 +89,12 @@
                        if (test_bit(i+16, &subs->active_mask)) {
                                if (!test_and_set_bit(i+16,
&subs->unlink_mask)) {
                                        struct urb *u = subs->syncurb[i].urb;
-                                       if (async)
+                                       if (async) {
+#ifdef URB_ASYNC_UNLINK
+                                               u->transfer_flags |=
URB_ASYNC_UNLINK;
+#endif
                                                usb_unlink_urb(u);
-                                       else
+                                       } else
                                                usb_kill_urb(u);
                                }
                        }
@@ -192,7 +204,11 @@
 /*
  * complete callback from sync urb
  */
+#if !defined(OLD_USB) && !defined(CONFIG_SND_NEW_IRQ_HANDLER)
+static void snd_complete_sync_urb(struct urb *urb, struct pt_regs *regs)
+#else
 static void snd_complete_sync_urb(struct urb *urb)
+#endif
 {
        struct snd_urb_ctx *ctx = urb->context;
        struct snd_usb_substream *subs = ctx->subs;
@@ -341,7 +357,9 @@
                        if (!u->urb)
                                goto out_of_memory;
                        u->urb->transfer_buffer = subs->syncbuf + i * 4;
+#ifdef HAVE_USB_BUFFERS
                        u->urb->transfer_dma = subs->sync_dma + i * 4;
+#endif
                        u->urb->transfer_buffer_length = 4;
                        u->urb->pipe = subs->syncpipe;
                        u->urb->transfer_flags = URB_ISO_ASAP |

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Takashi Iwai]

At Tue, 1 Nov 2011 20:03:02 +0100,
Felix Homann wrote:
> 
> On my machine building from alsa-driver/alsa-kernel fails.
> 
> I've got my alsa-kernel from here:
> 
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> alsa-kernel
> 
> alsa-driver is this one:
> 
> git clone git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/alsa-driver-build.git
> alsa-driver
> 
> After successfully patching alsa-kernel the build fails:
> 
> ./gitcompile  --with-oss=no --with-pcm-oss-plugins=no
> --with-debug=full --enable-dynamic-minors --with-moddir=updates
> 
> [.....]
> 
> copying file alsa-kernel/usb/endpoint.cpatching file endpoint.cHunk #2
> FAILED at 72.Hunk #3 FAILED at 84.Hunk #4 succeeded at 194 with fuzz 2
> (offset 29 lines).Hunk #5 FAILED at 192.Hunk #6 FAILED at 341.4 out of
> 6 hunks FAILED -- saving rejects to file endpoint.c.rej

Yeah, the patching in alsa-driver tree would fail with this update.
It's better to use 3.1 kernel and pull sound tree in it.


Takashi


> Here's endpoint.c.rej:
> 
> --- ../alsa-kernel/usb/endpoint.c       2011-09-14 13:57:01.622042886 +0200
> +++ endpoint.c  2011-09-14 13:57:20.178761685 +0200
> @@ -72,9 +74,12 @@
>                 if (test_bit(i, &subs->active_mask)) {
>                         if (!test_and_set_bit(i, &subs->unlink_mask)) {
>                                 struct urb *u = subs->dataurb[i].urb;
> -                               if (async)
> +                               if (async) {
> +#ifdef URB_ASYNC_UNLINK
> +                                       u->transfer_flags |= URB_ASYNC_UNLINK;
> +#endif
>                                         usb_unlink_urb(u);
> -                               else
> +                               } else
>                                         usb_kill_urb(u);
>                         }
>                 }
> @@ -84,9 +89,12 @@
>                         if (test_bit(i+16, &subs->active_mask)) {
>                                 if (!test_and_set_bit(i+16,
> &subs->unlink_mask)) {
>                                         struct urb *u = subs->syncurb[i].urb;
> -                                       if (async)
> +                                       if (async) {
> +#ifdef URB_ASYNC_UNLINK
> +                                               u->transfer_flags |=
> URB_ASYNC_UNLINK;
> +#endif
>                                                 usb_unlink_urb(u);
> -                                       else
> +                                       } else
>                                                 usb_kill_urb(u);
>                                 }
>                         }
> @@ -192,7 +204,11 @@
>  /*
>   * complete callback from sync urb
>   */
> +#if !defined(OLD_USB) && !defined(CONFIG_SND_NEW_IRQ_HANDLER)
> +static void snd_complete_sync_urb(struct urb *urb, struct pt_regs *regs)
> +#else
>  static void snd_complete_sync_urb(struct urb *urb)
> +#endif
>  {
>         struct snd_urb_ctx *ctx = urb->context;
>         struct snd_usb_substream *subs = ctx->subs;
> @@ -341,7 +357,9 @@
>                         if (!u->urb)
>                                 goto out_of_memory;
>                         u->urb->transfer_buffer = subs->syncbuf + i * 4;
> +#ifdef HAVE_USB_BUFFERS
>                         u->urb->transfer_dma = subs->sync_dma + i * 4;
> +#endif
>                         u->urb->transfer_buffer_length = 4;
>                         u->urb->pipe = subs->syncpipe;
>                         u->urb->transfer_flags = URB_ISO_ASAP |
> 

Re: [PATCH 0/5] RFC for snd-usb: rework usb endpoint logic

[Daniel Mack]

Hi Takashi,

On 11/01/2011 05:19 PM, Takashi Iwai wrote:
 > The overall design looks good to me.  Nice work!
 > A few nitpicking:

Thanks for your review!

 > - No need for check of activated flags at starting streams?

Such a condition would be a bug in the driver. But I'll add a check.

 > - Better to clear subs->data_endpoint&  co at closing.

Jup.

 > - There might be unbalances when deactivate_endpoints() isn't called
 >    in prepare callback before activate_endpoints().  The apps may call
 >    like
 > 	open ->  hw_params ->  prepare ->  hw_params ->  prepare ->  trigger
 >    and
 > 	trigger (stop) ->  prepare ->  trigger (start)
 >    So, be careful about refcounting and active flags.

Hmm, I don't think I follow here. snd_usb_endpoint_{de,}activate() don't 
actually touch the refcounts but act upon the "activated" flag, and 
snd_usb_endpoint_{start,stop}() will touch the refcounts only. Hence, an 
unbalanced call to activate() would basically be a noop. Do I miss your 
point?

 > And, yes, the protection for snd_usb_add_endpoint() would be really
 > needed for concurrent access.  The card-global mutex would be enough.

Ok. However, there is actually no card-global mutex for that purpose, 
right? I'll add one :)


Thanks,
Daniel