linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] media: tw686x: Improvements
@ 2016-04-01 22:38 Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 1/7] tw686x: Specify that the DMA is 32 bits Ezequiel Garcia
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Hi,

This patchset contains two groups of improvements:

 * Add support for frame (zero-copy) DMA mode,
   and also scatter-gather DMA mode.

 * Audio improvements: prevent hw param changes,
   and allow period size configuration.

The DMA modes are selected at module insertion time,
through a module parameter called dma_mode.

While it should be theoretically possible to switch this at
runtime, I believe the complexity involved does not worth the
benefit: users will be interested in some specific use case,
and thus will set the DMA mode right from the start.

Note that in scatter-gather mode, only V4L2_FIELD_SEQ_TB
is possible. As far as I can see, in this DMA mode, the
device delivers a sequential top-bottom field frame,
and cannot deliver separate top/bottom frames.

The only sane thing we can do is limit the DMA length
and get a top (or bottom field), but I doubt that's
useful for anyone.

Then again, any additional support can be done as follow
up patches. For instance, the current series does not add
support for the "FIELD" mode provided by the device.

The reason for this is that it looked quite complex, and
required a lot of changes. I'd really like to avoid adding
any complexity until we have some users demanding it.

Series based on v4.6-rc1, plus patch "media: Support
Intersil/Techwell TW686x-based video capture cards"
(https://patchwork.linuxtv.org/patch/33546/)

Ezequiel Garcia (7):
  tw686x: Specify that the DMA is 32 bits
  tw686x: Introduce an interface to support multiple DMA modes
  tw686x: Add support for DMA contiguous interlaced frame mode
  tw686x: Add support for DMA scatter-gather mode
  tw686x: audio: Implement non-memcpy capture
  tw686x: audio: Allow to configure the period size
  tw686x: audio: Prevent hw param changes while busy

 drivers/media/pci/tw686x/Kconfig        |   2 +
 drivers/media/pci/tw686x/tw686x-audio.c |  92 ++++--
 drivers/media/pci/tw686x/tw686x-core.c  |  56 +++-
 drivers/media/pci/tw686x/tw686x-regs.h  |   9 +
 drivers/media/pci/tw686x/tw686x-video.c | 493 +++++++++++++++++++++++++-------
 drivers/media/pci/tw686x/tw686x.h       |  41 ++-
 6 files changed, 545 insertions(+), 148 deletions(-)

-- 
2.7.0


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

* [PATCH 1/7] tw686x: Specify that the DMA is 32 bits
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 2/7] tw686x: Introduce an interface to support multiple DMA modes Ezequiel Garcia
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Set vb2_queue.gfp_flags to GFP_DMA32. Otherwise it will start to
create bounce buffers which is something you want to avoid since
those are in limited supply.

Without this patch, DMA scatter-gather may not work because
machines can ran out of buffers easily.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
--
Hans,

Feel free to squash this patch into the previous patch,
or to apply it independently.

Thanks,
Ezequiel
---
 drivers/media/pci/tw686x/tw686x-video.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index a3505aa66b77..19a348fe04e3 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -848,6 +848,7 @@ int tw686x_video_init(struct tw686x_dev *dev)
 		vc->vidq.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		vc->vidq.min_buffers_needed = 2;
 		vc->vidq.lock = &vc->vb_mutex;
+		vc->vidq.gfp_flags = GFP_DMA32;
 
 		err = vb2_queue_init(&vc->vidq);
 		if (err) {
-- 
2.7.0


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

* [PATCH 2/7] tw686x: Introduce an interface to support multiple DMA modes
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 1/7] tw686x: Specify that the DMA is 32 bits Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 3/7] tw686x: Add support for DMA contiguous interlaced frame mode Ezequiel Garcia
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Let's set the corner stone to support all the DMA modes
available on this device.

For stability reasons, the driver is currently setting DMA frame
mode, and using single DMA buffers to get the P and B buffers.
Each frame is then memcpy'ed into the user buffer.

However, other platforms might be interested in avoiding this
memcpy, or in taking advantage of the chip's DMA scatter-gather
capabilities.

To achieve this, this commit introduces a "dma_mode" module parameter,
and a tw686x_dma_ops struct. This will allow to define functions to
alloc/free DMA buffers, and to return the frames to userspace.

The memcpy-based method described above is named as dma_mode="memcpy".
Current alloc/free functions are renamed as tw686x_memcpy_xxx,
and are now used through a memcpy_dma_ops.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/pci/tw686x/tw686x-core.c  |  48 +++++-
 drivers/media/pci/tw686x/tw686x-regs.h  |   5 +
 drivers/media/pci/tw686x/tw686x-video.c | 254 ++++++++++++++++++--------------
 drivers/media/pci/tw686x/tw686x.h       |  20 ++-
 4 files changed, 206 insertions(+), 121 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-core.c b/drivers/media/pci/tw686x/tw686x-core.c
index cf53b0e97be2..01c06bb59e78 100644
--- a/drivers/media/pci/tw686x/tw686x-core.c
+++ b/drivers/media/pci/tw686x/tw686x-core.c
@@ -21,12 +21,14 @@
  * under stress testings it has been found that the machine can
  * freeze completely if DMA registers are programmed while streaming
  * is active.
- * This driver tries to access hardware registers as infrequently
- * as possible by:
- *   i.  allocating fixed DMA buffers and memcpy'ing into
- *       vmalloc'ed buffers
- *   ii. using a timer to mitigate the rate of DMA reset operations,
- *       on DMA channels error.
+ *
+ * Therefore, driver implements a dma_mode called 'memcpy' which
+ * avoids cycling the DMA buffers, and insteads allocates extra DMA buffers
+ * and then copies into vmalloc'ed user buffers.
+ *
+ * In addition to this, when streaming is on, the driver tries to access
+ * hardware registers as infrequently as possible. This is done by using
+ * a timer to limit the rate at which DMA is reset on DMA channels error.
  */
 
 #include <linux/init.h>
@@ -55,6 +57,34 @@ static u32 dma_interval = 0x00098968;
 module_param(dma_interval, int, 0444);
 MODULE_PARM_DESC(dma_interval, "Minimum time span for DMA interrupting host");
 
