Skip to content

Commit 1161db6

Browse files
matweymchehab
authored andcommitted
media: usb: pwc: Don't use coherent DMA buffers for ISO transfer
DMA cocherency slows the transfer down on systems without hardware coherent DMA. Instead we use noncocherent DMA memory and explicit sync at data receive handler. Based on previous commit the following performance benchmarks have been carried out. Average memcpy() data transfer rate (rate) and handler completion time (time) have been measured when running video stream at 640x480 resolution at 10fps. x86_64 based system (Intel Core i5-3470). This platform has hardware coherent DMA support and proposed change doesn't make big difference here. * kmalloc: rate = (2.0 +- 0.4) GBps time = (5.0 +- 3.0) usec * usb_alloc_coherent: rate = (3.4 +- 1.2) GBps time = (3.5 +- 3.0) usec We see that the measurements agree within error ranges in this case. So theoretically predicted performance downgrade cannot be reliably measured here. armv7l based system (TI AM335x BeagleBone Black @ 300MHz). This platform has no hardware coherent DMA support. DMA coherence is implemented via disabled page caching that slows down memcpy() due to memory controller behaviour. * kmalloc: rate = ( 94 +- 4) MBps time = (101 +- 4) usec * usb_alloc_coherent: rate = (28.1 +- 0.1) MBps time = (341 +- 2) usec Note, that quantative difference leads (this commit leads to 3.3 times acceleration) to qualitative behavior change in this case. As it was stated before, the video stream cannot be successfully received at AM335x platforms with MUSB based USB host controller due to performance issues [1]. [1] https://www.spinics.net/lists/linux-usb/msg165735.html Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
1 parent c1d5fb0 commit 1161db6

File tree

1 file changed

+49
-13
lines changed

1 file changed

+49
-13
lines changed

drivers/media/usb/pwc/pwc-if.c

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,32 @@ static const struct video_device pwc_template = {
159159
/***************************************************************************/
160160
/* Private functions */
161161

162+
static void *pwc_alloc_urb_buffer(struct device *dev,
163+
size_t size, dma_addr_t *dma_handle)
164+
{
165+
void *buffer = kmalloc(size, GFP_KERNEL);
166+
167+
if (!buffer)
168+
return NULL;
169+
170+
*dma_handle = dma_map_single(dev, buffer, size, DMA_FROM_DEVICE);
171+
if (dma_mapping_error(dev, *dma_handle)) {
172+
kfree(buffer);
173+
return NULL;
174+
}
175+
176+
return buffer;
177+
}
178+
179+
static void pwc_free_urb_buffer(struct device *dev,
180+
size_t size,
181+
void *buffer,
182+
dma_addr_t dma_handle)
183+
{
184+
dma_unmap_single(dev, dma_handle, size, DMA_FROM_DEVICE);
185+
kfree(buffer);
186+
}
187+
162188
static struct pwc_frame_buf *pwc_get_next_fill_buf(struct pwc_device *pdev)
163189
{
164190
unsigned long flags = 0;
@@ -306,6 +332,11 @@ static void pwc_isoc_handler(struct urb *urb)
306332
/* Reset ISOC error counter. We did get here, after all. */
307333
pdev->visoc_errors = 0;
308334

335+
dma_sync_single_for_cpu(&urb->dev->dev,
336+
urb->transfer_dma,
337+
urb->transfer_buffer_length,
338+
DMA_FROM_DEVICE);
339+
309340
/* vsync: 0 = don't copy data
310341
1 = sync-hunt
311342
2 = synched
@@ -352,6 +383,11 @@ static void pwc_isoc_handler(struct urb *urb)
352383
pdev->vlast_packet_size = flen;
353384
}
354385

386+
dma_sync_single_for_device(&urb->dev->dev,
387+
urb->transfer_dma,
388+
urb->transfer_buffer_length,
389+
DMA_FROM_DEVICE);
390+
355391
handler_end:
356392
trace_pwc_handler_exit(urb, pdev);
357393

@@ -428,16 +464,15 @@ static int pwc_isoc_init(struct pwc_device *pdev)
428464
urb->dev = udev;
429465
urb->pipe = usb_rcvisocpipe(udev, pdev->vendpoint);
430466
urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
431-
urb->transfer_buffer = usb_alloc_coherent(udev,
432-
ISO_BUFFER_SIZE,
433-
GFP_KERNEL,
434-
&urb->transfer_dma);
467+
urb->transfer_buffer_length = ISO_BUFFER_SIZE;
468+
urb->transfer_buffer = pwc_alloc_urb_buffer(&udev->dev,
469+
urb->transfer_buffer_length,
470+
&urb->transfer_dma);
435471
if (urb->transfer_buffer == NULL) {
436472
PWC_ERROR("Failed to allocate urb buffer %d\n", i);
437473
pwc_isoc_cleanup(pdev);
438474
return -ENOMEM;
439475
}
440-
urb->transfer_buffer_length = ISO_BUFFER_SIZE;
441476
urb->complete = pwc_isoc_handler;
442477
urb->context = pdev;
443478
urb->start_frame = 0;
@@ -488,15 +523,16 @@ static void pwc_iso_free(struct pwc_device *pdev)
488523

489524
/* Freeing ISOC buffers one by one */
490525
for (i = 0; i < MAX_ISO_BUFS; i++) {
491-
if (pdev->urbs[i]) {
526+
struct urb *urb = pdev->urbs[i];
527+
528+
if (urb) {
492529
PWC_DEBUG_MEMORY("Freeing URB\n");
493-
if (pdev->urbs[i]->transfer_buffer) {
494-
usb_free_coherent(pdev->udev,
495-
pdev->urbs[i]->transfer_buffer_length,
496-
pdev->urbs[i]->transfer_buffer,
497-
pdev->urbs[i]->transfer_dma);
498-
}
499-
usb_free_urb(pdev->urbs[i]);
530+
if (urb->transfer_buffer)
531+
pwc_free_urb_buffer(&urb->dev->dev,
532+
urb->transfer_buffer_length,
533+
urb->transfer_buffer,
534+
urb->transfer_dma);
535+
usb_free_urb(urb);
500536
pdev->urbs[i] = NULL;
501537
}
502538
}

0 commit comments

Comments
 (0)