| Message ID | 20180830172030.23344-4-ezequiel@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce usb_{alloc,free}_noncoherent API | expand |
> + dma_sync_single_for_cpu(&urb->dev->dev, urb->transfer_dma, > + urb->transfer_buffer_length, DMA_FROM_DEVICE); You can't ue dma_sync_single_for_cpu on non-coherent dma buffers, which is one of the major issues with them.
On Fri, Aug 31, 2018 at 2:59 AM Christoph Hellwig <hch@infradead.org> wrote: > > > + dma_sync_single_for_cpu(&urb->dev->dev, urb->transfer_dma, > > + urb->transfer_buffer_length, DMA_FROM_DEVICE); > > You can't ue dma_sync_single_for_cpu on non-coherent dma buffers, > which is one of the major issues with them. It's not an issue of DMA API, but just an API mismatch. By design, memory allocated for device (e.g. by DMA API) doesn't have to be physically contiguous, while dma_*_single() API expects a _single_, physically contiguous region of memory. We need a way to allocate non-coherent memory using DMA API to handle (on USB example, but applies to virtually any class of devices doing DMA): - DMA address range limitations (e.g. dma_mask) - while a USB HCD driver is normally aware of those, USB device driver should have no idea, - memory mapping capability === whether contiguous memory or a set of random pages can be allocated - this is a platform integration detail, which even a USB HCD driver may not be aware of, if a SoC IOMMU is just stuffed between the bus and HCD, - platform coherency specifics - there are practical scenarios when on a coherent-by-default system it's more efficient to allocate non-coherent memory and manage caches explicitly to avoid the costs of cache snooping. If DMA_ATTR_NON_CONSISTENT is not the right way to do it, there should be definitely a new API introduced, coupled closely to DMA API implementation on given platform, since it's the only place which can solve all the constraints above. Best regards, Tomasz
diff --git a/drivers/media/usb/stk1160/stk1160-video.c b/drivers/media/usb/stk1160/stk1160-video.c index 2811f612820f..aeb4264d1998 100644 --- a/drivers/media/usb/stk1160/stk1160-video.c +++ b/drivers/media/usb/stk1160/stk1160-video.c @@ -240,6 +240,9 @@ static void stk1160_process_isoc(struct stk1160 *dev, struct urb *urb) return; } + dma_sync_single_for_cpu(&urb->dev->dev, urb->transfer_dma, + urb->transfer_buffer_length, DMA_FROM_DEVICE); + for (i = 0; i < urb->number_of_packets; i++) { status = urb->iso_frame_desc[i].status; if (status < 0) { @@ -379,16 +382,11 @@ void stk1160_free_isoc(struct stk1160 *dev) urb = dev->isoc_ctl.urb[i]; if (urb) { - if (dev->isoc_ctl.transfer_buffer[i]) { -#ifndef CONFIG_DMA_NONCOHERENT - usb_free_coherent(dev->udev, + if (dev->isoc_ctl.transfer_buffer[i]) + usb_free_noncoherent(dev->udev, urb->transfer_buffer_length, dev->isoc_ctl.transfer_buffer[i], urb->transfer_dma); -#else - kfree(dev->isoc_ctl.transfer_buffer[i]); -#endif - } usb_free_urb(urb); dev->isoc_ctl.urb[i] = NULL; } @@ -461,12 +459,8 @@ int stk1160_alloc_isoc(struct stk1160 *dev) goto free_i_bufs; dev->isoc_ctl.urb[i] = urb; -#ifndef CONFIG_DMA_NONCOHERENT - dev->isoc_ctl.transfer_buffer[i] = usb_alloc_coherent(dev->udev, + dev->isoc_ctl.transfer_buffer[i] = usb_alloc_noncoherent(dev->udev, sb_size, GFP_KERNEL, &urb->transfer_dma); -#else - dev->isoc_ctl.transfer_buffer[i] = kmalloc(sb_size, GFP_KERNEL); -#endif if (!dev->isoc_ctl.transfer_buffer[i]) { stk1160_err("cannot alloc %d bytes for tx[%d] buffer\n", sb_size, i); @@ -490,11 +484,7 @@ int stk1160_alloc_isoc(struct stk1160 *dev) urb->interval = 1; urb->start_frame = 0; urb->number_of_packets = max_packets; -#ifndef CONFIG_DMA_NONCOHERENT urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP; -#else - urb->transfer_flags = URB_ISO_ASAP; -#endif k = 0; for (j = 0; j < max_packets; j++) {
Platforms without hardware coherency can benefit a lot from using non-coherent buffers. Moreover, platforms with hardware coherency aren't impacted by this change. For instance, on AM335x, while it's still not possible to capture full resolution frames, this patch enables half-resolution frame streams to work. Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> --- drivers/media/usb/stk1160/stk1160-video.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-)