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 <[email protected]>
Tested-by: Dylan Robinson <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Takashi Iwai <[email protected]>

Author: tiwai

sound/usb/card.c

@@ -980,18 +980,18 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message)
 {
 	struct snd_usb_audio *chip = usb_get_intfdata(intf);
 	struct snd_usb_stream *as;
+	struct snd_usb_endpoint *ep;
 	struct usb_mixer_interface *mixer;
 	struct list_head *p;
 
 	if (chip == (void *)-1L)
 		return 0;
 
 	if (!chip->num_suspended_intf++) {
-		list_for_each_entry(as, &chip->pcm_list, list) {
+		list_for_each_entry(as, &chip->pcm_list, list)
 			snd_usb_pcm_suspend(as);
-			as->substream[0].need_setup_ep =
-				as->substream[1].need_setup_ep = true;
-		}
+		list_for_each_entry(ep, &chip->ep_list, list)
+			snd_usb_endpoint_suspend(ep);
 		list_for_each(p, &chip->midi_list)
 			snd_usbmidi_suspend(p);
 		list_for_each_entry(mixer, &chip->mixer_list, list)

sound/usb/card.h

@@ -26,6 +26,7 @@ struct audioformat {
 	unsigned char sync_ep;		/* sync endpoint number */
 	unsigned char sync_iface;	/* sync EP interface */
 	unsigned char sync_altsetting;	/* sync EP alternate setting */
+	unsigned char sync_ep_idx;	/* sync EP array index */
 	unsigned char datainterval;	/* log_2 of data packet interval */
 	unsigned char protocol;		/* UAC_VERSION_1/2/3 */
 	unsigned int maxpacksize;	/* max. packet size */
@@ -58,6 +59,7 @@ struct snd_urb_ctx {
 struct snd_usb_endpoint {
 	struct snd_usb_audio *chip;
 
+	int opened;		/* open refcount; protect with chip->mutex */
 	int use_count;
 	int ep_num;		/* the referenced endpoint number */
 	int type;		/* SND_USB_ENDPOINT_TYPE_* */
@@ -110,14 +112,18 @@ struct snd_usb_endpoint {
 	unsigned char silence_value;
 	unsigned int stride;
 	int iface, altsetting;
+	unsigned char ep_idx;		/* endpoint array index */
 	int skip_packets;		/* quirks for devices to ignore the first n packets
 					   in a stream */
-	bool is_implicit_feedback;      /* This endpoint is used as implicit feedback */
+	bool implicit_fb_sync;		/* syncs with implicit feedback */
+	bool need_setup;		/* (re-)need for configure? */
 
 	/* for hw constraints */
+	struct audioformat *cur_audiofmt;
 	unsigned int cur_rate;
 	snd_pcm_format_t cur_format;
 	unsigned int cur_channels;
+	unsigned int cur_frame_bytes;
 	unsigned int cur_period_frames;
 	unsigned int cur_period_bytes;
 	unsigned int cur_buffer_periods;
@@ -152,7 +158,6 @@ struct snd_usb_substream {
 	unsigned int stream_offset_adj;	/* Bytes to drop from beginning of stream (for non-compliant devices) */
 
 	unsigned int running: 1;	/* running status */
-	unsigned int fixed_hw:1;	/* fixed hw constraints due to sync EP */
 
 	unsigned int hwptr_done;	/* processed byte position in the buffer */
 	unsigned int transfer_done;		/* processed frames since last period update */
@@ -163,8 +168,6 @@ struct snd_usb_substream {
 	struct snd_usb_endpoint *data_endpoint;
 	struct snd_usb_endpoint *sync_endpoint;
 	unsigned long flags;
-	bool need_setup_ep;		/* (re)configure EP at prepare? */
-	bool need_setup_fmt;		/* (re)configure fmt after resume? */
 	unsigned int speed;		/* USB_SPEED_XXX */
 
 	u64 formats;			/* format bitmasks (all or'ed) */

sound/usb/clock.c

@@ -613,7 +613,6 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip,
 static int set_sample_rate_v2v3(struct snd_usb_audio *chip,
 				struct audioformat *fmt, int rate)
 {
-	struct usb_device *dev = chip->dev;
 	int cur_rate, prev_rate;
 	int clock;
 
@@ -656,15 +655,6 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip,
 		return -ENXIO;
 	}
 
-	/* Some devices doesn't respond to sample rate changes while the
-	 * interface is active. */
-	if (rate != prev_rate) {
-		usb_set_interface(dev, fmt->iface, 0);
-		snd_usb_set_interface_quirk(chip);
-		usb_set_interface(dev, fmt->iface, fmt->altsetting);
-		snd_usb_set_interface_quirk(chip);
-	}
-
 validation:
 	/* validate clock after rate change */
 	if (!uac_clock_source_is_valid(chip, fmt, clock))
@@ -675,6 +665,9 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip,
 int snd_usb_init_sample_rate(struct snd_usb_audio *chip,
 			     struct audioformat *fmt, int rate)
 {
+	usb_audio_dbg(chip, "%d:%d Set sample rate %d, clock %d\n",
+		      fmt->iface, fmt->altsetting, rate, fmt->clock);
+
 	switch (fmt->protocol) {
 	case UAC_VERSION_1:
 	default: