linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] One more Marvell cam patch series
@ 2011-07-08 20:50 Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 1/6] marvell-cam: delete struct mcam_sio_buffer Jonathan Corbet
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media; +Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab

Here is yet another set of marvell-cam patches.  These ones clean up some
old cruft, fix one bug, and make it so that you don't have to drag in all
three videobuf2 modes if you don't want them.  It looks like a lot of
churn, but the bulk of it is simply moving functions around into a more
logical organization.

The one thing that's not there, unfortunately, is the removal of the ov7670
dependency.  If Linus does another round or two of rc's before 3.0, I may
just get that done for the merge window; otherwise that will have to be for
3.1.  It's the top item on my list.

The patches can be pulled from:

    	git://git.lwn.net/linux-2.6.git for-mauro

Patches in this series:

Jonathan Corbet (6):
      marvell-cam: delete struct mcam_sio_buffer
      marvell-cam: core code reorganization
      marvell-cam: remove {min,max}_buffers parameters
      marvell-cam: power down mmp camera on registration failure
      marvell-cam: Allow selection of supported buffer modes
      marvell-cam: clean up a couple of unused cam structure fields

 Kconfig      |    3 
 mcam-core.c  | 1048 ++++++++++++++++++++++++++++++-----------------------------
 mcam-core.h  |   69 ++-
 mmp-driver.c |    2 
 4 files changed, 601 insertions(+), 521 deletions(-)

Thanks,

jon



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

* [PATCH 1/6] marvell-cam: delete struct mcam_sio_buffer
  2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
@ 2011-07-08 20:50 ` Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 2/6] marvell-cam: core code reorganization Jonathan Corbet
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media
  Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab, Jonathan Corbet

This structure got passed over in the videobuf2 conversion; it has no
reason to exist now.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/video/marvell-ccic/mcam-core.h |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/media/video/marvell-ccic/mcam-core.h b/drivers/media/video/marvell-ccic/mcam-core.h
index 55fd078..9a39e08 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.h
+++ b/drivers/media/video/marvell-ccic/mcam-core.h
@@ -11,17 +11,6 @@
 #include <media/v4l2-dev.h>
 #include <media/videobuf2-core.h>
 
-/*
- * Tracking of streaming I/O buffers.
- * FIXME doesn't belong in this file
- */
-struct mcam_sio_buffer {
-	struct list_head list;
-	struct v4l2_buffer v4lbuf;
-	char *buffer;   /* Where it lives in kernel space */
-	int mapcount;
-	struct mcam_camera *cam;
-};
 
 enum mcam_state {
 	S_NOTREADY,	/* Not yet initialized */
-- 
1.7.6


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

* [PATCH 2/6] marvell-cam: core code reorganization
  2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 1/6] marvell-cam: delete struct mcam_sio_buffer Jonathan Corbet
@ 2011-07-08 20:50 ` Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 3/6] marvell-cam: remove {min,max}_buffers parameters Jonathan Corbet
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media
  Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab, Jonathan Corbet

This code shows signs of having been mucked with over the last five years
or so; things were kind of mixed up.  This patch reorders functions into a
more rational organization which, with luck, will facilitate making the
buffer modes selectable at configuration time.  Code movement only: no
functional changes here.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/video/marvell-ccic/mcam-core.c |  914 +++++++++++++-------------
 1 files changed, 465 insertions(+), 449 deletions(-)

diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c
index af5faa6..8a99ec2 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.c
+++ b/drivers/media/video/marvell-ccic/mcam-core.c
@@ -157,29 +157,20 @@ static struct mcam_format_struct *mcam_find_format(u32 pixelformat)
 }
 
 /*
- * Start over with DMA buffers - dev_lock needed.
+ * The default format we use until somebody says otherwise.
  */
-static void mcam_reset_buffers(struct mcam_camera *cam)
-{
-	int i;
-
-	cam->next_buf = -1;
-	for (i = 0; i < cam->nbufs; i++)
-		clear_bit(i, &cam->flags);
-}
+static const struct v4l2_pix_format mcam_def_pix_format = {
+	.width		= VGA_WIDTH,
+	.height		= VGA_HEIGHT,
+	.pixelformat	= V4L2_PIX_FMT_YUYV,
+	.field		= V4L2_FIELD_NONE,
+	.bytesperline	= VGA_WIDTH*2,
+	.sizeimage	= VGA_WIDTH*VGA_HEIGHT*2,
+};
 
-static inline int mcam_needs_config(struct mcam_camera *cam)
-{
-	return test_bit(CF_CONFIG_NEEDED, &cam->flags);
-}
+static const enum v4l2_mbus_pixelcode mcam_def_mbus_code =
+					V4L2_MBUS_FMT_YUYV8_2X8;
 
-static void mcam_set_config_needed(struct mcam_camera *cam, int needed)
-{
-	if (needed)
-		set_bit(CF_CONFIG_NEEDED, &cam->flags);
-	else
-		clear_bit(CF_CONFIG_NEEDED, &cam->flags);
-}
 
 /*
  * The two-word DMA descriptor format used by the Armada 610 and like.  There
@@ -210,6 +201,19 @@ static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_buffer *vb)
 	return container_of(vb, struct mcam_vb_buffer, vb_buf);
 }
 
+/*
+ * Hand a completed buffer back to user space.
+ */
+static void mcam_buffer_done(struct mcam_camera *cam, int frame,
+		struct vb2_buffer *vbuf)
+{
+	vbuf->v4l2_buf.bytesused = cam->pix_format.sizeimage;
+	vbuf->v4l2_buf.sequence = cam->buf_seq[frame];
+	vb2_set_plane_payload(vbuf, 0, cam->pix_format.sizeimage);
+	vb2_buffer_done(vbuf, VB2_BUF_STATE_DONE);
+}
+
+
 
 /*
  * Debugging and related.
@@ -222,11 +226,109 @@ static inline struct mcam_vb_buffer *vb_to_mvb(struct vb2_buffer *vb)
 	dev_dbg((cam)->dev, fmt, ##arg);
 
 
+/*
+ * Flag manipulation helpers
+ */
+static void mcam_reset_buffers(struct mcam_camera *cam)
+{
+	int i;
+
+	cam->next_buf = -1;
+	for (i = 0; i < cam->nbufs; i++)
+		clear_bit(i, &cam->flags);
+}
+
+static inline int mcam_needs_config(struct mcam_camera *cam)
+{
+	return test_bit(CF_CONFIG_NEEDED, &cam->flags);
+}
+
+static void mcam_set_config_needed(struct mcam_camera *cam, int needed)
+{
+	if (needed)
+		set_bit(CF_CONFIG_NEEDED, &cam->flags);
+	else
+		clear_bit(CF_CONFIG_NEEDED, &cam->flags);
+}
 
 /* ------------------------------------------------------------------- */
 /*
- * Deal with the controller.
+ * Make the controller start grabbing images.  Everything must
+ * be set up before doing this.
  */
