linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: zoran: move to dma-mapping interface
@ 2018-04-24 20:40 Arnd Bergmann
  2018-04-25  6:15 ` Christoph Hellwig
  2018-04-26 17:49 ` kbuild test robot
  0 siblings, 2 replies; 13+ messages in thread
From: Arnd Bergmann @ 2018-04-24 20:40 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Arnd Bergmann, Hans Verkuil, Arvind Yadav, mjpeg-users,
	linux-media, linux-kernel

No drivers should use virt_to_bus() any more. This converts
one of the few remaining ones to the DMA mapping interface.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/pci/zoran/Kconfig        |  2 +-
 drivers/media/pci/zoran/zoran.h        | 10 +++++--
 drivers/media/pci/zoran/zoran_card.c   | 10 +++++--
 drivers/media/pci/zoran/zoran_device.c | 16 +++++-----
 drivers/media/pci/zoran/zoran_driver.c | 54 +++++++++++++++++++++++++---------
 5 files changed, 63 insertions(+), 29 deletions(-)

diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig
index 39ec35bd21a5..26f40e124a32 100644
--- a/drivers/media/pci/zoran/Kconfig
+++ b/drivers/media/pci/zoran/Kconfig
@@ -1,6 +1,6 @@
 config VIDEO_ZORAN
 	tristate "Zoran ZR36057/36067 Video For Linux"
-	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS
+	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
 	depends on !ALPHA
 	help
 	  Say Y for support for MJPEG capture cards based on the Zoran
diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
index 9bb3c21aa275..9ff3a9acb60a 100644
--- a/drivers/media/pci/zoran/zoran.h
+++ b/drivers/media/pci/zoran/zoran.h
@@ -183,13 +183,14 @@ struct zoran_buffer {
 	struct zoran_sync bs;		/* DONE: info to return to application */
 	union {
 		struct {
-			__le32 *frag_tab;	/* addresses of frag table */
-			u32 frag_tab_bus;	/* same value cached to save time in ISR */
+			__le32 *frag_tab;	/* DMA addresses of frag table */
+			void **frag_virt_tab;	/* virtual addresses of frag table */
+			dma_addr_t frag_tab_dma;/* same value cached to save time in ISR */
 		} jpg;
 		struct {
 			char *fbuffer;		/* virtual address of frame buffer */
 			unsigned long fbuffer_phys;/* physical address of frame buffer */
-			unsigned long fbuffer_bus;/* bus address of frame buffer */
+			dma_addr_t fbuffer_dma;/* bus address of frame buffer */
 		} v4l;
 	};
 };
