All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7 v5] new ioctl()s and soc-camera implementation
@ 2011-08-24 18:41 Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 1/7 v5] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED Guennadi Liakhovetski
                   ` (7 more replies)
  0 siblings, 8 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

This patch set is now trying to implement our most recent decisions. Any 
improvements should be doable as incremental patches. As long as we are 
happy with ioctl()s themselves, the rest can be improved. I'll quote my 
earlier email with our strategic design decisions:

1. VIDIOC_CREATE_BUFS passes struct v4l2_create_buffers from the user to 
   the kernel, in which struct v4l2_format is embedded. The user _must_ 
   fill in .type member of struct v4l2_format. For .type == 
   V4L2_BUF_TYPE_VIDEO_CAPTURE or V4L2_BUF_TYPE_VIDEO_OUTPUT .fmt.pix is 
   used, for .type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE or 
   V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE .fmt.pix_mp is used. In both these 
   cases the user _must_ fill in .width, .height, .pixelformat, .field, 
   .colorspace by possibly calling VIDIOC_G_FMT or VIDIOC_TRY_FMT. The 
   user also _may_ optionally fill in any further buffer-size related 
   fields, if it believes to have any special requirements to them. On 
   a successful return from the ioctl() .count and .index fields are 
   filled in by the kernel, .format stays unchanged. The user has to call 
   VIDIOC_QUERYBUF to retrieve specific buffer information.

2. Videobuf2 drivers, that implement .vidioc_create_bufs() operation, call 
   vb2_create_bufs() with a pointer to struct v4l2_create_buffers as a 
   second argument. vb2_create_bufs() in turn calls the .queue_setup() 
   driver callback, whose prototype is modified as follows:

int (*queue_setup)(struct vb2_queue *q, const struct v4l2_format *fmt,
                        unsigned int *num_buffers,
                        unsigned int *num_planes, unsigned int sizes[],
                        void *alloc_ctxs[]);

   with &create->format as a second argument. As pointed out above, this 
   struct is not modified by V4L, instead, the usual arguments 3-6 are 
   filled in by the driver, which are then used by vb2_create_bufs() to 
   call __vb2_queue_alloc().

3. vb2_reqbufs() shall call .queue_setup() with fmt == NULL, which will be 
   a signal to the driver to use the current format.

I also split out ioctl() documentation. I'm sure it's still not perfect 
and, being a favourite target for improvement suggestions in the past, I 
humbly propose to leave any comma fixes to incremental patches too. BTW, 
yes, I did try to replace various constants with '&...;' links, it didn't 
work for me.

Thanks
Guennadi

Guennadi Liakhovetski (7):
  V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED
  V4L: add two new ioctl()s for multi-size videobuffer management
  V4L: document the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
    ioctl()s
  V4L: vb2: prepare to support multi-size buffers
  V4L: vb2: add support for buffers of different sizes on a single
    queue
  V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers
  V4L: soc-camera: add 2 new ioctl() handlers

 Documentation/DocBook/media/v4l/io.xml             |   17 +
 Documentation/DocBook/media/v4l/v4l2.xml           |    2 +
 .../DocBook/media/v4l/vidioc-create-bufs.xml       |  147 +++++++++
 .../DocBook/media/v4l/vidioc-prepare-buf.xml       |   96 ++++++
 drivers/media/video/atmel-isi.c                    |    6 +-
 drivers/media/video/marvell-ccic/mcam-core.c       |    3 +-
 drivers/media/video/mem2mem_testdev.c              |    7 +-
 drivers/media/video/mx3_camera.c                   |    1 +
 drivers/media/video/pwc/pwc-if.c                   |    6 +-
 drivers/media/video/s5p-fimc/fimc-capture.c        |    6 +-
 drivers/media/video/s5p-fimc/fimc-core.c           |    6 +-
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c          |    7 +-
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c          |    5 +-
 drivers/media/video/s5p-tv/mixer_video.c           |    4 +-
 drivers/media/video/sh_mobile_ceu_camera.c         |  122 +++++---
 drivers/media/video/soc_camera.c                   |   33 ++-
 drivers/media/video/v4l2-compat-ioctl32.c          |   67 ++++-
 drivers/media/video/v4l2-ioctl.c                   |   29 ++
 drivers/media/video/videobuf2-core.c               |  341 ++++++++++++++++----
 drivers/media/video/vivi.c                         |    6 +-
 include/linux/videodev2.h                          |   15 +
 include/media/v4l2-ioctl.h                         |    2 +
 include/media/videobuf2-core.h                     |   39 ++-
 23 files changed, 804 insertions(+), 163 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml

-- 
1.7.2.5


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

* [PATCH 1/7 v5] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 2/7 v5] V4L: add two new ioctl()s for multi-size videobuffer management Guennadi Liakhovetski
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

This patch prepares for a better separation of the buffer preparation
stage.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/videobuf2-core.c |   59 +++++++++++++++++++++------------
 include/media/videobuf2-core.h       |    2 +
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 3015e60..fb7a3ac 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -333,6 +333,7 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 		b->flags |= V4L2_BUF_FLAG_DONE;
 		break;
 	case VB2_BUF_STATE_DEQUEUED:
+	case VB2_BUF_STATE_PREPARED:
 		/* nothing */
 		break;
 	}
@@ -817,6 +818,31 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 	q->ops->buf_queue(vb);
 }
 
+static int __buf_prepare(struct vb2_buffer *vb, struct v4l2_buffer *b)
+{
+	struct vb2_queue *q = vb->vb2_queue;
+	int ret;
+
+	switch (q->memory) {
+	case V4L2_MEMORY_MMAP:
+		ret = __qbuf_mmap(vb, b);
+		break;
+	case V4L2_MEMORY_USERPTR:
+		ret = __qbuf_userptr(vb, b);
+		break;
+	default:
+		WARN(1, "Invalid queue type\n");
+		ret = -EINVAL;
+	}
+
+	if (!ret)
+		ret = call_qop(q, buf_prepare, vb);
+	if (ret)
+		dprintk(1, "qbuf: buffer preparation failed: %d\n", ret);
+
+	return ret;
+}
+
 /**
  * vb2_qbuf() - Queue a buffer from userspace
  * @q:		videobuf2 queue
@@ -826,8 +852,8 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
  * Should be called from vidioc_qbuf ioctl handler of a driver.
  * This function:
  * 1) verifies the passed buffer,
- * 2) calls buf_prepare callback in the driver (if provided), in which
- *    driver-specific buffer initialization can be performed,
+ * 2) if necessary, calls buf_prepare callback in the driver (if provided), in
+ *    which driver-specific buffer initialization can be performed,
  * 3) if streaming is on, queues the buffer in driver by the means of buf_queue
  *    callback for processing.
  *
@@ -837,7 +863,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 {
 	struct vb2_buffer *vb;
-	int ret = 0;
+	int ret;
 
 	if (q->fileio) {
 		dprintk(1, "qbuf: file io in progress\n");
@@ -866,29 +892,18 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
 		return -EINVAL;
 	}
 
-	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
+	switch (vb->state) {
+	case VB2_BUF_STATE_DEQUEUED:
+		ret = __buf_prepare(vb, b);
+		if (ret)
+			return ret;
+	case VB2_BUF_STATE_PREPARED:
+		break;
+	default:
 		dprintk(1, "qbuf: buffer already in use\n");
 		return -EINVAL;
 	}
 
-	if (q->memory == V4L2_MEMORY_MMAP)
-		ret = __qbuf_mmap(vb, b);
-	else if (q->memory == V4L2_MEMORY_USERPTR)
-		ret = __qbuf_userptr(vb, b);
-	else {
-		WARN(1, "Invalid queue type\n");
-		return -EINVAL;
-	}
-
-	if (ret)
-		return ret;
-
-	ret = call_qop(q, buf_prepare, vb);
-	if (ret) {
-		dprintk(1, "qbuf: buffer preparation failed\n");
-		return ret;
-	}
-
 	/*
 	 * Add to the queued buffers list, a buffer will stay on it until
 	 * dequeued in dqbuf.
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f87472a..65946c5 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -106,6 +106,7 @@ enum vb2_fileio_flags {
 /**
  * enum vb2_buffer_state - current video buffer state
  * @VB2_BUF_STATE_DEQUEUED:	buffer under userspace control
+ * @VB2_BUF_STATE_PREPARED:	buffer prepared in videobuf and by the driver
  * @VB2_BUF_STATE_QUEUED:	buffer queued in videobuf, but not in driver
  * @VB2_BUF_STATE_ACTIVE:	buffer queued in driver and possibly used
  *				in a hardware operation
@@ -117,6 +118,7 @@ enum vb2_fileio_flags {
  */
 enum vb2_buffer_state {
 	VB2_BUF_STATE_DEQUEUED,
+	VB2_BUF_STATE_PREPARED,
 	VB2_BUF_STATE_QUEUED,
 	VB2_BUF_STATE_ACTIVE,
 	VB2_BUF_STATE_DONE,
-- 
1.7.2.5


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

* [PATCH 2/7 v5] V4L: add two new ioctl()s for multi-size videobuffer management
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 1/7 v5] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 3/7 v5] V4L: document the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s Guennadi Liakhovetski
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

A possibility to preallocate and initialise buffers of different sizes
in V4L2 is required for an efficient implementation of a snapshot
mode. This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and
VIDIOC_PREPARE_BUF and defines respective data structures.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/v4l2-compat-ioctl32.c |   67 +++++++++++++++++++++++++---
 drivers/media/video/v4l2-ioctl.c          |   29 ++++++++++++
 include/linux/videodev2.h                 |   15 ++++++
 include/media/v4l2-ioctl.h                |    2 +
 4 files changed, 105 insertions(+), 8 deletions(-)

diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index 61979b7..85758d2 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -159,11 +159,16 @@ struct v4l2_format32 {
 	} fmt;
 };
 
-static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up)
+struct v4l2_create_buffers32 {
+	__u32			index;		/* output: buffers index...index + count - 1 have been created */
+	__u32			count;
+	enum v4l2_memory        memory;
+	struct v4l2_format32	format;		/* filled in by the user, plane sizes calculated by the driver */
+	__u32			reserved[8];
+};
+
+static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up)
 {
-	if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) ||
-			get_user(kp->type, &up->type))
-			return -EFAULT;
 	switch (kp->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
@@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user
 	}
 }
 
-static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up)
+static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up)
+{
+	if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) ||
+			get_user(kp->type, &up->type))
+			return -EFAULT;
+	return __get_v4l2_format32(kp, up);
+}
+
+static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up)
+{
+	if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) ||
+	    copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt)))
+			return -EFAULT;
+	return __get_v4l2_format32(&kp->format, &up->format);
+}
+
+static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up)
 {
-	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) ||
-		put_user(kp->type, &up->type))
-		return -EFAULT;
 	switch (kp->type) {
 	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
 	case V4L2_BUF_TYPE_VIDEO_OUTPUT:
@@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user
 	}
 }
 
+static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up)
+{
+	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) ||
+		put_user(kp->type, &up->type))
+		return -EFAULT;
+	return __put_v4l2_format32(kp, up);
+}
+
+static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up)
+{
+	if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) ||
+	    copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt)))
+			return -EFAULT;
+	return __put_v4l2_format32(&kp->format, &up->format);
+}
+
 struct v4l2_standard32 {
 	__u32		     index;
 	__u32		     id[2]; /* __u64 would get the alignment wrong */
@@ -702,6 +736,8 @@ static int put_v4l2_event32(struct v4l2_event *kp, struct v4l2_event32 __user *u
 #define VIDIOC_S_EXT_CTRLS32    _IOWR('V', 72, struct v4l2_ext_controls32)
 #define VIDIOC_TRY_EXT_CTRLS32  _IOWR('V', 73, struct v4l2_ext_controls32)
 #define	VIDIOC_DQEVENT32	_IOR ('V', 89, struct v4l2_event32)
+#define VIDIOC_CREATE_BUFS32	_IOWR('V', 92, struct v4l2_create_buffers32)
+#define VIDIOC_PREPARE_BUF32	_IOW ('V', 93, struct v4l2_buffer32)
 
 #define VIDIOC_OVERLAY32	_IOW ('V', 14, s32)
 #define VIDIOC_STREAMON32	_IOW ('V', 18, s32)
@@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 		struct v4l2_standard v2s;
 		struct v4l2_ext_controls v2ecs;
 		struct v4l2_event v2ev;
+		struct v4l2_create_buffers v2crt;
 		unsigned long vx;
 		int vi;
 	} karg;
@@ -751,6 +788,8 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 	case VIDIOC_S_INPUT32: cmd = VIDIOC_S_INPUT; break;
 	case VIDIOC_G_OUTPUT32: cmd = VIDIOC_G_OUTPUT; break;
 	case VIDIOC_S_OUTPUT32: cmd = VIDIOC_S_OUTPUT; break;
+	case VIDIOC_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break;
+	case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break;
 	}
 
 	switch (cmd) {
@@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 		compatible_arg = 0;
 		break;
 
+	case VIDIOC_CREATE_BUFS:
+		err = get_v4l2_create32(&karg.v2crt, up);
+		compatible_arg = 0;
+		break;
+
+	case VIDIOC_PREPARE_BUF:
 	case VIDIOC_QUERYBUF:
 	case VIDIOC_QBUF:
 	case VIDIOC_DQBUF:
@@ -860,6 +905,10 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar
 		err = put_v4l2_format32(&karg.v2f, up);
 		break;
 
+	case VIDIOC_CREATE_BUFS:
+		err = put_v4l2_create32(&karg.v2crt, up);
+		break;
+
 	case VIDIOC_QUERYBUF:
 	case VIDIOC_QBUF:
 	case VIDIOC_DQBUF:
@@ -959,6 +1008,8 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	case VIDIOC_DQEVENT32:
 	case VIDIOC_SUBSCRIBE_EVENT:
 	case VIDIOC_UNSUBSCRIBE_EVENT:
+	case VIDIOC_CREATE_BUFS32:
+	case VIDIOC_PREPARE_BUF32:
 		ret = do_video_ioctl(file, cmd, arg);
 		break;
 
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 002ce13..6fa8e04 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -260,6 +260,8 @@ static const char *v4l2_ioctls[] = {
 	[_IOC_NR(VIDIOC_DQEVENT)]	   = "VIDIOC_DQEVENT",
 	[_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)]  = "VIDIOC_SUBSCRIBE_EVENT",
 	[_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT",
+	[_IOC_NR(VIDIOC_CREATE_BUFS)]      = "VIDIOC_CREATE_BUFS",
+	[_IOC_NR(VIDIOC_PREPARE_BUF)]      = "VIDIOC_PREPARE_BUF",
 };
 #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls)
 
@@ -2216,6 +2218,33 @@ static long __video_do_ioctl(struct file *file,
 		dbgarg(cmd, "type=0x%8.8x", sub->type);
 		break;
 	}
+	case VIDIOC_CREATE_BUFS:
+	{
+		struct v4l2_create_buffers *create = arg;
+
+		if (!ops->vidioc_create_bufs)
+			break;
+		ret = check_fmt(ops, create->format.type);
+		if (ret)
+			break;
+
+		ret = ops->vidioc_create_bufs(file, fh, create);
+
+		dbgarg(cmd, "count=%d @ %d\n", create->count, create->index);
+		break;
+	}
+	case VIDIOC_PREPARE_BUF:
+	{
+		struct v4l2_buffer *b = arg;
+
+		if (!ops->vidioc_prepare_buf)
+			break;
+
+		ret = ops->vidioc_prepare_buf(file, fh, b);
+
+		dbgarg(cmd, "index=%d", b->index);
+		break;
+	}
 	default:
 	{
 		bool valid_prio = true;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index fca24cc..988e1be 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -653,6 +653,9 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_ERROR	0x0040
 #define V4L2_BUF_FLAG_TIMECODE	0x0100	/* timecode field is valid */
 #define V4L2_BUF_FLAG_INPUT     0x0200  /* input field is valid */
+/* Cache handling flags */
+#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0400
+#define V4L2_BUF_FLAG_NO_CACHE_CLEAN		0x0800
 
 /*
  *	O V E R L A Y   P R E V I E W
@@ -2092,6 +2095,15 @@ struct v4l2_dbg_chip_ident {
 	__u32 revision;    /* chip revision, chip specific */
 } __attribute__ ((packed));
 
+/* VIDIOC_CREATE_BUFS */
+struct v4l2_create_buffers {
+	__u32			index;		/* output: buffers index...index + count - 1 have been created */
+	__u32			count;
+	enum v4l2_memory        memory;
+	struct v4l2_format	format;		/* "type" is used always, the rest if sizeimage == 0 */
+	__u32			reserved[8];
+};
+
 /*
  *	I O C T L   C O D E S   F O R   V I D E O   D E V I C E S
  *
@@ -2182,6 +2194,9 @@ struct v4l2_dbg_chip_ident {
 #define	VIDIOC_SUBSCRIBE_EVENT	 _IOW('V', 90, struct v4l2_event_subscription)
 #define	VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct v4l2_event_subscription)
 
+#define VIDIOC_CREATE_BUFS	_IOWR('V', 92, struct v4l2_create_buffers)
+#define VIDIOC_PREPARE_BUF	 _IOW('V', 93, struct v4l2_buffer)
+
 /* Reminder: when adding new ioctls please add support for them to
    drivers/media/video/v4l2-compat-ioctl32.c as well! */
 
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index dd9f1e7..55cf8ae 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -122,6 +122,8 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
 	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
 
+	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
+	int (*vidioc_prepare_buf)(struct file *file, void *fh, const struct v4l2_buffer *b);
 
 	int (*vidioc_overlay) (struct file *file, void *fh, unsigned int i);
 	int (*vidioc_g_fbuf)   (struct file *file, void *fh,
-- 
1.7.2.5


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

* [PATCH 3/7 v5] V4L: document the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 1/7 v5] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 2/7 v5] V4L: add two new ioctl()s for multi-size videobuffer management Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 4/7 v5] V4L: vb2: prepare to support multi-size buffers Guennadi Liakhovetski
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 Documentation/DocBook/media/v4l/io.xml             |   17 +++
 Documentation/DocBook/media/v4l/v4l2.xml           |    2 +
 .../DocBook/media/v4l/vidioc-create-bufs.xml       |  147 ++++++++++++++++++++
 .../DocBook/media/v4l/vidioc-prepare-buf.xml       |   96 +++++++++++++
 4 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml

diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index 227e7ac..ff03dd2 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -927,6 +927,23 @@ ioctl is called.</entry>
 Applications set or clear this flag before calling the
 <constant>VIDIOC_QBUF</constant> ioctl.</entry>
 	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry>
+	    <entry>0x0400</entry>
+	    <entry>Caches do not have to be invalidated for this buffer.
+Typically applications shall use this flag if the data captured in the buffer
+is not going to be touched by the CPU, instead the buffer will, probably, be
+passed on to a DMA-capable hardware unit for further processing or output.
+</entry>
+	  </row>
+	  <row>
+	    <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry>
+	    <entry>0x0800</entry>
+	    <entry>Caches do not have to be cleaned for this buffer.
+Typically applications shall use this flag for output buffers if the data
+in this buffer has not been created by the CPU but by some DMA-capable unit,
+in which case caches have not been used.</entry>
+	  </row>
 	</tbody>
       </tgroup>
     </table>
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 0d05e87..06bb179 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -462,6 +462,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-close;
     &sub-ioctl;
     <!-- All ioctls go here. -->
+    &sub-create-bufs;
     &sub-cropcap;
     &sub-dbg-g-chip-ident;
     &sub-dbg-g-register;
@@ -504,6 +505,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-queryctrl;
     &sub-query-dv-preset;
     &sub-querystd;
+    &sub-prepare-buf;
     &sub-reqbufs;
     &sub-s-hw-freq-seek;
     &sub-streamon;
diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
new file mode 100644
index 0000000..eb99604
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml
@@ -0,0 +1,147 @@
+<refentry id="vidioc-create-bufs">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_CREATE_BUFS</refname>
+    <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_CREATE_BUFS</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>This ioctl is used to create buffers for <link linkend="mmap">memory
+mapped</link> or <link linkend="userp">user pointer</link>
+I/O. It can be used as an alternative or in addition to the
+<constant>VIDIOC_REQBUFS</constant> ioctl, when a tighter control over buffers
+is required. This ioctl can be called multiple times to create buffers of
+different sizes.</para>
+
+    <para>To allocate device buffers applications initialize relevant fields of
+the <structname>v4l2_create_buffers</structname> structure. They set the
+<structfield>type</structfield> field in the
+<structname>v4l2_format</structname> structure, embedded in this
+structure, to the respective stream or buffer type.
+<structfield>count</structfield> must be set to the number of required buffers.
+<structfield>memory</structfield> specifies the required I/O method. The
+<structfield>format</structfield> field shall typically be filled in using
+either the <constant>VIDIOC_TRY_FMT</constant> or
+<constant>VIDIOC_G_FMT</constant> ioctl(). Additionally, applications can adjust
+<structfield>sizeimage</structfield> fields to fit their specific needs. The
+<structfield>reserved</structfield> array must be zeroed.</para>
+
+    <para>When the ioctl is called with a pointer to this structure the driver
+will attempt to allocate up to the requested number of buffers and store the
+actual number allocated and the starting index in the
+<structfield>count</structfield> and the <structfield>index</structfield> fields
+respectively. On return <structfield>count</structfield> can be smaller than
+the number requested. The driver may also adjust buffer sizes as it sees fit,
+however, it will not update <structfield>sizeimage</structfield> fields, the
+user has to use <constant>VIDIOC_QUERYBUF</constant> to retrieve that
+information.</para>
+
+    <table pgwide="1" frame="none" id="v4l2-create-buffers">
+      <title>struct <structname>v4l2_create_buffers</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>index</structfield></entry>
+	    <entry>The starting buffer index, returned by the driver.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>count</structfield></entry>
+	    <entry>The number of buffers requested or granted.</entry>
+	  </row>
+	  <row>
+	    <entry>&v4l2-memory;</entry>
+	    <entry><structfield>memory</structfield></entry>
+	    <entry>Applications set this field to
+<constant>V4L2_MEMORY_MMAP</constant> or
+<constant>V4L2_MEMORY_USERPTR</constant>.</entry>
+	  </row>
+	  <row>
+	    <entry>&v4l2-format;</entry>
+	    <entry><structfield>format</structfield></entry>
+	    <entry>Filled in by the application, preserved by the driver.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved</structfield>[8]</entry>
+	    <entry>A place holder for future extensions.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>ENOMEM</errorcode></term>
+	<listitem>
+	  <para>No memory to allocate buffers for <link linkend="mmap">memory
+mapped</link> I/O.</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><errorcode>EINVAL</errorcode></term>
+	<listitem>
+	  <para>The buffer type (<structfield>type</structfield> field) or the
+requested I/O method (<structfield>memory</structfield>) is not
+supported.</para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+</refentry>
+
+<!--
+Local Variables:
+mode: sgml
+sgml-parent-document: "v4l2.sgml"
+indent-tabs-mode: nil
+End:
+-->
diff --git a/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml b/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml
new file mode 100644
index 0000000..509e752
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml
@@ -0,0 +1,96 @@
+<refentry id="vidioc-prepare-buf">
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_PREPARE_BUF</refname>
+    <refpurpose>Prepare a buffer for I/O</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_PREPARE_BUF</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <para>Applications can optionally call the
+<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer
+to the driver before actually enqueuing it, using the
+<constant>VIDIOC_QBUF</constant> ioctl, and to prepare it for future I/O.
+Such preparations may include cache invalidation or cleaning. Performing them
+in advance saves time during the actual I/O. In case such cache operations are
+not required, the application can use one of
+<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and
+<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective
+step.</para>
+
+    <para>The <structname>v4l2_buffer</structname> structure is
+specified in <xref linkend="buffer" />.</para>
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>EBUSY</errorcode></term>
+	<listitem>
+	  <para>File I/O is in progress.</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><errorcode>EINVAL</errorcode></term>
+	<listitem>
+	  <para>The buffer <structfield>type</structfield> is not
+supported, or the <structfield>index</structfield> is out of bounds,
+or no buffers have been allocated yet, or the
+<structfield>userptr</structfield> or
+<structfield>length</structfield> are invalid.</para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+</refentry>
+
+<!--
+Local Variables:
+mode: sgml
+sgml-parent-document: "v4l2.sgml"
+indent-tabs-mode: nil
+End:
+-->
-- 
1.7.2.5


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

* [PATCH 4/7 v5] V4L: vb2: prepare to support multi-size buffers
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
                   ` (2 preceding siblings ...)
  2011-08-24 18:41 ` [PATCH 3/7 v5] V4L: document the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue Guennadi Liakhovetski
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

In preparation for the forthcoming VIDIOC_CREATE_BUFS ioctl add a
"const struct v4l2_format *" argument to the .queue_setup() vb2
operation.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/atmel-isi.c              |    6 +++---
 drivers/media/video/marvell-ccic/mcam-core.c |    3 ++-
 drivers/media/video/mem2mem_testdev.c        |    7 ++++---
 drivers/media/video/mx3_camera.c             |    1 +
 drivers/media/video/pwc/pwc-if.c             |    6 +++---
 drivers/media/video/s5p-fimc/fimc-capture.c  |    6 +++---
 drivers/media/video/s5p-fimc/fimc-core.c     |    6 +++---
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c    |    7 ++++---
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c    |    5 +++--
 drivers/media/video/s5p-tv/mixer_video.c     |    4 ++--
 drivers/media/video/sh_mobile_ceu_camera.c   |    1 +
 drivers/media/video/videobuf2-core.c         |    6 +++---
 drivers/media/video/vivi.c                   |    6 +++---
 include/media/videobuf2-core.h               |    6 +++---
 14 files changed, 38 insertions(+), 32 deletions(-)

diff --git a/drivers/media/video/atmel-isi.c b/drivers/media/video/atmel-isi.c
index 3e3d4cc..7c41a87 100644
--- a/drivers/media/video/atmel-isi.c
+++ b/drivers/media/video/atmel-isi.c
@@ -249,9 +249,9 @@ static int atmel_isi_wait_status(struct atmel_isi *isi, int wait_reset)
 /* ------------------------------------------------------------------
 	Videobuf operations
    ------------------------------------------------------------------*/
-static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
-				unsigned int *nplanes, unsigned long sizes[],
-				void *alloc_ctxs[])
+static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+				unsigned int *nbuffers, unsigned int *nplanes,
+				unsigned long sizes[], void *alloc_ctxs[])
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
diff --git a/drivers/media/video/marvell-ccic/mcam-core.c b/drivers/media/video/marvell-ccic/mcam-core.c
index 83c1451..65517c8 100644
--- a/drivers/media/video/marvell-ccic/mcam-core.c
+++ b/drivers/media/video/marvell-ccic/mcam-core.c
@@ -883,7 +883,8 @@ static int mcam_read_setup(struct mcam_camera *cam)
  * Videobuf2 interface code.
  */
 
-static int mcam_vb_queue_setup(struct vb2_queue *vq, unsigned int *nbufs,
+static int mcam_vb_queue_setup(struct vb2_queue *vq,
+		const struct v4l2_format *fmt, unsigned int *nbufs,
 		unsigned int *num_planes, unsigned long sizes[],
 		void *alloc_ctxs[])
 {
diff --git a/drivers/media/video/mem2mem_testdev.c b/drivers/media/video/mem2mem_testdev.c
index 166bf93..c0e633f 100644
--- a/drivers/media/video/mem2mem_testdev.c
+++ b/drivers/media/video/mem2mem_testdev.c
@@ -738,9 +738,10 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = {
  * Queue operations
  */
 
-static int m2mtest_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
-				unsigned int *nplanes, unsigned long sizes[],
-				void *alloc_ctxs[])
+static int m2mtest_queue_setup(struct vb2_queue *vq,
+				const struct v4l2_format *fmt,
+				unsigned int *nbuffers, unsigned int *nplanes,
+				unsigned long sizes[], void *alloc_ctxs[])
 {
 	struct m2mtest_ctx *ctx = vb2_get_drv_priv(vq);
 	struct m2mtest_q_data *q_data;
diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index 3f37522f..6bfbce9 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -191,6 +191,7 @@ static void mx3_cam_dma_done(void *arg)
  * Calculate the __buffer__ (not data) size and number of buffers.
  */
 static int mx3_videobuf_setup(struct vb2_queue *vq,
+			const struct v4l2_format *fmt,
 			unsigned int *count, unsigned int *num_planes,
 			unsigned long sizes[], void *alloc_ctxs[])
 {
diff --git a/drivers/media/video/pwc/pwc-if.c b/drivers/media/video/pwc/pwc-if.c
index 51ca358..d6ff2c9 100644
--- a/drivers/media/video/pwc/pwc-if.c
+++ b/drivers/media/video/pwc/pwc-if.c
@@ -744,9 +744,9 @@ static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma)
 /***************************************************************************/
 /* Videobuf2 operations */
 
-static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
-				unsigned int *nplanes, unsigned long sizes[],
-				void *alloc_ctxs[])
+static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+				unsigned int *nbuffers, unsigned int *nplanes,
+				unsigned long sizes[], void *alloc_ctxs[])
 {
 	struct pwc_device *pdev = vb2_get_drv_priv(vq);
 
diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c
index 0d730e5..36d71a1 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -264,9 +264,9 @@ static unsigned int get_plane_size(struct fimc_frame *fr, unsigned int plane)
 	return fr->f_width * fr->f_height * fr->fmt->depth[plane] / 8;
 }
 
-static int queue_setup(struct vb2_queue *vq, unsigned int *num_buffers,
-		       unsigned int *num_planes, unsigned long sizes[],
-		       void *allocators[])
+static int queue_setup(struct vb2_queue *vq,  const struct v4l2_format *pfmt,
+		       unsigned int *num_buffers, unsigned int *num_planes,
+		       unsigned long sizes[], void *allocators[])
 {
 	struct fimc_ctx *ctx = vq->drv_priv;
 	struct fimc_fmt *fmt = ctx->d_frame.fmt;
diff --git a/drivers/media/video/s5p-fimc/fimc-core.c b/drivers/media/video/s5p-fimc/fimc-core.c
index aa55066..26348e2 100644
--- a/drivers/media/video/s5p-fimc/fimc-core.c
+++ b/drivers/media/video/s5p-fimc/fimc-core.c
@@ -691,9 +691,9 @@ static void fimc_job_abort(void *priv)
 	fimc_m2m_shutdown(priv);
 }
 
-static int fimc_queue_setup(struct vb2_queue *vq, unsigned int *num_buffers,
-			    unsigned int *num_planes, unsigned long sizes[],
-			    void *allocators[])
+static int fimc_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+			    unsigned int *num_buffers, unsigned int *num_planes,
+			    unsigned long sizes[], void *allocators[])
 {
 	struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
 	struct fimc_frame *f;
diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
index b2c5052..06f317c 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
@@ -744,9 +744,10 @@ static const struct v4l2_ioctl_ops s5p_mfc_dec_ioctl_ops = {
 	.vidioc_g_crop = vidioc_g_crop,
 };
 
-static int s5p_mfc_queue_setup(struct vb2_queue *vq, unsigned int *buf_count,
-			       unsigned int *plane_count, unsigned long psize[],
-			       void *allocators[])
+static int s5p_mfc_queue_setup(struct vb2_queue *vq,
+			const struct v4l2_format *fmt, unsigned int *buf_count,
+			unsigned int *plane_count, unsigned long psize[],
+			void *allocators[])
 {
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(vq->drv_priv);
 
diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
index fee094a..0c1a22f 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
@@ -1513,8 +1513,9 @@ static int check_vb_with_fmt(struct s5p_mfc_fmt *fmt, struct vb2_buffer *vb)
 }
 
 static int s5p_mfc_queue_setup(struct vb2_queue *vq,
-		       unsigned int *buf_count, unsigned int *plane_count,
-		       unsigned long psize[], void *allocators[])
+			const struct v4l2_format *fmt,
+			unsigned int *buf_count, unsigned int *plane_count,
+			unsigned long psize[], void *allocators[])
 {
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(vq->drv_priv);
 
diff --git a/drivers/media/video/s5p-tv/mixer_video.c b/drivers/media/video/s5p-tv/mixer_video.c
index 43ac22f..74b86aa 100644
--- a/drivers/media/video/s5p-tv/mixer_video.c
+++ b/drivers/media/video/s5p-tv/mixer_video.c
@@ -727,8 +727,8 @@ static const struct v4l2_file_operations mxr_fops = {
 	.unlocked_ioctl = video_ioctl2,
 };
 
-static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
-	unsigned int *nplanes, unsigned long sizes[],
+static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *pfmt,
+	unsigned int *nbuffers, unsigned int *nplanes, unsigned long sizes[],
 	void *alloc_ctxs[])
 {
 	struct mxr_layer *layer = vb2_get_drv_priv(vq);
diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index 8b344d4..db3a24d 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -193,6 +193,7 @@ static int sh_mobile_ceu_soft_reset(struct sh_mobile_ceu_dev *pcdev)
  *  Videobuf operations
  */
 static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq,
+			const struct v4l2_format *fmt,
 			unsigned int *count, unsigned int *num_planes,
 			unsigned long sizes[], void *alloc_ctxs[])
 {
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index fb7a3ac..8a81a89 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -525,7 +525,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	 * Ask the driver how many buffers and planes per buffer it requires.
 	 * Driver also sets the size and allocator context for each plane.
 	 */
-	ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
+	ret = call_qop(q, queue_setup, q, NULL, &num_buffers, &num_planes,
 		       plane_sizes, q->alloc_ctx);
 	if (ret)
 		return ret;
@@ -545,8 +545,8 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 		unsigned int orig_num_buffers;
 
 		orig_num_buffers = num_buffers = ret;
-		ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
-			       plane_sizes, q->alloc_ctx);
+		ret = call_qop(q, queue_setup, q, NULL, &num_buffers,
+			       &num_planes, plane_sizes, q->alloc_ctx);
 		if (ret)
 			goto free_mem;
 
diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c
index a848bd2..0b24508 100644
--- a/drivers/media/video/vivi.c
+++ b/drivers/media/video/vivi.c
@@ -650,9 +650,9 @@ static void vivi_stop_generating(struct vivi_dev *dev)
 /* ------------------------------------------------------------------
 	Videobuf operations
    ------------------------------------------------------------------*/
-static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
-				unsigned int *nplanes, unsigned long sizes[],
-				void *alloc_ctxs[])
+static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+				unsigned int *nbuffers, unsigned int *nplanes,
+				unsigned long sizes[], void *alloc_ctxs[])
 {
 	struct vivi_dev *dev = vb2_get_drv_priv(vq);
 	unsigned long size;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 65946c5..d043132 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -212,9 +212,9 @@ struct vb2_buffer {
  *			the buffer back by calling vb2_buffer_done() function
  */
 struct vb2_ops {
-	int (*queue_setup)(struct vb2_queue *q, unsigned int *num_buffers,
-			   unsigned int *num_planes, unsigned long sizes[],
-			   void *alloc_ctxs[]);
+	int (*queue_setup)(struct vb2_queue *q, const struct v4l2_format *fmt,
+			   unsigned int *num_buffers, unsigned int *num_planes,
+			   unsigned long sizes[], void *alloc_ctxs[]);
 
 	void (*wait_prepare)(struct vb2_queue *q);
 	void (*wait_finish)(struct vb2_queue *q);
-- 
1.7.2.5


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

* [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
                   ` (3 preceding siblings ...)
  2011-08-24 18:41 ` [PATCH 4/7 v5] V4L: vb2: prepare to support multi-size buffers Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-08-28 19:29   ` Pawel Osciak
  2011-08-24 18:41 ` [PATCH 6/7 v5] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers Guennadi Liakhovetski
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
allow user-space applications to allocate video buffers of different
sizes and hand them over to the driver for fast switching between
different frame formats. This patch adds support for buffers of different
sizes on the same buffer-queue to vb2.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/videobuf2-core.c |  278 ++++++++++++++++++++++++++++------
 include/media/videobuf2-core.h       |   31 +++--
 2 files changed, 252 insertions(+), 57 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 8a81a89..fed6f2d 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -44,7 +44,7 @@ module_param(debug, int, 0644);
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
  */
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb,
-				unsigned long *plane_sizes)
+			       const unsigned long *plane_sizes)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	void *mem_priv;
@@ -110,13 +110,22 @@ static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
  * __setup_offsets() - setup unique offsets ("cookies") for every plane in
  * every buffer on the queue
  */
-static void __setup_offsets(struct vb2_queue *q)
+static void __setup_offsets(struct vb2_queue *q, unsigned int n)
 {
 	unsigned int buffer, plane;
 	struct vb2_buffer *vb;
-	unsigned long off = 0;
+	unsigned long off;
 
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+	if (q->num_buffers) {
+		struct v4l2_plane *p;
+		vb = q->bufs[q->num_buffers - 1];
+		p = &vb->v4l2_planes[vb->num_planes - 1];
+		off = PAGE_ALIGN(p->m.mem_offset + p->length);
+	} else {
+		off = 0;
+	}
+
+	for (buffer = q->num_buffers; buffer < q->num_buffers + n; ++buffer) {
 		vb = q->bufs[buffer];
 		if (!vb)
 			continue;
@@ -142,7 +151,7 @@ static void __setup_offsets(struct vb2_queue *q)
  */
 static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 			     unsigned int num_buffers, unsigned int num_planes,
-			     unsigned long plane_sizes[])
+			     const unsigned long *plane_sizes)
 {
 	unsigned int buffer;
 	struct vb2_buffer *vb;
@@ -163,7 +172,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 		vb->state = VB2_BUF_STATE_DEQUEUED;
 		vb->vb2_queue = q;
 		vb->num_planes = num_planes;
-		vb->v4l2_buf.index = buffer;
+		vb->v4l2_buf.index = q->num_buffers + buffer;
 		vb->v4l2_buf.type = q->type;
 		vb->v4l2_buf.memory = memory;
 
@@ -191,15 +200,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 			}
 		}
 
-		q->bufs[buffer] = vb;
+		q->bufs[q->num_buffers + buffer] = vb;
 	}
 
-	q->num_buffers = buffer;
-
-	__setup_offsets(q);
+	__setup_offsets(q, buffer);
 
 	dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
-			q->num_buffers, num_planes);
+			buffer, num_planes);
 
 	return buffer;
 }
@@ -207,12 +214,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 /**
  * __vb2_free_mem() - release all video buffer memory for a given queue
  */
-static void __vb2_free_mem(struct vb2_queue *q)
+static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers)
 {
 	unsigned int buffer;
 	struct vb2_buffer *vb;
 
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+	     ++buffer) {
 		vb = q->bufs[buffer];
 		if (!vb)
 			continue;
@@ -226,17 +234,18 @@ static void __vb2_free_mem(struct vb2_queue *q)
 }
 
 /**
- * __vb2_queue_free() - free the queue - video memory and related information
- * and return the queue to an uninitialized state. Might be called even if the
- * queue has already been freed.
+ * __vb2_queue_free() - free buffers at the end of the queue - video memory and
+ * related information, if no buffers are left return the queue to an
+ * uninitialized state. Might be called even if the queue has already been freed.
  */
-static void __vb2_queue_free(struct vb2_queue *q)
+static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 {
 	unsigned int buffer;
 
 	/* Call driver-provided cleanup function for each buffer, if provided */
 	if (q->ops->buf_cleanup) {
-		for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+		for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+		     ++buffer) {
 			if (NULL == q->bufs[buffer])
 				continue;
 			q->ops->buf_cleanup(q->bufs[buffer]);
@@ -244,23 +253,25 @@ static void __vb2_queue_free(struct vb2_queue *q)
 	}
 
 	/* Release video buffer memory */
-	__vb2_free_mem(q);
+	__vb2_free_mem(q, buffers);
 
 	/* Free videobuf buffers */
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+	     ++buffer) {
 		kfree(q->bufs[buffer]);
 		q->bufs[buffer] = NULL;
 	}
 
-	q->num_buffers = 0;
-	q->memory = 0;
+	q->num_buffers -= buffers;
+	if (!q->num_buffers)
+		q->memory = 0;
 }
 
 /**
  * __verify_planes_array() - verify that the planes array passed in struct
  * v4l2_buffer from userspace can be safely used
  */
-static int __verify_planes_array(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	/* Is memory for copying plane information present? */
 	if (NULL == b->m.planes) {
@@ -454,7 +465,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
  */
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 {
-	unsigned int num_buffers, num_planes;
+	unsigned int num_buffers, allocated_buffers, num_planes = 0;
 	unsigned long plane_sizes[VIDEO_MAX_PLANES];
 	int ret = 0;
 
@@ -503,7 +514,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 			return -EBUSY;
 		}
 
-		__vb2_queue_free(q);
+		__vb2_queue_free(q, q->num_buffers);
 
 		/*
 		 * In case of REQBUFS(0) return immediately without calling
@@ -538,44 +549,166 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 		return -ENOMEM;
 	}
 
+	allocated_buffers = ret;
+
 	/*
 	 * Check if driver can handle the allocated number of buffers.
 	 */
-	if (ret < num_buffers) {
-		unsigned int orig_num_buffers;
+	if (allocated_buffers < num_buffers) {
+		num_buffers = allocated_buffers;
 
-		orig_num_buffers = num_buffers = ret;
 		ret = call_qop(q, queue_setup, q, NULL, &num_buffers,
 			       &num_planes, plane_sizes, q->alloc_ctx);
-		if (ret)
-			goto free_mem;
 
-		if (orig_num_buffers < num_buffers) {
+		if (!ret && allocated_buffers < num_buffers)
 			ret = -ENOMEM;
-			goto free_mem;
-		}
 
 		/*
-		 * Ok, driver accepted smaller number of buffers.
+		 * Either the driver has accepted a smaller number of buffers,
+		 * or .queue_setup() returned an error
 		 */
-		ret = num_buffers;
+	}
+
+	q->num_buffers = allocated_buffers;
+
+	if (ret < 0) {
+		__vb2_queue_free(q, allocated_buffers);
+		return ret;
 	}
 
 	/*
 	 * Return the number of successfully allocated buffers
 	 * to the userspace.
 	 */
-	req->count = ret;
+	req->count = allocated_buffers;
 
 	return 0;
-
-free_mem:
-	__vb2_queue_free(q);
-	return ret;
 }
 EXPORT_SYMBOL_GPL(vb2_reqbufs);
 
 /**
+ * vb2_create_bufs() - Allocate buffers and any required auxiliary structs
+ * @q:		videobuf2 queue
+ * @create:	creation parameters, passed from userspace to vidioc_create_bufs
+ *		handler in driver
+ *
+ * Should be called from vidioc_create_bufs ioctl handler of a driver.
+ * This function:
+ * 1) verifies parameter sanity
+ * 2) calls the .queue_setup() queue operation
+ * 3) performs any necessary memory allocations
+ *
+ * The return values from this function are intended to be directly returned
+ * from vidioc_create_bufs handler in driver.
+ */
+int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
+{
+	unsigned int num_planes, num_buffers = create->count, allocated_buffers;
+	unsigned long plane_sizes[VIDEO_MAX_PLANES];
+	int ret = 0;
+
+	if (q->fileio) {
+		dprintk(1, "%s(): file io in progress\n", __func__);
+		return -EBUSY;
+	}
+
+	if (create->memory != V4L2_MEMORY_MMAP
+			&& create->memory != V4L2_MEMORY_USERPTR) {
+		dprintk(1, "%s(): unsupported memory type\n", __func__);
+		return -EINVAL;
+	}
+
+	if (create->format.type != q->type) {
+		dprintk(1, "%s(): requested type is incorrect\n", __func__);
+		return -EINVAL;
+	}
+
+	/*
+	 * Make sure all the required memory ops for given memory type
+	 * are available.
+	 */
+	if (create->memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
+		dprintk(1, "%s(): MMAP for current setup unsupported\n", __func__);
+		return -EINVAL;
+	}
+
+	if (create->memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
+		dprintk(1, "%s(): USERPTR for current setup unsupported\n", __func__);
+		return -EINVAL;
+	}
+
+	if (q->num_buffers == VIDEO_MAX_FRAME) {
+		dprintk(1, "%s(): maximum number of buffers already allocated\n",
+			__func__);
+		return -ENOBUFS;
+	}
+
+	create->index = q->num_buffers;
+
+	if (!q->num_buffers) {
+		memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
+		q->memory = create->memory;
+	}
+
+	/*
+	 * Ask the driver, whether the requested number of buffers, planes per
+	 * buffer and their sizes are acceptable
+	 */
+	ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
+		       &num_planes, plane_sizes, q->alloc_ctx);
+	if (ret)
+		return ret;
+
+	/* Finally, allocate buffers and video memory */
+	ret = __vb2_queue_alloc(q, create->memory, num_buffers,
+				num_planes, plane_sizes);
+	if (ret < 0) {
+		dprintk(1, "Memory allocation failed with error: %d\n", ret);
+		return ret;
+	}
+
+	allocated_buffers = ret;
+
+	/*
+	 * Check if driver can handle the so far allocated number of buffers.
+	 */
+	if (ret < num_buffers) {
+		num_buffers = ret;
+
+		/*
+		 * q->num_buffers contains the total number of buffers, that the
+		 * queue driver has set up
+		 */
+		ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
+			       &num_planes, plane_sizes, q->alloc_ctx);
+
+		if (!ret && allocated_buffers < num_buffers)
+			ret = -ENOMEM;
+
+		/*
+		 * Either the driver has accepted a smaller number of buffers,
+		 * or .queue_setup() returned an error
+		 */
+	}
+
+	q->num_buffers += allocated_buffers;
+
+	if (ret < 0) {
+		__vb2_queue_free(q, allocated_buffers);
+		return ret;
+	}
+
+	/*
+	 * Return the number of successfully allocated buffers
+	 * to the userspace.
+	 */
+	create->count = allocated_buffers;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_create_bufs);
+
+/**
  * vb2_plane_vaddr() - Return a kernel virtual address of a given plane
  * @vb:		vb2_buffer to which the plane in question belongs to
  * @plane_no:	plane number for which the address is to be returned
@@ -659,7 +792,7 @@ EXPORT_SYMBOL_GPL(vb2_buffer_done);
  * __fill_vb2_buffer() - fill a vb2_buffer with information provided in
  * a v4l2_buffer by the userspace
  */
-static int __fill_vb2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b,
+static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 				struct v4l2_plane *v4l2_planes)
 {
 	unsigned int plane;
@@ -723,7 +856,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b,
 /**
  * __qbuf_userptr() - handle qbuf of a USERPTR buffer
  */
-static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	struct v4l2_plane planes[VIDEO_MAX_PLANES];
 	struct vb2_queue *q = vb->vb2_queue;
@@ -801,7 +934,7 @@ err:
 /**
  * __qbuf_mmap() - handle qbuf of an MMAP buffer
  */
-static int __qbuf_mmap(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	return __fill_vb2_buffer(vb, b, vb->v4l2_planes);
 }
@@ -818,7 +951,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 	q->ops->buf_queue(vb);
 }
 
-static int __buf_prepare(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	int ret;
@@ -844,6 +977,61 @@ static int __buf_prepare(struct vb2_buffer *vb, struct v4l2_buffer *b)
 }
 
 /**
+ * vb2_prepare_buf() - Pass ownership of a buffer from userspace to the kernel
+ * @q:		videobuf2 queue
+ * @b:		buffer structure passed from userspace to vidioc_prepare_buf
+ *		handler in driver
+ *
+ * Should be called from vidioc_prepare_buf ioctl handler of a driver.
+ * This function:
+ * 1) verifies the passed buffer,
+ * 2) calls buf_prepare callback in the driver (if provided), in which
+ *    driver-specific buffer initialization can be performed,
+ *
+ * The return values from this function are intended to be directly returned
+ * from vidioc_prepare_buf handler in driver.
+ */
+int vb2_prepare_buf(struct vb2_queue *q, const struct v4l2_buffer *b)
+{
+	struct vb2_buffer *vb;
+
+	if (q->fileio) {
+		dprintk(1, "%s(): file io in progress\n", __func__);
+		return -EBUSY;
+	}
+
+	if (b->type != q->type) {
+		dprintk(1, "%s(): invalid buffer type\n", __func__);
+		return -EINVAL;
+	}
+
+	if (b->index >= q->num_buffers) {
+		dprintk(1, "%s(): buffer index out of range\n", __func__);
+		return -EINVAL;
+	}
+
+	vb = q->bufs[b->index];
+	if (NULL == vb) {
+		/* Should never happen */
+		dprintk(1, "%s(): buffer is NULL\n", __func__);
+		return -EINVAL;
+	}
+
+	if (b->memory != q->memory) {
+		dprintk(1, "%s(): invalid memory type\n", __func__);
+		return -EINVAL;
+	}
+
+	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
+		dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state);
+		return -EINVAL;
+	}
+
+	return __buf_prepare(vb, b);
+}
+EXPORT_SYMBOL_GPL(vb2_prepare_buf);
+
+/**
  * vb2_qbuf() - Queue a buffer from userspace
  * @q:		videobuf2 queue
  * @b:		buffer structure passed from userspace to vidioc_qbuf handler
@@ -1476,7 +1664,7 @@ void vb2_queue_release(struct vb2_queue *q)
 {
 	__vb2_cleanup_fileio(q);
 	__vb2_queue_cancel(q);
-	__vb2_queue_free(q);
+	__vb2_queue_free(q, q->num_buffers);
 }
 EXPORT_SYMBOL_GPL(vb2_queue_release);
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index d043132..ddebcd3 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -172,13 +172,17 @@ struct vb2_buffer {
 /**
  * struct vb2_ops - driver-specific callbacks
  *
- * @queue_setup:	called from a VIDIOC_REQBUFS handler, before
- *			memory allocation; driver should return the required
- *			number of buffers in num_buffers, the required number
- *			of planes per buffer in num_planes; the size of each
- *			plane should be set in the sizes[] array and optional
- *			per-plane allocator specific context in alloc_ctxs[]
- *			array
+ * @queue_setup:	called from VIDIOC_REQBUFS and VIDIOC_CREATE_BUFS
+ *			handlers, before memory allocation. When called with
+ *			zeroed num_planes or plane sizes, the driver should
+ *			return the required number of buffers in num_buffers,
+ *			the required number of planes per buffer in num_planes;
+ *			the size of each plane should be set in the sizes[]
+ *			array and optional per-plane allocator specific context
+ *			in alloc_ctxs[] array. If num_planes and sizes[] are
+ *			both non-zero, the driver should use them. Otherwise the
+ *			driver must make no assumptions about the buffers, that
+ *			will be made available to it.
  * @wait_prepare:	release any locks taken while calling vb2 functions;
  *			it is called before an ioctl needs to wait for a new
  *			buffer to arrive; required to avoid a deadlock in
@@ -191,11 +195,11 @@ struct vb2_buffer {
  *			perform additional buffer-related initialization;
  *			initialization failure (return != 0) will prevent
  *			queue setup from completing successfully; optional
- * @buf_prepare:	called every time the buffer is queued from userspace;
- *			drivers may perform any initialization required before
- *			each hardware operation in this callback;
- *			if an error is returned, the buffer will not be queued
- *			in driver; optional
+ * @buf_prepare:	called every time the buffer is queued from userspace
+ *			and from the VIDIOC_PREPARE_BUF ioctl; drivers may
+ *			perform any initialization required before each hardware
+ *			operation in this callback; if an error is returned, the
+ *			buffer will not be queued in driver; optional
  * @buf_finish:		called before every dequeue of the buffer back to
  *			userspace; drivers may perform any operations required
  *			before userspace accesses the buffer; optional
@@ -293,6 +297,9 @@ int vb2_wait_for_all_buffers(struct vb2_queue *q);
 int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req);
 
+int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create);
+int vb2_prepare_buf(struct vb2_queue *q, const struct v4l2_buffer *b);
+
 int vb2_queue_init(struct vb2_queue *q);
 
 void vb2_queue_release(struct vb2_queue *q);
-- 
1.7.2.5


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

* [PATCH 6/7 v5] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
                   ` (4 preceding siblings ...)
  2011-08-24 18:41 ` [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 7/7 v5] V4L: soc-camera: add 2 new ioctl() handlers Guennadi Liakhovetski
  2011-08-25 16:45   ` Guennadi Liakhovetski
  7 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