+static void mcam_ctlr_start(struct mcam_camera *cam)
+{
+	/* set_bit performs a read, so no other barrier should be
+	   needed here */
+	mcam_reg_set_bit(cam, REG_CTRL0, C0_ENABLE);
+}
+
+static void mcam_ctlr_stop(struct mcam_camera *cam)
+{
+	mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE);
+}
+
+/* ------------------------------------------------------------------- */
+/*
+ * Code specific to the vmalloc buffer mode.
+ */
+
+/*
+ * Allocate in-kernel DMA buffers for vmalloc mode.
+ */
+static int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime)
+{
+	int i;
+
+	mcam_set_config_needed(cam, 1);
+	if (loadtime)
+		cam->dma_buf_size = dma_buf_size;
+	else
+		cam->dma_buf_size = cam->pix_format.sizeimage;
+	if (n_dma_bufs > 3)
+		n_dma_bufs = 3;
+
+	cam->nbufs = 0;
+	for (i = 0; i < n_dma_bufs; i++) {
+		cam->dma_bufs[i] = dma_alloc_coherent(cam->dev,
+				cam->dma_buf_size, cam->dma_handles + i,
+				GFP_KERNEL);
+		if (cam->dma_bufs[i] == NULL) {
+			cam_warn(cam, "Failed to allocate DMA buffer\n");
+			break;
+		}
+		(cam->nbufs)++;
+	}
+
+	switch (cam->nbufs) {
+	case 1:
+		dma_free_coherent(cam->dev, cam->dma_buf_size,
+				cam->dma_bufs[0], cam->dma_handles[0]);
+		cam->nbufs = 0;
+	case 0:
+		cam_err(cam, "Insufficient DMA buffers, cannot operate\n");
+		return -ENOMEM;
+
+	case 2:
+		if (n_dma_bufs > 2)
+			cam_warn(cam, "Will limp along with only 2 buffers\n");
+		break;
+	}
+	return 0;
+}
+
+static void mcam_free_dma_bufs(struct mcam_camera *cam)
+{
+	int i;
+
+	for (i = 0; i < cam->nbufs; i++) {
+		dma_free_coherent(cam->dev, cam->dma_buf_size,
+				cam->dma_bufs[i], cam->dma_handles[i]);
+		cam->dma_bufs[i] = NULL;
+	}
+	cam->nbufs = 0;
+}
+
 
 /*
  * Set up DMA buffers when operating in vmalloc mode
@@ -251,6 +353,52 @@ static void mcam_ctlr_dma_vmalloc(struct mcam_camera *cam)
 }
 
 /*
+ * Copy data out to user space in the vmalloc case
+ */
+static void mcam_frame_tasklet(unsigned long data)
+{
+	struct mcam_camera *cam = (struct mcam_camera *) data;
+	int i;
+	unsigned long flags;
+	struct mcam_vb_buffer *buf;
+
+	spin_lock_irqsave(&cam->dev_lock, flags);
+	for (i = 0; i < cam->nbufs; i++) {
+		int bufno = cam->next_buf;
+
+		if (cam->state != S_STREAMING || bufno < 0)
+			break;  /* I/O got stopped */
+		if (++(cam->next_buf) >= cam->nbufs)
+			cam->next_buf = 0;
+		if (!test_bit(bufno, &cam->flags))
+			continue;
+		if (list_empty(&cam->buffers)) {
+			singles++;
+			break;  /* Leave it valid, hope for better later */
+		}
+		delivered++;
+		clear_bit(bufno, &cam->flags);
+		buf = list_first_entry(&cam->buffers, struct mcam_vb_buffer,
+				queue);
+		list_del_init(&buf->queue);
+		/*
+		 * Drop the lock during the big copy.  This *should* be safe...
+		 */
+		spin_unlock_irqrestore(&cam->dev_lock, flags);
+		memcpy(vb2_plane_vaddr(&buf->vb_buf, 0), cam->dma_bufs[bufno],
+				cam->pix_format.sizeimage);
+		mcam_buffer_done(cam, bufno, &buf->vb_buf);
+		spin_lock_irqsave(&cam->dev_lock, flags);
+	}
+	spin_unlock_irqrestore(&cam->dev_lock, flags);
+}
+
+
+/* ---------------------------------------------------------------------- */
+/*
+ * DMA-contiguous code.
+ */
+/*
  * Set up a contiguous buffer for the given frame.  Here also is where
  * the underrun strategy is set: if there is no buffer available, reuse
  * the buffer from the other BAR and set the CF_SINGLE_BUFFER flag to
@@ -295,6 +443,26 @@ static void mcam_ctlr_dma_contig(struct mcam_camera *cam)
 	mcam_set_contig_buffer(cam, 1);
 }
 
+/*
+ * Frame completion handling.
+ */
+static void mcam_dma_contig_done(struct mcam_camera *cam, int frame)
+{
+	struct mcam_vb_buffer *buf = cam->vb_bufs[frame];
+
+	if (!test_bit(CF_SINGLE_BUFFER, &cam->flags)) {
+		delivered++;
+		mcam_buffer_done(cam, frame, &buf->vb_buf);
+	}
+	mcam_set_contig_buffer(cam, frame);
+}
+
+
+
+/* ---------------------------------------------------------------------- */
+/*
+ * Scatter/gather-specific code.
+ */
 
 /*
  * Set up the next buffer for S/G I/O; caller should be sure that
@@ -325,8 +493,74 @@ static void mcam_ctlr_dma_sg(struct mcam_camera *cam)
 	cam->nbufs = 3;
 }
 
+
 /*
- * Image format setup, independent of DMA scheme.
+ * Frame completion with S/G is trickier.  We can't muck with
+ * a descriptor chain on the fly, since the controller buffers it
+ * internally.  So we have to actually stop and restart; Marvell
+ * says this is the way to do it.
+ *
+ * Of course, stopping is easier said than done; experience shows
+ * that the controller can start a frame *after* C0_ENABLE has been
+ * cleared.  So when running in S/G mode, the controller is "stopped"
+ * on receipt of the start-of-frame interrupt.  That means we can
+ * safely change the DMA descriptor array here and restart things
+ * (assuming there's another buffer waiting to go).
+ */
+static void mcam_dma_sg_done(struct mcam_camera *cam, int frame)
+{
+	struct mcam_vb_buffer *buf = cam->vb_bufs[0];
+
+	/*
+	 * Very Bad Not Good Things happen if you don't clear
+	 * C1_DESC_ENA before making any descriptor changes.
+	 */
+	mcam_reg_clear_bit(cam, REG_CTRL1, C1_DESC_ENA);
+	/*
+	 * If we have another buffer available, put it in and
+	 * restart the engine.
+	 */
+	if (!list_empty(&cam->buffers)) {
+		mcam_sg_next_buffer(cam);
+		mcam_reg_set_bit(cam, REG_CTRL1, C1_DESC_ENA);
+		mcam_ctlr_start(cam);
+	/*
+	 * Otherwise set CF_SG_RESTART and the controller will
+	 * be restarted once another buffer shows up.
+	 */
+	} else {
+		set_bit(CF_SG_RESTART, &cam->flags);
+		singles++;
+	}
+	/*
+	 * Now we can give the completed frame back to user space.
+	 */
+	delivered++;
+	mcam_buffer_done(cam, frame, &buf->vb_buf);
+}
+
+
+/*
+ * Scatter/gather mode requires stopping the controller between
+ * frames so we can put in a new DMA descriptor array.  If no new
+ * buffer exists at frame completion, the controller is left stopped;
+ * this function is charged with gettig things going again.
+ */
+static void mcam_sg_restart(struct mcam_camera *cam)
+{
+	mcam_ctlr_dma_sg(cam);
+	mcam_ctlr_start(cam);
+	clear_bit(CF_SG_RESTART, &cam->flags);
+}
+
+
+/* ---------------------------------------------------------------------- */
+/*
+ * Buffer-mode-independent controller code.
+ */
+
+/*
+ * Image format setup
  */
 static void mcam_ctlr_image(struct mcam_camera *cam)
 {
@@ -417,34 +651,7 @@ static void mcam_ctlr_irq_disable(struct mcam_camera *cam)
 	mcam_reg_clear_bit(cam, REG_IRQMASK, FRAMEIRQS);
 }
 
-/*
- * Make the controller start grabbing images.  Everything must
- * be set up before doing this.
- */
-static void mcam_ctlr_start(struct mcam_camera *cam)
-{
-	/* set_bit performs a read, so no other barrier should be
-	   needed here */
-	mcam_reg_set_bit(cam, REG_CTRL0, C0_ENABLE);
-}
 
-static void mcam_ctlr_stop(struct mcam_camera *cam)
-{
-	mcam_reg_clear_bit(cam, REG_CTRL0, C0_ENABLE);
-}
-
-/*
- * Scatter/gather mode requires stopping the controller between
- * frames so we can put in a new DMA descriptor array.  If no new
- * buffer exists at frame completion, the controller is left stopped;
- * this function is charged with gettig things going again.
- */
-static void mcam_sg_restart(struct mcam_camera *cam)
-{
-	mcam_ctlr_dma_sg(cam);
-	mcam_ctlr_start(cam);
-	clear_bit(CF_SG_RESTART, &cam->flags);
-}
 
 static void mcam_ctlr_init(struct mcam_camera *cam)
 {
@@ -564,113 +771,44 @@ static int mcam_cam_init(struct mcam_camera *cam)
 		goto out;
 	}
 /* Get/set parameters? */
