[RFT PATCH] Input: sur40 - fix DMA handling in video capture

[RFT PATCH] Input: sur40 - fix DMA handling in video capture

[Karl Mehltretter]

Video capture has failed since commit 6eb0233ec2d0 ("usb: don't
inherity DMA properties for USB devices"). sur40 gives the USB
interface device to vb2_dma_sg, but the interface no longer has a DMA
mask. The mapping fails, leaving no entries for usb_sg_init().

Using the host controller device with vb2_dma_sg would give
usb_sg_init() an already DMA-mapped scatterlist. The USB core maps that
list for the host controller itself. Mapping it twice overwrites the DMA
addresses, and vb2 later unmaps addresses it does not own.

Imported dma-bufs cannot supply the CPU-side scatterlist that
usb_sg_init() needs. Importers may use only the DMA fields of the
attachment table. DMABUF_DEBUG makes this misuse deterministic by
clearing its page and length fields.

Use vb2_vmalloc for capture buffers. Receive each frame into a
driver-owned, page-backed scatterlist, then copy it through the vb2
mapping. Synchronize CPU writes to imported dma-bufs with
dma_buf_begin_cpu_access() and dma_buf_end_cpu_access().

Verified with a custom QEMU model and raw-gadget emulation. Not tested
on real hardware.

Fixes: 6eb0233ec2d0 ("usb: don't inherity DMA properties for USB devices")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
RFT because I do not have SUR40 hardware. Testing on a Microsoft Surface
2.0 / Samsung SUR40 would be appreciated.

The current RFT passed an x86-64 W=1 build of sur40.o with
DMABUF_DEBUG=y. Runtime testing used v7.3-rc4-70-gfe2ec83746e5 on QEMU
TCG with KASAN, DMA_API_DEBUG and DMABUF_DEBUG:

- a custom SUR40 model behind qemu-xhci and Intel IOMMU strict mode;
- raw-gadget on dummy_hcd.

Each setup captured 300 MMAP frames and 300 udmabuf frames. Every frame
contained the expected sequence data. The xHCI/IOMMU run produced no
DMA-API report. The dummy_hcd run reported udmabuf's separate maximum
segment-size issue, which also reproduces through DMA_BUF_IOCTL_SYNC
without sur40.

Nguyen Ngoc Thang's pending disconnect fixes move resource cleanup to a
v4l2 release callback:

https://lore.kernel.org/r/20260920113949.12726-1-ngocthang2710.1999@gmail.com/

If that series lands first, the sgl_free() added here must move to the
release callback as well.

 drivers/input/touchscreen/Kconfig |  5 ++-
 drivers/input/touchscreen/sur40.c | 68 ++++++++++++++++++++++++++-----
 2 files changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 9b9ae8ac3f7f..e433db3d340f 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1276,9 +1276,10 @@ config TOUCHSCREEN_SUN4I
 
 config TOUCHSCREEN_SUR40
 	tristate "Samsung SUR40 (Surface 2.0/PixelSense) touchscreen"
-	depends on USB && MEDIA_USB_SUPPORT && HAS_DMA
+	depends on USB && MEDIA_USB_SUPPORT
 	depends on VIDEO_DEV
-	select VIDEOBUF2_DMA_SG
+	select SGL_ALLOC
+	select VIDEOBUF2_VMALLOC
 	help
 	  Say Y here if you want support for the Samsung SUR40 touchscreen
 	  (also known as Microsoft Surface 2.0 or Microsoft PixelSense).
diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 09d8c5f8d09f..72e3f0fa1f4c 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -20,6 +20,7 @@
 #include <linux/kernel.h>
 #include <linux/errno.h>
 #include <linux/delay.h>
+#include <linux/dma-buf.h>
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/module.h>
@@ -36,7 +37,7 @@
 #include <media/v4l2-ioctl.h>
 #include <media/v4l2-ctrls.h>
 #include <media/videobuf2-v4l2.h>
-#include <media/videobuf2-dma-sg.h>
+#include <media/videobuf2-vmalloc.h>
 
 /* read 512 bytes from endpoint 0x86 -> get header + blobs */
 struct sur40_header {
@@ -221,6 +222,8 @@ struct sur40_state {
 
 	struct sur40_data *bulk_in_buffer;
 	size_t bulk_in_size;
+	struct scatterlist *video_sgl;
+	unsigned int video_nents;
 	u8 bulk_in_epaddr;
 	u8 vsvideo;
 
@@ -531,8 +534,10 @@ static void sur40_process_video(struct sur40_state *sur40)
 	struct sur40_image_header *img = (void *)(sur40->bulk_in_buffer);
 	struct sur40_buffer *new_buf;
 	struct usb_sg_request sgr;
-	struct sg_table *sgt;
+	unsigned int size = sur40->pix_fmt.sizeimage;
+	struct dma_buf *dbuf = NULL;
 	int result, bulk_read;
+	void *vaddr;
 
 	if (!vb2_start_streaming_called(&sur40->queue))
 		return;
@@ -579,11 +584,17 @@ static void sur40_process_video(struct sur40_state *sur40)
 
 	dev_dbg(sur40->dev, "header acquired\n");
 
-	sgt = vb2_dma_sg_plane_desc(&new_buf->vb.vb2_buf, 0);
+	vaddr = vb2_plane_vaddr(&new_buf->vb.vb2_buf, 0);
+	if (!vaddr)
+		goto err_poll;
 
+	/*
+	 * vb2_plane_vaddr() may return a vmalloc or vmap address. Receive
+	 * into page-backed memory so the USB core can map it for DMA.
+	 */
 	result = usb_sg_init(&sgr, sur40->usbdev,
 		usb_rcvbulkpipe(sur40->usbdev, VIDEO_ENDPOINT), 0,
-		sgt->sgl, sgt->nents, sur40->pix_fmt.sizeimage, 0);
+		sur40->video_sgl, sur40->video_nents, size, 0);
 	if (result < 0) {
 		dev_err(sur40->dev, "error %d in usb_sg_init\n", result);
 		goto err_poll;
@@ -595,6 +606,39 @@ static void sur40_process_video(struct sur40_state *sur40)
 		goto err_poll;
 	}
 
+	if (sgr.bytes != size) {
+		dev_err(sur40->dev, "short image (%zu of %u bytes)\n",
+			sgr.bytes, size);
+		goto err_poll;
+	}
+
+	/*
+	 * vb2_vmalloc does not synchronize CPU access to imported dma-bufs,
+	 * so bracket the copy into one here.
+	 */
+	if (new_buf->vb.vb2_buf.memory == VB2_MEMORY_DMABUF)
+		dbuf = new_buf->vb.vb2_buf.planes[0].dbuf;
+
+	if (dbuf) {
+		result = dma_buf_begin_cpu_access(dbuf, DMA_TO_DEVICE);
+		if (result) {
+			dev_err(sur40->dev, "error %d in begin_cpu_access\n",
+				result);
+			goto err_poll;
+		}
+	}
+
+	sg_copy_to_buffer(sur40->video_sgl, sur40->video_nents, vaddr, size);
+
+	if (dbuf) {
+		result = dma_buf_end_cpu_access(dbuf, DMA_TO_DEVICE);
+		if (result) {
+			dev_err(sur40->dev, "error %d in end_cpu_access\n",
+				result);
+			goto err_poll;
+		}
+	}
+
 	dev_dbg(sur40->dev, "image acquired\n");
 
 	/* return error if streaming was stopped in the meantime */
@@ -725,6 +769,13 @@ static int sur40_probe(struct usb_interface *interface,
 		goto err_free_input;
 	}
 
+	sur40->video_sgl = sgl_alloc(sur40_pix_format[0].sizeimage, GFP_KERNEL,
+				     &sur40->video_nents);
+	if (!sur40->video_sgl) {
+		error = -ENOMEM;
+		goto err_free_buffer;
+	}
+
 	/* register the video master device */
 	snprintf(sur40->v4l2.name, sizeof(sur40->v4l2.name), "%s", DRIVER_LONG);
 	error = v4l2_device_register(sur40->dev, &sur40->v4l2);
@@ -811,6 +862,7 @@ static int sur40_probe(struct usb_interface *interface,
 err_unreg_v4l2:
 	v4l2_device_unregister(&sur40->v4l2);
 err_free_buffer:
+	sgl_free(sur40->video_sgl);
 	kfree(sur40->bulk_in_buffer);
 err_free_input:
 	input_free_device(input);
@@ -831,6 +883,7 @@ static void sur40_disconnect(struct usb_interface *interface)
 	video_unregister_device(&sur40->vdev);
 	v4l2_device_unregister(&sur40->v4l2);
 
+	sgl_free(sur40->video_sgl);
 	kfree(sur40->bulk_in_buffer);
 	kfree(sur40);
 
@@ -1114,15 +1167,10 @@ static const struct vb2_ops sur40_queue_ops = {
 
 static const struct vb2_queue sur40_queue = {
 	.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
-	/*
-	 * VB2_USERPTR in currently not enabled: passing a user pointer to
-	 * dma-sg will result in segment sizes that are not a multiple of
-	 * 512 bytes, which is required by the host controller.
-	*/
 	.io_modes = VB2_MMAP | VB2_READ | VB2_DMABUF,
 	.buf_struct_size = sizeof(struct sur40_buffer),
 	.ops = &sur40_queue_ops,
-	.mem_ops = &vb2_dma_sg_memops,
+	.mem_ops = &vb2_vmalloc_memops,
 	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
 	.min_queued_buffers = 3,
 };

base-commit: 2c3418fffa9d037b2038a6db48be63f9e2291806
-- 
2.53.0

Re: [RFT PATCH] Input: sur40 - fix DMA handling in video capture

[kernel test robot]

Hi Karl,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 2c3418fffa9d037b2038a6db48be63f9e2291806]

url:    https://github.com/intel-lab-lkp/linux/commits/Karl-Mehltretter/Input-sur40-fix-DMA-handling-in-video-capture/20261007-074430
base:   2c3418fffa9d037b2038a6db48be63f9e2291806
patch link:    https://lore.kernel.org/r/20261007054430.40155-1-kmehltretter%40gmail.com
patch subject: [RFT PATCH] Input: sur40 - fix DMA handling in video capture
config: arm64-randconfig-1003-20261007 (https://download.01.org/0day-ci/archive/20261008/202610081252.0UqYKs2X-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261008/202610081252.0UqYKs2X-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610081252.0UqYKs2X-lkp@intel.com/

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> WARNING: modpost: drivers/input/touchscreen/sur40.ko: module uses symbol 'dma_buf_begin_cpu_access' from namespace 'DMA_BUF', but does not import it.
>> WARNING: modpost: drivers/input/touchscreen/sur40.ko: module uses symbol 'dma_buf_end_cpu_access' from namespace 'DMA_BUF', but does not import it.

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki