ALSA: usb-audio: Refactor endpoint management

This is an intensive surgery for the endpoint and stream management
for achieving more robust and clean code.

The goals of this patch are:
- More clear endpoint resource changes
- The interface altsetting control in a single place
Below are brief description of the whole changes.

First off, most of the endpoint operations are moved into endpoint.c,
so that the snd_usb_endpoint object is only referred in other places.
The endpoint object is acquired and released via the new functions
snd_usb_endpoint_open() and snd_usb_endpoint_close() that are called
at PCM hw_params and hw_free callbacks, respectively.  Those are
ref-counted and EPs can manage the multiple opens.

The open callback receives the audioformat and hw_params arguments,
and those are used for initializing the EP parameters; especially the
endpoint, interface and altset numbers are read from there, as well as
the PCM parameters like the format, rate and channels.  Those are
stored in snd_usb_endpoint object.  If it's the secondary open, the
function checks whether the given parameters are compatible with the
already opened EP setup, too.

The coupling with a sync EP (including an implicit feedback sync) is
done by the sole snd_usb_endpoint_set_sync() call.

The configuration of each endpoint is done in a single shot via
snd_usb_endpoint_configure() call.  This is the place where most of
PCM configurations are done.  A few flags and special handling in the
snd_usb_substream are dropped along with this change.

A significant difference wrt the configuration from the previous code
is the order of USB host interface setups.  Now the interface is
always disabled at beginning and (re-)enabled at the last step of
snd_usb_endpoint_configure(), in order to be compliant with the
standard UAC2/3.  For UAC1, the interface is set before the parameter
setups since there seem devices that require it (e.g. Yamaha THR10),
just like how it was done in the previous driver code.

The start/stop are almost same as before, also single-shots.  The URB
callbacks need to be set via snd_usb_endpoint_set_callback() like the
previous code at the trigger phase, too.

Finally, the flag for the re-setup is set at the device suspend
through the full EP list, instead of PCM trigger.  This catches the
overlooked cases where the PCM hasn't been running yet but the device
needs the full setup after resume.

Tested-by: Keith Milner <kamilner@superlative.org>
Tested-by: Dylan Robinson <dylan_robinson@motu.com>
Link: https://lore.kernel.org/r/20201123085347.19667-26-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>

1 files changed, 336 insertions, 326 deletions

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c
index 7012fdafc3d8..eee74313603e 100644
--- a/sound/usb/endpoint.c
+++ b/sound/usb/endpoint.c
@@ -18,6 +18,7 @@
 #include "card.h"
 #include "endpoint.h"
 #include "pcm.h"
+#include "clock.h"
 #include "quirks.h"
 
 #define EP_FLAG_RUNNING		1
@@ -116,10 +117,7 @@ static const char *usb_error_string(int err)
  */
 int snd_usb_endpoint_implicit_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);
+	return  ep->implicit_fb_sync && usb_pipeout(ep->pipe);
 }
 
 /*
@@ -185,18 +183,24 @@ static void retire_outbound_urb(struct snd_usb_endpoint *ep,
 	call_retire_callback(ep, urb_ctx->urb);
 }
 
+static void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
+				    struct snd_usb_endpoint *sender,
+				    const struct urb *urb);
+
 static void retire_inbound_urb(struct snd_usb_endpoint *ep,
 			       struct snd_urb_ctx *urb_ctx)
 {
 	struct urb *urb = urb_ctx->urb;
+	struct snd_usb_endpoint *sync_slave;
 
 	if (unlikely(ep->skip_packets > 0)) {
 		ep->skip_packets--;
 		return;
 	}
 
-	if (ep->sync_slave)
-		snd_usb_handle_sync_urb(ep->sync_slave, ep, urb);
+	sync_slave = READ_ONCE(ep->sync_slave);
+	if (sync_slave)
+		snd_usb_handle_sync_urb(sync_slave, ep, urb);
 
 	call_retire_callback(ep, urb);
 }
@@ -518,25 +522,155 @@ int snd_usb_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type)
 }
 
 /* Set up syncinterval and maxsyncsize for a sync EP */