-	ret = 0;
-	cam->state = S_IDLE;
-out:
-	mcam_ctlr_power_down(cam);
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-/*
- * Configure the sensor to match the parameters we have.  Caller should
- * hold s_mutex
- */
-static int mcam_cam_set_flip(struct mcam_camera *cam)
-{
-	struct v4l2_control ctrl;
-
-	memset(&ctrl, 0, sizeof(ctrl));
-	ctrl.id = V4L2_CID_VFLIP;
-	ctrl.value = flip;
-	return sensor_call(cam, core, s_ctrl, &ctrl);
-}
-
-
-static int mcam_cam_configure(struct mcam_camera *cam)
-{
-	struct v4l2_mbus_framefmt mbus_fmt;
-	int ret;
-
-	v4l2_fill_mbus_format(&mbus_fmt, &cam->pix_format, cam->mbus_code);
-	ret = sensor_call(cam, core, init, 0);
-	if (ret == 0)
-		ret = sensor_call(cam, video, s_mbus_fmt, &mbus_fmt);
-	/*
-	 * OV7670 does weird things if flip is set *before* format...
-	 */
-	ret += mcam_cam_set_flip(cam);
-	return ret;
-}
-
-/* -------------------------------------------------------------------- */
-/*
- * DMA buffer management.  These functions need s_mutex held.
- */
-
-/*
- * Allocate in-kernel DMA buffers for vmalloc mode.
- */
-static int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime)
-{
-	int i;
-
-	mcam_set_config_needed(cam, 1);
-	if (loadtime)
-		cam->dma_buf_size = dma_buf_size;
-	else
-		cam->dma_buf_size = cam->pix_format.sizeimage;
-	if (n_dma_bufs > 3)
-		n_dma_bufs = 3;
-
-	cam->nbufs = 0;
-	for (i = 0; i < n_dma_bufs; i++) {
-		cam->dma_bufs[i] = dma_alloc_coherent(cam->dev,
-				cam->dma_buf_size, cam->dma_handles + i,
-				GFP_KERNEL);
-		if (cam->dma_bufs[i] == NULL) {
-			cam_warn(cam, "Failed to allocate DMA buffer\n");
-			break;
-		}
-		(cam->nbufs)++;
-	}
-
-	switch (cam->nbufs) {
-	case 1:
-		dma_free_coherent(cam->dev, cam->dma_buf_size,
-				cam->dma_bufs[0], cam->dma_handles[0]);
-		cam->nbufs = 0;
-	case 0:
-		cam_err(cam, "Insufficient DMA buffers, cannot operate\n");
-		return -ENOMEM;
-
-	case 2:
-		if (n_dma_bufs > 2)
-			cam_warn(cam, "Will limp along with only 2 buffers\n");
-		break;
-	}
-	return 0;
+	ret = 0;
+	cam->state = S_IDLE;
+out:
+	mcam_ctlr_power_down(cam);
+	mutex_unlock(&cam->s_mutex);
+	return ret;
 }
 
-static void mcam_free_dma_bufs(struct mcam_camera *cam)
+/*
+ * Configure the sensor to match the parameters we have.  Caller should
+ * hold s_mutex
+ */
+static int mcam_cam_set_flip(struct mcam_camera *cam)
 {
-	int i;
+	struct v4l2_control ctrl;
 
-	for (i = 0; i < cam->nbufs; i++) {
-		dma_free_coherent(cam->dev, cam->dma_buf_size,
-				cam->dma_bufs[i], cam->dma_handles[i]);
-		cam->dma_bufs[i] = NULL;
-	}
-	cam->nbufs = 0;
+	memset(&ctrl, 0, sizeof(ctrl));
+	ctrl.id = V4L2_CID_VFLIP;
+	ctrl.value = flip;
+	return sensor_call(cam, core, s_ctrl, &ctrl);
 }
 
 
+static int mcam_cam_configure(struct mcam_camera *cam)
+{
+	struct v4l2_mbus_framefmt mbus_fmt;
+	int ret;
 
-/* ----------------------------------------------------------------------- */
-/*
- * Here starts the V4L2 interface code.
- */
-
+	v4l2_fill_mbus_format(&mbus_fmt, &cam->pix_format, cam->mbus_code);
+	ret = sensor_call(cam, core, init, 0);
+	if (ret == 0)
+		ret = sensor_call(cam, video, s_mbus_fmt, &mbus_fmt);
+	/*
+	 * OV7670 does weird things if flip is set *before* format...
+	 */
+	ret += mcam_cam_set_flip(cam);
+	return ret;
+}
 
 /*
  * Get everything ready, and start grabbing frames.
@@ -728,44 +866,6 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq, unsigned int *nbufs,
 	return 0;
 }
 
-/* DMA_sg only */
-static int mcam_vb_sg_buf_init(struct vb2_buffer *vb)
-{
-	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
-	int ndesc = cam->pix_format.sizeimage/PAGE_SIZE + 1;
-
-	mvb->dma_desc = dma_alloc_coherent(cam->dev,
-			ndesc * sizeof(struct mcam_dma_desc),
-			&mvb->dma_desc_pa, GFP_KERNEL);
-	if (mvb->dma_desc == NULL) {
-		cam_err(cam, "Unable to get DMA descriptor array\n");
-		return -ENOMEM;
-	}
-	return 0;
-}
-
-static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
-{
-	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
-	struct vb2_dma_sg_desc *sgd = vb2_dma_sg_plane_desc(vb, 0);
-	struct mcam_dma_desc *desc = mvb->dma_desc;
-	struct scatterlist *sg;
-	int i;
-
-	mvb->dma_desc_nent = dma_map_sg(cam->dev, sgd->sglist, sgd->num_pages,
-			DMA_FROM_DEVICE);
-	if (mvb->dma_desc_nent <= 0)
-		return -EIO;  /* Not sure what's right here */
-	for_each_sg(sgd->sglist, sg, mvb->dma_desc_nent, i) {
-		desc->dma_addr = sg_dma_address(sg);
-		desc->segment_len = sg_dma_len(sg);
-		desc++;
-	}
-	return 0;
-}
-
 
 static void mcam_vb_buf_queue(struct vb2_buffer *vb)
 {
@@ -785,26 +885,6 @@ static void mcam_vb_buf_queue(struct vb2_buffer *vb)
 }
 
 
-static int mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
-{
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
-	struct vb2_dma_sg_desc *sgd = vb2_dma_sg_plane_desc(vb, 0);
-
-	dma_unmap_sg(cam->dev, sgd->sglist, sgd->num_pages, DMA_FROM_DEVICE);
-	return 0;
-}
-
-static void mcam_vb_sg_buf_cleanup(struct vb2_buffer *vb)
-{
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
-	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
-	int ndesc = cam->pix_format.sizeimage/PAGE_SIZE + 1;
-
-	dma_free_coherent(cam->dev, ndesc * sizeof(struct mcam_dma_desc),
-			mvb->dma_desc, mvb->dma_desc_pa);
-}
-
-
 /*
  * vb2 uses these to release the mutex when waiting in dqbuf.  I'm
  * not actually sure we need to do this (I'm not sure that vb2_dqbuf() needs
@@ -882,8 +962,66 @@ static const struct vb2_ops mcam_vb2_ops = {
 };
 
 /*
- * Scatter/gather mode complicates things somewhat.
+ * Scatter/gather mode uses all of the above functions plus a
+ * few extras to deal with DMA mapping.
  */
