The PCM trigger callback is atomic, hence we must not call a function like usb_set_interface() there. Calling it from there would lead to a kernel Oops.

Fix it by moving the usb_set_interface() call to set_sync_endpoint().

Also, apply the snd_usb_set_interface_quirk() for consistency, too.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 3265155df1b5..380d7275d187 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -232,21 +232,6 @@ static int start_endpoints(struct snd_usb_substream *subs) !test_and_set_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { struct snd_usb_endpoint *ep = subs->sync_endpoint;

- if (subs->data_endpoint->iface != subs->sync_endpoint->iface || - subs->data_endpoint->altsetting != subs->sync_endpoint->altsetting) { - err = usb_set_interface(subs->dev, - subs->sync_endpoint->iface, - subs->sync_endpoint->altsetting); - if (err < 0) { - clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); - dev_err(&subs->dev->dev, - "%d:%d: cannot set interface (%d)\n", - subs->sync_endpoint->iface, - subs->sync_endpoint->altsetting, err); - return -EIO; - } - } - dev_dbg(&subs->dev->dev, "Starting sync EP @%p\n", ep);

ep->sync_slave = subs->data_endpoint; @@ -530,6 +515,19 @@ static int set_sync_endpoint(struct snd_usb_substream *subs,

subs->data_endpoint->sync_master = subs->sync_endpoint;

+ if (subs->data_endpoint->iface != subs->sync_endpoint->iface || + subs->data_endpoint->altsetting != subs->sync_endpoint->altsetting) { + err = usb_set_interface(subs->dev, + subs->sync_endpoint->iface, + subs->sync_endpoint->altsetting); + if (err < 0) + return err; + dev_dbg(&dev->dev, "setting usb interface %d:%d\n", + subs->sync_endpoint->iface, + subs->sync_endpoint->altsetting); + snd_usb_set_interface_quirk(dev); + } + return 0; }

It seems that many UAC2 devices are with the implicit feedback, but they couldn't be probed properly because the assumption the driver takes currently isn't applied: they have the single endpoint for both data and implicit-fb streams, while we checked only the classical sync endpoints assigned to the next altsetting in the same interface.

This patch extends the search to match with those typical cases where the implicit fb stream is found in the next interface number.

While we're at it, slightly refactor the code, not returning 0/-ERROR but use the standard bool to success/failur, which is more intuitive in this particular case.

Reported-by: Dylan Robinson dylan_robinson@motu.com Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 83 +++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 17 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 380d7275d187..2b11c2c837bf 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -272,33 +272,70 @@ static int snd_usb_pcm_sync_stop(struct snd_pcm_substream *substream) return 0; }

-static int search_roland_implicit_fb(struct usb_device *dev, int ifnum, - unsigned int altsetting, - struct usb_host_interface **alts, - unsigned int *ep) +/* Check whether the given iface:altsetting points to an implicit fb source */ +static bool search_generic_implicit_fb(struct usb_device *dev, int ifnum, + unsigned int altsetting, + struct usb_host_interface **altsp, + unsigned int *ep) { struct usb_interface *iface; + struct usb_host_interface *alts; + struct usb_interface_descriptor *altsd; + struct usb_endpoint_descriptor *epd; + + iface = usb_ifnum_to_if(dev, ifnum); + if (!iface) + return false; + alts = usb_altnum_to_altsetting(iface, altsetting); + if (!alts) + return false; + altsd = get_iface_desc(alts); + if (altsd->bInterfaceClass != USB_CLASS_AUDIO || + altsd->bInterfaceSubClass != USB_SUBCLASS_AUDIOSTREAMING || + altsd->bInterfaceProtocol != UAC_VERSION_2 || + altsd->bNumEndpoints < 1) + return false; + epd = get_endpoint(alts, 0); + if (!usb_endpoint_is_isoc_in(epd) || + (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != + USB_ENDPOINT_USAGE_IMPLICIT_FB) + return false; + *ep = epd->bEndpointAddress; + *altsp = alts; + return true; +} + +/* Like the function above, but specific to Roland with vendor class and hack */ +static bool search_roland_implicit_fb(struct usb_device *dev, int ifnum, + unsigned int altsetting, + struct usb_host_interface **altsp, + unsigned int *ep) +{ + struct usb_interface *iface; + struct usb_host_interface *alts; struct usb_interface_descriptor *altsd; struct usb_endpoint_descriptor *epd;

iface = usb_ifnum_to_if(dev, ifnum); - if (!iface || iface->num_altsetting < altsetting + 1) - return -ENOENT; - *alts = &iface->altsetting[altsetting]; - altsd = get_iface_desc(*alts); - if (altsd->bAlternateSetting != altsetting || - altsd->bInterfaceClass != USB_CLASS_VENDOR_SPEC || + if (!iface) + return false; + alts = usb_altnum_to_altsetting(iface, altsetting); + if (!alts) + return false; + altsd = get_iface_desc(alts); + if (altsd->bInterfaceClass != USB_CLASS_VENDOR_SPEC || (altsd->bInterfaceSubClass != 2 && - altsd->bInterfaceProtocol != 2 ) || + altsd->bInterfaceProtocol != 2) || altsd->bNumEndpoints < 1) - return -ENOENT; - epd = get_endpoint(*alts, 0); + return false; + epd = get_endpoint(alts, 0); if (!usb_endpoint_is_isoc_in(epd) || (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != USB_ENDPOINT_USAGE_IMPLICIT_FB) - return -ENOENT; + return false; *ep = epd->bEndpointAddress; - return 0; + *altsp = alts; + return true; }

/* Setup an implicit feedback endpoint from a quirk. Returns 0 if no quirk @@ -375,6 +412,19 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs, return 0; }

+ /* Generic UAC2 implicit feedback */ + if (attr == USB_ENDPOINT_SYNC_ASYNC && + altsd->bInterfaceClass == USB_CLASS_AUDIO && + altsd->bInterfaceProtocol == UAC_VERSION_2 && + altsd->bNumEndpoints == 1) { + ifnum = altsd->bInterfaceNumber + 1; + if (search_generic_implicit_fb(dev, ifnum, + altsd->bAlternateSetting, + &alts, &ep)) + goto add_sync_ep; + } + + /* Roland/BOSS implicit feedback with vendor spec class */ if (attr == USB_ENDPOINT_SYNC_ASYNC && altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC && altsd->bInterfaceProtocol == 2 && @@ -382,9 +432,8 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs, USB_ID_VENDOR(subs->stream->chip->usb_id) == 0x0582 /* Roland */ && search_roland_implicit_fb(dev, altsd->bInterfaceNumber + 1, altsd->bAlternateSetting, - &alts, &ep) >= 0) { + &alts, &ep)) goto add_sync_ep; - }

/* No quirk */ return 0;

The sync EP setup isn't cleared at stopping the stream but expected to be cleared at the next stream start. This may leave the sync link setup stale and can spoof wrongly when full duplex streams were running in the implicit fb sync. Let's initialize them properly at start and end of the stream.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 2b11c2c837bf..8800ec627a73 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -238,6 +238,7 @@ static int start_endpoints(struct snd_usb_substream *subs) err = snd_usb_endpoint_start(ep); if (err < 0) { clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); + ep->sync_slave = NULL; return err; } } @@ -253,8 +254,10 @@ static void sync_pending_stops(struct snd_usb_substream *subs)

static void stop_endpoints(struct snd_usb_substream *subs) { - if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) + if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { snd_usb_endpoint_stop(subs->sync_endpoint); + subs->sync_endpoint->sync_slave = NULL; + }

if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) snd_usb_endpoint_stop(subs->data_endpoint); @@ -471,26 +474,10 @@ static int set_sync_endpoint(struct snd_usb_substream *subs, bool implicit_fb; int err;

- /* we need a sync pipe in async OUT or adaptive IN mode */ - /* check the number of EP, since some devices have broken - * descriptors which fool us. if it has only one EP, - * assume it as adaptive-out or sync-in. - */ attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE;

- if ((is_playback && (attr != USB_ENDPOINT_SYNC_ASYNC)) || - (!is_playback && (attr != USB_ENDPOINT_SYNC_ADAPTIVE))) { - - /* - * In these modes the notion of sync_endpoint is irrelevant. - * Reset pointers to avoid using stale data from previously - * used settings, e.g. when configuration and endpoints were - * changed - */ - - subs->sync_endpoint = NULL; - subs->data_endpoint->sync_master = NULL; - } + subs->sync_endpoint = NULL; + subs->data_endpoint->sync_master = NULL;

err = set_sync_ep_implicit_fb_quirk(subs, dev, altsd, attr); if (err < 0) @@ -939,6 +926,11 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) sync_pending_stops(subs); snd_usb_endpoint_deactivate(subs->sync_endpoint); snd_usb_endpoint_deactivate(subs->data_endpoint); + if (subs->data_endpoint) { + subs->data_endpoint->sync_master = NULL; + subs->data_endpoint = NULL; + } + subs->sync_endpoint = NULL; snd_usb_unlock_shutdown(subs->stream->chip); }

Instead of parsing and evaluating the sync endpoint and the implicit feedback mode at each time the audio stream is opened, let's parse it once at the probe time, as the all needed information can be obtained statically from the descriptor or from the quirk.

This patch extends audioformat struct to record the sync endpoint, interface and altsetting as well as the implicit feedback flag, which are filled at parsing the streams. Then, set_sync_endpoint() is much simplified just to follow the already parsed data.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.h | 4 ++ sound/usb/pcm.c | 149 +++++++++++++++++++++++++++++++++-------------------- sound/usb/pcm.h | 2 + sound/usb/stream.c | 2 + 4 files changed, 100 insertions(+), 57 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h index 3cc668f98f43..898a283576df 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -22,6 +22,10 @@ struct audioformat { unsigned char attributes; /* corresponding attributes of cs endpoint */ unsigned char endpoint; /* endpoint */ unsigned char ep_attr; /* endpoint attributes */ + bool implicit_fb; /* implicit feedback endpoint */ + unsigned char sync_ep; /* sync endpoint number */ + unsigned char sync_iface; /* sync EP interface */ + unsigned char sync_altsetting; /* sync EP alternate setting */ unsigned char datainterval; /* log_2 of data packet interval */ unsigned char protocol; /* UAC_VERSION_1/2/3 */ unsigned int maxpacksize; /* max. packet size */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 8f4fe65d5c37..fea2764163b4 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -349,21 +349,18 @@ static bool search_roland_implicit_fb(struct usb_device *dev, int ifnum, /* Setup an implicit feedback endpoint from a quirk. Returns 0 if no quirk * applies. Returns 1 if a quirk was found. */ -static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs, - struct usb_device *dev, - struct usb_interface_descriptor *altsd, - unsigned int attr) +static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_interface *iface, + struct usb_host_interface *alts) { - struct usb_host_interface *alts; - struct usb_interface *iface; + struct usb_device *dev = chip->dev; + struct usb_interface_descriptor *altsd = get_iface_desc(alts); + unsigned int attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE; unsigned int ep; unsigned int ifnum;

- /* Implicit feedback sync EPs consumers are always playback EPs */ - if (subs->direction != SNDRV_PCM_STREAM_PLAYBACK) - return 0; - - switch (subs->stream->chip->usb_id) { + switch (chip->usb_id) { case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ case USB_ID(0x0763, 0x2031): /* M-Audio Fast Track C600 */ case USB_ID(0x22f0, 0x0006): /* Allen&Heath Qu-16 */ @@ -437,11 +434,13 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs, altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC && altsd->bInterfaceProtocol == 2 && altsd->bNumEndpoints == 1 && - USB_ID_VENDOR(subs->stream->chip->usb_id) == 0x0582 /* Roland */ && - search_roland_implicit_fb(dev, altsd->bInterfaceNumber + 1, - altsd->bAlternateSetting, - &alts, &ep)) - goto add_sync_ep; + USB_ID_VENDOR(chip->usb_id) == 0x0582 /* Roland */) { + ifnum = altsd->bInterfaceNumber + 1; + if (search_roland_implicit_fb(dev, ifnum, + altsd->bAlternateSetting, + &alts, &ep)) + goto add_sync_ep; + }

/* No quirk */ return 0; @@ -450,56 +449,59 @@ static int set_sync_ep_implicit_fb_quirk(struct snd_usb_substream *subs, iface = usb_ifnum_to_if(dev, ifnum);

if (!iface || iface->num_altsetting < 2) - return -EINVAL; + return 0;

alts = &iface->altsetting[1];

add_sync_ep: - subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip, - alts, ep, !subs->direction, - SND_USB_ENDPOINT_TYPE_DATA); - if (!subs->sync_endpoint) - return -EINVAL; - - subs->sync_endpoint->is_implicit_feedback = 1; - - subs->data_endpoint->sync_master = subs->sync_endpoint; + fmt->sync_ep = ep; + fmt->sync_iface = ifnum; + fmt->sync_altsetting = alts->desc.bAlternateSetting; + fmt->implicit_fb = 1; + dev_dbg(&dev->dev, "%d:%d: found implicit_fb sync_ep=%x, iface=%d, alt=%d\n", + fmt->iface, fmt->altsetting, fmt->sync_ep, fmt->sync_iface, + fmt->sync_altsetting);

return 1; }

-static int set_sync_endpoint(struct snd_usb_substream *subs, - struct audioformat *fmt, - struct usb_device *dev, - struct usb_host_interface *alts, - struct usb_interface_descriptor *altsd) +int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, + struct audioformat *fmt) { - int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK; - unsigned int ep, attr; - bool implicit_fb; + struct usb_device *dev = chip->dev; + struct usb_interface *iface; + struct usb_host_interface *alts; + struct usb_interface_descriptor *altsd; + unsigned int ep, attr, sync_attr; + bool is_playback; int err;

- attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE; - - subs->sync_endpoint = NULL; - subs->data_endpoint->sync_master = NULL; - - err = set_sync_ep_implicit_fb_quirk(subs, dev, altsd, attr); - if (err < 0) - return err; - - /* endpoint set by quirk */ - if (err > 0) + iface = usb_ifnum_to_if(dev, fmt->iface); + if (!iface) + return 0; + alts = usb_altnum_to_altsetting(iface, fmt->altsetting); + if (!alts) return 0; + altsd = get_iface_desc(alts); + + is_playback = !(get_endpoint(alts, 0)->bEndpointAddress & USB_DIR_IN); + if (is_playback) { + err = audioformat_implicit_fb_quirk(chip, fmt, iface, alts); + if (err > 0) + return 0; + }

if (altsd->bNumEndpoints < 2) return 0;

+ attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE; if ((is_playback && (attr == USB_ENDPOINT_SYNC_SYNC || attr == USB_ENDPOINT_SYNC_ADAPTIVE)) || (!is_playback && attr != USB_ENDPOINT_SYNC_ADAPTIVE)) return 0;

+ sync_attr = get_endpoint(alts, 1)->bmAttributes; + /* * In case of illegal SYNC_NONE for OUT endpoint, we keep going to see * if we don't find a sync endpoint, as on M-Audio Transit. In case of @@ -510,7 +512,7 @@ static int set_sync_endpoint(struct snd_usb_substream *subs, /* ... and check descriptor size before accessing bSynchAddress because there is a version of the SB Audigy 2 NX firmware lacking the audio fields in the endpoint descriptors */ - if ((get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_ISOC || + if ((sync_attr & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_ISOC || (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE && get_endpoint(alts, 1)->bSynchAddress != 0)) { dev_err(&dev->dev, @@ -537,22 +539,57 @@ static int set_sync_endpoint(struct snd_usb_substream *subs, return -EINVAL; }

- implicit_fb = (get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_USAGE_MASK) - == USB_ENDPOINT_USAGE_IMPLICIT_FB; + fmt->sync_ep = ep; + fmt->sync_iface = altsd->bInterfaceNumber; + fmt->sync_altsetting = altsd->bAlternateSetting; + if ((sync_attr & USB_ENDPOINT_USAGE_MASK) == USB_ENDPOINT_USAGE_IMPLICIT_FB) + fmt->implicit_fb = 1; + + dev_dbg(&dev->dev, "%d:%d: found sync_ep=0x%x, iface=%d, alt=%d, implicit_fb=%d\n", + fmt->iface, fmt->altsetting, fmt->sync_ep, fmt->sync_iface, + fmt->sync_altsetting, fmt->implicit_fb); + + return 0; +} + +static int set_sync_endpoint(struct snd_usb_substream *subs, + struct audioformat *fmt) +{ + struct usb_device *dev = subs->dev; + struct usb_interface *iface; + struct usb_host_interface *alts; + int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK; + unsigned int ep; + int err; + + subs->sync_endpoint = NULL; + subs->data_endpoint->sync_master = NULL; + + ep = fmt->sync_ep; + if (!ep) + return 0; + + iface = usb_ifnum_to_if(dev, fmt->sync_iface); + if (!iface) + return 0; + + alts = usb_altnum_to_altsetting(iface, fmt->altsetting); + if (!alts) + return 0;

subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip, alts, ep, !subs->direction, - implicit_fb ? - SND_USB_ENDPOINT_TYPE_DATA : - SND_USB_ENDPOINT_TYPE_SYNC); - + fmt->implicit_fb ? + SND_USB_ENDPOINT_TYPE_DATA : + SND_USB_ENDPOINT_TYPE_SYNC); if (!subs->sync_endpoint) { - if (is_playback && attr == USB_ENDPOINT_SYNC_NONE) + if (is_playback && + (fmt->ep_attr & USB_ENDPOINT_SYNCTYPE) == USB_ENDPOINT_SYNC_NONE) return 0; return -EINVAL; }

- subs->sync_endpoint->is_implicit_feedback = implicit_fb; + subs->sync_endpoint->is_implicit_feedback = fmt->implicit_fb;

subs->data_endpoint->sync_master = subs->sync_endpoint;

@@ -579,7 +616,6 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) { struct usb_device *dev = subs->dev; struct usb_host_interface *alts; - struct usb_interface_descriptor *altsd; struct usb_interface *iface; int err;

@@ -589,7 +625,6 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) alts = usb_altnum_to_altsetting(iface, fmt->altsetting); if (WARN_ON(!alts)) return -EINVAL; - altsd = get_iface_desc(alts);

if (fmt == subs->cur_audiofmt && !subs->need_setup_fmt) return 0; @@ -639,7 +674,7 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) if (!subs->data_endpoint) return -EINVAL;

- err = set_sync_endpoint(subs, fmt, dev, alts, altsd); + err = set_sync_endpoint(subs, fmt); if (err < 0) return err;

diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h index 9833627c1eca..362782c2df5c 100644 --- a/sound/usb/pcm.h +++ b/sound/usb/pcm.h @@ -14,5 +14,7 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface, struct audioformat *fmt); void snd_usb_preallocate_buffer(struct snd_usb_substream *subs);

+int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, + struct audioformat *fmt);

#endif /* __USBAUDIO_PCM_H */ diff --git a/sound/usb/stream.c b/sound/usb/stream.c index f17913e0b5b4..7087ee2c8174 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -1193,6 +1193,8 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, continue; }

+ snd_usb_audioformat_set_sync_ep(chip, fp); + dev_dbg(&dev->dev, "%u:%d: add audio endpoint %#x\n", iface_no, altno, fp->endpoint); if (protocol == UAC_VERSION_3) err = snd_usb_add_audio_stream_v3(chip, stream, fp, pd);

In the current code, there is no check at the stream open time whether the endpoint is being already used by others. In the normal operations, this shouldn't happen, but in the case of the implicit feedback mode, it's a common problem with the full duplex operation, because the capture stream is always opened by the playback stream as an implicit sync source.

Although we recently introduced the check of such a conflict of parameters at the PCM hw_params time, it doesn't give any hint at the hw_params itself and just gives the error. This isn't quite comfortable, and it caused problems on many applications.

This patch attempts to make the parameter handling easier by introducing the strict hw constraint matching with the counterpart stream that is being used. That said, when an implicit feedback playback stream is running before a capture stream is opened, the capture stream carries the PCM hw-constraint to allow only the same sample rate, format, periods and period frames as the running playback stream. If not opened or there is no conflict of endpoints, the behavior remains as same as before.

Note that this kind of "weak link" should work for most cases, but this is no concrete solution; e.g. if an application changes the hw params multiple times while another stream is opened, this would lead to inconsistencies.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.h | 9 ++++ sound/usb/endpoint.c | 72 ++++++++++++++++++------- sound/usb/pcm.c | 148 +++++++++++++++++++++++++++++++++++++++------------ 3 files changed, 175 insertions(+), 54 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h index 898a283576df..1f61be98a31d 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -114,6 +114,14 @@ struct snd_usb_endpoint { in a stream */ bool is_implicit_feedback; /* This endpoint is used as implicit feedback */

+ /* for hw constraints */ + unsigned int cur_rate; + snd_pcm_format_t cur_format; + unsigned int cur_channels; + unsigned int cur_period_frames; + unsigned int cur_period_bytes; + unsigned int cur_buffer_periods; + spinlock_t lock; struct list_head list; }; @@ -144,6 +152,7 @@ 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 */ diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 8205a64a734e..94490d706013 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -459,6 +459,9 @@ snd_usb_get_endpoint(struct snd_usb_audio *chip, return NULL; }

+#define ep_type_name(type) \ + (type == SND_USB_ENDPOINT_TYPE_DATA ? "data" : "sync") + /** * snd_usb_add_endpoint: Add an endpoint to an USB audio chip * @@ -500,9 +503,9 @@ struct snd_usb_endpoint *snd_usb_add_endpoint(struct snd_usb_audio *chip, }

usb_audio_dbg(chip, "Creating new %s %s endpoint #%x\n", - is_playback ? "playback" : "capture", - type == SND_USB_ENDPOINT_TYPE_DATA ? "data" : "sync", - ep_num); + is_playback ? "playback" : "capture", + ep_type_name(type), + ep_num);

ep = kzalloc(sizeof(*ep), GFP_KERNEL); if (!ep) @@ -644,13 +647,14 @@ 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, - struct audioformat *fmt, - struct snd_usb_endpoint *sync_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; @@ -660,6 +664,14 @@ static bool check_ep_params(struct snd_usb_endpoint *ep, 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 @@ -917,7 +929,8 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, * as their corresponding capture endpoint. */ if (usb_pipein(ep->pipe) || - snd_usb_endpoint_implicit_feedback_sink(ep)) { + ep->is_implicit_feedback || + snd_usb_endpoint_implicit_feedback_sink(ep)) {

urb_packs = packs_per_ms; /* @@ -1076,12 +1089,17 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, { 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, - fmt, sync_ep); + check_ep_params(ep, pcm_format, channels, period_bytes, + period_frames, buffer_periods, rate, + fmt, sync_ep);

if (!check) { usb_audio_warn(ep->chip, @@ -1134,11 +1152,22 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, err = -EINVAL; }

- usb_audio_dbg(ep->chip, - "Setting params for ep #%x (type %d, %d urbs), ret=%d\n", - ep->ep_num, ep->type, ep->nurbs, err); + usb_audio_dbg(ep->chip, "Set up %d URBS, ret=%d\n", ep->nurbs, err);

- return 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); + + return 0; }

/** @@ -1273,6 +1302,11 @@ void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep)

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); }

/** diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 78933b6571d0..6d1f5277cd90 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -81,30 +81,33 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream /* * find a matching audio format */ -static struct audioformat *find_format(struct snd_usb_substream *subs) +static struct audioformat *find_format(struct list_head *fmt_list_head, + snd_pcm_format_t format, + unsigned int rate, + unsigned int channels, + struct snd_usb_substream *subs) { struct audioformat *fp; struct audioformat *found = NULL; int cur_attr = 0, attr;

- list_for_each_entry(fp, &subs->fmt_list, list) { - if (!(fp->formats & pcm_format_to_bits(subs->pcm_format))) + list_for_each_entry(fp, fmt_list_head, list) { + if (!(fp->formats & pcm_format_to_bits(format))) continue; - if (fp->channels != subs->channels) + if (fp->channels != channels) continue; - if (subs->cur_rate < fp->rate_min || - subs->cur_rate > fp->rate_max) + if (rate < fp->rate_min || rate > fp->rate_max) continue; - if (! (fp->rates & SNDRV_PCM_RATE_CONTINUOUS)) { + if (!(fp->rates & SNDRV_PCM_RATE_CONTINUOUS)) { unsigned int i; for (i = 0; i < fp->nr_rates; i++) - if (fp->rate_table[i] == subs->cur_rate) + if (fp->rate_table[i] == rate) break; if (i >= fp->nr_rates) continue; } attr = fp->ep_attr & USB_ENDPOINT_SYNCTYPE; - if (! found) { + if (!found) { found = fp; cur_attr = attr; continue; @@ -114,7 +117,7 @@ static struct audioformat *find_format(struct snd_usb_substream *subs) * this is a workaround for the case like * M-audio audiophile USB. */ - if (attr != cur_attr) { + if (subs && attr != cur_attr) { if ((attr == USB_ENDPOINT_SYNC_ASYNC && subs->direction == SNDRV_PCM_STREAM_PLAYBACK) || (attr == USB_ENDPOINT_SYNC_ADAPTIVE && @@ -138,6 +141,12 @@ static struct audioformat *find_format(struct snd_usb_substream *subs) return found; }

+static struct audioformat *find_substream_format(struct snd_usb_substream *subs) +{ + return find_format(&subs->fmt_list, subs->pcm_format, subs->cur_rate, + subs->channels, subs); +} + static int init_pitch_v1(struct snd_usb_audio *chip, int iface, struct usb_host_interface *alts, struct audioformat *fmt) @@ -744,7 +753,6 @@ static int match_endpoint_audioformats(struct snd_usb_substream *subs, */ static int configure_sync_endpoint(struct snd_usb_substream *subs) { - int ret; struct audioformat *fp; struct audioformat *sync_fp = NULL; int cur_score = 0; @@ -752,16 +760,16 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs) struct snd_usb_substream *sync_subs = &subs->stream->substream[subs->direction ^ 1];

- if (subs->sync_endpoint->type != SND_USB_ENDPOINT_TYPE_DATA || - !subs->stream) - return snd_usb_endpoint_set_params(subs->sync_endpoint, - subs->pcm_format, - subs->channels, - subs->period_bytes, - 0, 0, - subs->cur_rate, - subs->cur_audiofmt, - NULL); + if (subs->fixed_hw || + !subs->sync_endpoint->is_implicit_feedback) { + sync_fp = subs->cur_audiofmt; + goto configure; + } + + sync_fp = find_format(&sync_subs->fmt_list, subs->pcm_format, + subs->cur_rate, subs->channels, NULL); + if (sync_fp) + goto configure;

/* Try to find the best matching audioformat. */ list_for_each_entry(fp, &sync_subs->fmt_list, list) { @@ -794,16 +802,16 @@ static int configure_sync_endpoint(struct snd_usb_substream *subs) __func__, subs->period_bytes, sync_period_bytes); }

- ret = snd_usb_endpoint_set_params(subs->sync_endpoint, - subs->pcm_format, - sync_fp->channels, - sync_period_bytes, - 0, 0, - subs->cur_rate, - sync_fp, - NULL); - - return ret; + configure: + return snd_usb_endpoint_set_params(subs->sync_endpoint, + subs->pcm_format, + sync_fp->channels, + sync_period_bytes, + subs->period_frames, + subs->buffer_periods, + subs->cur_rate, + sync_fp, + NULL); }

/* @@ -912,7 +920,7 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, subs->channels = params_channels(hw_params); subs->cur_rate = params_rate(hw_params);

- fmt = find_format(subs); + fmt = find_substream_format(subs); if (!fmt) { dev_dbg(&subs->dev->dev, "cannot set format: format = %#x, rate = %d, channels = %d\n", @@ -956,12 +964,13 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, static int snd_usb_hw_free(struct snd_pcm_substream *substream) { struct snd_usb_substream *subs = substream->runtime->private_data; + struct snd_usb_audio *chip = subs->stream->chip;

snd_media_stop_pipeline(subs); subs->cur_audiofmt = NULL; subs->cur_rate = 0; subs->period_bytes = 0; - if (!snd_usb_lock_shutdown(subs->stream->chip)) { + if (!snd_usb_lock_shutdown(chip)) { stop_endpoints(subs); sync_pending_stops(subs); snd_usb_endpoint_deactivate(subs->sync_endpoint); @@ -971,7 +980,7 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) subs->data_endpoint = NULL; } subs->sync_endpoint = NULL; - snd_usb_unlock_shutdown(subs->stream->chip); + snd_usb_unlock_shutdown(chip); }

return 0; @@ -1288,6 +1297,64 @@ static int hw_rule_period_time(struct snd_pcm_hw_params *params, return changed; }

+/* apply PCM hw constraints from the concurrent sync EP */ +static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime, + struct snd_usb_substream *subs) +{ + struct snd_usb_audio *chip = subs->stream->chip; + struct snd_usb_endpoint *ep; + struct audioformat *fp; + int err; + + subs->fixed_hw = 0; + list_for_each_entry(fp, &subs->fmt_list, list) { + ep = snd_usb_get_endpoint(chip, fp->endpoint, fp->iface, + fp->altsetting); + if (ep && ep->cur_rate) + goto found; + if (!fp->implicit_fb) + continue; + /* for the implicit fb, check the sync ep as well */ + ep = snd_usb_get_endpoint(chip, fp->sync_ep, fp->sync_iface, + fp->sync_altsetting); + if (ep && ep->cur_rate) + goto found; + } + return 0; + + found: + if (!find_format(&subs->fmt_list, ep->cur_format, ep->cur_rate, + ep->cur_channels, NULL)) { + usb_audio_dbg(chip, "EP 0x%x being used, but not applicable\n", + ep->ep_num); + return 0; + } + + usb_audio_dbg(chip, "EP 0x%x being used, using fixed params:\n", + ep->ep_num); + usb_audio_dbg(chip, "rate=%d, format=%s, channels=%d, period_size=%d, periods=%d\n", + ep->cur_rate, snd_pcm_format_name(ep->cur_format), + ep->cur_channels, ep->cur_period_frames, + ep->cur_buffer_periods); + + runtime->hw.formats = pcm_format_to_bits(ep->cur_format); + runtime->hw.rate_min = runtime->hw.rate_max = ep->cur_rate; + runtime->hw.channels_min = runtime->hw.channels_max = + ep->cur_channels; + runtime->hw.rates = SNDRV_PCM_RATE_KNOT; + runtime->hw.periods_min = runtime->hw.periods_max = + ep->cur_buffer_periods; + subs->fixed_hw = 1; + + err = snd_pcm_hw_constraint_minmax(runtime, + SNDRV_PCM_HW_PARAM_PERIOD_SIZE, + ep->cur_period_frames, + ep->cur_period_frames); + if (err < 0) + return err; + + return 1; /* notify the finding */ +}

/* * set up the runtime hardware information. @@ -1295,11 +1362,20 @@ static int hw_rule_period_time(struct snd_pcm_hw_params *params,

static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substream *subs) { + struct snd_usb_audio *chip = subs->stream->chip; struct audioformat *fp; unsigned int pt, ptmin; - int param_period_time_if_needed; + int param_period_time_if_needed = -1; int err;

+ mutex_lock(&chip->mutex); + err = apply_hw_constraint_from_sync(runtime, subs); + mutex_unlock(&chip->mutex); + if (err < 0) + return err; + if (err > 0) /* found the matching? */ + goto add_extra_rules; + runtime->hw.formats = subs->formats;

runtime->hw.rate_min = 0x7fffffff; @@ -1350,6 +1426,8 @@ static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substre -1); if (err < 0) return err; + +add_extra_rules: err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, hw_rule_channels, subs, SNDRV_PCM_HW_PARAM_FORMAT,

The file debug.h contains a simple macro for debug prints, and it's used only in two places, the format parser and the hw_params rules. The former actually should print a more informative message instead, so the only users are the hw_parmas rules.

This patch moves the contents of debug.h into the hw_params rules local code and remove the unneeded includes. Also, the debug print in the format parser is replaced with the information print with more useful information, and the raw printk() call is replaced with pr_debug().

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.c | 1 - sound/usb/debug.h | 16 ---------------- sound/usb/format.c | 5 +++-- sound/usb/pcm.c | 11 ++++++++++- 4 files changed, 13 insertions(+), 20 deletions(-) delete mode 100644 sound/usb/debug.h

diff --git a/sound/usb/card.c b/sound/usb/card.c index 4457214a3ae6..096dd8e3c64b 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -49,7 +49,6 @@ #include "quirks.h" #include "endpoint.h" #include "helper.h" -#include "debug.h" #include "pcm.h" #include "format.h" #include "power.h" diff --git a/sound/usb/debug.h b/sound/usb/debug.h deleted file mode 100644 index 7dd983c35001..000000000000 --- a/sound/usb/debug.h +++ /dev/null @@ -1,16 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef __USBAUDIO_DEBUG_H -#define __USBAUDIO_DEBUG_H - -/* - * h/w constraints - */ - -#ifdef HW_CONST_DEBUG -#define hwc_debug(fmt, args...) printk(KERN_DEBUG fmt, ##args) -#else -#define hwc_debug(fmt, args...) do { } while(0) -#endif - -#endif /* __USBAUDIO_DEBUG_H */ - diff --git a/sound/usb/format.c b/sound/usb/format.c index 3348032daa16..7641716f0c6c 100644 --- a/sound/usb/format.c +++ b/sound/usb/format.c @@ -16,7 +16,6 @@ #include "card.h" #include "quirks.h" #include "helper.h" -#include "debug.h" #include "clock.h" #include "format.h"

@@ -227,7 +226,9 @@ static int parse_audio_format_rates_v1(struct snd_usb_audio *chip, struct audiof fp->nr_rates++; } if (!fp->nr_rates) { - hwc_debug("All rates were zero. Skipping format!\n"); + usb_audio_info(chip, + "%u:%d: All rates were zero\n", + fp->iface, fp->altsetting); return -EINVAL; } } else { diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ecc6bf9b42f0..d83a6a6ac023 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -17,7 +17,6 @@ #include "usbaudio.h" #include "card.h" #include "quirks.h" -#include "debug.h" #include "endpoint.h" #include "helper.h" #include "pcm.h" @@ -1061,6 +1060,16 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) return ret; }

+/* + * h/w constraints + */ + +#ifdef HW_CONST_DEBUG +#define hwc_debug(fmt, args...) pr_debug(fmt, ##args) +#else +#define hwc_debug(fmt, args...) do { } while(0) +#endif + static const struct snd_pcm_hardware snd_usb_hardware = { .info = SNDRV_PCM_INFO_MMAP |

Currently snd_usb_endpoint objects are created at first when the substream is opened and tries to assign the endpoints corresponding to the matching audioformat. But since basically the all endpoints have been already parsed and the information have been obtained, we may create the endpoint objects statically at the init phase. It's easier to manage for the implicit fb case, for example.

This patch changes the endpoint object management and lets the parser to create the all endpoint objects.

This change shouldn't bring any functional changes.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 87 ++++++++++++++++++++-------------------------------- sound/usb/endpoint.h | 10 +++--- sound/usb/pcm.c | 27 ++++++++-------- sound/usb/stream.c | 16 ++++++++++ 4 files changed, 67 insertions(+), 73 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 94490d706013..eb459db511f8 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -440,22 +440,19 @@ static void snd_complete_urb(struct urb *urb) }

/* - * Get the existing endpoint object corresponding EP, iface and alt numbers + * Get the existing endpoint object corresponding EP * Returns NULL if not present. - * Call inside chip->mutex locking for avoiding the race. */ struct snd_usb_endpoint * -snd_usb_get_endpoint(struct snd_usb_audio *chip, - int ep_num, int iface, int altsetting) +snd_usb_get_endpoint(struct snd_usb_audio *chip, int ep_num) { struct snd_usb_endpoint *ep;

list_for_each_entry(ep, &chip->ep_list, list) { - if (ep->ep_num == ep_num && - ep->iface == iface && - ep->altsetting == altsetting) + if (ep->ep_num == ep_num) return ep; } + return NULL; }

@@ -466,14 +463,13 @@ snd_usb_get_endpoint(struct snd_usb_audio *chip, * snd_usb_add_endpoint: Add an endpoint to an USB audio chip * * @chip: The chip - * @alts: The USB host interface * @ep_num: The number of the endpoint to use - * @direction: SNDRV_PCM_STREAM_PLAYBACK or SNDRV_PCM_STREAM_CAPTURE * @type: SND_USB_ENDPOINT_TYPE_DATA or SND_USB_ENDPOINT_TYPE_SYNC * * If the requested endpoint has not been added to the given chip before, - * a new instance is created. Otherwise, a pointer to the previoulsy - * created instance is returned. In case of any error, NULL is returned. + * a new instance is created. + * + * Returns zero on success or a negative error code. * * New endpoints will be added to chip->ep_list and must be freed by * calling snd_usb_endpoint_free(). @@ -481,74 +477,59 @@ snd_usb_get_endpoint(struct snd_usb_audio *chip, * For SND_USB_ENDPOINT_TYPE_SYNC, the caller needs to guarantee that * bNumEndpoints > 1 beforehand. */ -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_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type) { struct snd_usb_endpoint *ep; - int is_playback = direction == SNDRV_PCM_STREAM_PLAYBACK; - - if (WARN_ON(!alts)) - return NULL; + bool is_playback;

- mutex_lock(&chip->mutex); - - ep = snd_usb_get_endpoint(chip, ep_num, - alts->desc.bInterfaceNumber, - alts->desc.bAlternateSetting); - if (ep) { - usb_audio_dbg(ep->chip, "Re-using EP %x in iface %d,%d\n", - ep_num, ep->iface, ep->altsetting); - goto __exit_unlock; - } + ep = snd_usb_get_endpoint(chip, ep_num); + if (ep) + return 0;

- usb_audio_dbg(chip, "Creating new %s %s endpoint #%x\n", - is_playback ? "playback" : "capture", + usb_audio_dbg(chip, "Creating new %s endpoint #%x\n", ep_type_name(type), ep_num); - ep = kzalloc(sizeof(*ep), GFP_KERNEL); if (!ep) - goto __exit_unlock; + return -ENOMEM;

ep->chip = chip; spin_lock_init(&ep->lock); ep->type = type; ep->ep_num = ep_num; - ep->iface = alts->desc.bInterfaceNumber; - ep->altsetting = alts->desc.bAlternateSetting; INIT_LIST_HEAD(&ep->ready_playback_urbs); - ep_num &= USB_ENDPOINT_NUMBER_MASK;

+ is_playback = ((ep_num & USB_ENDPOINT_DIR_MASK) == USB_DIR_OUT); + 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; + list_add_tail(&ep->list, &chip->ep_list); + return 0; +} + +/* 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) +{ + 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 (get_endpoint(alts, 1)->bInterval >= 1 && - get_endpoint(alts, 1)->bInterval <= 16) - ep->syncinterval = get_endpoint(alts, 1)->bInterval - 1; + else if (desc->bInterval >= 1 && desc->bInterval <= 16) + ep->syncinterval = desc->bInterval - 1; else ep->syncinterval = 3;

- ep->syncmaxsize = le16_to_cpu(get_endpoint(alts, 1)->wMaxPacketSize); + ep->syncmaxsize = le16_to_cpu(desc->wMaxPacketSize); } - - list_add_tail(&ep->list, &chip->ep_list); - - ep->is_implicit_feedback = 0; - -__exit_unlock: - mutex_unlock(&chip->mutex); - - return ep; }

/* diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 61487095a766..76b6de7de991 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -6,12 +6,9 @@ #define SND_USB_ENDPOINT_TYPE_SYNC 1

struct snd_usb_endpoint *snd_usb_get_endpoint(struct snd_usb_audio *chip, - int ep_num, int iface, - int altsetting); + int ep_num);

-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_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type);

int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, snd_pcm_format_t pcm_format, @@ -34,6 +31,9 @@ void snd_usb_endpoint_free(struct snd_usb_endpoint *ep); int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep); +void snd_usb_endpoint_set_syncinterval(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep, + struct usb_host_interface *alts);

void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, struct snd_usb_endpoint *sender, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 8ae7d2fdba0d..03b1a02bcff4 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -585,11 +585,7 @@ static int set_sync_endpoint(struct snd_usb_substream *subs, if (!alts) return 0;

- subs->sync_endpoint = snd_usb_add_endpoint(subs->stream->chip, - alts, ep, !subs->direction, - fmt->implicit_fb ? - SND_USB_ENDPOINT_TYPE_DATA : - SND_USB_ENDPOINT_TYPE_SYNC); + subs->sync_endpoint = snd_usb_get_endpoint(subs->stream->chip, ep); if (!subs->sync_endpoint) { if (is_playback && (fmt->ep_attr & USB_ENDPOINT_SYNCTYPE) == USB_ENDPOINT_SYNC_NONE) @@ -597,10 +593,14 @@ static int set_sync_endpoint(struct snd_usb_substream *subs, return -EINVAL; }

+ subs->sync_endpoint->iface = fmt->sync_iface; + subs->sync_endpoint->altsetting = fmt->sync_altsetting; subs->sync_endpoint->is_implicit_feedback = fmt->implicit_fb;

subs->data_endpoint->sync_master = subs->sync_endpoint;

+ snd_usb_endpoint_set_syncinterval(subs->stream->chip, subs->sync_endpoint, alts); + if (!subs->sync_endpoint->use_count && (subs->data_endpoint->iface != subs->sync_endpoint->iface || subs->data_endpoint->altsetting != subs->sync_endpoint->altsetting)) { @@ -641,8 +641,7 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)

/* shared EP with implicit fb */ if (fmt->implicit_fb && !subs->need_setup_fmt) { - ep = snd_usb_get_endpoint(subs->stream->chip, fmt->endpoint, - fmt->iface, fmt->altsetting); + ep = snd_usb_get_endpoint(subs->stream->chip, fmt->endpoint); if (ep && ep->use_count > 0) goto add_data_ep; } @@ -688,12 +687,12 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) add_data_ep: subs->interface = fmt->iface; subs->altset_idx = fmt->altset_idx; - subs->data_endpoint = snd_usb_add_endpoint(subs->stream->chip, - alts, fmt->endpoint, subs->direction, - SND_USB_ENDPOINT_TYPE_DATA); - + subs->data_endpoint = snd_usb_get_endpoint(subs->stream->chip, + fmt->endpoint); if (!subs->data_endpoint) return -EINVAL; + subs->data_endpoint->iface = fmt->iface; + subs->data_endpoint->altsetting = fmt->altsetting;

err = set_sync_endpoint(subs, fmt); if (err < 0) @@ -1294,15 +1293,13 @@ static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime,

subs->fixed_hw = 0; list_for_each_entry(fp, &subs->fmt_list, list) { - ep = snd_usb_get_endpoint(chip, fp->endpoint, fp->iface, - fp->altsetting); + ep = snd_usb_get_endpoint(chip, fp->endpoint); if (ep && ep->cur_rate) goto found; if (!fp->implicit_fb) continue; /* for the implicit fb, check the sync ep as well */ - ep = snd_usb_get_endpoint(chip, fp->sync_ep, fp->sync_iface, - fp->sync_altsetting); + ep = snd_usb_get_endpoint(chip, fp->sync_ep); if (ep && ep->cur_rate) goto found; } diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 7087ee2c8174..816fd3e5aada 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -1206,6 +1206,22 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, kfree(pd); return err; } + + /* add endpoints */ + err = snd_usb_add_endpoint(chip, fp->endpoint, + SND_USB_ENDPOINT_TYPE_DATA); + if (err < 0) + return err; + + if (fp->sync_ep) { + err = snd_usb_add_endpoint(chip, fp->sync_ep, + fp->implicit_fb ? + SND_USB_ENDPOINT_TYPE_DATA : + SND_USB_ENDPOINT_TYPE_SYNC); + if (err < 0) + return err; + } + /* try to set the interface... */ usb_set_interface(chip->dev, iface_no, altno); snd_usb_init_pitch(chip, iface_no, alts, fp);

This behavior turned out to be invalid from the USB spec POV and shouldn't be applied. As it's an optional flag that is set only via an card control element that must be hardly used, let's drop it again.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/mixer.c | 46 ---------------------------------------------- sound/usb/pcm.c | 17 +++++++---------- sound/usb/usbaudio.h | 3 --- 3 files changed, 7 insertions(+), 59 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 81e987eaf063..12b15ed59eaa 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -3454,48 +3454,6 @@ static int snd_usb_mixer_status_create(struct usb_mixer_interface *mixer) return 0; }

-static int keep_iface_ctl_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol); - - ucontrol->value.integer.value[0] = mixer->chip->keep_iface; - return 0; -} - -static int keep_iface_ctl_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) -{ - struct usb_mixer_interface *mixer = snd_kcontrol_chip(kcontrol); - bool keep_iface = !!ucontrol->value.integer.value[0]; - - if (mixer->chip->keep_iface == keep_iface) - return 0; - mixer->chip->keep_iface = keep_iface; - return 1; -} - -static const struct snd_kcontrol_new keep_iface_ctl = { - .iface = SNDRV_CTL_ELEM_IFACE_CARD, - .name = "Keep Interface", - .info = snd_ctl_boolean_mono_info, - .get = keep_iface_ctl_get, - .put = keep_iface_ctl_put, -}; - -static int create_keep_iface_ctl(struct usb_mixer_interface *mixer) -{ - struct snd_kcontrol *kctl = snd_ctl_new1(&keep_iface_ctl, mixer); - - /* need only one control per card */ - if (snd_ctl_find_id(mixer->chip->card, &kctl->id)) { - snd_ctl_free_one(kctl); - return 0; - } - - return snd_ctl_add(mixer->chip->card, kctl); -} - int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif, int ignore_error) { @@ -3548,10 +3506,6 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif, if (err < 0) goto _error;

- err = create_keep_iface_ctl(mixer); - if (err < 0) - goto _error; - err = snd_usb_mixer_apply_create_quirk(mixer); if (err < 0) goto _error; diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 03b1a02bcff4..392aa1cba61c 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -648,14 +648,12 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)

/* close the old interface */ if (subs->interface >= 0 && (subs->interface != fmt->iface || subs->need_setup_fmt)) { - if (!subs->stream->chip->keep_iface) { - err = usb_set_interface(subs->dev, subs->interface, 0); - if (err < 0) { - dev_err(&dev->dev, - "%d:%d: return to setting 0 failed (%d)\n", - fmt->iface, fmt->altsetting, err); - return -EIO; - } + err = usb_set_interface(subs->dev, subs->interface, 0); + if (err < 0) { + dev_err(&dev->dev, + "%d:%d: return to setting 0 failed (%d)\n", + fmt->iface, fmt->altsetting, err); + return -EIO; } subs->interface = -1; subs->altset_idx = 0; @@ -1483,8 +1481,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)

snd_media_stop_pipeline(subs);

- if (!as->chip->keep_iface && - subs->interface >= 0 && + if (subs->interface >= 0 && !snd_usb_lock_shutdown(subs->stream->chip)) { usb_set_interface(subs->dev, subs->interface, 0); subs->interface = -1; diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 0805b7f21272..aa017a93f7bd 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -53,9 +53,6 @@ struct snd_usb_audio {

int setup; /* from the 'device_setup' module param */ bool autoclock; /* from the 'autoclock' module param */ - bool keep_iface; /* keep interface/altset after closing - * or parameter change - */

struct usb_host_interface *ctrl_intf; /* the audio control interface */ struct media_device *media_dev;

Setting the active altsetting at changing sample rate seems unrecommended. The host should deselect the altsetting at first before that, then select it again.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/stream.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 816fd3e5aada..4501e042a944 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -1223,9 +1223,10 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, }

/* try to set the interface... */ - usb_set_interface(chip->dev, iface_no, altno); + usb_set_interface(chip->dev, iface_no, 0); snd_usb_init_pitch(chip, iface_no, alts, fp); snd_usb_init_sample_rate(chip, iface_no, alts, fp, fp->rate_max); + usb_set_interface(chip->dev, iface_no, altno); } return 0; }

A preliminary change for the later big changes. This is a minor code refactoring to drop the unnecessary arguments that can be retrieved in a different way.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/clock.c | 29 +++++++++++++++-------------- sound/usb/clock.h | 3 +-- sound/usb/pcm.c | 2 -- sound/usb/quirks.c | 2 +- sound/usb/stream.c | 2 +- 5 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/sound/usb/clock.c b/sound/usb/clock.c index 3298a654ce96..f25da11fce3a 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -481,15 +481,18 @@ int snd_usb_clock_find_source(struct snd_usb_audio *chip, } }

-static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface, - struct usb_host_interface *alts, +static int set_sample_rate_v1(struct snd_usb_audio *chip, struct audioformat *fmt, int rate) { struct usb_device *dev = chip->dev; + struct usb_host_interface *alts; unsigned int ep; unsigned char data[3]; int err, crate;

+ alts = snd_usb_get_host_interface(chip, fmt->iface, fmt->altsetting); + if (!alts) + return -EINVAL; if (get_iface_desc(alts)->bNumEndpoints < 1) return -EINVAL; ep = get_endpoint(alts, 0)->bEndpointAddress; @@ -507,7 +510,7 @@ static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface, data, sizeof(data)); if (err < 0) { dev_err(&dev->dev, "%d:%d: cannot set freq %d to ep %#x\n", - iface, fmt->altsetting, rate, ep); + fmt->iface, fmt->altsetting, rate, ep); return err; }

@@ -525,7 +528,7 @@ static int set_sample_rate_v1(struct snd_usb_audio *chip, int iface, data, sizeof(data)); if (err < 0) { dev_err(&dev->dev, "%d:%d: cannot get freq at ep %#x\n", - iface, fmt->altsetting, ep); + fmt->iface, fmt->altsetting, ep); chip->sample_rate_read_error++; return 0; /* some devices don't support reading */ } @@ -607,8 +610,7 @@ int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip, return get_sample_rate_v2v3(chip, fmt->iface, fmt->altsetting, clock); }

-static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, - struct usb_host_interface *alts, +static int set_sample_rate_v2v3(struct snd_usb_audio *chip, struct audioformat *fmt, int rate) { struct usb_device *dev = chip->dev; @@ -632,7 +634,7 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, return clock; }

- prev_rate = get_sample_rate_v2v3(chip, iface, fmt->altsetting, clock); + prev_rate = get_sample_rate_v2v3(chip, fmt->iface, fmt->altsetting, clock); if (prev_rate == rate) goto validation;

@@ -640,7 +642,7 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, if (cur_rate < 0) { usb_audio_err(chip, "%d:%d: cannot set freq %d (v2/v3): err %d\n", - iface, fmt->altsetting, rate, cur_rate); + fmt->iface, fmt->altsetting, rate, cur_rate); return cur_rate; }

