linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] add dma noncoherent API
@ 2025-07-04  9:57 Xu Yang
  2025-07-04  9:57 ` [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API Xu Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Xu Yang @ 2025-07-04  9:57 UTC (permalink / raw)
  To: ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh, xu.yang_2,
	mingo, tglx, andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

On architectures where there is no coherent caching such as ARM it's
proved that using dma_alloc_noncontiguous API and handling manually
the cache flushing will significantly improve performance.

Refer to:
commit 20e1dbf2bbe2 ("media: uvcvideo: Use dma_alloc_noncontiguous API")
commit 68d0c3311ec1 ("media: stk1160: use dma_alloc_noncontiguous API")

However, it's obvious that there is significant code duplication between
these two commits. Besides, a potential user USB Monitor may read outdated
data before the driver do DMA sync for CPU which will make the data
unreliable.

To reduce code duplication and avoid USB Monitor result unreliable, this
series will introduce DMA noncoherent API to USB core. And the USB core
layer will manage synchronization itself.

Then the last 2 patches have used the API.

I have tested uvcvideo driver. But I haven't tested stk1160 driver as I
don't have such boards. @Ezequiel Garcia, @Dafna Hirschfeld do you have
time to test it? Your support on this would be greatly appreciated.

Changes in v5:
 - improve if-else logic as suggested by Andy and Alan.
 - add Reviewed-by tag

Changes in v4:
 - https://lore.kernel.org/all/20250703103811.4048542-1-xu.yang_2@nxp.com/
 - improve if-else logic
 - remove uvc_stream_to_dmadev()

Changes in v3:
 - https://lore.kernel.org/all/20250702110222.3926355-1-xu.yang_2@nxp.com/
 - put Return section at the end of description
 - correct some abbreviations
 - remove usb_dma_noncoherent_sync_for_cpu() and
   usb_dma_noncoherent_sync_for_device()
 - do DMA sync in usb_hcd_map_urb_for_dma() and
   usb_hcd_unmap_urb_for_dma()
 - call flush_kernel_vmap_range() for OUT transfers
   and invalidate_kernel_vmap_range() for IN transfers 

Changes in v2:
 - https://lore.kernel.org/all/20250627101939.3649295-1-xu.yang_2@nxp.com/
 - handle it in USB core

v1:
 - https://lore.kernel.org/linux-usb/20250614132446.251218-1-xu.yang_2@nxp.com/

Xu Yang (3):
  usb: core: add dma-noncoherent buffer alloc and free API
  media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent()
  media: stk1160: use usb_alloc_noncoherent/usb_free_noncoherent()

 drivers/media/usb/stk1160/stk1160-v4l.c   |  4 --
 drivers/media/usb/stk1160/stk1160-video.c | 43 ++++--------
 drivers/media/usb/stk1160/stk1160.h       |  7 --
 drivers/media/usb/uvc/uvc_video.c         | 61 ++++-------------
 drivers/usb/core/hcd.c                    | 29 +++++---
 drivers/usb/core/usb.c                    | 80 +++++++++++++++++++++++
 include/linux/usb.h                       | 11 ++++
 7 files changed, 137 insertions(+), 98 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API
  2025-07-04  9:57 [PATCH v5 0/3] add dma noncoherent API Xu Yang
@ 2025-07-04  9:57 ` Xu Yang
  2025-07-04 13:51   ` Alan Stern
  2025-07-04  9:57 ` [PATCH v5 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent() Xu Yang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2025-07-04  9:57 UTC (permalink / raw)
  To: ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh, xu.yang_2,
	mingo, tglx, andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

This will add usb_alloc_noncoherent() and usb_free_noncoherent()
functions to support alloc and free buffer in a dma-noncoherent way.

To explicit manage the memory ownership for the kernel and device,
this will also add usb_dma_noncoherent_sync_for_cpu/device() functions
and call it at proper time.  The management requires the user save
sg_table returned by usb_alloc_noncoherent() to urb->sgt.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v5:
 - improve if-else logic again

Changes in v4:
 - improve if-else logic

Changes in v3:
 - put Return section at the end of description
 - correct some abbreviations
 - remove usb_dma_noncoherent_sync_for_cpu() and
   usb_dma_noncoherent_sync_for_device()
 - do DMA sync in usb_hcd_map_urb_for_dma() and
   usb_hcd_unmap_urb_for_dma()
 - call flush_kernel_vmap_range() for OUT transfers
   and invalidate_kernel_vmap_range() for IN transfers
---
 drivers/usb/core/hcd.c | 29 ++++++++++-----
 drivers/usb/core/usb.c | 80 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/usb.h    | 11 ++++++
 3 files changed, 112 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index c22de97432a0..03771bbc6c01 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1342,29 +1342,35 @@ void usb_hcd_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
 
 	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 	if (IS_ENABLED(CONFIG_HAS_DMA) &&
-	    (urb->transfer_flags & URB_DMA_MAP_SG))
+	    (urb->transfer_flags & URB_DMA_MAP_SG)) {
 		dma_unmap_sg(hcd->self.sysdev,
 				urb->sg,
 				urb->num_sgs,
 				dir);
-	else if (IS_ENABLED(CONFIG_HAS_DMA) &&
-		 (urb->transfer_flags & URB_DMA_MAP_PAGE))
+	} else if (IS_ENABLED(CONFIG_HAS_DMA) &&
+		 (urb->transfer_flags & URB_DMA_MAP_PAGE)) {
 		dma_unmap_page(hcd->self.sysdev,
 				urb->transfer_dma,
 				urb->transfer_buffer_length,
 				dir);
-	else if (IS_ENABLED(CONFIG_HAS_DMA) &&
-		 (urb->transfer_flags & URB_DMA_MAP_SINGLE))
+	} else if (IS_ENABLED(CONFIG_HAS_DMA) &&
+		 (urb->transfer_flags & URB_DMA_MAP_SINGLE)) {
 		dma_unmap_single(hcd->self.sysdev,
 				urb->transfer_dma,
 				urb->transfer_buffer_length,
 				dir);
-	else if (urb->transfer_flags & URB_MAP_LOCAL)
+	} else if (urb->transfer_flags & URB_MAP_LOCAL) {
 		hcd_free_coherent(urb->dev->bus,
 				&urb->transfer_dma,
 				&urb->transfer_buffer,
 				urb->transfer_buffer_length,
 				dir);
+	} else if ((urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) && urb->sgt) {
+		dma_sync_sgtable_for_cpu(hcd->self.sysdev, urb->sgt, dir);
+		if (dir == DMA_FROM_DEVICE)
+			invalidate_kernel_vmap_range(urb->transfer_buffer,
+						     urb->transfer_buffer_length);
+	}
 
 	/* Make it safe to call this routine more than once */
 	urb->transfer_flags &= ~(URB_DMA_MAP_SG | URB_DMA_MAP_PAGE |
@@ -1425,8 +1431,15 @@ int usb_hcd_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
 	}
 
 	dir = usb_urb_dir_in(urb) ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
