Based on the reality[1][2] that vb2_dma_sg_alloc() can't alloc buffer with
device DMA limits, those device will always get below error: "swiotlb
buffer is full (sz: 393216 bytes), total 65536 (slots), used 2358 (slots)"
and the uvc gadget function can't work at all.
The videobuf2-dma-sg.c driver doesn't has a formal improve about this issue
till now. For UVC gadget, the videobuf2 subsystem doesn't do dma_map() on
vmalloc returned big buffer when allocate the video buffers, however, it do
it for dma_sg returned buffer. So the issue happens for vb2_dma_sg_alloc().
To workaround the issue, lets retry vb2_reqbufs() with
vb_vmalloc_memops if it fails to allocate buffer with vb2_dma_sg_memops.
If use vmalloced buffer, UVC gadget will allocate some small buffers for
each usb_request to do dma transfer, then uvc driver will memcopy data
from big buffer to small buffer.
Link[1]: https://lore.kernel.org/linux-media/20230828075420.2009568-1-anle.pan@nxp.com/
Link[2]: https://lore.kernel.org/linux-media/20230914145812.12851-1-hui.fang@nxp.com/
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260113-uvc-gadget-fix-patch-v2-4-62950ef5bcb5@nxp.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Current req_payload_size calculation has 2 issue:
(1) When the first time calculate req_payload_size for all the buffers,
reqs_per_frame = 0 will be the divisor of DIV_ROUND_UP(). So
the result is undefined.
This happens because VIDIOC_STREAMON is always executed after
VIDIOC_QBUF. So video->reqs_per_frame will be 0 until VIDIOC_STREAMON
is run.
(2) The buf->req_payload_size may be bigger than max_req_size.
Take YUYV pixel format as example:
If bInterval = 1, video->interval = 666666, high-speed:
video->reqs_per_frame = 666666 / 1250 = 534
720p: buf->req_payload_size = 1843200 / 534 = 3452
1080p: buf->req_payload_size = 4147200 / 534 = 7766
Based on such req_payload_size, the controller can't run normally.
To fix above issue, assign max_req_size to buf->req_payload_size when
video->reqs_per_frame = 0. And limit buf->req_payload_size to
video->req_size if it's large than video->req_size. Since max_req_size
is used at many place, add it to struct uvc_video and set the value once
endpoint is enabled.
Fixes: 98ad032915 ("usb: gadget: uvc: set req_length based on payload by nreqs instead of req_size")
Cc: stable@vger.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
Link: https://patch.msgid.link/20260113-uvc-gadget-fix-patch-v2-1-62950ef5bcb5@nxp.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Compressed formats generate content depending amount of data that is set
in the vb2 buffer by the payload_size. When streaming those formats it
is better to scatter that smaller data over all requests. This patch is
doing that by introducing the calculated req_payload_size which is
updated by each frame. It the uses this amount of data to fill the
isoc requests instead of the video->req_size.
For uncompressed formats it will not make a difference since the payload
size will be equal to the imagesize. Therefore the code will have no
effecta as req_payload_size will be equal to req_size.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-6-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch is removing the initial imprecise and limited calculation of
requests needed to be used from the queue_setup callback. It instead
introduces the uvc_video_prep_requests function which is called
immediately before the request allocation.
With the information of the usb frame interval length it is possible to
calculate the number of requests needed during one frame duration.
Based on the calculated number of requests and the imagesize we
calculate the actual size per request. This calculation has the benefit
that the frame data is equally distributed over all allocated requests.
When the req_size is not in the range for the actually configured
max_req_size configured for the overall bandwidth we fallback
to use the max_req_size instead.
Since this calculations are only important for isoc transfers we just
use max_request_size for bulk and skip it.
As video->req_size will be recalculated on every video_enable resetting
it to 0 is not necessary anymore.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-5-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This reverts commit 19905240ae.
It was a new feature, and it doesn't even work properly yet, so revert
it from this branch as it is not needed for 6.1-final.
Cc: Michael Grzeschik <m.grzeschik@pengutronix.de>
Cc: stable <stable@kernel.org>
Fixes: 19905240ae ("usb: gadget: uvc: limit isoc_sg to super speed gadgets")
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If there is a transmission error the buffer will be returned too early,
causing a memory fault as subsequent requests for that buffer are still
queued up to be sent. Refactor the error handling to wait for the final
request to come in before reporting back the buffer to userspace for all
transfer types (bulk/isoc/isoc_sg). This ensures userspace knows if the
frame was successfully sent.
Fixes: e81e7f9a0e ("usb: gadget: uvc: add scatter gather support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Dan Vacura <w36195@motorola.com>
Link: https://lore.kernel.org/r/20221018215044.765044-4-w36195@motorola.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The current limitation of possible number of requests being handled is
dependent on the gadget speed. It makes more sense to depend on the
typical frame size when calculating the number of requests. This patch
is changing this and is using the previous limits as boundaries for
reasonable minimum and maximum number of requests.
For a 1080p jpeg encoded video stream with a maximum imagesize of
e.g. 800kB with a maxburst of 8 and an multiplier of 1 the resulting
number of requests is calculated to 49.
800768 1
nreqs = ------ * -------------- ~= 49
2 (1024 * 8 * 1)
Tested-by: Dan Vacura <w36195@motorola.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20220529223848.105914-2-m.grzeschik@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We need the USB fixes in here, and this resolves a merge issue in
drivers/usb/dwc3/drd.c
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The function uvcg_queue_next_buffer is used different than its name
suggests. The return value nextbuf is never used by any caller. This
patch reworks the function to its actual purpose, by removing the unused
code and renaming it. The function name uvcg_complete_buffer makes it
more clear that it is actually marking the current video buffer as
complete.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20220402232744.3622565-2-m.grzeschik@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The pause flag doesn't appear to serve a purpose as it is possible for
the pump worker thread to not be running when new buffers are queued.
Before the following change was introduced: "43cd0023872e usb: gadget:
uvc_video: add worker to handle the frame pumping", the pause flag
status was discarded and never returned. This masked a current issues of
the userspace receiving non-zero values (considered unsuccessful) and
prevents trace_v4l2_qbuf() from being called.
Signed-off-by: Dan Vacura <w36195@motorola.com>
Link: https://lore.kernel.org/r/20220413192402.26063-1-w36195@motorola.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
During the uvcg_video_pump() process, if an error occurs and
uvcg_queue_cancel() is called, the buffer queue will be cleared out, but
the current marker (queue->buf_used) of the active buffer (no longer
active) is not reset. On the next iteration of uvcg_video_pump() the
stale buf_used count will be used and the logic of min((unsigned
int)len, buf->bytesused - queue->buf_used) may incorrectly calculate a
nbytes size, causing an invalid memory access.
[80802.185460][ T315] configfs-gadget gadget: uvc: VS request completed
with status -18.
[80802.185519][ T315] configfs-gadget gadget: uvc: VS request completed
with status -18.
...
uvcg_queue_cancel() is called and the queue is cleared out, but the
marker queue->buf_used is not reset.
...
[80802.262328][ T8682] Unable to handle kernel paging request at virtual
address ffffffc03af9f000
...
...
[80802.263138][ T8682] Call trace:
[80802.263146][ T8682] __memcpy+0x12c/0x180
[80802.263155][ T8682] uvcg_video_pump+0xcc/0x1e0
[80802.263165][ T8682] process_one_work+0x2cc/0x568
[80802.263173][ T8682] worker_thread+0x28c/0x518
[80802.263181][ T8682] kthread+0x160/0x170
[80802.263188][ T8682] ret_from_fork+0x10/0x18
[80802.263198][ T8682] Code: a8c12829 a88130cb a8c130
Fixes: d692522577 ("usb: gadget/uvc: Port UVC webcam gadget to use videobuf2 framework")
Cc: <stable@vger.kernel.org>
Signed-off-by: Dan Vacura <w36195@motorola.com>
Link: https://lore.kernel.org/r/20220331184024.23918-1-w36195@motorola.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch adds support for scatter gather transfers. If the underlying
gadgets sg_supported == true, then the videeobuf2-dma-sg is used and the
encode routine maps all scatter entries to separate scatterlists for the
usb gadget.
When streaming 1080p with request size of 1024 times 3 bytes top shows a
difference of about 6.4% CPU load applying this patch:
PID USER PR NI VIRT RES %CPU %MEM TIME+ S COMMAND
64 root 0 -20 0.0m 0.0m 7.7 0.0 0:01.25 I [kworker/u5:0-uvcvideo]
83 root 0 -20 0.0m 0.0m 4.5 0.0 0:03.71 I [kworker/u5:3-uvcvideo]
307 root -51 0 0.0m 0.0m 3.8 0.0 0:01.05 S [irq/51-dwc3]
vs.
64 root 0 -20 0.0m 0.0m 5.8 0.0 0:01.79 I [kworker/u5:0-uvcvideo]
306 root -51 0 0.0m 0.0m 3.2 0.0 0:01.97 S [irq/51-dwc3]
82 root 0 -20 0.0m 0.0m 0.6 0.0 0:01.86 I [kworker/u5:1-uvcvideo]
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20210628155311.16762-5-m.grzeschik@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The vb2_ops structure can be const as it is only stored in the ops
field of a vb2_queue structure and this field is const.
Done with the help of Coccinelle.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
This implements the V4L2 part of the request support. The main
change is that vb2_qbuf and vb2_prepare_buf now have a new
media_device pointer. This required changes to several drivers
that did not use the vb2_ioctl_qbuf/prepare_buf helper functions.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
It's good to have SPDX identifiers in all files to make it easier to
audit the kernel tree for correct licenses.
Update the drivers/usb/ and include/linux/usb* files with the correct
SPDX license identifier based on the license text in the file itself.
The SPDX identifier is a legally binding shorthand, which can be used
instead of the full boiler plate text.
This work is based on a script and data from Thomas Gleixner, Philippe
Ombredanne, and Kate Stewart.
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Kate Stewart <kstewart@linuxfoundation.org>
Cc: Philippe Ombredanne <pombredanne@nexb.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Felipe Balbi <felipe.balbi@linux.intel.com>
Acked-by: Johan Hovold <johan@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Make this a proper typed array. Drop the old allocate context code since
that is no longer used.
Note that the memops functions now get a struct device pointer instead of
the struct device ** that was there initially (actually a void pointer to
a struct containing only a struct device pointer).
This code is now a lot cleaner.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>