@@ -657,9 +659,9 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, /* Some devices doesn't respond to sample rate changes while the * interface is active. */ if (rate != prev_rate) { - usb_set_interface(dev, iface, 0); + usb_set_interface(dev, fmt->iface, 0); snd_usb_set_interface_quirk(chip); - usb_set_interface(dev, iface, fmt->altsetting); + usb_set_interface(dev, fmt->iface, fmt->altsetting); snd_usb_set_interface_quirk(chip); }

@@ -670,14 +672,13 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface, return 0; }

-int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface, - struct usb_host_interface *alts, +int snd_usb_init_sample_rate(struct snd_usb_audio *chip, struct audioformat *fmt, int rate) { switch (fmt->protocol) { case UAC_VERSION_1: default: - return set_sample_rate_v1(chip, iface, alts, fmt, rate); + return set_sample_rate_v1(chip, fmt, rate);

case UAC_VERSION_3: if (chip->badd_profile >= UAC3_FUNCTION_SUBCLASS_GENERIC_IO) { @@ -688,7 +689,7 @@ int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface, } fallthrough; case UAC_VERSION_2: - return set_sample_rate_v2v3(chip, iface, alts, fmt, rate); + return set_sample_rate_v2v3(chip, fmt, rate); } }

diff --git a/sound/usb/clock.h b/sound/usb/clock.h index 97597f5a3c18..8d406ed294d6 100644 --- a/sound/usb/clock.h +++ b/sound/usb/clock.h @@ -2,8 +2,7 @@ #ifndef __USBAUDIO_CLOCK_H #define __USBAUDIO_CLOCK_H

-int snd_usb_init_sample_rate(struct snd_usb_audio *chip, int iface, - struct usb_host_interface *alts, +int snd_usb_init_sample_rate(struct snd_usb_audio *chip, struct audioformat *fmt, int rate);

int snd_usb_clock_find_source(struct snd_usb_audio *chip, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 2518d4c82ad5..38b461bdca86 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1017,8 +1017,6 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface); alts = &iface->altsetting[subs->cur_audiofmt->altset_idx]; ret = snd_usb_init_sample_rate(subs->stream->chip, - subs->cur_audiofmt->iface, - alts, subs->cur_audiofmt, subs->cur_rate); if (ret < 0) diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index bb4c1ae0a4a7..018c6f241ffb 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -178,7 +178,7 @@ static int create_fixed_stream_quirk(struct snd_usb_audio *chip, fp->maxpacksize = le16_to_cpu(get_endpoint(alts, 0)->wMaxPacketSize); usb_set_interface(chip->dev, fp->iface, 0); snd_usb_init_pitch(chip, fp->iface, alts, fp); - snd_usb_init_sample_rate(chip, fp->iface, alts, fp, fp->rate_max); + snd_usb_init_sample_rate(chip, fp, fp->rate_max); return 0;

error: diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 4501e042a944..23e881985123 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -1225,7 +1225,7 @@ static int __snd_usb_parse_audio_interface(struct snd_usb_audio *chip, /* try to set the interface... */ usb_set_interface(chip->dev, iface_no, 0); snd_usb_init_pitch(chip, iface_no, alts, fp); - snd_usb_init_sample_rate(chip, iface_no, alts, fp, fp->rate_max); + snd_usb_init_sample_rate(chip, fp, fp->rate_max); usb_set_interface(chip->dev, iface_no, altno); } return 0;

start_endpoints() may leave the data endpoint running if an error happens at starting the sync endpoint. We should stop both streams properly, instead.

While we're at it, move the debug prints into the endpoint.c that is a more suitable place.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 6 ++++++ sound/usb/pcm.c | 39 +++++++++++++++++---------------------- 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index eb459db511f8..0cc7e9c01263 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -1172,6 +1172,9 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (atomic_read(&ep->chip->shutdown)) return -EBADFD;

+ usb_audio_dbg(ep->chip, "Starting %s EP 0x%x (count %d)\n", + ep_type_name(ep->type), ep->ep_num, ep->use_count); + /* already running? */ if (++ep->use_count != 1) return 0; @@ -1254,6 +1257,9 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) if (!ep) return;

+ usb_audio_dbg(ep->chip, "Stopping %s EP 0x%x (count %d)\n", + ep_type_name(ep->type), ep->ep_num, ep->use_count); + if (snd_BUG_ON(ep->use_count == 0)) return;

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 0998be109af3..c4e39aa92a84 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -211,6 +211,17 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, } }

+static void stop_endpoints(struct snd_usb_substream *subs) +{ + if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { + snd_usb_endpoint_stop(subs->sync_endpoint); + subs->sync_endpoint->sync_slave = NULL; + } + + if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) + snd_usb_endpoint_stop(subs->data_endpoint); +} + static int start_endpoints(struct snd_usb_substream *subs) { int err; @@ -221,13 +232,11 @@ static int start_endpoints(struct snd_usb_substream *subs) if (!test_and_set_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) { struct snd_usb_endpoint *ep = subs->data_endpoint;

- dev_dbg(&subs->dev->dev, "Starting data EP 0x%x\n", ep->ep_num); - ep->data_subs = subs; err = snd_usb_endpoint_start(ep); if (err < 0) { clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags); - return err; + goto error; } }

@@ -235,18 +244,20 @@ static int start_endpoints(struct snd_usb_substream *subs) !test_and_set_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { struct snd_usb_endpoint *ep = subs->sync_endpoint;

- dev_dbg(&subs->dev->dev, "Starting sync EP 0x%x\n", ep->ep_num); - ep->sync_slave = subs->data_endpoint; err = snd_usb_endpoint_start(ep); if (err < 0) { clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); ep->sync_slave = NULL; - return err; + goto error; } }

return 0; + + error: + stop_endpoints(subs); + return err; }

static void sync_pending_stops(struct snd_usb_substream *subs) @@ -255,22 +266,6 @@ static void sync_pending_stops(struct snd_usb_substream *subs) snd_usb_endpoint_sync_pending_stop(subs->data_endpoint); }

-static void stop_endpoints(struct snd_usb_substream *subs) -{ - if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { - dev_dbg(&subs->dev->dev, "Stopping sync EP 0x%x\n", - subs->sync_endpoint->ep_num); - snd_usb_endpoint_stop(subs->sync_endpoint); - subs->sync_endpoint->sync_slave = NULL; - } - - if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) { - dev_dbg(&subs->dev->dev, "Stopping data EP 0x%x\n", - subs->data_endpoint->ep_num); - snd_usb_endpoint_stop(subs->data_endpoint); - } -} - /* PCM sync_stop callback */ static int snd_usb_pcm_sync_stop(struct snd_pcm_substream *substream) {

The commit 92adc96f8eec ("ALSA: usb-audio: set the interface format after resume on Dell WD19") introduced the workaround for the broken setup after the resume specifically on a Dell dock model. However, the full setup should have been performed after the resume on all devices, as we can't guarantee the same state. So this patch removes the conditional check and applies the workaround always.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/pcm.c | 20 ++++++-------------- sound/usb/quirks-table.h | 8 -------- sound/usb/quirks.c | 11 ----------- sound/usb/usbaudio.h | 1 - 4 files changed, 6 insertions(+), 34 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 32237623de96..ac6385a4eb70 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1835,6 +1835,9 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea subs); subs->running = 1; return 0; + case SNDRV_PCM_TRIGGER_SUSPEND: + subs->need_setup_fmt = true; + fallthrough; case SNDRV_PCM_TRIGGER_STOP: stop_endpoints(subs); snd_usb_endpoint_set_callback(subs->data_endpoint, @@ -1849,13 +1852,6 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea subs); subs->running = 0; return 0; - case SNDRV_PCM_TRIGGER_SUSPEND: - if (subs->stream->chip->setup_fmt_after_resume_quirk) { - stop_endpoints(subs); - subs->need_setup_fmt = true; - return 0; - } - break; }

return -EINVAL; @@ -1879,6 +1875,9 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream subs); subs->running = 1; return 0; + case SNDRV_PCM_TRIGGER_SUSPEND: + subs->need_setup_fmt = true; + fallthrough; case SNDRV_PCM_TRIGGER_STOP: stop_endpoints(subs); fallthrough; @@ -1887,13 +1886,6 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream NULL, NULL, NULL); subs->running = 0; return 0; - case SNDRV_PCM_TRIGGER_SUSPEND: - if (subs->stream->chip->setup_fmt_after_resume_quirk) { - stop_endpoints(subs); - subs->need_setup_fmt = true; - return 0; - } - break; }

return -EINVAL; diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index 3c1697f6b60c..85b99c6d3c61 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3256,14 +3256,6 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), } } }, -/* Dell WD19 Dock */ -{ - USB_DEVICE(0x0bda, 0x402e), - .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) { - .ifnum = QUIRK_ANY_INTERFACE, - .type = QUIRK_SETUP_FMT_AFTER_RESUME - } -}, /* MOTU Microbook II */ { USB_DEVICE_VENDOR_SPEC(0x07fd, 0x0004), diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 013ab93fb640..7e7f258011ff 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -508,16 +508,6 @@ static int create_standard_mixer_quirk(struct snd_usb_audio *chip, return snd_usb_create_mixer(chip, quirk->ifnum, 0); }

- -static int setup_fmt_after_resume_quirk(struct snd_usb_audio *chip, - struct usb_interface *iface, - struct usb_driver *driver, - const struct snd_usb_audio_quirk *quirk) -{ - chip->setup_fmt_after_resume_quirk = 1; - return 1; /* Continue with creating streams and mixer */ -} - static int setup_disable_autosuspend(struct snd_usb_audio *chip, struct usb_interface *iface, struct usb_driver *driver, @@ -565,7 +555,6 @@ int snd_usb_create_quirk(struct snd_usb_audio *chip, [QUIRK_AUDIO_EDIROL_UAXX] = create_uaxx_quirk, [QUIRK_AUDIO_ALIGN_TRANSFER] = create_align_transfer_quirk, [QUIRK_AUDIO_STANDARD_MIXER] = create_standard_mixer_quirk, - [QUIRK_SETUP_FMT_AFTER_RESUME] = setup_fmt_after_resume_quirk, [QUIRK_SETUP_DISABLE_AUTOSUSPEND] = setup_disable_autosuspend, };

diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index aa017a93f7bd..cc338e8e2597 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -35,7 +35,6 @@ struct snd_usb_audio { wait_queue_head_t shutdown_wait; unsigned int txfr_quirk:1; /* Subframe boundaries on transfers */ unsigned int tx_length_quirk:1; /* Put length specifier in transfers */ - unsigned int setup_fmt_after_resume_quirk:1; /* setup the format to interface after resume */ unsigned int need_delayed_register:1; /* warn for delayed registration */ int num_interfaces; int num_suspended_intf;

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 Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.c | 8 +- sound/usb/card.h | 11 +- sound/usb/clock.c | 13 +- sound/usb/endpoint.c | 662 ++++++++++++++++++++++++++------------------------- sound/usb/endpoint.h | 40 ++-- sound/usb/pcm.c | 616 +++++++++++++++++++---------------------------- 6 files changed, 616 insertions(+), 734 deletions(-)

diff --git a/sound/usb/card.c b/sound/usb/card.c index 096dd8e3c64b..58958afcec93 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -980,6 +980,7 @@ 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;

@@ -987,11 +988,10 @@ static int usb_audio_suspend(struct usb_interface *intf, pm_message_t message) 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) diff --git a/sound/usb/card.h b/sound/usb/card.h index 1f61be98a31d..66a249ae138f 100644 --- a/sound/usb/card.h +++ b/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) */ diff --git a/sound/usb/clock.c b/sound/usb/clock.c index f25da11fce3a..b869a711afbf 100644 --- a/sound/usb/clock.c +++ b/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: 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. */ @@ -646,219 +828,36 @@ static void release_urbs(struct snd_usb_endpoint *ep, int force) ep->nurbs = 0; }

-/* - * 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 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, */ 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 @@ static int sync_ep_set_params(struct snd_usb_endpoint *ep) 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,39 +1319,15 @@ 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); } }

-/** - * 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 * @@ -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; diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index e2fddb3dcf7a..2bfa6d3e029c 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -10,16 +10,25 @@ struct snd_usb_endpoint *snd_usb_get_endpoint(struct snd_usb_audio *chip,

int snd_usb_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type);

-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); - +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); +void snd_usb_endpoint_close(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep); +int snd_usb_endpoint_configure(struct snd_usb_audio *chip, + struct snd_usb_endpoint *ep); +void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep); + +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); + +void snd_usb_endpoint_set_sync(struct snd_usb_audio *chip, + struct snd_usb_endpoint *data_ep, + struct snd_usb_endpoint *sync_ep); void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, void (*prepare)(struct snd_usb_substream *subs, struct urb *urb), @@ -27,23 +36,16 @@ void snd_usb_endpoint_set_callback(struct snd_usb_endpoint *ep, struct urb *urb), struct snd_usb_substream *data_subs);

-int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep); void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); +void snd_usb_endpoint_suspend(struct snd_usb_endpoint *ep); int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); -void snd_usb_endpoint_deactivate(struct snd_usb_endpoint *ep); void snd_usb_endpoint_release(struct snd_usb_endpoint *ep); void snd_usb_endpoint_free(struct snd_usb_endpoint *ep);

int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep); int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep); -void snd_usb_endpoint_set_syncinterval(struct snd_usb_audio *chip, - struct snd_usb_endpoint *ep, - struct usb_host_interface *alts); - -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/pcm.c b/sound/usb/pcm.c index 45a692512d27..e80e8cf1e863 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -80,21 +80,22 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream /* * find a matching audio format */ -static struct audioformat *find_format(struct list_head *fmt_list_head, - snd_pcm_format_t format, - unsigned int rate, - unsigned int channels, - struct snd_usb_substream *subs) +static struct audioformat * +find_format(struct list_head *fmt_list_head, snd_pcm_format_t format, + unsigned int rate, unsigned int channels, bool strict_match, + struct snd_usb_substream *subs) { struct audioformat *fp; struct audioformat *found = NULL; int cur_attr = 0, attr;

list_for_each_entry(fp, fmt_list_head, list) { - if (!(fp->formats & pcm_format_to_bits(format))) - continue; - if (fp->channels != channels) - continue; + if (strict_match) { + if (!(fp->formats & pcm_format_to_bits(format))) + continue; + if (fp->channels != channels) + continue; + } if (rate < fp->rate_min || rate > fp->rate_max) continue; if (!(fp->rates & SNDRV_PCM_RATE_CONTINUOUS)) { @@ -140,38 +141,30 @@ static struct audioformat *find_format(struct list_head *fmt_list_head, return found; }

-static struct audioformat *find_substream_format(struct snd_usb_substream *subs) +static struct audioformat * +find_substream_format(struct snd_usb_substream *subs, + const struct snd_pcm_hw_params *params) { - return find_format(&subs->fmt_list, subs->pcm_format, subs->cur_rate, - subs->channels, subs); + return find_format(&subs->fmt_list, params_format(params), + params_rate(params), params_channels(params), + true, subs); }

-static int init_pitch_v1(struct snd_usb_audio *chip, - struct audioformat *fmt) +static int init_pitch_v1(struct snd_usb_audio *chip, int ep) { struct usb_device *dev = chip->dev; - unsigned int ep; unsigned char data[1]; int err;

- ep = fmt->endpoint; - data[0] = 1; err = snd_usb_ctl_msg(dev, usb_sndctrlpipe(dev, 0), UAC_SET_CUR, USB_TYPE_CLASS|USB_RECIP_ENDPOINT|USB_DIR_OUT, UAC_EP_CS_ATTR_PITCH_CONTROL << 8, ep, data, sizeof(data)); - if (err < 0) { - usb_audio_err(chip, "%d:%d: cannot set enable PITCH\n", - fmt->iface, ep); - return err; - } - - return 0; + return err; }

-static int init_pitch_v2(struct snd_usb_audio *chip, - struct audioformat *fmt) +static int init_pitch_v2(struct snd_usb_audio *chip, int ep) { struct usb_device *dev = chip->dev; unsigned char data[1]; @@ -182,13 +175,7 @@ static int init_pitch_v2(struct snd_usb_audio *chip, USB_TYPE_CLASS | USB_RECIP_ENDPOINT | USB_DIR_OUT, UAC2_EP_CS_PITCH << 8, 0, data, sizeof(data)); - if (err < 0) { - usb_audio_err(chip, "%d:%d: cannot set enable PITCH (v2)\n", - fmt->iface, fmt->altsetting); - return err; - } - - return 0; + return err; }

/* @@ -197,29 +184,47 @@ static int init_pitch_v2(struct snd_usb_audio *chip, int snd_usb_init_pitch(struct snd_usb_audio *chip, struct audioformat *fmt) { + int err; + /* if endpoint doesn't have pitch control, bail out */ if (!(fmt->attributes & UAC_EP_CS_ATTR_PITCH_CONTROL)) return 0;

+ usb_audio_dbg(chip, "enable PITCH for EP 0x%x\n", fmt->endpoint); + switch (fmt->protocol) { case UAC_VERSION_1: + err = init_pitch_v1(chip, fmt->endpoint); + break; + case UAC_VERSION_2: + err = init_pitch_v2(chip, fmt->endpoint); + break; default: - return init_pitch_v1(chip, fmt); + return 0; + }

- case UAC_VERSION_2: - return init_pitch_v2(chip, fmt); + if (err < 0) { + usb_audio_err(chip, "failed to enable PITCH for EP 0x%x\n", + fmt->endpoint); + return err; } + + return 0; }

-static void stop_endpoints(struct snd_usb_substream *subs) +static bool stop_endpoints(struct snd_usb_substream *subs) { + bool stopped = 0; + if (test_and_clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { snd_usb_endpoint_stop(subs->sync_endpoint); - subs->sync_endpoint->sync_slave = NULL; + stopped = true; } - - if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) + if (test_and_clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) { snd_usb_endpoint_stop(subs->data_endpoint); + stopped = true; + } + return stopped; }

static int start_endpoints(struct snd_usb_substream *subs) @@ -230,9 +235,7 @@ static int start_endpoints(struct snd_usb_substream *subs) return -EINVAL;

if (!test_and_set_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags)) { - struct snd_usb_endpoint *ep = subs->data_endpoint; - - err = snd_usb_endpoint_start(ep); + err = snd_usb_endpoint_start(subs->data_endpoint); if (err < 0) { clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags); goto error; @@ -241,13 +244,9 @@ static int start_endpoints(struct snd_usb_substream *subs)

if (subs->sync_endpoint && !test_and_set_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags)) { - struct snd_usb_endpoint *ep = subs->sync_endpoint; - - ep->sync_slave = subs->data_endpoint; - err = snd_usb_endpoint_start(ep); + err = snd_usb_endpoint_start(subs->sync_endpoint); if (err < 0) { clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); - ep->sync_slave = NULL; goto error; } } @@ -446,6 +445,7 @@ static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, fmt->sync_ep = ep; fmt->sync_iface = ifnum; fmt->sync_altsetting = alts->desc.bAlternateSetting; + fmt->sync_ep_idx = 0; fmt->implicit_fb = 1; dev_dbg(&dev->dev, "%d:%d: found implicit_fb sync_ep=%x, iface=%d, alt=%d\n", fmt->iface, fmt->altsetting, fmt->sync_ep, fmt->sync_iface, @@ -527,6 +527,7 @@ int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, fmt->sync_ep = ep; fmt->sync_iface = altsd->bInterfaceNumber; fmt->sync_altsetting = altsd->bAlternateSetting; + fmt->sync_ep_idx = 1; if ((sync_attr & USB_ENDPOINT_USAGE_MASK) == USB_ENDPOINT_USAGE_IMPLICIT_FB) fmt->implicit_fb = 1;

@@ -537,152 +538,6 @@ int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, return 0; }

-static int set_sync_endpoint(struct snd_usb_substream *subs, - struct audioformat *fmt) -{ - struct usb_device *dev = subs->dev; - struct usb_host_interface *alts; - struct snd_usb_audio *chip = subs->stream->chip; - int is_playback = subs->direction == SNDRV_PCM_STREAM_PLAYBACK; - unsigned int ep; - int err; - - subs->sync_endpoint = NULL; - subs->data_endpoint->sync_master = NULL; - - ep = fmt->sync_ep; - if (!ep) - return 0; - - alts = snd_usb_get_host_interface(subs->stream->chip, fmt->sync_iface, - fmt->altsetting); - if (!alts) - return 0; - - subs->sync_endpoint = snd_usb_get_endpoint(chip, ep); - if (!subs->sync_endpoint) { - if (is_playback && - (fmt->ep_attr & USB_ENDPOINT_SYNCTYPE) == USB_ENDPOINT_SYNC_NONE) - return 0; - return -EINVAL; - } - - subs->sync_endpoint->iface = fmt->sync_iface; - subs->sync_endpoint->altsetting = fmt->sync_altsetting; - subs->sync_endpoint->is_implicit_feedback = fmt->implicit_fb; - - subs->data_endpoint->sync_master = subs->sync_endpoint; - - snd_usb_endpoint_set_syncinterval(subs->stream->chip, subs->sync_endpoint, alts); - - if (!subs->sync_endpoint->use_count && - (subs->data_endpoint->iface != subs->sync_endpoint->iface || - subs->data_endpoint->altsetting != subs->sync_endpoint->altsetting)) { - err = usb_set_interface(subs->dev, - subs->sync_endpoint->iface, - subs->sync_endpoint->altsetting); - if (err < 0) - return err; - dev_dbg(&dev->dev, "setting usb interface %d:%d\n", - subs->sync_endpoint->iface, - subs->sync_endpoint->altsetting); - snd_usb_set_interface_quirk(chip); - } - - return 0; -} - -/* - * find a matching format and set up the interface - */ -static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) -{ - struct usb_device *dev = subs->dev; - struct snd_usb_audio *chip = subs->stream->chip; - struct usb_host_interface *alts; - struct usb_interface *iface; - struct snd_usb_endpoint *ep; - int err; - - iface = usb_ifnum_to_if(dev, fmt->iface); - if (WARN_ON(!iface)) - return -EINVAL; - alts = usb_altnum_to_altsetting(iface, fmt->altsetting); - if (WARN_ON(!alts)) - return -EINVAL; - - if (fmt == subs->cur_audiofmt && !subs->need_setup_fmt) - return 0; - - /* shared EP with implicit fb */ - if (fmt->implicit_fb && !subs->need_setup_fmt) { - ep = snd_usb_get_endpoint(chip, fmt->endpoint); - if (ep && ep->use_count > 0) - goto add_data_ep; - } - - /* close the old interface */ - if (subs->interface >= 0 && (subs->interface != fmt->iface || subs->need_setup_fmt)) { - err = usb_set_interface(subs->dev, subs->interface, 0); - if (err < 0) { - dev_err(&dev->dev, - "%d:%d: return to setting 0 failed (%d)\n", - fmt->iface, fmt->altsetting, err); - return -EIO; - } - subs->interface = -1; - subs->altset_idx = 0; - } - - if (subs->need_setup_fmt) - subs->need_setup_fmt = false; - - /* set interface */ - if (iface->cur_altsetting != alts) { - err = snd_usb_select_mode_quirk(chip, fmt); - if (err < 0) - return -EIO; - - err = usb_set_interface(dev, fmt->iface, fmt->altsetting); - if (err < 0) { - dev_err(&dev->dev, - "%d:%d: usb_set_interface failed (%d)\n", - fmt->iface, fmt->altsetting, err); - return -EIO; - } - dev_dbg(&dev->dev, "setting usb interface %d:%d\n", - fmt->iface, fmt->altsetting); - snd_usb_set_interface_quirk(chip); - } - - subs->need_setup_ep = true; - - add_data_ep: - subs->interface = fmt->iface; - subs->altset_idx = fmt->altset_idx; - subs->data_endpoint = snd_usb_get_endpoint(chip, fmt->endpoint); - if (!subs->data_endpoint) - return -EINVAL; - subs->data_endpoint->iface = fmt->iface; - subs->data_endpoint->altsetting = fmt->altsetting; - - err = set_sync_endpoint(subs, fmt); - if (err < 0) - return err; - - if (subs->need_setup_ep) { - err = snd_usb_init_pitch(chip, fmt); - if (err < 0) - return err; - } - - subs->cur_audiofmt = fmt; - - snd_usb_set_format_quirk(subs, fmt); - - return 0; -} - /* * Return the score of matching two audioformats. * Veto the audioformat if: @@ -691,8 +546,8 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) * - Requested sample rate is not supported. */ static int match_endpoint_audioformats(struct snd_usb_substream *subs, - struct audioformat *fp, - struct audioformat *match, int rate, + const struct audioformat *fp, + int rate, int channels, snd_pcm_format_t pcm_format) { int i, score; @@ -716,108 +571,12 @@ static int match_endpoint_audioformats(struct snd_usb_substream *subs, }

score = 1; - if (fp->channels == match->channels) + if (fp->channels == channels) score++;

return score; }