-	if (urb->transfer_buffer_length != 0
-	    && !(urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP)) {
+	if (urb->transfer_flags & URB_NO_TRANSFER_DMA_MAP) {
+		if (!urb->sgt)
+			return 0;
+
+		if (dir == DMA_TO_DEVICE)
+			flush_kernel_vmap_range(urb->transfer_buffer,
+						urb->transfer_buffer_length);
+		dma_sync_sgtable_for_device(hcd->self.sysdev, urb->sgt, dir);
+	} else if (urb->transfer_buffer_length != 0) {
 		if (hcd->localmem_pool) {
 			ret = hcd_alloc_coherent(
 					urb->dev->bus, mem_flags,
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 118fa4c93a79..fca7735fc660 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -1030,6 +1030,86 @@ void usb_free_coherent(struct usb_device *dev, size_t size, void *addr,
 }
 EXPORT_SYMBOL_GPL(usb_free_coherent);
 
+/**
+ * usb_alloc_noncoherent - allocate dma-noncoherent buffer for URB_NO_xxx_DMA_MAP
+ * @dev: device the buffer will be used with
+ * @size: requested buffer size
+ * @mem_flags: affect whether allocation may block
+ * @dma: used to return DMA address of buffer
+ * @dir: DMA transfer direction
+ * @table: used to return sg_table of allocated memory
+ *
+ * To explicit manage the memory ownership for the kernel vs the device by
+ * USB core, the user needs save sg_table to urb->sgt. Then USB core will
+ * do DMA sync for CPU and device properly.
+ *
+ * When the buffer is no longer used, free it with usb_free_noncoherent().
+ *
+ * Return: Either null (indicating no buffer could be allocated), or the
+ * cpu-space pointer to a buffer that may be used to perform DMA to the
+ * specified device.  Such cpu-space buffers are returned along with the DMA
+ * address (through the pointer provided).
+ */
+void *usb_alloc_noncoherent(struct usb_device *dev, size_t size,
+			    gfp_t mem_flags, dma_addr_t *dma,
+			    enum dma_data_direction dir,
+			    struct sg_table **table)
+{
+	struct device *dmadev;
+	struct sg_table *sgt;
+	void *buffer;
+
+	if (!dev || !dev->bus)
+		return NULL;
+
+	dmadev = bus_to_hcd(dev->bus)->self.sysdev;
+
+	sgt = dma_alloc_noncontiguous(dmadev, size, dir, mem_flags, 0);
+	if (!sgt)
+		return NULL;
+
+	buffer = dma_vmap_noncontiguous(dmadev, size, sgt);
+	if (!buffer) {
+		dma_free_noncontiguous(dmadev, size, sgt, dir);
+		return NULL;
+	}
+
+	*table = sgt;
+	*dma = sg_dma_address(sgt->sgl);
+
+	return buffer;
+}
+EXPORT_SYMBOL_GPL(usb_alloc_noncoherent);
+
+/**
+ * usb_free_noncoherent - free memory allocated with usb_alloc_noncoherent()
+ * @dev: device the buffer was used with
+ * @size: requested buffer size
+ * @addr: CPU address of buffer
+ * @dir: DMA transfer direction
+ * @table: describe the allocated and DMA mapped memory,
+ *
+ * This reclaims an I/O buffer, letting it be reused.  The memory must have
+ * been allocated using usb_alloc_noncoherent(), and the parameters must match
+ * those provided in that allocation request.
+ */
+void usb_free_noncoherent(struct usb_device *dev, size_t size,
+			  void *addr, enum dma_data_direction dir,
+			  struct sg_table *table)
+{
+	struct device *dmadev;
+
+	if (!dev || !dev->bus)
+		return;
+	if (!addr)
+		return;
+
+	dmadev = bus_to_hcd(dev->bus)->self.sysdev;
+	dma_vunmap_noncontiguous(dmadev, addr);
+	dma_free_noncontiguous(dmadev, size, table, dir);
+}
+EXPORT_SYMBOL_GPL(usb_free_noncoherent);
+
 /*
  * Notifications of device and interface registration
  */
diff --git a/include/linux/usb.h b/include/linux/usb.h
index e8662843e68c..9ade441ab4c8 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -1619,6 +1619,7 @@ struct urb {
 	void *transfer_buffer;		/* (in) associated data buffer */
 	dma_addr_t transfer_dma;	/* (in) dma addr for transfer_buffer */
 	struct scatterlist *sg;		/* (in) scatter gather buffer list */
+	struct sg_table *sgt;		/* (in) scatter gather table for noncoherent buffer */
 	int num_mapped_sgs;		/* (internal) mapped sg entries */
 	int num_sgs;			/* (in) number of entries in the sg list */
 	u32 transfer_buffer_length;	/* (in) data buffer length */
@@ -1824,6 +1825,16 @@ void *usb_alloc_coherent(struct usb_device *dev, size_t size,
 void usb_free_coherent(struct usb_device *dev, size_t size,
 	void *addr, dma_addr_t dma);
 
+enum dma_data_direction;
+
+void *usb_alloc_noncoherent(struct usb_device *dev, size_t size,
+			    gfp_t mem_flags, dma_addr_t *dma,
+			    enum dma_data_direction dir,
+			    struct sg_table **table);
+void usb_free_noncoherent(struct usb_device *dev, size_t size,
+			  void *addr, enum dma_data_direction dir,
+			  struct sg_table *table);
+
 /*-------------------------------------------------------------------*
  *                         SYNCHRONOUS CALL SUPPORT                  *
  *-------------------------------------------------------------------*/
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v5 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent()
  2025-07-04  9:57 [PATCH v5 0/3] add dma noncoherent API Xu Yang
  2025-07-04  9:57 ` [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API Xu Yang
@ 2025-07-04  9:57 ` Xu Yang
  2025-07-07  9:50   ` Hans de Goede
  2025-07-04  9:57 ` [PATCH v5 3/3] media: stk1160: " Xu Yang
  2025-07-07 10:02 ` [PATCH v5 0/3] add dma noncoherent API Hans de Goede
  3 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2025-07-04  9:57 UTC (permalink / raw)
  To: ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh, xu.yang_2,
	mingo, tglx, andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

This will use USB noncoherent API to alloc/free urb buffers, then
uvc driver needn't to do dma sync operations by itself.

Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v5:
 - add Rb tag
Changes in v4:
 - remove uvc_stream_to_dmadev()
Changes in v3:
 - no changes
---
 drivers/media/usb/uvc/uvc_video.c | 61 +++++++------------------------
 1 file changed, 14 insertions(+), 47 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index e3567aeb0007..a75af314e46b 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1275,20 +1275,6 @@ static inline enum dma_data_direction uvc_stream_dir(
 		return DMA_TO_DEVICE;
 }
 
-static inline struct device *uvc_stream_to_dmadev(struct uvc_streaming *stream)
-{
-	return bus_to_hcd(stream->dev->udev->bus)->self.sysdev;
-}
-
-static int uvc_submit_urb(struct uvc_urb *uvc_urb, gfp_t mem_flags)
-{
-	/* Sync DMA. */
-	dma_sync_sgtable_for_device(uvc_stream_to_dmadev(uvc_urb->stream),
-				    uvc_urb->sgt,
-				    uvc_stream_dir(uvc_urb->stream));
-	return usb_submit_urb(uvc_urb->urb, mem_flags);
-}
-
 /*
  * uvc_video_decode_data_work: Asynchronous memcpy processing
  *
@@ -1310,7 +1296,7 @@ static void uvc_video_copy_data_work(struct work_struct *work)
 		uvc_queue_buffer_release(op->buf);
 	}
 
-	ret = uvc_submit_urb(uvc_urb, GFP_KERNEL);
+	ret = usb_submit_urb(uvc_urb->urb, GFP_KERNEL);
 	if (ret < 0)
 		dev_err(&uvc_urb->stream->intf->dev,
 			"Failed to resubmit video URB (%d).\n", ret);
@@ -1736,12 +1722,6 @@ static void uvc_video_complete(struct urb *urb)
 	/* Re-initialise the URB async work. */
 	uvc_urb->async_operations = 0;
 
-	/* Sync DMA and invalidate vmap range. */
-	dma_sync_sgtable_for_cpu(uvc_stream_to_dmadev(uvc_urb->stream),
-				 uvc_urb->sgt, uvc_stream_dir(stream));
-	invalidate_kernel_vmap_range(uvc_urb->buffer,
-				     uvc_urb->stream->urb_size);
-
 	/*
 	 * Process the URB headers, and optionally queue expensive memcpy tasks
 	 * to be deferred to a work queue.
@@ -1750,7 +1730,7 @@ static void uvc_video_complete(struct urb *urb)
 
 	/* If no async work is needed, resubmit the URB immediately. */
 	if (!uvc_urb->async_operations) {
-		ret = uvc_submit_urb(uvc_urb, GFP_ATOMIC);
+		ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
 		if (ret < 0)
 			dev_err(&stream->intf->dev,
 				"Failed to resubmit video URB (%d).\n", ret);
@@ -1765,17 +1745,15 @@ static void uvc_video_complete(struct urb *urb)
  */
 static void uvc_free_urb_buffers(struct uvc_streaming *stream)
 {
-	struct device *dma_dev = uvc_stream_to_dmadev(stream);
+	struct usb_device *udev = stream->dev->udev;
 	struct uvc_urb *uvc_urb;
 
 	for_each_uvc_urb(uvc_urb, stream) {
 		if (!uvc_urb->buffer)
 			continue;
 
-		dma_vunmap_noncontiguous(dma_dev, uvc_urb->buffer);
-		dma_free_noncontiguous(dma_dev, stream->urb_size, uvc_urb->sgt,
-				       uvc_stream_dir(stream));
-
+		usb_free_noncoherent(udev, stream->urb_size, uvc_urb->buffer,
+				     uvc_stream_dir(stream), uvc_urb->sgt);
 		uvc_urb->buffer = NULL;
 		uvc_urb->sgt = NULL;
 	}
@@ -1786,26 +1764,13 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream)
 static bool uvc_alloc_urb_buffer(struct uvc_streaming *stream,
 				 struct uvc_urb *uvc_urb, gfp_t gfp_flags)
 {
-	struct device *dma_dev = uvc_stream_to_dmadev(stream);
-
-	uvc_urb->sgt = dma_alloc_noncontiguous(dma_dev, stream->urb_size,
-					       uvc_stream_dir(stream),
-					       gfp_flags, 0);
-	if (!uvc_urb->sgt)
-		return false;
-	uvc_urb->dma = uvc_urb->sgt->sgl->dma_address;
-
-	uvc_urb->buffer = dma_vmap_noncontiguous(dma_dev, stream->urb_size,
-						 uvc_urb->sgt);
-	if (!uvc_urb->buffer) {
-		dma_free_noncontiguous(dma_dev, stream->urb_size,
-				       uvc_urb->sgt,
-				       uvc_stream_dir(stream));
-		uvc_urb->sgt = NULL;
-		return false;
-	}
+	struct usb_device *udev = stream->dev->udev;
 
-	return true;
+	uvc_urb->buffer = usb_alloc_noncoherent(udev, stream->urb_size,
+						gfp_flags, &uvc_urb->dma,
+						uvc_stream_dir(stream),
+						&uvc_urb->sgt);
+	return !!uvc_urb->buffer;
 }
 
 /*
@@ -1953,6 +1918,7 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
 		urb->complete = uvc_video_complete;
 		urb->number_of_packets = npackets;
 		urb->transfer_buffer_length = size;
+		urb->sgt = uvc_urb->sgt;
 
 		for (i = 0; i < npackets; ++i) {
 			urb->iso_frame_desc[i].offset = i * psize;
@@ -2009,6 +1975,7 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream,
 				  size, uvc_video_complete, uvc_urb);
 		urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
 		urb->transfer_dma = uvc_urb->dma;
+		urb->sgt = uvc_urb->sgt;
 
 		uvc_urb->urb = urb;
 	}
@@ -2120,7 +2087,7 @@ static int uvc_video_start_transfer(struct uvc_streaming *stream,
 
 	/* Submit the URBs. */
 	for_each_uvc_urb(uvc_urb, stream) {
-		ret = uvc_submit_urb(uvc_urb, gfp_flags);
+		ret = usb_submit_urb(uvc_urb->urb, gfp_flags);
 		if (ret < 0) {
 			dev_err(&stream->intf->dev,
 				"Failed to submit URB %u (%d).\n",
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v5 3/3] media: stk1160: use usb_alloc_noncoherent/usb_free_noncoherent()
  2025-07-04  9:57 [PATCH v5 0/3] add dma noncoherent API Xu Yang
  2025-07-04  9:57 ` [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API Xu Yang
  2025-07-04  9:57 ` [PATCH v5 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent() Xu Yang
@ 2025-07-04  9:57 ` Xu Yang
  2025-07-07 10:00   ` Hans de Goede
  2025-07-07 10:02 ` [PATCH v5 0/3] add dma noncoherent API Hans de Goede
  3 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2025-07-04  9:57 UTC (permalink / raw)
  To: ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh, xu.yang_2,
	mingo, tglx, andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

This will use USB noncoherent API to alloc/free urb buffers, then
stk1160 driver needn't to do dma sync operations by itself.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v5:
 - no changes
Changes in v4:
 - no changes
Changes in v3:
 - no changes
---
 drivers/media/usb/stk1160/stk1160-v4l.c   |  4 ---
 drivers/media/usb/stk1160/stk1160-video.c | 43 ++++++-----------------
 drivers/media/usb/stk1160/stk1160.h       |  7 ----
 3 files changed, 11 insertions(+), 43 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
index 5ba3d9c4b3fb..715ce1dcb304 100644
--- a/drivers/media/usb/stk1160/stk1160-v4l.c
+++ b/drivers/media/usb/stk1160/stk1160-v4l.c
@@ -232,10 +232,6 @@ static int stk1160_start_streaming(struct stk1160 *dev)
 
 	/* submit urbs and enables IRQ */
 	for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
-		struct stk1160_urb *stk_urb = &dev->isoc_ctl.urb_ctl[i];
-
-		dma_sync_sgtable_for_device(stk1160_get_dmadev(dev), stk_urb->sgt,
-					    DMA_FROM_DEVICE);
 		rc = usb_submit_urb(dev->isoc_ctl.urb_ctl[i].urb, GFP_KERNEL);
 		if (rc) {
 			stk1160_err("cannot submit urb[%d] (%d)\n", i, rc);
diff --git a/drivers/media/usb/stk1160/stk1160-video.c b/drivers/media/usb/stk1160/stk1160-video.c
index 9cbd957ecc90..416cb74377eb 100644
--- a/drivers/media/usb/stk1160/stk1160-video.c
+++ b/drivers/media/usb/stk1160/stk1160-video.c
@@ -298,9 +298,7 @@ static void stk1160_process_isoc(struct stk1160 *dev, struct urb *urb)
 static void stk1160_isoc_irq(struct urb *urb)
 {
 	int i, rc;
-	struct stk1160_urb *stk_urb = urb->context;
-	struct stk1160 *dev = stk_urb->dev;
-	struct device *dma_dev = stk1160_get_dmadev(dev);
+	struct stk1160 *dev = urb->context;
 
 	switch (urb->status) {
 	case 0:
@@ -315,10 +313,6 @@ static void stk1160_isoc_irq(struct urb *urb)
 		return;
 	}
 
-	invalidate_kernel_vmap_range(stk_urb->transfer_buffer,
-				     urb->transfer_buffer_length);
-	dma_sync_sgtable_for_cpu(dma_dev, stk_urb->sgt, DMA_FROM_DEVICE);
-
 	stk1160_process_isoc(dev, urb);
 
 	/* Reset urb buffers */
@@ -327,7 +321,6 @@ static void stk1160_isoc_irq(struct urb *urb)
 		urb->iso_frame_desc[i].actual_length = 0;
 	}
 
-	dma_sync_sgtable_for_device(dma_dev, stk_urb->sgt, DMA_FROM_DEVICE);
 	rc = usb_submit_urb(urb, GFP_ATOMIC);
 	if (rc)
 		stk1160_err("urb re-submit failed (%d)\n", rc);
@@ -365,11 +358,9 @@ void stk1160_cancel_isoc(struct stk1160 *dev)
 
 static void stk_free_urb(struct stk1160 *dev, struct stk1160_urb *stk_urb)
 {
-	struct device *dma_dev = stk1160_get_dmadev(dev);
-
-	dma_vunmap_noncontiguous(dma_dev, stk_urb->transfer_buffer);
-	dma_free_noncontiguous(dma_dev, stk_urb->urb->transfer_buffer_length,
-			       stk_urb->sgt, DMA_FROM_DEVICE);
+	usb_free_noncoherent(dev->udev, stk_urb->urb->transfer_buffer_length,
+			     stk_urb->transfer_buffer, DMA_FROM_DEVICE,
+			     stk_urb->sgt);
 	usb_free_urb(stk_urb->urb);
 
 	stk_urb->transfer_buffer = NULL;
@@ -410,32 +401,19 @@ void stk1160_uninit_isoc(struct stk1160 *dev)
 static int stk1160_fill_urb(struct stk1160 *dev, struct stk1160_urb *stk_urb,
 			    int sb_size, int max_packets)
 {
-	struct device *dma_dev = stk1160_get_dmadev(dev);
-
 	stk_urb->urb = usb_alloc_urb(max_packets, GFP_KERNEL);
 	if (!stk_urb->urb)
 		return -ENOMEM;
-	stk_urb->sgt = dma_alloc_noncontiguous(dma_dev, sb_size,
-					       DMA_FROM_DEVICE, GFP_KERNEL, 0);
-
-	/*
-	 * If the buffer allocation failed, we exit but return 0 since
-	 * we allow the driver working with less buffers
-	 */
-	if (!stk_urb->sgt)
-		goto free_urb;
 
-	stk_urb->transfer_buffer = dma_vmap_noncontiguous(dma_dev, sb_size,
-							  stk_urb->sgt);
+	stk_urb->transfer_buffer = usb_alloc_noncoherent(dev->udev, sb_size,
+							 GFP_KERNEL, &stk_urb->dma,
+							 DMA_FROM_DEVICE, &stk_urb->sgt);
 	if (!stk_urb->transfer_buffer)
-		goto free_sgt;
+		goto free_urb;
 
-	stk_urb->dma = stk_urb->sgt->sgl->dma_address;
 	stk_urb->dev = dev;
 	return 0;
-free_sgt:
-	dma_free_noncontiguous(dma_dev, sb_size, stk_urb->sgt, DMA_FROM_DEVICE);
-	stk_urb->sgt = NULL;
+
 free_urb:
 	usb_free_urb(stk_urb->urb);
 	stk_urb->urb = NULL;
@@ -494,12 +472,13 @@ int stk1160_alloc_isoc(struct stk1160 *dev)
 		urb->transfer_buffer = dev->isoc_ctl.urb_ctl[i].transfer_buffer;
 		urb->transfer_buffer_length = sb_size;
 		urb->complete = stk1160_isoc_irq;
-		urb->context = &dev->isoc_ctl.urb_ctl[i];
+		urb->context = dev;
 		urb->interval = 1;
 		urb->start_frame = 0;
 		urb->number_of_packets = max_packets;
 		urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
 		urb->transfer_dma = dev->isoc_ctl.urb_ctl[i].dma;
+		urb->sgt = dev->isoc_ctl.urb_ctl[i].sgt;
 
 		k = 0;
 		for (j = 0; j < max_packets; j++) {
diff --git a/drivers/media/usb/stk1160/stk1160.h b/drivers/media/usb/stk1160/stk1160.h
index 7b498d14ed7a..4cbcb0a03bab 100644
--- a/drivers/media/usb/stk1160/stk1160.h
+++ b/drivers/media/usb/stk1160/stk1160.h
@@ -16,8 +16,6 @@
 #include <media/videobuf2-v4l2.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
-#include <linux/usb.h>
-#include <linux/usb/hcd.h>
 
 #define STK1160_VERSION		"0.9.5"
 #define STK1160_VERSION_NUM	0x000905
@@ -195,8 +193,3 @@ void stk1160_select_input(struct stk1160 *dev);
 
 /* Provided by stk1160-ac97.c */
 void stk1160_ac97_setup(struct stk1160 *dev);
-
-static inline struct device *stk1160_get_dmadev(struct stk1160 *dev)
-{
-	return bus_to_hcd(dev->udev->bus)->self.sysdev;
-}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API
  2025-07-04  9:57 ` [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API Xu Yang
@ 2025-07-04 13:51   ` Alan Stern
  0 siblings, 0 replies; 9+ messages in thread
From: Alan Stern @ 2025-07-04 13:51 UTC (permalink / raw)
  To: Xu Yang
  Cc: ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh, mingo,
	tglx, andriy.shevchenko, viro, thomas.weissschuh, linux-media,
	linux-kernel, linux-usb, imx, jun.li

On Fri, Jul 04, 2025 at 05:57:49PM +0800, Xu Yang wrote:
> This will add usb_alloc_noncoherent() and usb_free_noncoherent()
> functions to support alloc and free buffer in a dma-noncoherent way.
> 
> To explicit manage the memory ownership for the kernel and device,
> this will also add usb_dma_noncoherent_sync_for_cpu/device() functions
> and call it at proper time.  The management requires the user save
> sg_table returned by usb_alloc_noncoherent() to urb->sgt.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Reviewed-by: Alan Stern <stern@rowland.harvard.edu>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent()
  2025-07-04  9:57 ` [PATCH v5 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent() Xu Yang
@ 2025-07-07  9:50   ` Hans de Goede
  0 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2025-07-07  9:50 UTC (permalink / raw)
  To: Xu Yang, ezequiel, mchehab, laurent.pinchart, gregkh, mingo, tglx,
	andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

Hi,

On 4-Jul-25 11:57, Xu Yang wrote:
> This will use USB noncoherent API to alloc/free urb buffers, then
> uvc driver needn't to do dma sync operations by itself.
> 
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans


> ---
> Changes in v5:
>  - add Rb tag
> Changes in v4:
>  - remove uvc_stream_to_dmadev()
> Changes in v3:
>  - no changes
> ---
>  drivers/media/usb/uvc/uvc_video.c | 61 +++++++------------------------
>  1 file changed, 14 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index e3567aeb0007..a75af314e46b 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1275,20 +1275,6 @@ static inline enum dma_data_direction uvc_stream_dir(
>  		return DMA_TO_DEVICE;
>  }
>  
> -static inline struct device *uvc_stream_to_dmadev(struct uvc_streaming *stream)
> -{
> -	return bus_to_hcd(stream->dev->udev->bus)->self.sysdev;
> -}
> -
> -static int uvc_submit_urb(struct uvc_urb *uvc_urb, gfp_t mem_flags)
> -{
> -	/* Sync DMA. */
> -	dma_sync_sgtable_for_device(uvc_stream_to_dmadev(uvc_urb->stream),
> -				    uvc_urb->sgt,
> -				    uvc_stream_dir(uvc_urb->stream));
> -	return usb_submit_urb(uvc_urb->urb, mem_flags);
> -}
> -
>  /*
>   * uvc_video_decode_data_work: Asynchronous memcpy processing
>   *
> @@ -1310,7 +1296,7 @@ static void uvc_video_copy_data_work(struct work_struct *work)
>  		uvc_queue_buffer_release(op->buf);
>  	}
>  
> -	ret = uvc_submit_urb(uvc_urb, GFP_KERNEL);
> +	ret = usb_submit_urb(uvc_urb->urb, GFP_KERNEL);
>  	if (ret < 0)
>  		dev_err(&uvc_urb->stream->intf->dev,
>  			"Failed to resubmit video URB (%d).\n", ret);
> @@ -1736,12 +1722,6 @@ static void uvc_video_complete(struct urb *urb)
>  	/* Re-initialise the URB async work. */
>  	uvc_urb->async_operations = 0;
>  
> -	/* Sync DMA and invalidate vmap range. */
> -	dma_sync_sgtable_for_cpu(uvc_stream_to_dmadev(uvc_urb->stream),
> -				 uvc_urb->sgt, uvc_stream_dir(stream));
> -	invalidate_kernel_vmap_range(uvc_urb->buffer,
> -				     uvc_urb->stream->urb_size);
> -
>  	/*
>  	 * Process the URB headers, and optionally queue expensive memcpy tasks
>  	 * to be deferred to a work queue.
> @@ -1750,7 +1730,7 @@ static void uvc_video_complete(struct urb *urb)
>  
>  	/* If no async work is needed, resubmit the URB immediately. */
>  	if (!uvc_urb->async_operations) {
> -		ret = uvc_submit_urb(uvc_urb, GFP_ATOMIC);
> +		ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
>  		if (ret < 0)
>  			dev_err(&stream->intf->dev,
>  				"Failed to resubmit video URB (%d).\n", ret);
> @@ -1765,17 +1745,15 @@ static void uvc_video_complete(struct urb *urb)
>   */
>  static void uvc_free_urb_buffers(struct uvc_streaming *stream)
>  {
> -	struct device *dma_dev = uvc_stream_to_dmadev(stream);
> +	struct usb_device *udev = stream->dev->udev;
>  	struct uvc_urb *uvc_urb;
>  
>  	for_each_uvc_urb(uvc_urb, stream) {
>  		if (!uvc_urb->buffer)
>  			continue;
>  
> -		dma_vunmap_noncontiguous(dma_dev, uvc_urb->buffer);
> -		dma_free_noncontiguous(dma_dev, stream->urb_size, uvc_urb->sgt,
> -				       uvc_stream_dir(stream));
> -
> +		usb_free_noncoherent(udev, stream->urb_size, uvc_urb->buffer,
> +				     uvc_stream_dir(stream), uvc_urb->sgt);
>  		uvc_urb->buffer = NULL;
>  		uvc_urb->sgt = NULL;
>  	}
> @@ -1786,26 +1764,13 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream)
>  static bool uvc_alloc_urb_buffer(struct uvc_streaming *stream,
>  				 struct uvc_urb *uvc_urb, gfp_t gfp_flags)
>  {
> -	struct device *dma_dev = uvc_stream_to_dmadev(stream);
> -
> -	uvc_urb->sgt = dma_alloc_noncontiguous(dma_dev, stream->urb_size,
> -					       uvc_stream_dir(stream),
> -					       gfp_flags, 0);
> -	if (!uvc_urb->sgt)
> -		return false;
> -	uvc_urb->dma = uvc_urb->sgt->sgl->dma_address;
> -
> -	uvc_urb->buffer = dma_vmap_noncontiguous(dma_dev, stream->urb_size,
> -						 uvc_urb->sgt);
> -	if (!uvc_urb->buffer) {
> -		dma_free_noncontiguous(dma_dev, stream->urb_size,
> -				       uvc_urb->sgt,
> -				       uvc_stream_dir(stream));
> -		uvc_urb->sgt = NULL;
> -		return false;
> -	}
> +	struct usb_device *udev = stream->dev->udev;
>  
> -	return true;
> +	uvc_urb->buffer = usb_alloc_noncoherent(udev, stream->urb_size,
> +						gfp_flags, &uvc_urb->dma,
> +						uvc_stream_dir(stream),
> +						&uvc_urb->sgt);
> +	return !!uvc_urb->buffer;
>  }
>  
>  /*
> @@ -1953,6 +1918,7 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
>  		urb->complete = uvc_video_complete;
>  		urb->number_of_packets = npackets;
>  		urb->transfer_buffer_length = size;
> +		urb->sgt = uvc_urb->sgt;
>  
>  		for (i = 0; i < npackets; ++i) {
>  			urb->iso_frame_desc[i].offset = i * psize;
> @@ -2009,6 +1975,7 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream,
>  				  size, uvc_video_complete, uvc_urb);
>  		urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
>  		urb->transfer_dma = uvc_urb->dma;
> +		urb->sgt = uvc_urb->sgt;
>  
>  		uvc_urb->urb = urb;
>  	}
> @@ -2120,7 +2087,7 @@ static int uvc_video_start_transfer(struct uvc_streaming *stream,
>  
>  	/* Submit the URBs. */
>  	for_each_uvc_urb(uvc_urb, stream) {
> -		ret = uvc_submit_urb(uvc_urb, gfp_flags);
> +		ret = usb_submit_urb(uvc_urb->urb, gfp_flags);
>  		if (ret < 0) {
>  			dev_err(&stream->intf->dev,
>  				"Failed to submit URB %u (%d).\n",


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 3/3] media: stk1160: use usb_alloc_noncoherent/usb_free_noncoherent()
  2025-07-04  9:57 ` [PATCH v5 3/3] media: stk1160: " Xu Yang
@ 2025-07-07 10:00   ` Hans de Goede
  0 siblings, 0 replies; 9+ messages in thread
From: Hans de Goede @ 2025-07-07 10:00 UTC (permalink / raw)
  To: Xu Yang, ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh,
	mingo, tglx, andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

Hi,

On 4-Jul-25 11:57, Xu Yang wrote:
> This will use USB noncoherent API to alloc/free urb buffers, then
> stk1160 driver needn't to do dma sync operations by itself.
> 
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
> Changes in v5:
>  - no changes
> Changes in v4:
>  - no changes
> Changes in v3:
>  - no changes
> ---
>  drivers/media/usb/stk1160/stk1160-v4l.c   |  4 ---
>  drivers/media/usb/stk1160/stk1160-video.c | 43 ++++++-----------------
>  drivers/media/usb/stk1160/stk1160.h       |  7 ----
>  3 files changed, 11 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/media/usb/stk1160/stk1160-v4l.c b/drivers/media/usb/stk1160/stk1160-v4l.c
> index 5ba3d9c4b3fb..715ce1dcb304 100644
> --- a/drivers/media/usb/stk1160/stk1160-v4l.c
> +++ b/drivers/media/usb/stk1160/stk1160-v4l.c
> @@ -232,10 +232,6 @@ static int stk1160_start_streaming(struct stk1160 *dev)
>  
>  	/* submit urbs and enables IRQ */
>  	for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
> -		struct stk1160_urb *stk_urb = &dev->isoc_ctl.urb_ctl[i];
> -
> -		dma_sync_sgtable_for_device(stk1160_get_dmadev(dev), stk_urb->sgt,
> -					    DMA_FROM_DEVICE);
>  		rc = usb_submit_urb(dev->isoc_ctl.urb_ctl[i].urb, GFP_KERNEL);
>  		if (rc) {
>  			stk1160_err("cannot submit urb[%d] (%d)\n", i, rc);
> diff --git a/drivers/media/usb/stk1160/stk1160-video.c b/drivers/media/usb/stk1160/stk1160-video.c
> index 9cbd957ecc90..416cb74377eb 100644
> --- a/drivers/media/usb/stk1160/stk1160-video.c
> +++ b/drivers/media/usb/stk1160/stk1160-video.c
> @@ -298,9 +298,7 @@ static void stk1160_process_isoc(struct stk1160 *dev, struct urb *urb)
>  static void stk1160_isoc_irq(struct urb *urb)
>  {
>  	int i, rc;
> -	struct stk1160_urb *stk_urb = urb->context;
> -	struct stk1160 *dev = stk_urb->dev;
> -	struct device *dma_dev = stk1160_get_dmadev(dev);
> +	struct stk1160 *dev = urb->context;
>  
>  	switch (urb->status) {
>  	case 0:
> @@ -315,10 +313,6 @@ static void stk1160_isoc_irq(struct urb *urb)
>  		return;
>  	}
>  
> -	invalidate_kernel_vmap_range(stk_urb->transfer_buffer,
> -				     urb->transfer_buffer_length);
> -	dma_sync_sgtable_for_cpu(dma_dev, stk_urb->sgt, DMA_FROM_DEVICE);
> -
>  	stk1160_process_isoc(dev, urb);
>  
>  	/* Reset urb buffers */
> @@ -327,7 +321,6 @@ static void stk1160_isoc_irq(struct urb *urb)
>  		urb->iso_frame_desc[i].actual_length = 0;
>  	}
>  
> -	dma_sync_sgtable_for_device(dma_dev, stk_urb->sgt, DMA_FROM_DEVICE);
>  	rc = usb_submit_urb(urb, GFP_ATOMIC);
>  	if (rc)
>  		stk1160_err("urb re-submit failed (%d)\n", rc);
> @@ -365,11 +358,9 @@ void stk1160_cancel_isoc(struct stk1160 *dev)
>  
>  static void stk_free_urb(struct stk1160 *dev, struct stk1160_urb *stk_urb)
>  {
> -	struct device *dma_dev = stk1160_get_dmadev(dev);
> -
> -	dma_vunmap_noncontiguous(dma_dev, stk_urb->transfer_buffer);
> -	dma_free_noncontiguous(dma_dev, stk_urb->urb->transfer_buffer_length,
> -			       stk_urb->sgt, DMA_FROM_DEVICE);
> +	usb_free_noncoherent(dev->udev, stk_urb->urb->transfer_buffer_length,
> +			     stk_urb->transfer_buffer, DMA_FROM_DEVICE,
> +			     stk_urb->sgt);
>  	usb_free_urb(stk_urb->urb);
>  
>  	stk_urb->transfer_buffer = NULL;
> @@ -410,32 +401,19 @@ void stk1160_uninit_isoc(struct stk1160 *dev)
>  static int stk1160_fill_urb(struct stk1160 *dev, struct stk1160_urb *stk_urb,
>  			    int sb_size, int max_packets)
>  {
> -	struct device *dma_dev = stk1160_get_dmadev(dev);
> -
>  	stk_urb->urb = usb_alloc_urb(max_packets, GFP_KERNEL);
>  	if (!stk_urb->urb)
>  		return -ENOMEM;
> -	stk_urb->sgt = dma_alloc_noncontiguous(dma_dev, sb_size,
> -					       DMA_FROM_DEVICE, GFP_KERNEL, 0);
> -
> -	/*
> -	 * If the buffer allocation failed, we exit but return 0 since
> -	 * we allow the driver working with less buffers
> -	 */
> -	if (!stk_urb->sgt)
> -		goto free_urb;
>  
> -	stk_urb->transfer_buffer = dma_vmap_noncontiguous(dma_dev, sb_size,
> -							  stk_urb->sgt);
> +	stk_urb->transfer_buffer = usb_alloc_noncoherent(dev->udev, sb_size,
> +							 GFP_KERNEL, &stk_urb->dma,
> +							 DMA_FROM_DEVICE, &stk_urb->sgt);
>  	if (!stk_urb->transfer_buffer)
> -		goto free_sgt;
> +		goto free_urb;
>  
> -	stk_urb->dma = stk_urb->sgt->sgl->dma_address;
>  	stk_urb->dev = dev;
>  	return 0;
> -free_sgt:
> -	dma_free_noncontiguous(dma_dev, sb_size, stk_urb->sgt, DMA_FROM_DEVICE);
> -	stk_urb->sgt = NULL;
> +
>  free_urb:
>  	usb_free_urb(stk_urb->urb);
>  	stk_urb->urb = NULL;
> @@ -494,12 +472,13 @@ int stk1160_alloc_isoc(struct stk1160 *dev)
>  		urb->transfer_buffer = dev->isoc_ctl.urb_ctl[i].transfer_buffer;
>  		urb->transfer_buffer_length = sb_size;
>  		urb->complete = stk1160_isoc_irq;
> -		urb->context = &dev->isoc_ctl.urb_ctl[i];
> +		urb->context = dev;
>  		urb->interval = 1;
>  		urb->start_frame = 0;
>  		urb->number_of_packets = max_packets;
>  		urb->transfer_flags = URB_ISO_ASAP | URB_NO_TRANSFER_DMA_MAP;
>  		urb->transfer_dma = dev->isoc_ctl.urb_ctl[i].dma;
> +		urb->sgt = dev->isoc_ctl.urb_ctl[i].sgt;
>  
>  		k = 0;
>  		for (j = 0; j < max_packets; j++) {
> diff --git a/drivers/media/usb/stk1160/stk1160.h b/drivers/media/usb/stk1160/stk1160.h
> index 7b498d14ed7a..4cbcb0a03bab 100644
> --- a/drivers/media/usb/stk1160/stk1160.h
> +++ b/drivers/media/usb/stk1160/stk1160.h
> @@ -16,8 +16,6 @@
>  #include <media/videobuf2-v4l2.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
> -#include <linux/usb.h>
> -#include <linux/usb/hcd.h>
>  
>  #define STK1160_VERSION		"0.9.5"
>  #define STK1160_VERSION_NUM	0x000905
> @@ -195,8 +193,3 @@ void stk1160_select_input(struct stk1160 *dev);
>  
>  /* Provided by stk1160-ac97.c */
>  void stk1160_ac97_setup(struct stk1160 *dev);
> -
> -static inline struct device *stk1160_get_dmadev(struct stk1160 *dev)
> -{
> -	return bus_to_hcd(dev->udev->bus)->self.sysdev;
> -}


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 0/3] add dma noncoherent API
  2025-07-04  9:57 [PATCH v5 0/3] add dma noncoherent API Xu Yang
                   ` (2 preceding siblings ...)
  2025-07-04  9:57 ` [PATCH v5 3/3] media: stk1160: " Xu Yang
@ 2025-07-07 10:02 ` Hans de Goede
  2025-07-09 10:12   ` Greg KH
  3 siblings, 1 reply; 9+ messages in thread
From: Hans de Goede @ 2025-07-07 10:02 UTC (permalink / raw)
  To: Xu Yang, ezequiel, mchehab, laurent.pinchart, hdegoede, gregkh,
	mingo, tglx, andriy.shevchenko, viro, thomas.weissschuh
  Cc: linux-media, linux-kernel, linux-usb, imx, jun.li

Hi all,

On 4-Jul-25 11:57, Xu Yang wrote:
> On architectures where there is no coherent caching such as ARM it's
> proved that using dma_alloc_noncontiguous API and handling manually
> the cache flushing will significantly improve performance.
> 
> Refer to:
> commit 20e1dbf2bbe2 ("media: uvcvideo: Use dma_alloc_noncontiguous API")
> commit 68d0c3311ec1 ("media: stk1160: use dma_alloc_noncontiguous API")
> 
> However, it's obvious that there is significant code duplication between
> these two commits. Besides, a potential user USB Monitor may read outdated
> data before the driver do DMA sync for CPU which will make the data
> unreliable.
> 
> To reduce code duplication and avoid USB Monitor result unreliable, this
> series will introduce DMA noncoherent API to USB core. And the USB core
> layer will manage synchronization itself.
> 
> Then the last 2 patches have used the API.
> 
> I have tested uvcvideo driver. But I haven't tested stk1160 driver as I
> don't have such boards. @Ezequiel Garcia, @Dafna Hirschfeld do you have
> time to test it? Your support on this would be greatly appreciated.

It seems that patches 1 + 2 are ready for merging now
(for patch 3 we should probably wait for testing).

I think that it would be best for both patches 1 + 2 to
be merged through the USB tree. The changed code in the UVC
driver is not touched that often so I do not expect any
conflicts.

Regards,

Hans




> Changes in v5:
>  - improve if-else logic as suggested by Andy and Alan.
>  - add Reviewed-by tag
> 
> Changes in v4:
>  - https://lore.kernel.org/all/20250703103811.4048542-1-xu.yang_2@nxp.com/
>  - improve if-else logic
>  - remove uvc_stream_to_dmadev()
> 
> Changes in v3:
>  - https://lore.kernel.org/all/20250702110222.3926355-1-xu.yang_2@nxp.com/
>  - put Return section at the end of description
>  - correct some abbreviations
>  - remove usb_dma_noncoherent_sync_for_cpu() and
>    usb_dma_noncoherent_sync_for_device()
>  - do DMA sync in usb_hcd_map_urb_for_dma() and
>    usb_hcd_unmap_urb_for_dma()
>  - call flush_kernel_vmap_range() for OUT transfers
>    and invalidate_kernel_vmap_range() for IN transfers 
> 
> Changes in v2:
>  - https://lore.kernel.org/all/20250627101939.3649295-1-xu.yang_2@nxp.com/
>  - handle it in USB core
> 
> v1:
>  - https://lore.kernel.org/linux-usb/20250614132446.251218-1-xu.yang_2@nxp.com/
> 
> Xu Yang (3):
>   usb: core: add dma-noncoherent buffer alloc and free API
>   media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent()
>   media: stk1160: use usb_alloc_noncoherent/usb_free_noncoherent()
> 
>  drivers/media/usb/stk1160/stk1160-v4l.c   |  4 --
>  drivers/media/usb/stk1160/stk1160-video.c | 43 ++++--------
>  drivers/media/usb/stk1160/stk1160.h       |  7 --
>  drivers/media/usb/uvc/uvc_video.c         | 61 ++++-------------
>  drivers/usb/core/hcd.c                    | 29 +++++---
>  drivers/usb/core/usb.c                    | 80 +++++++++++++++++++++++
>  include/linux/usb.h                       | 11 ++++
>  7 files changed, 137 insertions(+), 98 deletions(-)
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 0/3] add dma noncoherent API
  2025-07-07 10:02 ` [PATCH v5 0/3] add dma noncoherent API Hans de Goede
@ 2025-07-09 10:12   ` Greg KH
  0 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2025-07-09 10:12 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Xu Yang, ezequiel, mchehab, laurent.pinchart, hdegoede, mingo,
	tglx, andriy.shevchenko, viro, thomas.weissschuh, linux-media,
	linux-kernel, linux-usb, imx, jun.li

On Mon, Jul 07, 2025 at 12:02:41PM +0200, Hans de Goede wrote:
> Hi all,
> 
> On 4-Jul-25 11:57, Xu Yang wrote:
> > On architectures where there is no coherent caching such as ARM it's
> > proved that using dma_alloc_noncontiguous API and handling manually
> > the cache flushing will significantly improve performance.
> > 
> > Refer to:
> > commit 20e1dbf2bbe2 ("media: uvcvideo: Use dma_alloc_noncontiguous API")
> > commit 68d0c3311ec1 ("media: stk1160: use dma_alloc_noncontiguous API")
> > 
> > However, it's obvious that there is significant code duplication between
> > these two commits. Besides, a potential user USB Monitor may read outdated
> > data before the driver do DMA sync for CPU which will make the data
> > unreliable.
> > 
> > To reduce code duplication and avoid USB Monitor result unreliable, this
> > series will introduce DMA noncoherent API to USB core. And the USB core
> > layer will manage synchronization itself.
> > 
> > Then the last 2 patches have used the API.
> > 
> > I have tested uvcvideo driver. But I haven't tested stk1160 driver as I
> > don't have such boards. @Ezequiel Garcia, @Dafna Hirschfeld do you have
> > time to test it? Your support on this would be greatly appreciated.
> 
> It seems that patches 1 + 2 are ready for merging now
> (for patch 3 we should probably wait for testing).
> 
> I think that it would be best for both patches 1 + 2 to
> be merged through the USB tree. The changed code in the UVC
> driver is not touched that often so I do not expect any
> conflicts.

Ok, thanks, I'll take them through the USB tree now.

greg k-h

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-07-09 10:12 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-04  9:57 [PATCH v5 0/3] add dma noncoherent API Xu Yang
2025-07-04  9:57 ` [PATCH v5 1/3] usb: core: add dma-noncoherent buffer alloc and free API Xu Yang
2025-07-04 13:51   ` Alan Stern
2025-07-04  9:57 ` [PATCH v5 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent() Xu Yang
2025-07-07  9:50   ` Hans de Goede
2025-07-04  9:57 ` [PATCH v5 3/3] media: stk1160: " Xu Yang
2025-07-07 10:00   ` Hans de Goede
2025-07-07 10:02 ` [PATCH v5 0/3] add dma noncoherent API Hans de Goede
2025-07-09 10:12   ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).