Hi~ Marek.
Some comments in here.
Saturday, November 20, 2010 12:56 AM Marek Szyprowski wrote :
> To: linux-media@vger.kernel.org
> Cc: m.szyprow...@samsung.com; pa...@osciak.com; kyungmin.p...@samsung.com
> Subject: [PATCH 1/7] v4l: add videobuf2 Video for Linux 2 driver framework
>
> From: Pawel Osciak <p.osc...@samsung.com>
>
> Videobuf2 is a Video for Linux 2 API-compatible driver framework for
> multimedia devices. It acts as an intermediate layer between userspace
> applications and device drivers. It also provides low-level, modular
> memory management functions for drivers.
>
> Videobuf2 eases driver development, reduces drivers' code size and aids in
> proper and consistent implementation of V4L2 API in drivers.
>
> Videobuf2 memory management backend is fully modular. This allows custom
> memory management routines for devices and platforms with non-standard
> memory management requirements to be plugged in, without changing the
> high-level buffer management functions and API.
>
> The framework provides:
> - implementations of streaming I/O V4L2 ioctls and file operations
> - high-level video buffer, video queue and state management functions
> - video buffer memory allocation and management
>
> Signed-off-by: Pawel Osciak <p.osc...@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> CC: Pawel Osciak <pa...@osciak.com>
> ---
> drivers/media/video/Kconfig | 3 +
> drivers/media/video/Makefile | 2 +
> drivers/media/video/videobuf2-core.c | 1435
> ++++++++++++++++++++++++++++++++++
> include/media/videobuf2-core.h | 376 +++++++++
> 4 files changed, 1816 insertions(+), 0 deletions(-)
> create mode 100644 drivers/media/video/videobuf2-core.c
> create mode 100644 include/media/videobuf2-core.h
>
> diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig
> index ac16e81..fef6a14 100644
> --- a/drivers/media/video/Kconfig
> +++ b/drivers/media/video/Kconfig
> @@ -49,6 +49,9 @@ config V4L2_MEM2MEM_DEV
> tristate
> depends on VIDEOBUF_GEN
>
> +config VIDEOBUF2_CORE
> + tristate
> +
> #
> # Multimedia Video device configuration
> #
> diff --git a/drivers/media/video/Makefile b/drivers/media/video/Makefile
> index af79d47..77c4f85 100644
> --- a/drivers/media/video/Makefile
> +++ b/drivers/media/video/Makefile
> @@ -114,6 +114,8 @@ obj-$(CONFIG_VIDEOBUF_VMALLOC) += videobuf-vmalloc.o
> obj-$(CONFIG_VIDEOBUF_DVB) += videobuf-dvb.o
> obj-$(CONFIG_VIDEO_BTCX) += btcx-risc.o
>
> +obj-$(CONFIG_VIDEOBUF2_CORE) += videobuf2-core.o
> +
> obj-$(CONFIG_V4L2_MEM2MEM_DEV) += v4l2-mem2mem.o
>
> obj-$(CONFIG_VIDEO_M32R_AR_M64278) += arv.o
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c
> new file mode 100644
> index 0000000..828803f
> --- /dev/null
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -0,0 +1,1435 @@
> +/*
> + * videobuf2-core.c - V4L2 driver helper framework
> + *
> + * Copyright (C) 2010 Samsung Electronics
> + *
> + * Author: Pawel Osciak <p.osc...@samsung.com>
> + * Marek Szyprowski <m.szyprow...@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +#include <linux/err.h>
> +#include <linux/poll.h>
> +
> +#include <media/videobuf2-core.h>
> +
> +static int debug;
> +module_param(debug, int, 0644);
> +
> +#define dprintk(level, fmt, arg...) \
> + do { \
> + if (debug >= level) \
> + printk(KERN_DEBUG "vb2: " fmt, ## arg); \
> + } while (0)
> +
> +#define mem_ops(q, plane) ((q)->alloc_ctx[plane]->mem_ops)
> +
> +#define call_memop(q, plane, op, args...) \
> + ((mem_ops(q, plane)->op) ? \
> + (mem_ops(q, plane)->op(args)) : 0)
> +
> +#define call_qop(q, op, args...) \
> + (((q)->ops->op) ? ((q)->ops->op(args)) : 0)
> +
snip
> +/**
> + * __vb2_wait_for_done_vb() - wait for a buffer to become available
> + * for dequeuing
> + *
> + * Will sleep if required for nonblocking == false.
> + */
> +static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
> +{
> +checks:
> + if (!q->streaming) {
> + dprintk(1, "Streaming off, will not wait for buffers\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Buffers may be added to vb_done_list without holding the
> driver's
> + * lock, but removal is performed only while holding both driver's
> + * lock and the vb_done_lock spinlock. Thus we can be sure that as
> + * long as we hold lock, the list will remain not empty if this
> + * check succeeds.
> + */
> + if (list_empty(&q->done_list)) {
> + int retval;
> + if (nonblocking) {
> + dprintk(1, "Nonblocking and no buffers to dequeue, "
> + "will not wait\n");
> + return -EAGAIN;
> + }
> +
> + /*
> + * We are streaming and nonblocking, wait for another buffer
> to
> + * become ready or for streamoff. Driver's lock is released
> to
> + * allow streamoff or qbuf to be called while waiting.
> + */
> + call_qop(q, unlock, q);
> +
> + /*
> + * Although the mutex is released here, we will be
> reevaluating
> + * both conditions again after reacquiring it.
> + */
> + dprintk(3, "Will sleep waiting for buffers\n");
> + retval = wait_event_interruptible(q->done_wq,
> + !list_empty(&q->done_list) || !q->streaming);
How about use wait_event_timeout() or other wait functions?
If h/w or sub-device(e.g camera sensor module) does not operating due to
any reason, then maybe fall into blocking state forever.
> + call_qop(q, lock, q);
> +
> + if (retval)
> + return -EINTR;
> +
> + goto checks;
> + }
> + return 0;
> +}
> +
snip
> +int vb2_queue_init(struct vb2_queue *q, const struct vb2_ops *ops,
> + const struct vb2_alloc_ctx *alloc_ctx,
> + enum v4l2_buf_type type, void *drv_priv)
> +{
> + unsigned int i;
> +
> + BUG_ON(!q);
> + BUG_ON(!ops);
> + BUG_ON(!ops->queue_negotiate);
> + BUG_ON(!ops->plane_setup);
> + BUG_ON(!ops->buf_queue);
> +
> + BUG_ON(!alloc_ctx);
> + BUG_ON(!alloc_ctx->mem_ops);
> +
> + memset(q, 0, sizeof *q);
> + q->ops = ops;
> +
> + for (i = 0; i < VIDEO_MAX_PLANES; ++i)
> + q->alloc_ctx[i] = alloc_ctx;
According to my understanding, the alloc_ctx means that
each plane per buffer use each allocator.
for example,
It is possible alloc_ctx[0] use CMA, alloc_ctx[1] use DMA-coherent allocator.
How to set in these case?
I think you don't control about it like above for-loop.
> +
> + q->type = type;
> + q->drv_priv = drv_priv;
> +
> + INIT_LIST_HEAD(&q->queued_list);
> + INIT_LIST_HEAD(&q->done_list);
> + spin_lock_init(&q->done_lock);
> + init_waitqueue_head(&q->done_wq);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_queue_init);
snip
> +
> +struct vb2_queue;
> +
> +/**
> + * struct vb2_buffer - represents a video buffer
> + * @v4l2_buf: struct v4l2_buffer associated with this buffer;
> can
> + * be read by the driver and relevant entries can be
> + * changed by the driver in case of CAPTURE types
> + * (such as timestamp)
> + * @v4l2_planes: struct v4l2_planes associated with this buffer; can
> + * be read by the driver and relevant entries can be
> + * changed by the driver in case of CAPTURE types
> + * (such as bytesused); NOTE that even for single-planar
> + * types, the v4l2_planes[0] struct should be used
> + * instead of v4l2_buf for filling bytesused - drivers
> + * should use the vb2_set_plane_payload() function for
> that
> + * @vb2_queue: the queue to which this driver belongs
> + * @drv_entry: list entry to be used by driver for storing the
> buffer
Outdated comment. VB2 doesn't use this field any longer.
>From now on,
driver will use own buffer that is included vb2_buffer, right?
I wonder why remove drv_entry field in struct vb2_buffer?
> + * @num_planes: number of planes in the buffer
> + * on an internal driver queue
> + * @state: current buffer state; do not change
> + * @queued_entry: entry on the queued buffers list, which holds all
> + * buffers queued from userspace
> + * @done_entry: entry on the list that stores all buffers ready
> to
> + * be dequeued to userspace
> + * @planes: private per-plane information; do not change
> + * @num_planes_mapped: number of mapped planes; do not change
> + */
snip
> +/**
> + * struct vb2_queue - a videobuf queue
> + *
> + * @type: current queue type
> + * @memory: current memory type used
> + * @drv_priv: driver private data, passed on vb2_queue_init
> + * @bufs: videobuf buffer structures
> + * @num_buffers: number of allocated/used buffers
> + * @vb_lock: for ioctl handler and queue state changes synchronization
> + * @queued_list: list of buffers currently queued from userspace
> + * @done_list: list of buffers ready to be dequeued to userspace
> + * @done_lock: lock to protect done_list list
> + * @done_wq: waitqueue for processes waiting for buffers ready to be
> dequeued
> + * @ops: driver-specific callbacks
> + * @alloc_ctx: memory type/allocator-specific callbacks
> + * @streaming: current streaming state
> + * @userptr_supported: true if queue supports USERPTR types
> + * @mmap_supported: true if queue supports MMAP types
> + */
> +struct vb2_queue {
> + enum v4l2_buf_type type;
> + enum v4l2_memory memory;
> + void *drv_priv;
> +
> +/* private: internal use only */
> + struct vb2_buffer *bufs[VIDEO_MAX_FRAME];
> + unsigned int num_buffers;
> +
> + struct list_head queued_list;
> +
> + struct list_head done_list;
> + spinlock_t done_lock;
> + wait_queue_head_t done_wq;
> +
> + const struct vb2_ops *ops;
> + const struct vb2_alloc_ctx *alloc_ctx[VIDEO_MAX_PLANES];
> +
> + int streaming:1;
> + int userptr_supported:1;
> + int mmap_supported:1;
> +};
As you know, VIDIOC_REQBUFS is checking I/O support or not.
Memory mapping(or user pointer) I/O is not supported the ioctl
returns an EINVAL error code.
Therefore, two kinds of variables(userptr_supported, mmap_supported)
to use vb2_queue, right?
But, there's no usage anywhere.
Why don’t you initialize in vb2_queue_init() when is called from driver?
and check this variables in vb2_reqbufs().
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