-/* - * Configure the sync ep using the rate and pcm format of the data ep. - */ -static int configure_sync_endpoint(struct snd_usb_substream *subs) -{ - struct audioformat *fp; - struct audioformat *sync_fp = NULL; - int cur_score = 0; - int sync_period_bytes = subs->period_bytes; - struct snd_usb_substream *sync_subs = - &subs->stream->substream[subs->direction ^ 1]; - - if (subs->fixed_hw || - !subs->sync_endpoint->is_implicit_feedback) { - sync_fp = subs->cur_audiofmt; - goto configure; - } - - sync_fp = find_format(&sync_subs->fmt_list, subs->pcm_format, - subs->cur_rate, subs->channels, NULL); - if (sync_fp) - goto configure; - - /* Try to find the best matching audioformat. */ - list_for_each_entry(fp, &sync_subs->fmt_list, list) { - int score = match_endpoint_audioformats(subs, - fp, subs->cur_audiofmt, - subs->cur_rate, subs->pcm_format); - - if (score > cur_score) { - sync_fp = fp; - cur_score = score; - } - } - - if (unlikely(sync_fp == NULL)) { - dev_err(&subs->dev->dev, - "%s: no valid audioformat for sync ep %x found\n", - __func__, sync_subs->ep_num); - return -EINVAL; - } - - /* - * Recalculate the period bytes if channel number differ between - * data and sync ep audioformat. - */ - if (sync_fp->channels != subs->channels) { - sync_period_bytes = (subs->period_bytes / subs->channels) * - sync_fp->channels; - dev_dbg(&subs->dev->dev, - "%s: adjusted sync ep period bytes (%d -> %d)\n", - __func__, subs->period_bytes, sync_period_bytes); - } - - configure: - return snd_usb_endpoint_set_params(subs->sync_endpoint, - subs->pcm_format, - sync_fp->channels, - sync_period_bytes, - subs->period_frames, - subs->buffer_periods, - subs->cur_rate, - sync_fp, - NULL); -} - -/* - * configure endpoint params - * - * called during initial setup and upon resume - */ -static int configure_endpoint(struct snd_usb_substream *subs) -{ - int ret; - - /* format changed */ - stop_endpoints(subs); - sync_pending_stops(subs); - ret = snd_usb_endpoint_set_params(subs->data_endpoint, - subs->pcm_format, - subs->channels, - subs->period_bytes, - subs->period_frames, - subs->buffer_periods, - subs->cur_rate, - subs->cur_audiofmt, - subs->sync_endpoint); - if (ret < 0) - return ret; - - if (subs->sync_endpoint) - ret = configure_sync_endpoint(subs); - - return ret; -} - static int snd_usb_pcm_change_state(struct snd_usb_substream *subs, int state) { int ret; @@ -866,6 +625,92 @@ int snd_usb_pcm_resume(struct snd_usb_stream *as) return 0; }

+static struct snd_usb_substream * +find_matching_substream(struct snd_usb_audio *chip, int stream, int ep_num, + int fmt_type) +{ + struct snd_usb_stream *as; + struct snd_usb_substream *subs; + + list_for_each_entry(as, &chip->pcm_list, list) { + subs = &as->substream[stream]; + if (as->fmt_type == fmt_type && subs->ep_num == ep_num) + return subs; + } + + return NULL; +} + +static struct audioformat * +find_implicit_fb_sync_format(struct snd_usb_audio *chip, + const struct audioformat *target, + const struct snd_pcm_hw_params *params, + int stream) +{ + struct snd_usb_substream *subs; + struct audioformat *fp, *sync_fmt; + int score, high_score; + + subs = find_matching_substream(chip, stream, target->sync_ep, + target->fmt_type); + if (!subs) + return NULL; + + sync_fmt = NULL; + high_score = 0; + list_for_each_entry(fp, &subs->fmt_list, list) { + score = match_endpoint_audioformats(subs, fp, + params_rate(params), + params_channels(params), + params_format(params)); + if (score > high_score) { + sync_fmt = fp; + high_score = score; + } + } + + return sync_fmt; +} + +static void close_endpoints(struct snd_usb_audio *chip, + struct snd_usb_substream *subs) +{ + if (subs->data_endpoint) { + snd_usb_endpoint_set_sync(chip, subs->data_endpoint, NULL); + snd_usb_endpoint_close(chip, subs->data_endpoint); + subs->data_endpoint = NULL; + } + + if (subs->sync_endpoint) { + snd_usb_endpoint_close(chip, subs->sync_endpoint); + subs->sync_endpoint = NULL; + } +} + +static int configure_endpoints(struct snd_usb_audio *chip, + struct snd_usb_substream *subs) +{ + int err; + + if (subs->data_endpoint->need_setup) { + /* stop any running stream beforehand */ + if (stop_endpoints(subs)) + sync_pending_stops(subs); + err = snd_usb_endpoint_configure(chip, subs->data_endpoint); + if (err < 0) + return err; + snd_usb_set_format_quirk(subs, subs->cur_audiofmt); + } + + if (subs->sync_endpoint) { + err = snd_usb_endpoint_configure(chip, subs->sync_endpoint); + if (err < 0) + return err; + } + + return 0; +} + /* * hw_params callback * @@ -880,30 +725,45 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *hw_params) { struct snd_usb_substream *subs = substream->runtime->private_data; + struct snd_usb_audio *chip = subs->stream->chip; struct audioformat *fmt; + struct audioformat *sync_fmt; int ret;

ret = snd_media_start_pipeline(subs); if (ret) return ret;

- subs->pcm_format = params_format(hw_params); - subs->period_bytes = params_period_bytes(hw_params); - subs->period_frames = params_period_size(hw_params); - subs->buffer_periods = params_periods(hw_params); - subs->channels = params_channels(hw_params); - subs->cur_rate = params_rate(hw_params); - - fmt = find_substream_format(subs); + fmt = find_substream_format(subs, hw_params); if (!fmt) { - dev_dbg(&subs->dev->dev, - "cannot set format: format = %#x, rate = %d, channels = %d\n", - subs->pcm_format, subs->cur_rate, subs->channels); + usb_audio_dbg(chip, + "cannot find format: format=%s, rate=%d, channels=%d\n", + snd_pcm_format_name(params_format(hw_params)), + params_rate(hw_params), params_channels(hw_params)); ret = -EINVAL; goto stop_pipeline; }

- ret = snd_usb_lock_shutdown(subs->stream->chip); + if (fmt->implicit_fb && + (fmt->iface != fmt->sync_iface || + fmt->altsetting != fmt->sync_altsetting)) { + sync_fmt = find_implicit_fb_sync_format(chip, fmt, hw_params, + !substream->stream); + if (!sync_fmt) { + usb_audio_dbg(chip, + "cannot find sync format: ep=0x%x, iface=%d:%d, format=%s, rate=%d, channels=%d\n", + fmt->sync_ep, fmt->sync_iface, + fmt->sync_altsetting, + snd_pcm_format_name(params_format(hw_params)), + params_rate(hw_params), params_channels(hw_params)); + ret = -EINVAL; + goto stop_pipeline; + } + } else { + sync_fmt = fmt; + } + + ret = snd_usb_lock_shutdown(chip); if (ret < 0) goto stop_pipeline;

@@ -911,18 +771,56 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, if (ret < 0) goto unlock;

- ret = set_format(subs, fmt); + if (subs->data_endpoint) { + if (snd_usb_endpoint_compatible(chip, subs->data_endpoint, + fmt, hw_params)) + goto unlock; + close_endpoints(chip, subs); + } + + subs->data_endpoint = snd_usb_endpoint_open(chip, fmt, hw_params, false); + if (!subs->data_endpoint) { + ret = -EINVAL; + goto unlock; + } + + if (fmt->sync_ep) { + subs->sync_endpoint = snd_usb_endpoint_open(chip, sync_fmt, + hw_params, + fmt == sync_fmt); + if (!subs->sync_endpoint) { + ret = -EINVAL; + goto unlock; + } + + snd_usb_endpoint_set_sync(chip, subs->data_endpoint, + subs->sync_endpoint); + } + + subs->interface = fmt->iface; + subs->altset_idx = fmt->altset_idx; + subs->cur_audiofmt = fmt; + + ret = configure_endpoints(chip, subs); if (ret < 0) goto unlock;

+ subs->pcm_format = params_format(hw_params); + subs->period_bytes = params_period_bytes(hw_params); + subs->period_frames = params_period_size(hw_params); + subs->buffer_periods = params_periods(hw_params); + subs->channels = params_channels(hw_params); + subs->cur_rate = params_rate(hw_params); + unlock: - snd_usb_unlock_shutdown(subs->stream->chip); if (ret < 0) - goto stop_pipeline; - return ret; + close_endpoints(chip, subs);

+ snd_usb_unlock_shutdown(chip); stop_pipeline: - snd_media_stop_pipeline(subs); + if (ret < 0) + snd_media_stop_pipeline(subs); + return ret; }

@@ -941,15 +839,9 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream) subs->cur_rate = 0; subs->period_bytes = 0; if (!snd_usb_lock_shutdown(chip)) { - stop_endpoints(subs); - sync_pending_stops(subs); - snd_usb_endpoint_deactivate(subs->sync_endpoint); - snd_usb_endpoint_deactivate(subs->data_endpoint); - if (subs->data_endpoint) { - subs->data_endpoint->sync_master = NULL; - subs->data_endpoint = NULL; - } - subs->sync_endpoint = NULL; + if (stop_endpoints(subs)) + sync_pending_stops(subs); + close_endpoints(chip, subs); snd_usb_unlock_shutdown(chip); }

@@ -965,16 +857,10 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; struct snd_usb_substream *subs = runtime->private_data; - struct usb_host_interface *alts; - struct usb_interface *iface; + struct snd_usb_audio *chip = subs->stream->chip; int ret;

- if (! subs->cur_audiofmt) { - dev_err(&subs->dev->dev, "no format is specified!\n"); - return -ENXIO; - } - - ret = snd_usb_lock_shutdown(subs->stream->chip); + ret = snd_usb_lock_shutdown(chip); if (ret < 0) return ret; if (snd_BUG_ON(!subs->data_endpoint)) { @@ -982,36 +868,10 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) goto unlock; }

- ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D0); - if (ret < 0) - goto unlock; - - ret = set_format(subs, subs->cur_audiofmt); + ret = configure_endpoints(chip, subs); if (ret < 0) goto unlock;

- if (subs->need_setup_ep) { - - iface = usb_ifnum_to_if(subs->dev, subs->cur_audiofmt->iface); - alts = &iface->altsetting[subs->cur_audiofmt->altset_idx]; - ret = snd_usb_init_sample_rate(subs->stream->chip, - subs->cur_audiofmt, - subs->cur_rate); - if (ret < 0) - goto unlock; - - ret = configure_endpoint(subs); - if (ret < 0) - goto unlock; - subs->need_setup_ep = false; - } - - /* some unit conversions in runtime */ - 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; @@ -1025,7 +885,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) ret = start_endpoints(subs);

unlock: - snd_usb_unlock_shutdown(subs->stream->chip); + snd_usb_unlock_shutdown(chip); return ret; }

@@ -1047,6 +907,8 @@ static const struct snd_pcm_hardware snd_usb_hardware = SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BLOCK_TRANSFER | SNDRV_PCM_INFO_PAUSE, + .channels_min = 1, + .channels_max = 256, .buffer_bytes_max = 1024 * 1024, .period_bytes_min = 64, .period_bytes_max = 512 * 1024, @@ -1250,7 +1112,6 @@ static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime, struct audioformat *fp; int err;

- subs->fixed_hw = 0; list_for_each_entry(fp, &subs->fmt_list, list) { ep = snd_usb_get_endpoint(chip, fp->endpoint); if (ep && ep->cur_rate) @@ -1266,7 +1127,7 @@ static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime,

found: if (!find_format(&subs->fmt_list, ep->cur_format, ep->cur_rate, - ep->cur_channels, NULL)) { + ep->cur_channels, false, NULL)) { usb_audio_dbg(chip, "EP 0x%x being used, but not applicable\n", ep->ep_num); return 0; @@ -1274,19 +1135,23 @@ static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime,

usb_audio_dbg(chip, "EP 0x%x being used, using fixed params:\n", ep->ep_num); - usb_audio_dbg(chip, "rate=%d, format=%s, channels=%d, period_size=%d, periods=%d\n", - ep->cur_rate, snd_pcm_format_name(ep->cur_format), - ep->cur_channels, ep->cur_period_frames, + usb_audio_dbg(chip, "rate=%d, period_size=%d, periods=%d\n", + ep->cur_rate, ep->cur_period_frames, ep->cur_buffer_periods);

- runtime->hw.formats = pcm_format_to_bits(ep->cur_format); + runtime->hw.formats = subs->formats; runtime->hw.rate_min = runtime->hw.rate_max = ep->cur_rate; - runtime->hw.channels_min = runtime->hw.channels_max = - ep->cur_channels; runtime->hw.rates = SNDRV_PCM_RATE_KNOT; runtime->hw.periods_min = runtime->hw.periods_max = ep->cur_buffer_periods; - subs->fixed_hw = 1; + + err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS, + hw_rule_channels, subs, + SNDRV_PCM_HW_PARAM_FORMAT, + SNDRV_PCM_HW_PARAM_RATE, + -1); + if (err < 0) + return err;

err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE, @@ -1442,9 +1307,7 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)

snd_media_stop_pipeline(subs);

- if (subs->interface >= 0 && - !snd_usb_lock_shutdown(subs->stream->chip)) { - usb_set_interface(subs->dev, subs->interface, 0); + if (!snd_usb_lock_shutdown(subs->stream->chip)) { subs->interface = -1; ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D1); snd_usb_unlock_shutdown(subs->stream->chip); @@ -1823,15 +1686,19 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea retire_playback_urb, subs); subs->running = 1; + dev_dbg(&subs->dev->dev, "%d:%d Start Playback PCM\n", + subs->cur_audiofmt->iface, + subs->cur_audiofmt->altsetting); return 0; case SNDRV_PCM_TRIGGER_SUSPEND: - subs->need_setup_fmt = true; - fallthrough; case SNDRV_PCM_TRIGGER_STOP: stop_endpoints(subs); snd_usb_endpoint_set_callback(subs->data_endpoint, NULL, NULL, NULL); subs->running = 0; + dev_dbg(&subs->dev->dev, "%d:%d Stop Playback PCM\n", + subs->cur_audiofmt->iface, + subs->cur_audiofmt->altsetting); return 0; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: /* keep retire_data_urb for delay calculation */ @@ -1840,6 +1707,9 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea retire_playback_urb, subs); subs->running = 0; + dev_dbg(&subs->dev->dev, "%d:%d Pause Playback PCM\n", + subs->cur_audiofmt->iface, + subs->cur_audiofmt->altsetting); return 0; }

@@ -1863,10 +1733,11 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream NULL, retire_capture_urb, subs); subs->running = 1; + dev_dbg(&subs->dev->dev, "%d:%d Start Capture PCM\n", + subs->cur_audiofmt->iface, + subs->cur_audiofmt->altsetting); return 0; case SNDRV_PCM_TRIGGER_SUSPEND: - subs->need_setup_fmt = true; - fallthrough; case SNDRV_PCM_TRIGGER_STOP: stop_endpoints(subs); fallthrough; @@ -1874,6 +1745,9 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream snd_usb_endpoint_set_callback(subs->data_endpoint, NULL, NULL, NULL); subs->running = 0; + dev_dbg(&subs->dev->dev, "%d:%d Stop Capture PCM\n", + subs->cur_audiofmt->iface, + subs->cur_audiofmt->altsetting); return 0; }

The audioformat is referred in many places but most of usages are read-only. Let's add const prefix in the possible places.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.h | 4 ++-- sound/usb/clock.c | 16 ++++++++-------- sound/usb/clock.h | 4 ++-- sound/usb/endpoint.c | 2 +- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 32 ++++++++++++++++---------------- sound/usb/pcm.h | 2 +- sound/usb/quirks.c | 9 +++++---- sound/usb/quirks.h | 6 +++--- 9 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h index cde492e9581a..53f0ce61f858 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -120,7 +120,7 @@ struct snd_usb_endpoint { bool need_setup; /* (re-)need for configure? */

