On Tue, Apr 20, 2010 at 01:16:58AM +0100, Pedro Ribeiro wrote: > On 15 April 2010 16:20, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, 15 Apr 2010, Pedro Ribeiro wrote: > > > >> I enabled CONFIG_DMAR_BROKEN_GFX_WA=y and the result is the same. A > >> delay in the boot process and usb devices don't work properly, > >> including my USB mouse. > >> > >> Strange, since you have the same platform as me. The extra usb devices > >> you were seeing are because of my docking station - but it makes no > >> difference whether I'm docked or not for the purposes of the original > >> bug or this situation right now. The dmesg I'm attaching is without > >> the computer being docked. > > > > It's not possible to determine the reason for the timeout errors > > between timestamps 16 and 53 from the small amount of debugging > > information in the log. Clearly something is going wrong with the > > communication between the computer and the EHCI controller. And > > clearly the kernel config changes are responsible. > > > > But I don't know what to do to track it down any farther. > > > > Alan Stern > > > > > > I guess this is pretty much a dead end until anyone else can reproduce it! Hmm, I think I finally found the reason for all this confusion. No idea why I didn't come up with the following explanation any earlier. The problem is again (summarized): On 64bit machines, with 4GB or more, the allocated buffers for USB transfers might be beyond the 32bit boundary. In this case, the IOMMU should take care and install DMA bounce buffer to copy over the buffer before the transfer actually happens. The problem is, however, that this copy mechanism takes place when the URB with its associated buffer is submitted, not when the EHCI will actually do the transfer. In the particular case of audio drivers, though, the contents of the buffers are likely to change after the submission. What we do here is that we map the audio stream buffers which are used by ALSA to the output URBs, so they're filled asychronously. Once the buffer is actually sent out on the bus, it is believed to contain proper audio date. If it doesn't, that's due to too tight audio timing or other problems. This breaks once buffers are magically bounced in the background. So - long story short: these audio buffers need to be DMA coherent. The patch below does that, and Pedro excessively tested this patch for weeks now :) It was just the final explanation _why_ it does the right thing that was yet missing. If nobody has objections, can we still squeeze it into .34? Thanks, Daniel >From 5672ca44b9b4617f6c29c88409da13b1bf475547 Mon Sep 17 00:00:00 2001 From: Daniel Mack <daniel@xxxxxxxx> Date: Wed, 7 Apr 2010 01:03:09 +0200 Subject: [PATCH] ALSA: usb/caiaq: use usb_buffer_alloc() Use usb_buffer_alloc() and usb_buffer_free() for transfer buffers. We need DMA-coherent memory in this case as buffer contents are likely to be modified after the URB was submitted, because the URB buffers are mapped to the audio streams. On x86_64, buffers allocated with kmalloc() may be beyond the boundaries of 32bit accessible memory, and DMA bounce buffers will live at other locations, unaccessible by the driver, and hence outside of the audio buffer mapping. Signed-off-by: Daniel Mack <daniel@xxxxxxxx> Tested-by: Pedro Ribeiro <pedrib@xxxxxxxxx> Cc: Takashi Iwai <tiwai@xxxxxxx> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> Cc: Chris Wright <chrisw@xxxxxxxxxxxx> Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx> Cc: Andi Kleen <andi@xxxxxxxxxxxxxx> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> Cc: Greg KH <gregkh@xxxxxxx> Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx Cc: Kernel development list <linux-kernel@xxxxxxxxxxxxxxx> Cc: USB list <linux-usb@xxxxxxxxxxxxxxx> Cc: stable@xxxxxxxxxx --- sound/usb/caiaq/audio.c | 57 ++++++++++++++++++++++++++-------------------- 1 files changed, 32 insertions(+), 25 deletions(-) diff --git a/sound/usb/caiaq/audio.c b/sound/usb/caiaq/audio.c index 4328cad..adbeefd 100644 --- a/sound/usb/caiaq/audio.c +++ b/sound/usb/caiaq/audio.c @@ -552,46 +552,47 @@ static struct urb **alloc_urbs(struct snd_usb_caiaqdev *dev, int dir, int *ret) } for (i = 0; i < N_URBS; i++) { - urbs[i] = usb_alloc_urb(FRAMES_PER_URB, GFP_KERNEL); - if (!urbs[i]) { + struct urb *u = usb_alloc_urb(FRAMES_PER_URB, GFP_KERNEL); + if (!u) { log("unable to usb_alloc_urb(), OOM!?\n"); *ret = -ENOMEM; return urbs; } - urbs[i]->transfer_buffer = - kmalloc(FRAMES_PER_URB * BYTES_PER_FRAME, GFP_KERNEL); - if (!urbs[i]->transfer_buffer) { - log("unable to kmalloc() transfer buffer, OOM!?\n"); + urbs[i] = u; + u->dev = usb_dev; + u->pipe = pipe; + u->transfer_buffer_length = + FRAMES_PER_URB * BYTES_PER_FRAME; + u->context = &dev->data_cb_info[i]; + u->interval = 1; + u->transfer_flags = URB_ISO_ASAP; + u->number_of_packets = FRAMES_PER_URB; + u->complete = (dir == SNDRV_PCM_STREAM_CAPTURE) ? + read_completed : write_completed; + u->transfer_buffer = usb_alloc_coherent(usb_dev, + u->transfer_buffer_length, + GFP_KERNEL, &u->transfer_dma); + if (!u->transfer_buffer) { + log("usb_alloc_coherent() failed, OOM!?\n"); *ret = -ENOMEM; return urbs; } for (frame = 0; frame < FRAMES_PER_URB; frame++) { struct usb_iso_packet_descriptor *iso = - &urbs[i]->iso_frame_desc[frame]; + &u->iso_frame_desc[frame]; iso->offset = BYTES_PER_FRAME * frame; iso->length = BYTES_PER_FRAME; } - - urbs[i]->dev = usb_dev; - urbs[i]->pipe = pipe; - urbs[i]->transfer_buffer_length = FRAMES_PER_URB - * BYTES_PER_FRAME; - urbs[i]->context = &dev->data_cb_info[i]; - urbs[i]->interval = 1; - urbs[i]->transfer_flags = URB_ISO_ASAP; - urbs[i]->number_of_packets = FRAMES_PER_URB; - urbs[i]->complete = (dir == SNDRV_PCM_STREAM_CAPTURE) ? - read_completed : write_completed; } *ret = 0; return urbs; } -static void free_urbs(struct urb **urbs) +static void free_urbs(struct usb_device *usb_dev, struct urb **urbs) { int i; @@ -603,7 +604,10 @@ static void free_urbs(struct urb **urbs) continue; usb_kill_urb(urbs[i]); - kfree(urbs[i]->transfer_buffer); + usb_free_coherent(usb_dev, + urbs[i]->transfer_buffer_length, + urbs[i]->transfer_buffer, + urbs[i]->transfer_dma); usb_free_urb(urbs[i]); } @@ -613,6 +617,7 @@ static void free_urbs(struct urb **urbs) int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) { int i, ret; + struct usb_device *usb_dev = dev->chip.dev; dev->n_audio_in = max(dev->spec.num_analog_audio_in, dev->spec.num_digital_audio_in) / @@ -689,15 +694,15 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) dev->data_urbs_in = alloc_urbs(dev, SNDRV_PCM_STREAM_CAPTURE, &ret); if (ret < 0) { kfree(dev->data_cb_info); - free_urbs(dev->data_urbs_in); + free_urbs(usb_dev, dev->data_urbs_in); return ret; } dev->data_urbs_out = alloc_urbs(dev, SNDRV_PCM_STREAM_PLAYBACK, &ret); if (ret < 0) { kfree(dev->data_cb_info); - free_urbs(dev->data_urbs_in); - free_urbs(dev->data_urbs_out); + free_urbs(usb_dev, dev->data_urbs_in); + free_urbs(usb_dev, dev->data_urbs_out); return ret; } @@ -706,10 +711,12 @@ int snd_usb_caiaq_audio_init(struct snd_usb_caiaqdev *dev) void snd_usb_caiaq_audio_free(struct snd_usb_caiaqdev *dev) { + struct usb_device *usb_dev = dev->chip.dev; + debug("%s(%p)\n", __func__, dev); stream_stop(dev); - free_urbs(dev->data_urbs_in); - free_urbs(dev->data_urbs_out); + free_urbs(usb_dev, dev->data_urbs_in); + free_urbs(usb_dev, dev->data_urbs_out); kfree(dev->data_cb_info); } -- 1.7.1 _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel