[alsa-devel] [FT C400,PATCH RFC,v4 00/10] M-Audio Fast Track C400
Hi,
The following patches adds support for the M-Audio Fast Track C400. This is version 4 of the patch series.
* Incoroprated the feedback from Clemens Ladisch clemens@ladisch.de: Moved the quirks structure to the right place (also in other places, though not strictly required) and removed the "set quirks" log messages. Thanks, Clemens!
* Added a patch to address the regression introduced by commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint". I believe there are no other devices that affected by this issue. Opening only the playback stream without first opening the capture stream now works with the C400.
v3: http://mailman.alsa-project.org/pipermail/alsa-devel/2012-November/057477.ht...
v2: http://mailman.alsa-project.org/pipermail/alsa-devel/2012-November/056750.ht...
This series was tested with the latest mainline tree, 3.7-rc7 (HEAD e23739b4ade80a3a7f87198f008f6c44a7cbc9fd )
Also applies against current sound-unstable (not tested). (HEAD 900b0690ee7b2a227c5db5f1642ceb67d90c49fc )
* Everything works: mixer controls, effects control, playback and capture (with the correct sampling frequency set). Note that the device doesn't really have a master control or left/right pan. It's just "emulated" in the software.
* To deal with the mixer channel IDs, limited by cmask bit length, I introduced an offset value. This allows to get rid of the shorehorning of the IDs into cmask and device-referencing in the low-level functions. Is this a good approach?
* Audio in/out seems to work good in this series, mainly because the playback is an implicit feedback endpoint, and it is now configured with the capture endpoint as its sync-buddy. Also needed to correct the calculation, because of the different channel numbers between the two endpoints on the C400 (6 playback - implicit feedback, 4 capture).
* The clock sources are still named "Unit 129" (0x81, internal) and "Unit 130" (0x82, SPDIF). I'll fix this later. I also plan to fix the channel names (i.e., instead of "AIn3", use "SPDIF In L").
* I've touched one FTU mixer creation function. I'd appreciate if someone can test this with the FTU to make sure I didn't break something. (I haven't gotten any feedback on that one yet)
Cheers, Eldad
Signed-off-by: Eldad Zack eldad@fogrefinery.com
Eldad Zack (10): usb-audio: replace hardcoded value with const usb-audio: correct sync endpoint initialization usb-audio: use sender stride for implicit feedback usb-audio: add control index offset usb-audio: skip UAC2 EFFECT_UNIT usb-audio: parameterize FTU effect unit control usb-audio: M-Audio Fast Track C400 quirks table usb-audio: Fast Track C400 mixer ranges usb-audio: Fast Track C400 mixer controls usb-audio: FT C400 sync playback EP to capture EP
sound/usb/endpoint.c | 9 ++- sound/usb/mixer.c | 44 +++++++++- sound/usb/mixer.h | 1 + sound/usb/mixer_quirks.c | 216 ++++++++++++++++++++++++++++++++++++++++++++-- sound/usb/pcm.c | 94 ++++++++++++++++++-- sound/usb/quirks-table.h | 71 +++++++++++++++ 6 files changed, 414 insertions(+), 21 deletions(-)
In this context, 0x01 is USB_ENDPOINT_XFER_ISOC.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/pcm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ef6fa24..ff8cbbf 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -381,7 +381,7 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) /* ... 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) != 0x01 || + if ((get_endpoint(alts, 1)->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK) != USB_ENDPOINT_XFER_ISOC || (get_endpoint(alts, 1)->bLength >= USB_DT_ENDPOINT_AUDIO_SIZE && get_endpoint(alts, 1)->bSynchAddress != 0 && !implicit_fb)) {
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 "ALSA: snd-usb: properly initialize the sync endpoint" introduced a regression for devices that has a sync endpoint with a channel number different than the data endpoint channel. Due to a different channel and period bytes count, attempting to initialize the sync endpoint will fail. With xhci it yields:
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, an audioformat with a matching number of channels is preferred. Otherwise, an audioformat will be picked randomly (i.e., first audioformat with a non-zero channel count) and the period bytes count is adjusted accordingly.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/pcm.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ff8cbbf..2158943 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -438,6 +438,76 @@ add_sync_ep: }
/* + * Configure the sync ep using the rate and pcm format of the data ep. + */ +static int configure_sync_endpoint(struct snd_usb_substream *subs) +{ + int ret; + struct list_head *p; + struct audioformat *sync_fp; + int sync_channels = 0; + int sync_period_bytes = subs->period_bytes; + int direction = subs->direction ^ 1; + struct snd_usb_substream *sync_subs = + &subs->stream->substream[direction]; + + if (sync_subs->ep_num != subs->sync_endpoint->ep_num) { + snd_printk(KERN_ERR "%s: cannot find the sync ep (direction %d, found %x, wanted %x).\n", + __func__, direction, + subs->stream->substream[direction].ep_num, + subs->sync_endpoint->ep_num); + return -EINVAL; + } + + /* + * Try to find a format with a number of channels matching + * the data substream. If no such format was found, pick one + * with non zero channel number at random. + */ + list_for_each(p, &sync_subs->fmt_list) { + struct audioformat *fp; + fp = list_entry(p, struct audioformat, list); + + if (sync_channels == 0) { + sync_fp = fp; + sync_channels = fp->channels; + } else { + if (fp->channels == subs->channels) { + sync_channels = fp->channels; + sync_fp = fp; + } + } + } + + if (sync_channels < 1) { + snd_printk(KERN_ERR "%s: no valid audioformat for sync ep %x found\n", + __func__, sync_subs->ep_num); + return -EINVAL; + } + + /* + * Recalculated the period bytes if channel number differ between + * data and sync ep. + */ + if (sync_channels != subs->channels) { + sync_period_bytes = (subs->period_bytes / subs->channels) * + sync_channels; + snd_printdd(KERN_INFO "%s: sync ep period bytes recalc %d -> %d\n", + __func__, subs->period_bytes, sync_period_bytes); + } + + ret = snd_usb_endpoint_set_params(subs->sync_endpoint, + subs->pcm_format, + sync_channels, + sync_period_bytes, + subs->cur_rate, + sync_fp, + NULL); + + return ret; +} + +/* * configure endpoint params * * called during initial setup and upon resume @@ -459,13 +529,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint) - ret = snd_usb_endpoint_set_params(subs->sync_endpoint, - subs->pcm_format, - subs->channels, - subs->period_bytes, - subs->cur_rate, - subs->cur_audiofmt, - NULL); + ret = configure_sync_endpoint(subs); + return ret; }
At Wed, 28 Nov 2012 23:55:33 +0100, Eldad Zack wrote:
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 "ALSA: snd-usb: properly initialize the sync endpoint" introduced a regression for devices that has a sync endpoint with a channel number different than the data endpoint channel. Due to a different channel and period bytes count, attempting to initialize the sync endpoint will fail. With xhci it yields:
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, an audioformat with a matching number of channels is preferred. Otherwise, an audioformat will be picked randomly (i.e., first audioformat with a non-zero channel count) and the period bytes count is adjusted accordingly.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com
sound/usb/pcm.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 72 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ff8cbbf..2158943 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -438,6 +438,76 @@ add_sync_ep: }
/*
- Configure the sync ep using the rate and pcm format of the data ep.
- */
+static int configure_sync_endpoint(struct snd_usb_substream *subs) +{
- int ret;
- struct list_head *p;
- struct audioformat *sync_fp;
- int sync_channels = 0;
- int sync_period_bytes = subs->period_bytes;
- int direction = subs->direction ^ 1;
Better to rename for avoiding misunderstanding. "direction" sounds like the direction of this substream.
- struct snd_usb_substream *sync_subs =
&subs->stream->substream[direction];- if (sync_subs->ep_num != subs->sync_endpoint->ep_num) {
snd_printk(KERN_ERR "%s: cannot find the sync ep (direction %d, found %x, wanted %x).\n",__func__, direction,subs->stream->substream[direction].ep_num,subs->sync_endpoint->ep_num);return -EINVAL;- }
This sanity check would suit better where the implicit_fb is checked. It doesn't have to be performed at each time.
And, the whole restriction you introduced is only for implicit fb case, no?
- /*
* Try to find a format with a number of channels matching* the data substream. If no such format was found, pick one* with non zero channel number at random.*/- list_for_each(p, &sync_subs->fmt_list) {
Cleaner to use list_for_each_entry(). (Yes, there are other codes using list_for_each() there, but remember how old the usb-audio driver code is :)
struct audioformat *fp;fp = list_entry(p, struct audioformat, list);if (sync_channels == 0) {sync_fp = fp;sync_channels = fp->channels;} else {if (fp->channels == subs->channels) {sync_channels = fp->channels;sync_fp = fp;}
Can be simplified like
if (!sync_channels || fp->channels == sub->channels) { sync_channels = fp->channels; sync_fp = fp; }
But, I wonder whether you don't have to check other parameters such as rate, format, etc?
thanks,
Takashi
}- }
- if (sync_channels < 1) {
snd_printk(KERN_ERR "%s: no valid audioformat for sync ep %x found\n",__func__, sync_subs->ep_num);return -EINVAL;- }
- /*
* Recalculated the period bytes if channel number differ between* data and sync ep.*/- if (sync_channels != subs->channels) {
sync_period_bytes = (subs->period_bytes / subs->channels) *sync_channels;snd_printdd(KERN_INFO "%s: sync ep period bytes recalc %d -> %d\n",__func__, subs->period_bytes, sync_period_bytes);- }
- ret = snd_usb_endpoint_set_params(subs->sync_endpoint,
subs->pcm_format,sync_channels,sync_period_bytes,subs->cur_rate,sync_fp,NULL);- return ret;
+}
+/*
- configure endpoint params
- called during initial setup and upon resume
@@ -459,13 +529,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint)
ret = snd_usb_endpoint_set_params(subs->sync_endpoint,subs->pcm_format,subs->channels,subs->period_bytes,subs->cur_rate,subs->cur_audiofmt,NULL);
ret = configure_sync_endpoint(subs);- return ret;
}
-- 1.7.8.6
Hi Takashi,
On Thu, 29 Nov 2012, Takashi Iwai wrote:
At Wed, 28 Nov 2012 23:55:33 +0100, Eldad Zack wrote:
/*
- Configure the sync ep using the rate and pcm format of the data ep.
- */
+static int configure_sync_endpoint(struct snd_usb_substream *subs) +{
- int ret;
- struct list_head *p;
- struct audioformat *sync_fp;
- int sync_channels = 0;
- int sync_period_bytes = subs->period_bytes;
- int direction = subs->direction ^ 1;
Better to rename for avoiding misunderstanding. "direction" sounds like the direction of this substream.
Thanks for the pointer. I ended up getting rid rid of it altogether, since the following is redundant:
- struct snd_usb_substream *sync_subs =
&subs->stream->substream[direction];- if (sync_subs->ep_num != subs->sync_endpoint->ep_num) {
snd_printk(KERN_ERR "%s: cannot find the sync ep (direction %d, found %x, wanted %x).\n",__func__, direction,subs->stream->substream[direction].ep_num,subs->sync_endpoint->ep_num);return -EINVAL;- }
This sanity check would suit better where the implicit_fb is checked. It doesn't have to be performed at each time.
And, the whole restriction you introduced is only for implicit fb case, no?
Yes, I took a closer look and saw that the only way for the sync_endpoint member to be at all initialized is when it is 1) implicit fb, 2) in the same stream as the data and in the opposite direction, so there's no point in checking that at all.
- /*
* Try to find a format with a number of channels matching* the data substream. If no such format was found, pick one* with non zero channel number at random.*/- list_for_each(p, &sync_subs->fmt_list) {
Cleaner to use list_for_each_entry(). (Yes, there are other codes using list_for_each() there, but remember how old the usb-audio driver code is :)
Thanks again, it looks more concise that way. I'll see if it is possible to clean up other spots the same way and post later.
Can be simplified like
if (!sync_channels || fp->channels == sub->channels) { sync_channels = fp->channels; sync_fp = fp; }But, I wonder whether you don't have to check other parameters such as rate, format, etc?
Yes, especially since both rate and format will influence the period bytes count and then the usb host driver will error out. It is unlikely that such a configuration exist, which doesn't support the same rate and PCM format in both direction, but it is worth checking.
Patch follows.
Cheers, Eldad
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint with a channel number different than the data endpoint channel from functioning. Due to a different channel and period bytes count. attempting to initialize the sync endpoint will fail at the usb host driver: (example, when using xhci:)
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is perferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the PCM audio format and the rate must be supported by the selected audioformat.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/pcm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5d3cf85..e539a5a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -451,6 +451,103 @@ add_sync_ep: }
/* + * 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 audioformat *fp, + struct audioformat *match, int rate, + snd_pcm_format_t pcm_format) +{ + int i; + int score = 0; + + if (fp->channels < 1) { + snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp); + return 0; + } + + if (!(fp->formats && pcm_format)) { + snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__, + fp, pcm_format); + return 0; + } + + for (i = 0; i < 4; i++) { + if (fp->rate_table[i] == rate) { + score++; + break; + } + } + if (!score) { + snd_printdd("%s: (fmt @%p) no match for rate %d\n", __func__, + fp, rate); + return 0; + } + + if (fp->channels == match->channels) + score++; + + snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, 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) +{ + int ret; + 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]; + + /* Try to find the best matching audioformat. */ + list_for_each_entry(fp, &sync_subs->fmt_list, list) { + int score = match_endpoint_audioformats(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)) { + snd_printk(KERN_ERR "%s: no valid audioformat for sync ep %x found\n", + __func__, sync_subs->ep_num); + return -EINVAL; + } + + /* + * Recalculated 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; + snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n", + __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, + subs->cur_rate, + sync_fp, + NULL); + + return ret; +} + +/* * configure endpoint params * * called during initial setup and upon resume @@ -472,13 +569,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint) - ret = snd_usb_endpoint_set_params(subs->sync_endpoint, - subs->pcm_format, - subs->channels, - subs->period_bytes, - subs->cur_rate, - subs->cur_audiofmt, - NULL); + ret = configure_sync_endpoint(subs); + return ret; }
At Sat, 1 Dec 2012 23:50:26 +0100, Eldad Zack wrote:
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint with a channel number different than the data endpoint channel from functioning. Due to a different channel and period bytes count. attempting to initialize the sync endpoint will fail at the usb host driver: (example, when using xhci:)
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is perferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the PCM audio format and the rate must be supported by the selected audioformat.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com
sound/usb/pcm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5d3cf85..e539a5a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -451,6 +451,103 @@ add_sync_ep: }
/*
- 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 audioformat *fp,
- struct audioformat *match, int rate,
- snd_pcm_format_t pcm_format)
+{
- int i;
- int score = 0;
- if (fp->channels < 1) {
snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp);return 0;- }
- if (!(fp->formats && pcm_format)) {
snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__,fp, pcm_format);return 0;- }
- for (i = 0; i < 4; i++) {
Use fp->nr_rates instead of 4. Otherwise looks good to me.
thanks,
Takashi
if (fp->rate_table[i] == rate) {score++;break;}- }
- if (!score) {
snd_printdd("%s: (fmt @%p) no match for rate %d\n", __func__,fp, rate);return 0;- }
- if (fp->channels == match->channels)
score++;- snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, 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) +{
- int ret;
- 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];- /* Try to find the best matching audioformat. */
- list_for_each_entry(fp, &sync_subs->fmt_list, list) {
int score = match_endpoint_audioformats(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)) {
snd_printk(KERN_ERR "%s: no valid audioformat for sync ep %x found\n",__func__, sync_subs->ep_num);return -EINVAL;- }
- /*
* Recalculated 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;snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n",__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,subs->cur_rate,sync_fp,NULL);- return ret;
+}
+/*
- configure endpoint params
- called during initial setup and upon resume
@@ -472,13 +569,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint)
ret = snd_usb_endpoint_set_params(subs->sync_endpoint,subs->pcm_format,subs->channels,subs->period_bytes,subs->cur_rate,subs->cur_audiofmt,NULL);
ret = configure_sync_endpoint(subs);- return ret;
}
-- 1.7.8.6
On 01.12.2012 23:50, Eldad Zack wrote:
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint with a channel number different than the data endpoint channel from functioning. Due to a different channel and period bytes count. attempting to initialize the sync endpoint will fail at the usb host driver: (example, when using xhci:)
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is perferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the PCM audio format and the rate must be supported by the selected audioformat.
Thanks for fixing this up, and sorry for the late response on this.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com
sound/usb/pcm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5d3cf85..e539a5a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -451,6 +451,103 @@ add_sync_ep: }
/*
- 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 audioformat *fp,
- struct audioformat *match, int rate,
- snd_pcm_format_t pcm_format)
+{
- int i;
- int score = 0;
- if (fp->channels < 1) {
snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp);return 0;- }
- if (!(fp->formats && pcm_format)) {
That should be a binary &, no?
Otherwise, together with the fix Takashi suggested, looks good to me as well.
Thanks, Daniel
snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__,fp, pcm_format);return 0;- }
- for (i = 0; i < 4; i++) {
if (fp->rate_table[i] == rate) {score++;break;}- }
- if (!score) {
snd_printdd("%s: (fmt @%p) no match for rate %d\n", __func__,fp, rate);return 0;- }
- if (fp->channels == match->channels)
score++;- snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, 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) +{
- int ret;
- 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];- /* Try to find the best matching audioformat. */
- list_for_each_entry(fp, &sync_subs->fmt_list, list) {
int score = match_endpoint_audioformats(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)) {
snd_printk(KERN_ERR "%s: no valid audioformat for sync ep %x found\n",__func__, sync_subs->ep_num);return -EINVAL;- }
- /*
* Recalculated 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;snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n",__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,subs->cur_rate,sync_fp,NULL);- return ret;
+}
+/*
- configure endpoint params
- called during initial setup and upon resume
@@ -472,13 +569,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint)
ret = snd_usb_endpoint_set_params(subs->sync_endpoint,subs->pcm_format,subs->channels,subs->period_bytes,subs->cur_rate,subs->cur_audiofmt,NULL);
ret = configure_sync_endpoint(subs);- return ret;
}
At Mon, 03 Dec 2012 10:34:29 +0100, Daniel Mack wrote:
On 01.12.2012 23:50, Eldad Zack wrote:
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint with a channel number different than the data endpoint channel from functioning. Due to a different channel and period bytes count. attempting to initialize the sync endpoint will fail at the usb host driver: (example, when using xhci:)
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is perferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the PCM audio format and the rate must be supported by the selected audioformat.
Thanks for fixing this up, and sorry for the late response on this.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com
sound/usb/pcm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5d3cf85..e539a5a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -451,6 +451,103 @@ add_sync_ep: }
/*
- 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 audioformat *fp,
- struct audioformat *match, int rate,
- snd_pcm_format_t pcm_format)
+{
- int i;
- int score = 0;
- if (fp->channels < 1) {
snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp);return 0;- }
- if (!(fp->formats && pcm_format)) {
That should be a binary &, no?
Oh yeah, this must be
if (!(fp->formats && (1ULL << pcm_format)))
Takashi
Otherwise, together with the fix Takashi suggested, looks good to me as well.
Thanks, Daniel
snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__,fp, pcm_format);return 0;- }
- for (i = 0; i < 4; i++) {
if (fp->rate_table[i] == rate) {score++;break;}- }
- if (!score) {
snd_printdd("%s: (fmt @%p) no match for rate %d\n", __func__,fp, rate);return 0;- }
- if (fp->channels == match->channels)
score++;- snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, 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) +{
- int ret;
- 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];- /* Try to find the best matching audioformat. */
- list_for_each_entry(fp, &sync_subs->fmt_list, list) {
int score = match_endpoint_audioformats(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)) {
snd_printk(KERN_ERR "%s: no valid audioformat for sync ep %x found\n",__func__, sync_subs->ep_num);return -EINVAL;- }
- /*
* Recalculated 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;snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n",__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,subs->cur_rate,sync_fp,NULL);- return ret;
+}
+/*
- configure endpoint params
- called during initial setup and upon resume
@@ -472,13 +569,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint)
ret = snd_usb_endpoint_set_params(subs->sync_endpoint,subs->pcm_format,subs->channels,subs->period_bytes,subs->cur_rate,subs->cur_audiofmt,NULL);
ret = configure_sync_endpoint(subs);- return ret;
}
Hi Daniel, Takashi,
On Mon, 3 Dec 2012, Takashi Iwai wrote:
At Mon, 03 Dec 2012 10:34:29 +0100, Daniel Mack wrote:
On 01.12.2012 23:50, Eldad Zack wrote:
Thanks for fixing this up, and sorry for the late response on this.
It was fun to write, so I'm glad you were busy with other things :)
- if (!(fp->formats && pcm_format)) {
That should be a binary &, no?
Oh yeah, this must be
if (!(fp->formats && (1ULL << pcm_format)))
Oh, I see now I managed to somehow concoct two bugs in one line. At least it worked because of the logical & ...
Fixed now, and of course the nr_rates. I'll test and submit soon.
Many thanks to both of you, I appreciate it!
Cheers, Eldad
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint, with a channel number different than that of the data endpoint, from functioning. Due to a different channel and period bytes count, attempting to initialize the sync endpoint will fail at the usb host driver. For example, when using xhci:
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is preferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the requested PCM audio format and the rate must be supported by the selected audioformat.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/pcm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5d3cf85..ec0318c 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -451,6 +451,103 @@ add_sync_ep: }
/* + * 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 audioformat *fp, + struct audioformat *match, int rate, + snd_pcm_format_t pcm_format) +{ + int i; + int score = 0; + + if (fp->channels < 1) { + snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp); + return 0; + } + + if (!(fp->formats & (1ULL << pcm_format))) { + snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__, + fp, pcm_format); + return 0; + } + + for (i = 0; i < fp->nr_rates; i++) { + if (fp->rate_table[i] == rate) { + score++; + break; + } + } + if (!score) { + snd_printdd("%s: (fmt @%p) no match for rate %d\n", __func__, + fp, rate); + return 0; + } + + if (fp->channels == match->channels) + score++; + + snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, 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) +{ + int ret; + 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]; + + /* Try to find the best matching audioformat. */ + list_for_each_entry(fp, &sync_subs->fmt_list, list) { + int score = match_endpoint_audioformats(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)) { + snd_printk(KERN_ERR "%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; + snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n", + __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, + subs->cur_rate, + sync_fp, + NULL); + + return ret; +} + +/* * configure endpoint params * * called during initial setup and upon resume @@ -472,13 +569,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint) - ret = snd_usb_endpoint_set_params(subs->sync_endpoint, - subs->pcm_format, - subs->channels, - subs->period_bytes, - subs->cur_rate, - subs->cur_audiofmt, - NULL); + ret = configure_sync_endpoint(subs); + return ret; }
At Mon, 3 Dec 2012 20:30:09 +0100, Eldad Zack wrote:
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint, with a channel number different than that of the data endpoint, from functioning. Due to a different channel and period bytes count, attempting to initialize the sync endpoint will fail at the usb host driver. For example, when using xhci:
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is preferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the requested PCM audio format and the rate must be supported by the selected audioformat.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com
Thanks, applied.
At the next time, however, create a patch based on either for-next or for-linus branch of sound git tree. sound-unstable tree is a place to play with experimental patches. The place to put the real patches that will be fed to the upstream is sound git tree.
Takashi
sound/usb/pcm.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 5d3cf85..ec0318c 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -451,6 +451,103 @@ add_sync_ep: }
/*
- 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 audioformat *fp,
- struct audioformat *match, int rate,
- snd_pcm_format_t pcm_format)
+{
- int i;
- int score = 0;
- if (fp->channels < 1) {
snd_printdd("%s: (fmt @%p) no channels\n", __func__, fp);return 0;- }
- if (!(fp->formats & (1ULL << pcm_format))) {
snd_printdd("%s: (fmt @%p) no match for format %d\n", __func__,fp, pcm_format);return 0;- }
- for (i = 0; i < fp->nr_rates; i++) {
if (fp->rate_table[i] == rate) {score++;break;}- }
- if (!score) {
snd_printdd("%s: (fmt @%p) no match for rate %d\n", __func__,fp, rate);return 0;- }
- if (fp->channels == match->channels)
score++;- snd_printdd("%s: (fmt @%p) score %d\n", __func__, fp, 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) +{
- int ret;
- 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];- /* Try to find the best matching audioformat. */
- list_for_each_entry(fp, &sync_subs->fmt_list, list) {
int score = match_endpoint_audioformats(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)) {
snd_printk(KERN_ERR "%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;snd_printdd("%s: adjusted sync ep period bytes (%d -> %d)\n",__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,subs->cur_rate,sync_fp,NULL);- return ret;
+}
+/*
- configure endpoint params
- called during initial setup and upon resume
@@ -472,13 +569,8 @@ static int configure_endpoint(struct snd_usb_substream *subs) return ret;
if (subs->sync_endpoint)
ret = snd_usb_endpoint_set_params(subs->sync_endpoint,subs->pcm_format,subs->channels,subs->period_bytes,subs->cur_rate,subs->cur_audiofmt,NULL);
ret = configure_sync_endpoint(subs);- return ret;
}
-- 1.7.8.6
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Tue, 4 Dec 2012, Takashi Iwai wrote:
At Mon, 3 Dec 2012 20:30:09 +0100, Eldad Zack wrote:
Commit 947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint", while correcting the initialization of the sync endpoint when opening just the data endpoint, prevents devices that has a sync endpoint, with a channel number different than that of the data endpoint, from functioning. Due to a different channel and period bytes count, attempting to initialize the sync endpoint will fail at the usb host driver. For example, when using xhci:
cannot submit urb 0, error -90: internal error
With this patch, if a sync endpoint has multiple audioformats, a matching audioformat is preferred. An audioformat must be found with at least one channel and support the requested sample rate and PCM format, otherwise the stream will not be opened.
If the number of channels differ between the selected audioformat and the requested format, adjust the period bytes count accordingly. It is safe to perform the calculation on the basis of the channel count, since the requested PCM audio format and the rate must be supported by the selected audioformat.
Cc: Jeffrey Barish jeff_barish@earthlink.net Cc: Daniel Mack zonque@gmail.com Signed-off-by: Eldad Zack eldad@fogrefinery.com
Thanks, applied.
Great!
At the next time, however, create a patch based on either for-next or for-linus branch of sound git tree. sound-unstable tree is a place to play with experimental patches. The place to put the real patches that will be fed to the upstream is sound git tree.
Ah, sorry, I mixed up unstable with to-next then. Thanks for the heads up!
Cheers, Eldad
For implicit feedback endpoints, the number of bytes for each packet is matched by the corresponding synchronizing endpoint. The size is calculated by taking the actual size and dividing it by the stride - currently by the endpoint's stride, but we should use the synchronization source's stride. This is evident when the number of channels differ between the synchronization source and the implicitly fed-back endpoint, as with M-Audio Fast Track C400 - the synchronization source (capture) has 4 channels, while the implicit feedback mode endpoint has 6.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/endpoint.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 34de6f2..72e711c 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -1034,15 +1034,18 @@ void snd_usb_handle_sync_urb(struct snd_usb_endpoint *ep, /* * Iterate through the inbound packet and prepare the lengths * for the output packet. The OUT packet we are about to send - * will have the same amount of payload bytes than the IN - * packet we just received. + * will have the same amount of payload bytes per stride as the + * IN packet we just received. Since the actual size is scaled + * by the stride, use the sender stride to calculate the length + * in case the number of channels differ between the implicitly + * fed-back endpoint and the synchronizing endpoint. */
out_packet->packets = in_ctx->packets; for (i = 0; i < in_ctx->packets; i++) { if (urb->iso_frame_desc[i].status == 0) out_packet->packet_size[i] = - urb->iso_frame_desc[i].actual_length / ep->stride; + urb->iso_frame_desc[i].actual_length / sender->stride; else out_packet->packet_size[i] = 0; }
Currently, channel IDs exceeding 31 (0x1f) cannot be used. The channel ID is derived from the cmask. Extending cmask to a 64-bit type would only allow it to go up to 63 (0x3f). Some devices have channel IDs exceeding that as well. To address that, add an offset to the mixer element which is then accounted for in the UAC set/get functions.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/mixer.c | 4 ++++ sound/usb/mixer.h | 1 + sound/usb/mixer_quirks.c | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 298070e..b0fc6ae 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -382,6 +382,8 @@ error:
static int get_ctl_value(struct usb_mixer_elem_info *cval, int request, int validx, int *value_ret) { + validx += cval->idx_off; + return (cval->mixer->protocol == UAC_VERSION_1) ? get_ctl_value_v1(cval, request, validx, value_ret) : get_ctl_value_v2(cval, request, validx, value_ret); @@ -432,6 +434,8 @@ int snd_usb_mixer_set_ctl_value(struct usb_mixer_elem_info *cval, unsigned char buf[2]; int idx = 0, val_len, err, timeout = 10;
+ validx += cval->idx_off; + if (cval->mixer->protocol == UAC_VERSION_1) { val_len = cval->val_type >= USB_MIXER_S16 ? 2 : 1; } else { /* UAC_VERSION_2 */ diff --git a/sound/usb/mixer.h b/sound/usb/mixer.h index a7f3d45..aab80df 100644 --- a/sound/usb/mixer.h +++ b/sound/usb/mixer.h @@ -43,6 +43,7 @@ struct usb_mixer_elem_info { unsigned int id; unsigned int control; /* CS or ICN (high byte) */ unsigned int cmask; /* channel mask bitmap: 0 = master */ + unsigned int idx_off; /* Control index offset */ unsigned int ch_readonly; unsigned int master_readonly; int channels; diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index ae2b714..4199b97 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -63,11 +63,12 @@ static void usb_mixer_elem_free(struct snd_kcontrol *kctl) * Since there doesn't seem to be a devices that needs a multichannel * version, we keep it mono for simplicity. */ -static int snd_create_std_mono_ctl(struct usb_mixer_interface *mixer, +static int snd_create_std_mono_ctl_offset(struct usb_mixer_interface *mixer, unsigned int unitid, unsigned int control, unsigned int cmask, int val_type, + unsigned int idx_off, const char *name, snd_kcontrol_tlv_rw_t *tlv_callback) { @@ -85,6 +86,7 @@ static int snd_create_std_mono_ctl(struct usb_mixer_interface *mixer, cval->channels = 1; cval->control = control; cval->cmask = cmask; + cval->idx_off = idx_off;
/* get_min_max() is called only for integer volumes later, * so provide a short-cut for booleans */ @@ -120,6 +122,18 @@ static int snd_create_std_mono_ctl(struct usb_mixer_interface *mixer, return 0; }
+static int snd_create_std_mono_ctl(struct usb_mixer_interface *mixer, + unsigned int unitid, + unsigned int control, + unsigned int cmask, + int val_type, + const char *name, + snd_kcontrol_tlv_rw_t *tlv_callback) +{ + return snd_create_std_mono_ctl_offset(mixer, unitid, control, cmask, + val_type, 0 /* Offset */, name, tlv_callback); +} + /* * Create a set of standard UAC controls from a table */
Current code mishandles the case where the device is a UAC2 and the bDescriptorSubtype is a UAC2 Effect Unit (0x07). It tries to parse it as a Processing Unit (which is similar to two other UAC1 units with overlapping subtypes), but since the structure is different (See: 4.7.2.10, 4.7.2.11 in UAC2 standard), the parsing is done incorrectly and prevents the device from initializing. For now, just ignore the unit.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/mixer.c | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index b0fc6ae..4eacbe2 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -723,8 +723,19 @@ static int check_input_term(struct mixer_build *state, int id, struct usb_audio_ return 0; } case UAC1_PROCESSING_UNIT: - case UAC1_EXTENSION_UNIT: { + case UAC1_EXTENSION_UNIT: + /* UAC2_PROCESSING_UNIT_V2 */ + /* UAC2_EFFECT_UNIT */ { struct uac_processing_unit_descriptor *d = p1; + + if (state->mixer->protocol == UAC_VERSION_2 && + hdr[2] == UAC2_EFFECT_UNIT) { + /* UAC2/UAC1 unit IDs overlap here in an + * uncompatible way. Ignore this unit for now. + */ + return 0; + } + if (d->bNrInPins) { id = d->baSourceID[0]; break; /* continue to parse */
Adds the unit ID and the control as parameters to the creation of the effect unit control for the M-Audio Fast Track Ultra. This allows the code to be shared with other devices that use different unit ID and control, such as the M-Audio Fast Track C400.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/mixer_quirks.c | 24 ++++++++++++++++-------- 1 files changed, 16 insertions(+), 8 deletions(-)
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index 4199b97..a614dab 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -635,11 +635,13 @@ static int snd_nativeinstruments_create_mixer(struct usb_mixer_interface *mixer, }
/* M-Audio FastTrack Ultra quirks */ -/* FTU Effect switch */ +/* FTU Effect switch (also used by C400) */ struct snd_ftu_eff_switch_priv_val { struct usb_mixer_interface *mixer; int cached_value; int is_cached; + int bUnitID; + int validx; };
static int snd_ftu_eff_switch_info(struct snd_kcontrol *kcontrol, @@ -674,9 +676,8 @@ static int snd_ftu_eff_switch_get(struct snd_kcontrol *kctl, struct snd_ftu_eff_switch_priv_val *pval; int err; unsigned char value[2]; + int id, validx;
- const int id = 6; - const int validx = 1; const int val_len = 2;
value[0] = 0x00; @@ -698,6 +699,8 @@ static int snd_ftu_eff_switch_get(struct snd_kcontrol *kctl, if (snd_BUG_ON(!chip)) return -EINVAL;
+ id = pval->bUnitID; + validx = pval->validx;
down_read(&mixer->chip->shutdown_rwsem); if (mixer->chip->shutdown) @@ -728,10 +731,8 @@ static int snd_ftu_eff_switch_put(struct snd_kcontrol *kctl, struct usb_mixer_interface *mixer; int changed, cur_val, err, new_val; unsigned char value[2]; + int id, validx;
- - const int id = 6; - const int validx = 1; const int val_len = 2;
changed = 0; @@ -749,6 +750,9 @@ static int snd_ftu_eff_switch_put(struct snd_kcontrol *kctl, if (snd_BUG_ON(!chip)) return -EINVAL;
+ id = pval->bUnitID; + validx = pval->validx; + if (!pval->is_cached) { /* Read current value */ down_read(&mixer->chip->shutdown_rwsem); @@ -793,7 +797,8 @@ static int snd_ftu_eff_switch_put(struct snd_kcontrol *kctl, return changed; }
-static int snd_ftu_create_effect_switch(struct usb_mixer_interface *mixer) +static int snd_ftu_create_effect_switch(struct usb_mixer_interface *mixer, + int validx, int bUnitID) { static struct snd_kcontrol_new template = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, @@ -816,6 +821,8 @@ static int snd_ftu_create_effect_switch(struct usb_mixer_interface *mixer) pval->cached_value = 0; pval->is_cached = 0; pval->mixer = mixer; + pval->bUnitID = bUnitID; + pval->validx = validx;
template.private_value = (unsigned long) pval; kctl = snd_ctl_new1(&template, mixer->chip); @@ -974,9 +981,10 @@ static int snd_ftu_create_mixer(struct usb_mixer_interface *mixer) if (err < 0) return err;
- err = snd_ftu_create_effect_switch(mixer); + err = snd_ftu_create_effect_switch(mixer, 1, 6); if (err < 0) return err; + err = snd_ftu_create_effect_volume_ctl(mixer); if (err < 0) return err;
Adds a quirks table for the M-Audio Fast Track C400. Thanks to Clemens Ladisch clemens@ladisch.de for pointing out that the table must be sorted.
Based on the following patch from the alsa-devel list: http://mailman.alsa-project.org/pipermail/alsa-devel/2012-May/051676.html
See also: http://mailman.alsa-project.org/pipermail/alsa-devel/2012-April/051219.html
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/quirks-table.h | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 71 insertions(+), 0 deletions(-)
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index 88d8ceb..0635801 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -2163,6 +2163,77 @@ YAMAHA_DEVICE(0x7010, "UB99"), } }, { + USB_DEVICE_VENDOR_SPEC(0x0763, 0x2030), + .driver_info = (unsigned long) &(const struct snd_usb_audio_quirk) { + /* .vendor_name = "M-Audio", */ + /* .product_name = "Fast Track C400", */ + .ifnum = QUIRK_ANY_INTERFACE, + .type = QUIRK_COMPOSITE, + .data = &(const struct snd_usb_audio_quirk[]) { + { + .ifnum = 1, + .type = QUIRK_AUDIO_STANDARD_MIXER, + }, + /* Playback */ + { + .ifnum = 2, + .type = QUIRK_AUDIO_FIXED_ENDPOINT, + .data = &(const struct audioformat) { + .formats = SNDRV_PCM_FMTBIT_S24_3LE, + .channels = 6, + .iface = 2, + .altsetting = 1, + .altset_idx = 1, + .attributes = UAC_EP_CS_ATTR_SAMPLE_RATE, + .endpoint = 0x01, + .ep_attr = 0x09, + .rates = SNDRV_PCM_RATE_44100 | + SNDRV_PCM_RATE_48000 | + SNDRV_PCM_RATE_88200 | + SNDRV_PCM_RATE_96000, + .rate_min = 44100, + .rate_max = 96000, + .nr_rates = 4, + .rate_table = (unsigned int[]) { + 44100, 48000, 88200, 96000 + }, + .clock = 0x81, + } + }, + /* Capture */ + { + .ifnum = 3, + .type = QUIRK_AUDIO_FIXED_ENDPOINT, + .data = &(const struct audioformat) { + .formats = SNDRV_PCM_FMTBIT_S24_3LE, + .channels = 4, + .iface = 3, + .altsetting = 1, + .altset_idx = 1, + .attributes = UAC_EP_CS_ATTR_SAMPLE_RATE, + .endpoint = 0x81, + .ep_attr = 0x05, + .rates = SNDRV_PCM_RATE_44100 | + SNDRV_PCM_RATE_48000 | + SNDRV_PCM_RATE_88200 | + SNDRV_PCM_RATE_96000, + .rate_min = 44100, + .rate_max = 96000, + .nr_rates = 4, + .rate_table = (unsigned int[]) { + 44100, 48000, 88200, 96000 + }, + .clock = 0x81, + } + }, + /* MIDI */ + { + .ifnum = -1 /* Interface = 4 */ + } + } + } +}, +{ USB_DEVICE_VENDOR_SPEC(0x0763, 0x2080), .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) { /* .vendor_name = "M-Audio", */
Add ranges for various Fast Track C400 controls, as observed while using the vendor's mixer control software (res values are an estimation).
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/mixer.c | 27 +++++++++++++++++++++++++++ 1 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 4eacbe2..feed500 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -806,6 +806,33 @@ static void volume_control_quirks(struct usb_mixer_elem_info *cval, struct snd_kcontrol *kctl) { switch (cval->mixer->chip->usb_id) { + case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ + if (strcmp(kctl->id.name, "Effect Duration") == 0) { + cval->min = 0x0000; + cval->max = 0xffff; + cval->res = 0x00e6; + break; + } + if (strcmp(kctl->id.name, "Effect Volume") == 0 || + strcmp(kctl->id.name, "Effect Feedback Volume") == 0) { + cval->min = 0x00; + cval->max = 0xff; + break; + } + if (strstr(kctl->id.name, "Effect Return") != NULL) { + cval->min = 0xb706; + cval->max = 0xff7b; + cval->res = 0x0073; + break; + } + if ((strstr(kctl->id.name, "Playback Volume") != NULL) || + (strstr(kctl->id.name, "Effect Send") != NULL)) { + cval->min = 0xb5fb; /* -73 dB = 0xb6ff */ + cval->max = 0xfcfe; + cval->res = 0x0073; + } + break; + case USB_ID(0x0763, 0x2081): /* M-Audio Fast Track Ultra 8R */ case USB_ID(0x0763, 0x2080): /* M-Audio Fast Track Ultra */ if (strcmp(kctl->id.name, "Effect Duration") == 0) {
Add a mixer quirks for the M-Audio Fast Track C400 and create the following:
* Volume controls * Effect Type (reusing FTU controls) * Effect Volume * Effect Send/Return * Effect Program * Effect Feedback
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/mixer_quirks.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 176 insertions(+), 0 deletions(-)
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c index a614dab..bf28a1b 100644 --- a/sound/usb/mixer_quirks.c +++ b/sound/usb/mixer_quirks.c @@ -1027,6 +1027,178 @@ void snd_emuusb_set_samplerate(struct snd_usb_audio *chip, } }
+/* M-Audio Fast Track C400 */ +/* C400 volume controls, this control needs a volume quirk, see mixer.c */ +static int snd_c400_create_vol_ctls(struct usb_mixer_interface *mixer) +{ + char name[64]; + unsigned int cmask, offset; + int out, chan, err; + + const unsigned int id = 0x40; + const int val_type = USB_MIXER_S16; + const int control = 1; + + for (chan = 0; chan < 10; chan++) { + for (out = 0; out < 6; out++) { + if (chan < 6) { + snprintf(name, sizeof(name), + "PCM%d-Out%d Playback Volume", + chan + 1, out + 1); + } else { + snprintf(name, sizeof(name), + "In%d-Out%d Playback Volume", + chan - 5, out + 1); + } + + cmask = (out == 0) ? 0 : 1 << (out - 1); + offset = chan * 6; + err = snd_create_std_mono_ctl_offset(mixer, id, control, + cmask, val_type, offset, name, + &snd_usb_mixer_vol_tlv); + if (err < 0) + return err; + } + } + + return 0; +} + +/* This control needs a volume quirk, see mixer.c */ +static int snd_c400_create_effect_volume_ctl(struct usb_mixer_interface *mixer) +{ + static const char name[] = "Effect Volume"; + const unsigned int id = 0x43; + const int val_type = USB_MIXER_U8; + const unsigned int control = 3; + const unsigned int cmask = 0; + + return snd_create_std_mono_ctl(mixer, id, control, cmask, val_type, + name, snd_usb_mixer_vol_tlv); +} + +/* This control needs a volume quirk, see mixer.c */ +static int snd_c400_create_effect_duration_ctl(struct usb_mixer_interface *mixer) +{ + static const char name[] = "Effect Duration"; + const unsigned int id = 0x43; + const int val_type = USB_MIXER_S16; + const unsigned int control = 4; + const unsigned int cmask = 0; + + return snd_create_std_mono_ctl(mixer, id, control, cmask, val_type, + name, snd_usb_mixer_vol_tlv); +} + +/* This control needs a volume quirk, see mixer.c */ +static int snd_c400_create_effect_feedback_ctl(struct usb_mixer_interface *mixer) +{ + static const char name[] = "Effect Feedback Volume"; + const unsigned int id = 0x43; + const int val_type = USB_MIXER_U8; + const unsigned int control = 5; + const unsigned int cmask = 0; + + return snd_create_std_mono_ctl(mixer, id, control, cmask, val_type, + name, NULL); +} + +static int snd_c400_create_effect_vol_ctls(struct usb_mixer_interface *mixer) +{ + char name[64]; + unsigned int cmask; + int chan, err; + + const unsigned int id = 0x42; + const int val_type = USB_MIXER_S16; + const int control = 1; + + for (chan = 0; chan < 10; chan++) { + if (chan < 6) { + snprintf(name, sizeof(name), + "Effect Send DOut%d", + chan + 1); + } else { + snprintf(name, sizeof(name), + "Effect Send AIn%d", + chan - 5); + } + + cmask = (chan == 0) ? 0 : 1 << (chan - 1); + err = snd_create_std_mono_ctl(mixer, id, control, + cmask, val_type, name, + &snd_usb_mixer_vol_tlv); + if (err < 0) + return err; + } + + return 0; +} + +static int snd_c400_create_effect_ret_vol_ctls(struct usb_mixer_interface *mixer) +{ + char name[64]; + unsigned int cmask; + int chan, err; + + const unsigned int id = 0x40; + const int val_type = USB_MIXER_S16; + const int control = 1; + const int chan_id[6] = { 0, 7, 2, 9, 4, 0xb }; + const unsigned int offset = 0x3c; + /* { 0x3c, 0x43, 0x3e, 0x45, 0x40, 0x47 } */ + + for (chan = 0; chan < 6; chan++) { + snprintf(name, sizeof(name), + "Effect Return %d", + chan + 1); + + cmask = (chan_id[chan] == 0) ? 0 : 1 << (chan_id[chan] - 1); + err = snd_create_std_mono_ctl_offset(mixer, id, control, + cmask, val_type, offset, name, + &snd_usb_mixer_vol_tlv); + if (err < 0) + return err; + } + + return 0; +} + +static int snd_c400_create_mixer(struct usb_mixer_interface *mixer) +{ + int err; + + err = snd_c400_create_vol_ctls(mixer); + if (err < 0) + return err; + + err = snd_c400_create_effect_vol_ctls(mixer); + if (err < 0) + return err; + + err = snd_c400_create_effect_ret_vol_ctls(mixer); + if (err < 0) + return err; + + err = snd_ftu_create_effect_switch(mixer, 2, 0x43); + if (err < 0) + return err; + + err = snd_c400_create_effect_volume_ctl(mixer); + if (err < 0) + return err; + + err = snd_c400_create_effect_duration_ctl(mixer); + if (err < 0) + return err; + + err = snd_c400_create_effect_feedback_ctl(mixer); + if (err < 0) + return err; + + return 0; +} + /* * The mixer units for Ebox-44 are corrupt, and even where they * are valid they presents mono controls as L and R channels of @@ -1124,6 +1296,10 @@ int snd_usb_mixer_apply_create_quirk(struct usb_mixer_interface *mixer) snd_audigy2nx_proc_read); break;
+ case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ + err = snd_c400_create_mixer(mixer); + break; + case USB_ID(0x0763, 0x2080): /* M-Audio Fast Track Ultra */ case USB_ID(0x0763, 0x2081): /* M-Audio Fast Track Ultra 8R */ err = snd_ftu_create_mixer(mixer);
The playback endpoint uses implicit feedback mode, similar to the M-Audio FTU. Like with the FTU, we need to associate the sync pipe ourselves.
Signed-off-by: Eldad Zack eldad@fogrefinery.com --- sound/usb/pcm.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 2158943..f30f46ca 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -359,6 +359,19 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt) attr = fmt->ep_attr & USB_ENDPOINT_SYNCTYPE;
switch (subs->stream->chip->usb_id) { + case USB_ID(0x0763, 0x2030): /* M-Audio Fast Track C400 */ + if (is_playback) { + implicit_fb = 1; + ep = 0x81; + iface = usb_ifnum_to_if(dev, 3); + + if (!iface || iface->num_altsetting == 0) + return -EINVAL; + + alts = &iface->altsetting[1]; + goto add_sync_ep; + } + break; case USB_ID(0x0763, 0x2080): /* M-Audio FastTrack Ultra */ case USB_ID(0x0763, 0x2081): if (is_playback) {
At Wed, 28 Nov 2012 23:55:31 +0100, Eldad Zack wrote:
Hi,
The following patches adds support for the M-Audio Fast Track C400. This is version 4 of the patch series.
- Incoroprated the feedback from Clemens Ladisch clemens@ladisch.de:
Moved the quirks structure to the right place (also in other places, though not strictly required) and removed the "set quirks" log messages. Thanks, Clemens!
- Added a patch to address the regression introduced by commit
947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint". I believe there are no other devices that affected by this issue. Opening only the playback stream without first opening the capture stream now works with the C400.
Thanks. I applied all patches except for the new one (2/10 usb-audio: correct sync ep init). This patch needs a bit more care, as it seems.
I know it must be fixed for FT C400 but the fix itself is independent from others, so I merged others first to the rest work easier.
(snip)
- To deal with the mixer channel IDs, limited by cmask bit length,
I introduced an offset value. This allows to get rid of the shorehorning of the IDs into cmask and device-referencing in the low-level functions. Is this a good approach?
Looks good enough to me. It has a minimum impact, so safe to apply.
Takashi
Hi Takashi,
On Thu, 29 Nov 2012, Takashi Iwai wrote:
At Wed, 28 Nov 2012 23:55:31 +0100, Eldad Zack wrote:
- Incoroprated the feedback from Clemens Ladisch clemens@ladisch.de:
Moved the quirks structure to the right place (also in other places, though not strictly required) and removed the "set quirks" log messages. Thanks, Clemens!
- Added a patch to address the regression introduced by commit
947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint". I believe there are no other devices that affected by this issue. Opening only the playback stream without first opening the capture stream now works with the C400.
Thanks. I applied all patches except for the new one (2/10 usb-audio: correct sync ep init). This patch needs a bit more care, as it seems.
I know it must be fixed for FT C400 but the fix itself is independent from others, so I merged others first to the rest work easier.
Thanks! That's great, now I only need to concentrate on this one issue. I went through your helpful comments and I'll try to work out a better patch soon and send it for review. Should I wait with it until the 3.8 merge window closes?
BTW - given that the code is old, isn't it time to slightly change these? :)
card.c: * (Tentative) USB Audio Driver for ALSA mixer.c: * (Tentative) USB Audio Driver for ALSA usbaudio.h: * (Tentative) USB Audio Driver for ALSA
Cheers, Eldad
At Fri, 30 Nov 2012 15:06:00 +0100 (CET), Eldad Zack wrote:
Hi Takashi,
On Thu, 29 Nov 2012, Takashi Iwai wrote:
At Wed, 28 Nov 2012 23:55:31 +0100, Eldad Zack wrote:
- Incoroprated the feedback from Clemens Ladisch clemens@ladisch.de:
Moved the quirks structure to the right place (also in other places, though not strictly required) and removed the "set quirks" log messages. Thanks, Clemens!
- Added a patch to address the regression introduced by commit
947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly initialize the sync endpoint". I believe there are no other devices that affected by this issue. Opening only the playback stream without first opening the capture stream now works with the C400.
Thanks. I applied all patches except for the new one (2/10 usb-audio: correct sync ep init). This patch needs a bit more care, as it seems.
I know it must be fixed for FT C400 but the fix itself is independent from others, so I merged others first to the rest work easier.
Thanks! That's great, now I only need to concentrate on this one issue. I went through your helpful comments and I'll try to work out a better patch soon and send it for review. Should I wait with it until the 3.8 merge window closes?
Your other patches have been already merged, so better be quick, so that the last piece can be put in to 3.8, too :)
BTW - given that the code is old, isn't it time to slightly change these? :)
card.c: * (Tentative) USB Audio Driver for ALSA mixer.c: * (Tentative) USB Audio Driver for ALSA usbaudio.h: * (Tentative) USB Audio Driver for ALSA
Yes, clean up patches are welcome. But these are in a lower priority, so will be queued for 3.9 later.
thanks,
Takashi
Sorry for being late on this. I've just tested Takashi's kernel with your patches included. Everything seems fine for the FTU 8R.
Kind regards, Felix
2012/11/30 Takashi Iwai tiwai@suse.de
At Fri, 30 Nov 2012 15:06:00 +0100 (CET), Eldad Zack wrote:
Hi Takashi,
On Thu, 29 Nov 2012, Takashi Iwai wrote:
At Wed, 28 Nov 2012 23:55:31 +0100, Eldad Zack wrote:
- Incoroprated the feedback from Clemens Ladisch <clemens@ladisch.de
:
Moved the quirks structure to the right place (also in other places,
though
not strictly required) and removed the "set quirks" log messages. Thanks, Clemens!
- Added a patch to address the regression introduced by commit
947d299686aa9cc8aecf749d54e8475c6e498956 , "ALSA: snd-usb: properly
initialize
the sync endpoint". I believe there are no other devices that
affected by this
issue. Opening only the playback stream without first opening the
capture
stream now works with the C400.
Thanks. I applied all patches except for the new one (2/10 usb-audio: correct sync ep init). This patch needs a bit more care, as it seems.
I know it must be fixed for FT C400 but the fix itself is independent from others, so I merged others first to the rest work easier.
Thanks! That's great, now I only need to concentrate on this one issue. I went through your helpful comments and I'll try to work out a better patch soon and send it for review. Should I wait with it until the 3.8 merge window closes?
Your other patches have been already merged, so better be quick, so that the last piece can be put in to 3.8, too :)
BTW - given that the code is old, isn't it time to slightly change these? :)
card.c: * (Tentative) USB Audio Driver for ALSA mixer.c: * (Tentative) USB Audio Driver for ALSA usbaudio.h: * (Tentative) USB Audio Driver for ALSA
Yes, clean up patches are welcome. But these are in a lower priority, so will be queued for 3.9 later.
thanks,
Takashi
On Fri, 7 Dec 2012, Felix Homann wrote:
Sorry for being late on this. I've just tested Takashi's kernel with your patches included. Everything seems fine for the FTU 8R.
Kind regards, Felix
That's good news, thanks for testing!
Cheers, Eldad
participants (4)
-
Daniel Mack -
Eldad Zack -
Felix Homann -
Takashi Iwai