+static int mcam_vb_sg_buf_init(struct vb2_buffer *vb)
+{
+	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
+	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+	int ndesc = cam->pix_format.sizeimage/PAGE_SIZE + 1;
+
+	mvb->dma_desc = dma_alloc_coherent(cam->dev,
+			ndesc * sizeof(struct mcam_dma_desc),
+			&mvb->dma_desc_pa, GFP_KERNEL);
+	if (mvb->dma_desc == NULL) {
+		cam_err(cam, "Unable to get DMA descriptor array\n");
+		return -ENOMEM;
+	}
+	return 0;
+}
+
+static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
+{
+	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
+	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_dma_sg_desc *sgd = vb2_dma_sg_plane_desc(vb, 0);
+	struct mcam_dma_desc *desc = mvb->dma_desc;
+	struct scatterlist *sg;
+	int i;
+
+	mvb->dma_desc_nent = dma_map_sg(cam->dev, sgd->sglist, sgd->num_pages,
+			DMA_FROM_DEVICE);
+	if (mvb->dma_desc_nent <= 0)
+		return -EIO;  /* Not sure what's right here */
+	for_each_sg(sgd->sglist, sg, mvb->dma_desc_nent, i) {
+		desc->dma_addr = sg_dma_address(sg);
+		desc->segment_len = sg_dma_len(sg);
+		desc++;
+	}
+	return 0;
+}
+
+static int mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
+{
+	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_dma_sg_desc *sgd = vb2_dma_sg_plane_desc(vb, 0);
+
+	dma_unmap_sg(cam->dev, sgd->sglist, sgd->num_pages, DMA_FROM_DEVICE);
+	return 0;
+}
+
+static void mcam_vb_sg_buf_cleanup(struct vb2_buffer *vb)
+{
+	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
+	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
+	int ndesc = cam->pix_format.sizeimage/PAGE_SIZE + 1;
+
+	dma_free_coherent(cam->dev, ndesc * sizeof(struct mcam_dma_desc),
+			mvb->dma_desc, mvb->dma_desc_pa);
+}
+
+
 static const struct vb2_ops mcam_vb2_sg_ops = {
 	.queue_setup		= mcam_vb_queue_setup,
 	.buf_init		= mcam_vb_sg_buf_init,
@@ -934,23 +1072,10 @@ static void mcam_cleanup_vb2(struct mcam_camera *cam)
 		vb2_dma_contig_cleanup_ctx(cam->vb_alloc_ctx);
 }
 