/* for hw constraints */ - struct audioformat *cur_audiofmt; + const struct audioformat *cur_audiofmt; unsigned int cur_rate; snd_pcm_format_t cur_format; unsigned int cur_channels; @@ -142,7 +142,7 @@ struct snd_usb_substream { int direction; /* playback or capture */ int interface; /* current interface */ int endpoint; /* assigned endpoint */ - struct audioformat *cur_audiofmt; /* current audioformat pointer (for hw_params callback) */ + const struct audioformat *cur_audiofmt; /* current audioformat pointer (for hw_params callback) */ struct snd_usb_power_domain *str_pd; /* UAC3 Power Domain for streaming path */ snd_pcm_format_t pcm_format; /* current audio format (for hw_params callback) */ unsigned int channels; /* current number of channels (for hw_params callback) */ diff --git a/sound/usb/clock.c b/sound/usb/clock.c index b869a711afbf..e940dcee792b 100644 --- a/sound/usb/clock.c +++ b/sound/usb/clock.c @@ -152,7 +152,7 @@ static int uac_clock_selector_set_val(struct snd_usb_audio *chip, int selector_i }

static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip, - struct audioformat *fmt, + const struct audioformat *fmt, int source_id) { bool ret = false; @@ -215,7 +215,7 @@ static bool uac_clock_source_is_valid_quirk(struct snd_usb_audio *chip, }

static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, - struct audioformat *fmt, + const struct audioformat *fmt, int source_id) { int err; @@ -264,7 +264,7 @@ static bool uac_clock_source_is_valid(struct snd_usb_audio *chip, }

static int __uac_clock_find_source(struct snd_usb_audio *chip, - struct audioformat *fmt, int entity_id, + const struct audioformat *fmt, int entity_id, unsigned long *visited, bool validate) { struct uac_clock_source_descriptor *source; @@ -358,7 +358,7 @@ static int __uac_clock_find_source(struct snd_usb_audio *chip, }

static int __uac3_clock_find_source(struct snd_usb_audio *chip, - struct audioformat *fmt, int entity_id, + const struct audioformat *fmt, int entity_id, unsigned long *visited, bool validate) { struct uac3_clock_source_descriptor *source; @@ -464,7 +464,7 @@ static int __uac3_clock_find_source(struct snd_usb_audio *chip, * Returns the clock source UnitID (>=0) on success, or an error. */ int snd_usb_clock_find_source(struct snd_usb_audio *chip, - struct audioformat *fmt, bool validate) + const struct audioformat *fmt, bool validate) { DECLARE_BITMAP(visited, 256); memset(visited, 0, sizeof(visited)); @@ -482,7 +482,7 @@ int snd_usb_clock_find_source(struct snd_usb_audio *chip, }

static int set_sample_rate_v1(struct snd_usb_audio *chip, - struct audioformat *fmt, int rate) + const struct audioformat *fmt, int rate) { struct usb_device *dev = chip->dev; struct usb_host_interface *alts; @@ -611,7 +611,7 @@ 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) + const struct audioformat *fmt, int rate) { int cur_rate, prev_rate; int clock; @@ -663,7 +663,7 @@ 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) + const 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); diff --git a/sound/usb/clock.h b/sound/usb/clock.h index 8d406ed294d6..ed9fc2dc0510 100644 --- a/sound/usb/clock.h +++ b/sound/usb/clock.h @@ -3,10 +3,10 @@ #define __USBAUDIO_CLOCK_H

int snd_usb_init_sample_rate(struct snd_usb_audio *chip, - struct audioformat *fmt, int rate); + const struct audioformat *fmt, int rate);

int snd_usb_clock_find_source(struct snd_usb_audio *chip, - struct audioformat *fmt, bool validate); + const struct audioformat *fmt, bool validate);

int snd_usb_set_sample_rate_v2v3(struct snd_usb_audio *chip, const struct audioformat *fmt, diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index b8f06a75fc2a..49fb934ee432 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -623,7 +623,7 @@ bool snd_usb_endpoint_compatible(struct snd_usb_audio *chip, */ struct snd_usb_endpoint * snd_usb_endpoint_open(struct snd_usb_audio *chip, - struct audioformat *fp, + const struct audioformat *fp, const struct snd_pcm_hw_params *params, bool is_sync_ep) { diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 2bfa6d3e029c..201011d89659 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -12,7 +12,7 @@ int snd_usb_add_endpoint(struct snd_usb_audio *chip, int ep_num, int type);

struct snd_usb_endpoint * snd_usb_endpoint_open(struct snd_usb_audio *chip, - struct audioformat *fp, + const struct audioformat *fp, const struct snd_pcm_hw_params *params, bool is_sync_ep); void snd_usb_endpoint_close(struct snd_usb_audio *chip, diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index e80e8cf1e863..fc028492dd1a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -80,13 +80,13 @@ static snd_pcm_uframes_t snd_usb_pcm_pointer(struct snd_pcm_substream *substream /* * find a matching audio format */ -static struct audioformat * +static const struct audioformat * find_format(struct list_head *fmt_list_head, snd_pcm_format_t format, unsigned int rate, unsigned int channels, bool strict_match, struct snd_usb_substream *subs) { - struct audioformat *fp; - struct audioformat *found = NULL; + const struct audioformat *fp; + const struct audioformat *found = NULL; int cur_attr = 0, attr;

list_for_each_entry(fp, fmt_list_head, list) { @@ -141,7 +141,7 @@ find_format(struct list_head *fmt_list_head, snd_pcm_format_t format, return found; }

-static struct audioformat * +static const struct audioformat * find_substream_format(struct snd_usb_substream *subs, const struct snd_pcm_hw_params *params) { @@ -182,7 +182,7 @@ static int init_pitch_v2(struct snd_usb_audio *chip, int ep) * initialize the pitch control and sample rate */ int snd_usb_init_pitch(struct snd_usb_audio *chip, - struct audioformat *fmt) + const struct audioformat *fmt) { int err;

@@ -641,14 +641,14 @@ find_matching_substream(struct snd_usb_audio *chip, int stream, int ep_num, return NULL; }

-static struct audioformat * +static const struct audioformat * find_implicit_fb_sync_format(struct snd_usb_audio *chip, const struct audioformat *target, const struct snd_pcm_hw_params *params, int stream) { struct snd_usb_substream *subs; - struct audioformat *fp, *sync_fmt; + const struct audioformat *fp, *sync_fmt; int score, high_score;

subs = find_matching_substream(chip, stream, target->sync_ep, @@ -726,8 +726,8 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, { struct snd_usb_substream *subs = substream->runtime->private_data; struct snd_usb_audio *chip = subs->stream->chip; - struct audioformat *fmt; - struct audioformat *sync_fmt; + const struct audioformat *fmt; + const struct audioformat *sync_fmt; int ret;

ret = snd_media_start_pipeline(subs); @@ -918,7 +918,7 @@ static const struct snd_pcm_hardware snd_usb_hardware =

static int hw_check_valid_format(struct snd_usb_substream *subs, struct snd_pcm_hw_params *params, - struct audioformat *fp) + const struct audioformat *fp) { struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); struct snd_interval *ct = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); @@ -995,7 +995,7 @@ static int hw_rule_rate(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct audioformat *fp; + const struct audioformat *fp; struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); unsigned int rmin, rmax, r; int i; @@ -1028,7 +1028,7 @@ static int hw_rule_channels(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct audioformat *fp; + const struct audioformat *fp; struct snd_interval *it = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); unsigned int rmin, rmax;

@@ -1049,7 +1049,7 @@ static int hw_rule_format(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct audioformat *fp; + const struct audioformat *fp; struct snd_mask *fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); u64 fbits; u32 oldbits[2]; @@ -1080,7 +1080,7 @@ static int hw_rule_period_time(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { struct snd_usb_substream *subs = rule->private; - struct audioformat *fp; + const struct audioformat *fp; struct snd_interval *it; unsigned char min_datainterval; unsigned int pmin; @@ -1109,7 +1109,7 @@ static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime, { struct snd_usb_audio *chip = subs->stream->chip; struct snd_usb_endpoint *ep; - struct audioformat *fp; + const struct audioformat *fp; int err;

list_for_each_entry(fp, &subs->fmt_list, list) { @@ -1170,7 +1170,7 @@ static int apply_hw_constraint_from_sync(struct snd_pcm_runtime *runtime, static int setup_hw_info(struct snd_pcm_runtime *runtime, struct snd_usb_substream *subs) { struct snd_usb_audio *chip = subs->stream->chip; - struct audioformat *fp; + const struct audioformat *fp; unsigned int pt, ptmin; int param_period_time_if_needed = -1; int err; diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h index a4f784225abc..06c586467d3f 100644 --- a/sound/usb/pcm.h +++ b/sound/usb/pcm.h @@ -10,7 +10,7 @@ int snd_usb_pcm_suspend(struct snd_usb_stream *as); int snd_usb_pcm_resume(struct snd_usb_stream *as);

int snd_usb_init_pitch(struct snd_usb_audio *chip, - struct audioformat *fmt); + const struct audioformat *fmt); void snd_usb_preallocate_buffer(struct snd_usb_substream *subs);

int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 7e7f258011ff..5510c8a98447 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -1375,7 +1375,8 @@ int snd_usb_apply_boot_quirk_once(struct usb_device *dev, /* * check if the device uses big-endian samples */ -int snd_usb_is_big_endian_format(struct snd_usb_audio *chip, struct audioformat *fp) +int snd_usb_is_big_endian_format(struct snd_usb_audio *chip, + const struct audioformat *fp) { /* it depends on altsetting whether the device is big-endian or not */ switch (chip->usb_id) { @@ -1414,7 +1415,7 @@ enum { };

static void set_format_emu_quirk(struct snd_usb_substream *subs, - struct audioformat *fmt) + const struct audioformat *fmt) { unsigned char emu_samplerate_id = 0;

@@ -1476,7 +1477,7 @@ static int pioneer_djm_set_format_quirk(struct snd_usb_substream *subs) }

void snd_usb_set_format_quirk(struct snd_usb_substream *subs, - struct audioformat *fmt) + const struct audioformat *fmt) { switch (subs->stream->chip->usb_id) { case USB_ID(0x041e, 0x3f02): /* E-Mu 0202 USB */ @@ -1543,7 +1544,7 @@ static bool is_itf_usb_dsd_dac(unsigned int id) }

int snd_usb_select_mode_quirk(struct snd_usb_audio *chip, - struct audioformat *fmt) + const struct audioformat *fmt) { struct usb_device *dev = chip->dev; int err; diff --git a/sound/usb/quirks.h b/sound/usb/quirks.h index 011f22cf2bf6..67a02303c820 100644 --- a/sound/usb/quirks.h +++ b/sound/usb/quirks.h @@ -26,12 +26,12 @@ int snd_usb_apply_boot_quirk_once(struct usb_device *dev, unsigned int usb_id);

void snd_usb_set_format_quirk(struct snd_usb_substream *subs, - struct audioformat *fmt); + const struct audioformat *fmt);

bool snd_usb_get_sample_rate_quirk(struct snd_usb_audio *chip);

int snd_usb_is_big_endian_format(struct snd_usb_audio *chip, - struct audioformat *fp); + const struct audioformat *fp);

void snd_usb_endpoint_start_quirk(struct snd_usb_endpoint *ep);

@@ -41,7 +41,7 @@ void snd_usb_ctl_msg_quirk(struct usb_device *dev, unsigned int pipe, __u16 index, void *data, __u16 size);

int snd_usb_select_mode_quirk(struct snd_usb_audio *chip, - struct audioformat *fmt); + const struct audioformat *fmt);

u64 snd_usb_interface_dsd_format_quirks(struct snd_usb_audio *chip, struct audioformat *fp,

Minor code refactoring to consolidate the URB deactivation code in endpoint.c. A slight behavior change is that the error handling in snd_usb_endpoint_start() leaves EP_FLAG_STOPPING now. This should be synced with the later PCM sync_stop callback.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 4d733b2d8287..35c84c2264e1 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -777,6 +777,9 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) unsigned long end_time = jiffies + msecs_to_jiffies(1000); int alive;

+ if (!test_bit(EP_FLAG_STOPPING, &ep->flags)) + return 0; + do { alive = bitmap_weight(&ep->active_mask, ep->nurbs); if (!alive) @@ -802,22 +805,31 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) */ void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep) { - if (ep && test_bit(EP_FLAG_STOPPING, &ep->flags)) + if (ep) wait_clear_urbs(ep); }

/* - * unlink active urbs. + * Stop and unlink active urbs. + * + * This function checks and clears EP_FLAG_RUNNING state. + * When @wait_sync is set, it waits until all pending URBs are killed. */ -static int deactivate_urbs(struct snd_usb_endpoint *ep, bool force) +static int stop_and_unlink_urbs(struct snd_usb_endpoint *ep, bool force, + bool wait_sync) { unsigned int i;

if (!force && atomic_read(&ep->chip->shutdown)) /* to be sure... */ return -EBADFD;

- clear_bit(EP_FLAG_RUNNING, &ep->flags); + if (atomic_read(&ep->running)) + return -EBUSY; + + if (!test_and_clear_bit(EP_FLAG_RUNNING, &ep->flags)) + goto out;

+ set_bit(EP_FLAG_STOPPING, &ep->flags); INIT_LIST_HEAD(&ep->ready_playback_urbs); ep->next_packet_head = 0; ep->next_packet_queued = 0; @@ -831,6 +843,9 @@ static int deactivate_urbs(struct snd_usb_endpoint *ep, bool force) } }

+ out: + if (wait_sync) + return wait_clear_urbs(ep); return 0; }

@@ -845,8 +860,7 @@ static void release_urbs(struct snd_usb_endpoint *ep, int force) snd_usb_endpoint_set_callback(ep, NULL, NULL, NULL);

/* stop urbs */ - deactivate_urbs(ep, force); - wait_clear_urbs(ep); + stop_and_unlink_urbs(ep, force, true);

for (i = 0; i < ep->nurbs; i++) release_urb_ctx(&ep->urb[i]); @@ -1261,9 +1275,6 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) if (atomic_inc_return(&ep->running) != 1) return 0;

- /* just to be sure */ - deactivate_urbs(ep, false); - ep->active_mask = 0; ep->unlink_mask = 0; ep->phase = 0; @@ -1317,11 +1328,7 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) return 0;

__error: - if (ep->sync_master) - WRITE_ONCE(ep->sync_master->sync_slave, NULL); - clear_bit(EP_FLAG_RUNNING, &ep->flags); - atomic_dec(&ep->running); - deactivate_urbs(ep, false); + snd_usb_endpoint_stop(ep); return -EPIPE; }

@@ -1354,10 +1361,8 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) if (ep->sync_master) WRITE_ONCE(ep->sync_master->sync_slave, NULL);

- if (!atomic_dec_return(&ep->running)) { - deactivate_urbs(ep, false); - set_bit(EP_FLAG_STOPPING, &ep->flags); - } + if (!atomic_dec_return(&ep->running)) + stop_and_unlink_urbs(ep, false, false); }

/**

There are two places calculating the next packet size for the playback stream in the exactly same way. Provide the single helper for this purpose and use it from both places gracefully.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/endpoint.c | 40 ++++++++++++++++++++++++---------------- sound/usb/endpoint.h | 4 ++-- sound/usb/pcm.c | 8 +------- 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 35c84c2264e1..5d618724bd75 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -121,13 +121,13 @@ int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep) }

/* - * For streaming based on information derived from sync endpoints, - * prepare_outbound_urb_sizes() will call slave_next_packet_size() to - * determine the number of samples to be sent in the next packet. + * Return the number of samples to be sent in the next packet + * for streaming based on information derived from sync endpoints * - * For implicit feedback, slave_next_packet_size() is unused. + * This won't be used for implicit feedback which takes the packet size + * returned from the sync source */ -int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep) +static int slave_next_packet_size(struct snd_usb_endpoint *ep) { unsigned long flags; int ret; @@ -145,11 +145,10 @@ int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep) }

/* - * For adaptive and synchronous endpoints, prepare_outbound_urb_sizes() - * will call next_packet_size() to determine the number of samples to be - * sent in the next packet. + * Return the number of samples to be sent in the next packet + * for adaptive and synchronous endpoints */ -int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep) +static int next_packet_size(struct snd_usb_endpoint *ep) { int ret;

@@ -167,6 +166,21 @@ int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep) return ret; }

+/* + * snd_usb_endpoint_next_packet_size: Return the number of samples to be sent + * in the next packet + */ +int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx, int idx) +{ + if (ctx->packet_size[idx]) + return ctx->packet_size[idx]; + else if (ep->sync_master) + return slave_next_packet_size(ep); + else + return next_packet_size(ep); +} + static void call_retire_callback(struct snd_usb_endpoint *ep, struct urb *urb) { @@ -223,13 +237,7 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, unsigned int length; int counts;

- if (ctx->packet_size[i]) - counts = ctx->packet_size[i]; - else if (ep->sync_master) - counts = snd_usb_endpoint_slave_next_packet_size(ep); - else - counts = snd_usb_endpoint_next_packet_size(ep); - + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i); length = counts * ep->stride; /* number of silent bytes */ offset = offs * ep->stride + extra * i; urb->iso_frame_desc[i].offset = offset; diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 201011d89659..11e3bb839fd7 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -45,7 +45,7 @@ void snd_usb_endpoint_release(struct snd_usb_endpoint *ep); void snd_usb_endpoint_free(struct snd_usb_endpoint *ep);

int snd_usb_endpoint_implicit_feedback_sink(struct snd_usb_endpoint *ep); -int snd_usb_endpoint_slave_next_packet_size(struct snd_usb_endpoint *ep); -int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep); +int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx, int idx);

#endif /* __USBAUDIO_ENDPOINT_H */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 95a6a854dc55..5953e22a72c5 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1512,13 +1512,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, spin_lock_irqsave(&subs->lock, flags); subs->frame_limit += ep->max_urb_frames; for (i = 0; i < ctx->packets; i++) { - if (ctx->packet_size[i]) - counts = ctx->packet_size[i]; - else if (ep->sync_master) - counts = snd_usb_endpoint_slave_next_packet_size(ep); - else - counts = snd_usb_endpoint_next_packet_size(ep); - + counts = snd_usb_endpoint_next_packet_size(ep, ctx, i); /* set up descriptor */ urb->iso_frame_desc[i].offset = frames * ep->stride; urb->iso_frame_desc[i].length = counts * ep->stride;

Follow the inclusive terminology, just replace sync_master/sync_slave with sync_source/sync_sink. It's also a bit clearer from its meaning, too.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/card.h | 4 ++-- sound/usb/endpoint.c | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/sound/usb/card.h b/sound/usb/card.h index 1dd7a514d1d5..3e16b9288693 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -71,8 +71,8 @@ struct snd_usb_endpoint { struct urb *urb);

struct snd_usb_substream *data_subs; - struct snd_usb_endpoint *sync_master; - struct snd_usb_endpoint *sync_slave; + struct snd_usb_endpoint *sync_source; + struct snd_usb_endpoint *sync_sink;

struct snd_urb_ctx urb[MAX_URBS];

diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 5d618724bd75..5f1d5f1ed8db 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -175,7 +175,7 @@ int snd_usb_endpoint_next_packet_size(struct snd_usb_endpoint *ep, { if (ctx->packet_size[idx]) return ctx->packet_size[idx]; - else if (ep->sync_master) + else if (ep->sync_source) return slave_next_packet_size(ep); else return next_packet_size(ep); @@ -205,16 +205,16 @@ 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; + struct snd_usb_endpoint *sync_sink;

if (unlikely(ep->skip_packets > 0)) { ep->skip_packets--; return; }

- sync_slave = READ_ONCE(ep->sync_slave); - if (sync_slave) - snd_usb_handle_sync_urb(sync_slave, ep, urb); + sync_sink = READ_ONCE(ep->sync_sink); + if (sync_sink) + snd_usb_handle_sync_urb(sync_sink, ep, urb);

call_retire_callback(ep, urb); } @@ -708,7 +708,7 @@ 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; + data_ep->sync_source = sync_ep; }

/* @@ -802,7 +802,7 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) alive, ep->ep_num); clear_bit(EP_FLAG_STOPPING, &ep->flags);

- ep->sync_slave = NULL; + ep->sync_sink = NULL; snd_usb_endpoint_set_callback(ep, NULL, NULL, NULL);

return 0; @@ -969,9 +969,9 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep) packs_per_ms = 1; max_packs_per_urb = MAX_PACKS; } - if (ep->sync_master && !ep->implicit_fb_sync) + if (ep->sync_source && !ep->implicit_fb_sync) max_packs_per_urb = min(max_packs_per_urb, - 1U << ep->sync_master->syncinterval); + 1U << ep->sync_source->syncinterval); max_packs_per_urb = max(1u, max_packs_per_urb >> ep->datainterval);

/* @@ -1015,7 +1015,7 @@ 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 (ep->sync_master) + if (ep->sync_source) minsize -= minsize >> 3; minsize = max(minsize, 1u);

@@ -1272,8 +1272,8 @@ 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); + if (ep->sync_source) + WRITE_ONCE(ep->sync_source->sync_sink, ep);

usb_audio_dbg(ep->chip, "Starting %s EP 0x%x (running %d)\n", ep_type_name(ep->type), ep->ep_num, @@ -1366,8 +1366,8 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) if (snd_BUG_ON(!atomic_read(&ep->running))) return;

- if (ep->sync_master) - WRITE_ONCE(ep->sync_master->sync_slave, NULL); + if (ep->sync_source) + WRITE_ONCE(ep->sync_source->sync_sink, NULL);

if (!atomic_dec_return(&ep->running)) stop_and_unlink_urbs(ep, false, false);

Now the sync endpoints have been parsed at the beginning and won't be changed dynamically, let's show them in the proc outputs for helping debugging.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/proc.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/sound/usb/proc.c b/sound/usb/proc.c index 447ba32e281c..e9bbaea7b2fa 100644 --- a/sound/usb/proc.c +++ b/sound/usb/proc.c @@ -108,7 +108,8 @@ static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct s snd_pcm_format_name(fmt)); snd_iprintf(buffer, "\n"); snd_iprintf(buffer, " Channels: %d\n", fp->channels); - snd_iprintf(buffer, " Endpoint: %d %s (%s)\n", + snd_iprintf(buffer, " Endpoint: 0x%02x (%d %s) (%s)\n", + fp->endpoint, fp->endpoint & USB_ENDPOINT_NUMBER_MASK, fp->endpoint & USB_DIR_IN ? "IN" : "OUT", sync_types[(fp->ep_attr & USB_ENDPOINT_SYNCTYPE) >> 2]); @@ -150,6 +151,19 @@ static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct s snd_iprintf(buffer, "\n"); }

+ if (fp->sync_ep) { + snd_iprintf(buffer, " Sync Endpoint: 0x%02x (%d %s)\n", + fp->sync_ep, + fp->sync_ep & USB_ENDPOINT_NUMBER_MASK, + fp->sync_ep & USB_DIR_IN ? "IN" : "OUT"); + snd_iprintf(buffer, " Sync EP Interface: %d\n", + fp->sync_iface); + snd_iprintf(buffer, " Sync EP Altset: %d\n", + fp->sync_altsetting); + snd_iprintf(buffer, " Implicit Feedback Mode: %s\n", + fp->implicit_fb ? "Yes" : "No"); + } + // snd_iprintf(buffer, " Max Packet Size = %d\n", fp->maxpacksize); // snd_iprintf(buffer, " EP Attribute = %#x\n", fp->attributes); }

The code dealing with the implicit feedback mode grew recently, and it's becoming messy. As we receive more and more devices that need the similar handling, it's better to be processed through a table instead of the open code.

This patch moves the code that is relevant with parsing the implicit feedback mode and some helpers into another file, implicit.c. The detection and the setup of the implicit feedback sync EPs are rewritten to use the ID/class matching table instead.

There should be no functional changes.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/Makefile | 1 + sound/usb/implicit.c | 347 +++++++++++++++++++++++++++++++++++++++++++++++++++ sound/usb/implicit.h | 14 +++ sound/usb/pcm.c | 320 +++-------------------------------------------- 4 files changed, 376 insertions(+), 306 deletions(-) create mode 100644 sound/usb/implicit.c create mode 100644 sound/usb/implicit.h

diff --git a/sound/usb/Makefile b/sound/usb/Makefile index 56031026b113..9ccb21a4ff8a 100644 --- a/sound/usb/Makefile +++ b/sound/usb/Makefile @@ -8,6 +8,7 @@ snd-usb-audio-objs := card.o \ endpoint.o \ format.o \ helper.o \ + implicit.o \ mixer.o \ mixer_quirks.o \ mixer_scarlett.o \ diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c new file mode 100644 index 000000000000..bc7edecff946 --- /dev/null +++ b/sound/usb/implicit.c @@ -0,0 +1,347 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +// +// Special handling for implicit feedback mode +// + +#include <linux/init.h> +#include <linux/usb.h> +#include <linux/usb/audio.h> +#include <linux/usb/audio-v2.h> + +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> + +#include "usbaudio.h" +#include "card.h" +#include "helper.h" +#include "implicit.h" + +enum { + IMPLICIT_FB_NONE, + IMPLICIT_FB_FIXED, +}; + +struct snd_usb_implicit_fb_match { + unsigned int id; + unsigned int iface_class; + unsigned int ep_num; + unsigned int iface; + int type; +}; + +#define IMPLICIT_FB_FIXED_DEV(vend, prod, ep, ifnum) \ + { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_FIXED, .ep_num = (ep),\ + .iface = (ifnum) } +#define IMPLICIT_FB_SKIP_DEV(vend, prod) \ + { .id = USB_ID(vend, prod), .type = IMPLICIT_FB_NONE } + +/* Implicit feedback quirk table for playback */ +static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = { + /* Fixed EP */ + IMPLICIT_FB_FIXED_DEV(0x1397, 0x0001, 0x81, 1), /* Behringer UFX1604 */ + IMPLICIT_FB_FIXED_DEV(0x1397, 0x0002, 0x81, 1), /* Behringer UFX1204 */ + IMPLICIT_FB_FIXED_DEV(0x0763, 0x2080, 0x81, 2), /* M-Audio FastTrack Ultra */ + IMPLICIT_FB_FIXED_DEV(0x0763, 0x2081, 0x81, 2), /* M-Audio FastTrack Ultra */ + IMPLICIT_FB_FIXED_DEV(0x2466, 0x8010, 0x81, 2), /* Fractal Audio Axe-Fx III */ + IMPLICIT_FB_FIXED_DEV(0x07fd, 0x0008, 0x81, 2), /* MOTU M Series */ + IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0001, 0x81, 2), /* Solid State Logic SSL2 */ + IMPLICIT_FB_FIXED_DEV(0x31e9, 0x0002, 0x81, 2), /* Solid State Logic SSL2+ */ + IMPLICIT_FB_FIXED_DEV(0x0499, 0x172f, 0x81, 2), /* Steinberg UR22C */ + IMPLICIT_FB_FIXED_DEV(0x0d9a, 0x00df, 0x81, 2), /* RTX6001 */ + IMPLICIT_FB_FIXED_DEV(0x0763, 0x2030, 0x81, 3), /* M-Audio Fast Track C400 */ + IMPLICIT_FB_FIXED_DEV(0x0763, 0x2031, 0x81, 3), /* M-Audio Fast Track C600 */ + IMPLICIT_FB_FIXED_DEV(0x22f0, 0x0006, 0x81, 3), /* Allen&Heath Qu-16 */ + IMPLICIT_FB_FIXED_DEV(0x2b73, 0x000a, 0x82, 0), /* Pioneer DJ DJM-900NXS2 */ + IMPLICIT_FB_FIXED_DEV(0x2b73, 0x0017, 0x82, 0), /* Pioneer DJ DJM-250MK2 */ + IMPLICIT_FB_FIXED_DEV(0x1686, 0xf029, 0x82, 2), /* Zoom UAC-2 */ + IMPLICIT_FB_FIXED_DEV(0x2466, 0x8003, 0x86, 2), /* Fractal Audio Axe-Fx II */ + IMPLICIT_FB_FIXED_DEV(0x0499, 0x172a, 0x86, 2), /* Yamaha MODX */ + + /* Special matching */ + { .id = USB_ID(0x07fd, 0x0004), .iface_class = USB_CLASS_AUDIO, + .type = IMPLICIT_FB_NONE }, /* MicroBook IIc */ + /* ep = 0x84, ifnum = 0 */ + { .id = USB_ID(0x07fd, 0x0004), .iface_class = USB_CLASS_VENDOR_SPEC, + .type = IMPLICIT_FB_FIXED, + .ep_num = 0x84, .iface = 0 }, /* MOTU MicroBook II */ + + /* No quirk */ + IMPLICIT_FB_SKIP_DEV(0x0582, 0x01d8), /* BOSS Katana */ + + /* No quirk for playback but with capture quirk (see below) */ + IMPLICIT_FB_SKIP_DEV(0x0582, 0x01e5), /* BOSS GT-001 */ + + {} /* terminator */ +}; + +/* Implicit feedback quirk table for capture */ +static const struct snd_usb_implicit_fb_match capture_implicit_fb_quirks[] = { + IMPLICIT_FB_FIXED_DEV(0x0582, 0x01e5, 0x0d, 0x01), /* BOSS GT-001 */ + + {} /* terminator */ +}; + +/* set up sync EP information on the audioformat */ +static int add_implicit_fb_sync_ep(struct snd_usb_audio *chip, + struct audioformat *fmt, + int ep, int ifnum, + const struct usb_host_interface *alts) +{ + struct usb_interface *iface; + + if (!alts) { + iface = usb_ifnum_to_if(chip->dev, ifnum); + if (!iface || iface->num_altsetting < 2) + return 0; + alts = &iface->altsetting[1]; + } + + fmt->sync_ep = ep; + fmt->sync_iface = ifnum; + fmt->sync_altsetting = alts->desc.bAlternateSetting; + fmt->sync_ep_idx = 0; + fmt->implicit_fb = 1; + usb_audio_dbg(chip, + "%d:%d: added %s implicit_fb sync_ep %x, iface %d:%d\n", + fmt->iface, fmt->altsetting, + (ep & USB_DIR_IN) ? "playback" : "capture", + fmt->sync_ep, fmt->sync_iface, fmt->sync_altsetting); + return 1; +} + +/* Check whether the given UAC2 iface:altset points to an implicit fb source */ +static int add_generic_uac2_implicit_fb(struct snd_usb_audio *chip, + struct audioformat *fmt, + unsigned int ifnum, + unsigned int altsetting) +{ + struct usb_host_interface *alts; + struct usb_endpoint_descriptor *epd; + + alts = snd_usb_get_host_interface(chip, ifnum, altsetting); + if (!alts) + return 0; + if (alts->desc.bInterfaceClass != USB_CLASS_AUDIO || + alts->desc.bInterfaceSubClass != USB_SUBCLASS_AUDIOSTREAMING || + alts->desc.bInterfaceProtocol != UAC_VERSION_2 || + alts->desc.bNumEndpoints < 1) + return 0; + epd = get_endpoint(alts, 0); + if (!usb_endpoint_is_isoc_in(epd) || + (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != + USB_ENDPOINT_USAGE_IMPLICIT_FB) + return 0; + return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, + ifnum, alts); +} + +/* Like the function above, but specific to Roland with vendor class and hack */ +static int add_roland_implicit_fb(struct snd_usb_audio *chip, + struct audioformat *fmt, + unsigned int ifnum, + unsigned int altsetting) +{ + struct usb_host_interface *alts; + struct usb_endpoint_descriptor *epd; + + alts = snd_usb_get_host_interface(chip, ifnum, altsetting); + if (!alts) + return 0; + if (alts->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC || + (alts->desc.bInterfaceSubClass != 2 && + alts->desc.bInterfaceProtocol != 2) || + alts->desc.bNumEndpoints < 1) + return 0; + epd = get_endpoint(alts, 0); + if (!usb_endpoint_is_isoc_in(epd) || + (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != + USB_ENDPOINT_USAGE_IMPLICIT_FB) + return 0; + return add_implicit_fb_sync_ep(chip, fmt, epd->bEndpointAddress, + ifnum, alts); +} + +static const struct snd_usb_implicit_fb_match * +find_implicit_fb_entry(struct snd_usb_audio *chip, + const struct snd_usb_implicit_fb_match *match, + const struct usb_host_interface *alts) +{ + for (; match->id; match++) + if (match->id == chip->usb_id && + (!match->iface_class || + (alts->desc.bInterfaceClass == match->iface_class))) + return match; + + return NULL; +} + +/* Setup an implicit feedback endpoint from a quirk. Returns 0 if no quirk + * applies. Returns 1 if a quirk was found. + */ +static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_host_interface *alts) +{ + const struct snd_usb_implicit_fb_match *p; + unsigned int attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE; + + p = find_implicit_fb_entry(chip, playback_implicit_fb_quirks, alts); + if (p) { + switch (p->type) { + case IMPLICIT_FB_NONE: + return 0; /* No quirk */ + case IMPLICIT_FB_FIXED: + return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, + p->iface, NULL); + } + } + + /* Generic UAC2 implicit feedback */ + if (attr == USB_ENDPOINT_SYNC_ASYNC && + alts->desc.bInterfaceClass == USB_CLASS_AUDIO && + alts->desc.bInterfaceProtocol == UAC_VERSION_2 && + alts->desc.bNumEndpoints == 1) { + if (add_generic_uac2_implicit_fb(chip, fmt, + alts->desc.bInterfaceNumber + 1, + alts->desc.bAlternateSetting)) + return 1; + } + + /* Roland/BOSS implicit feedback with vendor spec class */ + if (attr == USB_ENDPOINT_SYNC_ASYNC && + alts->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC && + alts->desc.bInterfaceProtocol == 2 && + alts->desc.bNumEndpoints == 1 && + USB_ID_VENDOR(chip->usb_id) == 0x0582 /* Roland */) { + if (add_roland_implicit_fb(chip, fmt, + alts->desc.bInterfaceNumber + 1, + alts->desc.bAlternateSetting)) + return 1; + } + + /* No quirk */ + return 0; +} + +/* same for capture, but only handling FIXED entry */ +static int audioformat_capture_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_host_interface *alts) +{ + const struct snd_usb_implicit_fb_match *p; + + p = find_implicit_fb_entry(chip, capture_implicit_fb_quirks, alts); + if (p && p->type == IMPLICIT_FB_FIXED) + return add_implicit_fb_sync_ep(chip, fmt, p->ep_num, p->iface, + NULL); + return 0; +} + +/* + * Parse altset and set up implicit feedback endpoint on the audioformat + */ +int snd_usb_parse_implicit_fb_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_host_interface *alts) +{ + if (fmt->endpoint & USB_DIR_IN) + return audioformat_capture_quirk(chip, fmt, alts); + else + return audioformat_implicit_fb_quirk(chip, fmt, alts); +} + +/* + * Return the score of matching two audioformats. + * Veto the audioformat if: + * - It has no channels for some reason. + * - Requested PCM format is not supported. + * - Requested sample rate is not supported. + */ +static int match_endpoint_audioformats(struct snd_usb_substream *subs, + const struct audioformat *fp, + int rate, int channels, + snd_pcm_format_t pcm_format) +{ + int i, score; + + if (fp->channels < 1) + return 0; + + if (!(fp->formats & pcm_format_to_bits(pcm_format))) + return 0; + + if (fp->rates & SNDRV_PCM_RATE_CONTINUOUS) { + if (rate < fp->rate_min || rate > fp->rate_max) + return 0; + } else { + for (i = 0; i < fp->nr_rates; i++) { + if (fp->rate_table[i] == rate) + break; + } + if (i >= fp->nr_rates) + return 0; + } + + score = 1; + if (fp->channels == channels) + score++; + + return score; +} + +static struct snd_usb_substream * +find_matching_substream(struct snd_usb_audio *chip, int stream, int ep_num, + int fmt_type) +{ + struct snd_usb_stream *as; + struct snd_usb_substream *subs; + + list_for_each_entry(as, &chip->pcm_list, list) { + subs = &as->substream[stream]; + if (as->fmt_type == fmt_type && subs->ep_num == ep_num) + return subs; + } + + return NULL; +} + +/* + * Return the audioformat that is suitable for the implicit fb + */ +const struct audioformat * +snd_usb_find_implicit_fb_sync_format(struct snd_usb_audio *chip, + const struct audioformat *target, + const struct snd_pcm_hw_params *params, + int stream) +{ + struct snd_usb_substream *subs; + const struct audioformat *fp, *sync_fmt; + int score, high_score; + + /* When sharing the same altset, use the original audioformat */ + if (target->iface == target->sync_iface && + target->altsetting == target->sync_altsetting) + return target; + + subs = find_matching_substream(chip, stream, target->sync_ep, + target->fmt_type); + if (!subs) + return NULL; + + sync_fmt = NULL; + high_score = 0; + list_for_each_entry(fp, &subs->fmt_list, list) { + score = match_endpoint_audioformats(subs, fp, + params_rate(params), + params_channels(params), + params_format(params)); + if (score > high_score) { + sync_fmt = fp; + high_score = score; + } + } + + return sync_fmt; +} + diff --git a/sound/usb/implicit.h b/sound/usb/implicit.h new file mode 100644 index 000000000000..ccb415a0ea86 --- /dev/null +++ b/sound/usb/implicit.h @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef __USBAUDIO_IMPLICIT_H +#define __USBAUDIO_IMPLICIT_H + +int snd_usb_parse_implicit_fb_quirk(struct snd_usb_audio *chip, + struct audioformat *fmt, + struct usb_host_interface *alts); +const struct audioformat * +snd_usb_find_implicit_fb_sync_format(struct snd_usb_audio *chip, + const struct audioformat *target, + const struct snd_pcm_hw_params *params, + int stream); + +#endif /* __USBAUDIO_IMPLICIT_H */ diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 676acc20595b..56079901769f 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -23,6 +23,7 @@ #include "clock.h" #include "power.h" #include "media.h" +#include "implicit.h"

#define SUBSTREAM_FLAG_DATA_EP_STARTED 0 #define SUBSTREAM_FLAG_SYNC_EP_STARTED 1 @@ -276,211 +277,7 @@ static int snd_usb_pcm_sync_stop(struct snd_pcm_substream *substream) return 0; }

-/* Check whether the given iface:altsetting points to an implicit fb source */ -static bool search_generic_implicit_fb(struct snd_usb_audio *chip, int ifnum, - unsigned int altsetting, - struct usb_host_interface **altsp, - unsigned int *ep) -{ - struct usb_host_interface *alts; - struct usb_interface_descriptor *altsd; - struct usb_endpoint_descriptor *epd; - - alts = snd_usb_get_host_interface(chip, ifnum, altsetting); - if (!alts) - return false; - altsd = get_iface_desc(alts); - if (altsd->bInterfaceClass != USB_CLASS_AUDIO || - altsd->bInterfaceSubClass != USB_SUBCLASS_AUDIOSTREAMING || - altsd->bInterfaceProtocol != UAC_VERSION_2 || - altsd->bNumEndpoints < 1) - return false; - epd = get_endpoint(alts, 0); - if (!usb_endpoint_is_isoc_in(epd) || - (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != - USB_ENDPOINT_USAGE_IMPLICIT_FB) - return false; - *ep = epd->bEndpointAddress; - *altsp = alts; - return true; -} - -/* Like the function above, but specific to Roland with vendor class and hack */ -static bool search_roland_implicit_fb(struct snd_usb_audio *chip, int ifnum, - unsigned int altsetting, - struct usb_host_interface **altsp, - unsigned int *ep) -{ - struct usb_host_interface *alts; - struct usb_interface_descriptor *altsd; - struct usb_endpoint_descriptor *epd; - - alts = snd_usb_get_host_interface(chip, ifnum, altsetting); - if (!alts) - return false; - altsd = get_iface_desc(alts); - if (altsd->bInterfaceClass != USB_CLASS_VENDOR_SPEC || - (altsd->bInterfaceSubClass != 2 && - altsd->bInterfaceProtocol != 2) || - altsd->bNumEndpoints < 1) - return false; - epd = get_endpoint(alts, 0); - if (!usb_endpoint_is_isoc_in(epd) || - (epd->bmAttributes & USB_ENDPOINT_USAGE_MASK) != - USB_ENDPOINT_USAGE_IMPLICIT_FB) - return false; - *ep = epd->bEndpointAddress; - *altsp = alts; - return true; -} - -/* Setup an implicit feedback endpoint from a quirk. Returns 0 if no quirk - * applies. Returns 1 if a quirk was found. - */ -static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, - struct audioformat *fmt, - struct usb_host_interface *alts) -{ - struct usb_device *dev = chip->dev; - struct usb_interface_descriptor *altsd = get_iface_desc(alts); - struct usb_interface *iface; - unsigned int attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE; - unsigned int ep; - unsigned int ifnum; - - switch (chip->usb_id) { - case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ - case USB_ID(0x0763, 0x2031): /* M-Audio Fast Track C600 */ - case USB_ID(0x22f0, 0x0006): /* Allen&Heath Qu-16 */ - ep = 0x81; - ifnum = 3; - goto add_sync_ep_from_ifnum; - case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */ - case USB_ID(0x0763, 0x2081): - ep = 0x81; - ifnum = 2; - goto add_sync_ep_from_ifnum; - case USB_ID(0x2466, 0x8003): /* Fractal Audio Axe-Fx II */ - case USB_ID(0x0499, 0x172a): /* Yamaha MODX */ - ep = 0x86; - ifnum = 2; - goto add_sync_ep_from_ifnum; - case USB_ID(0x2466, 0x8010): /* Fractal Audio Axe-Fx III */ - ep = 0x81; - ifnum = 2; - goto add_sync_ep_from_ifnum; - case USB_ID(0x1686, 0xf029): /* Zoom UAC-2 */ - ep = 0x82; - ifnum = 2; - goto add_sync_ep_from_ifnum; - case USB_ID(0x1397, 0x0001): /* Behringer UFX1604 */ - case USB_ID(0x1397, 0x0002): /* Behringer UFX1204 */ - ep = 0x81; - ifnum = 1; - goto add_sync_ep_from_ifnum; - case USB_ID(0x07fd, 0x0004): /* MOTU MicroBook II/IIc */ - /* MicroBook IIc */ - if (altsd->bInterfaceClass == USB_CLASS_AUDIO) - return 0; - - /* MicroBook II */ - ep = 0x84; - ifnum = 0; - goto add_sync_ep_from_ifnum; - case USB_ID(0x07fd, 0x0008): /* MOTU M Series */ - case USB_ID(0x31e9, 0x0001): /* Solid State Logic SSL2 */ - case USB_ID(0x31e9, 0x0002): /* Solid State Logic SSL2+ */ - case USB_ID(0x0499, 0x172f): /* Steinberg UR22C */ - case USB_ID(0x0d9a, 0x00df): /* RTX6001 */ - ep = 0x81; - ifnum = 2; - goto add_sync_ep_from_ifnum; - case USB_ID(0x2b73, 0x000a): /* Pioneer DJ DJM-900NXS2 */ - case USB_ID(0x2b73, 0x0017): /* Pioneer DJ DJM-250MK2 */ - ep = 0x82; - ifnum = 0; - goto add_sync_ep_from_ifnum; - case USB_ID(0x0582, 0x01d8): /* BOSS Katana */ - /* BOSS Katana amplifiers do not need quirks */ - return 0; - case USB_ID(0x0582, 0x01e5): /* BOSS GT-001 */ - /* BOSS GT-001 needs no implicit fb for playback */ - return 0; - } - - /* Generic UAC2 implicit feedback */ - if (attr == USB_ENDPOINT_SYNC_ASYNC && - altsd->bInterfaceClass == USB_CLASS_AUDIO && - altsd->bInterfaceProtocol == UAC_VERSION_2 && - altsd->bNumEndpoints == 1) { - ifnum = altsd->bInterfaceNumber + 1; - if (search_generic_implicit_fb(chip, ifnum, - altsd->bAlternateSetting, - &alts, &ep)) - goto add_sync_ep; - } - - /* Roland/BOSS implicit feedback with vendor spec class */ - if (attr == USB_ENDPOINT_SYNC_ASYNC && - altsd->bInterfaceClass == USB_CLASS_VENDOR_SPEC && - altsd->bInterfaceProtocol == 2 && - altsd->bNumEndpoints == 1 && - USB_ID_VENDOR(chip->usb_id) == 0x0582 /* Roland */) { - ifnum = altsd->bInterfaceNumber + 1; - if (search_roland_implicit_fb(chip, ifnum, - altsd->bAlternateSetting, - &alts, &ep)) - goto add_sync_ep; - } - - /* No quirk */ - return 0; - -add_sync_ep_from_ifnum: - iface = usb_ifnum_to_if(dev, ifnum); - - if (!iface || iface->num_altsetting < 2) - return 0; - - alts = &iface->altsetting[1]; - -add_sync_ep: - fmt->sync_ep = ep; - fmt->sync_iface = ifnum; - fmt->sync_altsetting = alts->desc.bAlternateSetting; - fmt->sync_ep_idx = 0; - fmt->implicit_fb = 1; - dev_dbg(&dev->dev, "%d:%d: found implicit_fb sync_ep=%x, iface=%d, alt=%d\n", - fmt->iface, fmt->altsetting, fmt->sync_ep, fmt->sync_iface, - fmt->sync_altsetting); - - return 1; -} - -static int audioformat_capture_quirk(struct snd_usb_audio *chip, - struct audioformat *fmt, - struct usb_host_interface *alts) -{ - struct usb_device *dev = chip->dev; - - switch (chip->usb_id) { - case USB_ID(0x0582, 0x01e5): /* BOSS GT-001 */ - if (!snd_usb_get_host_interface(chip, 0x01, 0x01)) - return 0; - fmt->sync_ep = 0x0d; - fmt->sync_iface = 0x01; - fmt->sync_altsetting = 0x01; - fmt->sync_ep_idx = 0; - fmt->implicit_fb = 1; - dev_dbg(&dev->dev, "%d:%d: added fake capture sync sync_ep=%x, iface=%d, alt=%d\n", - fmt->iface, fmt->altsetting, fmt->sync_ep, fmt->sync_iface, - fmt->sync_altsetting); - return 1; - } - return 0; - -} - +/* Set up sync endpoint */ int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, struct audioformat *fmt) { @@ -496,20 +293,18 @@ int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, return 0; altsd = get_iface_desc(alts);

- is_playback = !(get_endpoint(alts, 0)->bEndpointAddress & USB_DIR_IN); - if (is_playback) { - err = audioformat_implicit_fb_quirk(chip, fmt, alts); - if (err > 0) - return 0; - } else { - err = audioformat_capture_quirk(chip, fmt, alts); - if (err > 0) - return 0; - } + err = snd_usb_parse_implicit_fb_quirk(chip, fmt, alts); + if (err > 0) + return 0; /* matched */ + + /* + * Generic sync EP handling + */

if (altsd->bNumEndpoints < 2) return 0;

+ is_playback = !(get_endpoint(alts, 0)->bEndpointAddress & USB_DIR_IN); attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE; if ((is_playback && (attr == USB_ENDPOINT_SYNC_SYNC || attr == USB_ENDPOINT_SYNC_ADAPTIVE)) || @@ -569,45 +364,6 @@ int snd_usb_audioformat_set_sync_ep(struct snd_usb_audio *chip, return 0; }

-/* - * Return the score of matching two audioformats. - * Veto the audioformat if: - * - It has no channels for some reason. - * - Requested PCM format is not supported. - * - Requested sample rate is not supported. - */ -static int match_endpoint_audioformats(struct snd_usb_substream *subs, - const struct audioformat *fp, - int rate, int channels, - snd_pcm_format_t pcm_format) -{ - int i, score; - - if (fp->channels < 1) - return 0; - - if (!(fp->formats & pcm_format_to_bits(pcm_format))) - return 0; - - if (fp->rates & SNDRV_PCM_RATE_CONTINUOUS) { - if (rate < fp->rate_min || rate > fp->rate_max) - return 0; - } else { - for (i = 0; i < fp->nr_rates; i++) { - if (fp->rate_table[i] == rate) - break; - } - if (i >= fp->nr_rates) - return 0; - } - - score = 1; - if (fp->channels == channels) - score++; - - return score; -} - static int snd_usb_pcm_change_state(struct snd_usb_substream *subs, int state) { int ret; @@ -656,53 +412,6 @@ int snd_usb_pcm_resume(struct snd_usb_stream *as) return 0; }

-static struct snd_usb_substream * -find_matching_substream(struct snd_usb_audio *chip, int stream, int ep_num, - int fmt_type) -{ - struct snd_usb_stream *as; - struct snd_usb_substream *subs; - - list_for_each_entry(as, &chip->pcm_list, list) { - subs = &as->substream[stream]; - if (as->fmt_type == fmt_type && subs->ep_num == ep_num) - return subs; - } - - return NULL; -} - -static const struct audioformat * -find_implicit_fb_sync_format(struct snd_usb_audio *chip, - const struct audioformat *target, - const struct snd_pcm_hw_params *params, - int stream) -{ - struct snd_usb_substream *subs; - const struct audioformat *fp, *sync_fmt; - int score, high_score; - - subs = find_matching_substream(chip, stream, target->sync_ep, - target->fmt_type); - if (!subs) - return NULL; - - sync_fmt = NULL; - high_score = 0; - list_for_each_entry(fp, &subs->fmt_list, list) { - score = match_endpoint_audioformats(subs, fp, - params_rate(params), - params_channels(params), - params_format(params)); - if (score > high_score) { - sync_fmt = fp; - high_score = score; - } - } - - return sync_fmt; -} - static void close_endpoints(struct snd_usb_audio *chip, struct snd_usb_substream *subs) { @@ -775,11 +484,10 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream, goto stop_pipeline; }

- if (fmt->implicit_fb && - (fmt->iface != fmt->sync_iface || - fmt->altsetting != fmt->sync_altsetting)) { - sync_fmt = find_implicit_fb_sync_format(chip, fmt, hw_params, - !substream->stream); + if (fmt->implicit_fb) { + sync_fmt = snd_usb_find_implicit_fb_sync_format(chip, fmt, + hw_params, + !substream->stream); if (!sync_fmt) { usb_audio_dbg(chip, "cannot find sync format: ep=0x%x, iface=%d:%d, format=%s, rate=%d, channels=%d\n",

A new module option, implicit_fb, is added to specify the driver looking for the implicit feedback sync. This can be useful for a device that could be working better in the implicit feed back mode and user wants to test it quickly. When this works, we can add the quirk entry easily.

Tested-by: Keith Milner kamilner@superlative.org Tested-by: Dylan Robinson dylan_robinson@motu.com Signed-off-by: Takashi Iwai tiwai@suse.de --- Documentation/sound/alsa-configuration.rst | 5 +++++ sound/usb/card.c | 4 ++++ sound/usb/implicit.c | 4 ++++ sound/usb/usbaudio.h | 1 + 4 files changed, 14 insertions(+)

diff --git a/Documentation/sound/alsa-configuration.rst b/Documentation/sound/alsa-configuration.rst index c755b1c5e16f..fe52c314b763 100644 --- a/Documentation/sound/alsa-configuration.rst +++ b/Documentation/sound/alsa-configuration.rst @@ -2227,6 +2227,11 @@ quirk_alias Quirk alias list, pass strings like ``0123abcd:5678beef``, which applies the existing quirk for the device 5678:beef to a new device 0123:abcd. +implicit_fb + Apply the generic implicit feedback sync mode. When this is set + and the playback stream sync mode is ASYNC, the driver tries to + tie an adjacent ASYNC capture stream as the implicit feedback + source. use_vmalloc Use vmalloc() for allocations of the PCM buffers (default: yes). For architectures with non-coherent memory like ARM or MIPS, the diff --git a/sound/usb/card.c b/sound/usb/card.c index 7940b3bff5bc..cb0b6582dfee 100644 --- a/sound/usb/card.c +++ b/sound/usb/card.c @@ -72,6 +72,7 @@ static bool ignore_ctl_error; static bool autoclock = true; static char *quirk_alias[SNDRV_CARDS]; static char *delayed_register[SNDRV_CARDS]; +static bool implicit_fb[SNDRV_CARDS];

bool snd_usb_use_vmalloc = true; bool snd_usb_skip_validation; @@ -97,6 +98,8 @@ module_param_array(quirk_alias, charp, NULL, 0444); MODULE_PARM_DESC(quirk_alias, "Quirk aliases, e.g. 0123abcd:5678beef."); module_param_array(delayed_register, charp, NULL, 0444); MODULE_PARM_DESC(delayed_register, "Quirk for delayed registration, given by id:iface, e.g. 0123abcd:4."); +module_param_array(implicit_fb, bool, NULL, 0444); +MODULE_PARM_DESC(implicit_fb, "Apply generic implicit feedback sync mode."); module_param_named(use_vmalloc, snd_usb_use_vmalloc, bool, 0444); MODULE_PARM_DESC(use_vmalloc, "Use vmalloc for PCM intermediate buffers (default: yes)."); module_param_named(skip_validation, snd_usb_skip_validation, bool, 0444); @@ -596,6 +599,7 @@ static int snd_usb_audio_create(struct usb_interface *intf, chip->dev = dev; chip->card = card; chip->setup = device_setup[idx]; + chip->generic_implicit_fb = implicit_fb[idx]; chip->autoclock = autoclock; atomic_set(&chip->active, 1); /* avoid autopm during probing */ atomic_set(&chip->usage_count, 0); diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 16b1fb55b7b9..1d87fcdae95b 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -269,6 +269,10 @@ static int audioformat_implicit_fb_quirk(struct snd_usb_audio *chip, return 1; }

+ /* Try the generic implicit fb if available */ + if (chip->generic_implicit_fb) + return add_generic_implicit_fb(chip, fmt, alts); + /* No quirk */ return 0; } diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index cc338e8e2597..980287aadd36 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -51,6 +51,7 @@ struct snd_usb_audio { struct list_head mixer_list; /* list of mixer interfaces */

int setup; /* from the 'device_setup' module param */ + bool generic_implicit_fb; /* from the 'implicit_fb' module param */ bool autoclock; /* from the 'autoclock' module param */

struct usb_host_interface *ctrl_intf; /* the audio control interface */

A few other BOSS devices (BR-80, GT-100v2, Katana) seem requiring the same quirk as BOSS GT-001, i.e. no implicit feedback for playback but tying with capture. Add and correct the corresponding quirk table entries for them.

Reported-and-tested-by: Keith Milner kamilner@superlative.org Signed-off-by: Takashi Iwai tiwai@suse.de --- sound/usb/implicit.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/usb/implicit.c b/sound/usb/implicit.c index 1d87fcdae95b..1f7fb036c5b6 100644 --- a/sound/usb/implicit.c +++ b/sound/usb/implicit.c @@ -72,10 +72,10 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = { .type = IMPLICIT_FB_FIXED, .ep_num = 0x84, .iface = 0 }, /* MOTU MicroBook II */

- /* No quirk */ - IMPLICIT_FB_SKIP_DEV(0x0582, 0x01d8), /* BOSS Katana */ - /* No quirk for playback but with capture quirk (see below) */ + IMPLICIT_FB_SKIP_DEV(0x0582, 0x0130), /* BOSS BR-80 */ + IMPLICIT_FB_SKIP_DEV(0x0582, 0x0189), /* BOSS GT-100v2 */ + IMPLICIT_FB_SKIP_DEV(0x0582, 0x01d8), /* BOSS Katana */ IMPLICIT_FB_SKIP_DEV(0x0582, 0x01e5), /* BOSS GT-001 */

{} /* terminator */ @@ -83,6 +83,9 @@ static const struct snd_usb_implicit_fb_match playback_implicit_fb_quirks[] = {

/* Implicit feedback quirk table for capture: only FIXED type */ static const struct snd_usb_implicit_fb_match capture_implicit_fb_quirks[] = { + IMPLICIT_FB_FIXED_DEV(0x0582, 0x0130, 0x0d, 0x01), /* BOSS BR-80 */ + IMPLICIT_FB_FIXED_DEV(0x0582, 0x0189, 0x0d, 0x01), /* BOSS GT-100v2 */ + IMPLICIT_FB_FIXED_DEV(0x0582, 0x01d8, 0x0d, 0x01), /* BOSS Katana */ IMPLICIT_FB_FIXED_DEV(0x0582, 0x01e5, 0x0d, 0x01), /* BOSS GT-001 */

{} /* terminator */