[opensuse-kernel] [PATCH 0/3] Fix: boo#1054610
From: "Matwey V. Kornilov" <matwey@sai.msu.ru> Hello, The series is to (partially) fix boo#1054610 "[BeagleBoneBlack, musb]: Broken isochronous transfer". The patches are intended to speedup URB completion handler for Philips webcams on armv7l hardware. The series has been accepted upstream and it is a part of linux-next branch now. Could you please apply it to openSUSE-15.0, openSUSE-15.1, stable, master? Reference: boo#1054610 Hans Verkuil (1): MAINTAINERS: added include/trace/events/pwc.h Matwey V. Kornilov (2): media: usb: pwc: Introduce TRACE_EVENTs for pwc_isoc_handler() media: usb: pwc: Don't use coherent DMA buffers for ISO transfer MAINTAINERS | 1 + drivers/media/usb/pwc/pwc-if.c | 69 ++++++++++++++++++++++++++++++++++-------- include/trace/events/pwc.h | 65 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 include/trace/events/pwc.h -- 2.16.4 -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
From: "Matwey V. Kornilov" <matwey@sai.msu.ru> There were reports that PWC-based webcams don't work at some embedded ARM platforms. [1] Isochronous transfer handler seems to work too long leading to the issues in MUSB USB host subsystem. Also note, that urb->giveback() handlers are still called with disabled interrupts. In order to be able to measure performance of PWC driver, traces are introduced in URB handler section. [1] https://www.spinics.net/lists/linux-usb/msg165735.html Signed-off-by: Matwey V. Kornilov <matwey@sai.msu.ru> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org> 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> References: boo#1054610 Patch-mainline: v5.1-rc1 Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git Git-commit: c1d5fb019f65e1093c77734356f97b6e7ac4d629 Signed-off-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> --- drivers/media/usb/pwc/pwc-if.c | 7 +++++ include/trace/events/pwc.h | 65 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 include/trace/events/pwc.h diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c index 22420c14ac98..e94b3b22eed3 100644 --- a/drivers/media/usb/pwc/pwc-if.c +++ b/drivers/media/usb/pwc/pwc-if.c @@ -76,6 +76,9 @@ #include "pwc-dec23.h" #include "pwc-dec1.h" +#define CREATE_TRACE_POINTS +#include <trace/events/pwc.h> + /* Function prototypes and driver templates */ /* hotplug device table support */ @@ -260,6 +263,8 @@ static void pwc_isoc_handler(struct urb *urb) int i, fst, flen; unsigned char *iso_buf = NULL; + trace_pwc_handler_enter(urb, pdev); + if (urb->status == -ENOENT || urb->status == -ECONNRESET || urb->status == -ESHUTDOWN) { PWC_DEBUG_OPEN("URB (%p) unlinked %ssynchronuously.\n", urb, urb->status == -ENOENT ? "" : "a"); @@ -347,6 +352,8 @@ static void pwc_isoc_handler(struct urb *urb) } handler_end: + trace_pwc_handler_exit(urb, pdev); + i = usb_submit_urb(urb, GFP_ATOMIC); if (i != 0) PWC_ERROR("Error (%d) re-submitting urb in pwc_isoc_handler.\n", i); diff --git a/include/trace/events/pwc.h b/include/trace/events/pwc.h new file mode 100644 index 000000000000..a2da764a3b41 --- /dev/null +++ b/include/trace/events/pwc.h @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#if !defined(_TRACE_PWC_H) || defined(TRACE_HEADER_MULTI_READ) +#define _TRACE_PWC_H + +#include <linux/usb.h> +#include <linux/tracepoint.h> + +#undef TRACE_SYSTEM +#define TRACE_SYSTEM pwc + +TRACE_EVENT(pwc_handler_enter, + TP_PROTO(struct urb *urb, struct pwc_device *pdev), + TP_ARGS(urb, pdev), + TP_STRUCT__entry( + __field(struct urb*, urb) + __field(struct pwc_frame_buf*, fbuf) + __field(int, urb__status) + __field(u32, urb__actual_length) + __field(int, fbuf__filled) + __string(name, pdev->v4l2_dev.name) + ), + TP_fast_assign( + __entry->urb = urb; + __entry->fbuf = pdev->fill_buf; + __entry->urb__status = urb->status; + __entry->urb__actual_length = urb->actual_length; + __entry->fbuf__filled = (pdev->fill_buf + ? pdev->fill_buf->filled : 0); + __assign_str(name, pdev->v4l2_dev.name); + ), + TP_printk("dev=%s (fbuf=%p filled=%d) urb=%p (status=%d actual_length=%u)", + __get_str(name), + __entry->fbuf, + __entry->fbuf__filled, + __entry->urb, + __entry->urb__status, + __entry->urb__actual_length) +); + +TRACE_EVENT(pwc_handler_exit, + TP_PROTO(struct urb *urb, struct pwc_device *pdev), + TP_ARGS(urb, pdev), + TP_STRUCT__entry( + __field(struct urb*, urb) + __field(struct pwc_frame_buf*, fbuf) + __field(int, fbuf__filled) + __string(name, pdev->v4l2_dev.name) + ), + TP_fast_assign( + __entry->urb = urb; + __entry->fbuf = pdev->fill_buf; + __entry->fbuf__filled = pdev->fill_buf->filled; + __assign_str(name, pdev->v4l2_dev.name); + ), + TP_printk(" dev=%s (fbuf=%p filled=%d) urb=%p", + __get_str(name), + __entry->fbuf, + __entry->fbuf__filled, + __entry->urb) +); + +#endif /* _TRACE_PWC_H */ + +/* This part must be outside protection */ +#include <trace/define_trace.h> -- 2.16.4 -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
From: "Matwey V. Kornilov" <matwey@sai.msu.ru> 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> References: boo#1054610 Patch-mainline: v5.1-rc1 Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git Git-commit: 1161db6776bd1c35257e1e362e91bcb1b3fe4347 Signed-off-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> --- drivers/media/usb/pwc/pwc-if.c | 62 +++++++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c index e94b3b22eed3..8733720d6198 100644 --- a/drivers/media/usb/pwc/pwc-if.c +++ b/drivers/media/usb/pwc/pwc-if.c @@ -159,6 +159,32 @@ static struct video_device pwc_template = { /***************************************************************************/ /* Private functions */ +static void *pwc_alloc_urb_buffer(struct device *dev, + size_t size, dma_addr_t *dma_handle) +{ + void *buffer = kmalloc(size, GFP_KERNEL); + + if (!buffer) + return NULL; + + *dma_handle = dma_map_single(dev, buffer, size, DMA_FROM_DEVICE); + if (dma_mapping_error(dev, *dma_handle)) { + kfree(buffer); + return NULL; + } + + return buffer; +} + +static void pwc_free_urb_buffer(struct device *dev, + size_t size, + void *buffer, + dma_addr_t dma_handle) +{ + dma_unmap_single(dev, dma_handle, size, DMA_FROM_DEVICE); + kfree(buffer); +} + static struct pwc_frame_buf *pwc_get_next_fill_buf(struct pwc_device *pdev) { unsigned long flags = 0; @@ -305,6 +331,11 @@ static void pwc_isoc_handler(struct urb *urb) /* Reset ISOC error counter. We did get here, after all. */ pdev->visoc_errors = 0; + dma_sync_single_for_cpu(&urb->dev->dev, + urb->transfer_dma, + urb->transfer_buffer_length, + DMA_FROM_DEVICE); + /* vsync: 0 = don't copy data 1 = sync-hunt 2 = synched @@ -351,6 +382,11 @@ static void pwc_isoc_handler(struct urb *urb) pdev->vlast_packet_size = flen; } + dma_sync_single_for_device(&urb->dev->dev, + urb->transfer_dma, + urb->transfer_buffer_length, + DMA_FROM_DEVICE); + handler_end: trace_pwc_handler_exit(urb, pdev); @@ -427,16 +463,15 @@ static int pwc_isoc_init(struct pwc_device *pdev) urb->dev = udev; urb->pipe = usb_rcvisocpipe(udev, pdev->vendpoint); urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP; - urb->transfer_buffer = usb_alloc_coherent(udev, - ISO_BUFFER_SIZE, - GFP_KERNEL, - &urb->transfer_dma); + urb->transfer_buffer_length = ISO_BUFFER_SIZE; + urb->transfer_buffer = pwc_alloc_urb_buffer(&udev->dev, + urb->transfer_buffer_length, + &urb->transfer_dma); if (urb->transfer_buffer == NULL) { PWC_ERROR("Failed to allocate urb buffer %d\n", i); pwc_isoc_cleanup(pdev); return -ENOMEM; } - urb->transfer_buffer_length = ISO_BUFFER_SIZE; urb->complete = pwc_isoc_handler; urb->context = pdev; urb->start_frame = 0; @@ -487,15 +522,16 @@ static void pwc_iso_free(struct pwc_device *pdev) /* Freeing ISOC buffers one by one */ for (i = 0; i < MAX_ISO_BUFS; i++) { - if (pdev->urbs[i]) { + struct urb *urb = pdev->urbs[i]; + + if (urb) { PWC_DEBUG_MEMORY("Freeing URB\n"); - if (pdev->urbs[i]->transfer_buffer) { - usb_free_coherent(pdev->udev, - pdev->urbs[i]->transfer_buffer_length, - pdev->urbs[i]->transfer_buffer, - pdev->urbs[i]->transfer_dma); - } - usb_free_urb(pdev->urbs[i]); + if (urb->transfer_buffer) + pwc_free_urb_buffer(&urb->dev->dev, + urb->transfer_buffer_length, + urb->transfer_buffer, + urb->transfer_dma); + usb_free_urb(urb); pdev->urbs[i] = NULL; } } -- 2.16.4 -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
From: Hans Verkuil <hverkuil@xs4all.nl> Added include/trace/events/pwc.h to the list of files. Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> References: boo#1054610 Patch-mainline: v5.1-rc1 Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git Git-commit: 986ac47eaff77892a6393b4685c36ab9c9a4e089 Signed-off-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 4e91ec37668a..1297fb7a209e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -10423,6 +10423,7 @@ L: linux-media@vger.kernel.org T: git git://linuxtv.org/media_tree.git S: Odd Fixes F: drivers/media/usb/pwc/* +F: include/trace/events/pwc.h PWM FAN DRIVER M: Kamil Debski <kamil@wypas.org> -- 2.16.4 -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
On Sat, 19 Jan 2019 11:37:42 +0100, matwey.kornilov@gmail.com wrote:
From: "Matwey V. Kornilov" <matwey@sai.msu.ru>
Hello,
The series is to (partially) fix boo#1054610 "[BeagleBoneBlack, musb]: Broken isochronous transfer". The patches are intended to speedup URB completion handler for Philips webcams on armv7l hardware. The series has been accepted upstream and it is a part of linux-next branch now.
Could you please apply it to openSUSE-15.0, openSUSE-15.1, stable, master?
Reference: boo#1054610
Please update the bugzilla entry at first and discuss / agree the merge there. thanks, Takashi -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-kernel+owner@opensuse.org
participants (2)
-
matwey.kornilov＠gmail.com -
Takashi Iwai