Prepare the sh_mobile_ceu_camera friver to support the new
VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup()
vb2 operation must be able to handle buffer sizes, provided by the
caller, and the .buf_prepare() operation must not use the currently
configured frame format for its operation.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/sh_mobile_ceu_camera.c |  121 ++++++++++++++++++----------
 1 files changed, 78 insertions(+), 43 deletions(-)

diff --git a/drivers/media/video/sh_mobile_ceu_camera.c b/drivers/media/video/sh_mobile_ceu_camera.c
index db3a24d..08af456 100644
--- a/drivers/media/video/sh_mobile_ceu_camera.c
+++ b/drivers/media/video/sh_mobile_ceu_camera.c
@@ -90,7 +90,6 @@
 struct sh_mobile_ceu_buffer {
 	struct vb2_buffer vb; /* v4l buffer must be first */
 	struct list_head queue;
-	enum v4l2_mbus_pixelcode code;
 };
 
 struct sh_mobile_ceu_dev {
@@ -100,7 +99,8 @@ struct sh_mobile_ceu_dev {
 
 	unsigned int irq;
 	void __iomem *base;
-	unsigned long video_limit;
+	size_t video_limit;
+	size_t buf_total;
 
 	spinlock_t lock;		/* Protects video buffer lists */
 	struct list_head capture;
@@ -192,6 +192,12 @@ static int sh_mobile_ceu_soft_reset(struct sh_mobile_ceu_dev *pcdev)
 /*
  *  Videobuf operations
  */
+
+/*
+ * .queue_setup() is called to check, whether the driver can accept the
+ *		  requested number of buffers and to fill in plane sizes
+ *		  for the current frame format if required
+ */
 static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq,
 			const struct v4l2_format *fmt,
 			unsigned int *count, unsigned int *num_planes,
@@ -200,25 +206,43 @@ static int sh_mobile_ceu_videobuf_setup(struct vb2_queue *vq,
 	struct soc_camera_device *icd = container_of(vq, struct soc_camera_device, vb2_vidq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct sh_mobile_ceu_dev *pcdev = ici->priv;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
+	int bytes_per_line;
+	unsigned int height;
+	size_t size;
+
+	if (fmt) {
+		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
+								fmt->fmt.pix.pixelformat);
+		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
+							 xlate->host_fmt);
+		height = fmt->fmt.pix.height;
+	} else {
+		/* Called from VIDIOC_REQBUFS or in compatibility mode */
+		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
 						icd->current_fmt->host_fmt);
-
+		height = icd->user_height;
+	}
 	if (bytes_per_line < 0)
 		return bytes_per_line;
 
+	sizes[0] = bytes_per_line * height;
+
 	*num_planes = 1;
 
-	pcdev->sequence = 0;
-	sizes[0] = bytes_per_line * icd->user_height;
 	alloc_ctxs[0] = pcdev->alloc_ctx;
 
+	if (!vq->num_buffers)
+		pcdev->sequence = 0;
+
 	if (!*count)
 		*count = 2;
 
-	if (pcdev->video_limit) {
-		if (PAGE_ALIGN(sizes[0]) * *count > pcdev->video_limit)
-			*count = pcdev->video_limit / PAGE_ALIGN(sizes[0]);
-	}
+	size = PAGE_ALIGN(sizes[0]) * *count;
+
+	if (pcdev->video_limit &&
+	    size + pcdev->buf_total > pcdev->video_limit)
+		*count = (pcdev->video_limit - pcdev->buf_total) /
+			PAGE_ALIGN(sizes[0]);
 
 	dev_dbg(icd->parent, "count=%d, size=%lu\n", *count, sizes[0]);
 
@@ -331,23 +355,40 @@ static int sh_mobile_ceu_capture(struct sh_mobile_ceu_dev *pcdev)
 
 static int sh_mobile_ceu_videobuf_prepare(struct vb2_buffer *vb)
 {
+	struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb);
+
+	/* Added list head initialization on alloc */
+	WARN(!list_empty(&buf->queue), "Buffer %p on queue!\n", vb);
+
+	return 0;
+}
+
+static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb)
+{
 	struct soc_camera_device *icd = container_of(vb->vb2_queue, struct soc_camera_device, vb2_vidq);
-	struct sh_mobile_ceu_buffer *buf;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	struct sh_mobile_ceu_dev *pcdev = ici->priv;
+	struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb);
+	unsigned long size;
 	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
 						icd->current_fmt->host_fmt);
