[alsa-devel] [PATCH 0/7] ALSA: USB-audio: Support Zoom R16 playback
The Zoom R16 has a nonstandard playback format where each isochronous packet contains a length descriptor in the first four bytes. Curiously, capture data does not contain this and requires no quirk.
The quirk involves adding the extra length descriptor whenever outgoing isochronous packets are generated, both in pcm.c (outgoing audio) and endpoint.c (silent data). The calculation for the max packet size in endpoint.c is also affected.
This patch series refactors some code in pcm.c and endpoint.c before the actual quirk implementation. A commit then cleans up the entry in the quirks table.
Tested with a Zoom R16, both separately using arecord and aplay (8 channel capture and 2 channel playback, respectively), as well as capture and playback together together using the Ardour digitial audio workstation application. The R24 reportedly is compatible with the R16, but I don't have access to one so I can't test it, and consequently the patch series only mentions the R16.
Also tested using an Edirol UA-5 in both class compliant (16-bit) and "advanced" (24 bit, forces the use of quirks) modes in order to do some form of regression testing.
Ricard Wanderlof (7): Break out copying to urb from prepare_playback_urb() Also move out hwptr_done wrap from prepare_playback_urb() Break out creation of silent urbs from prepare_outbound_urb() Add offset parameter to copy_to_urb() Add quirk for Zoom R16 playback Adjust max packet size calculation for tx_length_quirk Remove mixer entry from Zoom R16/24 quirk
sound/usb/card.h | 1 + sound/usb/endpoint.c | 73 +++++++++++++++++++++++++++++++++-------------- sound/usb/pcm.c | 74 +++++++++++++++++++++++++++++++++++++----------- sound/usb/quirks-table.h | 14 +++------ sound/usb/quirks.c | 3 ++ sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 1 + 7 files changed, 119 insertions(+), 48 deletions(-)
Refactoring in preparation for adding Zoom R16 quirk.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/pcm.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index cdac517..4292bad 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1385,6 +1385,26 @@ static inline void fill_playback_urb_dsd_dop(struct snd_usb_substream *subs, } }
+static void copy_to_urb(struct snd_usb_substream *subs, + struct urb *urb, int stride, unsigned int bytes) +{ + struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime; + + if (subs->hwptr_done + bytes > runtime->buffer_size * stride) { + /* err, the transferred area goes over buffer boundary. */ + unsigned int bytes1 = + runtime->buffer_size * stride - subs->hwptr_done; + memcpy(urb->transfer_buffer, + runtime->dma_area + subs->hwptr_done, bytes1); + memcpy(urb->transfer_buffer + bytes1, + runtime->dma_area, bytes - bytes1); + } else { + memcpy(urb->transfer_buffer, + runtime->dma_area + subs->hwptr_done, bytes); + } + subs->hwptr_done += bytes; +} + static void prepare_playback_urb(struct snd_usb_substream *subs, struct urb *urb) { @@ -1462,20 +1482,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->hwptr_done += bytes; } else { /* usual PCM */ - if (subs->hwptr_done + bytes > runtime->buffer_size * stride) { - /* err, the transferred area goes over buffer boundary. */ - unsigned int bytes1 = - runtime->buffer_size * stride - subs->hwptr_done; - memcpy(urb->transfer_buffer, - runtime->dma_area + subs->hwptr_done, bytes1); - memcpy(urb->transfer_buffer + bytes1, - runtime->dma_area, bytes - bytes1); - } else { - memcpy(urb->transfer_buffer, - runtime->dma_area + subs->hwptr_done, bytes); - } - - subs->hwptr_done += bytes; + copy_to_urb(subs, urb, stride, bytes); }
if (subs->hwptr_done >= runtime->buffer_size * stride)
Refactoring in preparation for adding Zoom R16 quirk.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/pcm.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 4292bad..ee8dd9e 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1383,6 +1383,8 @@ static inline void fill_playback_urb_dsd_dop(struct snd_usb_substream *subs, subs->hwptr_done++; } } + if (subs->hwptr_done >= runtime->buffer_size * stride) + subs->hwptr_done -= runtime->buffer_size * stride; }
static void copy_to_urb(struct snd_usb_substream *subs, @@ -1403,6 +1405,8 @@ static void copy_to_urb(struct snd_usb_substream *subs, runtime->dma_area + subs->hwptr_done, bytes); } subs->hwptr_done += bytes; + if (subs->hwptr_done >= runtime->buffer_size * stride) + subs->hwptr_done -= runtime->buffer_size * stride; }
static void prepare_playback_urb(struct snd_usb_substream *subs, @@ -1480,14 +1484,13 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, }
subs->hwptr_done += bytes; + if (subs->hwptr_done >= runtime->buffer_size * stride) + subs->hwptr_done -= runtime->buffer_size * stride; } else { /* usual PCM */ copy_to_urb(subs, urb, stride, bytes); }
- if (subs->hwptr_done >= runtime->buffer_size * stride) - subs->hwptr_done -= runtime->buffer_size * stride; - /* update delay with exact number of samples queued */ runtime->delay = subs->last_delay; runtime->delay += frames;
Refactoring in preparation for adding Zoom R16 quirk.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/endpoint.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index a77d9c8..825a06c 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -183,13 +183,38 @@ static void retire_inbound_urb(struct snd_usb_endpoint *ep, ep->retire_data_urb(ep->data_subs, urb); }
+static void prepare_silent_urb(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx) +{ + struct urb *urb = ctx->urb; + unsigned int offs = 0; + int i; + + for (i = 0; i < ctx->packets; ++i) { + int counts; + + if (ctx->packet_size[i]) + counts = ctx->packet_size[i]; + else + counts = snd_usb_endpoint_next_packet_size(ep); + + urb->iso_frame_desc[i].offset = offs * ep->stride; + urb->iso_frame_desc[i].length = counts * ep->stride; + offs += counts; + } + + urb->number_of_packets = ctx->packets; + urb->transfer_buffer_length = offs * ep->stride; + memset(urb->transfer_buffer, ep->silence_value, + offs * ep->stride); +} + /* * Prepare a PLAYBACK urb for submission to the bus. */ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, struct snd_urb_ctx *ctx) { - int i; struct urb *urb = ctx->urb; unsigned char *cp = urb->transfer_buffer;
@@ -201,24 +226,7 @@ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, ep->prepare_data_urb(ep->data_subs, urb); } else { /* no data provider, so send silence */ - unsigned int offs = 0; - for (i = 0; i < ctx->packets; ++i) { - int counts; - - if (ctx->packet_size[i]) - counts = ctx->packet_size[i]; - else - counts = snd_usb_endpoint_next_packet_size(ep); - - urb->iso_frame_desc[i].offset = offs * ep->stride; - urb->iso_frame_desc[i].length = counts * ep->stride; - offs += counts; - } - - urb->number_of_packets = ctx->packets; - urb->transfer_buffer_length = offs * ep->stride; - memset(urb->transfer_buffer, ep->silence_value, - offs * ep->stride); + prepare_silent_urb(ep, ctx); } break;
Preparation for adding Zoom R16 quirk.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/pcm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ee8dd9e..e3c5bc0 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1387,8 +1387,8 @@ static inline void fill_playback_urb_dsd_dop(struct snd_usb_substream *subs, subs->hwptr_done -= runtime->buffer_size * stride; }
-static void copy_to_urb(struct snd_usb_substream *subs, - struct urb *urb, int stride, unsigned int bytes) +static void copy_to_urb(struct snd_usb_substream *subs, struct urb *urb, + int offset, int stride, unsigned int bytes) { struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime;
@@ -1396,12 +1396,12 @@ static void copy_to_urb(struct snd_usb_substream *subs, /* err, the transferred area goes over buffer boundary. */ unsigned int bytes1 = runtime->buffer_size * stride - subs->hwptr_done; - memcpy(urb->transfer_buffer, + memcpy(urb->transfer_buffer + offset, runtime->dma_area + subs->hwptr_done, bytes1); - memcpy(urb->transfer_buffer + bytes1, + memcpy(urb->transfer_buffer + offset + bytes1, runtime->dma_area, bytes - bytes1); } else { - memcpy(urb->transfer_buffer, + memcpy(urb->transfer_buffer + offset, runtime->dma_area + subs->hwptr_done, bytes); } subs->hwptr_done += bytes; @@ -1488,7 +1488,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->hwptr_done -= runtime->buffer_size * stride; } else { /* usual PCM */ - copy_to_urb(subs, urb, stride, bytes); + copy_to_urb(subs, urb, 0, stride, bytes); }
/* update delay with exact number of samples queued */
Stuff 4 byte (S32_LE) length descriptor at the start of each isochronous packet.
Tested with Zoom R16, full duplex.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/card.h | 1 + sound/usb/endpoint.c | 25 ++++++++++++++++++++----- sound/usb/pcm.c | 32 +++++++++++++++++++++++++++++++- sound/usb/quirks-table.h | 7 ++++--- sound/usb/quirks.c | 3 +++ sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 1 + 7 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/sound/usb/card.h b/sound/usb/card.h index ef580b4..71778ca 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -122,6 +122,7 @@ struct snd_usb_substream { unsigned int buffer_periods; /* current periods per buffer */ unsigned int altset_idx; /* USB data format: index of alternate setting */ unsigned int txfr_quirk:1; /* allow sub-frame alignment */ + unsigned int tx_length_quirk:1; /* add length specifier to transfers */ unsigned int fmt_type; /* USB audio format type (1-3) */ unsigned int pkt_offset_adj; /* Bytes to drop from beginning of packets (for non-compliant devices) */
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 825a06c..1c5280a 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -188,9 +188,17 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, { struct urb *urb = ctx->urb; unsigned int offs = 0; + unsigned int extra = 0; + u32 packet_length; int i;
+ /* For tx_length_quirk, put packet length at start of packet */ + if (ep->chip->tx_length_quirk) + extra = sizeof(packet_length); + for (i = 0; i < ctx->packets; ++i) { + unsigned int offset; + unsigned int length; int counts;
if (ctx->packet_size[i]) @@ -198,15 +206,22 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, else counts = snd_usb_endpoint_next_packet_size(ep);
- urb->iso_frame_desc[i].offset = offs * ep->stride; - urb->iso_frame_desc[i].length = counts * ep->stride; + length = counts * ep->stride; /* number of silent bytes */ + offset = offs * ep->stride + extra * i; + urb->iso_frame_desc[i].offset = offset; + urb->iso_frame_desc[i].length = length + extra; + if (extra) { + packet_length = cpu_to_le32(length); + memcpy(urb->transfer_buffer + offset, + &packet_length, sizeof(packet_length)); + } + memset(urb->transfer_buffer + offset + extra, + ep->silence_value, length); offs += counts; }
urb->number_of_packets = ctx->packets; - urb->transfer_buffer_length = offs * ep->stride; - memset(urb->transfer_buffer, ep->silence_value, - offs * ep->stride); + urb->transfer_buffer_length = offs * ep->stride + ctx->packets * extra; }
/* diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index e3c5bc0..ec3d381 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1409,6 +1409,32 @@ static void copy_to_urb(struct snd_usb_substream *subs, struct urb *urb, subs->hwptr_done -= runtime->buffer_size * stride; }
+static unsigned int copy_to_urb_quirk(struct snd_usb_substream *subs, + struct urb *urb, int stride, + unsigned int bytes) +{ + u32 packet_length; + int i; + + /* Put U32_LE length descriptor at start of each packet. */ + for (i = 0; i < urb->number_of_packets; i++) { + unsigned int length = urb->iso_frame_desc[i].length; + unsigned int offset = urb->iso_frame_desc[i].offset; + + packet_length = cpu_to_le32(length); + offset += i * sizeof(packet_length); + urb->iso_frame_desc[i].offset = offset; + urb->iso_frame_desc[i].length += sizeof(packet_length); + memcpy(urb->transfer_buffer + offset, + &packet_length, sizeof(packet_length)); + copy_to_urb(subs, urb, offset + sizeof(packet_length), + stride, length); + } + /* Adjust transfer size accordingly. */ + bytes += urb->number_of_packets * sizeof(packet_length); + return bytes; +} + static void prepare_playback_urb(struct snd_usb_substream *subs, struct urb *urb) { @@ -1488,7 +1514,11 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->hwptr_done -= runtime->buffer_size * stride; } else { /* usual PCM */ - copy_to_urb(subs, urb, 0, stride, bytes); + if (!subs->tx_length_quirk) + copy_to_urb(subs, urb, 0, stride, bytes); + else + bytes = copy_to_urb_quirk(subs, urb, stride, bytes); + /* bytes is now amount of outgoing data */ }
/* update delay with exact number of samples queued */ diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index e475665..4a8ca637 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3184,8 +3184,9 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), * ZOOM R16/24 in audio interface mode. * Mixer descriptors are garbage, further quirks will be needed * to make any of it functional, thus disabled for now. - * Playback stream appears to start and run fine but no sound - * is produced, so also disabled for now. + * Playback requires an extra four byte LE length indicator + * at the start of each isochronous packet. This quirk is + * enabled in create_standard_audio_quirk(). */ USB_DEVICE(0x1686, 0x00dd), .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) { @@ -3200,7 +3201,7 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), { /* Playback */ .ifnum = 1, - .type = QUIRK_IGNORE_INTERFACE, + .type = QUIRK_AUDIO_STANDARD_INTERFACE, }, { /* Capture */ diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 00ebc0c..79a9540 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -115,6 +115,9 @@ static int create_standard_audio_quirk(struct snd_usb_audio *chip, struct usb_interface_descriptor *altsd; int err;
+ if (chip->usb_id == USB_ID(0x1686, 0x00dd)) /* Zoom R16 */ + chip->tx_length_quirk = 1; + alts = &iface->altsetting[0]; altsd = get_iface_desc(alts); err = snd_usb_parse_audio_interface(chip, altsd->bInterfaceNumber); diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 9700860..8ee14f2 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -92,6 +92,7 @@ static void snd_usb_init_substream(struct snd_usb_stream *as, subs->direction = stream; subs->dev = as->chip->dev; subs->txfr_quirk = as->chip->txfr_quirk; + subs->tx_length_quirk = as->chip->tx_length_quirk; subs->speed = snd_usb_get_speed(subs->dev); subs->pkt_offset_adj = 0;
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 33a1764..15a1271 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -43,6 +43,7 @@ struct snd_usb_audio { atomic_t usage_count; 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 */ int num_interfaces; int num_suspended_intf;
Hi Ricard,
[auto build test WARNING on sound/for-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/Ricard-Wanderlof/ALSA-USB-audio-Sup... reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
sound/usb/endpoint.c:214:39: sparse: incorrect type in assignment (different base types)
sound/usb/endpoint.c:214:39: expected unsigned int [unsigned] [usertype] packet_length sound/usb/endpoint.c:214:39: got restricted __le32 [usertype] <noident> --
sound/usb/pcm.c:1424:31: sparse: incorrect type in assignment (different base types)
sound/usb/pcm.c:1424:31: expected unsigned int [unsigned] [usertype] packet_length sound/usb/pcm.c:1424:31: got restricted __le32 [usertype] <noident>
vim +214 sound/usb/endpoint.c
198 199 for (i = 0; i < ctx->packets; ++i) { 200 unsigned int offset; 201 unsigned int length; 202 int counts; 203 204 if (ctx->packet_size[i]) 205 counts = ctx->packet_size[i]; 206 else 207 counts = snd_usb_endpoint_next_packet_size(ep); 208 209 length = counts * ep->stride; /* number of silent bytes */ 210 offset = offs * ep->stride + extra * i; 211 urb->iso_frame_desc[i].offset = offset; 212 urb->iso_frame_desc[i].length = length + extra; 213 if (extra) {
214 packet_length = cpu_to_le32(length);
215 memcpy(urb->transfer_buffer + offset, 216 &packet_length, sizeof(packet_length)); 217 } 218 memset(urb->transfer_buffer + offset + extra, 219 ep->silence_value, length); 220 offs += counts; 221 } 222
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/endpoint.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 1c5280a..2acc603 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -617,6 +617,8 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, 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 && + usb_pipeout(ep->pipe));
if (pcm_format == SNDRV_PCM_FORMAT_DSD_U16_LE && fmt->dsd_dop) { /* @@ -650,11 +652,17 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, */ maxsize = (((ep->freqmax << ep->datainterval) + 0xffff) >> 16) * (frame_bits >> 3); + if (tx_length_quirk) + maxsize += sizeof(u32); /* Space for u32 length descriptor */ /* but wMaxPacketSize might reduce this */ if (ep->maxpacksize && ep->maxpacksize < maxsize) { /* whatever fits into a max. size packet */ - maxsize = ep->maxpacksize; - ep->freqmax = (maxsize / (frame_bits >> 3)) + unsigned int data_maxsize = maxsize = ep->maxpacksize; + + if (tx_length_quirk) + /* Need to remove the length descriptor to calc freq */ + data_maxsize -= sizeof(u32); + ep->freqmax = (data_maxsize / (frame_bits >> 3)) << (16 - ep->datainterval); }
The device has no mixer (and identifies itself as such), so just skip the mixer definition.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/quirks-table.h | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index 4a8ca637..722b77b 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3182,8 +3182,6 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), { /* * ZOOM R16/24 in audio interface mode. - * Mixer descriptors are garbage, further quirks will be needed - * to make any of it functional, thus disabled for now. * Playback requires an extra four byte LE length indicator * at the start of each isochronous packet. This quirk is * enabled in create_standard_audio_quirk(). @@ -3194,11 +3192,6 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), .type = QUIRK_COMPOSITE, .data = (const struct snd_usb_audio_quirk[]) { { - /* Mixer */ - .ifnum = 0, - .type = QUIRK_IGNORE_INTERFACE, - }, - { /* Playback */ .ifnum = 1, .type = QUIRK_AUDIO_STANDARD_INTERFACE,
On Thu, Oct 15, 2015 at 12:57:30PM +0200, Ricard Wanderlof wrote:
The Zoom R16 has a nonstandard playback format where each isochronous packet contains a length descriptor in the first four bytes. Curiously, capture data does not contain this and requires no quirk. [...]
For the series,
Reviewed-by: James Cameron quozl@laptop.org
On Thu, 15 Oct 2015 12:57:30 +0200, Ricard Wanderlof wrote:
The Zoom R16 has a nonstandard playback format where each isochronous packet contains a length descriptor in the first four bytes. Curiously, capture data does not contain this and requires no quirk.
The quirk involves adding the extra length descriptor whenever outgoing isochronous packets are generated, both in pcm.c (outgoing audio) and endpoint.c (silent data). The calculation for the max packet size in endpoint.c is also affected.
This patch series refactors some code in pcm.c and endpoint.c before the actual quirk implementation. A commit then cleans up the entry in the quirks table.
Tested with a Zoom R16, both separately using arecord and aplay (8 channel capture and 2 channel playback, respectively), as well as capture and playback together together using the Ardour digitial audio workstation application. The R24 reportedly is compatible with the R16, but I don't have access to one so I can't test it, and consequently the patch series only mentions the R16.
Also tested using an Edirol UA-5 in both class compliant (16-bit) and "advanced" (24 bit, forces the use of quirks) modes in order to do some form of regression testing.
The changes look OK, the only point to be fixed would be the use of __le32 for a variable as kbuild bot suggested. However, there are way too few description in the changelog. The patches 1-4 are only code shuffling, so it's OK (though, better to mention more explicitly that there is no functional change). But patches 5-6 are the core part, and this doesn't contain *why* this is needed. The irony is that you put so many good texts in the cover letter.
So, please make the changelogs more readable and enjoyable instead of the cover letter texts. The cover letter won't be included in the commits, after all.
thanks,
Takashi
Ricard Wanderlof (7): Break out copying to urb from prepare_playback_urb() Also move out hwptr_done wrap from prepare_playback_urb() Break out creation of silent urbs from prepare_outbound_urb() Add offset parameter to copy_to_urb() Add quirk for Zoom R16 playback Adjust max packet size calculation for tx_length_quirk Remove mixer entry from Zoom R16/24 quirk
sound/usb/card.h | 1 + sound/usb/endpoint.c | 73 +++++++++++++++++++++++++++++++++-------------- sound/usb/pcm.c | 74 +++++++++++++++++++++++++++++++++++++----------- sound/usb/quirks-table.h | 14 +++------ sound/usb/quirks.c | 3 ++ sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 1 + 7 files changed, 119 insertions(+), 48 deletions(-)
-- 2.1.4
On Fri, 16 Oct 2015, Takashi Iwai wrote:
The changes look OK, the only point to be fixed would be the use of __le32 for a variable as kbuild bot suggested.
Ok, however I interpreted the kbuild warning as that the output type of cpu_to_le32() is the same as the input type, so the resulting 'unsigned int' might not necessarily fit into the target u32 variable. So when the input type is an unsigned int, it should be cast to an u32 to be the same as the output type.
I looked around in various parts of the kernel code to see how cpu_to_le32() was used, and most cases have an u32 as the argument, and when not, the argument is simply cast to u32, i.e.
unsigned int length; u32 packet_length = cpu_to_le32((u32)length);
However, there are way too few description in the changelog. The patches 1-4 are only code shuffling, so it's OK (though, better to mention more explicitly that there is no functional change). But patches 5-6 are the core part, and this doesn't contain *why* this is needed. The irony is that you put so many good texts in the cover letter.
Same problem as with the single usb packet length fix I submitted a few days ago then. :-)
So, please make the changelogs more readable and enjoyable instead of the cover letter texts. The cover letter won't be included in the commits, after all.
Ok, I'll rework the changelogs over the next few days and resubmit the set.
Thanks!
/Ricard
On Fri, 16 Oct 2015 09:22:01 +0200, Ricard Wanderlof wrote:
On Fri, 16 Oct 2015, Takashi Iwai wrote:
The changes look OK, the only point to be fixed would be the use of __le32 for a variable as kbuild bot suggested.
Ok, however I interpreted the kbuild warning as that the output type of cpu_to_le32() is the same as the input type, so the resulting 'unsigned int' might not necessarily fit into the target u32 variable. So when the input type is an unsigned int, it should be cast to an u32 to be the same as the output type.
I looked around in various parts of the kernel code to see how cpu_to_le32() was used, and most cases have an u32 as the argument, and when not, the argument is simply cast to u32, i.e.
unsigned int length; u32 packet_length = cpu_to_le32((u32)length);
No, other way round: the problem is that packet_length is declared as u32 while it should be __le32. The length variable must be CPU endian, of course, as you apply C arithmetic.
Takashi
On Fri, 16 Oct 2015, Takashi Iwai wrote:
unsigned int length; u32 packet_length = cpu_to_le32((u32)length);
No, other way round: the problem is that packet_length is declared as u32 while it should be __le32. The length variable must be CPU endian, of course, as you apply C arithmetic.
So the cast shouldn't be necessary then, i.e. just simply:
unsigned int length; __le32 packet_length = cpu_to_le32(length);
?
It makes sense to me (although that doesn't necessarily make it right. :-) )
/Ricard
On Fri, 16 Oct 2015 13:20:22 +0200, Ricard Wanderlof wrote:
On Fri, 16 Oct 2015, Takashi Iwai wrote:
unsigned int length; u32 packet_length = cpu_to_le32((u32)length);
No, other way round: the problem is that packet_length is declared as u32 while it should be __le32. The length variable must be CPU endian, of course, as you apply C arithmetic.
So the cast shouldn't be necessary then, i.e. just simply:
unsigned int length; __le32 packet_length = cpu_to_le32(length);
?
It makes sense to me (although that doesn't necessarily make it right. :-) )
Right, it's the correct usage. The __le32 data converted via cpu_to_le32() means just a 32bit raw data and can't be used for arithmetic operation. It's different from integer. By annotation, we can catch such failures more easily.
Takashi
V2: Change u32 to __le32 (kbuild test robot warning). Since Zoom R24 most likely behaves as R16 when used as an audio interface, refer to "R16/24" throughout in all comments (no code change). Tested both in Linux 3.16.7 (with appropriate backports) and 4.3.0 . Made changelogs more descriptive.
The Zoom R16 has a nonstandard playback format where each isochronous packet contains a length descriptor in the first four bytes.
This patch series implements a quirk which involves adding the extra length descriptor whenever outgoing isochronous packets are generated.
The first four patches refactor some code in pcm.c and endpoint.c before the actual quirk implementation. A final commit cleans up the entry in the quirks table.
Big thanks to Panu for testing this patch.
Signed-off-by: Ricard Wanderlof ricardw@axis.com Tested-by: Panu Matilainen pmatilai@laiskiainen.org
Ricard Wanderlof (7): Break out copying to urb from prepare_playback_urb() Also move out hwptr_done wrap from prepare_playback_urb() Break out creation of silent urbs from prepare_outbound_urb() Add offset parameter to copy_to_urb() Add quirk for Zoom R16/24 playback Adjust max packet size calculation for tx_length_quirk Remove mixer entry from Zoom R16/24 quirk
sound/usb/card.h | 1 + sound/usb/endpoint.c | 73 +++++++++++++++++++++++++++++++++-------------- sound/usb/pcm.c | 74 +++++++++++++++++++++++++++++++++++++----------- sound/usb/quirks-table.h | 14 +++------ sound/usb/quirks.c | 3 ++ sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 1 + 7 files changed, 119 insertions(+), 48 deletions(-)
Refactoring in preparation for adding Zoom R16/24 quirk. No functional change.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/pcm.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index cdac517..4292bad 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1385,6 +1385,26 @@ static inline void fill_playback_urb_dsd_dop(struct snd_usb_substream *subs, } }
+static void copy_to_urb(struct snd_usb_substream *subs, + struct urb *urb, int stride, unsigned int bytes) +{ + struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime; + + if (subs->hwptr_done + bytes > runtime->buffer_size * stride) { + /* err, the transferred area goes over buffer boundary. */ + unsigned int bytes1 = + runtime->buffer_size * stride - subs->hwptr_done; + memcpy(urb->transfer_buffer, + runtime->dma_area + subs->hwptr_done, bytes1); + memcpy(urb->transfer_buffer + bytes1, + runtime->dma_area, bytes - bytes1); + } else { + memcpy(urb->transfer_buffer, + runtime->dma_area + subs->hwptr_done, bytes); + } + subs->hwptr_done += bytes; +} + static void prepare_playback_urb(struct snd_usb_substream *subs, struct urb *urb) { @@ -1462,20 +1482,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->hwptr_done += bytes; } else { /* usual PCM */ - if (subs->hwptr_done + bytes > runtime->buffer_size * stride) { - /* err, the transferred area goes over buffer boundary. */ - unsigned int bytes1 = - runtime->buffer_size * stride - subs->hwptr_done; - memcpy(urb->transfer_buffer, - runtime->dma_area + subs->hwptr_done, bytes1); - memcpy(urb->transfer_buffer + bytes1, - runtime->dma_area, bytes - bytes1); - } else { - memcpy(urb->transfer_buffer, - runtime->dma_area + subs->hwptr_done, bytes); - } - - subs->hwptr_done += bytes; + copy_to_urb(subs, urb, stride, bytes); }
if (subs->hwptr_done >= runtime->buffer_size * stride)
Refactoring in preparation for adding Zoom R16/24 quirk. No functional change.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/pcm.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 4292bad..ee8dd9e 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1383,6 +1383,8 @@ static inline void fill_playback_urb_dsd_dop(struct snd_usb_substream *subs, subs->hwptr_done++; } } + if (subs->hwptr_done >= runtime->buffer_size * stride) + subs->hwptr_done -= runtime->buffer_size * stride; }
static void copy_to_urb(struct snd_usb_substream *subs, @@ -1403,6 +1405,8 @@ static void copy_to_urb(struct snd_usb_substream *subs, runtime->dma_area + subs->hwptr_done, bytes); } subs->hwptr_done += bytes; + if (subs->hwptr_done >= runtime->buffer_size * stride) + subs->hwptr_done -= runtime->buffer_size * stride; }
static void prepare_playback_urb(struct snd_usb_substream *subs, @@ -1480,14 +1484,13 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, }
subs->hwptr_done += bytes; + if (subs->hwptr_done >= runtime->buffer_size * stride) + subs->hwptr_done -= runtime->buffer_size * stride; } else { /* usual PCM */ copy_to_urb(subs, urb, stride, bytes); }
- if (subs->hwptr_done >= runtime->buffer_size * stride) - subs->hwptr_done -= runtime->buffer_size * stride; - /* update delay with exact number of samples queued */ runtime->delay = subs->last_delay; runtime->delay += frames;
Refactoring in preparation for adding Zoom R16/24 quirk. No functional change.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/endpoint.c | 46 +++++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 19 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index a77d9c8..825a06c 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -183,13 +183,38 @@ static void retire_inbound_urb(struct snd_usb_endpoint *ep, ep->retire_data_urb(ep->data_subs, urb); }
+static void prepare_silent_urb(struct snd_usb_endpoint *ep, + struct snd_urb_ctx *ctx) +{ + struct urb *urb = ctx->urb; + unsigned int offs = 0; + int i; + + for (i = 0; i < ctx->packets; ++i) { + int counts; + + if (ctx->packet_size[i]) + counts = ctx->packet_size[i]; + else + counts = snd_usb_endpoint_next_packet_size(ep); + + urb->iso_frame_desc[i].offset = offs * ep->stride; + urb->iso_frame_desc[i].length = counts * ep->stride; + offs += counts; + } + + urb->number_of_packets = ctx->packets; + urb->transfer_buffer_length = offs * ep->stride; + memset(urb->transfer_buffer, ep->silence_value, + offs * ep->stride); +} + /* * Prepare a PLAYBACK urb for submission to the bus. */ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, struct snd_urb_ctx *ctx) { - int i; struct urb *urb = ctx->urb; unsigned char *cp = urb->transfer_buffer;
@@ -201,24 +226,7 @@ static void prepare_outbound_urb(struct snd_usb_endpoint *ep, ep->prepare_data_urb(ep->data_subs, urb); } else { /* no data provider, so send silence */ - unsigned int offs = 0; - for (i = 0; i < ctx->packets; ++i) { - int counts; - - if (ctx->packet_size[i]) - counts = ctx->packet_size[i]; - else - counts = snd_usb_endpoint_next_packet_size(ep); - - urb->iso_frame_desc[i].offset = offs * ep->stride; - urb->iso_frame_desc[i].length = counts * ep->stride; - offs += counts; - } - - urb->number_of_packets = ctx->packets; - urb->transfer_buffer_length = offs * ep->stride; - memset(urb->transfer_buffer, ep->silence_value, - offs * ep->stride); + prepare_silent_urb(ep, ctx); } break;
Preparation for adding Zoom R16/24 quirk. No functional change.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/pcm.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index ee8dd9e..e3c5bc0 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1387,8 +1387,8 @@ static inline void fill_playback_urb_dsd_dop(struct snd_usb_substream *subs, subs->hwptr_done -= runtime->buffer_size * stride; }
-static void copy_to_urb(struct snd_usb_substream *subs, - struct urb *urb, int stride, unsigned int bytes) +static void copy_to_urb(struct snd_usb_substream *subs, struct urb *urb, + int offset, int stride, unsigned int bytes) { struct snd_pcm_runtime *runtime = subs->pcm_substream->runtime;
@@ -1396,12 +1396,12 @@ static void copy_to_urb(struct snd_usb_substream *subs, /* err, the transferred area goes over buffer boundary. */ unsigned int bytes1 = runtime->buffer_size * stride - subs->hwptr_done; - memcpy(urb->transfer_buffer, + memcpy(urb->transfer_buffer + offset, runtime->dma_area + subs->hwptr_done, bytes1); - memcpy(urb->transfer_buffer + bytes1, + memcpy(urb->transfer_buffer + offset + bytes1, runtime->dma_area, bytes - bytes1); } else { - memcpy(urb->transfer_buffer, + memcpy(urb->transfer_buffer + offset, runtime->dma_area + subs->hwptr_done, bytes); } subs->hwptr_done += bytes; @@ -1488,7 +1488,7 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->hwptr_done -= runtime->buffer_size * stride; } else { /* usual PCM */ - copy_to_urb(subs, urb, stride, bytes); + copy_to_urb(subs, urb, 0, stride, bytes); }
/* update delay with exact number of samples queued */
The Zoom R16/24 have a nonstandard playback format where each isochronous packet contains a length descriptor in the first four bytes. (Curiously, capture data does not contain this and requires no quirk.)
The quirk involves adding the extra length descriptor whenever outgoing isochronous packets are generated, both in pcm.c (outgoing audio) and endpoint.c (silent data).
In order to make the quirk as unintrusive as possible, for pcm.c:prepare_playback_urb(), the isochronous packet descriptors are initially set up in the same way no matter if the quirk is enabled or not. Once it is time to actually copy the data into the outgoing packet buffer (together with the added length descriptors) the isochronous descriptors are adjusted in order take the increased payload length into account.
For endpoint.c:prepare_silent_urb() it makes more sense to modify the actual function, partly because the function is less complex to start with and partly because it is not as time-critical as prepare_playback_urb() (whose bulk is run with interrupts disabled), so the (minute) additional time spent in the non-quirk case is motivated by the simplicity of having a single function for all cases.
The quirk is controlled by the new tx_length_quirk member in struct snd_usb_substream and struct snd_usb_audio, which is conveyed to pcm.c and endpoint.c from quirks.c in a similar manner to the txfr_quirk member in the same structs.
In contrast to txfr_quirk however, the quirk is enabled directly in quirks.c:create_standard_audio_quirk() by checking the USB ID in that function. Another option would be to introduce a new QUIRK_AUDIO_ZOOM_INTERFACE or somesuch, which would have made the quirk very plain to see in the quirk table, but it was felt that the additional code needed to implement it this way would just make the implementation more complex with no real gain.
Tested with a Zoom R16, both by doing capture and playback separately using arecord and aplay (8 channel capture and 2 channel playback, respectively), as well as capture and playback together using Ardour, as well as Audacity and Qtractor together with jackd.
The R24 is reportedly compatible with the R16 when used as an audio interface. Both devices share the same USB ID and have the same number of inputs (8) and outputs (2). Therefore "R16/24" is mentioned throughout the patch.
Regression tested using an Edirol UA-5 in both class compliant (16-bit) and "advanced" (24 bit, forces the use of quirks) modes.
Signed-off-by: Ricard Wanderlof ricardw@axis.com Tested-by: Panu Matilainen pmatilai@laiskiainen.org --- sound/usb/card.h | 1 + sound/usb/endpoint.c | 25 ++++++++++++++++++++----- sound/usb/pcm.c | 32 +++++++++++++++++++++++++++++++- sound/usb/quirks-table.h | 7 ++++--- sound/usb/quirks.c | 3 +++ sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 1 + 7 files changed, 61 insertions(+), 9 deletions(-)
diff --git a/sound/usb/card.h b/sound/usb/card.h index ef580b4..71778ca 100644 --- a/sound/usb/card.h +++ b/sound/usb/card.h @@ -122,6 +122,7 @@ struct snd_usb_substream { unsigned int buffer_periods; /* current periods per buffer */ unsigned int altset_idx; /* USB data format: index of alternate setting */ unsigned int txfr_quirk:1; /* allow sub-frame alignment */ + unsigned int tx_length_quirk:1; /* add length specifier to transfers */ unsigned int fmt_type; /* USB audio format type (1-3) */ unsigned int pkt_offset_adj; /* Bytes to drop from beginning of packets (for non-compliant devices) */
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 825a06c..0cc64bd 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -188,9 +188,17 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, { struct urb *urb = ctx->urb; unsigned int offs = 0; + unsigned int extra = 0; + __le32 packet_length; int i;
+ /* For tx_length_quirk, put packet length at start of packet */ + if (ep->chip->tx_length_quirk) + extra = sizeof(packet_length); + for (i = 0; i < ctx->packets; ++i) { + unsigned int offset; + unsigned int length; int counts;
if (ctx->packet_size[i]) @@ -198,15 +206,22 @@ static void prepare_silent_urb(struct snd_usb_endpoint *ep, else counts = snd_usb_endpoint_next_packet_size(ep);
- urb->iso_frame_desc[i].offset = offs * ep->stride; - urb->iso_frame_desc[i].length = counts * ep->stride; + length = counts * ep->stride; /* number of silent bytes */ + offset = offs * ep->stride + extra * i; + urb->iso_frame_desc[i].offset = offset; + urb->iso_frame_desc[i].length = length + extra; + if (extra) { + packet_length = cpu_to_le32(length); + memcpy(urb->transfer_buffer + offset, + &packet_length, sizeof(packet_length)); + } + memset(urb->transfer_buffer + offset + extra, + ep->silence_value, length); offs += counts; }
urb->number_of_packets = ctx->packets; - urb->transfer_buffer_length = offs * ep->stride; - memset(urb->transfer_buffer, ep->silence_value, - offs * ep->stride); + urb->transfer_buffer_length = offs * ep->stride + ctx->packets * extra; }
/* diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index e3c5bc0..f310144 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -1409,6 +1409,32 @@ static void copy_to_urb(struct snd_usb_substream *subs, struct urb *urb, subs->hwptr_done -= runtime->buffer_size * stride; }
+static unsigned int copy_to_urb_quirk(struct snd_usb_substream *subs, + struct urb *urb, int stride, + unsigned int bytes) +{ + __le32 packet_length; + int i; + + /* Put __le32 length descriptor at start of each packet. */ + for (i = 0; i < urb->number_of_packets; i++) { + unsigned int length = urb->iso_frame_desc[i].length; + unsigned int offset = urb->iso_frame_desc[i].offset; + + packet_length = cpu_to_le32(length); + offset += i * sizeof(packet_length); + urb->iso_frame_desc[i].offset = offset; + urb->iso_frame_desc[i].length += sizeof(packet_length); + memcpy(urb->transfer_buffer + offset, + &packet_length, sizeof(packet_length)); + copy_to_urb(subs, urb, offset + sizeof(packet_length), + stride, length); + } + /* Adjust transfer size accordingly. */ + bytes += urb->number_of_packets * sizeof(packet_length); + return bytes; +} + static void prepare_playback_urb(struct snd_usb_substream *subs, struct urb *urb) { @@ -1488,7 +1514,11 @@ static void prepare_playback_urb(struct snd_usb_substream *subs, subs->hwptr_done -= runtime->buffer_size * stride; } else { /* usual PCM */ - copy_to_urb(subs, urb, 0, stride, bytes); + if (!subs->tx_length_quirk) + copy_to_urb(subs, urb, 0, stride, bytes); + else + bytes = copy_to_urb_quirk(subs, urb, stride, bytes); + /* bytes is now amount of outgoing data */ }
/* update delay with exact number of samples queued */ diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index e475665..4a8ca637 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3184,8 +3184,9 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), * ZOOM R16/24 in audio interface mode. * Mixer descriptors are garbage, further quirks will be needed * to make any of it functional, thus disabled for now. - * Playback stream appears to start and run fine but no sound - * is produced, so also disabled for now. + * Playback requires an extra four byte LE length indicator + * at the start of each isochronous packet. This quirk is + * enabled in create_standard_audio_quirk(). */ USB_DEVICE(0x1686, 0x00dd), .driver_info = (unsigned long) & (const struct snd_usb_audio_quirk) { @@ -3200,7 +3201,7 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), { /* Playback */ .ifnum = 1, - .type = QUIRK_IGNORE_INTERFACE, + .type = QUIRK_AUDIO_STANDARD_INTERFACE, }, { /* Capture */ diff --git a/sound/usb/quirks.c b/sound/usb/quirks.c index 00ebc0c..4897ea1 100644 --- a/sound/usb/quirks.c +++ b/sound/usb/quirks.c @@ -115,6 +115,9 @@ static int create_standard_audio_quirk(struct snd_usb_audio *chip, struct usb_interface_descriptor *altsd; int err;
+ if (chip->usb_id == USB_ID(0x1686, 0x00dd)) /* Zoom R16/24 */ + chip->tx_length_quirk = 1; + alts = &iface->altsetting[0]; altsd = get_iface_desc(alts); err = snd_usb_parse_audio_interface(chip, altsd->bInterfaceNumber); diff --git a/sound/usb/stream.c b/sound/usb/stream.c index 9700860..8ee14f2 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -92,6 +92,7 @@ static void snd_usb_init_substream(struct snd_usb_stream *as, subs->direction = stream; subs->dev = as->chip->dev; subs->txfr_quirk = as->chip->txfr_quirk; + subs->tx_length_quirk = as->chip->tx_length_quirk; subs->speed = snd_usb_get_speed(subs->dev); subs->pkt_offset_adj = 0;
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h index 33a1764..15a1271 100644 --- a/sound/usb/usbaudio.h +++ b/sound/usb/usbaudio.h @@ -43,6 +43,7 @@ struct snd_usb_audio { atomic_t usage_count; 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 */ int num_interfaces; int num_suspended_intf;
For the Zoom R16/24 (tx_length_quirk set), when calculating the maximum sample frequency, consideration must be made for the fact that four bytes of the packet contain a length descriptor and consequently must not be counted as part of the audio data.
This is corroborated by the wMaxPacketSize for this device, which is 108 bytes according for the USB playback endpoint descriptor. The frame size is 8 bytes (2 channels of 4 bytes each), and the 108 bytes thus work out as 13 * 8 + 4, i.e. corresponding to 13 frames plus the additional 4 byte length descriptor.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/endpoint.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 0cc64bd..a8c751a 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -617,6 +617,8 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, 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 && + usb_pipeout(ep->pipe));
if (pcm_format == SNDRV_PCM_FORMAT_DSD_U16_LE && fmt->dsd_dop) { /* @@ -650,11 +652,17 @@ static int data_ep_set_params(struct snd_usb_endpoint *ep, */ 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 */ - maxsize = ep->maxpacksize; - ep->freqmax = (maxsize / (frame_bits >> 3)) + unsigned int data_maxsize = maxsize = ep->maxpacksize; + + if (tx_length_quirk) + /* Need to remove the length descriptor to calc freq */ + data_maxsize -= sizeof(__le32); + ep->freqmax = (data_maxsize / (frame_bits >> 3)) << (16 - ep->datainterval); }
The device has no mixer (and identifies itself as such), so just skip the mixer definition.
Signed-off-by: Ricard Wanderlof ricardw@axis.com --- sound/usb/quirks-table.h | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/sound/usb/quirks-table.h b/sound/usb/quirks-table.h index 4a8ca637..722b77b 100644 --- a/sound/usb/quirks-table.h +++ b/sound/usb/quirks-table.h @@ -3182,8 +3182,6 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), { /* * ZOOM R16/24 in audio interface mode. - * Mixer descriptors are garbage, further quirks will be needed - * to make any of it functional, thus disabled for now. * Playback requires an extra four byte LE length indicator * at the start of each isochronous packet. This quirk is * enabled in create_standard_audio_quirk(). @@ -3194,11 +3192,6 @@ AU0828_DEVICE(0x2040, 0x7270, "Hauppauge", "HVR-950Q"), .type = QUIRK_COMPOSITE, .data = (const struct snd_usb_audio_quirk[]) { { - /* Mixer */ - .ifnum = 0, - .type = QUIRK_IGNORE_INTERFACE, - }, - { /* Playback */ .ifnum = 1, .type = QUIRK_AUDIO_STANDARD_INTERFACE,
On Mon, 19 Oct 2015 08:52:48 +0200, Ricard Wanderlof wrote:
V2: Change u32 to __le32 (kbuild test robot warning). Since Zoom R24 most likely behaves as R16 when used as an audio interface, refer to "R16/24" throughout in all comments (no code change). Tested both in Linux 3.16.7 (with appropriate backports) and 4.3.0 . Made changelogs more descriptive.
The Zoom R16 has a nonstandard playback format where each isochronous packet contains a length descriptor in the first four bytes.
This patch series implements a quirk which involves adding the extra length descriptor whenever outgoing isochronous packets are generated.
The first four patches refactor some code in pcm.c and endpoint.c before the actual quirk implementation. A final commit cleans up the entry in the quirks table.
Big thanks to Panu for testing this patch.
Signed-off-by: Ricard Wanderlof ricardw@axis.com Tested-by: Panu Matilainen pmatilai@laiskiainen.org
Ricard Wanderlof (7): Break out copying to urb from prepare_playback_urb() Also move out hwptr_done wrap from prepare_playback_urb() Break out creation of silent urbs from prepare_outbound_urb() Add offset parameter to copy_to_urb() Add quirk for Zoom R16/24 playback Adjust max packet size calculation for tx_length_quirk Remove mixer entry from Zoom R16/24 quirk
Applied all patches now. Thanks.
Takashi
sound/usb/card.h | 1 + sound/usb/endpoint.c | 73 +++++++++++++++++++++++++++++++++-------------- sound/usb/pcm.c | 74 +++++++++++++++++++++++++++++++++++++----------- sound/usb/quirks-table.h | 14 +++------ sound/usb/quirks.c | 3 ++ sound/usb/stream.c | 1 + sound/usb/usbaudio.h | 1 + 7 files changed, 119 insertions(+), 48 deletions(-)
-- 2.1.4
participants (4)
-
James Cameron -
kbuild test robot -
Ricard Wanderlof -
Takashi Iwai