-void snd_usb_endpoint_set_syncinterval(struct snd_usb_audio *chip,
-				       struct snd_usb_endpoint *ep,
-				       struct usb_host_interface *alts)
+static void endpoint_set_syncinterval(struct snd_usb_audio *chip,
+				      struct snd_usb_endpoint *ep)
 {
-	struct usb_endpoint_descriptor *desc = get_endpoint(alts, 1);
-
-	if (ep->type == SND_USB_ENDPOINT_TYPE_SYNC) {
-		if (desc->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
-		    desc->bRefresh >= 1 && desc->bRefresh <= 9)
-			ep->syncinterval = desc->bRefresh;
-		else if (snd_usb_get_speed(chip->dev) == USB_SPEED_FULL)
-			ep->syncinterval = 1;
-		else if (desc->bInterval >= 1 && desc->bInterval <= 16)
-			ep->syncinterval = desc->bInterval - 1;
-		else
-			ep->syncinterval = 3;
+	struct usb_host_interface *alts;
+	struct usb_endpoint_descriptor *desc;
+
+	alts = snd_usb_get_host_interface(chip, ep->iface, ep->altsetting);
+	if (!alts)
+		return;
+
+	desc = get_endpoint(alts, ep->ep_idx);
+	if (desc->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE &&
+	    desc->bRefresh >= 1 && desc->bRefresh <= 9)
+		ep->syncinterval = desc->bRefresh;
+	else if (snd_usb_get_speed(chip->dev) == USB_SPEED_FULL)
+		ep->syncinterval = 1;
+	else if (desc->bInterval >= 1 && desc->bInterval <= 16)
+		ep->syncinterval = desc->bInterval - 1;
+	else
+		ep->syncinterval = 3;
+
+	ep->syncmaxsize = le16_to_cpu(desc->wMaxPacketSize);
+}
+
+static bool endpoint_compatible(struct snd_usb_endpoint *ep,
+				const struct audioformat *fp,
+				const struct snd_pcm_hw_params *params)
+{
+	if (!ep->opened)
+		return false;
+	if (ep->cur_audiofmt != fp)
+		return false;
+	if (ep->cur_rate != params_rate(params) ||
+	    ep->cur_format != params_format(params) ||
+	    ep->cur_period_frames != params_period_size(params) ||
+	    ep->cur_buffer_periods != params_periods(params))
+		return false;
+	return true;
+}
+
+/*
+ * Check whether the given fp and hw params are compatbile with the current
+ * setup of the target EP for implicit feedback sync
+ */
+bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip,
+				 struct snd_usb_endpoint *ep,
+				 const struct audioformat *fp,
+				 const struct snd_pcm_hw_params *params)
+{
+	bool ret;
+
+	mutex_lock(&chip->mutex);
+	ret = endpoint_compatible(ep, fp, params);
+	mutex_unlock(&chip->mutex);
+	return ret;
+}
+
+/*
+ * snd_usb_endpoint_open: Open the endpoint
+ *
+ * Called from hw_params to assign the endpoint to the substream.
+ * It's reference-counted, and only the first opener is allowed to set up
+ * arbitrary parameters.  The later opener must be compatible with the
+ * former opened parameters.
+ * The endpoint needs to be closed via snd_usb_endpoint_close() later.
+ *
+ * Note that this function doesn't configure the endpoint.  The substream
+ * needs to set it up later via snd_usb_endpoint_configure().
+ */
+struct snd_usb_endpoint *
+snd_usb_endpoint_open(struct snd_usb_audio *chip,
+		      struct audioformat *fp,
+		      const struct snd_pcm_hw_params *params,
+		      bool is_sync_ep)
+{
+	struct snd_usb_endpoint *ep;
+	int ep_num = is_sync_ep ? fp->sync_ep : fp->endpoint;
 
-		ep->syncmaxsize = le16_to_cpu(desc->wMaxPacketSize);
+	mutex_lock(&chip->mutex);
+	ep = snd_usb_get_endpoint(chip, ep_num);
+	if (!ep) {
+		usb_audio_err(chip, "Cannot find EP 0x%x to open\n", ep_num);
+		goto unlock;
 	}
+
+	if (!ep->opened) {
+		if (is_sync_ep) {
+			ep->iface = fp->sync_iface;
+			ep->altsetting = fp->sync_altsetting;
+			ep->ep_idx = fp->sync_ep_idx;
+		} else {
+			ep->iface = fp->iface;
+			ep->altsetting = fp->altsetting;
+			ep->ep_idx = 0;
+		}
+		usb_audio_dbg(chip, "Open EP 0x%x, iface=%d:%d, idx=%d\n",
+			      ep_num, ep->iface, ep->altsetting, ep->ep_idx);
+
+		ep->cur_audiofmt = fp;
+		ep->cur_channels = fp->channels;
+		ep->cur_rate = params_rate(params);
+		ep->cur_format = params_format(params);
+		ep->cur_frame_bytes = snd_pcm_format_physical_width(ep->cur_format) *
+			ep->cur_channels / 8;
+		ep->cur_period_frames = params_period_size(params);
+		ep->cur_period_bytes = ep->cur_period_frames * ep->cur_frame_bytes;
+		ep->cur_buffer_periods = params_periods(params);
+
+		if (ep->type == SND_USB_ENDPOINT_TYPE_SYNC)
+			endpoint_set_syncinterval(chip, ep);
+
+		ep->implicit_fb_sync = fp->implicit_fb;
+		ep->need_setup = true;
+
+		usb_audio_dbg(chip, "  channels=%d, rate=%d, format=%s, period_bytes=%d, periods=%d, implicit_fb=%d\n",
+			      ep->cur_channels, ep->cur_rate,
+			      snd_pcm_format_name(ep->cur_format),
+			      ep->cur_period_bytes, ep->cur_buffer_periods,
+			      ep->implicit_fb_sync);
+
+	} else {
+		if (!endpoint_compatible(ep, fp, params)) {
+			usb_audio_err(chip, "Incompatible EP setup for 0x%x\n",
+				      ep_num);
+			ep = NULL;
+			goto unlock;
+		}
+
+		usb_audio_dbg(chip, "Reopened EP 0x%x (count %d)\n",
+			      ep_num, ep->opened);
+	}
+
+	ep->opened++;
+
+ unlock:
+	mutex_unlock(&chip->mutex);
+	return ep;
+}
+
+/*
+ * snd_usb_endpoint_set_sync: Link data and sync endpoints
+ *
+ * Pass NULL to sync_ep to unlink again
+ */
+void snd_usb_endpoint_set_sync(struct snd_usb_audio *chip,
+			       struct snd_usb_endpoint *data_ep,
+			       struct snd_usb_endpoint *sync_ep)
+{
+	data_ep->sync_master = sync_ep;
 }
 
 /*
@@ -557,6 +691,54 @@ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep,
 	WRITE_ONCE(ep->data_subs, data_subs);
 }
 
+static int endpoint_set_interface(struct snd_usb_audio *chip,
+				  struct snd_usb_endpoint *ep,
+				  bool set)
+{
+	int altset = set ? ep->altsetting : 0;
+	int err;
+
+	usb_audio_dbg(chip, "Setting usb interface %d:%d for EP 0x%x\n",
+		      ep->iface, altset, ep->ep_num);
+	err = usb_set_interface(chip->dev, ep->iface, altset);
+	if (err < 0) {
+		usb_audio_err(chip, "%d:%d: usb_set_interface failed (%d)\n",
+			      ep->iface, altset, err);
+		return err;
+	}
+
+	snd_usb_set_interface_quirk(chip);
+	return 0;
+}
+
+/*
+ * snd_usb_endpoint_close: Close the endpoint
+ *
+ * Unreference the already opened endpoint via snd_usb_endpoint_open().
+ */
+void snd_usb_endpoint_close(struct snd_usb_audio *chip,
+			    struct snd_usb_endpoint *ep)
+{
+	mutex_lock(&chip->mutex);
+	usb_audio_dbg(chip, "Closing EP 0x%x (count %d)\n",
+		      ep->ep_num, ep->opened);
+	if (!--ep->opened) {
+		endpoint_set_interface(chip, ep, false);
+		ep->iface = -1;
+		ep->altsetting = 0;
+		ep->cur_audiofmt = NULL;
+		ep->cur_rate = 0;
+		usb_audio_dbg(chip, "EP 0x%x closed\n", ep->ep_num);
+	}
+	mutex_unlock(&chip->mutex);
+}
+
+/* Prepare for suspening EP, called from the main suspend handler */
+void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep)
+{
+	ep->need_setup = true;
+}
+
 /*
  *  wait until all urbs are processed.
  */