-	unsigned long size;
 
 	if (bytes_per_line < 0)
-		return bytes_per_line;
+		goto error;
+
+	size = icd->user_height * bytes_per_line;
+
+	if (vb2_plane_size(vb, 0) < size) {
+		dev_err(icd->parent, "Buffer #%d too small (%lu < %lu)\n",
+			vb->v4l2_buf.index, vb2_plane_size(vb, 0), size);
+		goto error;
+	}
 
-	buf = to_ceu_vb(vb);
+	vb2_set_plane_payload(vb, 0, size);
 
 	dev_dbg(icd->parent, "%s (vb=0x%p) 0x%p %lu\n", __func__,
 		vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0));
 
-	/* Added list head initialization on alloc */
-	WARN(!list_empty(&buf->queue), "Buffer %p on queue!\n", vb);
-
 #ifdef DEBUG
 	/*
 	 * This can be useful if you want to see if we actually fill
@@ -357,31 +398,6 @@ static int sh_mobile_ceu_videobuf_prepare(struct vb2_buffer *vb)
 		memset(vb2_plane_vaddr(vb, 0), 0xaa, vb2_get_plane_payload(vb, 0));
 #endif
 
-	BUG_ON(NULL == icd->current_fmt);
-
-	size = icd->user_height * bytes_per_line;
-
-	if (vb2_plane_size(vb, 0) < size) {
-		dev_err(icd->parent, "Buffer too small (%lu < %lu)\n",
-			vb2_plane_size(vb, 0), size);
-		return -ENOBUFS;
-	}
-
-	vb2_set_plane_payload(vb, 0, size);
-
-	return 0;
-}
-
-static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb)
-{
-	struct soc_camera_device *icd = container_of(vb->vb2_queue, struct soc_camera_device, vb2_vidq);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct sh_mobile_ceu_dev *pcdev = ici->priv;
-	struct sh_mobile_ceu_buffer *buf = to_ceu_vb(vb);
-
-	dev_dbg(icd->parent, "%s (vb=0x%p) 0x%p %lu\n", __func__,
-		vb, vb2_plane_vaddr(vb, 0), vb2_get_plane_payload(vb, 0));
-
 	spin_lock_irq(&pcdev->lock);
 	list_add_tail(&buf->queue, &pcdev->capture);
 
@@ -395,6 +411,11 @@ static void sh_mobile_ceu_videobuf_queue(struct vb2_buffer *vb)
 		sh_mobile_ceu_capture(pcdev);
 	}
 	spin_unlock_irq(&pcdev->lock);
+
+	return;
+
+error:
+	vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
 }
 
 static void sh_mobile_ceu_videobuf_release(struct vb2_buffer *vb)
@@ -419,11 +440,23 @@ static void sh_mobile_ceu_videobuf_release(struct vb2_buffer *vb)
 	if (buf->queue.next)
 		list_del_init(&buf->queue);
 
+	pcdev->buf_total -= PAGE_ALIGN(vb2_plane_size(vb, 0));
+	dev_dbg(icd->parent, "%s() %zu bytes buffers\n", __func__,
+		pcdev->buf_total);
+
 	spin_unlock_irq(&pcdev->lock);
 }
 
 static int sh_mobile_ceu_videobuf_init(struct vb2_buffer *vb)
 {
+	struct soc_camera_device *icd = container_of(vb->vb2_queue, struct soc_camera_device, vb2_vidq);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	struct sh_mobile_ceu_dev *pcdev = ici->priv;
+
+	pcdev->buf_total += PAGE_ALIGN(vb2_plane_size(vb, 0));
+	dev_dbg(icd->parent, "%s() %zu bytes buffers\n", __func__,
+		pcdev->buf_total);
+
 	/* This is for locking debugging only */
 	INIT_LIST_HEAD(&to_ceu_vb(vb)->queue);
 	return 0;
@@ -525,6 +558,8 @@ static int sh_mobile_ceu_add_device(struct soc_camera_device *icd)
 
 	pm_runtime_get_sync(ici->v4l2_dev.dev);
 
+	pcdev->buf_total = 0;
+
 	ret = sh_mobile_ceu_soft_reset(pcdev);
 
 	csi2_sd = find_csi2(pcdev);
@@ -1674,7 +1709,7 @@ static int sh_mobile_ceu_set_fmt(struct soc_camera_device *icd,
 		image_mode = false;
 	}
 
-	dev_info(dev, "S_FMT(pix=0x%x, fld 0x%x, code 0x%x, %ux%u)\n", pixfmt, mf.field, mf.code,
+	dev_geo(dev, "S_FMT(pix=0x%x, fld 0x%x, code 0x%x, %ux%u)\n", pixfmt, mf.field, mf.code,
 		pix->width, pix->height);
 
 	dev_geo(dev, "4: request camera output %ux%u\n", mf.width, mf.height);
-- 
1.7.2.5


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

* [PATCH 7/7 v5] V4L: soc-camera: add 2 new ioctl() handlers
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
                   ` (5 preceding siblings ...)
  2011-08-24 18:41 ` [PATCH 6/7 v5] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers Guennadi Liakhovetski
@ 2011-08-24 18:41 ` Guennadi Liakhovetski
  2011-09-28 13:20   ` [PATCH 7/7 v9] " Guennadi Liakhovetski
  2011-08-25 16:45   ` Guennadi Liakhovetski
  7 siblings, 1 reply; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-24 18:41 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

This patch adds two new ioctl() handlers: .vidioc_create_bufs() and
.vidioc_prepare_buf() for compliant vb2 soc-camera hosts.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
---
 drivers/media/video/soc_camera.c |   33 +++++++++++++++++++++++++++++++--
 1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index ac23916..5943235 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -318,6 +318,32 @@ static int soc_camera_dqbuf(struct file *file, void *priv,
 		return vb2_dqbuf(&icd->vb2_vidq, p, file->f_flags & O_NONBLOCK);
 }
 
+static int soc_camera_create_bufs(struct file *file, void *priv,
+			    struct v4l2_create_buffers *create)
+{
+	struct soc_camera_device *icd = file->private_data;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+
+	/* videobuf2 only */
+	if (ici->ops->init_videobuf)
+		return -EINVAL;
+	else
+		return vb2_create_bufs(&icd->vb2_vidq, create);
+}
+
+static int soc_camera_prepare_buf(struct file *file, void *priv,
+				  const struct v4l2_buffer *b)
+{
+	struct soc_camera_device *icd = file->private_data;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+
+	/* videobuf2 only */
+	if (ici->ops->init_videobuf)
+		return -EINVAL;
+	else
+		return vb2_prepare_buf(&icd->vb2_vidq, b);
+}
+
 /* Always entered with .video_lock held */
 static int soc_camera_init_user_formats(struct soc_camera_device *icd)
 {
@@ -1101,6 +1127,7 @@ static int soc_camera_probe(struct soc_camera_device *icd)
 		if (!control || !control->driver || !dev_get_drvdata(control) ||
 		    !try_module_get(control->driver->owner)) {
 			icl->del_device(icd);
+			ret = -ENODEV;
 			goto enodrv;
 		}
 	}
@@ -1366,19 +1393,21 @@ static int soc_camera_device_register(struct soc_camera_device *icd)
 
 static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = {
 	.vidioc_querycap	 = soc_camera_querycap,
+	.vidioc_try_fmt_vid_cap  = soc_camera_try_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap    = soc_camera_g_fmt_vid_cap,
-	.vidioc_enum_fmt_vid_cap = soc_camera_enum_fmt_vid_cap,
 	.vidioc_s_fmt_vid_cap    = soc_camera_s_fmt_vid_cap,
+	.vidioc_enum_fmt_vid_cap = soc_camera_enum_fmt_vid_cap,
 	.vidioc_enum_input	 = soc_camera_enum_input,
 	.vidioc_g_input		 = soc_camera_g_input,
 	.vidioc_s_input		 = soc_camera_s_input,
 	.vidioc_s_std		 = soc_camera_s_std,
 	.vidioc_enum_framesizes  = soc_camera_enum_fsizes,
 	.vidioc_reqbufs		 = soc_camera_reqbufs,
-	.vidioc_try_fmt_vid_cap  = soc_camera_try_fmt_vid_cap,
 	.vidioc_querybuf	 = soc_camera_querybuf,
 	.vidioc_qbuf		 = soc_camera_qbuf,
 	.vidioc_dqbuf		 = soc_camera_dqbuf,
+	.vidioc_create_bufs	 = soc_camera_create_bufs,
+	.vidioc_prepare_buf	 = soc_camera_prepare_buf,
 	.vidioc_streamon	 = soc_camera_streamon,
 	.vidioc_streamoff	 = soc_camera_streamoff,
 	.vidioc_queryctrl	 = soc_camera_queryctrl,
-- 
1.7.2.5


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

* [PATCH 0/2] i.MX3: support multi-size buffers in V4L
  2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
@ 2011-08-25 16:45   ` Guennadi Liakhovetski
  2011-08-24 18:41 ` [PATCH 2/7 v5] V4L: add two new ioctl()s for multi-size videobuffer management Guennadi Liakhovetski
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 16:45 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski, linux-arm-kernel,
	linux-kernel, Vinod Koul, Dan Williams, Sascha Hauer

Supporting new V4L2 ioctl()s in the mx3_camera driver also requires an 
extension for the ipu-idmac dmaengine driver. The original thread, adding 
the ioctl()s can be found here:

http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/37143

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* [PATCH 0/2] i.MX3: support multi-size buffers in V4L
@ 2011-08-25 16:45   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

Supporting new V4L2 ioctl()s in the mx3_camera driver also requires an 
extension for the ipu-idmac dmaengine driver. The original thread, adding 
the ioctl()s can be found here:

http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/37143

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
  2011-08-25 16:45   ` Guennadi Liakhovetski
@ 2011-08-25 16:45     ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 16:45 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski, linux-arm-kernel,
	linux-kernel, Vinod Koul, Dan Williams, Sascha Hauer

To support multi-size buffers in the mx3_camera V4L2 driver we have to be
able to stop DMA on a channel without releasing descriptors and completely
halting the hardware. Use the DMA_PAUSE control to implement this mode.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/dma/ipu/ipu_idmac.c |   65 +++++++++++++++++++++++++++---------------
 1 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index c1a125e..42cdf1c 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
 	    ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) {
 		callback = descnew->txd.callback;
 		callback_param = descnew->txd.callback_param;
+		list_del_init(&descnew->list);
 		spin_unlock(&ichan->lock);
 		if (callback)
 			callback(callback_param);
@@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 {
 	struct idmac_channel *ichan = to_idmac_chan(chan);
 	struct idmac *idmac = to_idmac(chan->device);
+	struct ipu *ipu = to_ipu(idmac);
+	struct list_head *list, *tmp;
 	unsigned long flags;
 	int i;
 
-	/* Only supports DMA_TERMINATE_ALL */
-	if (cmd != DMA_TERMINATE_ALL)
-		return -ENXIO;
+	switch (cmd) {
+	case DMA_PAUSE:
+		spin_lock_irqsave(&ipu->lock, flags);
+		ipu_ic_disable_task(ipu, chan->chan_id);
 
-	ipu_disable_channel(idmac, ichan,
-			    ichan->status >= IPU_CHANNEL_ENABLED);
+		/* Return all descriptors into "prepared" state */
+		list_for_each_safe(list, tmp, &ichan->queue)
+			list_del_init(list);
 
-	tasklet_disable(&to_ipu(idmac)->tasklet);
+		ichan->sg[0] = NULL;
+		ichan->sg[1] = NULL;
 
-	/* ichan->queue is modified in ISR, have to spinlock */
-	spin_lock_irqsave(&ichan->lock, flags);
-	list_splice_init(&ichan->queue, &ichan->free_list);
+		spin_unlock_irqrestore(&ipu->lock, flags);
 
-	if (ichan->desc)
-		for (i = 0; i < ichan->n_tx_desc; i++) {
-			struct idmac_tx_desc *desc = ichan->desc + i;
-			if (list_empty(&desc->list))
-				/* Descriptor was prepared, but not submitted */
-				list_add(&desc->list, &ichan->free_list);
+		ichan->status = IPU_CHANNEL_INITIALIZED;
+		break;
+	case DMA_TERMINATE_ALL:
+		ipu_disable_channel(idmac, ichan,
+				    ichan->status >= IPU_CHANNEL_ENABLED);
 
-			async_tx_clear_ack(&desc->txd);
-		}
+		tasklet_disable(&ipu->tasklet);
 
-	ichan->sg[0] = NULL;
-	ichan->sg[1] = NULL;
-	spin_unlock_irqrestore(&ichan->lock, flags);
+		/* ichan->queue is modified in ISR, have to spinlock */
+		spin_lock_irqsave(&ichan->lock, flags);
+		list_splice_init(&ichan->queue, &ichan->free_list);
 
-	tasklet_enable(&to_ipu(idmac)->tasklet);
+		if (ichan->desc)
+			for (i = 0; i < ichan->n_tx_desc; i++) {
+				struct idmac_tx_desc *desc = ichan->desc + i;
+				if (list_empty(&desc->list))
+					/* Descriptor was prepared, but not submitted */
+					list_add(&desc->list, &ichan->free_list);
 
-	ichan->status = IPU_CHANNEL_INITIALIZED;
+				async_tx_clear_ack(&desc->txd);
+			}
+
+		ichan->sg[0] = NULL;
+		ichan->sg[1] = NULL;
+		spin_unlock_irqrestore(&ichan->lock, flags);
+
+		tasklet_enable(&ipu->tasklet);
+
+		ichan->status = IPU_CHANNEL_INITIALIZED;
+		break;
+	default:
+		return -ENOSYS;
+	}
 
 	return 0;
 }
@@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu)
 		struct idmac_channel *ichan = ipu->channel + i;
 
 		idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0);