-static ssize_t mcam_v4l_read(struct file *filp,
-		char __user *buffer, size_t len, loff_t *pos)
-{
-	struct mcam_camera *cam = filp->private_data;
-	int ret;
-
-	mutex_lock(&cam->s_mutex);
-	ret = vb2_read(&cam->vb_queue, buffer, len, pos,
-			filp->f_flags & O_NONBLOCK);
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-
 
+/* ---------------------------------------------------------------------- */
 /*
- * Streaming I/O support.
+ * The long list of V4L2 ioctl() operations.
  */
 
 static int mcam_vidioc_streamon(struct file *filp, void *priv,
@@ -999,105 +1124,31 @@ static int mcam_vidioc_querybuf(struct file *filp, void *priv,
 	int ret;
 
 	mutex_lock(&cam->s_mutex);
-	ret = vb2_querybuf(&cam->vb_queue, buf);
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-static int mcam_vidioc_qbuf(struct file *filp, void *priv,
-		struct v4l2_buffer *buf)
-{
-	struct mcam_camera *cam = filp->private_data;
-	int ret;
-
-	mutex_lock(&cam->s_mutex);
-	ret = vb2_qbuf(&cam->vb_queue, buf);
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-static int mcam_vidioc_dqbuf(struct file *filp, void *priv,
-		struct v4l2_buffer *buf)
-{
-	struct mcam_camera *cam = filp->private_data;
-	int ret;
-
-	mutex_lock(&cam->s_mutex);
-	ret = vb2_dqbuf(&cam->vb_queue, buf, filp->f_flags & O_NONBLOCK);
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-
-static int mcam_v4l_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-	struct mcam_camera *cam = filp->private_data;
-	int ret;
-
-	mutex_lock(&cam->s_mutex);
-	ret = vb2_mmap(&cam->vb_queue, vma);
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-
-
-static int mcam_v4l_open(struct file *filp)
-{
-	struct mcam_camera *cam = video_drvdata(filp);
-	int ret = 0;
-
-	filp->private_data = cam;
-
-	frames = singles = delivered = 0;
-	mutex_lock(&cam->s_mutex);
-	if (cam->users == 0) {
-		ret = mcam_setup_vb2(cam);
-		if (ret)
-			goto out;
-		mcam_ctlr_power_up(cam);
-		__mcam_cam_reset(cam);
-		mcam_set_config_needed(cam, 1);
-	}
-	(cam->users)++;
-out:
-	mutex_unlock(&cam->s_mutex);
-	return ret;
-}
-
-
-static int mcam_v4l_release(struct file *filp)
-{
-	struct mcam_camera *cam = filp->private_data;
-
-	cam_err(cam, "Release, %d frames, %d singles, %d delivered\n", frames,
-			singles, delivered);
-	mutex_lock(&cam->s_mutex);
-	(cam->users)--;
-	if (filp == cam->owner) {
-		mcam_ctlr_stop_dma(cam);
-		cam->owner = NULL;
-	}
-	if (cam->users == 0) {
-		mcam_cleanup_vb2(cam);
-		mcam_ctlr_power_down(cam);
-		if (cam->buffer_mode == B_vmalloc && alloc_bufs_at_read)
-			mcam_free_dma_bufs(cam);
-	}
+	ret = vb2_querybuf(&cam->vb_queue, buf);
 	mutex_unlock(&cam->s_mutex);
-	return 0;
+	return ret;
 }
 
+static int mcam_vidioc_qbuf(struct file *filp, void *priv,
+		struct v4l2_buffer *buf)
+{
+	struct mcam_camera *cam = filp->private_data;
+	int ret;
 
+	mutex_lock(&cam->s_mutex);
+	ret = vb2_qbuf(&cam->vb_queue, buf);
+	mutex_unlock(&cam->s_mutex);
+	return ret;
+}
 
-static unsigned int mcam_v4l_poll(struct file *filp,
-		struct poll_table_struct *pt)
+static int mcam_vidioc_dqbuf(struct file *filp, void *priv,
+		struct v4l2_buffer *buf)
 {
 	struct mcam_camera *cam = filp->private_data;
 	int ret;
 
 	mutex_lock(&cam->s_mutex);
-	ret = vb2_poll(&cam->vb_queue, filp, pt);
+	ret = vb2_dqbuf(&cam->vb_queue, buf, filp->f_flags & O_NONBLOCK);
 	mutex_unlock(&cam->s_mutex);
 	return ret;
 }
@@ -1155,21 +1206,6 @@ static int mcam_vidioc_querycap(struct file *file, void *priv,
 }
 
 
-/*
- * The default format we use until somebody says otherwise.
- */
-static const struct v4l2_pix_format mcam_def_pix_format = {
-	.width		= VGA_WIDTH,
-	.height		= VGA_HEIGHT,
-	.pixelformat	= V4L2_PIX_FMT_YUYV,
-	.field		= V4L2_FIELD_NONE,
-	.bytesperline	= VGA_WIDTH*2,
-	.sizeimage	= VGA_WIDTH*VGA_HEIGHT*2,
-};
-
-static const enum v4l2_mbus_pixelcode mcam_def_mbus_code =
-					V4L2_MBUS_FMT_YUYV8_2X8;
-
 static int mcam_vidioc_enum_fmt_vid_cap(struct file *filp,
 		void *priv, struct v4l2_fmtdesc *fmt)
 {
@@ -1395,21 +1431,6 @@ static int mcam_vidioc_s_register(struct file *file, void *priv,
 }
 #endif
 
-/*
- * This template device holds all of those v4l2 methods; we
- * clone it for specific real devices.
- */
-
-static const struct v4l2_file_operations mcam_v4l_fops = {
-	.owner = THIS_MODULE,
-	.open = mcam_v4l_open,
-	.release = mcam_v4l_release,
-	.read = mcam_v4l_read,
-	.poll = mcam_v4l_poll,
-	.mmap = mcam_v4l_mmap,
-	.unlocked_ioctl = video_ioctl2,
-};
-
 static const struct v4l2_ioctl_ops mcam_v4l_ioctl_ops = {
 	.vidioc_querycap	= mcam_vidioc_querycap,
 	.vidioc_enum_fmt_vid_cap = mcam_vidioc_enum_fmt_vid_cap,
@@ -1440,133 +1461,126 @@ static const struct v4l2_ioctl_ops mcam_v4l_ioctl_ops = {
 #endif
 };
 
-static struct video_device mcam_v4l_template = {
-	.name = "mcam",
-	.tvnorms = V4L2_STD_NTSC_M,
-	.current_norm = V4L2_STD_NTSC_M,  /* make mplayer happy */
-
-	.fops = &mcam_v4l_fops,
-	.ioctl_ops = &mcam_v4l_ioctl_ops,
-	.release = video_device_release_empty,
-};
-
 /* ---------------------------------------------------------------------- */
 /*
- * Interrupt handler stuff
+ * Our various file operations.
  */
+static int mcam_v4l_open(struct file *filp)
+{
+	struct mcam_camera *cam = video_drvdata(filp);
+	int ret = 0;
 
+	filp->private_data = cam;
 
-static void mcam_buffer_done(struct mcam_camera *cam, int frame,
-		struct vb2_buffer *vbuf)
-{
-	vbuf->v4l2_buf.bytesused = cam->pix_format.sizeimage;
-	vbuf->v4l2_buf.sequence = cam->buf_seq[frame];
-	vb2_set_plane_payload(vbuf, 0, cam->pix_format.sizeimage);
-	vb2_buffer_done(vbuf, VB2_BUF_STATE_DONE);
+	frames = singles = delivered = 0;
+	mutex_lock(&cam->s_mutex);
+	if (cam->users == 0) {
+		ret = mcam_setup_vb2(cam);
+		if (ret)
+			goto out;
+		mcam_ctlr_power_up(cam);
+		__mcam_cam_reset(cam);
+		mcam_set_config_needed(cam, 1);
+	}
+	(cam->users)++;
+out:
+	mutex_unlock(&cam->s_mutex);
+	return ret;
 }
 
-/*
- * Copy data out to user space in the vmalloc case
- */
-static void mcam_frame_tasklet(unsigned long data)
-{
-	struct mcam_camera *cam = (struct mcam_camera *) data;
-	int i;
-	unsigned long flags;
-	struct mcam_vb_buffer *buf;
 
-	spin_lock_irqsave(&cam->dev_lock, flags);
-	for (i = 0; i < cam->nbufs; i++) {
-		int bufno = cam->next_buf;
+static int mcam_v4l_release(struct file *filp)
+{
+	struct mcam_camera *cam = filp->private_data;
 
-		if (cam->state != S_STREAMING || bufno < 0)
-			break;  /* I/O got stopped */
-		if (++(cam->next_buf) >= cam->nbufs)
-			cam->next_buf = 0;
-		if (!test_bit(bufno, &cam->flags))
-			continue;
-		if (list_empty(&cam->buffers)) {
-			singles++;
-			break;  /* Leave it valid, hope for better later */
-		}
-		delivered++;
-		clear_bit(bufno, &cam->flags);
-		buf = list_first_entry(&cam->buffers, struct mcam_vb_buffer,
-				queue);
-		list_del_init(&buf->queue);
-		/*
-		 * Drop the lock during the big copy.  This *should* be safe...
-		 */
-		spin_unlock_irqrestore(&cam->dev_lock, flags);
-		memcpy(vb2_plane_vaddr(&buf->vb_buf, 0), cam->dma_bufs[bufno],
-				cam->pix_format.sizeimage);
-		mcam_buffer_done(cam, bufno, &buf->vb_buf);
-		spin_lock_irqsave(&cam->dev_lock, flags);
+	cam_err(cam, "Release, %d frames, %d singles, %d delivered\n", frames,
+			singles, delivered);
+	mutex_lock(&cam->s_mutex);
+	(cam->users)--;
+	if (filp == cam->owner) {
+		mcam_ctlr_stop_dma(cam);
+		cam->owner = NULL;
 	}
-	spin_unlock_irqrestore(&cam->dev_lock, flags);
+	if (cam->users == 0) {
+		mcam_cleanup_vb2(cam);
+		mcam_ctlr_power_down(cam);
+		if (cam->buffer_mode == B_vmalloc && alloc_bufs_at_read)
+			mcam_free_dma_bufs(cam);
+	}
+	mutex_unlock(&cam->s_mutex);
+	return 0;
 }
 
-/*
- * For direct DMA, mark the buffer ready and set up another one.
- */
-static void mcam_dma_contig_done(struct mcam_camera *cam, int frame)
+static ssize_t mcam_v4l_read(struct file *filp,
+		char __user *buffer, size_t len, loff_t *pos)
 {
-	struct mcam_vb_buffer *buf = cam->vb_bufs[frame];
+	struct mcam_camera *cam = filp->private_data;
+	int ret;
 
-	if (!test_bit(CF_SINGLE_BUFFER, &cam->flags)) {
-		delivered++;
-		mcam_buffer_done(cam, frame, &buf->vb_buf);
-	}
-	mcam_set_contig_buffer(cam, frame);
+	mutex_lock(&cam->s_mutex);
+	ret = vb2_read(&cam->vb_queue, buffer, len, pos,
+			filp->f_flags & O_NONBLOCK);
+	mutex_unlock(&cam->s_mutex);
+	return ret;
 }
 
-/*
- * Frame completion with S/G is trickier.  We can't muck with
- * a descriptor chain on the fly, since the controller buffers it
- * internally.  So we have to actually stop and restart; Marvell
- * says this is the way to do it.
- *
- * Of course, stopping is easier said than done; experience shows
- * that the controller can start a frame *after* C0_ENABLE has been
- * cleared.  So when running in S/G mode, the controller is "stopped"
- * on receipt of the start-of-frame interrupt.  That means we can
- * safely change the DMA descriptor array here and restart things
- * (assuming there's another buffer waiting to go).
- */
-static void mcam_dma_sg_done(struct mcam_camera *cam, int frame)
+
+
+static unsigned int mcam_v4l_poll(struct file *filp,
+		struct poll_table_struct *pt)
 {
-	struct mcam_vb_buffer *buf = cam->vb_bufs[0];
+	struct mcam_camera *cam = filp->private_data;
+	int ret;
 
-	/*
-	 * Very Bad Not Good Things happen if you don't clear
-	 * C1_DESC_ENA before making any descriptor changes.
-	 */
-	mcam_reg_clear_bit(cam, REG_CTRL1, C1_DESC_ENA);
-	/*
-	 * If we have another buffer available, put it in and
-	 * restart the engine.
-	 */
-	if (!list_empty(&cam->buffers)) {
-		mcam_sg_next_buffer(cam);
-		mcam_reg_set_bit(cam, REG_CTRL1, C1_DESC_ENA);
-		mcam_ctlr_start(cam);
-	/*
-	 * Otherwise set CF_SG_RESTART and the controller will
-	 * be restarted once another buffer shows up.
-	 */
-	} else {
-		set_bit(CF_SG_RESTART, &cam->flags);
-		singles++;
-	}
-	/*
-	 * Now we can give the completed frame back to user space.
-	 */
-	delivered++;
-	mcam_buffer_done(cam, frame, &buf->vb_buf);
+	mutex_lock(&cam->s_mutex);
+	ret = vb2_poll(&cam->vb_queue, filp, pt);
+	mutex_unlock(&cam->s_mutex);
+	return ret;
+}
+
+
+static int mcam_v4l_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+	struct mcam_camera *cam = filp->private_data;
+	int ret;
+
+	mutex_lock(&cam->s_mutex);
+	ret = vb2_mmap(&cam->vb_queue, vma);
+	mutex_unlock(&cam->s_mutex);
+	return ret;
 }
 
 
 
+static const struct v4l2_file_operations mcam_v4l_fops = {
+	.owner = THIS_MODULE,
+	.open = mcam_v4l_open,
+	.release = mcam_v4l_release,
+	.read = mcam_v4l_read,
+	.poll = mcam_v4l_poll,
+	.mmap = mcam_v4l_mmap,
+	.unlocked_ioctl = video_ioctl2,
+};
+
+
+/*
+ * This template device holds all of those v4l2 methods; we
+ * clone it for specific real devices.
+ */
+static struct video_device mcam_v4l_template = {
+	.name = "mcam",
+	.tvnorms = V4L2_STD_NTSC_M,
+	.current_norm = V4L2_STD_NTSC_M,  /* make mplayer happy */
+
+	.fops = &mcam_v4l_fops,
+	.ioctl_ops = &mcam_v4l_ioctl_ops,
+	.release = video_device_release_empty,
+};
+
+/* ---------------------------------------------------------------------- */
+/*
+ * Interrupt handler stuff
+ */
 static void mcam_frame_complete(struct mcam_camera *cam, int frame)
 {
 	/*
@@ -1600,8 +1614,10 @@ static void mcam_frame_complete(struct mcam_camera *cam, int frame)
 }
 
 
-
-
+/*
+ * The interrupt handler; this needs to be called from the
+ * platform irq handler with the lock held.
+ */
 int mccic_irq(struct mcam_camera *cam, unsigned int irqs)
 {
 	unsigned int frame, handled = 0;
@@ -1636,10 +1652,10 @@ int mccic_irq(struct mcam_camera *cam, unsigned int irqs)
 	return handled;
 }
 
+/* ---------------------------------------------------------------------- */
 /*
  * Registration and such.
  */
-
 static struct ov7670_config sensor_cfg = {
 	/*
 	 * Exclude QCIF mode, because it only captures a tiny portion
-- 
1.7.6


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

* [PATCH 3/6] marvell-cam: remove {min,max}_buffers parameters
  2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 1/6] marvell-cam: delete struct mcam_sio_buffer Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 2/6] marvell-cam: core code reorganization Jonathan Corbet
@ 2011-07-08 20:50 ` Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 4/6] marvell-cam: power down mmp camera on registration failure Jonathan Corbet
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media
  Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab, Jonathan Corbet

Somewhere along the way the code stopped actually paying any attention to
them, and I doubt anybody has ever made use of them.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/video/marvell-ccic/mcam-core.c |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c
index 8a99ec2..9867b3b 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.c
+++ b/drivers/media/video/marvell-ccic/mcam-core.c
@@ -72,19 +72,6 @@ MODULE_PARM_DESC(dma_buf_size,
 		"parameters require larger buffers, an attempt to reallocate "
 		"will be made.");
 
-static int min_buffers = 1;
-module_param(min_buffers, uint, 0644);
-MODULE_PARM_DESC(min_buffers,
-		"The minimum number of streaming I/O buffers we are willing "
-		"to work with.");
-
-static int max_buffers = 10;
-module_param(max_buffers, uint, 0644);
-MODULE_PARM_DESC(max_buffers,
-		"The maximum number of streaming I/O buffers an application "
-		"will be allowed to allocate.  These buffers are big and live "
-		"in vmalloc space.");
-
 static int flip;
 module_param(flip, bool, 0444);
 MODULE_PARM_DESC(flip,
-- 
1.7.6


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

* [PATCH 4/6] marvell-cam: power down mmp camera on registration failure
  2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
                   ` (2 preceding siblings ...)
  2011-07-08 20:50 ` [PATCH 3/6] marvell-cam: remove {min,max}_buffers parameters Jonathan Corbet
@ 2011-07-08 20:50 ` Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 5/6] marvell-cam: Allow selection of supported buffer modes Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 6/6] marvell-cam: clean up a couple of unused cam structure fields Jonathan Corbet
  5 siblings, 0 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media
  Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab, Jonathan Corbet

If registration does not work, we don't want to leave the sensor powered on.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/video/marvell-ccic/mmp-driver.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/marvell-ccic/mmp-driver.c b/drivers/media/video/marvell-ccic/mmp-driver.c
index 8415915..d6b7645 100644
--- a/drivers/media/video/marvell-ccic/mmp-driver.c
+++ b/drivers/media/video/marvell-ccic/mmp-driver.c
@@ -267,8 +267,8 @@ static int mmpcam_probe(struct platform_device *pdev)
 
 out_unregister:
 	mccic_shutdown(mcam);
-	mmpcam_power_down(mcam);
 out_gpio2:
+	mmpcam_power_down(mcam);
 	gpio_free(pdata->sensor_reset_gpio);
 out_gpio:
 	gpio_free(pdata->sensor_power_gpio);
-- 
1.7.6


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

* [PATCH 5/6] marvell-cam: Allow selection of supported buffer modes
  2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
                   ` (3 preceding siblings ...)
  2011-07-08 20:50 ` [PATCH 4/6] marvell-cam: power down mmp camera on registration failure Jonathan Corbet
@ 2011-07-08 20:50 ` Jonathan Corbet
  2011-07-08 20:50 ` [PATCH 6/6] marvell-cam: clean up a couple of unused cam structure fields Jonathan Corbet
  5 siblings, 0 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media
  Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab, Jonathan Corbet

The Marvell camera core can support all three videobuf2 buffer modes, which
is slick, but it also requires that all three modes be built and present,
even though only one is likely to be used.  This patch allows the supported
modes to be selected at configuration time, reducing the footprint of the
driver.  Prior to this patch, the MMP camera driver looked like this:

mmp_camera             19092  0
videobuf2_core         15542  1 mmp_camera
videobuf2_dma_sg        3173  1 mmp_camera
videobuf2_dma_contig     2188  1 mmp_camera
videobuf2_vmalloc       1718  1 mmp_camera
videobuf2_memops        2100  3 videobuf2_dma_sg,videobuf2_dma_contig,videobuf2_vmalloc

Afterward, instead, with scatter/gather only configured:

mmp_camera             16021  0
videobuf2_core         15542  1 mmp_camera
videobuf2_dma_sg        3173  1 mmp_camera
videobuf2_memops        2100  1 videobuf2_dma_sg

The total goes from 43,813 bytes to 36,836.

The emphasis has been on simplicity and minimal #ifdef use rather than on
squeezing out every possible byte of code.  For configuration, the driver
simply looks at which videobuf2 modes have been configured in and supports
them all; it's simplistic but should be good enough.

The cafe driver is set to support vmalloc and dma-contig; mmp supports only
dma-sg, since that's the only mode that really makes sense to use.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/video/marvell-ccic/Kconfig     |    3 -
 drivers/media/video/marvell-ccic/mcam-core.c |  149 +++++++++++++++++---------
 drivers/media/video/marvell-ccic/mcam-core.h |   59 +++++++++-
 3 files changed, 152 insertions(+), 59 deletions(-)

diff --git a/drivers/media/video/marvell-ccic/Kconfig b/drivers/media/video/marvell-ccic/Kconfig
index 5be66e2..bf739e3 100644
--- a/drivers/media/video/marvell-ccic/Kconfig
+++ b/drivers/media/video/marvell-ccic/Kconfig
@@ -4,7 +4,6 @@ config VIDEO_CAFE_CCIC
 	select VIDEO_OV7670
 	select VIDEOBUF2_VMALLOC
 	select VIDEOBUF2_DMA_CONTIG
-	select VIDEOBUF2_DMA_SG
 	---help---
 	  This is a video4linux2 driver for the Marvell 88ALP01 integrated
 	  CMOS camera controller.  This is the controller found on first-
@@ -15,8 +14,6 @@ config VIDEO_MMP_CAMERA
 	depends on ARCH_MMP && I2C && VIDEO_V4L2
 	select VIDEO_OV7670
 	select I2C_GPIO
-	select VIDEOBUF2_VMALLOC
-	select VIDEOBUF2_DMA_CONTIG
 	select VIDEOBUF2_DMA_SG
 	---help---
 	  This is a Video4Linux2 driver for the integrated camera
diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c
index 9867b3b..073e72c 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.c
+++ b/drivers/media/video/marvell-ccic/mcam-core.c
@@ -37,6 +37,7 @@ static int frames;
 static int singles;
 static int delivered;
 
+#ifdef MCAM_MODE_VMALLOC
 /*
  * Internal DMA buffer management.  Since the controller cannot do S/G I/O,
  * we must have physically contiguous buffers to bring frames into.
@@ -71,6 +72,10 @@ MODULE_PARM_DESC(dma_buf_size,
 		"The size of the allocated DMA buffers.  If actual operating "
 		"parameters require larger buffers, an attempt to reallocate "
 		"will be made.");
+#else /* MCAM_MODE_VMALLOC */
+static const int alloc_bufs_at_read = 0;
+static const int n_dma_bufs = 3;  /* Used by S/G_PARM */
+#endif /* MCAM_MODE_VMALLOC */
 
 static int flip;
 module_param(flip, bool, 0444);
@@ -256,6 +261,8 @@ static void mcam_ctlr_stop(struct mcam_camera *cam)
 }
 
 /* ------------------------------------------------------------------- */
+
+#ifdef MCAM_MODE_VMALLOC
 /*
  * Code specific to the vmalloc buffer mode.
  */
@@ -381,6 +388,46 @@ static void mcam_frame_tasklet(unsigned long data)
 }
 
 
+/*
+ * Make sure our allocated buffers are up to the task.
+ */
+static int mcam_check_dma_buffers(struct mcam_camera *cam)
+{
+	if (cam->nbufs > 0 && cam->dma_buf_size < cam->pix_format.sizeimage)
+			mcam_free_dma_bufs(cam);
+	if (cam->nbufs == 0)
+		return mcam_alloc_dma_bufs(cam, 0);
+	return 0;
+}
+
+static void mcam_vmalloc_done(struct mcam_camera *cam, int frame)
+{
+	tasklet_schedule(&cam->s_tasklet);
+}
+
+#else /* MCAM_MODE_VMALLOC */
+
+static inline int mcam_alloc_dma_bufs(struct mcam_camera *cam, int loadtime)
+{
+	return 0;
+}
+
+static inline void mcam_free_dma_bufs(struct mcam_camera *cam)
+{
+	return;
+}
+
+static inline int mcam_check_dma_buffers(struct mcam_camera *cam)
+{
+	return 0;
+}
+
+
+
+#endif /* MCAM_MODE_VMALLOC */
+
+
+#ifdef MCAM_MODE_DMA_CONTIG
 /* ---------------------------------------------------------------------- */
 /*
  * DMA-contiguous code.
@@ -444,8 +491,9 @@ static void mcam_dma_contig_done(struct mcam_camera *cam, int frame)
 	mcam_set_contig_buffer(cam, frame);
 }
 
+#endif /* MCAM_MODE_DMA_CONTIG */
 
-
+#ifdef MCAM_MODE_DMA_SG
 /* ---------------------------------------------------------------------- */
 /*
  * Scatter/gather-specific code.
@@ -540,6 +588,14 @@ static void mcam_sg_restart(struct mcam_camera *cam)
 	clear_bit(CF_SG_RESTART, &cam->flags);
 }
 
+#else /* MCAM_MODE_DMA_SG */
+
+static inline void mcam_sg_restart(struct mcam_camera *cam)
+{
+	return;
+}
+
+#endif /* MCAM_MODE_DMA_SG */
 
 /* ---------------------------------------------------------------------- */
 /*
@@ -605,17 +661,7 @@ static int mcam_ctlr_configure(struct mcam_camera *cam)
 	unsigned long flags;
 
 	spin_lock_irqsave(&cam->dev_lock, flags);
-	switch (cam->buffer_mode) {
-	case B_vmalloc:
-		mcam_ctlr_dma_vmalloc(cam);
-		break;
-	case B_DMA_contig:
-		mcam_ctlr_dma_contig(cam);
-		break;
-	case B_DMA_sg:
-		mcam_ctlr_dma_sg(cam);
-		break;
-	}
+	cam->dma_setup(cam);
 	mcam_ctlr_image(cam);
 	mcam_set_config_needed(cam, 0);
 	clear_bit(CF_SG_RESTART, &cam->flags);
@@ -948,6 +994,8 @@ static const struct vb2_ops mcam_vb2_ops = {
 	.wait_finish		= mcam_vb_wait_finish,
 };
 
+
+#ifdef MCAM_MODE_DMA_SG
 /*
  * Scatter/gather mode uses all of the above functions plus a
  * few extras to deal with DMA mapping.
@@ -1022,6 +1070,8 @@ static const struct vb2_ops mcam_vb2_sg_ops = {
 	.wait_finish		= mcam_vb_wait_finish,
 };
 
+#endif /* MCAM_MODE_DMA_SG */
+
 static int mcam_setup_vb2(struct mcam_camera *cam)
 {
 	struct vb2_queue *vq = &cam->vb_queue;
@@ -1032,21 +1082,35 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
 	INIT_LIST_HEAD(&cam->buffers);
 	switch (cam->buffer_mode) {
 	case B_DMA_contig:
+#ifdef MCAM_MODE_DMA_CONTIG
 		vq->ops = &mcam_vb2_ops;
 		vq->mem_ops = &vb2_dma_contig_memops;
 		cam->vb_alloc_ctx = vb2_dma_contig_init_ctx(cam->dev);
 		vq->io_modes = VB2_MMAP | VB2_USERPTR;
+		cam->dma_setup = mcam_ctlr_dma_contig;
+		cam->frame_complete = mcam_dma_contig_done;
+#endif
 		break;
 	case B_DMA_sg:
+#ifdef MCAM_MODE_DMA_SG
 		vq->ops = &mcam_vb2_sg_ops;
 		vq->mem_ops = &vb2_dma_sg_memops;
 		vq->io_modes = VB2_MMAP | VB2_USERPTR;
+		cam->dma_setup = mcam_ctlr_dma_sg;
+		cam->frame_complete = mcam_dma_sg_done;
+#endif
 		break;
 	case B_vmalloc:
+#ifdef MCAM_MODE_VMALLOC
+		tasklet_init(&cam->s_tasklet, mcam_frame_tasklet,
+				(unsigned long) cam);
 		vq->ops = &mcam_vb2_ops;
 		vq->mem_ops = &vb2_vmalloc_memops;
 		vq->buf_struct_size = sizeof(struct mcam_vb_buffer);
 		vq->io_modes = VB2_MMAP;
+		cam->dma_setup = mcam_ctlr_dma_vmalloc;
+		cam->frame_complete = mcam_vmalloc_done;
+#endif
 		break;
 	}
 	return vb2_queue_init(vq);
@@ -1055,8 +1119,10 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
 static void mcam_cleanup_vb2(struct mcam_camera *cam)
 {
 	vb2_queue_release(&cam->vb_queue);
+#ifdef MCAM_MODE_DMA_CONTIG
 	if (cam->buffer_mode == B_DMA_contig)
 		vb2_dma_contig_cleanup_ctx(cam->vb_alloc_ctx);
+#endif
 }
 
 
@@ -1258,15 +1324,10 @@ static int mcam_vidioc_s_fmt_vid_cap(struct file *filp, void *priv,
 	/*
 	 * Make sure we have appropriate DMA buffers.
 	 */
-	ret = -ENOMEM;
 	if (cam->buffer_mode == B_vmalloc) {
-		if (cam->nbufs > 0 &&
-				cam->dma_buf_size < cam->pix_format.sizeimage)
-			mcam_free_dma_bufs(cam);
-		if (cam->nbufs == 0) {
-			if (mcam_alloc_dma_bufs(cam, 0))
-				goto out;
-		}
+		ret = mcam_check_dma_buffers(cam);
+		if (ret)
+			goto out;
 	}
 	mcam_set_config_needed(cam, 1);
 	ret = 0;
@@ -1587,17 +1648,7 @@ static void mcam_frame_complete(struct mcam_camera *cam, int frame)
 	/*
 	 * Process the frame and set up the next one.
 	 */
-	switch (cam->buffer_mode) {
-	case B_vmalloc:
-	    tasklet_schedule(&cam->s_tasklet);
-	    break;
-	case B_DMA_contig:
-	    mcam_dma_contig_done(cam, frame);
-	    break;
-	case B_DMA_sg:
-	    mcam_dma_sg_done(cam, frame);
-	    break;
-	}
+	cam->frame_complete(cam, frame);
 }
 
 
@@ -1663,6 +1714,22 @@ int mccic_register(struct mcam_camera *cam)
 	int ret;
 
 	/*
+	 * Validate the requested buffer mode.
+	 */
+	if (buffer_mode >= 0)
+		cam->buffer_mode = buffer_mode;
+	if (cam->buffer_mode == B_DMA_sg &&
+			cam->chip_id == V4L2_IDENT_CAFE) {
+		printk(KERN_ERR "marvell-cam: Cafe can't do S/G I/O, "
+			"attempting vmalloc mode instead\n");
+		cam->buffer_mode = B_vmalloc;
+	}
+	if (!mcam_buffer_mode_supported(cam->buffer_mode)) {
+		printk(KERN_ERR "marvell-cam: buffer mode %d unsupported\n",
+				cam->buffer_mode);
+		return -EINVAL;
+	}
+	/*
 	 * Register with V4L
 	 */
 	ret = v4l2_device_register(cam->dev, &cam->v4l2_dev);
@@ -1676,26 +1743,6 @@ int mccic_register(struct mcam_camera *cam)
 	cam->mbus_code = mcam_def_mbus_code;
 	INIT_LIST_HEAD(&cam->dev_list);
 	INIT_LIST_HEAD(&cam->buffers);
-	tasklet_init(&cam->s_tasklet, mcam_frame_tasklet, (unsigned long) cam);
-	/*
-	 * User space may want to override the asked-for buffer mode;
-	 * here's hoping they know what they're doing.
-	 */
-	if (buffer_mode == 0)
-		cam->buffer_mode = B_vmalloc;
-	else if (buffer_mode == 1)
-		cam->buffer_mode = B_DMA_contig;
-	else if (buffer_mode == 2) {
-		if (cam->chip_id == V4L2_IDENT_ARMADA610)
-			cam->buffer_mode = B_DMA_sg;
-		else {
-			printk(KERN_ERR "marvell-cam: Cafe can't do S/G I/O\n");
-			cam->buffer_mode = B_vmalloc;
-		}
-	} else if (buffer_mode != -1)
-		printk(KERN_ERR "marvell-cam: "
-				"Strange module buffer mode %d - ignoring\n",
-				buffer_mode);
 	mcam_ctlr_init(cam);
 
 	/*
diff --git a/drivers/media/video/marvell-ccic/mcam-core.h b/drivers/media/video/marvell-ccic/mcam-core.h
index 9a39e08..aa55255 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.h
+++ b/drivers/media/video/marvell-ccic/mcam-core.h
@@ -11,6 +11,27 @@
 #include <media/v4l2-dev.h>
 #include <media/videobuf2-core.h>
 
+/*
+ * Create our own symbols for the supported buffer modes, but, for now,
+ * base them entirely on which videobuf2 options have been selected.
+ */
+#if defined(CONFIG_VIDEOBUF2_VMALLOC) || defined(CONFIG_VIDEOBUF2_VMALLOC_MODULE)
+#define MCAM_MODE_VMALLOC 1
+#endif
+
+#if defined(CONFIG_VIDEOBUF2_DMA_CONTIG) || defined(CONFIG_VIDEOBUF2_DMA_CONTIG_MODULE)
+#define MCAM_MODE_DMA_CONTIG 1
+#endif
+
+#if defined(CONFIG_VIDEOBUF2_DMA_SG) || defined(CONFIG_VIDEOBUF2_DMA_SG_MODULE)
+#define MCAM_MODE_DMA_SG 1
+#endif
+
+#if !defined(MCAM_MODE_VMALLOC) && !defined(MCAM_MODE_DMA_CONTIG) && \
+	!defined(MCAM_MODE_DMA_SG)
+#error One of the videobuf buffer modes must be selected in the config
+#endif
+
 
 enum mcam_state {
 	S_NOTREADY,	/* Not yet initialized */
@@ -27,11 +48,33 @@ enum mcam_state {
  */
 enum mcam_buffer_mode {
 	B_vmalloc = 0,
-	B_DMA_contig,
-	B_DMA_sg
+	B_DMA_contig = 1,
+	B_DMA_sg = 2
 };
 
 /*
+ * Is a given buffer mode supported by the current kernel configuration?
+ */
+static inline int mcam_buffer_mode_supported(enum mcam_buffer_mode mode)
+{
+	switch (mode) {
+#ifdef MCAM_MODE_VMALLOC
+	case B_vmalloc:
+#endif
+#ifdef MCAM_MODE_DMA_CONTIG
+	case B_DMA_contig:
+#endif
+#ifdef MCAM_MODE_DMA_SG
+	case B_DMA_sg:
+#endif
+		return 1;
+	default:
+		return 0;
+	}
+}
+
+
+/*
  * A description of one of our devices.
  * Locking: controlled by s_mutex.  Certain fields, however, require
  *          the dev_lock spinlock; they are marked as such by comments.
@@ -79,21 +122,27 @@ struct mcam_camera {
 	struct vb2_queue vb_queue;
 	struct list_head buffers;	/* Available frames */
 
-	/* DMA buffers - vmalloc mode */
 	unsigned int nbufs;		/* How many are alloc'd */
 	int next_buf;			/* Next to consume (dev_lock) */
+
+	/* DMA buffers - vmalloc mode */
+#ifdef MCAM_MODE_VMALLOC
 	unsigned int dma_buf_size;	/* allocated size */
 	void *dma_bufs[MAX_DMA_BUFS];	/* Internal buffer addresses */
 	dma_addr_t dma_handles[MAX_DMA_BUFS]; /* Buffer bus addresses */
+	struct tasklet_struct s_tasklet;
+#endif
 	unsigned int sequence;		/* Frame sequence number */
 	unsigned int buf_seq[MAX_DMA_BUFS]; /* Sequence for individual bufs */
 
-	/* DMA buffers - contiguous DMA mode */
+	/* DMA buffers - DMA modes */
 	struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
 	struct vb2_alloc_ctx *vb_alloc_ctx;
 	unsigned short last_delivered;
 
-	struct tasklet_struct s_tasklet;
+	/* Mode-specific ops, set at open time */
+	void (*dma_setup)(struct mcam_camera *cam);
+	void (*frame_complete)(struct mcam_camera *cam, int frame);
 
 	/* Current operating parameters */
 	u32 sensor_type;		/* Currently ov7670 only */
-- 
1.7.6


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

* [PATCH 6/6] marvell-cam: clean up a couple of unused cam structure fields
  2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
                   ` (4 preceding siblings ...)
  2011-07-08 20:50 ` [PATCH 5/6] marvell-cam: Allow selection of supported buffer modes Jonathan Corbet
@ 2011-07-08 20:50 ` Jonathan Corbet
  5 siblings, 0 replies; 7+ messages in thread
From: Jonathan Corbet @ 2011-07-08 20:50 UTC (permalink / raw)
  To: linux-media
  Cc: Marek Szyprowski, Kassey Lee, Mauro Carvalho Chehab, Jonathan Corbet

Delete a couple of leftover fields whose time has passed.

Signed-off-by: Jonathan Corbet <corbet@lwn.net>
---
 drivers/media/video/marvell-ccic/mcam-core.c |    2 --
 drivers/media/video/marvell-ccic/mcam-core.h |    3 ---
 2 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c
index 073e72c..83c1451 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.c
+++ b/drivers/media/video/marvell-ccic/mcam-core.c
@@ -1638,7 +1638,6 @@ static void mcam_frame_complete(struct mcam_camera *cam, int frame)
 	clear_bit(CF_DMA_ACTIVE, &cam->flags);
 	cam->next_buf = frame;
 	cam->buf_seq[frame] = ++(cam->sequence);
-	cam->last_delivered = frame;
 	frames++;
 	/*
 	 * "This should never happen"
@@ -1741,7 +1740,6 @@ int mccic_register(struct mcam_camera *cam)
 	mcam_set_config_needed(cam, 1);
 	cam->pix_format = mcam_def_pix_format;
 	cam->mbus_code = mcam_def_mbus_code;
-	INIT_LIST_HEAD(&cam->dev_list);
 	INIT_LIST_HEAD(&cam->buffers);
 	mcam_ctlr_init(cam);
 
diff --git a/drivers/media/video/marvell-ccic/mcam-core.h b/drivers/media/video/marvell-ccic/mcam-core.h
index aa55255..917200e 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.h
+++ b/drivers/media/video/marvell-ccic/mcam-core.h
@@ -116,8 +116,6 @@ struct mcam_camera {
 	struct v4l2_subdev *sensor;
 	unsigned short sensor_addr;
 
-	struct list_head dev_list;	/* link to other devices */
-
 	/* Videobuf2 stuff */
 	struct vb2_queue vb_queue;
 	struct list_head buffers;	/* Available frames */
@@ -138,7 +136,6 @@ struct mcam_camera {
 	/* DMA buffers - DMA modes */
 	struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
 	struct vb2_alloc_ctx *vb_alloc_ctx;
-	unsigned short last_delivered;
 
 	/* Mode-specific ops, set at open time */
 	void (*dma_setup)(struct mcam_camera *cam);
-- 
1.7.6


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

end of thread, other threads:[~2011-07-08 20:52 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-08 20:50 [PATCH] One more Marvell cam patch series Jonathan Corbet
2011-07-08 20:50 ` [PATCH 1/6] marvell-cam: delete struct mcam_sio_buffer Jonathan Corbet
2011-07-08 20:50 ` [PATCH 2/6] marvell-cam: core code reorganization Jonathan Corbet
2011-07-08 20:50 ` [PATCH 3/6] marvell-cam: remove {min,max}_buffers parameters Jonathan Corbet
2011-07-08 20:50 ` [PATCH 4/6] marvell-cam: power down mmp camera on registration failure Jonathan Corbet
2011-07-08 20:50 ` [PATCH 5/6] marvell-cam: Allow selection of supported buffer modes Jonathan Corbet
2011-07-08 20:50 ` [PATCH 6/6] marvell-cam: clean up a couple of unused cam structure fields Jonathan Corbet

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