@@ -647,218 +829,35 @@ static void release_urbs(struct snd_usb_endpoint *ep, int force)
 }
 
 /*
- * Check data endpoint for format differences
- */
-static bool check_ep_params(struct snd_usb_endpoint *ep,
-			    snd_pcm_format_t pcm_format,
-			    unsigned int channels,
-			    unsigned int period_bytes,
-			    unsigned int frames_per_period,
-			    unsigned int periods_per_buffer,
-			    unsigned int rate,
-			    struct audioformat *fmt,
-			    struct snd_usb_endpoint *sync_ep)
-{
-	unsigned int maxsize, minsize, packs_per_ms, max_packs_per_urb;
-	unsigned int max_packs_per_period, urbs_per_period, urb_packs;
-	unsigned int max_urbs;
-	int frame_bits = snd_pcm_format_physical_width(pcm_format) * channels;
-	int tx_length_quirk = (ep->chip->tx_length_quirk &&
-			       usb_pipeout(ep->pipe));
-	bool ret = 1;
-
-	/* matching with the saved parameters? */
-	if (ep->cur_rate == rate &&
-	    ep->cur_format == pcm_format &&
-	    ep->cur_channels == channels &&
-	    ep->cur_period_frames == frames_per_period &&
-	    ep->cur_buffer_periods == periods_per_buffer)
-		return true;
-
-	if (pcm_format == SNDRV_PCM_FORMAT_DSD_U16_LE && fmt->dsd_dop) {
-		/*
-		 * When operating in DSD DOP mode, the size of a sample frame
-		 * in hardware differs from the actual physical format width
-		 * because we need to make room for the DOP markers.
-		 */
-		frame_bits += channels << 3;
-	}
-
-	ret = ret && (ep->datainterval == fmt->datainterval);
-	ret = ret && (ep->stride == frame_bits >> 3);
-
-	switch (pcm_format) {
-	case SNDRV_PCM_FORMAT_U8:
-		ret = ret && (ep->silence_value == 0x80);
-		break;
-	case SNDRV_PCM_FORMAT_DSD_U8:
-	case SNDRV_PCM_FORMAT_DSD_U16_LE:
-	case SNDRV_PCM_FORMAT_DSD_U32_LE:
-	case SNDRV_PCM_FORMAT_DSD_U16_BE:
-	case SNDRV_PCM_FORMAT_DSD_U32_BE:
-		ret = ret && (ep->silence_value == 0x69);
-		break;
-	default:
-		ret = ret && (ep->silence_value == 0);
-	}
-
-	/* assume max. frequency is 50% higher than nominal */
-	ret = ret && (ep->freqmax == ep->freqn + (ep->freqn >> 1));
-	/* Round up freqmax to nearest integer in order to calculate maximum
-	 * packet size, which must represent a whole number of frames.
-	 * This is accomplished by adding 0x0.ffff before converting the
-	 * Q16.16 format into integer.
-	 * In order to accurately calculate the maximum packet size when
-	 * the data interval is more than 1 (i.e. ep->datainterval > 0),
-	 * multiply by the data interval prior to rounding. For instance,
-	 * a freqmax of 41 kHz will result in a max packet size of 6 (5.125)
-	 * frames with a data interval of 1, but 11 (10.25) frames with a
-	 * data interval of 2.
-	 * (ep->freqmax << ep->datainterval overflows at 8.192 MHz for the
-	 * maximum datainterval value of 3, at USB full speed, higher for
-	 * USB high speed, noting that ep->freqmax is in units of
-	 * frames per packet in Q16.16 format.)
-	 */
-	maxsize = (((ep->freqmax << ep->datainterval) + 0xffff) >> 16) *
-			 (frame_bits >> 3);
-	if (tx_length_quirk)
-		maxsize += sizeof(__le32); /* Space for length descriptor */
-	/* but wMaxPacketSize might reduce this */
-	if (ep->maxpacksize && ep->maxpacksize < maxsize) {
-		/* whatever fits into a max. size packet */
-		unsigned int data_maxsize = maxsize = ep->maxpacksize;
-
-		if (tx_length_quirk)
-			/* Need to remove the length descriptor to calc freq */
-			data_maxsize -= sizeof(__le32);
-		ret = ret && (ep->freqmax == (data_maxsize / (frame_bits >> 3))
-				<< (16 - ep->datainterval));
-	}
-
-	if (ep->fill_max)
-		ret = ret && (ep->curpacksize == ep->maxpacksize);
-	else
-		ret = ret && (ep->curpacksize == maxsize);
-
-	if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) {
-		packs_per_ms = 8 >> ep->datainterval;
-		max_packs_per_urb = MAX_PACKS_HS;
-	} else {
-		packs_per_ms = 1;
-		max_packs_per_urb = MAX_PACKS;
-	}
-	if (sync_ep && !snd_usb_endpoint_implicit_feedback_sink(ep))
-		max_packs_per_urb = min(max_packs_per_urb,
-					1U << sync_ep->syncinterval);
-	max_packs_per_urb = max(1u, max_packs_per_urb >> ep->datainterval);
-
-	/*
-	 * Capture endpoints need to use small URBs because there's no way
-	 * to tell in advance where the next period will end, and we don't
-	 * want the next URB to complete much after the period ends.
-	 *
-	 * Playback endpoints with implicit sync much use the same parameters
-	 * as their corresponding capture endpoint.
-	 */
-	if (usb_pipein(ep->pipe) ||
-			snd_usb_endpoint_implicit_feedback_sink(ep)) {
-
-		urb_packs = packs_per_ms;
-		/*
-		 * Wireless devices can poll at a max rate of once per 4ms.
-		 * For dataintervals less than 5, increase the packet count to
-		 * allow the host controller to use bursting to fill in the
-		 * gaps.
-		 */
-		if (snd_usb_get_speed(ep->chip->dev) == USB_SPEED_WIRELESS) {
-			int interval = ep->datainterval;
-
-			while (interval < 5) {
-				urb_packs <<= 1;
-				++interval;
-			}
-		}
-		/* make capture URBs <= 1 ms and smaller than a period */
-		urb_packs = min(max_packs_per_urb, urb_packs);
-		while (urb_packs > 1 && urb_packs * maxsize >= period_bytes)
-			urb_packs >>= 1;
-		ret = ret && (ep->nurbs == MAX_URBS);
-
-	/*
-	 * Playback endpoints without implicit sync are adjusted so that
-	 * a period fits as evenly as possible in the smallest number of
-	 * URBs.  The total number of URBs is adjusted to the size of the
-	 * ALSA buffer, subject to the MAX_URBS and MAX_QUEUE limits.
-	 */
-	} else {
-		/* 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);
-
-		/* how many packets will contain an entire ALSA period? */
-		max_packs_per_period = DIV_ROUND_UP(period_bytes, minsize);
-
-		/* how many URBs will contain a period? */
-		urbs_per_period = DIV_ROUND_UP(max_packs_per_period,
-				max_packs_per_urb);
-		/* how many packets are needed in each URB? */
-		urb_packs = DIV_ROUND_UP(max_packs_per_period, urbs_per_period);
-
-		/* limit the number of frames in a single URB */
-		ret = ret && (ep->max_urb_frames ==
-			DIV_ROUND_UP(frames_per_period, urbs_per_period));
-
-		/* try to use enough URBs to contain an entire ALSA buffer */
-		max_urbs = min((unsigned) MAX_URBS,
-				MAX_QUEUE * packs_per_ms / urb_packs);
-		ret = ret && (ep->nurbs == min(max_urbs,
-				urbs_per_period * periods_per_buffer));
-	}
-
-	ret = ret && (ep->datainterval == fmt->datainterval);
-	ret = ret && (ep->maxpacksize == fmt->maxpacksize);
-	ret = ret &&
-		(ep->fill_max == !!(fmt->attributes & UAC_EP_CS_ATTR_FILL_MAX));
-
-	return ret;
-}
-
-/*
  * configure a data endpoint
  */