-		idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0);
 	}
 
 	dma_async_device_unregister(&idmac->dma);
-- 
1.7.2.5


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

* [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
@ 2011-08-25 16:45     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 16:45 UTC (permalink / raw)
  To: linux-arm-kernel

To support multi-size buffers in the mx3_camera V4L2 driver we have to be
able to stop DMA on a channel without releasing descriptors and completely
halting the hardware. Use the DMA_PAUSE control to implement this mode.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/dma/ipu/ipu_idmac.c |   65 +++++++++++++++++++++++++++---------------
 1 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index c1a125e..42cdf1c 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
 	    ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) {
 		callback = descnew->txd.callback;
 		callback_param = descnew->txd.callback_param;
+		list_del_init(&descnew->list);
 		spin_unlock(&ichan->lock);
 		if (callback)
 			callback(callback_param);
@@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
 {
 	struct idmac_channel *ichan = to_idmac_chan(chan);
 	struct idmac *idmac = to_idmac(chan->device);
+	struct ipu *ipu = to_ipu(idmac);
+	struct list_head *list, *tmp;
 	unsigned long flags;
 	int i;
 
-	/* Only supports DMA_TERMINATE_ALL */
-	if (cmd != DMA_TERMINATE_ALL)
-		return -ENXIO;
+	switch (cmd) {
+	case DMA_PAUSE:
+		spin_lock_irqsave(&ipu->lock, flags);
+		ipu_ic_disable_task(ipu, chan->chan_id);
 
-	ipu_disable_channel(idmac, ichan,
-			    ichan->status >= IPU_CHANNEL_ENABLED);
+		/* Return all descriptors into "prepared" state */
+		list_for_each_safe(list, tmp, &ichan->queue)
+			list_del_init(list);
 
-	tasklet_disable(&to_ipu(idmac)->tasklet);
+		ichan->sg[0] = NULL;
+		ichan->sg[1] = NULL;
 
-	/* ichan->queue is modified in ISR, have to spinlock */
-	spin_lock_irqsave(&ichan->lock, flags);
-	list_splice_init(&ichan->queue, &ichan->free_list);
+		spin_unlock_irqrestore(&ipu->lock, flags);
 
-	if (ichan->desc)
-		for (i = 0; i < ichan->n_tx_desc; i++) {
-			struct idmac_tx_desc *desc = ichan->desc + i;
-			if (list_empty(&desc->list))
-				/* Descriptor was prepared, but not submitted */
-				list_add(&desc->list, &ichan->free_list);
+		ichan->status = IPU_CHANNEL_INITIALIZED;
+		break;
+	case DMA_TERMINATE_ALL:
+		ipu_disable_channel(idmac, ichan,
+				    ichan->status >= IPU_CHANNEL_ENABLED);
 
-			async_tx_clear_ack(&desc->txd);
-		}
+		tasklet_disable(&ipu->tasklet);
 
-	ichan->sg[0] = NULL;
-	ichan->sg[1] = NULL;
-	spin_unlock_irqrestore(&ichan->lock, flags);
+		/* ichan->queue is modified in ISR, have to spinlock */
+		spin_lock_irqsave(&ichan->lock, flags);
+		list_splice_init(&ichan->queue, &ichan->free_list);
 
-	tasklet_enable(&to_ipu(idmac)->tasklet);
+		if (ichan->desc)
+			for (i = 0; i < ichan->n_tx_desc; i++) {
+				struct idmac_tx_desc *desc = ichan->desc + i;
+				if (list_empty(&desc->list))
+					/* Descriptor was prepared, but not submitted */
+					list_add(&desc->list, &ichan->free_list);
 
-	ichan->status = IPU_CHANNEL_INITIALIZED;
+				async_tx_clear_ack(&desc->txd);
+			}
+
+		ichan->sg[0] = NULL;
+		ichan->sg[1] = NULL;
+		spin_unlock_irqrestore(&ichan->lock, flags);
+
+		tasklet_enable(&ipu->tasklet);
+
+		ichan->status = IPU_CHANNEL_INITIALIZED;
+		break;
+	default:
+		return -ENOSYS;
+	}
 
 	return 0;
 }
@@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu)
 		struct idmac_channel *ichan = ipu->channel + i;
 
 		idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0);
-		idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0);
 	}
 
 	dma_async_device_unregister(&idmac->dma);
-- 
1.7.2.5

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

* [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
  2011-08-25 16:45   ` Guennadi Liakhovetski
@ 2011-08-25 16:46     ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 16:46 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski, linux-arm-kernel,
	linux-kernel, Vinod Koul, Dan Williams, Sascha Hauer

Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and
VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be
able to handle buffer sizes, provided by the caller, and the
.buf_prepare() operation must not use the currently configured frame
format for its operation, which makes it superfluous for this driver.
Its functionality is moved into .buf_queue().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mx3_camera.c |  153 +++++++++++++++++++------------------
 1 files changed, 79 insertions(+), 74 deletions(-)

diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index 6bfbce9..51eee4e 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -114,6 +114,7 @@ struct mx3_camera_dev {
 	struct list_head	capture;
 	spinlock_t		lock;		/* Protects video buffer lists */
 	struct mx3_camera_buffer *active;
+	size_t			buf_total;
 	struct vb2_alloc_ctx	*alloc_ctx;
 	enum v4l2_field		field;
 	int			sequence;
@@ -198,118 +199,117 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
+	int bytes_per_line;
+	unsigned int height;
 
+	if (!mx3_cam->idmac_channel[0])
+		return -EINVAL;
+
+	if (fmt) {
+		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
+								fmt->fmt.pix.pixelformat);
+		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
+							 xlate->host_fmt);
+		height = fmt->fmt.pix.height;
+	} else {
+		/* Called from VIDIOC_REQBUFS or in compatibility mode */
+		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
+						icd->current_fmt->host_fmt);
+		height = icd->user_height;
+	}
 	if (bytes_per_line < 0)
 		return bytes_per_line;
 
-	if (!mx3_cam->idmac_channel[0])
-		return -EINVAL;
+	sizes[0] = bytes_per_line * height;
 
 	*num_planes = 1;
 
-	mx3_cam->sequence = 0;
-	sizes[0] = bytes_per_line * icd->user_height;
 	alloc_ctxs[0] = mx3_cam->alloc_ctx;
 
+	if (!vq->num_buffers)
+		mx3_cam->sequence = 0;
+
 	if (!*count)
 		*count = 32;
 
-	if (sizes[0] * *count > MAX_VIDEO_MEM * 1024 * 1024)
-		*count = MAX_VIDEO_MEM * 1024 * 1024 / sizes[0];
+	if (sizes[0] * *count + mx3_cam->buf_total > MAX_VIDEO_MEM * 1024 * 1024)
+		*count = (MAX_VIDEO_MEM * 1024 * 1024 - mx3_cam->buf_total) /
+			sizes[0];
 
 	return 0;
 }
 
-static int mx3_videobuf_prepare(struct vb2_buffer *vb)
+static enum pixel_fmt fourcc_to_ipu_pix(__u32 fourcc)
+{
+	/* Add more formats as need arises and test possibilities appear... */
+	switch (fourcc) {
+	case V4L2_PIX_FMT_RGB24:
+		return IPU_PIX_FMT_RGB24;
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_RGB565:
+	default:
+		return IPU_PIX_FMT_GENERIC;
+	}
+}
+
+static void mx3_videobuf_queue(struct vb2_buffer *vb)
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
+	struct mx3_camera_buffer *buf = to_mx3_vb(vb);
+	struct scatterlist *sg = &buf->sg;
+	struct dma_async_tx_descriptor *txd;
 	struct idmac_channel *ichan = mx3_cam->idmac_channel[0];
-	struct scatterlist *sg;
-	struct mx3_camera_buffer *buf;
+	struct idmac_video_param *video = &ichan->params.video;
+	const struct soc_mbus_pixelfmt *host_fmt = icd->current_fmt->host_fmt;
+	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, host_fmt);
+	unsigned long flags;
+	dma_cookie_t cookie;
 	size_t new_size;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
 
-	buf = to_mx3_vb(vb);
-	sg = &buf->sg;
+	BUG_ON(bytes_per_line <= 0);
 
 	new_size = bytes_per_line * icd->user_height;
 
 	if (vb2_plane_size(vb, 0) < new_size) {
-		dev_err(icd->parent, "Buffer too small (%lu < %zu)\n",
-			vb2_plane_size(vb, 0), new_size);
-		return -ENOBUFS;
+		dev_err(icd->parent, "Buffer #%d too small (%lu < %zu)\n",
+			vb->v4l2_buf.index, vb2_plane_size(vb, 0), new_size);
+		goto error;
 	}
 
 	if (buf->state == CSI_BUF_NEEDS_INIT) {
 		sg_dma_address(sg)	= vb2_dma_contig_plane_paddr(vb, 0);
 		sg_dma_len(sg)		= new_size;
 
-		buf->txd = ichan->dma_chan.device->device_prep_slave_sg(
+		txd = ichan->dma_chan.device->device_prep_slave_sg(
 			&ichan->dma_chan, sg, 1, DMA_FROM_DEVICE,
 			DMA_PREP_INTERRUPT);
-		if (!buf->txd)
-			return -EIO;
+		if (!txd)
+			goto error;
 
-		buf->txd->callback_param	= buf->txd;
-		buf->txd->callback		= mx3_cam_dma_done;
+		txd->callback_param	= txd;
+		txd->callback		= mx3_cam_dma_done;
 
-		buf->state = CSI_BUF_PREPARED;
+		buf->state		= CSI_BUF_PREPARED;
+		buf->txd		= txd;
+	} else {
+		txd = buf->txd;
 	}
 
 	vb2_set_plane_payload(vb, 0, new_size);
 
-	return 0;
-}
-
-static enum pixel_fmt fourcc_to_ipu_pix(__u32 fourcc)
-{
-	/* Add more formats as need arises and test possibilities appear... */
-	switch (fourcc) {
-	case V4L2_PIX_FMT_RGB24:
-		return IPU_PIX_FMT_RGB24;
-	case V4L2_PIX_FMT_UYVY:
-	case V4L2_PIX_FMT_RGB565:
-	default:
-		return IPU_PIX_FMT_GENERIC;
-	}
-}
-
-static void mx3_videobuf_queue(struct vb2_buffer *vb)
-{
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct mx3_camera_dev *mx3_cam = ici->priv;
-	struct mx3_camera_buffer *buf = to_mx3_vb(vb);
-	struct dma_async_tx_descriptor *txd = buf->txd;
-	struct idmac_channel *ichan = to_idmac_chan(txd->chan);
-	struct idmac_video_param *video = &ichan->params.video;
-	dma_cookie_t cookie;
-	u32 fourcc = icd->current_fmt->host_fmt->fourcc;
-	unsigned long flags;
-
 	/* This is the configuration of one sg-element */
-	video->out_pixel_fmt	= fourcc_to_ipu_pix(fourcc);
+	video->out_pixel_fmt = fourcc_to_ipu_pix(host_fmt->fourcc);
 
 	if (video->out_pixel_fmt == IPU_PIX_FMT_GENERIC) {
 		/*
-		 * If the IPU DMA channel is configured to transport
-		 * generic 8-bit data, we have to set up correctly the
-		 * geometry parameters upon the current pixel format.
-		 * So, since the DMA horizontal parameters are expressed
-		 * in bytes not pixels, convert these in the right unit.
+		 * If the IPU DMA channel is configured to transfer generic
+		 * 8-bit data, we have to set up the geometry parameters
+		 * correctly, according to the current pixel format. The DMA
+		 * horizontal parameters in this case are expressed in bytes,
+		 * not in pixels.
 		 */
-		int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-		BUG_ON(bytes_per_line <= 0);
-
 		video->out_width	= bytes_per_line;
 		video->out_height	= icd->user_height;
 		video->out_stride	= bytes_per_line;
@@ -353,6 +353,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
 		mx3_cam->active = NULL;
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
+error:
 	vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
 }
 
@@ -386,17 +387,24 @@ static void mx3_videobuf_release(struct vb2_buffer *vb)
 	}
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
+
+	mx3_cam->buf_total -= vb2_plane_size(vb, 0);
 }
 
 static int mx3_videobuf_init(struct vb2_buffer *vb)
 {
+	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	struct mx3_camera_dev *mx3_cam = ici->priv;
 	struct mx3_camera_buffer *buf = to_mx3_vb(vb);
+
 	/* This is for locking debugging only */
 	INIT_LIST_HEAD(&buf->queue);
 	sg_init_table(&buf->sg, 1);
 
 	buf->state = CSI_BUF_NEEDS_INIT;
-	buf->txd = NULL;
+
+	mx3_cam->buf_total += vb2_plane_size(vb, 0);
 
 	return 0;
 }
@@ -407,13 +415,12 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
 	struct idmac_channel *ichan = mx3_cam->idmac_channel[0];
-	struct dma_chan *chan;
 	struct mx3_camera_buffer *buf, *tmp;
 	unsigned long flags;
 
 	if (ichan) {
-		chan = &ichan->dma_chan;
-		chan->device->device_control(chan, DMA_TERMINATE_ALL, 0);
+		struct dma_chan *chan = &ichan->dma_chan;
+		chan->device->device_control(chan, DMA_PAUSE, 0);
 	}
 
 	spin_lock_irqsave(&mx3_cam->lock, flags);
@@ -421,8 +428,8 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 	mx3_cam->active = NULL;
 
 	list_for_each_entry_safe(buf, tmp, &mx3_cam->capture, queue) {
-		buf->state = CSI_BUF_NEEDS_INIT;
 		list_del_init(&buf->queue);
+		vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
 	}
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
@@ -432,7 +439,6 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 
 static struct vb2_ops mx3_videobuf_ops = {
 	.queue_setup	= mx3_videobuf_setup,
-	.buf_prepare	= mx3_videobuf_prepare,
 	.buf_queue	= mx3_videobuf_queue,
 	.buf_cleanup	= mx3_videobuf_release,
 	.buf_init	= mx3_videobuf_init,
@@ -516,6 +522,7 @@ static int mx3_camera_add_device(struct soc_camera_device *icd)
 
 	mx3_camera_activate(mx3_cam, icd);
 
+	mx3_cam->buf_total = 0;
 	mx3_cam->icd = icd;
 
 	dev_info(icd->parent, "MX3 Camera driver attached to camera %d\n",
@@ -1263,8 +1270,6 @@ static int __devexit mx3_camera_remove(struct platform_device *pdev)
 
 	dmaengine_put();
 
-	dev_info(&pdev->dev, "i.MX3x Camera driver unloaded\n");
-
 	return 0;
 }
 
-- 
1.7.2.5


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

* [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
@ 2011-08-25 16:46     ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 16:46 UTC (permalink / raw)
  To: linux-arm-kernel

Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and
VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be
able to handle buffer sizes, provided by the caller, and the
.buf_prepare() operation must not use the currently configured frame
format for its operation, which makes it superfluous for this driver.
Its functionality is moved into .buf_queue().

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/media/video/mx3_camera.c |  153 +++++++++++++++++++------------------
 1 files changed, 79 insertions(+), 74 deletions(-)

diff --git a/drivers/media/video/mx3_camera.c b/drivers/media/video/mx3_camera.c
index 6bfbce9..51eee4e 100644
--- a/drivers/media/video/mx3_camera.c
+++ b/drivers/media/video/mx3_camera.c
@@ -114,6 +114,7 @@ struct mx3_camera_dev {
 	struct list_head	capture;
 	spinlock_t		lock;		/* Protects video buffer lists */
 	struct mx3_camera_buffer *active;
+	size_t			buf_total;
 	struct vb2_alloc_ctx	*alloc_ctx;
 	enum v4l2_field		field;
 	int			sequence;
@@ -198,118 +199,117 @@ static int mx3_videobuf_setup(struct vb2_queue *vq,
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vq);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
+	int bytes_per_line;
+	unsigned int height;
 
+	if (!mx3_cam->idmac_channel[0])
+		return -EINVAL;
+
+	if (fmt) {
+		const struct soc_camera_format_xlate *xlate = soc_camera_xlate_by_fourcc(icd,
+								fmt->fmt.pix.pixelformat);
+		bytes_per_line = soc_mbus_bytes_per_line(fmt->fmt.pix.width,
+							 xlate->host_fmt);
+		height = fmt->fmt.pix.height;
+	} else {
+		/* Called from VIDIOC_REQBUFS or in compatibility mode */
+		bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
+						icd->current_fmt->host_fmt);
+		height = icd->user_height;
+	}
 	if (bytes_per_line < 0)
 		return bytes_per_line;
 
-	if (!mx3_cam->idmac_channel[0])
-		return -EINVAL;
+	sizes[0] = bytes_per_line * height;
 
 	*num_planes = 1;
 
-	mx3_cam->sequence = 0;
-	sizes[0] = bytes_per_line * icd->user_height;
 	alloc_ctxs[0] = mx3_cam->alloc_ctx;
 
+	if (!vq->num_buffers)
+		mx3_cam->sequence = 0;
+
 	if (!*count)
 		*count = 32;
 
-	if (sizes[0] * *count > MAX_VIDEO_MEM * 1024 * 1024)
-		*count = MAX_VIDEO_MEM * 1024 * 1024 / sizes[0];
+	if (sizes[0] * *count + mx3_cam->buf_total > MAX_VIDEO_MEM * 1024 * 1024)
+		*count = (MAX_VIDEO_MEM * 1024 * 1024 - mx3_cam->buf_total) /
+			sizes[0];
 
 	return 0;
 }
 
-static int mx3_videobuf_prepare(struct vb2_buffer *vb)
+static enum pixel_fmt fourcc_to_ipu_pix(__u32 fourcc)
+{
+	/* Add more formats as need arises and test possibilities appear... */
+	switch (fourcc) {
+	case V4L2_PIX_FMT_RGB24:
+		return IPU_PIX_FMT_RGB24;
+	case V4L2_PIX_FMT_UYVY:
+	case V4L2_PIX_FMT_RGB565:
+	default:
+		return IPU_PIX_FMT_GENERIC;
+	}
+}
+
+static void mx3_videobuf_queue(struct vb2_buffer *vb)
 {
 	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
+	struct mx3_camera_buffer *buf = to_mx3_vb(vb);
+	struct scatterlist *sg = &buf->sg;
+	struct dma_async_tx_descriptor *txd;
 	struct idmac_channel *ichan = mx3_cam->idmac_channel[0];
-	struct scatterlist *sg;
-	struct mx3_camera_buffer *buf;
+	struct idmac_video_param *video = &ichan->params.video;
+	const struct soc_mbus_pixelfmt *host_fmt = icd->current_fmt->host_fmt;
+	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width, host_fmt);
+	unsigned long flags;
+	dma_cookie_t cookie;
 	size_t new_size;
-	int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-
-	if (bytes_per_line < 0)
-		return bytes_per_line;
 
-	buf = to_mx3_vb(vb);
-	sg = &buf->sg;
+	BUG_ON(bytes_per_line <= 0);
 
 	new_size = bytes_per_line * icd->user_height;
 
 	if (vb2_plane_size(vb, 0) < new_size) {
-		dev_err(icd->parent, "Buffer too small (%lu < %zu)\n",
-			vb2_plane_size(vb, 0), new_size);
-		return -ENOBUFS;
+		dev_err(icd->parent, "Buffer #%d too small (%lu < %zu)\n",
+			vb->v4l2_buf.index, vb2_plane_size(vb, 0), new_size);
+		goto error;
 	}
 
 	if (buf->state == CSI_BUF_NEEDS_INIT) {
 		sg_dma_address(sg)	= vb2_dma_contig_plane_paddr(vb, 0);
 		sg_dma_len(sg)		= new_size;
 
-		buf->txd = ichan->dma_chan.device->device_prep_slave_sg(
+		txd = ichan->dma_chan.device->device_prep_slave_sg(
 			&ichan->dma_chan, sg, 1, DMA_FROM_DEVICE,
 			DMA_PREP_INTERRUPT);
-		if (!buf->txd)
-			return -EIO;
+		if (!txd)
+			goto error;
 
-		buf->txd->callback_param	= buf->txd;
-		buf->txd->callback		= mx3_cam_dma_done;
+		txd->callback_param	= txd;
+		txd->callback		= mx3_cam_dma_done;
 
-		buf->state = CSI_BUF_PREPARED;
+		buf->state		= CSI_BUF_PREPARED;
+		buf->txd		= txd;
+	} else {
+		txd = buf->txd;
 	}
 
 	vb2_set_plane_payload(vb, 0, new_size);
 
-	return 0;
-}
-
-static enum pixel_fmt fourcc_to_ipu_pix(__u32 fourcc)
-{
-	/* Add more formats as need arises and test possibilities appear... */
-	switch (fourcc) {
-	case V4L2_PIX_FMT_RGB24:
-		return IPU_PIX_FMT_RGB24;
-	case V4L2_PIX_FMT_UYVY:
-	case V4L2_PIX_FMT_RGB565:
-	default:
-		return IPU_PIX_FMT_GENERIC;
-	}
-}
-
-static void mx3_videobuf_queue(struct vb2_buffer *vb)
-{
-	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
-	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
-	struct mx3_camera_dev *mx3_cam = ici->priv;
-	struct mx3_camera_buffer *buf = to_mx3_vb(vb);
-	struct dma_async_tx_descriptor *txd = buf->txd;
-	struct idmac_channel *ichan = to_idmac_chan(txd->chan);
-	struct idmac_video_param *video = &ichan->params.video;
-	dma_cookie_t cookie;
-	u32 fourcc = icd->current_fmt->host_fmt->fourcc;
-	unsigned long flags;
-
 	/* This is the configuration of one sg-element */
-	video->out_pixel_fmt	= fourcc_to_ipu_pix(fourcc);
+	video->out_pixel_fmt = fourcc_to_ipu_pix(host_fmt->fourcc);
 
 	if (video->out_pixel_fmt == IPU_PIX_FMT_GENERIC) {
 		/*
-		 * If the IPU DMA channel is configured to transport
-		 * generic 8-bit data, we have to set up correctly the
-		 * geometry parameters upon the current pixel format.
-		 * So, since the DMA horizontal parameters are expressed
-		 * in bytes not pixels, convert these in the right unit.
+		 * If the IPU DMA channel is configured to transfer generic
+		 * 8-bit data, we have to set up the geometry parameters
+		 * correctly, according to the current pixel format. The DMA
+		 * horizontal parameters in this case are expressed in bytes,
+		 * not in pixels.
 		 */
-		int bytes_per_line = soc_mbus_bytes_per_line(icd->user_width,
-						icd->current_fmt->host_fmt);
-		BUG_ON(bytes_per_line <= 0);
-
 		video->out_width	= bytes_per_line;
 		video->out_height	= icd->user_height;
 		video->out_stride	= bytes_per_line;
@@ -353,6 +353,7 @@ static void mx3_videobuf_queue(struct vb2_buffer *vb)
 		mx3_cam->active = NULL;
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
+error:
 	vb2_buffer_done(vb, VB2_BUF_STATE_ERROR);
 }
 
@@ -386,17 +387,24 @@ static void mx3_videobuf_release(struct vb2_buffer *vb)
 	}
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
+
+	mx3_cam->buf_total -= vb2_plane_size(vb, 0);
 }
 
 static int mx3_videobuf_init(struct vb2_buffer *vb)
 {
+	struct soc_camera_device *icd = soc_camera_from_vb2q(vb->vb2_queue);
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+	struct mx3_camera_dev *mx3_cam = ici->priv;
 	struct mx3_camera_buffer *buf = to_mx3_vb(vb);
+
 	/* This is for locking debugging only */
 	INIT_LIST_HEAD(&buf->queue);
 	sg_init_table(&buf->sg, 1);
 
 	buf->state = CSI_BUF_NEEDS_INIT;
-	buf->txd = NULL;
+
+	mx3_cam->buf_total += vb2_plane_size(vb, 0);
 
 	return 0;
 }
@@ -407,13 +415,12 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
 	struct mx3_camera_dev *mx3_cam = ici->priv;
 	struct idmac_channel *ichan = mx3_cam->idmac_channel[0];
-	struct dma_chan *chan;
 	struct mx3_camera_buffer *buf, *tmp;
 	unsigned long flags;
 
 	if (ichan) {
-		chan = &ichan->dma_chan;
-		chan->device->device_control(chan, DMA_TERMINATE_ALL, 0);
+		struct dma_chan *chan = &ichan->dma_chan;
+		chan->device->device_control(chan, DMA_PAUSE, 0);
 	}
 
 	spin_lock_irqsave(&mx3_cam->lock, flags);
@@ -421,8 +428,8 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 	mx3_cam->active = NULL;
 
 	list_for_each_entry_safe(buf, tmp, &mx3_cam->capture, queue) {
-		buf->state = CSI_BUF_NEEDS_INIT;
 		list_del_init(&buf->queue);
+		vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
 	}
 
 	spin_unlock_irqrestore(&mx3_cam->lock, flags);
@@ -432,7 +439,6 @@ static int mx3_stop_streaming(struct vb2_queue *q)
 
 static struct vb2_ops mx3_videobuf_ops = {
 	.queue_setup	= mx3_videobuf_setup,
-	.buf_prepare	= mx3_videobuf_prepare,
 	.buf_queue	= mx3_videobuf_queue,
 	.buf_cleanup	= mx3_videobuf_release,
 	.buf_init	= mx3_videobuf_init,
@@ -516,6 +522,7 @@ static int mx3_camera_add_device(struct soc_camera_device *icd)
 
 	mx3_camera_activate(mx3_cam, icd);
 
+	mx3_cam->buf_total = 0;
 	mx3_cam->icd = icd;
 
 	dev_info(icd->parent, "MX3 Camera driver attached to camera %d\n",
@@ -1263,8 +1270,6 @@ static int __devexit mx3_camera_remove(struct platform_device *pdev)
 
 	dmaengine_put();
 
-	dev_info(&pdev->dev, "i.MX3x Camera driver unloaded\n");
-
 	return 0;
 }
 
-- 
1.7.2.5

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

* Re: [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
  2011-08-25 16:46     ` Guennadi Liakhovetski
@ 2011-08-25 16:57       ` Laurent Pinchart
  -1 siblings, 0 replies; 27+ messages in thread
From: Laurent Pinchart @ 2011-08-25 16:57 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Hans Verkuil, Pawel Osciak,
	Sakari Ailus, Mauro Carvalho Chehab, Marek Szyprowski,
	linux-arm-kernel, linux-kernel, Vinod Koul, Dan Williams,
	Sascha Hauer

Hi Guennadi,

On Thursday 25 August 2011 18:46:03 Guennadi Liakhovetski wrote:
> Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and
> VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be
> able to handle buffer sizes, provided by the caller, and the
> .buf_prepare() operation must not use the currently configured frame
> format for its operation, which makes it superfluous for this driver.
> Its functionality is moved into .buf_queue().

You're moving the ichan->dma_chan.device->device_prep_slave_sg() call from 
.buf_prepare() to .buf_queue(). Is that call cheap ? Otherwise it would be 
better to keep the .buf_prepare() callback.

-- 
Regards,

Laurent Pinchart

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

* [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
@ 2011-08-25 16:57       ` Laurent Pinchart
  0 siblings, 0 replies; 27+ messages in thread
From: Laurent Pinchart @ 2011-08-25 16:57 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Guennadi,

On Thursday 25 August 2011 18:46:03 Guennadi Liakhovetski wrote:
> Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and
> VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be
> able to handle buffer sizes, provided by the caller, and the
> .buf_prepare() operation must not use the currently configured frame
> format for its operation, which makes it superfluous for this driver.
> Its functionality is moved into .buf_queue().

You're moving the ichan->dma_chan.device->device_prep_slave_sg() call from 
.buf_prepare() to .buf_queue(). Is that call cheap ? Otherwise it would be 
better to keep the .buf_prepare() callback.

-- 
Regards,

Laurent Pinchart

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

* Re: [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
  2011-08-25 16:57       ` Laurent Pinchart
@ 2011-08-25 23:07         ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 23:07 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Linux Media Mailing List, Hans Verkuil, Pawel Osciak,
	Sakari Ailus, Mauro Carvalho Chehab, Marek Szyprowski,
	linux-arm-kernel, linux-kernel, Vinod Koul, Dan Williams,
	Sascha Hauer

On Thu, 25 Aug 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Thursday 25 August 2011 18:46:03 Guennadi Liakhovetski wrote:
> > Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and
> > VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be
> > able to handle buffer sizes, provided by the caller, and the
> > .buf_prepare() operation must not use the currently configured frame
> > format for its operation, which makes it superfluous for this driver.
> > Its functionality is moved into .buf_queue().
> 
> You're moving the ichan->dma_chan.device->device_prep_slave_sg() call from 
> .buf_prepare() to .buf_queue(). Is that call cheap ? Otherwise it would be 
> better to keep the .buf_prepare() callback.

But only if (buf->state == CSI_BUF_NEEDS_INIT), i.e., only on the first 
invocation. In any case, look at idmac_prep_slave_sg() - it is cheap. To 
do this in .buf_prepare I'd have to store the frame format from the 
.queue_setup() with a list of indices, to which it applies, and then use 
it in .buf_prepare()...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers
@ 2011-08-25 23:07         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-25 23:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 25 Aug 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Thursday 25 August 2011 18:46:03 Guennadi Liakhovetski wrote:
> > Prepare the mx3_camera friver to support the new VIDIOC_CREATE_BUFS and
> > VIDIOC_PREPARE_BUF ioctl()s. The .queue_setup() vb2 operation must be
> > able to handle buffer sizes, provided by the caller, and the
> > .buf_prepare() operation must not use the currently configured frame
> > format for its operation, which makes it superfluous for this driver.
> > Its functionality is moved into .buf_queue().
> 
> You're moving the ichan->dma_chan.device->device_prep_slave_sg() call from 
> .buf_prepare() to .buf_queue(). Is that call cheap ? Otherwise it would be 
> better to keep the .buf_prepare() callback.

But only if (buf->state == CSI_BUF_NEEDS_INIT), i.e., only on the first 
invocation. In any case, look at idmac_prep_slave_sg() - it is cheap. To 
do this in .buf_prepare I'd have to store the frame format from the 
.queue_setup() with a list of indices, to which it applies, and then use 
it in .buf_prepare()...

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
  2011-08-24 18:41 ` [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue Guennadi Liakhovetski
@ 2011-08-28 19:29   ` Pawel Osciak
  2011-08-29  8:30     ` Guennadi Liakhovetski
  0 siblings, 1 reply; 27+ messages in thread
From: Pawel Osciak @ 2011-08-28 19:29 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Hans Verkuil, Laurent Pinchart,
	Sakari Ailus, Mauro Carvalho Chehab, Marek Szyprowski

Hi Guennadi,

On Wed, Aug 24, 2011 at 11:41, Guennadi Liakhovetski
<g.liakhovetski@gmx.de> wrote:
> The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
> allow user-space applications to allocate video buffers of different
> sizes and hand them over to the driver for fast switching between
> different frame formats. This patch adds support for buffers of different
> sizes on the same buffer-queue to vb2.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Cc: Hans Verkuil <hverkuil@xs4all.nl>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> Cc: Pawel Osciak <pawel@osciak.com>
> Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> ---
>  drivers/media/video/videobuf2-core.c |  278 ++++++++++++++++++++++++++++------
>  include/media/videobuf2-core.h       |   31 +++--
>  2 files changed, 252 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 8a81a89..fed6f2d 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c

<snip>

> @@ -454,7 +465,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
>  */
>  int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>  {
> -       unsigned int num_buffers, num_planes;
> +       unsigned int num_buffers, allocated_buffers, num_planes = 0;
>        unsigned long plane_sizes[VIDEO_MAX_PLANES];
>        int ret = 0;
>
> @@ -503,7 +514,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>                        return -EBUSY;
>                }
>
> -               __vb2_queue_free(q);
> +               __vb2_queue_free(q, q->num_buffers);
>
>                /*
>                 * In case of REQBUFS(0) return immediately without calling
> @@ -538,44 +549,166 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>                return -ENOMEM;
>        }
>
> +       allocated_buffers = ret;
> +
>        /*
>         * Check if driver can handle the allocated number of buffers.
>         */
> -       if (ret < num_buffers) {
> -               unsigned int orig_num_buffers;
> +       if (allocated_buffers < num_buffers) {
> +               num_buffers = allocated_buffers;
>
> -               orig_num_buffers = num_buffers = ret;
>                ret = call_qop(q, queue_setup, q, NULL, &num_buffers,
>                               &num_planes, plane_sizes, q->alloc_ctx);
> -               if (ret)
> -                       goto free_mem;
>
> -               if (orig_num_buffers < num_buffers) {
> +               if (!ret && allocated_buffers < num_buffers)
>                        ret = -ENOMEM;
> -                       goto free_mem;
> -               }
>
>                /*
> -                * Ok, driver accepted smaller number of buffers.
> +                * Either the driver has accepted a smaller number of buffers,
> +                * or .queue_setup() returned an error
>                 */
> -               ret = num_buffers;
> +       }
> +
> +       q->num_buffers = allocated_buffers;
> +
> +       if (ret < 0) {
> +               __vb2_queue_free(q, allocated_buffers);
> +               return ret;
>        }
>
>        /*
>         * Return the number of successfully allocated buffers
>         * to the userspace.
>         */
> -       req->count = ret;
> +       req->count = allocated_buffers;
>
>        return 0;
> -
> -free_mem:
> -       __vb2_queue_free(q);
> -       return ret;
>  }
>  EXPORT_SYMBOL_GPL(vb2_reqbufs);
>
>  /**
> + * vb2_create_bufs() - Allocate buffers and any required auxiliary structs
> + * @q:         videobuf2 queue
> + * @create:    creation parameters, passed from userspace to vidioc_create_bufs
> + *             handler in driver
> + *
> + * Should be called from vidioc_create_bufs ioctl handler of a driver.
> + * This function:
> + * 1) verifies parameter sanity
> + * 2) calls the .queue_setup() queue operation
> + * 3) performs any necessary memory allocations
> + *
> + * The return values from this function are intended to be directly returned
> + * from vidioc_create_bufs handler in driver.
> + */
> +int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> +{
> +       unsigned int num_planes, num_buffers = create->count, allocated_buffers;
> +       unsigned long plane_sizes[VIDEO_MAX_PLANES];
> +       int ret = 0;
> +
> +       if (q->fileio) {
> +               dprintk(1, "%s(): file io in progress\n", __func__);
> +               return -EBUSY;
> +       }
> +
> +       if (create->memory != V4L2_MEMORY_MMAP
> +                       && create->memory != V4L2_MEMORY_USERPTR) {
> +               dprintk(1, "%s(): unsupported memory type\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (create->format.type != q->type) {
> +               dprintk(1, "%s(): requested type is incorrect\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       /*
> +        * Make sure all the required memory ops for given memory type
> +        * are available.
> +        */
> +       if (create->memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
> +               dprintk(1, "%s(): MMAP for current setup unsupported\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (create->memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
> +               dprintk(1, "%s(): USERPTR for current setup unsupported\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (q->num_buffers == VIDEO_MAX_FRAME) {
> +               dprintk(1, "%s(): maximum number of buffers already allocated\n",
> +                       __func__);
> +               return -ENOBUFS;
> +       }

I think we should be verifying that q->num_buffers + create->count <=
VIDEO_MAX_FRAME.

> +
> +       create->index = q->num_buffers;
> +
> +       if (!q->num_buffers) {
> +               memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> +               q->memory = create->memory;
> +       }
> +
> +       /*
> +        * Ask the driver, whether the requested number of buffers, planes per
> +        * buffer and their sizes are acceptable
> +        */
> +       ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
> +                      &num_planes, plane_sizes, q->alloc_ctx);

I don't see you zeroing neither num_planes nor plane_sizes[] in
vb2_create_bufs and vb2_reqbufs. Since instead of always requiring the
driver to fill them, you've introduced the "non-zero num_planes and/or
plane_sizes" case (see my comment for queue_setup() documentation in
videobuf2-core.h), it looks to me that the drivers will be getting
random values in num_planes and plane_sizes in queue_setup() and will
have to attempt to use them. Ditto for all other qop calls to
queue_setup in this file (in vb2_reqbufs as well).

> +       if (ret)
> +               return ret;
> +
> +       /* Finally, allocate buffers and video memory */
> +       ret = __vb2_queue_alloc(q, create->memory, num_buffers,
> +                               num_planes, plane_sizes);
> +       if (ret < 0) {
> +               dprintk(1, "Memory allocation failed with error: %d\n", ret);
> +               return ret;
> +       }
> +
> +       allocated_buffers = ret;
> +
> +       /*
> +        * Check if driver can handle the so far allocated number of buffers.
> +        */
> +       if (ret < num_buffers) {
> +               num_buffers = ret;
> +
> +               /*
> +                * q->num_buffers contains the total number of buffers, that the
> +                * queue driver has set up
> +                */
> +               ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
> +                              &num_planes, plane_sizes, q->alloc_ctx);

We need to be sure that drivers understand that num_buffers is _in
addition_ to previous allocations if fmt!=NULL (see my comments for
videobuf2-core.h below).

> +
> +               if (!ret && allocated_buffers < num_buffers)
> +                       ret = -ENOMEM;
> +
> +               /*
> +                * Either the driver has accepted a smaller number of buffers,
> +                * or .queue_setup() returned an error
> +                */
> +       }
> +
> +       q->num_buffers += allocated_buffers;
> +
> +       if (ret < 0) {
> +               __vb2_queue_free(q, allocated_buffers);
> +               return ret;
> +       }
> +
> +       /*
> +        * Return the number of successfully allocated buffers
> +        * to the userspace.
> +        */
> +       create->count = allocated_buffers;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_create_bufs);
> +

<snip>

>  /**
> + * vb2_prepare_buf() - Pass ownership of a buffer from userspace to the kernel
> + * @q:         videobuf2 queue
> + * @b:         buffer structure passed from userspace to vidioc_prepare_buf
> + *             handler in driver
> + *
> + * Should be called from vidioc_prepare_buf ioctl handler of a driver.
> + * This function:
> + * 1) verifies the passed buffer,
> + * 2) calls buf_prepare callback in the driver (if provided), in which
> + *    driver-specific buffer initialization can be performed,
> + *
> + * The return values from this function are intended to be directly returned
> + * from vidioc_prepare_buf handler in driver.
> + */
> +int vb2_prepare_buf(struct vb2_queue *q, const struct v4l2_buffer *b)
> +{
> +       struct vb2_buffer *vb;
> +
> +       if (q->fileio) {
> +               dprintk(1, "%s(): file io in progress\n", __func__);
> +               return -EBUSY;
> +       }
> +
> +       if (b->type != q->type) {
> +               dprintk(1, "%s(): invalid buffer type\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (b->index >= q->num_buffers) {
> +               dprintk(1, "%s(): buffer index out of range\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       vb = q->bufs[b->index];
> +       if (NULL == vb) {
> +               /* Should never happen */
> +               dprintk(1, "%s(): buffer is NULL\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (b->memory != q->memory) {
> +               dprintk(1, "%s(): invalid memory type\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> +               dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state);
> +               return -EINVAL;
> +       }
> +
> +       return __buf_prepare(vb, b);
> +}
> +EXPORT_SYMBOL_GPL(vb2_prepare_buf);
> +

I don't see vb->state being set to VB2_BUF_STATE_PREPARED anywhere...
Shouldn't it be done here? Otherwise we'll be calling buf_prepare
again on qbuf...

<snip>

> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index d043132..ddebcd3 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -172,13 +172,17 @@ struct vb2_buffer {
>  /**
>  * struct vb2_ops - driver-specific callbacks
>  *
> - * @queue_setup:       called from a VIDIOC_REQBUFS handler, before
> - *                     memory allocation; driver should return the required
> - *                     number of buffers in num_buffers, the required number
> - *                     of planes per buffer in num_planes; the size of each
> - *                     plane should be set in the sizes[] array and optional
> - *                     per-plane allocator specific context in alloc_ctxs[]
> - *                     array
> + * @queue_setup:       called from VIDIOC_REQBUFS and VIDIOC_CREATE_BUFS
> + *                     handlers, before memory allocation. When called with
> + *                     zeroed num_planes or plane sizes, the driver should
> + *                     return the required number of buffers in num_buffers,
> + *                     the required number of planes per buffer in num_planes;
> + *                     the size of each plane should be set in the sizes[]
> + *                     array and optional per-plane allocator specific context
> + *                     in alloc_ctxs[] array. If num_planes and sizes[] are
> + *                     both non-zero, the driver should use them. Otherwise the
> + *                     driver must make no assumptions about the buffers, that
> + *                     will be made available to it.


Could you explain why different behavior from a driver is required
depending on the values of num_planes and plane_sizes? Are you
expecting vb2 to fill num_planes and plane_sizes? That would require
it to parse the format...

Another subtle problem here is that on vb2_reqbufs, we call
queue_setup() asking the driver whether it can manage the allocated
number of buffers. Now with queue_setup being called from
vb2_create_bufs as well, we need to make sure that the driver
understands that num_buffers passed then is only for buffers of the
format passed to create. In other words, if fmt==NULL, driver must be
able to handle num_buffers buffers of current format; else if fmt !=
NULL, the driver must be able to handle any previously allocated
buffers _plus_ the number passed to this queue_setup call. This not
only means last call to queue_setup with fmt==NULL plus current call,
but previous call with fmt==NULL _plus_ all previous calls with
fmt!=NULL _plus_ the current call.

<snip>

Could you share the userspace code that you used for testing this? I
just wanted to get a feel of how those new ioctls fall into place
together. Also, did you try multiple CREATE_BUFS calls?

-- 
Best regards,
Pawel Osciak

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

* Re: [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
  2011-08-28 19:29   ` Pawel Osciak
@ 2011-08-29  8:30     ` Guennadi Liakhovetski
  2011-09-01 14:17       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-29  8:30 UTC (permalink / raw)
  To: Pawel Osciak
  Cc: Linux Media Mailing List, Hans Verkuil, Laurent Pinchart,
	Sakari Ailus, Mauro Carvalho Chehab, Marek Szyprowski

Hi Pawel

Thanks for the review.

On Sun, 28 Aug 2011, Pawel Osciak wrote:

> Hi Guennadi,
> 
> On Wed, Aug 24, 2011 at 11:41, Guennadi Liakhovetski
> <g.liakhovetski@gmx.de> wrote:
> > The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
> > allow user-space applications to allocate video buffers of different
> > sizes and hand them over to the driver for fast switching between
> > different frame formats. This patch adds support for buffers of different
> > sizes on the same buffer-queue to vb2.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > Cc: Hans Verkuil <hverkuil@xs4all.nl>
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> > Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> > Cc: Pawel Osciak <pawel@osciak.com>
> > Cc: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
> > ---
> >  drivers/media/video/videobuf2-core.c |  278 ++++++++++++++++++++++++++++------
> >  include/media/videobuf2-core.h       |   31 +++--
> >  2 files changed, 252 insertions(+), 57 deletions(-)
> >
> > diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> > index 8a81a89..fed6f2d 100644
> > --- a/drivers/media/video/videobuf2-core.c
> > +++ b/drivers/media/video/videobuf2-core.c

[snip]

> >  /**
> > + * vb2_create_bufs() - Allocate buffers and any required auxiliary structs
> > + * @q:         videobuf2 queue
> > + * @create:    creation parameters, passed from userspace to vidioc_create_bufs
> > + *             handler in driver
> > + *
> > + * Should be called from vidioc_create_bufs ioctl handler of a driver.
> > + * This function:
> > + * 1) verifies parameter sanity
> > + * 2) calls the .queue_setup() queue operation
> > + * 3) performs any necessary memory allocations
> > + *
> > + * The return values from this function are intended to be directly returned
> > + * from vidioc_create_bufs handler in driver.
> > + */
> > +int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
> > +{
> > +       unsigned int num_planes, num_buffers = create->count, allocated_buffers;
> > +       unsigned long plane_sizes[VIDEO_MAX_PLANES];
> > +       int ret = 0;
> > +
> > +       if (q->fileio) {
> > +               dprintk(1, "%s(): file io in progress\n", __func__);
> > +               return -EBUSY;
> > +       }
> > +
> > +       if (create->memory != V4L2_MEMORY_MMAP
> > +                       && create->memory != V4L2_MEMORY_USERPTR) {
> > +               dprintk(1, "%s(): unsupported memory type\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (create->format.type != q->type) {
> > +               dprintk(1, "%s(): requested type is incorrect\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       /*
> > +        * Make sure all the required memory ops for given memory type
> > +        * are available.
> > +        */
> > +       if (create->memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
> > +               dprintk(1, "%s(): MMAP for current setup unsupported\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (create->memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
> > +               dprintk(1, "%s(): USERPTR for current setup unsupported\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (q->num_buffers == VIDEO_MAX_FRAME) {
> > +               dprintk(1, "%s(): maximum number of buffers already allocated\n",
> > +                       __func__);
> > +               return -ENOBUFS;
> > +       }
> 
> I think we should be verifying that q->num_buffers + create->count <=
> VIDEO_MAX_FRAME.

Yeah, something like that, thanks. Some of us didn't like this 
VIDEO_MAX_FRAME limit at all, but as long as we have it, e-g-. as long as 
struct vb2_queue::bufs[] is a fixed-size array with exactly that many 
elements, we have to check for it. Increasing / eliminating it could be a 
separate patch.

> 
> > +
> > +       create->index = q->num_buffers;
> > +
> > +       if (!q->num_buffers) {
> > +               memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
> > +               q->memory = create->memory;
> > +       }
> > +
> > +       /*
> > +        * Ask the driver, whether the requested number of buffers, planes per
> > +        * buffer and their sizes are acceptable
> > +        */
> > +       ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
> > +                      &num_planes, plane_sizes, q->alloc_ctx);
> 
> I don't see you zeroing neither num_planes nor plane_sizes[] in
> vb2_create_bufs and vb2_reqbufs. Since instead of always requiring the
> driver to fill them, you've introduced the "non-zero num_planes and/or
> plane_sizes" case (see my comment for queue_setup() documentation in
> videobuf2-core.h), it looks to me that the drivers will be getting
> random values in num_planes and plane_sizes in queue_setup() and will
> have to attempt to use them. Ditto for all other qop calls to
> queue_setup in this file (in vb2_reqbufs as well).

No, it's the documentation that's wrong:-) Both num_planes and plane_sizes 
have always been pure output parameters of .queue_setup(), so, I don't 
think there's a need to initialise them. Will have to update the comment.

> 
> > +       if (ret)
> > +               return ret;
> > +
> > +       /* Finally, allocate buffers and video memory */
> > +       ret = __vb2_queue_alloc(q, create->memory, num_buffers,
> > +                               num_planes, plane_sizes);
> > +       if (ret < 0) {
> > +               dprintk(1, "Memory allocation failed with error: %d\n", ret);
> > +               return ret;
> > +       }
> > +
> > +       allocated_buffers = ret;
> > +
> > +       /*
> > +        * Check if driver can handle the so far allocated number of buffers.
> > +        */
> > +       if (ret < num_buffers) {
> > +               num_buffers = ret;
> > +
> > +               /*
> > +                * q->num_buffers contains the total number of buffers, that the
> > +                * queue driver has set up
> > +                */
> > +               ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
> > +                              &num_planes, plane_sizes, q->alloc_ctx);
> 
> We need to be sure that drivers understand that num_buffers is _in
> addition_ to previous allocations if fmt!=NULL (see my comments for
> videobuf2-core.h below).

Hm, no. If they really need to know that, they have to look at 
q->num_buffers. And yes, I think both my sh-mobile-ceu-camera and 
mx3-camera implementations do have a problem with this, thanks for 
pointing out.

> > +
> > +               if (!ret && allocated_buffers < num_buffers)
> > +                       ret = -ENOMEM;
> > +
> > +               /*
> > +                * Either the driver has accepted a smaller number of buffers,
> > +                * or .queue_setup() returned an error
> > +                */
> > +       }
> > +
> > +       q->num_buffers += allocated_buffers;
> > +
> > +       if (ret < 0) {
> > +               __vb2_queue_free(q, allocated_buffers);
> > +               return ret;
> > +       }
> > +
> > +       /*
> > +        * Return the number of successfully allocated buffers
> > +        * to the userspace.
> > +        */
> > +       create->count = allocated_buffers;
> > +
> > +       return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(vb2_create_bufs);
> > +
> 
> <snip>
> 
> >  /**
> > + * vb2_prepare_buf() - Pass ownership of a buffer from userspace to the kernel
> > + * @q:         videobuf2 queue
> > + * @b:         buffer structure passed from userspace to vidioc_prepare_buf
> > + *             handler in driver
> > + *
> > + * Should be called from vidioc_prepare_buf ioctl handler of a driver.
> > + * This function:
> > + * 1) verifies the passed buffer,
> > + * 2) calls buf_prepare callback in the driver (if provided), in which
> > + *    driver-specific buffer initialization can be performed,
> > + *
> > + * The return values from this function are intended to be directly returned
> > + * from vidioc_prepare_buf handler in driver.
> > + */
> > +int vb2_prepare_buf(struct vb2_queue *q, const struct v4l2_buffer *b)
> > +{
> > +       struct vb2_buffer *vb;
> > +
> > +       if (q->fileio) {
> > +               dprintk(1, "%s(): file io in progress\n", __func__);
> > +               return -EBUSY;
> > +       }
> > +
> > +       if (b->type != q->type) {
> > +               dprintk(1, "%s(): invalid buffer type\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (b->index >= q->num_buffers) {
> > +               dprintk(1, "%s(): buffer index out of range\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       vb = q->bufs[b->index];
> > +       if (NULL == vb) {
> > +               /* Should never happen */
> > +               dprintk(1, "%s(): buffer is NULL\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (b->memory != q->memory) {
> > +               dprintk(1, "%s(): invalid memory type\n", __func__);
> > +               return -EINVAL;
> > +       }
> > +
> > +       if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> > +               dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state);
> > +               return -EINVAL;
> > +       }
> > +
> > +       return __buf_prepare(vb, b);
> > +}
> > +EXPORT_SYMBOL_GPL(vb2_prepare_buf);
> > +
> 
> I don't see vb->state being set to VB2_BUF_STATE_PREPARED anywhere...
> Shouldn't it be done here? Otherwise we'll be calling buf_prepare
> again on qbuf...

Yes, you're right, it seems to have got lost in multiple revisions:-( But 
in fact it should be fixed in the earlier "V4L: add a new videobuf2 buffer 
state VB2_BUF_STATE_PREPARED" patch.

> <snip>
> 
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index d043132..ddebcd3 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -172,13 +172,17 @@ struct vb2_buffer {
> >  /**
> >  * struct vb2_ops - driver-specific callbacks
> >  *
> > - * @queue_setup:       called from a VIDIOC_REQBUFS handler, before
> > - *                     memory allocation; driver should return the required
> > - *                     number of buffers in num_buffers, the required number
> > - *                     of planes per buffer in num_planes; the size of each
> > - *                     plane should be set in the sizes[] array and optional
> > - *                     per-plane allocator specific context in alloc_ctxs[]
> > - *                     array
> > + * @queue_setup:       called from VIDIOC_REQBUFS and VIDIOC_CREATE_BUFS
> > + *                     handlers, before memory allocation. When called with
> > + *                     zeroed num_planes or plane sizes, the driver should
> > + *                     return the required number of buffers in num_buffers,
> > + *                     the required number of planes per buffer in num_planes;
> > + *                     the size of each plane should be set in the sizes[]
> > + *                     array and optional per-plane allocator specific context
> > + *                     in alloc_ctxs[] array. If num_planes and sizes[] are
> > + *                     both non-zero, the driver should use them. Otherwise the
> > + *                     driver must make no assumptions about the buffers, that
> > + *                     will be made available to it.
> 
> 
> Could you explain why different behavior from a driver is required
> depending on the values of num_planes and plane_sizes? Are you
> expecting vb2 to fill num_planes and plane_sizes? That would require
> it to parse the format...

As I said, that's outdated documentation, will fix.

> Another subtle problem here is that on vb2_reqbufs, we call
> queue_setup() asking the driver whether it can manage the allocated
> number of buffers. Now with queue_setup being called from
> vb2_create_bufs as well, we need to make sure that the driver
> understands that num_buffers passed then is only for buffers of the
> format passed to create. In other words, if fmt==NULL, driver must be
> able to handle num_buffers buffers of current format; else if fmt !=
> NULL, the driver must be able to handle any previously allocated
> buffers _plus_ the number passed to this queue_setup call. This not
> only means last call to queue_setup with fmt==NULL plus current call,
> but previous call with fmt==NULL _plus_ all previous calls with
> fmt!=NULL _plus_ the current call.

Yes, exactly. .queue_setup() has 3 modes of operation:

(1) when called from vb2_reqbufs() (fmt == NULL): only num_buffers of the 
currently configured format have to be set up. Both num_planes and 
plane_sizes can be considered purely output.

(2) first call from vb2_create_bufs() (fmt != NULL): new buffers have to 
be added to q->num_buffers of whatever formats, allocated up to now. After 
this, in case of the V4L2_MEMORY_MMAP memory, .buf_init() will be called 
for each new buffer.

(3) second call from vb2_create_bufs() (fmt != NULL): verify, that the 
num_buffers in addition to q->num_buffers is also ok. In case of 
V4L2_MEMORY_MMAP, those num_buffers are already allocated and initialised, 
we have to check, that that's enough.

And yes, it seems, checking for *num_planes == 0 is a simple way to 
differentiate between (2) and (3) in the driver.

> <snip>
> 
> Could you share the userspace code that you used for testing this? I
> just wanted to get a feel of how those new ioctls fall into place
> together.

Theoretically - yes, just will have to clean up the code a bit;-) Will try 
to find some time for this in the next couple of days.

> Also, did you try multiple CREATE_BUFS calls?

Yes, it currently does two CREATE_BUFS, I might try to mix REQBUFS with 
CREATE_BUFS too though.

Thanks for the comments!
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
  2011-08-25 16:45     ` Guennadi Liakhovetski
@ 2011-08-29 15:21       ` Vinod Koul
  -1 siblings, 0 replies; 27+ messages in thread
From: Vinod Koul @ 2011-08-29 15:21 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux Media Mailing List, Sakari Ailus, Pawel Osciak,
	linux-kernel, Mauro Carvalho Chehab, Hans Verkuil,
	Laurent Pinchart, Sascha Hauer, Dan Williams, linux-arm-kernel,
	Marek Szyprowski

On Thu, 2011-08-25 at 18:45 +0200, Guennadi Liakhovetski wrote:
> To support multi-size buffers in the mx3_camera V4L2 driver we have to be
> able to stop DMA on a channel without releasing descriptors and completely
> halting the hardware. Use the DMA_PAUSE control to implement this mode.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Acked-by Vinod Koul <vinod.koul@linux.intel.com>

Do you want this to go thru slave-dma or media tree?

-- 
~Vinod
> ---
>  drivers/dma/ipu/ipu_idmac.c |   65 +++++++++++++++++++++++++++---------------
>  1 files changed, 42 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> index c1a125e..42cdf1c 100644
> --- a/drivers/dma/ipu/ipu_idmac.c
> +++ b/drivers/dma/ipu/ipu_idmac.c
> @@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
>  	    ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) {
>  		callback = descnew->txd.callback;
>  		callback_param = descnew->txd.callback_param;
> +		list_del_init(&descnew->list);
>  		spin_unlock(&ichan->lock);
>  		if (callback)
>  			callback(callback_param);
> @@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  {
>  	struct idmac_channel *ichan = to_idmac_chan(chan);
>  	struct idmac *idmac = to_idmac(chan->device);
> +	struct ipu *ipu = to_ipu(idmac);
> +	struct list_head *list, *tmp;
>  	unsigned long flags;
>  	int i;
>  
> -	/* Only supports DMA_TERMINATE_ALL */
> -	if (cmd != DMA_TERMINATE_ALL)
> -		return -ENXIO;
> +	switch (cmd) {
> +	case DMA_PAUSE:
> +		spin_lock_irqsave(&ipu->lock, flags);
> +		ipu_ic_disable_task(ipu, chan->chan_id);
>  
> -	ipu_disable_channel(idmac, ichan,
> -			    ichan->status >= IPU_CHANNEL_ENABLED);
> +		/* Return all descriptors into "prepared" state */
> +		list_for_each_safe(list, tmp, &ichan->queue)
> +			list_del_init(list);
>  
> -	tasklet_disable(&to_ipu(idmac)->tasklet);
> +		ichan->sg[0] = NULL;
> +		ichan->sg[1] = NULL;
>  
> -	/* ichan->queue is modified in ISR, have to spinlock */
> -	spin_lock_irqsave(&ichan->lock, flags);
> -	list_splice_init(&ichan->queue, &ichan->free_list);
> +		spin_unlock_irqrestore(&ipu->lock, flags);
>  
> -	if (ichan->desc)
> -		for (i = 0; i < ichan->n_tx_desc; i++) {
> -			struct idmac_tx_desc *desc = ichan->desc + i;
> -			if (list_empty(&desc->list))
> -				/* Descriptor was prepared, but not submitted */
> -				list_add(&desc->list, &ichan->free_list);
> +		ichan->status = IPU_CHANNEL_INITIALIZED;
> +		break;
> +	case DMA_TERMINATE_ALL:
> +		ipu_disable_channel(idmac, ichan,
> +				    ichan->status >= IPU_CHANNEL_ENABLED);
>  
> -			async_tx_clear_ack(&desc->txd);
> -		}
> +		tasklet_disable(&ipu->tasklet);
>  
> -	ichan->sg[0] = NULL;
> -	ichan->sg[1] = NULL;
> -	spin_unlock_irqrestore(&ichan->lock, flags);
> +		/* ichan->queue is modified in ISR, have to spinlock */
> +		spin_lock_irqsave(&ichan->lock, flags);
> +		list_splice_init(&ichan->queue, &ichan->free_list);
>  
> -	tasklet_enable(&to_ipu(idmac)->tasklet);
> +		if (ichan->desc)
> +			for (i = 0; i < ichan->n_tx_desc; i++) {
> +				struct idmac_tx_desc *desc = ichan->desc + i;
> +				if (list_empty(&desc->list))
> +					/* Descriptor was prepared, but not submitted */
> +					list_add(&desc->list, &ichan->free_list);
>  
> -	ichan->status = IPU_CHANNEL_INITIALIZED;
> +				async_tx_clear_ack(&desc->txd);
> +			}
> +
> +		ichan->sg[0] = NULL;
> +		ichan->sg[1] = NULL;
> +		spin_unlock_irqrestore(&ichan->lock, flags);
> +
> +		tasklet_enable(&ipu->tasklet);
> +
> +		ichan->status = IPU_CHANNEL_INITIALIZED;
> +		break;
> +	default:
> +		return -ENOSYS;
> +	}
>  
>  	return 0;
>  }
> @@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu)
>  		struct idmac_channel *ichan = ipu->channel + i;
>  
>  		idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0);
> -		idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0);
>  	}
>  
>  	dma_async_device_unregister(&idmac->dma);




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

* [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
@ 2011-08-29 15:21       ` Vinod Koul
  0 siblings, 0 replies; 27+ messages in thread
From: Vinod Koul @ 2011-08-29 15:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 2011-08-25 at 18:45 +0200, Guennadi Liakhovetski wrote:
> To support multi-size buffers in the mx3_camera V4L2 driver we have to be
> able to stop DMA on a channel without releasing descriptors and completely
> halting the hardware. Use the DMA_PAUSE control to implement this mode.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Acked-by Vinod Koul <vinod.koul@linux.intel.com>

Do you want this to go thru slave-dma or media tree?

-- 
~Vinod
> ---
>  drivers/dma/ipu/ipu_idmac.c |   65 +++++++++++++++++++++++++++---------------
>  1 files changed, 42 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> index c1a125e..42cdf1c 100644
> --- a/drivers/dma/ipu/ipu_idmac.c
> +++ b/drivers/dma/ipu/ipu_idmac.c
> @@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
>  	    ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) {
>  		callback = descnew->txd.callback;
>  		callback_param = descnew->txd.callback_param;
> +		list_del_init(&descnew->list);
>  		spin_unlock(&ichan->lock);
>  		if (callback)
>  			callback(callback_param);
> @@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>  {
>  	struct idmac_channel *ichan = to_idmac_chan(chan);
>  	struct idmac *idmac = to_idmac(chan->device);
> +	struct ipu *ipu = to_ipu(idmac);
> +	struct list_head *list, *tmp;
>  	unsigned long flags;
>  	int i;
>  
> -	/* Only supports DMA_TERMINATE_ALL */
> -	if (cmd != DMA_TERMINATE_ALL)
> -		return -ENXIO;
> +	switch (cmd) {
> +	case DMA_PAUSE:
> +		spin_lock_irqsave(&ipu->lock, flags);
> +		ipu_ic_disable_task(ipu, chan->chan_id);
>  
> -	ipu_disable_channel(idmac, ichan,
> -			    ichan->status >= IPU_CHANNEL_ENABLED);
> +		/* Return all descriptors into "prepared" state */
> +		list_for_each_safe(list, tmp, &ichan->queue)
> +			list_del_init(list);
>  
> -	tasklet_disable(&to_ipu(idmac)->tasklet);
> +		ichan->sg[0] = NULL;
> +		ichan->sg[1] = NULL;
>  
> -	/* ichan->queue is modified in ISR, have to spinlock */
> -	spin_lock_irqsave(&ichan->lock, flags);
> -	list_splice_init(&ichan->queue, &ichan->free_list);
> +		spin_unlock_irqrestore(&ipu->lock, flags);
>  
> -	if (ichan->desc)
> -		for (i = 0; i < ichan->n_tx_desc; i++) {
> -			struct idmac_tx_desc *desc = ichan->desc + i;
> -			if (list_empty(&desc->list))
> -				/* Descriptor was prepared, but not submitted */
> -				list_add(&desc->list, &ichan->free_list);
> +		ichan->status = IPU_CHANNEL_INITIALIZED;
> +		break;
> +	case DMA_TERMINATE_ALL:
> +		ipu_disable_channel(idmac, ichan,
> +				    ichan->status >= IPU_CHANNEL_ENABLED);
>  
> -			async_tx_clear_ack(&desc->txd);
> -		}
> +		tasklet_disable(&ipu->tasklet);
>  
> -	ichan->sg[0] = NULL;
> -	ichan->sg[1] = NULL;
> -	spin_unlock_irqrestore(&ichan->lock, flags);
> +		/* ichan->queue is modified in ISR, have to spinlock */
> +		spin_lock_irqsave(&ichan->lock, flags);
> +		list_splice_init(&ichan->queue, &ichan->free_list);
>  
> -	tasklet_enable(&to_ipu(idmac)->tasklet);
> +		if (ichan->desc)
> +			for (i = 0; i < ichan->n_tx_desc; i++) {
> +				struct idmac_tx_desc *desc = ichan->desc + i;
> +				if (list_empty(&desc->list))
> +					/* Descriptor was prepared, but not submitted */
> +					list_add(&desc->list, &ichan->free_list);
>  
> -	ichan->status = IPU_CHANNEL_INITIALIZED;
> +				async_tx_clear_ack(&desc->txd);
> +			}
> +
> +		ichan->sg[0] = NULL;
> +		ichan->sg[1] = NULL;
> +		spin_unlock_irqrestore(&ichan->lock, flags);
> +
> +		tasklet_enable(&ipu->tasklet);
> +
> +		ichan->status = IPU_CHANNEL_INITIALIZED;
> +		break;
> +	default:
> +		return -ENOSYS;
> +	}
>  
>  	return 0;
>  }
> @@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu)
>  		struct idmac_channel *ichan = ipu->channel + i;
>  
>  		idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0);
> -		idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0);
>  	}
>  
>  	dma_async_device_unregister(&idmac->dma);

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

* Re: [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
  2011-08-29 15:21       ` Vinod Koul
@ 2011-08-29 17:50         ` Guennadi Liakhovetski
  -1 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-29 17:50 UTC (permalink / raw)
  To: Vinod Koul
  Cc: Linux Media Mailing List, Sakari Ailus, Pawel Osciak,
	linux-kernel, Mauro Carvalho Chehab, Hans Verkuil,
	Laurent Pinchart, Sascha Hauer, Dan Williams, linux-arm-kernel,
	Marek Szyprowski

On Mon, 29 Aug 2011, Vinod Koul wrote:

> On Thu, 2011-08-25 at 18:45 +0200, Guennadi Liakhovetski wrote:
> > To support multi-size buffers in the mx3_camera V4L2 driver we have to be
> > able to stop DMA on a channel without releasing descriptors and completely
> > halting the hardware. Use the DMA_PAUSE control to implement this mode.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Acked-by Vinod Koul <vinod.koul@linux.intel.com>
> 
> Do you want this to go thru slave-dma or media tree?

With your above ack I can pull it together with the final form of patch 
2/2, when it's ready, via media tree. 

Thanks
Guennadi

> 
> -- 
> ~Vinod
> > ---
> >  drivers/dma/ipu/ipu_idmac.c |   65 +++++++++++++++++++++++++++---------------
> >  1 files changed, 42 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> > index c1a125e..42cdf1c 100644
> > --- a/drivers/dma/ipu/ipu_idmac.c
> > +++ b/drivers/dma/ipu/ipu_idmac.c
> > @@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
> >  	    ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) {
> >  		callback = descnew->txd.callback;
> >  		callback_param = descnew->txd.callback_param;
> > +		list_del_init(&descnew->list);
> >  		spin_unlock(&ichan->lock);
> >  		if (callback)
> >  			callback(callback_param);
> > @@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> >  {
> >  	struct idmac_channel *ichan = to_idmac_chan(chan);
> >  	struct idmac *idmac = to_idmac(chan->device);
> > +	struct ipu *ipu = to_ipu(idmac);
> > +	struct list_head *list, *tmp;
> >  	unsigned long flags;
> >  	int i;
> >  
> > -	/* Only supports DMA_TERMINATE_ALL */
> > -	if (cmd != DMA_TERMINATE_ALL)
> > -		return -ENXIO;
> > +	switch (cmd) {
> > +	case DMA_PAUSE:
> > +		spin_lock_irqsave(&ipu->lock, flags);
> > +		ipu_ic_disable_task(ipu, chan->chan_id);
> >  
> > -	ipu_disable_channel(idmac, ichan,
> > -			    ichan->status >= IPU_CHANNEL_ENABLED);
> > +		/* Return all descriptors into "prepared" state */
> > +		list_for_each_safe(list, tmp, &ichan->queue)
> > +			list_del_init(list);
> >  
> > -	tasklet_disable(&to_ipu(idmac)->tasklet);
> > +		ichan->sg[0] = NULL;
> > +		ichan->sg[1] = NULL;
> >  
> > -	/* ichan->queue is modified in ISR, have to spinlock */
> > -	spin_lock_irqsave(&ichan->lock, flags);
> > -	list_splice_init(&ichan->queue, &ichan->free_list);
> > +		spin_unlock_irqrestore(&ipu->lock, flags);
> >  
> > -	if (ichan->desc)
> > -		for (i = 0; i < ichan->n_tx_desc; i++) {
> > -			struct idmac_tx_desc *desc = ichan->desc + i;
> > -			if (list_empty(&desc->list))
> > -				/* Descriptor was prepared, but not submitted */
> > -				list_add(&desc->list, &ichan->free_list);
> > +		ichan->status = IPU_CHANNEL_INITIALIZED;
> > +		break;
> > +	case DMA_TERMINATE_ALL:
> > +		ipu_disable_channel(idmac, ichan,
> > +				    ichan->status >= IPU_CHANNEL_ENABLED);
> >  
> > -			async_tx_clear_ack(&desc->txd);
> > -		}
> > +		tasklet_disable(&ipu->tasklet);
> >  
> > -	ichan->sg[0] = NULL;
> > -	ichan->sg[1] = NULL;
> > -	spin_unlock_irqrestore(&ichan->lock, flags);
> > +		/* ichan->queue is modified in ISR, have to spinlock */
> > +		spin_lock_irqsave(&ichan->lock, flags);
> > +		list_splice_init(&ichan->queue, &ichan->free_list);
> >  
> > -	tasklet_enable(&to_ipu(idmac)->tasklet);
> > +		if (ichan->desc)
> > +			for (i = 0; i < ichan->n_tx_desc; i++) {
> > +				struct idmac_tx_desc *desc = ichan->desc + i;
> > +				if (list_empty(&desc->list))
> > +					/* Descriptor was prepared, but not submitted */
> > +					list_add(&desc->list, &ichan->free_list);
> >  
> > -	ichan->status = IPU_CHANNEL_INITIALIZED;
> > +				async_tx_clear_ack(&desc->txd);
> > +			}
> > +
> > +		ichan->sg[0] = NULL;
> > +		ichan->sg[1] = NULL;
> > +		spin_unlock_irqrestore(&ichan->lock, flags);
> > +
> > +		tasklet_enable(&ipu->tasklet);
> > +
> > +		ichan->status = IPU_CHANNEL_INITIALIZED;
> > +		break;
> > +	default:
> > +		return -ENOSYS;
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu)
> >  		struct idmac_channel *ichan = ipu->channel + i;
> >  
> >  		idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0);
> > -		idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0);
> >  	}
> >  
> >  	dma_async_device_unregister(&idmac->dma);
> 
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control
@ 2011-08-29 17:50         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-08-29 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, 29 Aug 2011, Vinod Koul wrote:

> On Thu, 2011-08-25 at 18:45 +0200, Guennadi Liakhovetski wrote:
> > To support multi-size buffers in the mx3_camera V4L2 driver we have to be
> > able to stop DMA on a channel without releasing descriptors and completely
> > halting the hardware. Use the DMA_PAUSE control to implement this mode.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Acked-by Vinod Koul <vinod.koul@linux.intel.com>
> 
> Do you want this to go thru slave-dma or media tree?

With your above ack I can pull it together with the final form of patch 
2/2, when it's ready, via media tree. 

Thanks
Guennadi

> 
> -- 
> ~Vinod
> > ---
> >  drivers/dma/ipu/ipu_idmac.c |   65 +++++++++++++++++++++++++++---------------
> >  1 files changed, 42 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
> > index c1a125e..42cdf1c 100644
> > --- a/drivers/dma/ipu/ipu_idmac.c
> > +++ b/drivers/dma/ipu/ipu_idmac.c
> > @@ -1306,6 +1306,7 @@ static irqreturn_t idmac_interrupt(int irq, void *dev_id)
> >  	    ipu_submit_buffer(ichan, descnew, sgnew, ichan->active_buffer) < 0) {
> >  		callback = descnew->txd.callback;
> >  		callback_param = descnew->txd.callback_param;
> > +		list_del_init(&descnew->list);
> >  		spin_unlock(&ichan->lock);
> >  		if (callback)
> >  			callback(callback_param);
> > @@ -1427,39 +1428,58 @@ static int __idmac_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
> >  {
> >  	struct idmac_channel *ichan = to_idmac_chan(chan);
> >  	struct idmac *idmac = to_idmac(chan->device);
> > +	struct ipu *ipu = to_ipu(idmac);
> > +	struct list_head *list, *tmp;
> >  	unsigned long flags;
> >  	int i;
> >  
> > -	/* Only supports DMA_TERMINATE_ALL */
> > -	if (cmd != DMA_TERMINATE_ALL)
> > -		return -ENXIO;
> > +	switch (cmd) {
> > +	case DMA_PAUSE:
> > +		spin_lock_irqsave(&ipu->lock, flags);
> > +		ipu_ic_disable_task(ipu, chan->chan_id);
> >  
> > -	ipu_disable_channel(idmac, ichan,
> > -			    ichan->status >= IPU_CHANNEL_ENABLED);
> > +		/* Return all descriptors into "prepared" state */
> > +		list_for_each_safe(list, tmp, &ichan->queue)
> > +			list_del_init(list);
> >  
> > -	tasklet_disable(&to_ipu(idmac)->tasklet);
> > +		ichan->sg[0] = NULL;
> > +		ichan->sg[1] = NULL;
> >  
> > -	/* ichan->queue is modified in ISR, have to spinlock */
> > -	spin_lock_irqsave(&ichan->lock, flags);
> > -	list_splice_init(&ichan->queue, &ichan->free_list);
> > +		spin_unlock_irqrestore(&ipu->lock, flags);
> >  
> > -	if (ichan->desc)
> > -		for (i = 0; i < ichan->n_tx_desc; i++) {
> > -			struct idmac_tx_desc *desc = ichan->desc + i;
> > -			if (list_empty(&desc->list))
> > -				/* Descriptor was prepared, but not submitted */
> > -				list_add(&desc->list, &ichan->free_list);
> > +		ichan->status = IPU_CHANNEL_INITIALIZED;
> > +		break;
> > +	case DMA_TERMINATE_ALL:
> > +		ipu_disable_channel(idmac, ichan,
> > +				    ichan->status >= IPU_CHANNEL_ENABLED);
> >  
> > -			async_tx_clear_ack(&desc->txd);
> > -		}
> > +		tasklet_disable(&ipu->tasklet);
> >  
> > -	ichan->sg[0] = NULL;
> > -	ichan->sg[1] = NULL;
> > -	spin_unlock_irqrestore(&ichan->lock, flags);
> > +		/* ichan->queue is modified in ISR, have to spinlock */
> > +		spin_lock_irqsave(&ichan->lock, flags);
> > +		list_splice_init(&ichan->queue, &ichan->free_list);
> >  
> > -	tasklet_enable(&to_ipu(idmac)->tasklet);
> > +		if (ichan->desc)
> > +			for (i = 0; i < ichan->n_tx_desc; i++) {
> > +				struct idmac_tx_desc *desc = ichan->desc + i;
> > +				if (list_empty(&desc->list))
> > +					/* Descriptor was prepared, but not submitted */
> > +					list_add(&desc->list, &ichan->free_list);
> >  
> > -	ichan->status = IPU_CHANNEL_INITIALIZED;
> > +				async_tx_clear_ack(&desc->txd);
> > +			}
> > +
> > +		ichan->sg[0] = NULL;
> > +		ichan->sg[1] = NULL;
> > +		spin_unlock_irqrestore(&ichan->lock, flags);
> > +
> > +		tasklet_enable(&ipu->tasklet);
> > +
> > +		ichan->status = IPU_CHANNEL_INITIALIZED;
> > +		break;
> > +	default:
> > +		return -ENOSYS;
> > +	}
> >  
> >  	return 0;
> >  }
> > @@ -1662,7 +1682,6 @@ static void __exit ipu_idmac_exit(struct ipu *ipu)
> >  		struct idmac_channel *ichan = ipu->channel + i;
> >  
> >  		idmac_control(&ichan->dma_chan, DMA_TERMINATE_ALL, 0);
> > -		idmac_prep_slave_sg(&ichan->dma_chan, NULL, 0, DMA_NONE, 0);
> >  	}
> >  
> >  	dma_async_device_unregister(&idmac->dma);
> 
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

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

* Re: [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue
  2011-08-29  8:30     ` Guennadi Liakhovetski
@ 2011-09-01 14:17       ` Guennadi Liakhovetski
  2011-09-28 13:20         ` [PATCH 5/7 v9] " Guennadi Liakhovetski
  0 siblings, 1 reply; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-01 14:17 UTC (permalink / raw)
  To: Pawel Osciak
  Cc: Linux Media Mailing List, Hans Verkuil, Laurent Pinchart,
	Sakari Ailus, Mauro Carvalho Chehab, Marek Szyprowski

Hi Pawel

On Mon, 29 Aug 2011, Guennadi Liakhovetski wrote:

> On Sun, 28 Aug 2011, Pawel Osciak wrote:

[snip]

> > Could you share the userspace code that you used for testing this? I
> > just wanted to get a feel of how those new ioctls fall into place
> > together.
> 
> Theoretically - yes, just will have to clean up the code a bit;-) Will try 
> to find some time for this in the next couple of days.

Below is a patch to the current capture-example.c. It is not extremely 
intelligent, many things are hard-coded, but you should get the idea.

> > Also, did you try multiple CREATE_BUFS calls?
> 
> Yes, it currently does two CREATE_BUFS, I might try to mix REQBUFS with 
> CREATE_BUFS too though.

With this patch you can use the "-q" flag to choose, whether you want your 
first buffers to be allocated per REQBUFS or CREATE_BUFS. Both options 
work.

Locally I have an even much dirtier, but more useful version of this 
patch, that draws one buffer size on the framebuffer, and stores the other 
one in .pgm files, all works well.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/


diff --git a/contrib/test/capture-example.c b/contrib/test/capture-example.c
index 417615a..ab0d8fb 100644
--- a/contrib/test/capture-example.c
+++ b/contrib/test/capture-example.c
@@ -38,14 +38,23 @@ struct buffer {
 	size_t  length;
 };
 
+enum {
+	false	= 0,
+	true	= 1
+};
+
 static char            *dev_name;
 static enum io_method   io = IO_METHOD_MMAP;
 static int              fd = -1;
 struct buffer          *buffers;
-static unsigned int     n_buffers;
+static unsigned int     n_buffers, n_buffers_alt;
 static int		out_buf;
 static int              force_format;
 static int              frame_count = 70;
+static unsigned int	width, height, fourcc, colorspace;
+static unsigned int	width_alt = 320, height_alt = 240,
+	fourcc_alt = V4L2_PIX_FMT_NV12, colorspace_alt = V4L2_COLORSPACE_JPEG;
+static _Bool		use_request;
 
 static void errno_exit(const char *s)
 {
@@ -64,6 +73,61 @@ static int xioctl(int fh, unsigned long int request, void *arg)
 	return r;
 }
 
+static int create_buffers(unsigned int *width, unsigned int *height,
+			  unsigned int fcc, unsigned int clrspc, int n)
+{
+	int i, ret;
+	struct v4l2_create_buffers create = {
+		.memory = V4L2_MEMORY_MMAP,
+		.count = n,
+		.format = {
+			.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+			.fmt.pix = {
+				.width = *width,
+				.height = *height,
+				.pixelformat = fcc,
+				.field = V4L2_FIELD_ANY,
+				.colorspace = clrspc,
+			},
+		},
+	};
+	struct v4l2_buffer buf = {
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		.memory = V4L2_MEMORY_MMAP,
+		.field = V4L2_FIELD_ANY,
+	};
+	struct v4l2_pix_format *pix = &create.format.fmt.pix;
+
+	ret = xioctl(fd, VIDIOC_TRY_FMT, &create.format);
+	if (ret < 0) {
+		fprintf(stderr, "TRY_FMT should never fail!: %d\n", ret);
+		errno_exit("VIDIOC_TRY_FMT");
+	}
+	fprintf(stderr, "TRY_FMT(%ux%u:%x) -> %u\n", pix->width, pix->height,
+		 pix->pixelformat, pix->sizeimage);
+
+	*width = pix->width;
+	*height = pix->height;
+
+	ret = xioctl(fd, VIDIOC_CREATE_BUFS, &create);
+	if (ret < 0)
+		errno_exit("VIDIOC_CREATE_BUFS");
+
+	fprintf(stderr, "CREATE_BUFS(%d@%d)\n", create.count, create.index);
+
+	for (i = create.index; i < create.index + create.count; i++) {
+		buf.index = i;
+		ret = xioctl(fd, VIDIOC_PREPARE_BUF, &buf);
+		if (ret < 0)
+			errno_exit("VIDIOC_PREPARE_BUF");
+	}
+
+	if (i > create.index)
+		return i - create.index;
+
+	return ret;
+}
+
 static void process_image(const void *p, int size)
 {
 	if (out_buf)
@@ -74,7 +138,7 @@ static void process_image(const void *p, int size)
 	fflush(stdout);
 }
 
-static int read_frame(void)
+static int read_frame(_Bool do_queue)
 {
 	struct v4l2_buffer buf;
 	unsigned int i;
@@ -120,11 +184,11 @@ static int read_frame(void)
 			}
 		}
 
-		assert(buf.index < n_buffers);
+		assert(buf.index < n_buffers + n_buffers_alt);
 
 		process_image(buffers[buf.index].start, buf.bytesused);
 
-		if (-1 == xioctl(fd, VIDIOC_QBUF, &buf))
+		if (do_queue && -1 == xioctl(fd, VIDIOC_QBUF, &buf))
 			errno_exit("VIDIOC_QBUF");
 		break;
 
@@ -154,11 +218,11 @@ static int read_frame(void)
 			    && buf.length == buffers[i].length)
 				break;
 
-		assert(i < n_buffers);
+		assert(i < n_buffers + n_buffers_alt);
 
 		process_image((void *)buf.m.userptr, buf.bytesused);
 
-		if (-1 == xioctl(fd, VIDIOC_QBUF, &buf))
+		if (do_queue && -1 == xioctl(fd, VIDIOC_QBUF, &buf))
 			errno_exit("VIDIOC_QBUF");
 		break;
 	}
@@ -166,12 +230,8 @@ static int read_frame(void)
 	return 1;
 }
 
-static void mainloop(void)
+static void mainloop(unsigned int count, _Bool do_queue)
 {
-	unsigned int count;
-
-	count = frame_count;
-
 	while (count-- > 0) {
 		for (;;) {
 			fd_set fds;
@@ -198,7 +258,7 @@ static void mainloop(void)
 				exit(EXIT_FAILURE);
 			}
 
-			if (read_frame())
+			if (read_frame(do_queue))
 				break;
 			/* EAGAIN - continue select loop. */
 		}
@@ -223,7 +283,7 @@ static void stop_capturing(void)
 	}
 }
 
-static void start_capturing(void)
+static void start_capturing(unsigned int index, unsigned int count)
 {
 	unsigned int i;
 	enum v4l2_buf_type type;
@@ -234,7 +294,7 @@ static void start_capturing(void)
 		break;
 
 	case IO_METHOD_MMAP:
-		for (i = 0; i < n_buffers; ++i) {
+		for (i = index; i < index + count; ++i) {
 			struct v4l2_buffer buf;
 
 			CLEAR(buf);
@@ -251,7 +311,7 @@ static void start_capturing(void)
 		break;
 
 	case IO_METHOD_USERPTR:
-		for (i = 0; i < n_buffers; ++i) {
+		for (i = index; i < index + count; ++i) {
 			struct v4l2_buffer buf;
 
 			CLEAR(buf);
@@ -281,7 +341,7 @@ static void uninit_device(void)
 		break;
 
 	case IO_METHOD_MMAP:
-		for (i = 0; i < n_buffers; ++i)
+		for (i = 0; i < n_buffers + n_buffers_alt; ++i)
 			if (-1 == munmap(buffers[i].start, buffers[i].length))
 				errno_exit("munmap");
 		break;
@@ -313,15 +373,13 @@ static void init_read(unsigned int buffer_size)
 	}
 }
 
-static void init_mmap(void)
+static int request_buffers(unsigned int count)
 {
-	struct v4l2_requestbuffers req;
-
-	CLEAR(req);
-
-	req.count = 4;
-	req.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	req.memory = V4L2_MEMORY_MMAP;
+	struct v4l2_requestbuffers req = {
+		.count = count,
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		.memory = V4L2_MEMORY_MMAP,
+	};
 
 	if (-1 == xioctl(fd, VIDIOC_REQBUFS, &req)) {
 		if (EINVAL == errno) {
@@ -333,40 +391,56 @@ static void init_mmap(void)
 		}
 	}
 
-	if (req.count < 2) {
-		fprintf(stderr, "Insufficient buffer memory on %s\n",
-			 dev_name);
-		exit(EXIT_FAILURE);
-	}
+	return req.count;
+}
+
+static void init_mmap(void)
+{
+	int ret;
+	int nbuf, i;
+
+	if (use_request)
+		ret = request_buffers(4);
+	else
+		ret = create_buffers(&width, &height, fourcc, colorspace, 4);
+
+	if (ret < 0)
+		errno_exit("{create,request}_buffers(main)");
 
-	buffers = calloc(req.count, sizeof(*buffers));
+	n_buffers = ret;
 
+	ret = create_buffers(&width_alt, &height_alt, fourcc_alt, colorspace_alt, 4);
+	if (ret < 0)
+		errno_exit("create_buffers(alternate)");
+
+	n_buffers_alt = ret;
+	nbuf = n_buffers_alt + n_buffers;
+
+	buffers = calloc(nbuf, sizeof(*buffers));
 	if (!buffers) {
 		fprintf(stderr, "Out of memory\n");
 		exit(EXIT_FAILURE);
 	}
 
-	for (n_buffers = 0; n_buffers < req.count; ++n_buffers) {
-		struct v4l2_buffer buf;
-
-		CLEAR(buf);
-
-		buf.type        = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-		buf.memory      = V4L2_MEMORY_MMAP;
-		buf.index       = n_buffers;
+	for (i = 0; i < nbuf; ++i) {
+		struct v4l2_buffer buf = {
+			.type        = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+			.memory      = V4L2_MEMORY_MMAP,
+			.index       = i,
+		};
 
 		if (-1 == xioctl(fd, VIDIOC_QUERYBUF, &buf))
 			errno_exit("VIDIOC_QUERYBUF");
 
-		buffers[n_buffers].length = buf.length;
-		buffers[n_buffers].start =
+		buffers[i].length = buf.length;
+		buffers[i].start =
 			mmap(NULL /* start anywhere */,
 			      buf.length,
 			      PROT_READ | PROT_WRITE /* required */,
 			      MAP_SHARED /* recommended */,
 			      fd, buf.m.offset);
 
-		if (MAP_FAILED == buffers[n_buffers].start)
+		if (MAP_FAILED == buffers[i].start)
 			errno_exit("mmap");
 	}
 }
@@ -409,13 +483,29 @@ static void init_userp(unsigned int buffer_size)
 	}
 }
 
+static int s_fmt(unsigned int width, unsigned int height, unsigned int fcc, unsigned int clrspc)
+{
+	struct v4l2_format fmt = {
+		.type = V4L2_BUF_TYPE_VIDEO_CAPTURE,
+		.fmt.pix = {
+			.width = width,
+			.height = height,
+			.pixelformat = fcc,
+			.field = V4L2_FIELD_ANY,
+			.colorspace = clrspc,
+		},
+	};
+	fprintf(stderr, "S_FMT(%ux%u:%x)\n", width, height, fcc);
+
+	return xioctl(fd, VIDIOC_S_FMT, &fmt);
+}
+
 static void init_device(void)
 {
 	struct v4l2_capability cap;
 	struct v4l2_cropcap cropcap;
 	struct v4l2_crop crop;
 	struct v4l2_format fmt;
-	unsigned int min;
 
 	if (-1 == xioctl(fd, VIDIOC_QUERYCAP, &cap)) {
 		if (EINVAL == errno) {
@@ -485,8 +575,8 @@ static void init_device(void)
 	if (force_format) {
 		fmt.fmt.pix.width       = 640;
 		fmt.fmt.pix.height      = 480;
-		fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_YUYV;
-		fmt.fmt.pix.field       = V4L2_FIELD_INTERLACED;
+		fmt.fmt.pix.pixelformat = V4L2_PIX_FMT_NV16;
+		fmt.fmt.pix.field       = V4L2_FIELD_ANY;
 
 		if (-1 == xioctl(fd, VIDIOC_S_FMT, &fmt))
 			errno_exit("VIDIOC_S_FMT");
@@ -498,6 +588,11 @@ static void init_device(void)
 			errno_exit("VIDIOC_G_FMT");
 	}
 
+	width		= fmt.fmt.pix.width;
+	height		= fmt.fmt.pix.height;
+	colorspace	= fmt.fmt.pix.colorspace;
+	fourcc		= fmt.fmt.pix.pixelformat;
+
 	switch (io) {
 	case IO_METHOD_READ:
 		init_read(fmt.fmt.pix.sizeimage);
@@ -559,11 +654,12 @@ static void usage(FILE *fp, int argc, char **argv)
 		 "-o | --output        Outputs stream to stdout\n"
 		 "-f | --format        Force format to 640x480 YUYV\n"
 		 "-c | --count         Number of frames to grab [%i]\n"
+		 "-q | --request       Use REQBUFS for the first buffer set\n"
 		 "",
 		 argv[0], dev_name, frame_count);
 }
 
-static const char short_options[] = "d:hmruofc:";
+static const char short_options[] = "d:hmruofc:q";
 
 static const struct option
 long_options[] = {
@@ -575,6 +671,7 @@ long_options[] = {
 	{ "output", no_argument,       NULL, 'o' },
 	{ "format", no_argument,       NULL, 'f' },
 	{ "count",  required_argument, NULL, 'c' },
+	{ "request",no_argument,       NULL, 'q' },
 	{ 0, 0, 0, 0 }
 };
 
@@ -624,6 +721,10 @@ int main(int argc, char **argv)
 			force_format++;
 			break;
 
+		case 'q':
+			use_request = true;
+			break;
+
 		case 'c':
 			errno = 0;
 			frame_count = strtol(optarg, NULL, 0);
@@ -639,9 +740,26 @@ int main(int argc, char **argv)
 
 	open_device();
 	init_device();
-	start_capturing();
-	mainloop();
+	start_capturing(0, n_buffers);
+	mainloop(200, true);
 	stop_capturing();
+
+	if (io == IO_METHOD_MMAP) {
+		if (-1 == s_fmt(width_alt, height_alt, fourcc_alt, colorspace_alt))
+			errno_exit("S_FMT(alternate)");
+
+		start_capturing(n_buffers, n_buffers_alt);
+		mainloop(n_buffers_alt, false);
+		stop_capturing();
+
+		if (-1 == s_fmt(width, height, fourcc, colorspace))
+			errno_exit("S_FMT(main)");
+
+		start_capturing(0, n_buffers);
+		mainloop(200, true);
+		stop_capturing();
+	}
+
 	uninit_device();
 	close_device();
 	fprintf(stderr, "\n");

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

* [PATCH 5/7 v9] V4L: vb2: add support for buffers of different sizes on a single queue
  2011-09-01 14:17       ` Guennadi Liakhovetski
@ 2011-09-28 13:20         ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-28 13:20 UTC (permalink / raw)
  To: Pawel Osciak
  Cc: Linux Media Mailing List, Hans Verkuil, Laurent Pinchart,
	Sakari Ailus, Mauro Carvalho Chehab, Marek Szyprowski

The two recently added ioctl()s VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF
allow user-space applications to allocate video buffers of different
sizes and hand them over to the driver for fast switching between
different frame formats. This patch adds support for buffers of different
sizes on the same buffer-queue to vb2.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v9:

1. add support for the new V4L2_BUF_FLAG_PREPARED flag
2. removed "const" from an argument of vb2_prepare_buf()
3. update flags on return from VIDIOC_PREPARE_BUF

 drivers/media/video/videobuf2-core.c |  294 ++++++++++++++++++++++++++++------
 include/media/videobuf2-core.h       |   35 +++--
 2 files changed, 270 insertions(+), 59 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 8fcabcb..1250662 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -38,7 +38,8 @@ module_param(debug, int, 0644);
 	(((q)->ops->op) ? ((q)->ops->op(args)) : 0)
 
 #define V4L2_BUFFER_STATE_FLAGS	(V4L2_BUF_FLAG_MAPPED | V4L2_BUF_FLAG_QUEUED | \
-				 V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR)
+				 V4L2_BUF_FLAG_DONE | V4L2_BUF_FLAG_ERROR | \
+				 V4L2_BUF_FLAG_PREPARED)
 
 /**
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
@@ -109,13 +110,22 @@ static void __vb2_buf_userptr_put(struct vb2_buffer *vb)
  * __setup_offsets() - setup unique offsets ("cookies") for every plane in
  * every buffer on the queue
  */
-static void __setup_offsets(struct vb2_queue *q)
+static void __setup_offsets(struct vb2_queue *q, unsigned int n)
 {
 	unsigned int buffer, plane;
 	struct vb2_buffer *vb;
-	unsigned long off = 0;
+	unsigned long off;
 
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+	if (q->num_buffers) {
+		struct v4l2_plane *p;
+		vb = q->bufs[q->num_buffers - 1];
+		p = &vb->v4l2_planes[vb->num_planes - 1];
+		off = PAGE_ALIGN(p->m.mem_offset + p->length);
+	} else {
+		off = 0;
+	}
+
+	for (buffer = q->num_buffers; buffer < q->num_buffers + n; ++buffer) {
 		vb = q->bufs[buffer];
 		if (!vb)
 			continue;
@@ -161,7 +171,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 		vb->state = VB2_BUF_STATE_DEQUEUED;
 		vb->vb2_queue = q;
 		vb->num_planes = num_planes;
-		vb->v4l2_buf.index = buffer;
+		vb->v4l2_buf.index = q->num_buffers + buffer;
 		vb->v4l2_buf.type = q->type;
 		vb->v4l2_buf.memory = memory;
 
@@ -189,15 +199,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 			}
 		}
 
-		q->bufs[buffer] = vb;
+		q->bufs[q->num_buffers + buffer] = vb;
 	}
 
-	q->num_buffers = buffer;
-
-	__setup_offsets(q);
+	__setup_offsets(q, buffer);
 
 	dprintk(1, "Allocated %d buffers, %d plane(s) each\n",
-			q->num_buffers, num_planes);
+			buffer, num_planes);
 
 	return buffer;
 }
@@ -205,12 +213,13 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 /**
  * __vb2_free_mem() - release all video buffer memory for a given queue
  */
-static void __vb2_free_mem(struct vb2_queue *q)
+static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers)
 {
 	unsigned int buffer;
 	struct vb2_buffer *vb;
 
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+	     ++buffer) {
 		vb = q->bufs[buffer];
 		if (!vb)
 			continue;
@@ -224,17 +233,18 @@ static void __vb2_free_mem(struct vb2_queue *q)
 }
 
 /**
- * __vb2_queue_free() - free the queue - video memory and related information
- * and return the queue to an uninitialized state. Might be called even if the
- * queue has already been freed.
+ * __vb2_queue_free() - free buffers at the end of the queue - video memory and
+ * related information, if no buffers are left return the queue to an
+ * uninitialized state. Might be called even if the queue has already been freed.
  */
-static void __vb2_queue_free(struct vb2_queue *q)
+static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 {
 	unsigned int buffer;
 
 	/* Call driver-provided cleanup function for each buffer, if provided */
 	if (q->ops->buf_cleanup) {
-		for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+		for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+		     ++buffer) {
 			if (NULL == q->bufs[buffer])
 				continue;
 			q->ops->buf_cleanup(q->bufs[buffer]);
@@ -242,23 +252,25 @@ static void __vb2_queue_free(struct vb2_queue *q)
 	}
 
 	/* Release video buffer memory */
-	__vb2_free_mem(q);
+	__vb2_free_mem(q, buffers);
 
 	/* Free videobuf buffers */
-	for (buffer = 0; buffer < q->num_buffers; ++buffer) {
+	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
+	     ++buffer) {
 		kfree(q->bufs[buffer]);
 		q->bufs[buffer] = NULL;
 	}
 
-	q->num_buffers = 0;
-	q->memory = 0;
+	q->num_buffers -= buffers;
+	if (!q->num_buffers)
+		q->memory = 0;
 }
 
 /**
  * __verify_planes_array() - verify that the planes array passed in struct
  * v4l2_buffer from userspace can be safely used
  */
-static int __verify_planes_array(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	/* Is memory for copying plane information present? */
 	if (NULL == b->m.planes) {
@@ -318,7 +330,7 @@ static bool __buffers_in_use(struct vb2_queue *q)
 static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 {
 	struct vb2_queue *q = vb->vb2_queue;
-	int ret = 0;
+	int ret;
 
 	/* Copy back data such as timestamp, flags, input, etc. */
 	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
@@ -365,8 +377,10 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 	case VB2_BUF_STATE_DONE:
 		b->flags |= V4L2_BUF_FLAG_DONE;
 		break;
-	case VB2_BUF_STATE_DEQUEUED:
 	case VB2_BUF_STATE_PREPARED:
+		b->flags |= V4L2_BUF_FLAG_PREPARED;
+		break;
+	case VB2_BUF_STATE_DEQUEUED:
 		/* nothing */
 		break;
 	}
@@ -374,7 +388,7 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
 	if (__buffer_in_use(q, vb))
 		b->flags |= V4L2_BUF_FLAG_MAPPED;
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -460,7 +474,7 @@ static int __verify_mmap_ops(struct vb2_queue *q)
  */
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 {
-	unsigned int num_buffers, num_planes;
+	unsigned int num_buffers, allocated_buffers, num_planes = 0;
 	int ret = 0;
 
 	if (q->fileio) {
@@ -508,7 +522,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 			return -EBUSY;
 		}
 
-		__vb2_queue_free(q);
+		__vb2_queue_free(q, q->num_buffers);
 
 		/*
 		 * In case of REQBUFS(0) return immediately without calling
@@ -542,44 +556,168 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 		return -ENOMEM;
 	}
 
+	allocated_buffers = ret;
+
 	/*
 	 * Check if driver can handle the allocated number of buffers.
 	 */
-	if (ret < num_buffers) {
-		unsigned int orig_num_buffers;
+	if (allocated_buffers < num_buffers) {
+		num_buffers = allocated_buffers;
 
-		orig_num_buffers = num_buffers = ret;
 		ret = call_qop(q, queue_setup, q, NULL, &num_buffers,
 			       &num_planes, q->plane_sizes, q->alloc_ctx);
-		if (ret)
-			goto free_mem;
 
-		if (orig_num_buffers < num_buffers) {
+		if (!ret && allocated_buffers < num_buffers)
 			ret = -ENOMEM;
-			goto free_mem;
-		}
 
 		/*
-		 * Ok, driver accepted smaller number of buffers.
+		 * Either the driver has accepted a smaller number of buffers,
+		 * or .queue_setup() returned an error
 		 */
-		ret = num_buffers;
+	}
+
+	q->num_buffers = allocated_buffers;
+
+	if (ret < 0) {
+		__vb2_queue_free(q, allocated_buffers);
+		return ret;
 	}
 
 	/*
 	 * Return the number of successfully allocated buffers
 	 * to the userspace.
 	 */
-	req->count = ret;
+	req->count = allocated_buffers;
 
 	return 0;
-
-free_mem:
-	__vb2_queue_free(q);
-	return ret;
 }
 EXPORT_SYMBOL_GPL(vb2_reqbufs);
 
 /**
+ * vb2_create_bufs() - Allocate buffers and any required auxiliary structs
+ * @q:		videobuf2 queue
+ * @create:	creation parameters, passed from userspace to vidioc_create_bufs
+ *		handler in driver
+ *
+ * Should be called from vidioc_create_bufs ioctl handler of a driver.
+ * This function:
+ * 1) verifies parameter sanity
+ * 2) calls the .queue_setup() queue operation
+ * 3) performs any necessary memory allocations
+ *
+ * The return values from this function are intended to be directly returned
+ * from vidioc_create_bufs handler in driver.
+ */
+int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create)
+{
+	unsigned int num_planes = 0, num_buffers, allocated_buffers;
+	int ret = 0;
+
+	if (q->fileio) {
+		dprintk(1, "%s(): file io in progress\n", __func__);
+		return -EBUSY;
+	}
+
+	if (create->memory != V4L2_MEMORY_MMAP
+			&& create->memory != V4L2_MEMORY_USERPTR) {
+		dprintk(1, "%s(): unsupported memory type\n", __func__);
+		return -EINVAL;
+	}
+
+	if (create->format.type != q->type) {
+		dprintk(1, "%s(): requested type is incorrect\n", __func__);
+		return -EINVAL;
+	}
+
+	/*
+	 * Make sure all the required memory ops for given memory type
+	 * are available.
+	 */
+	if (create->memory == V4L2_MEMORY_MMAP && __verify_mmap_ops(q)) {
+		dprintk(1, "%s(): MMAP for current setup unsupported\n", __func__);
+		return -EINVAL;
+	}
+
+	if (create->memory == V4L2_MEMORY_USERPTR && __verify_userptr_ops(q)) {
+		dprintk(1, "%s(): USERPTR for current setup unsupported\n", __func__);
+		return -EINVAL;
+	}
+
+	if (q->num_buffers == VIDEO_MAX_FRAME) {
+		dprintk(1, "%s(): maximum number of buffers already allocated\n",
+			__func__);
+		return -ENOBUFS;
+	}
+
+	create->index = q->num_buffers;
+
+	if (!q->num_buffers) {
+		memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
+		memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
+		q->memory = create->memory;
+	}
+
+	num_buffers = min(create->count, VIDEO_MAX_FRAME - q->num_buffers);
+
+	/*
+	 * Ask the driver, whether the requested number of buffers, planes per
+	 * buffer and their sizes are acceptable
+	 */
+	ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
+		       &num_planes, q->plane_sizes, q->alloc_ctx);
+	if (ret)
+		return ret;
+
+	/* Finally, allocate buffers and video memory */
+	ret = __vb2_queue_alloc(q, create->memory, num_buffers,
+				num_planes);
+	if (ret < 0) {
+		dprintk(1, "Memory allocation failed with error: %d\n", ret);
+		return ret;
+	}
+
+	allocated_buffers = ret;
+
+	/*
+	 * Check if driver can handle the so far allocated number of buffers.
+	 */
+	if (ret < num_buffers) {
+		num_buffers = ret;
+
+		/*
+		 * q->num_buffers contains the total number of buffers, that the
+		 * queue driver has set up
+		 */
+		ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
+			       &num_planes, q->plane_sizes, q->alloc_ctx);
+
+		if (!ret && allocated_buffers < num_buffers)
+			ret = -ENOMEM;
+
+		/*
+		 * Either the driver has accepted a smaller number of buffers,
+		 * or .queue_setup() returned an error
+		 */
+	}
+
+	q->num_buffers += allocated_buffers;
+
+	if (ret < 0) {
+		__vb2_queue_free(q, allocated_buffers);
+		return ret;
+	}
+
+	/*
+	 * Return the number of successfully allocated buffers
+	 * to the userspace.
+	 */
+	create->count = allocated_buffers;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_create_bufs);
+
+/**
  * vb2_plane_vaddr() - Return a kernel virtual address of a given plane
  * @vb:		vb2_buffer to which the plane in question belongs to
  * @plane_no:	plane number for which the address is to be returned
@@ -663,7 +801,7 @@ EXPORT_SYMBOL_GPL(vb2_buffer_done);
  * __fill_vb2_buffer() - fill a vb2_buffer with information provided in
  * a v4l2_buffer by the userspace
  */
-static int __fill_vb2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b,
+static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
 				struct v4l2_plane *v4l2_planes)
 {
 	unsigned int plane;
@@ -727,7 +865,7 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b,
 /**
  * __qbuf_userptr() - handle qbuf of a USERPTR buffer
  */
-static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	struct v4l2_plane planes[VIDEO_MAX_PLANES];
 	struct vb2_queue *q = vb->vb2_queue;
@@ -816,7 +954,7 @@ err:
 /**
  * __qbuf_mmap() - handle qbuf of an MMAP buffer
  */
-static int __qbuf_mmap(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	return __fill_vb2_buffer(vb, b, vb->v4l2_planes);
 }
@@ -833,7 +971,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
 	q->ops->buf_queue(vb);
 }
 
-static int __buf_prepare(struct vb2_buffer *vb, struct v4l2_buffer *b)
+static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	int ret;
@@ -861,6 +999,68 @@ static int __buf_prepare(struct vb2_buffer *vb, struct v4l2_buffer *b)
 }
 
 /**
+ * vb2_prepare_buf() - Pass ownership of a buffer from userspace to the kernel
+ * @q:		videobuf2 queue
+ * @b:		buffer structure passed from userspace to vidioc_prepare_buf
+ *		handler in driver
+ *
+ * Should be called from vidioc_prepare_buf ioctl handler of a driver.
+ * This function:
+ * 1) verifies the passed buffer,
+ * 2) calls buf_prepare callback in the driver (if provided), in which
+ *    driver-specific buffer initialization can be performed,
+ *
+ * The return values from this function are intended to be directly returned
+ * from vidioc_prepare_buf handler in driver.
+ */
+int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
+{
+	struct vb2_buffer *vb;
+	int ret;
+
+	if (q->fileio) {
+		dprintk(1, "%s(): file io in progress\n", __func__);
+		return -EBUSY;
+	}
+
+	if (b->type != q->type) {
+		dprintk(1, "%s(): invalid buffer type\n", __func__);
+		return -EINVAL;
+	}
+
+	if (b->index >= q->num_buffers) {
+		dprintk(1, "%s(): buffer index out of range\n", __func__);
+		return -EINVAL;
+	}
+
+	vb = q->bufs[b->index];
+	if (NULL == vb) {
+		/* Should never happen */
+		dprintk(1, "%s(): buffer is NULL\n", __func__);
+		return -EINVAL;
+	}
+
+	if (b->memory != q->memory) {
+		dprintk(1, "%s(): invalid memory type\n", __func__);
+		return -EINVAL;
+	}
+
+	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
+		dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state);
+		return -EINVAL;
+	}
+
+	ret = __buf_prepare(vb, b);
+	if (ret < 0)
+		return ret;
+
+	__fill_v4l2_buffer(vb, b);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_prepare_buf);
+
+/**
  * vb2_qbuf() - Queue a buffer from userspace
  * @q:		videobuf2 queue
  * @b:		buffer structure passed from userspace to vidioc_qbuf handler
@@ -1484,7 +1684,7 @@ void vb2_queue_release(struct vb2_queue *q)
 {
 	__vb2_cleanup_fileio(q);
 	__vb2_queue_cancel(q);
-	__vb2_queue_free(q);
+	__vb2_queue_free(q, q->num_buffers);
 }
 EXPORT_SYMBOL_GPL(vb2_queue_release);
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 692e35c..55c57d3 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -169,13 +169,21 @@ struct vb2_buffer {
 /**
  * struct vb2_ops - driver-specific callbacks
  *
- * @queue_setup:	called from a VIDIOC_REQBUFS handler, before
- *			memory allocation; driver should return the required
- *			number of buffers in num_buffers, the required number
- *			of planes per buffer in num_planes; the size of each
- *			plane should be set in the sizes[] array and optional
- *			per-plane allocator specific context in alloc_ctxs[]
- *			array
+ * @queue_setup:	called from VIDIOC_REQBUFS and VIDIOC_CREATE_BUFS
+ *			handlers before memory allocation, or, if
+ *			*num_planes != 0, after the allocation to verify a
+ *			smaller number of buffers. Driver should return
+ *			the required number of buffers in *num_buffers, the
+ *			required number of planes per buffer in *num_planes; the
+ *			size of each plane should be set in the sizes[] array
+ *			and optional per-plane allocator specific context in the
+ *			alloc_ctxs[] array. When called from VIDIOC_REQBUFS,
+ *			fmt == NULL, the driver has to use the currently
+ *			configured format and *num_buffers is the total number
+ *			of buffers, that are being allocated. When called from
+ *			VIDIOC_CREATE_BUFS, fmt != NULL and it describes the
+ *			target frame format. In this case *num_buffers are being
+ *			allocated additionally to q->num_buffers.
  * @wait_prepare:	release any locks taken while calling vb2 functions;
  *			it is called before an ioctl needs to wait for a new
  *			buffer to arrive; required to avoid a deadlock in
@@ -188,11 +196,11 @@ struct vb2_buffer {
  *			perform additional buffer-related initialization;
  *			initialization failure (return != 0) will prevent
  *			queue setup from completing successfully; optional
- * @buf_prepare:	called every time the buffer is queued from userspace;
- *			drivers may perform any initialization required before
- *			each hardware operation in this callback;
- *			if an error is returned, the buffer will not be queued
- *			in driver; optional
+ * @buf_prepare:	called every time the buffer is queued from userspace
+ *			and from the VIDIOC_PREPARE_BUF ioctl; drivers may
+ *			perform any initialization required before each hardware
+ *			operation in this callback; if an error is returned, the
+ *			buffer will not be queued in driver; optional
  * @buf_finish:		called before every dequeue of the buffer back to
  *			userspace; drivers may perform any operations required
  *			before userspace accesses the buffer; optional
@@ -300,6 +308,9 @@ int vb2_wait_for_all_buffers(struct vb2_queue *q);
 int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req);
 
+int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create);
+int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b);
+
 int vb2_queue_init(struct vb2_queue *q);
 
 void vb2_queue_release(struct vb2_queue *q);
-- 
1.7.2.5


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

* [PATCH 7/7 v9] V4L: soc-camera: add 2 new ioctl() handlers
  2011-08-24 18:41 ` [PATCH 7/7 v5] V4L: soc-camera: add 2 new ioctl() handlers Guennadi Liakhovetski
@ 2011-09-28 13:20   ` Guennadi Liakhovetski
  0 siblings, 0 replies; 27+ messages in thread
From: Guennadi Liakhovetski @ 2011-09-28 13:20 UTC (permalink / raw)
  To: Linux Media Mailing List
  Cc: Hans Verkuil, Laurent Pinchart, Pawel Osciak, Sakari Ailus,
	Mauro Carvalho Chehab, Marek Szyprowski

This patch adds two new ioctl() handlers: .vidioc_create_bufs() and
.vidioc_prepare_buf() for compliant vb2 soc-camera hosts.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

v9: remove "const" from an argument of the .vidioc_prepare_buf() method

 drivers/media/video/soc_camera.c |   33 +++++++++++++++++++++++++++++++--
 1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index ac23916..088972d 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -318,6 +318,32 @@ static int soc_camera_dqbuf(struct file *file, void *priv,
 		return vb2_dqbuf(&icd->vb2_vidq, p, file->f_flags & O_NONBLOCK);
 }
 
+static int soc_camera_create_bufs(struct file *file, void *priv,
+			    struct v4l2_create_buffers *create)
+{
+	struct soc_camera_device *icd = file->private_data;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+
+	/* videobuf2 only */
+	if (ici->ops->init_videobuf)
+		return -EINVAL;
+	else
+		return vb2_create_bufs(&icd->vb2_vidq, create);
+}
+
+static int soc_camera_prepare_buf(struct file *file, void *priv,
+				  struct v4l2_buffer *b)
+{
+	struct soc_camera_device *icd = file->private_data;
+	struct soc_camera_host *ici = to_soc_camera_host(icd->parent);
+
+	/* videobuf2 only */
+	if (ici->ops->init_videobuf)
+		return -EINVAL;
+	else
+		return vb2_prepare_buf(&icd->vb2_vidq, b);
+}
+
 /* Always entered with .video_lock held */
 static int soc_camera_init_user_formats(struct soc_camera_device *icd)
 {
@@ -1101,6 +1127,7 @@ static int soc_camera_probe(struct soc_camera_device *icd)
 		if (!control || !control->driver || !dev_get_drvdata(control) ||
 		    !try_module_get(control->driver->owner)) {
 			icl->del_device(icd);
+			ret = -ENODEV;
 			goto enodrv;
 		}
 	}
@@ -1366,19 +1393,21 @@ static int soc_camera_device_register(struct soc_camera_device *icd)
 
 static const struct v4l2_ioctl_ops soc_camera_ioctl_ops = {
 	.vidioc_querycap	 = soc_camera_querycap,
+	.vidioc_try_fmt_vid_cap  = soc_camera_try_fmt_vid_cap,
 	.vidioc_g_fmt_vid_cap    = soc_camera_g_fmt_vid_cap,
-	.vidioc_enum_fmt_vid_cap = soc_camera_enum_fmt_vid_cap,
 	.vidioc_s_fmt_vid_cap    = soc_camera_s_fmt_vid_cap,
+	.vidioc_enum_fmt_vid_cap = soc_camera_enum_fmt_vid_cap,
 	.vidioc_enum_input	 = soc_camera_enum_input,
 	.vidioc_g_input		 = soc_camera_g_input,
 	.vidioc_s_input		 = soc_camera_s_input,
 	.vidioc_s_std		 = soc_camera_s_std,
 	.vidioc_enum_framesizes  = soc_camera_enum_fsizes,
 	.vidioc_reqbufs		 = soc_camera_reqbufs,
-	.vidioc_try_fmt_vid_cap  = soc_camera_try_fmt_vid_cap,
 	.vidioc_querybuf	 = soc_camera_querybuf,
 	.vidioc_qbuf		 = soc_camera_qbuf,
 	.vidioc_dqbuf		 = soc_camera_dqbuf,
+	.vidioc_create_bufs	 = soc_camera_create_bufs,
+	.vidioc_prepare_buf	 = soc_camera_prepare_buf,
 	.vidioc_streamon	 = soc_camera_streamon,
 	.vidioc_streamoff	 = soc_camera_streamoff,
 	.vidioc_queryctrl	 = soc_camera_queryctrl,
-- 
1.7.2.5


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

end of thread, other threads:[~2011-09-28 13:20 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-24 18:41 [PATCH 0/7 v5] new ioctl()s and soc-camera implementation Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 1/7 v5] V4L: add a new videobuf2 buffer state VB2_BUF_STATE_PREPARED Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 2/7 v5] V4L: add two new ioctl()s for multi-size videobuffer management Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 3/7 v5] V4L: document the new VIDIOC_CREATE_BUFS and VIDIOC_PREPARE_BUF ioctl()s Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 4/7 v5] V4L: vb2: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 5/7 v5] V4L: vb2: add support for buffers of different sizes on a single queue Guennadi Liakhovetski
2011-08-28 19:29   ` Pawel Osciak
2011-08-29  8:30     ` Guennadi Liakhovetski
2011-09-01 14:17       ` Guennadi Liakhovetski
2011-09-28 13:20         ` [PATCH 5/7 v9] " Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 6/7 v5] V4L: sh-mobile-ceu-camera: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-24 18:41 ` [PATCH 7/7 v5] V4L: soc-camera: add 2 new ioctl() handlers Guennadi Liakhovetski
2011-09-28 13:20   ` [PATCH 7/7 v9] " Guennadi Liakhovetski
2011-08-25 16:45 ` [PATCH 0/2] i.MX3: support multi-size buffers in V4L Guennadi Liakhovetski
2011-08-25 16:45   ` Guennadi Liakhovetski
2011-08-25 16:45   ` [PATCH 1/2] dmaengine: ipu-idmac: add support for the DMA_PAUSE control Guennadi Liakhovetski
2011-08-25 16:45     ` Guennadi Liakhovetski
2011-08-29 15:21     ` Vinod Koul
2011-08-29 15:21       ` Vinod Koul
2011-08-29 17:50       ` Guennadi Liakhovetski
2011-08-29 17:50         ` Guennadi Liakhovetski
2011-08-25 16:46   ` [PATCH 2/2] V4L: mx3-camera: prepare to support multi-size buffers Guennadi Liakhovetski
2011-08-25 16:46     ` Guennadi Liakhovetski
2011-08-25 16:57     ` Laurent Pinchart
2011-08-25 16:57       ` Laurent Pinchart
2011-08-25 23:07       ` Guennadi Liakhovetski
2011-08-25 23:07         ` Guennadi Liakhovetski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.