+static unsigned int dma_mode = TW686X_DMA_MODE_MEMCPY;
+static const char *dma_mode_name(unsigned int mode)
+{
+	switch (mode) {
+	case TW686X_DMA_MODE_MEMCPY:
+		return "memcpy";
+	default:
+		return "unknown";
+	}
+}
+
+static int tw686x_dma_mode_get(char *buffer, struct kernel_param *kp)
+{
+	return sprintf(buffer, dma_mode_name(dma_mode));
+}
+
+static int tw686x_dma_mode_set(const char *val, struct kernel_param *kp)
+{
+	if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_MEMCPY)))
+		dma_mode = TW686X_DMA_MODE_MEMCPY;
+	else
+		return -EINVAL;
+	return 0;
+}
+module_param_call(dma_mode, tw686x_dma_mode_set, tw686x_dma_mode_get,
+		  &dma_mode, S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(dma_mode, "DMA operation mode");
+
 void tw686x_disable_channel(struct tw686x_dev *dev, unsigned int channel)
 {
 	u32 dma_en = reg_read(dev, DMA_CHANNEL_ENABLE);
@@ -212,6 +242,7 @@ static int tw686x_probe(struct pci_dev *pci_dev,
 	if (!dev)
 		return -ENOMEM;
 	dev->type = pci_id->driver_data;
+	dev->dma_mode = dma_mode;
 	sprintf(dev->name, "tw%04X", pci_dev->device);
 
 	dev->video_channels = kcalloc(max_channels(dev),
@@ -228,9 +259,10 @@ static int tw686x_probe(struct pci_dev *pci_dev,
 		goto free_video;
 	}
 
-	pr_info("%s: PCI %s, IRQ %d, MMIO 0x%lx\n", dev->name,
+	pr_info("%s: PCI %s, IRQ %d, MMIO 0x%lx (%s mode)\n", dev->name,
 		pci_name(pci_dev), pci_dev->irq,
-		(unsigned long)pci_resource_start(pci_dev, 0));
+		(unsigned long)pci_resource_start(pci_dev, 0),
+		dma_mode_name(dma_mode));
 
 	dev->pci_dev = pci_dev;
 	if (pci_enable_device(pci_dev)) {
diff --git a/drivers/media/pci/tw686x/tw686x-regs.h b/drivers/media/pci/tw686x/tw686x-regs.h
index fcef586a4c8c..37c39bcd7572 100644
--- a/drivers/media/pci/tw686x/tw686x-regs.h
+++ b/drivers/media/pci/tw686x/tw686x-regs.h
@@ -119,4 +119,9 @@
 #define TW686X_STD_PAL_CN	5
 #define TW686X_STD_PAL_60	6
 
+#define TW686X_FIELD_MODE	0x3
+#define TW686X_FRAME_MODE	0x2
+/* 0x1 is reserved */
+#define TW686X_SG_MODE		0x0
+
 #define TW686X_FIFO_ERROR(x)	(x & ~(0xff))
diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index 19a348fe04e3..82ae607b1d01 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -43,6 +43,111 @@ static const struct tw686x_format formats[] = {
 	}
 };
 
+static void tw686x_buf_done(struct tw686x_video_channel *vc,
+			    unsigned int pb)
+{
+	struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
+	struct tw686x_dev *dev = vc->dev;
+	struct vb2_v4l2_buffer *vb;
+	struct vb2_buffer *vb2_buf;
+
+	if (vc->curr_bufs[pb]) {
+		vb = &vc->curr_bufs[pb]->vb;
+
+		vb->field = dev->dma_ops->field;
+		vb->sequence = vc->sequence++;
+		vb2_buf = &vb->vb2_buf;
+
+		if (dev->dma_mode == TW686X_DMA_MODE_MEMCPY)
+			memcpy(vb2_plane_vaddr(vb2_buf, 0), desc->virt,
+			       desc->size);
+		vb2_buf->timestamp = ktime_get_ns();
+		vb2_buffer_done(vb2_buf, VB2_BUF_STATE_DONE);
+	}
+
+	vc->pb = !pb;
+}
+
+/*
+ * We can call this even when alloc_dma failed for the given channel
+ */
+static void tw686x_memcpy_dma_free(struct tw686x_video_channel *vc,
+				   unsigned int pb)
+{
+	struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
+	struct tw686x_dev *dev = vc->dev;
+	struct pci_dev *pci_dev;
+	unsigned long flags;
+
+	/* Check device presence. Shouldn't really happen! */
+	spin_lock_irqsave(&dev->lock, flags);
+	pci_dev = dev->pci_dev;
+	spin_unlock_irqrestore(&dev->lock, flags);
+	if (!pci_dev) {
+		WARN(1, "trying to deallocate on missing device\n");
+		return;
+	}
+
+	if (desc->virt) {
+		pci_free_consistent(dev->pci_dev, desc->size,
+				    desc->virt, desc->phys);
+		desc->virt = NULL;
+	}
+}
+
+static int tw686x_memcpy_dma_alloc(struct tw686x_video_channel *vc,
+				   unsigned int pb)
+{
+	struct tw686x_dev *dev = vc->dev;
+	u32 reg = pb ? VDMA_B_ADDR[vc->ch] : VDMA_P_ADDR[vc->ch];
+	unsigned int len;
+	void *virt;
+
+	WARN(vc->dma_descs[pb].virt,
+	     "Allocating buffer but previous still here\n");
+
+	len = (vc->width * vc->height * vc->format->depth) >> 3;
+	virt = pci_alloc_consistent(dev->pci_dev, len,
+				    &vc->dma_descs[pb].phys);
+	if (!virt) {
+		v4l2_err(&dev->v4l2_dev,
+			 "dma%d: unable to allocate %s-buffer\n",
+			 vc->ch, pb ? "B" : "P");
+		return -ENOMEM;
+	}
+	vc->dma_descs[pb].size = len;
+	vc->dma_descs[pb].virt = virt;
+	reg_write(dev, reg, vc->dma_descs[pb].phys);
+
+	return 0;
+}
+
+static void tw686x_memcpy_buf_refill(struct tw686x_video_channel *vc,
+				     unsigned int pb)
+{
+	struct tw686x_v4l2_buf *buf;
+
+	while (!list_empty(&vc->vidq_queued)) {
+
+		buf = list_first_entry(&vc->vidq_queued,
+			struct tw686x_v4l2_buf, list);
+		list_del(&buf->list);
+
+		vc->curr_bufs[pb] = buf;
+		return;
+	}
+	vc->curr_bufs[pb] = NULL;
+}
+
+const struct tw686x_dma_ops memcpy_dma_ops = {
+	.alloc		= tw686x_memcpy_dma_alloc,
+	.free		= tw686x_memcpy_dma_free,
+	.buf_refill	= tw686x_memcpy_buf_refill,
+	.mem_ops	= &vb2_vmalloc_memops,
+	.hw_dma_mode	= TW686X_FRAME_MODE,
+	.field		= V4L2_FIELD_INTERLACED,
+};
+
 static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
 {
 	static const unsigned int map[15] = {
@@ -114,6 +219,7 @@ static int tw686x_queue_setup(struct vb2_queue *vq,
 		return 0;
 	}
 
+	alloc_ctxs[0] = vc->dev->alloc_ctx;
 	sizes[0] = szimage;
 	*nplanes = 1;
 	return 0;
@@ -143,75 +249,6 @@ static void tw686x_buf_queue(struct vb2_buffer *vb)
 	spin_unlock_irqrestore(&vc->qlock, flags);
 }
 
-/*
- * We can call this even when alloc_dma failed for the given channel
- */
-static void tw686x_free_dma(struct tw686x_video_channel *vc, unsigned int pb)
-{
-	struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
-	struct tw686x_dev *dev = vc->dev;
-	struct pci_dev *pci_dev;
-	unsigned long flags;
-
-	/* Check device presence. Shouldn't really happen! */
-	spin_lock_irqsave(&dev->lock, flags);
-	pci_dev = dev->pci_dev;
-	spin_unlock_irqrestore(&dev->lock, flags);
-	if (!pci_dev) {
-		WARN(1, "trying to deallocate on missing device\n");
-		return;
-	}
-
-	if (desc->virt) {
-		pci_free_consistent(dev->pci_dev, desc->size,
-				    desc->virt, desc->phys);
-		desc->virt = NULL;
-	}
-}
-
-static int tw686x_alloc_dma(struct tw686x_video_channel *vc, unsigned int pb)
-{
-	struct tw686x_dev *dev = vc->dev;
-	u32 reg = pb ? VDMA_B_ADDR[vc->ch] : VDMA_P_ADDR[vc->ch];
-	unsigned int len;
-	void *virt;
-
-	WARN(vc->dma_descs[pb].virt,
-	     "Allocating buffer but previous still here\n");
-
-	len = (vc->width * vc->height * vc->format->depth) >> 3;
-	virt = pci_alloc_consistent(dev->pci_dev, len,
-				    &vc->dma_descs[pb].phys);
-	if (!virt) {
-		v4l2_err(&dev->v4l2_dev,
-			 "dma%d: unable to allocate %s-buffer\n",
-			 vc->ch, pb ? "B" : "P");
-		return -ENOMEM;
-	}
-	vc->dma_descs[pb].size = len;
-	vc->dma_descs[pb].virt = virt;
-	reg_write(dev, reg, vc->dma_descs[pb].phys);
-
-	return 0;
-}
-
-static void tw686x_buffer_refill(struct tw686x_video_channel *vc,
-				 unsigned int pb)
-{
-	struct tw686x_v4l2_buf *buf;
-
-	while (!list_empty(&vc->vidq_queued)) {
-
-		buf = list_first_entry(&vc->vidq_queued,
-			struct tw686x_v4l2_buf, list);
-		list_del(&buf->list);
-
-		vc->curr_bufs[pb] = buf;
-		return;
-	}
-	vc->curr_bufs[pb] = NULL;
-}
-
 static void tw686x_clear_queue(struct tw686x_video_channel *vc,
 			       enum vb2_buffer_state state)
 {
@@ -253,7 +290,8 @@ static int tw686x_start_streaming(struct vb2_queue *vq, unsigned int count)
 	spin_lock_irqsave(&vc->qlock, flags);
 
 	/* Sanity check */
-	if (!vc->dma_descs[0].virt || !vc->dma_descs[1].virt) {
+	if (dev->dma_mode == TW686X_DMA_MODE_MEMCPY &&
+	    (!vc->dma_descs[0].virt || !vc->dma_descs[1].virt)) {
 		spin_unlock_irqrestore(&vc->qlock, flags);
 		v4l2_err(&dev->v4l2_dev,
 			 "video%d: refusing to start without DMA buffers\n",
@@ -263,7 +301,7 @@ static int tw686x_start_streaming(struct vb2_queue *vq, unsigned int count)
 	}
 
 	for (pb = 0; pb < 2; pb++)
-		tw686x_buffer_refill(vc, pb);
+		dev->dma_ops->buf_refill(vc, pb);
 	spin_unlock_irqrestore(&vc->qlock, flags);
 
 	vc->sequence = 0;
@@ -366,10 +404,11 @@ static int tw686x_g_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct tw686x_video_channel *vc = video_drvdata(file);
+	struct tw686x_dev *dev = vc->dev;
 
 	f->fmt.pix.width = vc->width;
 	f->fmt.pix.height = vc->height;
-	f->fmt.pix.field = V4L2_FIELD_INTERLACED;
+	f->fmt.pix.field = dev->dma_ops->field;
 	f->fmt.pix.pixelformat = vc->format->fourcc;
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
 	f->fmt.pix.bytesperline = (f->fmt.pix.width * vc->format->depth) / 8;
@@ -381,6 +420,7 @@ static int tw686x_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
 	struct tw686x_video_channel *vc = video_drvdata(file);
+	struct tw686x_dev *dev = vc->dev;
 	unsigned int video_height = TW686X_VIDEO_HEIGHT(vc->video_standard);
 	const struct tw686x_format *format;
 
@@ -403,7 +443,7 @@ static int tw686x_try_fmt_vid_cap(struct file *file, void *priv,
 	f->fmt.pix.bytesperline = (f->fmt.pix.width * format->depth) / 8;
 	f->fmt.pix.sizeimage = f->fmt.pix.height * f->fmt.pix.bytesperline;
 	f->fmt.pix.colorspace = V4L2_COLORSPACE_SMPTE170M;
-	f->fmt.pix.field = V4L2_FIELD_INTERLACED;
+	f->fmt.pix.field = dev->dma_ops->field;
 
 	return 0;
 }
@@ -412,6 +452,7 @@ static int tw686x_s_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct tw686x_video_channel *vc = video_drvdata(file);
+	struct tw686x_dev *dev = vc->dev;
 	u32 val, width, line_width, height;
 	unsigned long bitsperframe;
 	int err, pb;
@@ -429,15 +470,16 @@ static int tw686x_s_fmt_vid_cap(struct file *file, void *priv,
 	vc->height = f->fmt.pix.height;
 
 	/* We need new DMA buffers if the framesize has changed */
-	if (bitsperframe != vc->width * vc->height * vc->format->depth) {
+	if (dev->dma_ops->alloc &&
+	    bitsperframe != vc->width * vc->height * vc->format->depth) {
 		for (pb = 0; pb < 2; pb++)
-			tw686x_free_dma(vc, pb);
+			dev->dma_ops->free(vc, pb);
 
 		for (pb = 0; pb < 2; pb++) {
-			err = tw686x_alloc_dma(vc, pb);
+			err = dev->dma_ops->alloc(vc, pb);
 			if (err) {
 				if (pb > 0)
-					tw686x_free_dma(vc, 0);
+					dev->dma_ops->free(vc, 0);
 				return err;
 			}
 		}
@@ -704,26 +746,11 @@ const struct v4l2_ioctl_ops tw686x_video_ioctl_ops = {
 	.vidioc_unsubscribe_event	= v4l2_event_unsubscribe,
 };
 
-static void tw686x_buffer_copy(struct tw686x_video_channel *vc,
-			       unsigned int pb, struct vb2_v4l2_buffer *vb)
-{
-	struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
-	struct vb2_buffer *vb2_buf = &vb->vb2_buf;
-
-	vb->field = V4L2_FIELD_INTERLACED;
-	vb->sequence = vc->sequence++;
-
-	memcpy(vb2_plane_vaddr(vb2_buf, 0), desc->virt, desc->size);
-	vb2_buf->timestamp = ktime_get_ns();
-	vb2_buffer_done(vb2_buf, VB2_BUF_STATE_DONE);
-}
-
 void tw686x_video_irq(struct tw686x_dev *dev, unsigned long requests,
 		      unsigned int pb_status, unsigned int fifo_status,
 		      unsigned int *reset_ch)
 {
 	struct tw686x_video_channel *vc;
-	struct vb2_v4l2_buffer *vb;
 	unsigned long flags;
 	unsigned int ch, pb;
 
@@ -772,14 +799,9 @@ void tw686x_video_irq(struct tw686x_dev *dev, unsigned long requests,
 			continue;
 		}
 
-		/* handle video stream */
 		spin_lock_irqsave(&vc->qlock, flags);
-		if (vc->curr_bufs[pb]) {
-			vb = &vc->curr_bufs[pb]->vb;
-			tw686x_buffer_copy(vc, pb, vb);
-		}
-		vc->pb = !pb;
-		tw686x_buffer_refill(vc, pb);
+		tw686x_buf_done(vc, pb);
+		dev->dma_ops->buf_refill(vc, pb);
 		spin_unlock_irqrestore(&vc->qlock, flags);
 	}
 }
@@ -794,9 +816,13 @@ void tw686x_video_free(struct tw686x_dev *dev)
 		if (vc->device)
 			video_unregister_device(vc->device);
 
-		for (pb = 0; pb < 2; pb++)
-			tw686x_free_dma(vc, pb);
+		if (dev->dma_ops->free)
+			for (pb = 0; pb < 2; pb++)
+				dev->dma_ops->free(vc, pb);
 	}
+
+	if (dev->dma_ops->cleanup)
+		dev->dma_ops->cleanup(dev);
 }
 
 int tw686x_video_init(struct tw686x_dev *dev)
@@ -804,10 +830,21 @@ int tw686x_video_init(struct tw686x_dev *dev)
 	unsigned int ch, val, pb;
 	int err;
 
+	if (dev->dma_mode == TW686X_DMA_MODE_MEMCPY)
+		dev->dma_ops = &memcpy_dma_ops;
+	else
+		return -EINVAL;
+
 	err = v4l2_device_register(&dev->pci_dev->dev, &dev->v4l2_dev);
 	if (err)
 		return err;
 
+	if (dev->dma_ops->setup) {
+		err = dev->dma_ops->setup(dev);
+		if (err)
+			return err;
+	}
+
 	for (ch = 0; ch < max_channels(dev); ch++) {
 		struct tw686x_video_channel *vc = &dev->video_channels[ch];
 		struct video_device *vdev;
@@ -833,10 +870,12 @@ int tw686x_video_init(struct tw686x_dev *dev)
 		reg_write(dev, HACTIVE_LO[ch], 0xd0);
 		reg_write(dev, VIDEO_SIZE[ch], 0);
 
-		for (pb = 0; pb < 2; pb++) {
-			err = tw686x_alloc_dma(vc, pb);
-			if (err)
-				goto error;
+		if (dev->dma_ops->alloc) {
+			for (pb = 0; pb < 2; pb++) {
+				err = dev->dma_ops->alloc(vc, pb);
+				if (err)
+					goto error;
+			}
 		}
 
 		vc->vidq.io_modes = VB2_READ | VB2_MMAP | VB2_DMABUF;
@@ -844,7 +883,7 @@ int tw686x_video_init(struct tw686x_dev *dev)
 		vc->vidq.drv_priv = vc;
 		vc->vidq.buf_struct_size = sizeof(struct tw686x_v4l2_buf);
 		vc->vidq.ops = &tw686x_video_qops;
-		vc->vidq.mem_ops = &vb2_vmalloc_memops;
+		vc->vidq.mem_ops = dev->dma_ops->mem_ops;
 		vc->vidq.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 		vc->vidq.min_buffers_needed = 2;
 		vc->vidq.lock = &vc->vb_mutex;
@@ -906,10 +945,9 @@ int tw686x_video_init(struct tw686x_dev *dev)
 		vc->num = vdev->num;
 	}
 
-	/* Set DMA frame mode on all channels. Only supported mode for now. */
 	val = TW686X_DEF_PHASE_REF;
 	for (ch = 0; ch < max_channels(dev); ch++)
-		val |= TW686X_FRAME_MODE << (16 + ch * 2);
+		val |= dev->dma_ops->hw_dma_mode << (16 + ch * 2);
 	reg_write(dev, PHASE_REF, val);
 
 	reg_write(dev, MISC2[0], 0xe7);
diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
index 103dd4a0d951..2b9884b709e0 100644
--- a/drivers/media/pci/tw686x/tw686x.h
+++ b/drivers/media/pci/tw686x/tw686x.h
@@ -27,16 +27,13 @@
 #define TYPE_SECOND_GEN		0x10
 #define TW686X_DEF_PHASE_REF	0x1518
 
-#define TW686X_FIELD_MODE	0x3
-#define TW686X_FRAME_MODE	0x2
-/* 0x1 is reserved */
-#define TW686X_SG_MODE		0x0
-
 #define TW686X_AUDIO_PAGE_SZ		4096
 #define TW686X_AUDIO_PAGE_MAX		16
 #define TW686X_AUDIO_PERIODS_MIN	2
 #define TW686X_AUDIO_PERIODS_MAX	TW686X_AUDIO_PAGE_MAX
 
+#define TW686X_DMA_MODE_MEMCPY		0
+
 struct tw686x_format {
 	char *name;
 	unsigned fourcc;
@@ -99,6 +96,17 @@ struct tw686x_video_channel {
 	bool no_signal;
 };
 
+struct tw686x_dma_ops {
+	int (*setup)(struct tw686x_dev *dev);
+	void (*cleanup)(struct tw686x_dev *dev);
+	int (*alloc)(struct tw686x_video_channel *vc, unsigned int pb);
+	void (*free)(struct tw686x_video_channel *vc, unsigned int pb);
+	void (*buf_refill)(struct tw686x_video_channel *vc, unsigned int pb);
+	const struct vb2_mem_ops *mem_ops;
+	enum v4l2_field field;
+	u32 hw_dma_mode;
+};
+
 /**
  * struct tw686x_dev - global device status
  * @lock: spinlock controlling access to the
@@ -112,11 +120,13 @@ struct tw686x_dev {
 
 	char name[32];
 	unsigned int type;
+	unsigned int dma_mode;
 	struct pci_dev *pci_dev;
 	__u32 __iomem *mmio;
 
 	void *alloc_ctx;
 
+	const struct tw686x_dma_ops *dma_ops;
 	struct tw686x_video_channel *video_channels;
 	struct tw686x_audio_channel *audio_channels;
 
-- 
2.7.0


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

* [PATCH 3/7] tw686x: Add support for DMA contiguous interlaced frame mode
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 1/7] tw686x: Specify that the DMA is 32 bits Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 2/7] tw686x: Introduce an interface to support multiple DMA modes Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  2016-04-18 16:52   ` Tim Harvey
  2016-04-01 22:38 ` [PATCH 4/7] tw686x: Add support for DMA scatter-gather mode Ezequiel Garcia
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Now that the driver has the infrastructure to support more
DMA modes, let's add the DMA contiguous interlaced frame mode.

In this mode, the DMA P and B buffers are programmed with
the user-provided buffers. When a P (or B) frame is ready,
a new buffer is dequeued into P (or B).

In addition to interlaced fields, the device can also be
programmed to deliver alternate fields. Only interlaced
mode is supported for now.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/pci/tw686x/Kconfig        |  1 +
 drivers/media/pci/tw686x/tw686x-core.c  |  4 +++
 drivers/media/pci/tw686x/tw686x-video.c | 50 +++++++++++++++++++++++++++++++++
 drivers/media/pci/tw686x/tw686x.h       |  1 +
 4 files changed, 56 insertions(+)

diff --git a/drivers/media/pci/tw686x/Kconfig b/drivers/media/pci/tw686x/Kconfig
index fb8536974052..ef8ca85522f8 100644
--- a/drivers/media/pci/tw686x/Kconfig
+++ b/drivers/media/pci/tw686x/Kconfig
@@ -3,6 +3,7 @@ config VIDEO_TW686X
 	depends on PCI && VIDEO_DEV && VIDEO_V4L2 && SND
 	depends on HAS_DMA
 	select VIDEOBUF2_VMALLOC
+	select VIDEOBUF2_DMA_CONTIG
 	select SND_PCM
 	help
 	  Support for Intersil/Techwell TW686x-based frame grabber cards.
diff --git a/drivers/media/pci/tw686x/tw686x-core.c b/drivers/media/pci/tw686x/tw686x-core.c
index 01c06bb59e78..9a7646c0f9f6 100644
--- a/drivers/media/pci/tw686x/tw686x-core.c
+++ b/drivers/media/pci/tw686x/tw686x-core.c
@@ -63,6 +63,8 @@ static const char *dma_mode_name(unsigned int mode)
 	switch (mode) {
 	case TW686X_DMA_MODE_MEMCPY:
 		return "memcpy";
+	case TW686X_DMA_MODE_CONTIG:
+		return "contig";
 	default:
 		return "unknown";
 	}
@@ -77,6 +79,8 @@ static int tw686x_dma_mode_set(const char *val, struct kernel_param *kp)
 {
 	if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_MEMCPY)))
 		dma_mode = TW686X_DMA_MODE_MEMCPY;
+	else if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_CONTIG)))
+		dma_mode = TW686X_DMA_MODE_CONTIG;
 	else
 		return -EINVAL;
 	return 0;
diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index 82ae607b1d01..ed6abb4c41c2 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -19,6 +19,7 @@
 #include <linux/slab.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-event.h>
+#include <media/videobuf2-dma-contig.h>
 #include <media/videobuf2-vmalloc.h>
 #include "tw686x.h"
 #include "tw686x-regs.h"
@@ -148,6 +149,53 @@ const struct tw686x_dma_ops memcpy_dma_ops = {
 	.field		= V4L2_FIELD_INTERLACED,
 };
 
+static void tw686x_contig_buf_refill(struct tw686x_video_channel *vc,
+				     unsigned int pb)
+{
+	struct tw686x_v4l2_buf *buf;
+
+	while (!list_empty(&vc->vidq_queued)) {
+		u32 reg = pb ? VDMA_B_ADDR[vc->ch] : VDMA_P_ADDR[vc->ch];
+		dma_addr_t phys;
+
+		buf = list_first_entry(&vc->vidq_queued,
+			struct tw686x_v4l2_buf, list);
+		list_del(&buf->list);
+
+		phys = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
+		reg_write(vc->dev, reg, phys);
+
+		buf->vb.vb2_buf.state = VB2_BUF_STATE_ACTIVE;
+		vc->curr_bufs[pb] = buf;
+		return;
+	}
+	vc->curr_bufs[pb] = NULL;
+}
+
+static void tw686x_contig_cleanup(struct tw686x_dev *dev)
+{
+	vb2_dma_contig_cleanup_ctx(dev->alloc_ctx);
+}
+
+static int tw686x_contig_setup(struct tw686x_dev *dev)
+{
+	dev->alloc_ctx = vb2_dma_contig_init_ctx(&dev->pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		dev_err(&dev->pci_dev->dev, "unable to init DMA context\n");
+		return PTR_ERR(dev->alloc_ctx);
+	}
+	return 0;
+}
+
+const struct tw686x_dma_ops contig_dma_ops = {
+	.setup		= tw686x_contig_setup,
+	.cleanup	= tw686x_contig_cleanup,
+	.buf_refill	= tw686x_contig_buf_refill,
+	.mem_ops	= &vb2_dma_contig_memops,
+	.hw_dma_mode	= TW686X_FRAME_MODE,
+	.field		= V4L2_FIELD_INTERLACED,
+};
+
 static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
 {
 	static const unsigned int map[15] = {
@@ -832,6 +880,8 @@ int tw686x_video_init(struct tw686x_dev *dev)
 
 	if (dev->dma_mode == TW686X_DMA_MODE_MEMCPY)
 		dev->dma_ops = &memcpy_dma_ops;
+	else if (dev->dma_mode == TW686X_DMA_MODE_CONTIG)
+		dev->dma_ops = &contig_dma_ops;
 	else
 		return -EINVAL;
 
diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
index 2b9884b709e0..938f16b2449a 100644
--- a/drivers/media/pci/tw686x/tw686x.h
+++ b/drivers/media/pci/tw686x/tw686x.h
@@ -33,6 +33,7 @@
 #define TW686X_AUDIO_PERIODS_MAX	TW686X_AUDIO_PAGE_MAX
 
 #define TW686X_DMA_MODE_MEMCPY		0
+#define TW686X_DMA_MODE_CONTIG		1
 
 struct tw686x_format {
 	char *name;
-- 
2.7.0


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

* [PATCH 4/7] tw686x: Add support for DMA scatter-gather mode
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
                   ` (2 preceding siblings ...)
  2016-04-01 22:38 ` [PATCH 3/7] tw686x: Add support for DMA contiguous interlaced frame mode Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  2016-04-18 18:05   ` Tim Harvey
  2016-04-01 22:38 ` [PATCH 5/7] tw686x: audio: Implement non-memcpy capture Ezequiel Garcia
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Now that the driver has the infrastructure to support more
DMA modes, let's add the DMA scatter-gather mode.

In this mode, the device delivers sequential top-bottom
frames.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/pci/tw686x/Kconfig        |   1 +
 drivers/media/pci/tw686x/tw686x-core.c  |   4 +
 drivers/media/pci/tw686x/tw686x-video.c | 188 ++++++++++++++++++++++++++++++++
 drivers/media/pci/tw686x/tw686x.h       |  14 +++
 4 files changed, 207 insertions(+)

diff --git a/drivers/media/pci/tw686x/Kconfig b/drivers/media/pci/tw686x/Kconfig
index ef8ca85522f8..34ff37712313 100644
--- a/drivers/media/pci/tw686x/Kconfig
+++ b/drivers/media/pci/tw686x/Kconfig
@@ -4,6 +4,7 @@ config VIDEO_TW686X
 	depends on HAS_DMA
 	select VIDEOBUF2_VMALLOC
 	select VIDEOBUF2_DMA_CONTIG
+	select VIDEOBUF2_DMA_SG
 	select SND_PCM
 	help
 	  Support for Intersil/Techwell TW686x-based frame grabber cards.
diff --git a/drivers/media/pci/tw686x/tw686x-core.c b/drivers/media/pci/tw686x/tw686x-core.c
index 9a7646c0f9f6..586bc6723c93 100644
--- a/drivers/media/pci/tw686x/tw686x-core.c
+++ b/drivers/media/pci/tw686x/tw686x-core.c
@@ -65,6 +65,8 @@ static const char *dma_mode_name(unsigned int mode)
 		return "memcpy";
 	case TW686X_DMA_MODE_CONTIG:
 		return "contig";
+	case TW686X_DMA_MODE_SG:
+		return "sg";
 	default:
 		return "unknown";
 	}
@@ -81,6 +83,8 @@ static int tw686x_dma_mode_set(const char *val, struct kernel_param *kp)
 		dma_mode = TW686X_DMA_MODE_MEMCPY;
 	else if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_CONTIG)))
 		dma_mode = TW686X_DMA_MODE_CONTIG;
+	else if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_SG)))
+		dma_mode = TW686X_DMA_MODE_SG;
 	else
 		return -EINVAL;
 	return 0;
diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
index ed6abb4c41c2..16228c559f9a 100644
--- a/drivers/media/pci/tw686x/tw686x-video.c
+++ b/drivers/media/pci/tw686x/tw686x-video.c
@@ -20,6 +20,7 @@
 #include <media/v4l2-common.h>
 #include <media/v4l2-event.h>
 #include <media/videobuf2-dma-contig.h>
+#include <media/videobuf2-dma-sg.h>
 #include <media/videobuf2-vmalloc.h>
 #include "tw686x.h"
 #include "tw686x-regs.h"
@@ -28,6 +29,10 @@
 #define TW686X_VIDEO_WIDTH		720
 #define TW686X_VIDEO_HEIGHT(id)		((id & V4L2_STD_625_50) ? 576 : 480)
 
+#define TW686X_MAX_SG_ENTRY_SIZE	4096
+#define TW686X_MAX_SG_DESC_COUNT	256 /* PAL 720x576 needs 203 4-KB pages */
+#define TW686X_SG_TABLE_SIZE		(TW686X_MAX_SG_DESC_COUNT * sizeof(struct tw686x_sg_desc))
+
 static const struct tw686x_format formats[] = {
 	{
 		.fourcc = V4L2_PIX_FMT_UYVY,
@@ -196,6 +201,174 @@ const struct tw686x_dma_ops contig_dma_ops = {
 	.field		= V4L2_FIELD_INTERLACED,
 };
 
+static int tw686x_sg_desc_fill(struct tw686x_sg_desc *descs,
+			       struct tw686x_v4l2_buf *buf,
+			       unsigned int buf_len)
+{
+	struct sg_table *vbuf = vb2_dma_sg_plane_desc(&buf->vb.vb2_buf, 0);
+	unsigned int len, entry_len;
+	struct scatterlist *sg;
+	int i, count;
+
+	/* Clear the scatter-gather table */
+	memset(descs, 0, TW686X_SG_TABLE_SIZE);
+
+	count = 0;
+	for_each_sg(vbuf->sgl, sg, vbuf->nents, i) {
+		dma_addr_t phys = sg_dma_address(sg);
+		len = sg_dma_len(sg);
+
+		while (len && buf_len) {
+
+			if (count == TW686X_MAX_SG_DESC_COUNT)
+				return -ENOMEM;
+
+			entry_len = min_t(unsigned int, len,
+					  TW686X_MAX_SG_ENTRY_SIZE);
+			entry_len = min_t(unsigned int, entry_len, buf_len);
+			descs[count].phys = cpu_to_le32(phys);
+			descs[count++].flags_length =
+					cpu_to_le32(BIT(30) | entry_len);
+			phys += entry_len;
+			len -= entry_len;
+			buf_len -= entry_len;
+		}
+
+		if (!buf_len)
+			return 0;
+	}
+
+	return -ENOMEM;
+}
+
+static void tw686x_sg_buf_refill(struct tw686x_video_channel *vc,
+				 unsigned int pb)
+{
+	struct tw686x_dev *dev = vc->dev;
+	struct tw686x_v4l2_buf *buf;
+
+	while (!list_empty(&vc->vidq_queued)) {
+		unsigned int buf_len;
+
+		buf = list_first_entry(&vc->vidq_queued,
+			struct tw686x_v4l2_buf, list);
+		list_del(&buf->list);
+
+		buf_len = (vc->width * vc->height * vc->format->depth) >> 3;
+		if (tw686x_sg_desc_fill(vc->sg_descs[pb], buf, buf_len)) {
+			v4l2_err(&dev->v4l2_dev,
+				 "dma%d: unable to fill %s-buffer\n",
+				 vc->ch, pb ? "B" : "P");
+			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+			continue;
+		}
+
+		buf->vb.vb2_buf.state = VB2_BUF_STATE_ACTIVE;
+		vc->curr_bufs[pb] = buf;
+		return;
+	}
+
+	vc->curr_bufs[pb] = NULL;
+}
+
+static void tw686x_sg_dma_free(struct tw686x_video_channel *vc,
+			       unsigned int pb)
+{
+	struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
+	struct tw686x_dev *dev = vc->dev;
+
+	if (desc->size) {
+		pci_free_consistent(dev->pci_dev, desc->size,
+				    desc->virt, desc->phys);
+		desc->virt = NULL;
+	}
+
+	vc->sg_descs[pb] = NULL;
+}
+
+static int tw686x_sg_dma_alloc(struct tw686x_video_channel *vc,
+			       unsigned int pb)
+{
+	struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
+	struct tw686x_dev *dev = vc->dev;
+	u32 reg = pb ? DMA_PAGE_TABLE1_ADDR[vc->ch] :
+		       DMA_PAGE_TABLE0_ADDR[vc->ch];
+	void *virt;
+
+	if (desc->size) {
+
+		virt = pci_alloc_consistent(dev->pci_dev, desc->size,
+					    &desc->phys);
+		if (!virt) {
+			v4l2_err(&dev->v4l2_dev,
+				 "dma%d: unable to allocate %s-buffer\n",
+				 vc->ch, pb ? "B" : "P");
+			return -ENOMEM;
+		}
+		desc->virt = virt;
+		reg_write(dev, reg, desc->phys);
+	} else {
+		virt = dev->video_channels[0].dma_descs[pb].virt +
+		       vc->ch * TW686X_SG_TABLE_SIZE;
+	}
+
+	vc->sg_descs[pb] = virt;
+	return 0;
+}
+
+static void tw686x_sg_cleanup(struct tw686x_dev *dev)
+{
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
+}
+
+static int tw686x_sg_setup(struct tw686x_dev *dev)
+{
+	unsigned int sg_table_size, pb, ch, channels;
+
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&dev->pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		dev_err(&dev->pci_dev->dev, "unable to init DMA context\n");
+		return PTR_ERR(dev->alloc_ctx);
+	}
+
+	if (is_second_gen(dev)) {
+		/*
+		 * TW6865/TW6869: each channel needs a pair of
+		 * P-B descriptor tables.
+		 */
+		channels = max_channels(dev);
+		sg_table_size = TW686X_SG_TABLE_SIZE;
+	} else {
+		/*
+		 * TW6864/TW6868: we need to allocate a pair of
+		 * P-B descriptor tables, common for all channels.
+		 * Each table will be bigger than 4 KB.
+		 */
+		channels = 1;
+		sg_table_size = max_channels(dev) * TW686X_SG_TABLE_SIZE;
+	}
+
+	for (ch = 0; ch < channels; ch++) {
+		struct tw686x_video_channel *vc = &dev->video_channels[ch];
+
+		for (pb = 0; pb < 2; pb++)
+			vc->dma_descs[pb].size = sg_table_size;
+	}
+
+	return 0;
+}
+
+const struct tw686x_dma_ops sg_dma_ops = {
+	.setup		= tw686x_sg_setup,
+	.cleanup	= tw686x_sg_cleanup,
+	.alloc		= tw686x_sg_dma_alloc,
+	.free		= tw686x_sg_dma_free,
+	.buf_refill	= tw686x_sg_buf_refill,
+	.mem_ops	= &vb2_dma_sg_memops,
+	.hw_dma_mode	= TW686X_SG_MODE,
+	.field		= V4L2_FIELD_SEQ_TB,
+};
+
 static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
 {
 	static const unsigned int map[15] = {
@@ -545,6 +718,19 @@ static int tw686x_s_fmt_vid_cap(struct file *file, void *priv,
 	else
 		val &= ~BIT(24);
 
+	val &= ~0x7ffff;
+
+	/* Program the DMA scatter-gather */
+	if (dev->dma_mode == TW686X_DMA_MODE_SG) {
+		u32 start_idx, end_idx;
+
+		start_idx = is_second_gen(dev) ?
+				0 : vc->ch * TW686X_MAX_SG_DESC_COUNT;
+		end_idx = start_idx + TW686X_MAX_SG_DESC_COUNT - 1;
+
+		val |= (end_idx << 10) | start_idx;
+	}
+
 	val &= ~(0x7 << 20);
 	val |= vc->format->mode << 20;
 	reg_write(vc->dev, VDMA_CHANNEL_CONFIG[vc->ch], val);
@@ -882,6 +1068,8 @@ int tw686x_video_init(struct tw686x_dev *dev)
 		dev->dma_ops = &memcpy_dma_ops;
 	else if (dev->dma_mode == TW686X_DMA_MODE_CONTIG)
 		dev->dma_ops = &contig_dma_ops;
+	else if (dev->dma_mode == TW686X_DMA_MODE_SG)
+		dev->dma_ops = &sg_dma_ops;
 	else
 		return -EINVAL;
 
diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
index 938f16b2449a..fe848a40f9d0 100644
--- a/drivers/media/pci/tw686x/tw686x.h
+++ b/drivers/media/pci/tw686x/tw686x.h
@@ -34,6 +34,7 @@
 
 #define TW686X_DMA_MODE_MEMCPY		0
 #define TW686X_DMA_MODE_CONTIG		1
+#define TW686X_DMA_MODE_SG		2
 
 struct tw686x_format {
 	char *name;
@@ -48,6 +49,12 @@ struct tw686x_dma_desc {
 	unsigned size;
 };
 
+struct tw686x_sg_desc {
+	/* 3 MSBits for flags, 13 LSBits for length */
+	__le32 flags_length;
+	__le32 phys;
+};
+
 struct tw686x_audio_buf {
 	dma_addr_t dma;
 	void *virt;
@@ -80,6 +87,7 @@ struct tw686x_video_channel {
 	struct video_device *device;
 	struct tw686x_v4l2_buf *curr_bufs[2];
 	struct tw686x_dma_desc dma_descs[2];
+	struct tw686x_sg_desc *sg_descs[2];
 
 	struct v4l2_ctrl_handler ctrl_handler;
 	const struct tw686x_format *format;
@@ -154,6 +162,12 @@ static inline unsigned max_channels(struct tw686x_dev *dev)
 	return dev->type & TYPE_MAX_CHANNELS; /* 4 or 8 channels */
 }
 
+static inline unsigned is_second_gen(struct tw686x_dev *dev)
+{
+	/* each channel has its own DMA SG table */
+	return dev->type & TYPE_SECOND_GEN;
+}
+
 void tw686x_enable_channel(struct tw686x_dev *dev, unsigned int channel);
 void tw686x_disable_channel(struct tw686x_dev *dev, unsigned int channel);
 
-- 
2.7.0


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

* [PATCH 5/7] tw686x: audio: Implement non-memcpy capture
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
                   ` (3 preceding siblings ...)
  2016-04-01 22:38 ` [PATCH 4/7] tw686x: Add support for DMA scatter-gather mode Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 6/7] tw686x: audio: Allow to configure the period size Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 7/7] tw686x: audio: Prevent hw param changes while busy Ezequiel Garcia
  6 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Now that we've introduced the dma_mode parameter to pick the
DMA operation, let's use it to also select the audio DMA
operation.

When dma_mode != memcpy, the driver will avoid using memcpy
in the audio capture path, and the DMA hardware operation
will act directly on the ALSA buffers.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/pci/tw686x/tw686x-audio.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-audio.c b/drivers/media/pci/tw686x/tw686x-audio.c
index 91459ab715b2..a14d1b07edec 100644
--- a/drivers/media/pci/tw686x/tw686x-audio.c
+++ b/drivers/media/pci/tw686x/tw686x-audio.c
@@ -62,12 +62,22 @@ void tw686x_audio_irq(struct tw686x_dev *dev, unsigned long requests,
 		}
 		spin_unlock_irqrestore(&ac->lock, flags);
 
+		if (!done || !next)
+			continue;
+		/*
+		 * Checking for a non-nil dma_desc[pb]->virt buffer is
+		 * the same as checking for memcpy DMA mode.
+		 */
 		desc = &ac->dma_descs[pb];
-		if (done && next && desc->virt) {
-			memcpy(done->virt, desc->virt, desc->size);
-			ac->ptr = done->dma - ac->buf[0].dma;
-			snd_pcm_period_elapsed(ac->ss);
+		if (desc->virt) {
+			memcpy(done->virt, desc->virt,
+			       desc->size);
+		} else {
+			u32 reg = pb ? ADMA_B_ADDR[ch] : ADMA_P_ADDR[ch];
+			reg_write(dev, reg, next->dma);
 		}
+		ac->ptr = done->dma - ac->buf[0].dma;
+		snd_pcm_period_elapsed(ac->ss);
 	}
 }
 
@@ -181,6 +191,12 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss)
 	ac->curr_bufs[0] = p_buf;
 	ac->curr_bufs[1] = b_buf;
 	ac->ptr = 0;
+
+	if (dev->dma_mode != TW686X_DMA_MODE_MEMCPY) {
+		reg_write(dev, ADMA_P_ADDR[ac->ch], p_buf->dma);
+		reg_write(dev, ADMA_B_ADDR[ac->ch], b_buf->dma);
+	}
+
 	spin_unlock_irqrestore(&ac->lock, flags);
 
 	return 0;
@@ -290,6 +306,14 @@ static int tw686x_audio_dma_alloc(struct tw686x_dev *dev,
 {
 	int pb;
 
+	/*
+	 * In the memcpy DMA mode we allocate a consistent buffer
+	 * and use it for the DMA capture. Otherwise, DMA
+	 * acts on the ALSA buffers as received in pcm_prepare.
+	 */
+	if (dev->dma_mode != TW686X_DMA_MODE_MEMCPY)
+		return 0;
+
 	for (pb = 0; pb < 2; pb++) {
 		u32 reg = pb ? ADMA_B_ADDR[ac->ch] : ADMA_P_ADDR[ac->ch];
 		void *virt;
-- 
2.7.0


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

* [PATCH 6/7] tw686x: audio: Allow to configure the period size
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
                   ` (4 preceding siblings ...)
  2016-04-01 22:38 ` [PATCH 5/7] tw686x: audio: Implement non-memcpy capture Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  2016-04-01 22:38 ` [PATCH 7/7] tw686x: audio: Prevent hw param changes while busy Ezequiel Garcia
  6 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Currently, the driver has a fixed period size of 4096 bytes
(2048 frames). Since this hardware can configure the audio
capture size, this commit allows a period size range of [512-4096].

This is very useful to reduce the audio latency.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/pci/tw686x/tw686x-audio.c | 48 ++++++++++++++++++---------------
 drivers/media/pci/tw686x/tw686x-regs.h  |  4 +++
 drivers/media/pci/tw686x/tw686x.h       |  5 ++--
 3 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-audio.c b/drivers/media/pci/tw686x/tw686x-audio.c
index a14d1b07edec..987a22663525 100644
--- a/drivers/media/pci/tw686x/tw686x-audio.c
+++ b/drivers/media/pci/tw686x/tw686x-audio.c
@@ -71,7 +71,7 @@ void tw686x_audio_irq(struct tw686x_dev *dev, unsigned long requests,
 		desc = &ac->dma_descs[pb];
 		if (desc->virt) {
 			memcpy(done->virt, desc->virt,
-			       desc->size);
+			       dev->period_size);
 		} else {
 			u32 reg = pb ? ADMA_B_ADDR[ch] : ADMA_P_ADDR[ch];
 			reg_write(dev, reg, next->dma);
@@ -93,10 +93,11 @@ static int tw686x_pcm_hw_free(struct snd_pcm_substream *ss)
 }
 
 /*
- * The audio device rate is global and shared among all
+ * Audio parameters are global and shared among all
  * capture channels. The driver makes no effort to prevent
- * rate modifications. User is free change the rate, but it
- * means changing the rate for all capture sub-devices.
+ * any modifications. User is free change the audio rate,
+ * or period size, thus changing parameters for all capture
+ * sub-devices.
  */
 static const struct snd_pcm_hardware tw686x_capture_hw = {
 	.info			= (SNDRV_PCM_INFO_MMAP |
@@ -109,9 +110,9 @@ static const struct snd_pcm_hardware tw686x_capture_hw = {
 	.rate_max		= 48000,
 	.channels_min		= 1,
 	.channels_max		= 1,
-	.buffer_bytes_max	= TW686X_AUDIO_PAGE_MAX * TW686X_AUDIO_PAGE_SZ,
-	.period_bytes_min	= TW686X_AUDIO_PAGE_SZ,
-	.period_bytes_max	= TW686X_AUDIO_PAGE_SZ,
+	.buffer_bytes_max	= TW686X_AUDIO_PAGE_MAX * AUDIO_DMA_SIZE_MAX,
+	.period_bytes_min	= AUDIO_DMA_SIZE_MIN,
+	.period_bytes_max	= AUDIO_DMA_SIZE_MAX,
 	.periods_min		= TW686X_AUDIO_PERIODS_MIN,
 	.periods_max		= TW686X_AUDIO_PERIODS_MAX,
 };
@@ -166,12 +167,21 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss)
 		reg_write(dev, AUDIO_CONTROL2, reg);
 	}
 
-	if (period_size != TW686X_AUDIO_PAGE_SZ ||
-	    rt->periods < TW686X_AUDIO_PERIODS_MIN ||
-	    rt->periods > TW686X_AUDIO_PERIODS_MAX) {
-		return -EINVAL;
+	if (dev->period_size != period_size) {
+		u32 reg;
+
+		dev->period_size = period_size;
+		reg = reg_read(dev, AUDIO_CONTROL1);
+		reg &= ~(AUDIO_DMA_SIZE_MASK << AUDIO_DMA_SIZE_SHIFT);
+		reg |= period_size << AUDIO_DMA_SIZE_SHIFT;
+
+		reg_write(dev, AUDIO_CONTROL1, reg);
 	}
 
+	if (rt->periods < TW686X_AUDIO_PERIODS_MIN ||
+	    rt->periods > TW686X_AUDIO_PERIODS_MAX)
+		return -EINVAL;
+
 	spin_lock_irqsave(&ac->lock, flags);
 	INIT_LIST_HEAD(&ac->buf_list);
 
@@ -282,8 +292,8 @@ static int tw686x_snd_pcm_init(struct tw686x_dev *dev)
 	return snd_pcm_lib_preallocate_pages_for_all(pcm,
 				SNDRV_DMA_TYPE_DEV,
 				snd_dma_pci_data(dev->pci_dev),
-				TW686X_AUDIO_PAGE_MAX * TW686X_AUDIO_PAGE_SZ,
-				TW686X_AUDIO_PAGE_MAX * TW686X_AUDIO_PAGE_SZ);
+				TW686X_AUDIO_PAGE_MAX * AUDIO_DMA_SIZE_MAX,
+				TW686X_AUDIO_PAGE_MAX * AUDIO_DMA_SIZE_MAX);
 }
 
 static void tw686x_audio_dma_free(struct tw686x_dev *dev,
@@ -318,7 +328,7 @@ static int tw686x_audio_dma_alloc(struct tw686x_dev *dev,
 		u32 reg = pb ? ADMA_B_ADDR[ac->ch] : ADMA_P_ADDR[ac->ch];
 		void *virt;
 
-		virt = pci_alloc_consistent(dev->pci_dev, TW686X_AUDIO_PAGE_SZ,
+		virt = pci_alloc_consistent(dev->pci_dev, AUDIO_DMA_SIZE_MAX,
 					    &ac->dma_descs[pb].phys);
 		if (!virt) {
 			dev_err(&dev->pci_dev->dev,
@@ -327,7 +337,7 @@ static int tw686x_audio_dma_alloc(struct tw686x_dev *dev,
 			return -ENOMEM;
 		}
 		ac->dma_descs[pb].virt = virt;
-		ac->dma_descs[pb].size = TW686X_AUDIO_PAGE_SZ;
+		ac->dma_descs[pb].size = AUDIO_DMA_SIZE_MAX;
 		reg_write(dev, reg, ac->dma_descs[pb].phys);
 	}
 	return 0;
@@ -358,12 +368,8 @@ int tw686x_audio_init(struct tw686x_dev *dev)
 	struct snd_card *card;
 	int err, ch;
 
-	/*
-	 * AUDIO_CONTROL1
-	 * DMA byte length [31:19] = 4096 (i.e. ALSA period)
-	 * External audio enable [0] = enabled
-	 */
-	reg_write(dev, AUDIO_CONTROL1, 0x80000001);
+	/* Enable external audio */
+	reg_write(dev, AUDIO_CONTROL1, BIT(0));
 
 	err = snd_card_new(&pci_dev->dev, SNDRV_DEFAULT_IDX1,
 			   SNDRV_DEFAULT_STR1,
diff --git a/drivers/media/pci/tw686x/tw686x-regs.h b/drivers/media/pci/tw686x/tw686x-regs.h
index 37c39bcd7572..15a956642ef4 100644
--- a/drivers/media/pci/tw686x/tw686x-regs.h
+++ b/drivers/media/pci/tw686x/tw686x-regs.h
@@ -105,6 +105,10 @@
 						  0x2d0, 0x2d1, 0x2d2, 0x2d3 })
 
 #define SYS_MODE_DMA_SHIFT	13
+#define AUDIO_DMA_SIZE_SHIFT	19
+#define AUDIO_DMA_SIZE_MIN	SZ_512
+#define AUDIO_DMA_SIZE_MAX	SZ_4K
+#define AUDIO_DMA_SIZE_MASK	(SZ_8K - 1)
 
 #define DMA_CMD_ENABLE		BIT(31)
 #define INT_STATUS_DMA_TOUT	BIT(17)
diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
index fe848a40f9d0..a5e94e0454d0 100644
--- a/drivers/media/pci/tw686x/tw686x.h
+++ b/drivers/media/pci/tw686x/tw686x.h
@@ -27,7 +27,6 @@
 #define TYPE_SECOND_GEN		0x10
 #define TW686X_DEF_PHASE_REF	0x1518
 
-#define TW686X_AUDIO_PAGE_SZ		4096
 #define TW686X_AUDIO_PAGE_MAX		16
 #define TW686X_AUDIO_PERIODS_MIN	2
 #define TW686X_AUDIO_PERIODS_MAX	TW686X_AUDIO_PAGE_MAX
@@ -139,7 +138,9 @@ struct tw686x_dev {
 	struct tw686x_video_channel *video_channels;
 	struct tw686x_audio_channel *audio_channels;
 
-	int audio_rate; /* per-device value */
+	/* Per-device audio parameters */
+	int audio_rate;
+	int period_size;
 
 	struct timer_list dma_delay_timer;
 	u32 pending_dma_en; /* must be protected by lock */
-- 
2.7.0


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

* [PATCH 7/7] tw686x: audio: Prevent hw param changes while busy
  2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
                   ` (5 preceding siblings ...)
  2016-04-01 22:38 ` [PATCH 6/7] tw686x: audio: Allow to configure the period size Ezequiel Garcia
@ 2016-04-01 22:38 ` Ezequiel Garcia
  6 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2016-04-01 22:38 UTC (permalink / raw)
  To: linux-media; +Cc: Hans Verkuil, Ezequiel Garcia

Audio hw params are shared across all DMA channels,
so if the user changes any of these while any DMA channel is
enabled, it will impact the enabled channels, potentially causing
serious instability issues.

This commit avoids such situation, by preventing any hw param
change (on any DMA channel) if any other DMA audio channel is capturing.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/pci/tw686x/tw686x-audio.c | 20 ++++++++++++++++----
 drivers/media/pci/tw686x/tw686x.h       |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/tw686x/tw686x-audio.c b/drivers/media/pci/tw686x/tw686x-audio.c
index 987a22663525..96e444c49173 100644
--- a/drivers/media/pci/tw686x/tw686x-audio.c
+++ b/drivers/media/pci/tw686x/tw686x-audio.c
@@ -94,10 +94,8 @@ static int tw686x_pcm_hw_free(struct snd_pcm_substream *ss)
 
 /*
  * Audio parameters are global and shared among all
- * capture channels. The driver makes no effort to prevent
- * any modifications. User is free change the audio rate,
- * or period size, thus changing parameters for all capture
- * sub-devices.
+ * capture channels. The driver prevents changes to
+ * the parameters if any audio channel is capturing.
  */
 static const struct snd_pcm_hardware tw686x_capture_hw = {
 	.info			= (SNDRV_PCM_INFO_MMAP |
@@ -154,6 +152,14 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss)
 	int i;
 
 	spin_lock_irqsave(&dev->lock, flags);
+	/*
+	 * Given the audio parameters are global (i.e. shared across
+	 * DMA channels), we need to check new params are allowed.
+	 */
+	if (((dev->audio_rate != rt->rate) ||
+	     (dev->period_size != period_size)) && dev->audio_enabled)
+		goto err_audio_busy;
+
 	tw686x_disable_channel(dev, AUDIO_CHANNEL_OFFSET + ac->ch);
 	spin_unlock_irqrestore(&dev->lock, flags);
 
@@ -210,6 +216,10 @@ static int tw686x_pcm_prepare(struct snd_pcm_substream *ss)
 	spin_unlock_irqrestore(&ac->lock, flags);
 
 	return 0;
+
+err_audio_busy:
+	spin_unlock_irqrestore(&dev->lock, flags);
+	return -EBUSY;
 }
 
 static int tw686x_pcm_trigger(struct snd_pcm_substream *ss, int cmd)
@@ -223,6 +233,7 @@ static int tw686x_pcm_trigger(struct snd_pcm_substream *ss, int cmd)
 	case SNDRV_PCM_TRIGGER_START:
 		if (ac->curr_bufs[0] && ac->curr_bufs[1]) {
 			spin_lock_irqsave(&dev->lock, flags);
+			dev->audio_enabled = 1;
 			tw686x_enable_channel(dev,
 				AUDIO_CHANNEL_OFFSET + ac->ch);
 			spin_unlock_irqrestore(&dev->lock, flags);
@@ -235,6 +246,7 @@ static int tw686x_pcm_trigger(struct snd_pcm_substream *ss, int cmd)
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
 		spin_lock_irqsave(&dev->lock, flags);
+		dev->audio_enabled = 0;
 		tw686x_disable_channel(dev, AUDIO_CHANNEL_OFFSET + ac->ch);
 		spin_unlock_irqrestore(&dev->lock, flags);
 
diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
index a5e94e0454d0..66f1a33fdcec 100644
--- a/drivers/media/pci/tw686x/tw686x.h
+++ b/drivers/media/pci/tw686x/tw686x.h
@@ -141,6 +141,7 @@ struct tw686x_dev {
 	/* Per-device audio parameters */
 	int audio_rate;
 	int period_size;
+	int audio_enabled;
 
 	struct timer_list dma_delay_timer;
 	u32 pending_dma_en; /* must be protected by lock */
-- 
2.7.0


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

* Re: [PATCH 3/7] tw686x: Add support for DMA contiguous interlaced frame mode
  2016-04-01 22:38 ` [PATCH 3/7] tw686x: Add support for DMA contiguous interlaced frame mode Ezequiel Garcia
@ 2016-04-18 16:52   ` Tim Harvey
  0 siblings, 0 replies; 10+ messages in thread
From: Tim Harvey @ 2016-04-18 16:52 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, Krzysztof Hałasa

On Fri, Apr 1, 2016 at 3:38 PM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> Now that the driver has the infrastructure to support more
> DMA modes, let's add the DMA contiguous interlaced frame mode.
>
> In this mode, the DMA P and B buffers are programmed with
> the user-provided buffers. When a P (or B) frame is ready,
> a new buffer is dequeued into P (or B).
>
> In addition to interlaced fields, the device can also be
> programmed to deliver alternate fields. Only interlaced
> mode is supported for now.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/media/pci/tw686x/Kconfig        |  1 +
>  drivers/media/pci/tw686x/tw686x-core.c  |  4 +++
>  drivers/media/pci/tw686x/tw686x-video.c | 50 +++++++++++++++++++++++++++++++++
>  drivers/media/pci/tw686x/tw686x.h       |  1 +
>  4 files changed, 56 insertions(+)
>
> diff --git a/drivers/media/pci/tw686x/Kconfig b/drivers/media/pci/tw686x/Kconfig
> index fb8536974052..ef8ca85522f8 100644
> --- a/drivers/media/pci/tw686x/Kconfig
> +++ b/drivers/media/pci/tw686x/Kconfig
> @@ -3,6 +3,7 @@ config VIDEO_TW686X
>         depends on PCI && VIDEO_DEV && VIDEO_V4L2 && SND
>         depends on HAS_DMA
>         select VIDEOBUF2_VMALLOC
> +       select VIDEOBUF2_DMA_CONTIG
>         select SND_PCM
>         help
>           Support for Intersil/Techwell TW686x-based frame grabber cards.
> diff --git a/drivers/media/pci/tw686x/tw686x-core.c b/drivers/media/pci/tw686x/tw686x-core.c
> index 01c06bb59e78..9a7646c0f9f6 100644
> --- a/drivers/media/pci/tw686x/tw686x-core.c
> +++ b/drivers/media/pci/tw686x/tw686x-core.c
> @@ -63,6 +63,8 @@ static const char *dma_mode_name(unsigned int mode)
>         switch (mode) {
>         case TW686X_DMA_MODE_MEMCPY:
>                 return "memcpy";
> +       case TW686X_DMA_MODE_CONTIG:
> +               return "contig";
>         default:
>                 return "unknown";
>         }
> @@ -77,6 +79,8 @@ static int tw686x_dma_mode_set(const char *val, struct kernel_param *kp)
>  {
>         if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_MEMCPY)))
>                 dma_mode = TW686X_DMA_MODE_MEMCPY;
> +       else if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_CONTIG)))
> +               dma_mode = TW686X_DMA_MODE_CONTIG;
>         else
>                 return -EINVAL;
>         return 0;
> diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
> index 82ae607b1d01..ed6abb4c41c2 100644
> --- a/drivers/media/pci/tw686x/tw686x-video.c
> +++ b/drivers/media/pci/tw686x/tw686x-video.c
> @@ -19,6 +19,7 @@
>  #include <linux/slab.h>
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-event.h>
> +#include <media/videobuf2-dma-contig.h>
>  #include <media/videobuf2-vmalloc.h>
>  #include "tw686x.h"
>  #include "tw686x-regs.h"
> @@ -148,6 +149,53 @@ const struct tw686x_dma_ops memcpy_dma_ops = {
>         .field          = V4L2_FIELD_INTERLACED,
>  };
>
> +static void tw686x_contig_buf_refill(struct tw686x_video_channel *vc,
> +                                    unsigned int pb)
> +{
> +       struct tw686x_v4l2_buf *buf;
> +
> +       while (!list_empty(&vc->vidq_queued)) {
> +               u32 reg = pb ? VDMA_B_ADDR[vc->ch] : VDMA_P_ADDR[vc->ch];
> +               dma_addr_t phys;
> +
> +               buf = list_first_entry(&vc->vidq_queued,
> +                       struct tw686x_v4l2_buf, list);
> +               list_del(&buf->list);
> +
> +               phys = vb2_dma_contig_plane_dma_addr(&buf->vb.vb2_buf, 0);
> +               reg_write(vc->dev, reg, phys);
> +
> +               buf->vb.vb2_buf.state = VB2_BUF_STATE_ACTIVE;
> +               vc->curr_bufs[pb] = buf;
> +               return;
> +       }
> +       vc->curr_bufs[pb] = NULL;
> +}
> +
> +static void tw686x_contig_cleanup(struct tw686x_dev *dev)
> +{
> +       vb2_dma_contig_cleanup_ctx(dev->alloc_ctx);
> +}
> +
> +static int tw686x_contig_setup(struct tw686x_dev *dev)
> +{
> +       dev->alloc_ctx = vb2_dma_contig_init_ctx(&dev->pci_dev->dev);
> +       if (IS_ERR(dev->alloc_ctx)) {
> +               dev_err(&dev->pci_dev->dev, "unable to init DMA context\n");
> +               return PTR_ERR(dev->alloc_ctx);
> +       }
> +       return 0;
> +}
> +
> +const struct tw686x_dma_ops contig_dma_ops = {
> +       .setup          = tw686x_contig_setup,
> +       .cleanup        = tw686x_contig_cleanup,
> +       .buf_refill     = tw686x_contig_buf_refill,
> +       .mem_ops        = &vb2_dma_contig_memops,
> +       .hw_dma_mode    = TW686X_FRAME_MODE,
> +       .field          = V4L2_FIELD_INTERLACED,
> +};
> +
>  static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
>  {
>         static const unsigned int map[15] = {
> @@ -832,6 +880,8 @@ int tw686x_video_init(struct tw686x_dev *dev)
>
>         if (dev->dma_mode == TW686X_DMA_MODE_MEMCPY)
>                 dev->dma_ops = &memcpy_dma_ops;
> +       else if (dev->dma_mode == TW686X_DMA_MODE_CONTIG)
> +               dev->dma_ops = &contig_dma_ops;
>         else
>                 return -EINVAL;
>
> diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
> index 2b9884b709e0..938f16b2449a 100644
> --- a/drivers/media/pci/tw686x/tw686x.h
> +++ b/drivers/media/pci/tw686x/tw686x.h
> @@ -33,6 +33,7 @@
>  #define TW686X_AUDIO_PERIODS_MAX       TW686X_AUDIO_PAGE_MAX
>
>  #define TW686X_DMA_MODE_MEMCPY         0
> +#define TW686X_DMA_MODE_CONTIG         1
>
>  struct tw686x_format {
>         char *name;
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ezequiel,

Thanks for adding this support, and thanks to Krzysztof for the
orignal driver and efforts as well!

I am able to use this on an IMX6 board with a MiniPCIe Advanced Micro
avc8000 card with the TW6869. With this hardware configuration I can't
use the TW686X_DMA_MODE_MEMCPY as the IMX6 has a limited 16MB iATU for
PCI RX buffers and I can't get enough choherent_pool memory to support
the buffers needed and TW686X_DMA_MODE_CONTIG solves this issue.

Tested-by: Tim Harvey <tharvey@gateworks.com>

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

* Re: [PATCH 4/7] tw686x: Add support for DMA scatter-gather mode
  2016-04-01 22:38 ` [PATCH 4/7] tw686x: Add support for DMA scatter-gather mode Ezequiel Garcia
@ 2016-04-18 18:05   ` Tim Harvey
  0 siblings, 0 replies; 10+ messages in thread
From: Tim Harvey @ 2016-04-18 18:05 UTC (permalink / raw)
  To: Ezequiel Garcia; +Cc: linux-media, Hans Verkuil, Krzysztof Hałasa

On Fri, Apr 1, 2016 at 3:38 PM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> Now that the driver has the infrastructure to support more
> DMA modes, let's add the DMA scatter-gather mode.
>
> In this mode, the device delivers sequential top-bottom
> frames.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/media/pci/tw686x/Kconfig        |   1 +
>  drivers/media/pci/tw686x/tw686x-core.c  |   4 +
>  drivers/media/pci/tw686x/tw686x-video.c | 188 ++++++++++++++++++++++++++++++++
>  drivers/media/pci/tw686x/tw686x.h       |  14 +++
>  4 files changed, 207 insertions(+)
>
> diff --git a/drivers/media/pci/tw686x/Kconfig b/drivers/media/pci/tw686x/Kconfig
> index ef8ca85522f8..34ff37712313 100644
> --- a/drivers/media/pci/tw686x/Kconfig
> +++ b/drivers/media/pci/tw686x/Kconfig
> @@ -4,6 +4,7 @@ config VIDEO_TW686X
>         depends on HAS_DMA
>         select VIDEOBUF2_VMALLOC
>         select VIDEOBUF2_DMA_CONTIG
> +       select VIDEOBUF2_DMA_SG
>         select SND_PCM
>         help
>           Support for Intersil/Techwell TW686x-based frame grabber cards.
> diff --git a/drivers/media/pci/tw686x/tw686x-core.c b/drivers/media/pci/tw686x/tw686x-core.c
> index 9a7646c0f9f6..586bc6723c93 100644
> --- a/drivers/media/pci/tw686x/tw686x-core.c
> +++ b/drivers/media/pci/tw686x/tw686x-core.c
> @@ -65,6 +65,8 @@ static const char *dma_mode_name(unsigned int mode)
>                 return "memcpy";
>         case TW686X_DMA_MODE_CONTIG:
>                 return "contig";
> +       case TW686X_DMA_MODE_SG:
> +               return "sg";
>         default:
>                 return "unknown";
>         }
> @@ -81,6 +83,8 @@ static int tw686x_dma_mode_set(const char *val, struct kernel_param *kp)
>                 dma_mode = TW686X_DMA_MODE_MEMCPY;
>         else if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_CONTIG)))
>                 dma_mode = TW686X_DMA_MODE_CONTIG;
> +       else if (!strcasecmp(val, dma_mode_name(TW686X_DMA_MODE_SG)))
> +               dma_mode = TW686X_DMA_MODE_SG;
>         else
>                 return -EINVAL;
>         return 0;
> diff --git a/drivers/media/pci/tw686x/tw686x-video.c b/drivers/media/pci/tw686x/tw686x-video.c
> index ed6abb4c41c2..16228c559f9a 100644
> --- a/drivers/media/pci/tw686x/tw686x-video.c
> +++ b/drivers/media/pci/tw686x/tw686x-video.c
> @@ -20,6 +20,7 @@
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-event.h>
>  #include <media/videobuf2-dma-contig.h>
> +#include <media/videobuf2-dma-sg.h>
>  #include <media/videobuf2-vmalloc.h>
>  #include "tw686x.h"
>  #include "tw686x-regs.h"
> @@ -28,6 +29,10 @@
>  #define TW686X_VIDEO_WIDTH             720
>  #define TW686X_VIDEO_HEIGHT(id)                ((id & V4L2_STD_625_50) ? 576 : 480)
>
> +#define TW686X_MAX_SG_ENTRY_SIZE       4096
> +#define TW686X_MAX_SG_DESC_COUNT       256 /* PAL 720x576 needs 203 4-KB pages */
> +#define TW686X_SG_TABLE_SIZE           (TW686X_MAX_SG_DESC_COUNT * sizeof(struct tw686x_sg_desc))
> +
>  static const struct tw686x_format formats[] = {
>         {
>                 .fourcc = V4L2_PIX_FMT_UYVY,
> @@ -196,6 +201,174 @@ const struct tw686x_dma_ops contig_dma_ops = {
>         .field          = V4L2_FIELD_INTERLACED,
>  };
>
> +static int tw686x_sg_desc_fill(struct tw686x_sg_desc *descs,
> +                              struct tw686x_v4l2_buf *buf,
> +                              unsigned int buf_len)
> +{
> +       struct sg_table *vbuf = vb2_dma_sg_plane_desc(&buf->vb.vb2_buf, 0);
> +       unsigned int len, entry_len;
> +       struct scatterlist *sg;
> +       int i, count;
> +
> +       /* Clear the scatter-gather table */
> +       memset(descs, 0, TW686X_SG_TABLE_SIZE);
> +
> +       count = 0;
> +       for_each_sg(vbuf->sgl, sg, vbuf->nents, i) {
> +               dma_addr_t phys = sg_dma_address(sg);
> +               len = sg_dma_len(sg);
> +
> +               while (len && buf_len) {
> +
> +                       if (count == TW686X_MAX_SG_DESC_COUNT)
> +                               return -ENOMEM;
> +
> +                       entry_len = min_t(unsigned int, len,
> +                                         TW686X_MAX_SG_ENTRY_SIZE);
> +                       entry_len = min_t(unsigned int, entry_len, buf_len);
> +                       descs[count].phys = cpu_to_le32(phys);
> +                       descs[count++].flags_length =
> +                                       cpu_to_le32(BIT(30) | entry_len);
> +                       phys += entry_len;
> +                       len -= entry_len;
> +                       buf_len -= entry_len;
> +               }
> +
> +               if (!buf_len)
> +                       return 0;
> +       }
> +
> +       return -ENOMEM;
> +}
> +
> +static void tw686x_sg_buf_refill(struct tw686x_video_channel *vc,
> +                                unsigned int pb)
> +{
> +       struct tw686x_dev *dev = vc->dev;
> +       struct tw686x_v4l2_buf *buf;
> +
> +       while (!list_empty(&vc->vidq_queued)) {
> +               unsigned int buf_len;
> +
> +               buf = list_first_entry(&vc->vidq_queued,
> +                       struct tw686x_v4l2_buf, list);
> +               list_del(&buf->list);
> +
> +               buf_len = (vc->width * vc->height * vc->format->depth) >> 3;
> +               if (tw686x_sg_desc_fill(vc->sg_descs[pb], buf, buf_len)) {
> +                       v4l2_err(&dev->v4l2_dev,
> +                                "dma%d: unable to fill %s-buffer\n",
> +                                vc->ch, pb ? "B" : "P");
> +                       vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +                       continue;
> +               }
> +
> +               buf->vb.vb2_buf.state = VB2_BUF_STATE_ACTIVE;
> +               vc->curr_bufs[pb] = buf;
> +               return;
> +       }
> +
> +       vc->curr_bufs[pb] = NULL;
> +}
> +
> +static void tw686x_sg_dma_free(struct tw686x_video_channel *vc,
> +                              unsigned int pb)
> +{
> +       struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
> +       struct tw686x_dev *dev = vc->dev;
> +
> +       if (desc->size) {
> +               pci_free_consistent(dev->pci_dev, desc->size,
> +                                   desc->virt, desc->phys);
> +               desc->virt = NULL;
> +       }
> +
> +       vc->sg_descs[pb] = NULL;
> +}
> +
> +static int tw686x_sg_dma_alloc(struct tw686x_video_channel *vc,
> +                              unsigned int pb)
> +{
> +       struct tw686x_dma_desc *desc = &vc->dma_descs[pb];
> +       struct tw686x_dev *dev = vc->dev;
> +       u32 reg = pb ? DMA_PAGE_TABLE1_ADDR[vc->ch] :
> +                      DMA_PAGE_TABLE0_ADDR[vc->ch];
> +       void *virt;
> +
> +       if (desc->size) {
> +
> +               virt = pci_alloc_consistent(dev->pci_dev, desc->size,
> +                                           &desc->phys);
> +               if (!virt) {
> +                       v4l2_err(&dev->v4l2_dev,
> +                                "dma%d: unable to allocate %s-buffer\n",
> +                                vc->ch, pb ? "B" : "P");
> +                       return -ENOMEM;
> +               }
> +               desc->virt = virt;
> +               reg_write(dev, reg, desc->phys);
> +       } else {
> +               virt = dev->video_channels[0].dma_descs[pb].virt +
> +                      vc->ch * TW686X_SG_TABLE_SIZE;
> +       }
> +
> +       vc->sg_descs[pb] = virt;
> +       return 0;
> +}
> +
> +static void tw686x_sg_cleanup(struct tw686x_dev *dev)
> +{
> +       vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
> +}
> +
> +static int tw686x_sg_setup(struct tw686x_dev *dev)
> +{
> +       unsigned int sg_table_size, pb, ch, channels;
> +
> +       dev->alloc_ctx = vb2_dma_sg_init_ctx(&dev->pci_dev->dev);
> +       if (IS_ERR(dev->alloc_ctx)) {
> +               dev_err(&dev->pci_dev->dev, "unable to init DMA context\n");
> +               return PTR_ERR(dev->alloc_ctx);
> +       }
> +
> +       if (is_second_gen(dev)) {
> +               /*
> +                * TW6865/TW6869: each channel needs a pair of
> +                * P-B descriptor tables.
> +                */
> +               channels = max_channels(dev);
> +               sg_table_size = TW686X_SG_TABLE_SIZE;
> +       } else {
> +               /*
> +                * TW6864/TW6868: we need to allocate a pair of
> +                * P-B descriptor tables, common for all channels.
> +                * Each table will be bigger than 4 KB.
> +                */
> +               channels = 1;
> +               sg_table_size = max_channels(dev) * TW686X_SG_TABLE_SIZE;
> +       }
> +
> +       for (ch = 0; ch < channels; ch++) {
> +               struct tw686x_video_channel *vc = &dev->video_channels[ch];
> +
> +               for (pb = 0; pb < 2; pb++)
> +                       vc->dma_descs[pb].size = sg_table_size;
> +       }
> +
> +       return 0;
> +}
> +
> +const struct tw686x_dma_ops sg_dma_ops = {
> +       .setup          = tw686x_sg_setup,
> +       .cleanup        = tw686x_sg_cleanup,
> +       .alloc          = tw686x_sg_dma_alloc,
> +       .free           = tw686x_sg_dma_free,
> +       .buf_refill     = tw686x_sg_buf_refill,
> +       .mem_ops        = &vb2_dma_sg_memops,
> +       .hw_dma_mode    = TW686X_SG_MODE,
> +       .field          = V4L2_FIELD_SEQ_TB,
> +};
> +
>  static unsigned int tw686x_fields_map(v4l2_std_id std, unsigned int fps)
>  {
>         static const unsigned int map[15] = {
> @@ -545,6 +718,19 @@ static int tw686x_s_fmt_vid_cap(struct file *file, void *priv,
>         else
>                 val &= ~BIT(24);
>
> +       val &= ~0x7ffff;
> +
> +       /* Program the DMA scatter-gather */
> +       if (dev->dma_mode == TW686X_DMA_MODE_SG) {
> +               u32 start_idx, end_idx;
> +
> +               start_idx = is_second_gen(dev) ?
> +                               0 : vc->ch * TW686X_MAX_SG_DESC_COUNT;
> +               end_idx = start_idx + TW686X_MAX_SG_DESC_COUNT - 1;
> +
> +               val |= (end_idx << 10) | start_idx;
> +       }
> +
>         val &= ~(0x7 << 20);
>         val |= vc->format->mode << 20;
>         reg_write(vc->dev, VDMA_CHANNEL_CONFIG[vc->ch], val);
> @@ -882,6 +1068,8 @@ int tw686x_video_init(struct tw686x_dev *dev)
>                 dev->dma_ops = &memcpy_dma_ops;
>         else if (dev->dma_mode == TW686X_DMA_MODE_CONTIG)
>                 dev->dma_ops = &contig_dma_ops;
> +       else if (dev->dma_mode == TW686X_DMA_MODE_SG)
> +               dev->dma_ops = &sg_dma_ops;
>         else
>                 return -EINVAL;
>
> diff --git a/drivers/media/pci/tw686x/tw686x.h b/drivers/media/pci/tw686x/tw686x.h
> index 938f16b2449a..fe848a40f9d0 100644
> --- a/drivers/media/pci/tw686x/tw686x.h
> +++ b/drivers/media/pci/tw686x/tw686x.h
> @@ -34,6 +34,7 @@
>
>  #define TW686X_DMA_MODE_MEMCPY         0
>  #define TW686X_DMA_MODE_CONTIG         1
> +#define TW686X_DMA_MODE_SG             2
>
>  struct tw686x_format {
>         char *name;
> @@ -48,6 +49,12 @@ struct tw686x_dma_desc {
>         unsigned size;
>  };
>
> +struct tw686x_sg_desc {
> +       /* 3 MSBits for flags, 13 LSBits for length */
> +       __le32 flags_length;
> +       __le32 phys;
> +};
> +
>  struct tw686x_audio_buf {
>         dma_addr_t dma;
>         void *virt;
> @@ -80,6 +87,7 @@ struct tw686x_video_channel {
>         struct video_device *device;
>         struct tw686x_v4l2_buf *curr_bufs[2];
>         struct tw686x_dma_desc dma_descs[2];
> +       struct tw686x_sg_desc *sg_descs[2];
>
>         struct v4l2_ctrl_handler ctrl_handler;
>         const struct tw686x_format *format;
> @@ -154,6 +162,12 @@ static inline unsigned max_channels(struct tw686x_dev *dev)
>         return dev->type & TYPE_MAX_CHANNELS; /* 4 or 8 channels */
>  }
>
> +static inline unsigned is_second_gen(struct tw686x_dev *dev)
> +{
> +       /* each channel has its own DMA SG table */
> +       return dev->type & TYPE_SECOND_GEN;
> +}
> +
>  void tw686x_enable_channel(struct tw686x_dev *dev, unsigned int channel);
>  void tw686x_disable_channel(struct tw686x_dev *dev, unsigned int channel);
>
> --
> 2.7.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Ezequiel,

Thanks for adding this support, and thanks to Krzysztof for the
original driver and efforts as well!

As with the TW686X_DMA_MODE_CONTIG, this one also helps out in the
hardware config I have with limited coherent_pool mem.

Tested-by: Tim Harvey <tharvey@gateworks.com>

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

end of thread, other threads:[~2016-04-18 18:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-01 22:38 [PATCH 0/7] media: tw686x: Improvements Ezequiel Garcia
2016-04-01 22:38 ` [PATCH 1/7] tw686x: Specify that the DMA is 32 bits Ezequiel Garcia
2016-04-01 22:38 ` [PATCH 2/7] tw686x: Introduce an interface to support multiple DMA modes Ezequiel Garcia
2016-04-01 22:38 ` [PATCH 3/7] tw686x: Add support for DMA contiguous interlaced frame mode Ezequiel Garcia
2016-04-18 16:52   ` Tim Harvey
2016-04-01 22:38 ` [PATCH 4/7] tw686x: Add support for DMA scatter-gather mode Ezequiel Garcia
2016-04-18 18:05   ` Tim Harvey
2016-04-01 22:38 ` [PATCH 5/7] tw686x: audio: Implement non-memcpy capture Ezequiel Garcia
2016-04-01 22:38 ` [PATCH 6/7] tw686x: audio: Allow to configure the period size Ezequiel Garcia
2016-04-01 22:38 ` [PATCH 7/7] tw686x: audio: Prevent hw param changes while busy Ezequiel Garcia

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