-static int data_ep_set_params(struct snd_usb_endpoint *ep,
-			      snd_pcm_format_t pcm_format,
-			      unsigned int channels,
-			      unsigned int period_bytes,
-			      unsigned int frames_per_period,
-			      unsigned int periods_per_buffer,
-			      struct audioformat *fmt,
-			      struct snd_usb_endpoint *sync_ep)
+static int data_ep_set_params(struct snd_usb_endpoint *ep)
 {
+	struct snd_usb_audio *chip = ep->chip;
 	unsigned int maxsize, minsize, packs_per_ms, max_packs_per_urb;
 	unsigned int max_packs_per_period, urbs_per_period, urb_packs;
 	unsigned int max_urbs, i;
-	int frame_bits = snd_pcm_format_physical_width(pcm_format) * channels;
-	int tx_length_quirk = (ep->chip->tx_length_quirk &&
+	const struct audioformat *fmt = ep->cur_audiofmt;
+	int frame_bits = ep->cur_frame_bytes * 8;
+	int tx_length_quirk = (chip->tx_length_quirk &&
 			       usb_pipeout(ep->pipe));
 
-	if (pcm_format == SNDRV_PCM_FORMAT_DSD_U16_LE && fmt->dsd_dop) {
+	usb_audio_dbg(chip, "Setting params for data EP 0x%x, pipe 0x%x\n",
+		      ep->ep_num, ep->pipe);
+
+	if (ep->cur_format == SNDRV_PCM_FORMAT_DSD_U16_LE && fmt->dsd_dop) {
 		/*
 		 * When operating in DSD DOP mode, the size of a sample frame
 		 * in hardware differs from the actual physical format width
 		 * because we need to make room for the DOP markers.
 		 */
-		frame_bits += channels << 3;
+		frame_bits += ep->cur_channels << 3;
 	}
 
 	ep->datainterval = fmt->datainterval;
 	ep->stride = frame_bits >> 3;
 
-	switch (pcm_format) {
+	switch (ep->cur_format) {
 	case SNDRV_PCM_FORMAT_U8:
 		ep->silence_value = 0x80;
 		break;
@@ -911,16 +910,16 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 	else
 		ep->curpacksize = maxsize;
 
-	if (snd_usb_get_speed(ep->chip->dev) != USB_SPEED_FULL) {
+	if (snd_usb_get_speed(chip->dev) != USB_SPEED_FULL) {
 		packs_per_ms = 8 >> ep->datainterval;
 		max_packs_per_urb = MAX_PACKS_HS;
 	} else {
 		packs_per_ms = 1;
 		max_packs_per_urb = MAX_PACKS;
 	}
-	if (sync_ep && !snd_usb_endpoint_implicit_feedback_sink(ep))
+	if (ep->sync_master && !ep->implicit_fb_sync)
 		max_packs_per_urb = min(max_packs_per_urb,
-					1U << sync_ep->syncinterval);
+					1U << ep->sync_master->syncinterval);
 	max_packs_per_urb = max(1u, max_packs_per_urb >> ep->datainterval);
 
 	/*
@@ -931,9 +930,7 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 	 * Playback endpoints with implicit sync much use the same parameters
 	 * as their corresponding capture endpoint.
 	 */
-	if (usb_pipein(ep->pipe) ||
-	    ep->is_implicit_feedback ||
-	    snd_usb_endpoint_implicit_feedback_sink(ep)) {
+	if (usb_pipein(ep->pipe) || ep->implicit_fb_sync) {
 
 		urb_packs = packs_per_ms;
 		/*
@@ -942,7 +939,7 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 		 * allow the host controller to use bursting to fill in the
 		 * gaps.
 		 */
-		if (snd_usb_get_speed(ep->chip->dev) == USB_SPEED_WIRELESS) {
+		if (snd_usb_get_speed(chip->dev) == USB_SPEED_WIRELESS) {
 			int interval = ep->datainterval;
 			while (interval < 5) {
 				urb_packs <<= 1;
@@ -951,7 +948,7 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 		}
 		/* make capture URBs <= 1 ms and smaller than a period */
 		urb_packs = min(max_packs_per_urb, urb_packs);
-		while (urb_packs > 1 && urb_packs * maxsize >= period_bytes)
+		while (urb_packs > 1 && urb_packs * maxsize >= ep->cur_period_bytes)
 			urb_packs >>= 1;
 		ep->nurbs = MAX_URBS;
 
@@ -966,12 +963,12 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 		minsize = (ep->freqn >> (16 - ep->datainterval)) *
 				(frame_bits >> 3);
 		/* with sync from device, assume it can be 12% lower */
-		if (sync_ep)
+		if (ep->sync_master)
 			minsize -= minsize >> 3;
 		minsize = max(minsize, 1u);
 
 		/* how many packets will contain an entire ALSA period? */
-		max_packs_per_period = DIV_ROUND_UP(period_bytes, minsize);
+		max_packs_per_period = DIV_ROUND_UP(ep->cur_period_bytes, minsize);
 
 		/* how many URBs will contain a period? */
 		urbs_per_period = DIV_ROUND_UP(max_packs_per_period,
@@ -980,13 +977,13 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 		urb_packs = DIV_ROUND_UP(max_packs_per_period, urbs_per_period);
 
 		/* limit the number of frames in a single URB */
-		ep->max_urb_frames = DIV_ROUND_UP(frames_per_period,
-					urbs_per_period);
+		ep->max_urb_frames = DIV_ROUND_UP(ep->cur_period_frames,
+						  urbs_per_period);
 
 		/* try to use enough URBs to contain an entire ALSA buffer */
 		max_urbs = min((unsigned) MAX_URBS,
 				MAX_QUEUE * packs_per_ms / urb_packs);
-		ep->nurbs = min(max_urbs, urbs_per_period * periods_per_buffer);
+		ep->nurbs = min(max_urbs, urbs_per_period * ep->cur_buffer_periods);
 	}
 
 	/* allocate and initialize data urbs */
@@ -1004,7 +1001,7 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep,
 			goto out_of_memory;
 
 		u->urb->transfer_buffer =
-			usb_alloc_coherent(ep->chip->dev, u->buffer_size,
+			usb_alloc_coherent(chip->dev, u->buffer_size,
 					   GFP_KERNEL, &u->urb->transfer_dma);
 		if (!u->urb->transfer_buffer)
 			goto out_of_memory;
@@ -1028,9 +1025,13 @@ out_of_memory:
  */
 static int sync_ep_set_params(struct snd_usb_endpoint *ep)
 {
+	struct snd_usb_audio *chip = ep->chip;
 	int i;
 
-	ep->syncbuf = usb_alloc_coherent(ep->chip->dev, SYNC_URBS * 4,
+	usb_audio_dbg(chip, "Setting params for sync EP 0x%x, pipe 0x%x\n",
+		      ep->ep_num, ep->pipe);
+
+	ep->syncbuf = usb_alloc_coherent(chip->dev, SYNC_URBS * 4,
 					 GFP_KERNEL, &ep->sync_dma);
 	if (!ep->syncbuf)
 		return -ENOMEM;
@@ -1063,60 +1064,19 @@ out_of_memory:
 	return -ENOMEM;
 }
 
-/**
+/*
  * snd_usb_endpoint_set_params: configure an snd_usb_endpoint
  *
- * @ep: the snd_usb_endpoint to configure
- * @pcm_format: the audio fomat.
- * @channels: the number of audio channels.
- * @period_bytes: the number of bytes in one alsa period.
- * @period_frames: the number of frames in one alsa period.
- * @buffer_periods: the number of periods in one alsa buffer.
- * @rate: the frame rate.
- * @fmt: the USB audio format information
- * @sync_ep: the sync endpoint to use, if any
- *
  * Determine the number of URBs to be used on this endpoint.
  * An endpoint must be configured before it can be started.
  * An endpoint that is already running can not be reconfigured.
  */
-int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
-				snd_pcm_format_t pcm_format,
-				unsigned int channels,
-				unsigned int period_bytes,
-				unsigned int period_frames,
-				unsigned int buffer_periods,
-				unsigned int rate,
-				struct audioformat *fmt,
-				struct snd_usb_endpoint *sync_ep)
+static int snd_usb_endpoint_set_params(struct snd_usb_audio *chip,
+				       struct snd_usb_endpoint *ep)
 {
+	const struct audioformat *fmt = ep->cur_audiofmt;
 	int err;
 
-	usb_audio_dbg(ep->chip,
-		      "Setting params for ep %x (type %s, count %d), rate=%d, format=%s, channels=%d, period_bytes=%d, periods=%d\n",
-		      ep->ep_num, ep_type_name(ep->type), ep->use_count,
-		      rate, snd_pcm_format_name(pcm_format), channels,
-		      period_bytes, buffer_periods);
-
-	if (ep->use_count != 0) {
-		bool check = ep->is_implicit_feedback &&
-			check_ep_params(ep, pcm_format, channels, period_bytes,
-					period_frames, buffer_periods, rate,
-					fmt, sync_ep);
-
-		if (!check) {
-			usb_audio_warn(ep->chip,
-				"Unable to change format on ep #%x: already in use\n",
-				ep->ep_num);
-			return -EBUSY;
-		}
-
-		usb_audio_dbg(ep->chip,
-			      "Ep #%x already in use as implicit feedback but format not changed\n",
-			      ep->ep_num);
-		return 0;
-	}
-
 	/* release old buffers, if any */
 	release_urbs(ep, 0);
 
@@ -1124,17 +1084,17 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 	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(rate);
+	if (snd_usb_get_speed(chip->dev) == USB_SPEED_FULL) {
+		ep->freqn = get_usb_full_speed_rate(ep->cur_rate);
 		ep->pps = 1000 >> ep->datainterval;
 	} else {
-		ep->freqn = get_usb_high_speed_rate(rate);
+		ep->freqn = get_usb_high_speed_rate(ep->cur_rate);
 		ep->pps = 8000 >> ep->datainterval;
 	}
 
-	ep->sample_rem = rate % ep->pps;
-	ep->packsize[0] = rate / ep->pps;
-	ep->packsize[1] = (rate + (ep->pps - 1)) / ep->pps;
+	ep->sample_rem = ep->cur_rate % ep->pps;
+	ep->packsize[0] = ep->cur_rate / ep->pps;
+	ep->packsize[1] = (ep->cur_rate + (ep->pps - 1)) / ep->pps;
 
 	/* calculate the frequency in 16.16 format */
 	ep->freqm = ep->freqn;
@@ -1144,9 +1104,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 
 	switch (ep->type) {
 	case  SND_USB_ENDPOINT_TYPE_DATA:
-		err = data_ep_set_params(ep, pcm_format, channels,
-					 period_bytes, period_frames,
-					 buffer_periods, fmt, sync_ep);
+		err = data_ep_set_params(ep);
 		break;
 	case  SND_USB_ENDPOINT_TYPE_SYNC:
 		err = sync_ep_set_params(ep);
@@ -1155,24 +1113,92 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep,
 		err = -EINVAL;
 	}
 
-	usb_audio_dbg(ep->chip, "Set up %d URBS, ret=%d\n", ep->nurbs, err);
+	usb_audio_dbg(chip, "Set up %d URBS, ret=%d\n", ep->nurbs, err);
 
 	if (err < 0)
 		return err;
 
-	/* record the current set up in the endpoint (for implicit fb) */
-	spin_lock_irq(&ep->lock);
-	ep->cur_rate = rate;
-	ep->cur_channels = channels;
-	ep->cur_format = pcm_format;
-	ep->cur_period_frames = period_frames;
-	ep->cur_period_bytes = period_bytes;
-	ep->cur_buffer_periods = buffer_periods;
-	spin_unlock_irq(&ep->lock);
+	/* some unit conversions in runtime */
+	ep->maxframesize = ep->maxpacksize / ep->cur_frame_bytes;
+	ep->curframesize = ep->curpacksize / ep->cur_frame_bytes;
 
 	return 0;
 }
 
+/*
+ * snd_usb_endpoint_configure: Configure the endpoint
+ *
+ * This function sets up the EP to be fully usable state.
+ * It's called either from hw_params or prepare callback.
+ * The function checks need_setup flag, and perfoms nothing unless needed,
+ * so it's safe to call this multiple times.
+ *
+ * This returns zero if unchanged, 1 if the configuration has changed,
+ * or a negative error code.
+ */
+int snd_usb_endpoint_configure(struct snd_usb_audio *chip,
+			       struct snd_usb_endpoint *ep)
+{
+	bool iface_first;
+	int err = 0;
+
+	mutex_lock(&chip->mutex);
+	if (!ep->need_setup)
+		goto unlock;
+
+	/* No need to (re-)configure the sync EP belonging to the same altset */
+	if (ep->ep_idx) {
+		err = snd_usb_endpoint_set_params(chip, ep);
+		if (err < 0)
+			goto unlock;
+		goto done;
+	}
+
+	/* Need to deselect altsetting at first */
+	endpoint_set_interface(chip, ep, false);
+
+	/* Some UAC1 devices (e.g. Yamaha THR10) need the host interface
+	 * to be set up before parameter setups
+	 */
+	iface_first = ep->cur_audiofmt->protocol == UAC_VERSION_1;
+	if (iface_first) {
+		err = endpoint_set_interface(chip, ep, true);
+		if (err < 0)
+			goto unlock;
+	}
+
+	err = snd_usb_init_pitch(chip, ep->cur_audiofmt);
+	if (err < 0)
+		goto unlock;
+
+	err = snd_usb_init_sample_rate(chip, ep->cur_audiofmt, ep->cur_rate);
+	if (err < 0)
+		goto unlock;
+
+	err = snd_usb_endpoint_set_params(chip, ep);
+	if (err < 0)
+		goto unlock;
+
+	err = snd_usb_select_mode_quirk(chip, ep->cur_audiofmt);
+	if (err < 0)
+		goto unlock;
+
+	/* for UAC2/3, enable the interface altset here at last */
+	if (!iface_first) {
+		err = endpoint_set_interface(chip, ep, true);
+		if (err < 0)
+			goto unlock;
+	}
+
+ done:
+	ep->need_setup = false;
+	err = 1;
+
+unlock:
+	mutex_unlock(&chip->mutex);
+	return err;
+}
+
 /**
  * snd_usb_endpoint_start: start an snd_usb_endpoint
  *
@@ -1194,6 +1220,9 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 	if (atomic_read(&ep->chip->shutdown))
 		return -EBADFD;
 
+	if (ep->sync_master)
+		WRITE_ONCE(ep->sync_master->sync_slave, ep);
+
 	usb_audio_dbg(ep->chip, "Starting %s EP 0x%x (count %d)\n",
 		      ep_type_name(ep->type), ep->ep_num, ep->use_count);
 
@@ -1226,6 +1255,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 			list_add_tail(&ctx->ready_list, &ep->ready_playback_urbs);
 		}
 
+		usb_audio_dbg(ep->chip, "No URB submission due to implicit fb sync\n");
 		return 0;
 	}
 
@@ -1251,9 +1281,13 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep)
 		set_bit(i, &ep->active_mask);
 	}
 
+	usb_audio_dbg(ep->chip, "%d URBs submitted for EP 0x%x\n",
+		      ep->nurbs, ep->ep_num);
 	return 0;
 
 __error:
+	if (ep->sync_master)
+		WRITE_ONCE(ep->sync_master->sync_slave, NULL);
 	clear_bit(EP_FLAG_RUNNING, &ep->flags);
 	ep->use_count--;
 	deactivate_urbs(ep, false);
@@ -1285,6 +1319,9 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep)
 	if (snd_BUG_ON(ep->use_count == 0))
 		return;
 
+	if (ep->sync_master)
+		WRITE_ONCE(ep->sync_master->sync_slave, NULL);
+
 	if (--ep->use_count == 0) {
 		deactivate_urbs(ep, false);
 		set_bit(EP_FLAG_STOPPING, &ep->flags);
@@ -1292,33 +1329,6 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep)
 }
 
 /**
- * snd_usb_endpoint_deactivate: deactivate an snd_usb_endpoint
- *
- * @ep: the endpoint to deactivate
- *
- * If the endpoint is not currently in use, this functions will
- * deactivate its associated URBs.
- *
- * In case of any active users, this functions does nothing.
- */
-void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)
-{
-	if (!ep)
-		return;
-
-	if (ep->use_count != 0)
-		return;
-
-	deactivate_urbs(ep, true);
-	wait_clear_urbs(ep);
-
-	/* clear the saved hw params */
-	spin_lock_irq(&ep->lock);
-	ep->cur_rate = 0;
-	spin_unlock_irq(&ep->lock);
-}
-
-/**
  * snd_usb_endpoint_release: Tear down an snd_usb_endpoint
  *
  * @ep: the endpoint to release
@@ -1343,7 +1353,7 @@ void snd_usb_endpoint_free(struct snd_usb_endpoint *ep)
 	kfree(ep);
 }
 
-/**
+/*
  * snd_usb_handle_sync_urb: parse an USB sync packet
  *
  * @ep: the endpoint to handle the packet
@@ -1353,9 +1363,9 @@ void snd_usb_endpoint_free(struct snd_usb_endpoint *ep)
  * This function is called from the context of an endpoint that received
  * the packet and is used to let another endpoint object handle the payload.
  */
-void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep,
-			     struct snd_usb_endpoint *sender,
-			     const struct urb *urb)
+static 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;