@@ -221,6 +222,7 @@ struct zoran_fh {
 
 	struct zoran_overlay_settings overlay_settings;
 	u32 *overlay_mask;			/* overlay mask */
+	dma_addr_t overlay_mask_dma;
 	enum zoran_lock_activity overlay_active;/* feature currently in use? */
 
 	struct zoran_buffer_col buffers;	/* buffers' info */
@@ -307,6 +309,7 @@ struct zoran {
 
 	struct zoran_overlay_settings overlay_settings;
 	u32 *overlay_mask;	/* overlay mask */
+	dma_addr_t overlay_mask_dma;
 	enum zoran_lock_activity overlay_active;	/* feature currently in use? */
 
 	wait_queue_head_t v4l_capq;
@@ -346,6 +349,7 @@ struct zoran {
 
 	/* zr36057's code buffer table */
 	__le32 *stat_com;		/* stat_com[i] is indexed by dma_head/tail & BUZ_MASK_STAT_COM */
+	dma_addr_t stat_com_dma;
 
 	/* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
 	int jpg_pend[BUZ_MAX_FRAME];
diff --git a/drivers/media/pci/zoran/zoran_card.c b/drivers/media/pci/zoran/zoran_card.c
index a6b9ebd20263..dabd8bf77472 100644
--- a/drivers/media/pci/zoran/zoran_card.c
+++ b/drivers/media/pci/zoran/zoran_card.c
@@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
 	/* User must explicitly set a window */
 	zr->overlay_settings.is_set = 0;
 	zr->overlay_mask = NULL;
+	zr->overlay_mask_dma = 0;
 	zr->overlay_active = ZORAN_FREE;
 
 	zr->v4l_memgrab_active = 0;
@@ -1028,7 +1029,8 @@ static int zr36057_init (struct zoran *zr)
 
 	/* allocate memory *before* doing anything to the hardware
 	 * in case allocation fails */
-	zr->stat_com = kzalloc(BUZ_NUM_STAT_COM * 4, GFP_KERNEL);
+	zr->stat_com = dma_alloc_coherent(&zr->pci_dev->dev,
+				BUZ_NUM_STAT_COM * 4, &zr->stat_com_dma, GFP_KERNEL);
 	zr->video_dev = video_device_alloc();
 	if (!zr->stat_com || !zr->video_dev) {
 		dprintk(1,
@@ -1072,7 +1074,8 @@ static int zr36057_init (struct zoran *zr)
 	return 0;
 
 exit_free:
-	kfree(zr->stat_com);
+	dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * 4,
+			  zr->stat_com, zr->stat_com_dma);
 	kfree(zr->video_dev);
 	return err;
 }
@@ -1107,7 +1110,8 @@ static void zoran_remove(struct pci_dev *pdev)
 	btwrite(0, ZR36057_SPGPPCR);
 	free_irq(zr->pci_dev->irq, zr);
 	/* unmap and free memory */
-	kfree(zr->stat_com);
+	dma_free_coherent(&zr->pci_dev->dev, BUZ_NUM_STAT_COM * 4,
+			  zr->stat_com, zr->stat_com_dma);
 	zoran_proc_cleanup(zr);
 	iounmap(zr->zr36057_mem);
 	pci_disable_device(zr->pci_dev);
diff --git a/drivers/media/pci/zoran/zoran_device.c b/drivers/media/pci/zoran/zoran_device.c
index 40adceebca7e..1ac7810ddd25 100644
--- a/drivers/media/pci/zoran/zoran_device.c
+++ b/drivers/media/pci/zoran/zoran_device.c
@@ -430,9 +430,9 @@ zr36057_set_vfe (struct zoran              *zr,
 		 * zr->overlay_settings.width instead of video_width */
 
 		mask_line_size = (BUZ_MAX_WIDTH + 31) / 32;
-		reg = virt_to_bus(zr->overlay_mask);
+		reg = zr->overlay_mask_dma;
 		btwrite(reg, ZR36057_MMTR);
-		reg = virt_to_bus(zr->overlay_mask + mask_line_size);
+		reg = zr->overlay_mask_dma + mask_line_size;
 		btwrite(reg, ZR36057_MMBR);
 		reg =
 		    mask_line_size - (zr->overlay_settings.width +
@@ -775,7 +775,7 @@ zr36057_set_jpg (struct zoran          *zr,
 	//btor(ZR36057_VFESPFR_VCLKPol, ZR36057_VFESPFR);
 
 	/* code base address */
-	reg = virt_to_bus(zr->stat_com);
+	reg = zr->stat_com_dma;
 	btwrite(reg, ZR36057_JCBA);
 
 	/* FIFO threshold (FIFO is 160. double words) */
@@ -1097,7 +1097,7 @@ zoran_feed_stat_com (struct zoran *zr)
 			if (!(zr->stat_com[i] & cpu_to_le32(1)))
 				break;
 			zr->stat_com[i] =
-			    cpu_to_le32(zr->jpg_buffers.buffer[frame].jpg.frag_tab_bus);
+			    cpu_to_le32(zr->jpg_buffers.buffer[frame].jpg.frag_tab_dma);
 		} else {
 			/* fill 2 stat_com entries */
 			i = ((zr->jpg_dma_head -
@@ -1105,9 +1105,9 @@ zoran_feed_stat_com (struct zoran *zr)
 			if (!(zr->stat_com[i] & cpu_to_le32(1)))
 				break;
 			zr->stat_com[i] =
-			    cpu_to_le32(zr->jpg_buffers.buffer[frame].jpg.frag_tab_bus);
+			    cpu_to_le32(zr->jpg_buffers.buffer[frame].jpg.frag_tab_dma);
 			zr->stat_com[i + 1] =
-			    cpu_to_le32(zr->jpg_buffers.buffer[frame].jpg.frag_tab_bus);
+			    cpu_to_le32(zr->jpg_buffers.buffer[frame].jpg.frag_tab_dma);
 		}
 		zr->jpg_buffers.buffer[frame].state = BUZ_STATE_DMA;
 		zr->jpg_dma_head++;
@@ -1272,7 +1272,7 @@ error_handler (struct zoran *zr,
 		printk(KERN_INFO "stat_com frames:");
 		for (j = 0; j < BUZ_NUM_STAT_COM; j++) {
 			for (i = 0; i < zr->jpg_buffers.num_buffers; i++) {
-				if (le32_to_cpu(zr->stat_com[j]) == zr->jpg_buffers.buffer[i].jpg.frag_tab_bus)
+				if (le32_to_cpu(zr->stat_com[j]) == zr->jpg_buffers.buffer[i].jpg.frag_tab_dma)
 					printk(KERN_CONT "% d->%d", j, i);
 			}
 		}
@@ -1411,7 +1411,7 @@ zoran_irq (int             irq,
 
 					/* Buffer address */
 
-					reg = zr->v4l_buffers.buffer[frame].v4l.fbuffer_bus;
+					reg = zr->v4l_buffers.buffer[frame].v4l.fbuffer_dma;
 					btwrite(reg, ZR36057_VDTR);
 					if (zr->v4l_settings.height > BUZ_MAX_HEIGHT / 2)
 						reg += zr->v4l_settings.bytesperline;
diff --git a/drivers/media/pci/zoran/zoran_driver.c b/drivers/media/pci/zoran/zoran_driver.c
index 4b6466961b41..dad1fb02ced2 100644
--- a/drivers/media/pci/zoran/zoran_driver.c
+++ b/drivers/media/pci/zoran/zoran_driver.c
@@ -235,15 +235,20 @@ static int v4l_fbuffer_alloc(struct zoran_fh *fh)
 		}
 		fh->buffers.buffer[i].v4l.fbuffer = mem;
 		fh->buffers.buffer[i].v4l.fbuffer_phys = virt_to_phys(mem);
-		fh->buffers.buffer[i].v4l.fbuffer_bus = virt_to_bus(mem);
+		fh->buffers.buffer[i].v4l.fbuffer_dma =
+			dma_map_single(&zr->pci_dev->dev, mem,
+				       fh->buffers.buffer_size,
+				       DMA_FROM_DEVICE);
+		if (!fh->buffers.buffer[i].v4l.fbuffer_dma)
+			return -ENXIO;
 		for (off = 0; off < fh->buffers.buffer_size;
 		     off += PAGE_SIZE)
 			SetPageReserved(virt_to_page(mem + off));
 		dprintk(4,
 			KERN_INFO
-			"%s: %s - V4L frame %d mem %p (bus: 0x%llx)\n",
+			"%s: %s - V4L frame %d mem %p (bus: %pad)\n",
 			ZR_DEVNAME(zr), __func__, i, mem,
-			(unsigned long long)virt_to_bus(mem));
+			&fh->buffers.buffer[i].v4l.fbuffer_dma);
 	}
 
 	fh->buffers.allocated = 1;
@@ -308,6 +313,7 @@ static int jpg_fbuffer_alloc(struct zoran_fh *fh)
 	struct zoran *zr = fh->zr;
 	int i, j, off;
 	u8 *mem;
+	void **virt_tab;
 
 	for (i = 0; i < fh->buffers.num_buffers; i++) {
 		if (fh->buffers.buffer[i].jpg.frag_tab)
@@ -319,16 +325,19 @@ static int jpg_fbuffer_alloc(struct zoran_fh *fh)
 		/* Allocate fragment table for this buffer */
 
 		mem = (void *)get_zeroed_page(GFP_KERNEL);
-		if (!mem) {
+		virt_tab = (void *)get_zeroed_page(GFP_KERNEL);
+		if (!mem || !virt_tab) {
 			dprintk(1,
 				KERN_ERR
 				"%s: %s - get_zeroed_page (frag_tab) failed for buffer %d\n",
 				ZR_DEVNAME(zr), __func__, i);
+			kfree(mem);
+			kfree(virt_tab);
 			jpg_fbuffer_free(fh);
 			return -ENOBUFS;
 		}
 		fh->buffers.buffer[i].jpg.frag_tab = (__le32 *)mem;
-		fh->buffers.buffer[i].jpg.frag_tab_bus = virt_to_bus(mem);
+		fh->buffers.buffer[i].jpg.frag_virt_tab = virt_tab;
 
 		if (fh->buffers.need_contiguous) {
 			mem = kmalloc(fh->buffers.buffer_size, GFP_KERNEL);
@@ -340,8 +349,9 @@ static int jpg_fbuffer_alloc(struct zoran_fh *fh)
 				jpg_fbuffer_free(fh);
 				return -ENOBUFS;
 			}
+			fh->buffers.buffer[i].jpg.frag_virt_tab[0] = mem;
 			fh->buffers.buffer[i].jpg.frag_tab[0] =
-				cpu_to_le32(virt_to_bus(mem));
+				cpu_to_le32(dma_map_single(&zr->pci_dev->dev, mem, fh->buffers.buffer_size, DMA_FROM_DEVICE));
 			fh->buffers.buffer[i].jpg.frag_tab[1] =
 				cpu_to_le32((fh->buffers.buffer_size >> 1) | 1);
 			for (off = 0; off < fh->buffers.buffer_size; off += PAGE_SIZE)
@@ -359,8 +369,9 @@ static int jpg_fbuffer_alloc(struct zoran_fh *fh)
 					return -ENOBUFS;
 				}
 
+				fh->buffers.buffer[i].jpg.frag_virt_tab[j] = mem;
 				fh->buffers.buffer[i].jpg.frag_tab[2 * j] =
-					cpu_to_le32(virt_to_bus(mem));
+					cpu_to_le32(dma_map_single(&zr->pci_dev->dev, mem, fh->buffers.buffer_size, DMA_FROM_DEVICE));
 				fh->buffers.buffer[i].jpg.frag_tab[2 * j + 1] =
 					cpu_to_le32((PAGE_SIZE >> 2) << 1);
 				SetPageReserved(virt_to_page(mem));
@@ -368,6 +379,8 @@ static int jpg_fbuffer_alloc(struct zoran_fh *fh)
 
 			fh->buffers.buffer[i].jpg.frag_tab[2 * j - 1] |= cpu_to_le32(1);
 		}
+
+		fh->buffers.buffer[i].jpg.frag_tab_dma = dma_map_single(&zr->pci_dev->dev, mem, PAGE_SIZE, DMA_TO_DEVICE);
 	}
 
 	dprintk(4,
@@ -400,9 +413,10 @@ static void jpg_fbuffer_free(struct zoran_fh *fh)
 			frag_tab = buffer->jpg.frag_tab[0];
 
 			if (frag_tab) {
-				mem = bus_to_virt(le32_to_cpu(frag_tab));
+				mem = buffer->jpg.frag_virt_tab[0];
 				for (off = 0; off < fh->buffers.buffer_size; off += PAGE_SIZE)
 					ClearPageReserved(virt_to_page(mem + off));
+				dma_unmap_single(&zr->pci_dev->dev, frag_tab, PAGE_SIZE, DMA_FROM_DEVICE);
 				kfree(mem);
 				buffer->jpg.frag_tab[0] = 0;
 				buffer->jpg.frag_tab[1] = 0;
@@ -413,14 +427,19 @@ static void jpg_fbuffer_free(struct zoran_fh *fh)
 
 				if (!frag_tab)
 					break;
-				ClearPageReserved(virt_to_page(bus_to_virt(le32_to_cpu(frag_tab))));
-				free_page((unsigned long)bus_to_virt(le32_to_cpu(frag_tab)));
+				ClearPageReserved(virt_to_page(buffer->jpg.frag_virt_tab[j]));
+				dma_unmap_single(&zr->pci_dev->dev, le32_to_cpu(frag_tab), PAGE_SIZE, DMA_FROM_DEVICE);
+				free_page((unsigned long)buffer->jpg.frag_virt_tab[j]);
+				buffer->jpg.frag_virt_tab[j] = NULL;
 				buffer->jpg.frag_tab[2 * j] = 0;
 				buffer->jpg.frag_tab[2 * j + 1] = 0;
 			}
 		}
 
+		dma_unmap_single(&zr->pci_dev->dev, buffer->jpg.frag_tab_dma, PAGE_SIZE, DMA_TO_DEVICE);
 		free_page((unsigned long)buffer->jpg.frag_tab);
+		free_page((unsigned long)buffer->jpg.frag_virt_tab);
+		buffer->jpg.frag_virt_tab = NULL;
 		buffer->jpg.frag_tab = NULL;
 	}
 
@@ -873,6 +892,7 @@ static void zoran_close_end_session(struct zoran_fh *fh)
 		if (!zr->v4l_memgrab_active)
 			zr36057_overlay(zr, 0);
 		zr->overlay_mask = NULL;
+		zr->overlay_mask_dma = 0;
 	}
 
 	if (fh->map_mode == ZORAN_MAP_MODE_RAW) {
@@ -940,8 +960,10 @@ static int zoran_open(struct file *file)
 
 	/* used to be BUZ_MAX_WIDTH/HEIGHT, but that gives overflows
 	 * on norm-change! */
-	fh->overlay_mask =
-	    kmalloc(((768 + 31) / 32) * 576 * 4, GFP_KERNEL);
+	fh->overlay_mask = dma_alloc_wc(&zr->pci_dev->dev,
+					((768 + 31) / 32) * 576 * 4,
+					&fh->overlay_mask_dma,
+					GFP_KERNEL);
 	if (!fh->overlay_mask) {
 		dprintk(1,
 			KERN_ERR
@@ -1016,6 +1038,7 @@ zoran_close(struct file  *file)
 		zr->v4l_overlay_active = 0;
 		zr36057_overlay(zr, 0);
 		zr->overlay_mask = NULL;
+		zr->overlay_mask_dma = 0;
 
 		/* capture off */
 		wake_up_interruptible(&zr->v4l_capq);
@@ -1033,7 +1056,8 @@ zoran_close(struct file  *file)
 
 	v4l2_fh_del(&fh->fh);
 	v4l2_fh_exit(&fh->fh);
-	kfree(fh->overlay_mask);
+	dma_free_wc(&zr->pci_dev->dev, ((768 + 31) / 32) * 576 * 4,
+		    fh->overlay_mask, fh->overlay_mask_dma);
 	kfree(fh);
 
 	dprintk(4, KERN_INFO "%s: %s done\n", ZR_DEVNAME(zr), __func__);
@@ -1284,6 +1308,7 @@ static int setup_overlay(struct zoran_fh *fh, int on)
 		if (!zr->v4l_memgrab_active)
 			zr36057_overlay(zr, 0);
 		zr->overlay_mask = NULL;
+		zr->overlay_mask_dma = 0;
 	} else {
 		if (!zr->vbuf_base || !fh->overlay_settings.is_set) {
 			dprintk(1,
@@ -1302,6 +1327,7 @@ static int setup_overlay(struct zoran_fh *fh, int on)
 		zr->overlay_active = fh->overlay_active = ZORAN_LOCKED;
 		zr->v4l_overlay_active = 1;
 		zr->overlay_mask = fh->overlay_mask;
+		zr->overlay_mask_dma = fh->overlay_mask_dma;
 		zr->overlay_settings = fh->overlay_settings;
 		if (!zr->v4l_memgrab_active)
 			zr36057_overlay(zr, 1);
@@ -2763,7 +2789,7 @@ zoran_mmap (struct file           *file,
 				    le32_to_cpu(fh->buffers.
 				    buffer[i].jpg.frag_tab[2 * j]);
 				/* should just be pos on i386 */
-				page = virt_to_phys(bus_to_virt(pos))
+				page = virt_to_phys(fh->buffers.buffer[i].jpg.frag_virt_tab[j])
 								>> PAGE_SHIFT;
 				if (remap_pfn_range(vma, start, page,
 							todo, PAGE_SHARED)) {
-- 
2.9.0

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-24 20:40 [PATCH] media: zoran: move to dma-mapping interface Arnd Bergmann
@ 2018-04-25  6:15 ` Christoph Hellwig
  2018-04-25  7:08   ` Arnd Bergmann
  2018-04-26 17:49 ` kbuild test robot
  1 sibling, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2018-04-25  6:15 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Arvind Yadav, mjpeg-users,
	linux-media, linux-kernel

On Tue, Apr 24, 2018 at 10:40:45PM +0200, Arnd Bergmann wrote:
> No drivers should use virt_to_bus() any more. This converts
> one of the few remaining ones to the DMA mapping interface.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/pci/zoran/Kconfig        |  2 +-
>  drivers/media/pci/zoran/zoran.h        | 10 +++++--
>  drivers/media/pci/zoran/zoran_card.c   | 10 +++++--
>  drivers/media/pci/zoran/zoran_device.c | 16 +++++-----
>  drivers/media/pci/zoran/zoran_driver.c | 54 +++++++++++++++++++++++++---------
>  5 files changed, 63 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/media/pci/zoran/Kconfig b/drivers/media/pci/zoran/Kconfig
> index 39ec35bd21a5..26f40e124a32 100644
> --- a/drivers/media/pci/zoran/Kconfig
> +++ b/drivers/media/pci/zoran/Kconfig
> @@ -1,6 +1,6 @@
>  config VIDEO_ZORAN
>  	tristate "Zoran ZR36057/36067 Video For Linux"
> -	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2 && VIRT_TO_BUS
> +	depends on PCI && I2C_ALGOBIT && VIDEO_V4L2
>  	depends on !ALPHA
>  	help
>  	  Say Y for support for MJPEG capture cards based on the Zoran
> diff --git a/drivers/media/pci/zoran/zoran.h b/drivers/media/pci/zoran/zoran.h
> index 9bb3c21aa275..9ff3a9acb60a 100644
> --- a/drivers/media/pci/zoran/zoran.h
> +++ b/drivers/media/pci/zoran/zoran.h
> @@ -183,13 +183,14 @@ struct zoran_buffer {
>  	struct zoran_sync bs;		/* DONE: info to return to application */
>  	union {
>  		struct {
> -			__le32 *frag_tab;	/* addresses of frag table */
> -			u32 frag_tab_bus;	/* same value cached to save time in ISR */
> +			__le32 *frag_tab;	/* DMA addresses of frag table */
> +			void **frag_virt_tab;	/* virtual addresses of frag table */
> +			dma_addr_t frag_tab_dma;/* same value cached to save time in ISR */
>  		} jpg;
>  		struct {
>  			char *fbuffer;		/* virtual address of frame buffer */
>  			unsigned long fbuffer_phys;/* physical address of frame buffer */
> -			unsigned long fbuffer_bus;/* bus address of frame buffer */
> +			dma_addr_t fbuffer_dma;/* bus address of frame buffer */
>  		} v4l;
>  	};
>  };
> @@ -221,6 +222,7 @@ struct zoran_fh {
>  
>  	struct zoran_overlay_settings overlay_settings;
>  	u32 *overlay_mask;			/* overlay mask */
> +	dma_addr_t overlay_mask_dma;
>  	enum zoran_lock_activity overlay_active;/* feature currently in use? */
>  
>  	struct zoran_buffer_col buffers;	/* buffers' info */
> @@ -307,6 +309,7 @@ struct zoran {
>  
>  	struct zoran_overlay_settings overlay_settings;
>  	u32 *overlay_mask;	/* overlay mask */
> +	dma_addr_t overlay_mask_dma;
>  	enum zoran_lock_activity overlay_active;	/* feature currently in use? */
>  
>  	wait_queue_head_t v4l_capq;
> @@ -346,6 +349,7 @@ struct zoran {
>  
>  	/* zr36057's code buffer table */
>  	__le32 *stat_com;		/* stat_com[i] is indexed by dma_head/tail & BUZ_MASK_STAT_COM */
> +	dma_addr_t stat_com_dma;
>  
>  	/* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
>  	int jpg_pend[BUZ_MAX_FRAME];
> diff --git a/drivers/media/pci/zoran/zoran_card.c b/drivers/media/pci/zoran/zoran_card.c
> index a6b9ebd20263..dabd8bf77472 100644
> --- a/drivers/media/pci/zoran/zoran_card.c
> +++ b/drivers/media/pci/zoran/zoran_card.c
> @@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
>  	/* User must explicitly set a window */
>  	zr->overlay_settings.is_set = 0;
>  	zr->overlay_mask = NULL;
> +	zr->overlay_mask_dma = 0;

I don't think this zeroing is required, and given that 0 is a valid
dma address on some platforms is also is rather confusing.:w

>  
>  		mask_line_size = (BUZ_MAX_WIDTH + 31) / 32;
> -		reg = virt_to_bus(zr->overlay_mask);
> +		reg = zr->overlay_mask_dma;
>  		btwrite(reg, ZR36057_MMTR);
> -		reg = virt_to_bus(zr->overlay_mask + mask_line_size);
> +		reg = zr->overlay_mask_dma + mask_line_size;

I think this would be easier to read if the reg assignments got
removed, e.g.

	btwrite(zr->overlay_mask_dma, ZR36057_MMTR);
	btwrite(zr->overlay_mask_dma + mask_line_size, ZR36057_MMBR);

Same in a few other places.

> +		virt_tab = (void *)get_zeroed_page(GFP_KERNEL);
> +		if (!mem || !virt_tab) {
>  			dprintk(1,
>  				KERN_ERR
>  				"%s: %s - get_zeroed_page (frag_tab) failed for buffer %d\n",
>  				ZR_DEVNAME(zr), __func__, i);
> +			kfree(mem);
> +			kfree(virt_tab);
>  			jpg_fbuffer_free(fh);
>  			return -ENOBUFS;
>  		}
>  		fh->buffers.buffer[i].jpg.frag_tab = (__le32 *)mem;
> -		fh->buffers.buffer[i].jpg.frag_tab_bus = virt_to_bus(mem);
> +		fh->buffers.buffer[i].jpg.frag_virt_tab = virt_tab;

>From a quick look it seems like frag_tab should be a dma_alloc_coherent
allocation, or you would need a lot of cache sync operations.

That probably also means it can use dma_mmap_coherent instead of the
handcrafted remap_pfn_range loop and the PageReserved abuse.

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25  6:15 ` Christoph Hellwig
@ 2018-04-25  7:08   ` Arnd Bergmann
  2018-04-25  7:21     ` Christoph Hellwig
  0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2018-04-25  7:08 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Arvind Yadav, mjpeg-users,
	Linux Media Mailing List, Linux Kernel Mailing List

On Wed, Apr 25, 2018 at 8:15 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Apr 24, 2018 at 10:40:45PM +0200, Arnd Bergmann wrote:
>> @@ -221,6 +222,7 @@ struct zoran_fh {
>>
>>       struct zoran_overlay_settings overlay_settings;
>>       u32 *overlay_mask;                      /* overlay mask */
>> +     dma_addr_t overlay_mask_dma;
>>       enum zoran_lock_activity overlay_active;/* feature currently in use? */
>>
>>       struct zoran_buffer_col buffers;        /* buffers' info */
>> @@ -307,6 +309,7 @@ struct zoran {
>>
>>       struct zoran_overlay_settings overlay_settings;
>>       u32 *overlay_mask;      /* overlay mask */
>> +     dma_addr_t overlay_mask_dma;
>>       enum zoran_lock_activity overlay_active;        /* feature currently in use? */
>>
>>       wait_queue_head_t v4l_capq;
>> @@ -346,6 +349,7 @@ struct zoran {
>>
>>       /* zr36057's code buffer table */
>>       __le32 *stat_com;               /* stat_com[i] is indexed by dma_head/tail & BUZ_MASK_STAT_COM */
>> +     dma_addr_t stat_com_dma;
>>
>>       /* (value & BUZ_MASK_FRAME) corresponds to index in pend[] queue */
>>       int jpg_pend[BUZ_MAX_FRAME];
>> diff --git a/drivers/media/pci/zoran/zoran_card.c b/drivers/media/pci/zoran/zoran_card.c
>> index a6b9ebd20263..dabd8bf77472 100644
>> --- a/drivers/media/pci/zoran/zoran_card.c
>> +++ b/drivers/media/pci/zoran/zoran_card.c
>> @@ -890,6 +890,7 @@ zoran_open_init_params (struct zoran *zr)
>>       /* User must explicitly set a window */
>>       zr->overlay_settings.is_set = 0;
>>       zr->overlay_mask = NULL;
>> +     zr->overlay_mask_dma = 0;
>
> I don't think this zeroing is required, and given that 0 is a valid
> dma address on some platforms is also is rather confusing.:w

The driver does this everywhere, which I also noticed as unnecessary,
but it felt better to be consistent with the rest of the driver here than
to follow common practice.

There are some explicit 'if (zr->overlay_mask)' checks in the driver
that require overlay_mask to be initialized. I could set it to
DMA_ERROR_CODE, but you removed that ;-)

It's easy enough to remove the re-zeroing of the number though
if you still think that's better.

>>               mask_line_size = (BUZ_MAX_WIDTH + 31) / 32;
>> -             reg = virt_to_bus(zr->overlay_mask);
>> +             reg = zr->overlay_mask_dma;
>>               btwrite(reg, ZR36057_MMTR);
>> -             reg = virt_to_bus(zr->overlay_mask + mask_line_size);
>> +             reg = zr->overlay_mask_dma + mask_line_size;
>
> I think this would be easier to read if the reg assignments got
> removed, e.g.
>
>         btwrite(zr->overlay_mask_dma, ZR36057_MMTR);
>         btwrite(zr->overlay_mask_dma + mask_line_size, ZR36057_MMBR);
>
> Same in a few other places.

Right, good idea.

>> +             virt_tab = (void *)get_zeroed_page(GFP_KERNEL);
>> +             if (!mem || !virt_tab) {
>>                       dprintk(1,
>>                               KERN_ERR
>>                               "%s: %s - get_zeroed_page (frag_tab) failed for buffer %d\n",
>>                               ZR_DEVNAME(zr), __func__, i);
>> +                     kfree(mem);
>> +                     kfree(virt_tab);
>>                       jpg_fbuffer_free(fh);
>>                       return -ENOBUFS;
>>               }
>>               fh->buffers.buffer[i].jpg.frag_tab = (__le32 *)mem;
>> -             fh->buffers.buffer[i].jpg.frag_tab_bus = virt_to_bus(mem);
>> +             fh->buffers.buffer[i].jpg.frag_virt_tab = virt_tab;
>
> From a quick look it seems like frag_tab should be a dma_alloc_coherent
> allocation, or you would need a lot of cache sync operations.

Do you mean the table or the buffers it points to? In the code you
quote, we initialize the table with static data before mapping it,
so I would expect either interface to work fine here.

For the actual buffers, I tried to retain the current behavior and
regular kernel memory, in particular because I wasn't too sure
about changing the mmap() function without understanding what
it really does. ;-)

It looks like the buffers are never accessed from the kernel but
only from hardware and user space, so we don't care about whether
we get a mapping that is coherent between the kernel and hardware,
but we probably should ensure that the page table attributes are the
same in kernel and user space (at least powerpc gets a checkstop
for unmatched cacheability flags).

> That probably also means it can use dma_mmap_coherent instead of the
> handcrafted remap_pfn_range loop and the PageReserved abuse.

I'd rather not touch that code. How about adding a comment about
the fact that it should use dma_mmap_coherent()?

     Arnd

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25  7:08   ` Arnd Bergmann
@ 2018-04-25  7:21     ` Christoph Hellwig
  2018-04-25 11:15       ` Arnd Bergmann
  2018-04-25 17:27       ` [Mjpeg-users] " Bernhard Praschinger
  0 siblings, 2 replies; 13+ messages in thread
From: Christoph Hellwig @ 2018-04-25  7:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Mauro Carvalho Chehab, Hans Verkuil,
	Arvind Yadav, mjpeg-users, Linux Media Mailing List,
	Linux Kernel Mailing List

On Wed, Apr 25, 2018 at 09:08:13AM +0200, Arnd Bergmann wrote:
> > That probably also means it can use dma_mmap_coherent instead of the
> > handcrafted remap_pfn_range loop and the PageReserved abuse.
> 
> I'd rather not touch that code. How about adding a comment about
> the fact that it should use dma_mmap_coherent()?

Maybe the real question is if there is anyone that actually cares
for this driver, or if we are better off just removing it?

Same is true for various other virt_to_bus using drivers, e.g. the
grotty atm drivers.

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25  7:21     ` Christoph Hellwig
@ 2018-04-25 11:15       ` Arnd Bergmann
  2018-04-25 15:26         ` Christoph Hellwig
  2018-04-25 17:27       ` [Mjpeg-users] " Bernhard Praschinger
  1 sibling, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2018-04-25 11:15 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Arvind Yadav, mjpeg-users,
	Linux Media Mailing List, Linux Kernel Mailing List

On Wed, Apr 25, 2018 at 9:21 AM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Apr 25, 2018 at 09:08:13AM +0200, Arnd Bergmann wrote:
>> > That probably also means it can use dma_mmap_coherent instead of the
>> > handcrafted remap_pfn_range loop and the PageReserved abuse.
>>
>> I'd rather not touch that code. How about adding a comment about
>> the fact that it should use dma_mmap_coherent()?
>
> Maybe the real question is if there is anyone that actually cares
> for this driver, or if we are better off just removing it?
>
> Same is true for various other virt_to_bus using drivers, e.g. the
> grotty atm drivers.

That thought had occurred to me as well. I removed the oldest ISDN
drivers already some years ago, and the OSS sound drivers
got removed as well, and comedi got converted to the dma-mapping
interfaces, so there isn't much left at all now. This is what we
have as of v4.17-rc1:

$ git grep -wl '\<bus_to_virt\|virt_to_bus\>' drivers/
drivers/atm/ambassador.c
drivers/atm/firestream.c
drivers/atm/horizon.c
drivers/atm/zatm.c
drivers/block/swim3.c # power mac specific
drivers/gpu/drm/mga/mga_dma.c # commented out
drivers/infiniband/hw/nes/nes_verbs.c # commented out
drivers/isdn/hisax/netjet.c
drivers/net/appletalk/ltpc.c
drivers/net/ethernet/amd/au1000_eth.c # mips specific
drivers/net/ethernet/amd/ni65.c # only in comment
drivers/net/ethernet/apple/bmac.c # power mac specific
drivers/net/ethernet/apple/mace.c # power mac specific
drivers/net/ethernet/dec/tulip/de4x5.c  # trivially fixable
drivers/net/ethernet/i825xx/82596.c # m68k specific
drivers/net/ethernet/i825xx/lasi_82596.c # parisc specific
drivers/net/ethernet/i825xx/lib82596.c # only in comment
drivers/net/ethernet/sgi/ioc3-eth.c # mips specific
drivers/net/wan/cosa.c
drivers/net/wan/lmc/lmc_main.c
drivers/net/wan/z85230.c
drivers/scsi/3w-xxxx.c # only in comment
drivers/scsi/BusLogic.c
drivers/scsi/a2091.c # m68k specific
drivers/scsi/a3000.c # m68k specific
drivers/scsi/dc395x.c # only in comment
drivers/scsi/dpt_i2o.c
drivers/scsi/gvp11.c # m68k specific
drivers/scsi/mvme147.c # m68k specific
drivers/scsi/qla1280.c # comment only
drivers/staging/netlogic/xlr_net.c # mips specific
drivers/tty/serial/cpm_uart/cpm_uart_cpm2.c # ppc32 specific
drivers/vme/bridges/vme_ca91cx42.c

My feeling is that we want to keep most of the arch specific
ones, in particular removing the m68k drivers would break
a whole class of machines.

For the ones that don't have a comment on them, they
probably won't be missed much, but it's hard to know for
sure. What we do know is that they never worked on
x86_64, and most of them are for ISA cards.

        Arnd

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25 11:15       ` Arnd Bergmann
@ 2018-04-25 15:26         ` Christoph Hellwig
  2018-04-25 15:58           ` Arnd Bergmann
  0 siblings, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2018-04-25 15:26 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Mauro Carvalho Chehab, Hans Verkuil,
	Arvind Yadav, mjpeg-users, Linux Media Mailing List,
	Linux Kernel Mailing List

On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:
> That thought had occurred to me as well. I removed the oldest ISDN
> drivers already some years ago, and the OSS sound drivers
> got removed as well, and comedi got converted to the dma-mapping
> interfaces, so there isn't much left at all now. This is what we
> have as of v4.17-rc1:

Yes, I've been looking at various grotty old bits to purge.  Usually
I've been looking for some non-tree wide patches and CCed the last
active people to see if they care.  In a few cases people do, but
most often no one does.

> My feeling is that we want to keep most of the arch specific
> ones, in particular removing the m68k drivers would break
> a whole class of machines.

For the arch specific ones it would good to just ping the relevant
maintainers.  Especially m68k and parisc folks seems to be very
responsive.

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25 15:26         ` Christoph Hellwig
@ 2018-04-25 15:58           ` Arnd Bergmann
  2018-04-25 17:22             ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 13+ messages in thread
From: Arnd Bergmann @ 2018-04-25 15:58 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Mauro Carvalho Chehab, Hans Verkuil, Arvind Yadav, mjpeg-users,
	Linux Media Mailing List, Linux Kernel Mailing List,
	Trent Piepho

On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:
>> That thought had occurred to me as well. I removed the oldest ISDN
>> drivers already some years ago, and the OSS sound drivers
>> got removed as well, and comedi got converted to the dma-mapping
>> interfaces, so there isn't much left at all now. This is what we
>> have as of v4.17-rc1:
>
> Yes, I've been looking at various grotty old bits to purge.  Usually
> I've been looking for some non-tree wide patches and CCed the last
> active people to see if they care.  In a few cases people do, but
> most often no one does.

Let's start with this one (zoran) then, as Mauro is keen on having
all media drivers compile-testable on x86-64 and arm.

Trent Piepho and Hans Verkuil both worked on this driver in the
2008/2009 timeframe and those were the last commits from anyone
who appears to have tested their patches on actual hardware.

Trent, Hans: do you have reason to believe that there might still
be users out there?

       Arnd

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25 15:58           ` Arnd Bergmann
@ 2018-04-25 17:22             ` Mauro Carvalho Chehab
  2018-04-25 17:43               ` Trent Piepho
  2018-05-07  9:05               ` Hans Verkuil
  0 siblings, 2 replies; 13+ messages in thread
From: Mauro Carvalho Chehab @ 2018-04-25 17:22 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Christoph Hellwig, Mauro Carvalho Chehab, Hans Verkuil,
	Arvind Yadav, mjpeg-users, Linux Media Mailing List,
	Linux Kernel Mailing List, Trent Piepho

Em Wed, 25 Apr 2018 17:58:25 +0200
Arnd Bergmann <arnd@arndb.de> escreveu:

> On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <hch@infradead.org> wrote:
> > On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:  
> >> That thought had occurred to me as well. I removed the oldest ISDN
> >> drivers already some years ago, and the OSS sound drivers
> >> got removed as well, and comedi got converted to the dma-mapping
> >> interfaces, so there isn't much left at all now. This is what we
> >> have as of v4.17-rc1:  
> >
> > Yes, I've been looking at various grotty old bits to purge.  Usually
> > I've been looking for some non-tree wide patches and CCed the last
> > active people to see if they care.  In a few cases people do, but
> > most often no one does.  
> 
> Let's start with this one (zoran) then, as Mauro is keen on having
> all media drivers compile-testable on x86-64 and arm.
> 
> Trent Piepho and Hans Verkuil both worked on this driver in the
> 2008/2009 timeframe and those were the last commits from anyone
> who appears to have tested their patches on actual hardware.

Zoran is a driver for old hardware. I don't doubt that are people
out there still using it, but who knows?

I have a few those boards packed somewhere. I haven't work with PCI
hardware for a while. If needed, I can try to seek for them and
do some tests. I need first to unpack a machine with PCI slots...
the NUCs I generally use for development don't have any :-)

Anyway, except for virt_to_bus() and related stuff, I think that this
driver is in good shape, as Hans did a lot of work in the past to
make it to use the current media framework.

> 
> Trent, Hans: do you have reason to believe that there might still
> be users out there?
> 
>        Arnd



Thanks,
Mauro

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

* Re: [Mjpeg-users] [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25  7:21     ` Christoph Hellwig
  2018-04-25 11:15       ` Arnd Bergmann
@ 2018-04-25 17:27       ` Bernhard Praschinger
  1 sibling, 0 replies; 13+ messages in thread
From: Bernhard Praschinger @ 2018-04-25 17:27 UTC (permalink / raw)
  To: MJPEG-tools user list, Arnd Bergmann
  Cc: Linux Kernel Mailing List, Christoph Hellwig, Hans Verkuil,
	Arvind Yadav, Mauro Carvalho Chehab, Linux Media Mailing List

Hallo

Christoph Hellwig wrote:
> On Wed, Apr 25, 2018 at 09:08:13AM +0200, Arnd Bergmann wrote:
>>> That probably also means it can use dma_mmap_coherent instead of the
>>> handcrafted remap_pfn_range loop and the PageReserved abuse.
>>
>> I'd rather not touch that code. How about adding a comment about
>> the fact that it should use dma_mmap_coherent()?
>
> Maybe the real question is if there is anyone that actually cares
> for this driver, or if we are better off just removing it?
>
> Same is true for various other virt_to_bus using drivers, e.g. the
> grotty atm drivers.

I would appreciate if somebody would removes the driver from the linux 
kernel. I suggested that some years ago 2014-06-23 (just scroll down):
https://sourceforge.net/p/mjpeg/mailman/mjpeg-users/?viewmonth=201406

You should also find that thread in the kernel-janitors@vger.kernel.org 
and linux-media@vger.kernel.org Mailinglist archive.

I know of one person that is still using this old type of card's but he 
uses a 2.6.x Kernel on a old machine, using old software releases.
https://sourceforge.net/p/mjpeg/mailman/mjpeg-users/?viewmonth=201709

That type of card were introduced about ~20 years ago. I think it is a 
good moment to say goodbye to that type of cards.

auf hoffentlich bald,

Berni the Chaos of Woodquarter

Email: shadowlord@utanet.at
www: http://www.lysator.liu.se/~gz/bernhard

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25 17:22             ` Mauro Carvalho Chehab
@ 2018-04-25 17:43               ` Trent Piepho
  2018-05-07  9:05               ` Hans Verkuil
  1 sibling, 0 replies; 13+ messages in thread
From: Trent Piepho @ 2018-04-25 17:43 UTC (permalink / raw)
  To: mchehab+samsung, arnd
  Cc: hch, mchehab, arvind.yadav.cs, mjpeg-users, linux-media,
	hans.verkuil, linux-kernel

On Wed, 2018-04-25 at 14:22 -0300, Mauro Carvalho Chehab wrote:
> Em Wed, 25 Apr 2018 17:58:25 +0200
> Arnd Bergmann <arnd@arndb.de> escreveu:
> 
> > On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <hch@infradead.o
> > rg> wrote:
> > > On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:  
> > > > That thought had occurred to me as well. I removed the oldest ISDN
> > > > drivers already some years ago, and the OSS sound drivers
> > > > got removed as well, and comedi got converted to the dma-mapping
> > > > interfaces, so there isn't much left at all now. This is what we
> > > > have as of v4.17-rc1:  
> > > 
> > > Yes, I've been looking at various grotty old bits to purge.  Usually
> > > I've been looking for some non-tree wide patches and CCed the last
> > > active people to see if they care.  In a few cases people do, but
> > > most often no one does.  
> > 
> > Let's start with this one (zoran) then, as Mauro is keen on having
> > all media drivers compile-testable on x86-64 and arm.
> > 
> > Trent Piepho and Hans Verkuil both worked on this driver in the
> > 2008/2009 timeframe and those were the last commits from anyone
> > who appears to have tested their patches on actual hardware.
> 
> Zoran is a driver for old hardware. I don't doubt that are people
> out there still using it, but who knows?
> 
> I have a few those boards packed somewhere. I haven't work with PCI
> hardware for a while. If needed, I can try to seek for them and
> do some tests. I need first to unpack a machine with PCI slots...
> the NUCs I generally use for development don't have any :-)
> 
> Anyway, except for virt_to_bus() and related stuff, I think that this
> driver is in good shape, as Hans did a lot of work in the past to
> make it to use the current media framework.

I still have a zoran board.  And my recently purchased ryzen system has
PCI slots.  To my surprise they are not uncommon on new socket AM4
boards.  However, I think the zoran board I have is 5V PCI and that is
rather uncommon.  Also becoming uncommon is analog NTSC/PAL video that
this chip is designed for!

If anyone is using these still, they would be in legacy systems for
these legacy video formats.

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-24 20:40 [PATCH] media: zoran: move to dma-mapping interface Arnd Bergmann
  2018-04-25  6:15 ` Christoph Hellwig
@ 2018-04-26 17:49 ` kbuild test robot
  1 sibling, 0 replies; 13+ messages in thread
From: kbuild test robot @ 2018-04-26 17:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kbuild-all, Mauro Carvalho Chehab, Arnd Bergmann, Hans Verkuil,
	Arvind Yadav, mjpeg-users, linux-media, linux-kernel

Hi Arnd,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/media-zoran-move-to-dma-mapping-interface/20180426-032120
base:   git://linuxtv.org/media_tree.git master
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/media/pci/zoran/zoran_driver.c:419:33: sparse: incorrect type in argument 2 (different base types) @@    expected unsigned long long [unsigned] [usertype] addr @@    got nsigned long long [unsigned] [usertype] addr @@
   drivers/media/pci/zoran/zoran_driver.c:419:33:    expected unsigned long long [unsigned] [usertype] addr
   drivers/media/pci/zoran/zoran_driver.c:419:33:    got restricted __le32 [assigned] [usertype] frag_tab

vim +419 drivers/media/pci/zoran/zoran_driver.c

   395	
   396	/* free the MJPEG grab buffers */
   397	static void jpg_fbuffer_free(struct zoran_fh *fh)
   398	{
   399		struct zoran *zr = fh->zr;
   400		int i, j, off;
   401		unsigned char *mem;
   402		__le32 frag_tab;
   403		struct zoran_buffer *buffer;
   404	
   405		dprintk(4, KERN_DEBUG "%s: %s\n", ZR_DEVNAME(zr), __func__);
   406	
   407		for (i = 0, buffer = &fh->buffers.buffer[0];
   408		     i < fh->buffers.num_buffers; i++, buffer++) {
   409			if (!buffer->jpg.frag_tab)
   410				continue;
   411	
   412			if (fh->buffers.need_contiguous) {
   413				frag_tab = buffer->jpg.frag_tab[0];
   414	
   415				if (frag_tab) {
   416					mem = buffer->jpg.frag_virt_tab[0];
   417					for (off = 0; off < fh->buffers.buffer_size; off += PAGE_SIZE)
   418						ClearPageReserved(virt_to_page(mem + off));
 > 419					dma_unmap_single(&zr->pci_dev->dev, frag_tab, PAGE_SIZE, DMA_FROM_DEVICE);
   420					kfree(mem);
   421					buffer->jpg.frag_tab[0] = 0;
   422					buffer->jpg.frag_tab[1] = 0;
   423				}
   424			} else {
   425				for (j = 0; j < fh->buffers.buffer_size / PAGE_SIZE; j++) {
   426					frag_tab = buffer->jpg.frag_tab[2 * j];
   427	
   428					if (!frag_tab)
   429						break;
   430					ClearPageReserved(virt_to_page(buffer->jpg.frag_virt_tab[j]));
   431					dma_unmap_single(&zr->pci_dev->dev, le32_to_cpu(frag_tab), PAGE_SIZE, DMA_FROM_DEVICE);
   432					free_page((unsigned long)buffer->jpg.frag_virt_tab[j]);
   433					buffer->jpg.frag_virt_tab[j] = NULL;
   434					buffer->jpg.frag_tab[2 * j] = 0;
   435					buffer->jpg.frag_tab[2 * j + 1] = 0;
   436				}
   437			}
   438	
   439			dma_unmap_single(&zr->pci_dev->dev, buffer->jpg.frag_tab_dma, PAGE_SIZE, DMA_TO_DEVICE);
   440			free_page((unsigned long)buffer->jpg.frag_tab);
   441			free_page((unsigned long)buffer->jpg.frag_virt_tab);
   442			buffer->jpg.frag_virt_tab = NULL;
   443			buffer->jpg.frag_tab = NULL;
   444		}
   445	
   446		fh->buffers.allocated = 0;
   447	}
   448	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-04-25 17:22             ` Mauro Carvalho Chehab
  2018-04-25 17:43               ` Trent Piepho
@ 2018-05-07  9:05               ` Hans Verkuil
  2018-05-07 21:17                 ` Arnd Bergmann
  1 sibling, 1 reply; 13+ messages in thread
From: Hans Verkuil @ 2018-05-07  9:05 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Arnd Bergmann
  Cc: Christoph Hellwig, Mauro Carvalho Chehab, Hans Verkuil,
	Arvind Yadav, mjpeg-users, Linux Media Mailing List,
	Linux Kernel Mailing List, Trent Piepho

On 25/04/18 19:22, Mauro Carvalho Chehab wrote:
> Em Wed, 25 Apr 2018 17:58:25 +0200
> Arnd Bergmann <arnd@arndb.de> escreveu:
> 
>> On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <hch@infradead.org> wrote:
>>> On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:  
>>>> That thought had occurred to me as well. I removed the oldest ISDN
>>>> drivers already some years ago, and the OSS sound drivers
>>>> got removed as well, and comedi got converted to the dma-mapping
>>>> interfaces, so there isn't much left at all now. This is what we
>>>> have as of v4.17-rc1:  
>>>
>>> Yes, I've been looking at various grotty old bits to purge.  Usually
>>> I've been looking for some non-tree wide patches and CCed the last
>>> active people to see if they care.  In a few cases people do, but
>>> most often no one does.  
>>
>> Let's start with this one (zoran) then, as Mauro is keen on having
>> all media drivers compile-testable on x86-64 and arm.
>>
>> Trent Piepho and Hans Verkuil both worked on this driver in the
>> 2008/2009 timeframe and those were the last commits from anyone
>> who appears to have tested their patches on actual hardware.
> 
> Zoran is a driver for old hardware. I don't doubt that are people
> out there still using it, but who knows?
> 
> I have a few those boards packed somewhere. I haven't work with PCI
> hardware for a while. If needed, I can try to seek for them and
> do some tests. I need first to unpack a machine with PCI slots...
> the NUCs I generally use for development don't have any :-)
> 
> Anyway, except for virt_to_bus() and related stuff, I think that this
> driver is in good shape, as Hans did a lot of work in the past to
> make it to use the current media framework.
> 
>>
>> Trent, Hans: do you have reason to believe that there might still
>> be users out there?

I have no way of knowing this. However, I think they are easily replaced
by much cheaper USB alternatives today.

I did some work on the zoran driver several years ago, but it doesn't use
the vb2 framework (and not even the older vb1 framework!) so I'm sure there
are all sorts of bugs in that driver.

Personally I would be fine with moving this driver to staging and removing
it by, say, the end of the year.

Nobody is going to work on it and I think it is time to retire it.

Regards,

	Hans

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

* Re: [PATCH] media: zoran: move to dma-mapping interface
  2018-05-07  9:05               ` Hans Verkuil
@ 2018-05-07 21:17                 ` Arnd Bergmann
  0 siblings, 0 replies; 13+ messages in thread
From: Arnd Bergmann @ 2018-05-07 21:17 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Mauro Carvalho Chehab, Christoph Hellwig, Mauro Carvalho Chehab,
	Hans Verkuil, Arvind Yadav, mjpeg-users,
	Linux Media Mailing List, Linux Kernel Mailing List,
	Trent Piepho

On Mon, May 7, 2018 at 5:05 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 25/04/18 19:22, Mauro Carvalho Chehab wrote:
>> Em Wed, 25 Apr 2018 17:58:25 +0200
>> Arnd Bergmann <arnd@arndb.de> escreveu:
>>
>>> On Wed, Apr 25, 2018 at 5:26 PM, Christoph Hellwig <hch@infradead.org> wrote:
>>>> On Wed, Apr 25, 2018 at 01:15:18PM +0200, Arnd Bergmann wrote:
>>>>> That thought had occurred to me as well. I removed the oldest ISDN
>>>>> drivers already some years ago, and the OSS sound drivers
>>>>> got removed as well, and comedi got converted to the dma-mapping
>>>>> interfaces, so there isn't much left at all now. This is what we
>>>>> have as of v4.17-rc1:
>>>>
>>>> Yes, I've been looking at various grotty old bits to purge.  Usually
>>>> I've been looking for some non-tree wide patches and CCed the last
>>>> active people to see if they care.  In a few cases people do, but
>>>> most often no one does.
>>>
>>> Let's start with this one (zoran) then, as Mauro is keen on having
>>> all media drivers compile-testable on x86-64 and arm.
>>>
>>> Trent Piepho and Hans Verkuil both worked on this driver in the
>>> 2008/2009 timeframe and those were the last commits from anyone
>>> who appears to have tested their patches on actual hardware.
>>
>> Zoran is a driver for old hardware. I don't doubt that are people
>> out there still using it, but who knows?
>>
>> I have a few those boards packed somewhere. I haven't work with PCI
>> hardware for a while. If needed, I can try to seek for them and
>> do some tests. I need first to unpack a machine with PCI slots...
>> the NUCs I generally use for development don't have any :-)
>>
>> Anyway, except for virt_to_bus() and related stuff, I think that this
>> driver is in good shape, as Hans did a lot of work in the past to
>> make it to use the current media framework.
>>
>>>
>>> Trent, Hans: do you have reason to believe that there might still
>>> be users out there?
>
> I have no way of knowing this. However, I think they are easily replaced
> by much cheaper USB alternatives today.
>
> I did some work on the zoran driver several years ago, but it doesn't use
> the vb2 framework (and not even the older vb1 framework!) so I'm sure there
> are all sorts of bugs in that driver.
>
> Personally I would be fine with moving this driver to staging and removing
> it by, say, the end of the year.
>
> Nobody is going to work on it and I think it is time to retire it.

Based on the link to the old discussed that Bernhard provided, it
seems that the driver was already in a barely usable state 5 years
ago (the remaining users decided to use an even older 2.6 kernel instead),
and nobody has worked on fixing it since, so moving it to staging or
immediately removing it would both seem appropriate.

        Arnd

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

end of thread, other threads:[~2018-05-07 21:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-24 20:40 [PATCH] media: zoran: move to dma-mapping interface Arnd Bergmann
2018-04-25  6:15 ` Christoph Hellwig
2018-04-25  7:08   ` Arnd Bergmann
2018-04-25  7:21     ` Christoph Hellwig
2018-04-25 11:15       ` Arnd Bergmann
2018-04-25 15:26         ` Christoph Hellwig
2018-04-25 15:58           ` Arnd Bergmann
2018-04-25 17:22             ` Mauro Carvalho Chehab
2018-04-25 17:43               ` Trent Piepho
2018-05-07  9:05               ` Hans Verkuil
2018-05-07 21:17                 ` Arnd Bergmann
2018-04-25 17:27       ` [Mjpeg-users] " Bernhard Praschinger
2018-04-26 17:49 ` kbuild test robot

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).