[PATCH RFC 0/7] V4L2 mem-to-mem ioctl helpers

[PATCH RFC 0/7] V4L2 mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Hello,

This patch set adds ioctl helpers to the v4l2-mem2mem module so the
video mem-to-mem drivers can be simplified by removing functions that
are only a pass-through to the v4l2_m2m_* calls. In addition some of
the vb2 helper functions can be used as well.

These helpers are similar to the videobuf2 ioctl helpers introduced
in commit 4c1ffcaad5 "[media] videobuf2-core: add helper functions".

Currently the requirements to use helper function introduced in this
patch set is that both OUTPUT and CAPTURE vb2 buffer queues must use
same lock and the driver uses struct v4l2_fh.

I have only tested the first three patches in this series, Tested-by
for the mx2-emmaprp and exynos-gsc drivers are appreciated.

This patch series can be also found at:
 git://linuxtv.org/snawrocki/samsung.git m2m-helpers-v2

Thanks,
Sylwester

Sylwester Nawrocki (7):
  V4L: Add mem2mem ioctl and file operation helpers
  mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers
  exynos4-is: Use mem-to-mem ioctl helpers
  s5p-jpeg: Use mem-to-mem ioctl helpers
  mx2-emmaprp: Use mem-to-mem ioctl helpers
  exynos-gsc: Use mem-to-mem ioctl helpers
  s5p-g2d: Use mem-to-mem ioctl helpers

 drivers/media/platform/exynos-gsc/gsc-core.h |   12 ---
 drivers/media/platform/exynos-gsc/gsc-m2m.c  |  109 ++++-------------------
 drivers/media/platform/exynos4-is/fimc-m2m.c |  119 +++-----------------------
 drivers/media/platform/mem2mem_testdev.c     |   94 +++-----------------
 drivers/media/platform/mx2_emmaprp.c         |  108 +++--------------------
 drivers/media/platform/s5p-g2d/g2d.c         |  103 +++-------------------
 drivers/media/platform/s5p-jpeg/jpeg-core.c  |  111 +++---------------------
 drivers/media/v4l2-core/v4l2-mem2mem.c       |  110 ++++++++++++++++++++++++
 include/media/v4l2-fh.h                      |    4 +
 include/media/v4l2-mem2mem.h                 |   22 +++++
 10 files changed, 212 insertions(+), 580 deletions(-)

--
1.7.9.5

[PATCH RFC 1/7] V4L: Add mem2mem ioctl and file operation helpers

[Sylwester Nawrocki]

This patch adds ioctl helpers to the V4L2 mem-to-mem API, so we
can avoid several ioctl handlers in the mem-to-mem video node
drivers that are simply a pass-through to the v4l2_m2m_* calls.
These helpers will only be useful for drivers that use same mutex
for both OUTPUT and CAPTURE queue, which is the case for all
currently in tree v4l2 m2m drivers.
In order to use the helpers the driver are required to use
struct v4l2_fh.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/v4l2-core/v4l2-mem2mem.c |  110 ++++++++++++++++++++++++++++++++
 include/media/v4l2-fh.h                |    4 ++
 include/media/v4l2-mem2mem.h           |   22 +++++++
 3 files changed, 136 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
index 7c43712..dddad5b 100644
--- a/drivers/media/v4l2-core/v4l2-mem2mem.c
+++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
@@ -544,6 +544,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 
 	if (m2m_ctx->m2m_dev->m2m_ops->unlock)
 		m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
+	else if (m2m_ctx->q_lock)
+		mutex_unlock(m2m_ctx->q_lock);
 
 	if (list_empty(&src_q->done_list))
 		poll_wait(file, &src_q->done_wq, wait);
@@ -552,6 +554,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
 
 	if (m2m_ctx->m2m_dev->m2m_ops->lock)
 		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
+	else if (m2m_ctx->q_lock)
+		mutex_lock(m2m_ctx->q_lock);
 
 	spin_lock_irqsave(&src_q->done_lock, flags);
 	if (!list_empty(&src_q->done_list))
@@ -679,6 +683,13 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
 
 	if (ret)
 		goto err;
+	/*
+	 * If both queues use same mutex assign it as the common buffer
+	 * queues lock to the m2m context. This lock is used in the
+	 * v4l2_m2m_ioctl_* helpers.
+	 */
+	if (out_q_ctx->q.lock == cap_q_ctx->q.lock)
+		m2m_ctx->q_lock = out_q_ctx->q.lock;
 
 	return m2m_ctx;
 err:
@@ -726,3 +737,102 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, struct vb2_buffer *vb)
 }
 EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
 
+/* Videobuf2 ioctl helpers */
+
+int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
+				struct v4l2_requestbuffers *rb)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_reqbufs(file, fh->m2m_ctx, rb);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_reqbufs);
+
+int v4l2_m2m_ioctl_querybuf(struct file *file, void *priv,
+				struct v4l2_buffer *buf)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_querybuf(file, fh->m2m_ctx, buf);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_querybuf);
+
+int v4l2_m2m_ioctl_qbuf(struct file *file, void *priv,
+				struct v4l2_buffer *buf)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_qbuf);
+
+int v4l2_m2m_ioctl_dqbuf(struct file *file, void *priv,
+				struct v4l2_buffer *buf)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_dqbuf(file, fh->m2m_ctx, buf);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_dqbuf);
+
+int v4l2_m2m_ioctl_expbuf(struct file *file, void *priv,
+				struct v4l2_exportbuffer *eb)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_expbuf(file, fh->m2m_ctx, eb);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_expbuf);
+
+int v4l2_m2m_ioctl_streamon(struct file *file, void *priv,
+				enum v4l2_buf_type type)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_streamon(file, fh->m2m_ctx, type);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamon);
+
+int v4l2_m2m_ioctl_streamoff(struct file *file, void *priv,
+				enum v4l2_buf_type type)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_streamoff(file, fh->m2m_ctx, type);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamoff);
+
+/*
+ * v4l2_file_operations helpers. It is assumed here same lock is used
+ * for the output and the capture buffer queue.
+ */
+
+int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct v4l2_fh *fh = file->private_data;
+	struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx;
+	int ret;
+
+	if (m2m_ctx->q_lock && mutex_lock_interruptible(m2m_ctx->q_lock))
+		return -ERESTARTSYS;
+
+	ret = v4l2_m2m_mmap(file, m2m_ctx, vma);
+
+	if (m2m_ctx->q_lock)
+		mutex_unlock(m2m_ctx->q_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_fop_mmap);
+
+unsigned int v4l2_m2m_fop_poll(struct file *file, poll_table *wait)
+{
+	struct v4l2_fh *fh = file->private_data;
+	struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx;
+	unsigned int ret;
+
+	if (m2m_ctx->q_lock)
+		mutex_lock(m2m_ctx->q_lock);
+
+	ret = v4l2_m2m_poll(file, m2m_ctx, wait);
+
+	if (m2m_ctx->q_lock)
+		mutex_unlock(m2m_ctx->q_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_fop_poll);
+
diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
index a62ee18..d942f79 100644
--- a/include/media/v4l2-fh.h
+++ b/include/media/v4l2-fh.h
@@ -43,6 +43,10 @@ struct v4l2_fh {
 	struct list_head	available; /* Dequeueable event */
 	unsigned int		navailable;
 	u32			sequence;
+
+#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
+	struct v4l2_m2m_ctx	*m2m_ctx;
+#endif
 };
 
 /*
diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
index 44542a2..2a0e489 100644
--- a/include/media/v4l2-mem2mem.h
+++ b/include/media/v4l2-mem2mem.h
@@ -64,6 +64,9 @@ struct v4l2_m2m_queue_ctx {
 };
 
 struct v4l2_m2m_ctx {
+	/* optional cap/out vb2 queues lock */
+	struct mutex			*q_lock;
+
 /* private: internal use only */
 	struct v4l2_m2m_dev		*m2m_dev;
 
@@ -229,5 +232,24 @@ static inline void *v4l2_m2m_dst_buf_remove(struct v4l2_m2m_ctx *m2m_ctx)
 	return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx);
 }
 
+/* v4l2 ioctl helpers */
+
+int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
+				struct v4l2_requestbuffers *rb);
+int v4l2_m2m_ioctl_querybuf(struct file *file, void *fh,
+				struct v4l2_buffer *buf);
+int v4l2_m2m_ioctl_qbuf(struct file *file, void *fh,
+				struct v4l2_buffer *buf);
+int v4l2_m2m_ioctl_dqbuf(struct file *file, void *fh,
+				struct v4l2_buffer *buf);
+int v4l2_m2m_ioctl_expbuf(struct file *file, void *fh,
+				struct v4l2_exportbuffer *eb);
+int v4l2_m2m_ioctl_streamon(struct file *file, void *fh,
+				enum v4l2_buf_type type);
+int v4l2_m2m_ioctl_streamoff(struct file *file, void *fh,
+				enum v4l2_buf_type type);
+int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma);
+unsigned int v4l2_m2m_fop_poll(struct file *file, poll_table *wait);
+
 #endif /* _MEDIA_V4L2_MEM2MEM_H */
 
-- 
1.7.9.5

Re: [PATCH RFC 1/7] V4L: Add mem2mem ioctl and file operation helpers

[Philipp Zabel]

Hi Sylwester,

Am Freitag, den 13.09.2013, 14:56 +0200 schrieb Sylwester Nawrocki:
> This patch adds ioctl helpers to the V4L2 mem-to-mem API, so we
> can avoid several ioctl handlers in the mem-to-mem video node
> drivers that are simply a pass-through to the v4l2_m2m_* calls.
> These helpers will only be useful for drivers that use same mutex
> for both OUTPUT and CAPTURE queue, which is the case for all
> currently in tree v4l2 m2m drivers.
> In order to use the helpers the driver are required to use
> struct v4l2_fh.

this looks good to me.

> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c |  110 ++++++++++++++++++++++++++++++++
>  include/media/v4l2-fh.h                |    4 ++
>  include/media/v4l2-mem2mem.h           |   22 +++++++
>  3 files changed, 136 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 7c43712..dddad5b 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -544,6 +544,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  
>  	if (m2m_ctx->m2m_dev->m2m_ops->unlock)
>  		m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
> +	else if (m2m_ctx->q_lock)
> +		mutex_unlock(m2m_ctx->q_lock);
>  
>  	if (list_empty(&src_q->done_list))
>  		poll_wait(file, &src_q->done_wq, wait);
> @@ -552,6 +554,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  
>  	if (m2m_ctx->m2m_dev->m2m_ops->lock)
>  		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
> +	else if (m2m_ctx->q_lock)
> +		mutex_lock(m2m_ctx->q_lock);
>  
>  	spin_lock_irqsave(&src_q->done_lock, flags);
>  	if (!list_empty(&src_q->done_list))
> @@ -679,6 +683,13 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>  
>  	if (ret)
>  		goto err;
> +	/*
> +	 * If both queues use same mutex assign it as the common buffer
> +	 * queues lock to the m2m context. This lock is used in the
> +	 * v4l2_m2m_ioctl_* helpers.
> +	 */
> +	if (out_q_ctx->q.lock == cap_q_ctx->q.lock)
> +		m2m_ctx->q_lock = out_q_ctx->q.lock;
>  
>  	return m2m_ctx;
>  err:
> @@ -726,3 +737,102 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, struct vb2_buffer *vb)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
>  
> +/* Videobuf2 ioctl helpers */
> +
> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
> +				struct v4l2_requestbuffers *rb)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_reqbufs(file, fh->m2m_ctx, rb);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_reqbufs);
> +
> +int v4l2_m2m_ioctl_querybuf(struct file *file, void *priv,
> +				struct v4l2_buffer *buf)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_querybuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_querybuf);
> +
> +int v4l2_m2m_ioctl_qbuf(struct file *file, void *priv,
> +				struct v4l2_buffer *buf)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_qbuf);
> +
> +int v4l2_m2m_ioctl_dqbuf(struct file *file, void *priv,
> +				struct v4l2_buffer *buf)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_dqbuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_dqbuf);
> +

Here I'm missing one 

+int v4l2_m2m_ioctl_create_bufs(struct file *file, void *priv,
+			       struct v4l2_create_buffers *create)
+{
+	struct v4l2_fh *fh = file->private_data;
+	return v4l2_m2m_create_bufs(file, fh->m2m_ctx, create);
+}
+EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_create_bufs);

> +int v4l2_m2m_ioctl_expbuf(struct file *file, void *priv,
> +				struct v4l2_exportbuffer *eb)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_expbuf(file, fh->m2m_ctx, eb);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_expbuf);
> +
> +int v4l2_m2m_ioctl_streamon(struct file *file, void *priv,
> +				enum v4l2_buf_type type)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_streamon(file, fh->m2m_ctx, type);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamon);
> +
> +int v4l2_m2m_ioctl_streamoff(struct file *file, void *priv,
> +				enum v4l2_buf_type type)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_streamoff(file, fh->m2m_ctx, type);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamoff);
> +
> +/*
> + * v4l2_file_operations helpers. It is assumed here same lock is used
> + * for the output and the capture buffer queue.
> + */
> +
> +int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx;
> +	int ret;
> +
> +	if (m2m_ctx->q_lock && mutex_lock_interruptible(m2m_ctx->q_lock))
> +		return -ERESTARTSYS;
> +
> +	ret = v4l2_m2m_mmap(file, m2m_ctx, vma);
> +
> +	if (m2m_ctx->q_lock)
> +		mutex_unlock(m2m_ctx->q_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_fop_mmap);
> +
> +unsigned int v4l2_m2m_fop_poll(struct file *file, poll_table *wait)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx;
> +	unsigned int ret;
> +
> +	if (m2m_ctx->q_lock)
> +		mutex_lock(m2m_ctx->q_lock);
> +
> +	ret = v4l2_m2m_poll(file, m2m_ctx, wait);
> +
> +	if (m2m_ctx->q_lock)
> +		mutex_unlock(m2m_ctx->q_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_fop_poll);
> +
> diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
> index a62ee18..d942f79 100644
> --- a/include/media/v4l2-fh.h
> +++ b/include/media/v4l2-fh.h
> @@ -43,6 +43,10 @@ struct v4l2_fh {
>  	struct list_head	available; /* Dequeueable event */
>  	unsigned int		navailable;
>  	u32			sequence;
> +
> +#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
> +	struct v4l2_m2m_ctx	*m2m_ctx;
> +#endif
>  };
>  
>  /*
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 44542a2..2a0e489 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -64,6 +64,9 @@ struct v4l2_m2m_queue_ctx {
>  };
>  
>  struct v4l2_m2m_ctx {
> +	/* optional cap/out vb2 queues lock */
> +	struct mutex			*q_lock;
> +
>  /* private: internal use only */
>  	struct v4l2_m2m_dev		*m2m_dev;
>  
> @@ -229,5 +232,24 @@ static inline void *v4l2_m2m_dst_buf_remove(struct v4l2_m2m_ctx *m2m_ctx)
>  	return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx);
>  }
>  
> +/* v4l2 ioctl helpers */
> +
> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
> +				struct v4l2_requestbuffers *rb);
> +int v4l2_m2m_ioctl_querybuf(struct file *file, void *fh,
> +				struct v4l2_buffer *buf);
> +int v4l2_m2m_ioctl_qbuf(struct file *file, void *fh,
> +				struct v4l2_buffer *buf);
> +int v4l2_m2m_ioctl_dqbuf(struct file *file, void *fh,
> +				struct v4l2_buffer *buf);

and a

+int v4l2_m2m_ioctl_create_bufs(struct file *file, void *fh,
+			       struct v4l2_create_buffers *create);

> +int v4l2_m2m_ioctl_expbuf(struct file *file, void *fh,
> +				struct v4l2_exportbuffer *eb);
> +int v4l2_m2m_ioctl_streamon(struct file *file, void *fh,
> +				enum v4l2_buf_type type);
> +int v4l2_m2m_ioctl_streamoff(struct file *file, void *fh,
> +				enum v4l2_buf_type type);
> +int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma);
> +unsigned int v4l2_m2m_fop_poll(struct file *file, poll_table *wait);
> +
>  #endif /* _MEDIA_V4L2_MEM2MEM_H */
>  

regards
Philipp

Re: [PATCH RFC 1/7] V4L: Add mem2mem ioctl and file operation helpers

[Sylwester Nawrocki]

Hi Philipp,

On 09/13/2013 03:13 PM, Philipp Zabel wrote:
> Am Freitag, den 13.09.2013, 14:56 +0200 schrieb Sylwester Nawrocki:
>> This patch adds ioctl helpers to the V4L2 mem-to-mem API, so we
>> can avoid several ioctl handlers in the mem-to-mem video node
>> drivers that are simply a pass-through to the v4l2_m2m_* calls.
>> These helpers will only be useful for drivers that use same mutex
>> for both OUTPUT and CAPTURE queue, which is the case for all
>> currently in tree v4l2 m2m drivers.
>> In order to use the helpers the driver are required to use
>> struct v4l2_fh.
>
> this looks good to me.

Thank you for the review.

>> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
>> index 7c43712..dddad5b 100644
>> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
>> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
[...]
>> +/* Videobuf2 ioctl helpers */
>> +
>> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
>> +				struct v4l2_requestbuffers *rb)
>> +{
>> +	struct v4l2_fh *fh = file->private_data;
>> +	return v4l2_m2m_reqbufs(file, fh->m2m_ctx, rb);
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_reqbufs);
>> +
>> +int v4l2_m2m_ioctl_querybuf(struct file *file, void *priv,
>> +				struct v4l2_buffer *buf)
>> +{
>> +	struct v4l2_fh *fh = file->private_data;
>> +	return v4l2_m2m_querybuf(file, fh->m2m_ctx, buf);
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_querybuf);
>> +
>> +int v4l2_m2m_ioctl_qbuf(struct file *file, void *priv,
>> +				struct v4l2_buffer *buf)
>> +{
>> +	struct v4l2_fh *fh = file->private_data;
>> +	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_qbuf);
>> +
>> +int v4l2_m2m_ioctl_dqbuf(struct file *file, void *priv,
>> +				struct v4l2_buffer *buf)
>> +{
>> +	struct v4l2_fh *fh = file->private_data;
>> +	return v4l2_m2m_dqbuf(file, fh->m2m_ctx, buf);
>> +}
>> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_dqbuf);
>> +
>
> Here I'm missing one
>
> +int v4l2_m2m_ioctl_create_bufs(struct file *file, void *priv,
> +			       struct v4l2_create_buffers *create)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_create_bufs(file, fh->m2m_ctx, create);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_create_bufs);

OK, I'll add that in the next iteration. I thought I would need to
add v4l2_m2m_ioctl_prepare_buf() similarly, but it's not necessary,
since vidioc_create_buf() calls directly to videobuf2, so that's
even simpler.

--
Regards,
Sylwester

Re: [PATCH RFC 1/7] V4L: Add mem2mem ioctl and file operation helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> This patch adds ioctl helpers to the V4L2 mem-to-mem API, so we
> can avoid several ioctl handlers in the mem-to-mem video node
> drivers that are simply a pass-through to the v4l2_m2m_* calls.
> These helpers will only be useful for drivers that use same mutex
> for both OUTPUT and CAPTURE queue, which is the case for all
> currently in tree v4l2 m2m drivers.
> In order to use the helpers the driver are required to use
> struct v4l2_fh.

Looks good! I have one small comment below that you might want to address,
although it isn't blocking.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c |  110 ++++++++++++++++++++++++++++++++
>  include/media/v4l2-fh.h                |    4 ++
>  include/media/v4l2-mem2mem.h           |   22 +++++++
>  3 files changed, 136 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 7c43712..dddad5b 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -544,6 +544,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  
>  	if (m2m_ctx->m2m_dev->m2m_ops->unlock)
>  		m2m_ctx->m2m_dev->m2m_ops->unlock(m2m_ctx->priv);
> +	else if (m2m_ctx->q_lock)
> +		mutex_unlock(m2m_ctx->q_lock);
>  
>  	if (list_empty(&src_q->done_list))
>  		poll_wait(file, &src_q->done_wq, wait);
> @@ -552,6 +554,8 @@ unsigned int v4l2_m2m_poll(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  
>  	if (m2m_ctx->m2m_dev->m2m_ops->lock)
>  		m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
> +	else if (m2m_ctx->q_lock)
> +		mutex_lock(m2m_ctx->q_lock);
>  
>  	spin_lock_irqsave(&src_q->done_lock, flags);
>  	if (!list_empty(&src_q->done_list))
> @@ -679,6 +683,13 @@ struct v4l2_m2m_ctx *v4l2_m2m_ctx_init(struct v4l2_m2m_dev *m2m_dev,
>  
>  	if (ret)
>  		goto err;
> +	/*
> +	 * If both queues use same mutex assign it as the common buffer
> +	 * queues lock to the m2m context. This lock is used in the
> +	 * v4l2_m2m_ioctl_* helpers.
> +	 */
> +	if (out_q_ctx->q.lock == cap_q_ctx->q.lock)
> +		m2m_ctx->q_lock = out_q_ctx->q.lock;
>  
>  	return m2m_ctx;
>  err:
> @@ -726,3 +737,102 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, struct vb2_buffer *vb)
>  }
>  EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue);
>  
> +/* Videobuf2 ioctl helpers */
> +
> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
> +				struct v4l2_requestbuffers *rb)
> +{
> +	struct v4l2_fh *fh = file->private_data;

I prefer an empty line after the variable declaration. Ditto below.

> +	return v4l2_m2m_reqbufs(file, fh->m2m_ctx, rb);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_reqbufs);
> +
> +int v4l2_m2m_ioctl_querybuf(struct file *file, void *priv,
> +				struct v4l2_buffer *buf)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_querybuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_querybuf);
> +
> +int v4l2_m2m_ioctl_qbuf(struct file *file, void *priv,
> +				struct v4l2_buffer *buf)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_qbuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_qbuf);
> +
> +int v4l2_m2m_ioctl_dqbuf(struct file *file, void *priv,
> +				struct v4l2_buffer *buf)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_dqbuf(file, fh->m2m_ctx, buf);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_dqbuf);
> +
> +int v4l2_m2m_ioctl_expbuf(struct file *file, void *priv,
> +				struct v4l2_exportbuffer *eb)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_expbuf(file, fh->m2m_ctx, eb);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_expbuf);
> +
> +int v4l2_m2m_ioctl_streamon(struct file *file, void *priv,
> +				enum v4l2_buf_type type)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_streamon(file, fh->m2m_ctx, type);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamon);
> +
> +int v4l2_m2m_ioctl_streamoff(struct file *file, void *priv,
> +				enum v4l2_buf_type type)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	return v4l2_m2m_streamoff(file, fh->m2m_ctx, type);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_ioctl_streamoff);
> +
> +/*
> + * v4l2_file_operations helpers. It is assumed here same lock is used
> + * for the output and the capture buffer queue.
> + */
> +
> +int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx;
> +	int ret;
> +
> +	if (m2m_ctx->q_lock && mutex_lock_interruptible(m2m_ctx->q_lock))
> +		return -ERESTARTSYS;
> +
> +	ret = v4l2_m2m_mmap(file, m2m_ctx, vma);
> +
> +	if (m2m_ctx->q_lock)
> +		mutex_unlock(m2m_ctx->q_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_fop_mmap);
> +
> +unsigned int v4l2_m2m_fop_poll(struct file *file, poll_table *wait)
> +{
> +	struct v4l2_fh *fh = file->private_data;
> +	struct v4l2_m2m_ctx *m2m_ctx = fh->m2m_ctx;
> +	unsigned int ret;
> +
> +	if (m2m_ctx->q_lock)
> +		mutex_lock(m2m_ctx->q_lock);
> +
> +	ret = v4l2_m2m_poll(file, m2m_ctx, wait);
> +
> +	if (m2m_ctx->q_lock)
> +		mutex_unlock(m2m_ctx->q_lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_fop_poll);
> +
> diff --git a/include/media/v4l2-fh.h b/include/media/v4l2-fh.h
> index a62ee18..d942f79 100644
> --- a/include/media/v4l2-fh.h
> +++ b/include/media/v4l2-fh.h
> @@ -43,6 +43,10 @@ struct v4l2_fh {
>  	struct list_head	available; /* Dequeueable event */
>  	unsigned int		navailable;
>  	u32			sequence;
> +
> +#if IS_ENABLED(CONFIG_V4L2_MEM2MEM_DEV)
> +	struct v4l2_m2m_ctx	*m2m_ctx;
> +#endif
>  };
>  
>  /*
> diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h
> index 44542a2..2a0e489 100644
> --- a/include/media/v4l2-mem2mem.h
> +++ b/include/media/v4l2-mem2mem.h
> @@ -64,6 +64,9 @@ struct v4l2_m2m_queue_ctx {
>  };
>  
>  struct v4l2_m2m_ctx {
> +	/* optional cap/out vb2 queues lock */
> +	struct mutex			*q_lock;
> +
>  /* private: internal use only */
>  	struct v4l2_m2m_dev		*m2m_dev;
>  
> @@ -229,5 +232,24 @@ static inline void *v4l2_m2m_dst_buf_remove(struct v4l2_m2m_ctx *m2m_ctx)
>  	return v4l2_m2m_buf_remove(&m2m_ctx->cap_q_ctx);
>  }
>  
> +/* v4l2 ioctl helpers */
> +
> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
> +				struct v4l2_requestbuffers *rb);
> +int v4l2_m2m_ioctl_querybuf(struct file *file, void *fh,
> +				struct v4l2_buffer *buf);
> +int v4l2_m2m_ioctl_qbuf(struct file *file, void *fh,
> +				struct v4l2_buffer *buf);
> +int v4l2_m2m_ioctl_dqbuf(struct file *file, void *fh,
> +				struct v4l2_buffer *buf);
> +int v4l2_m2m_ioctl_expbuf(struct file *file, void *fh,
> +				struct v4l2_exportbuffer *eb);
> +int v4l2_m2m_ioctl_streamon(struct file *file, void *fh,
> +				enum v4l2_buf_type type);
> +int v4l2_m2m_ioctl_streamoff(struct file *file, void *fh,
> +				enum v4l2_buf_type type);
> +int v4l2_m2m_fop_mmap(struct file *file, struct vm_area_struct *vma);
> +unsigned int v4l2_m2m_fop_poll(struct file *file, poll_table *wait);
> +
>  #endif /* _MEDIA_V4L2_MEM2MEM_H */
>  
> 

Re: [PATCH RFC 1/7] V4L: Add mem2mem ioctl and file operation helpers

[Sylwester Nawrocki]

On 09/30/2013 11:41 AM, Hans Verkuil wrote:
> On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
>> This patch adds ioctl helpers to the V4L2 mem-to-mem API, so we
>> can avoid several ioctl handlers in the mem-to-mem video node
>> drivers that are simply a pass-through to the v4l2_m2m_* calls.
>> These helpers will only be useful for drivers that use same mutex
>> for both OUTPUT and CAPTURE queue, which is the case for all
>> currently in tree v4l2 m2m drivers.
>> In order to use the helpers the driver are required to use
>> struct v4l2_fh.
>
> Looks good! I have one small comment below that you might want to address,
> although it isn't blocking.
>
> Acked-by: Hans Verkuil<hans.verkuil@cisco.com>
>
> Regards,
>
> 	Hans
>
>> +/* Videobuf2 ioctl helpers */
>> +
>> +int v4l2_m2m_ioctl_reqbufs(struct file *file, void *priv,
>> +				struct v4l2_requestbuffers *rb)
>> +{
>> +	struct v4l2_fh *fh = file->private_data;
>
> I prefer an empty line after the variable declaration. Ditto below.

Thank you for the review. All right, I will include this change in next
iteration. Even though my feeling is that such two-liner functions look
better without an empty line and readability is still maintained.

--
Regards,
Sylwester

[PATCH RFC 2/7] mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers

[Sylwester Nawrocki]

Simplify the driver by using the m2m ioctl and vb2 helpers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/mem2mem_testdev.c |   94 ++++--------------------------
 1 file changed, 11 insertions(+), 83 deletions(-)

diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
index 6a17676..73df44f 100644
--- a/drivers/media/platform/mem2mem_testdev.c
+++ b/drivers/media/platform/mem2mem_testdev.c
@@ -359,21 +359,6 @@ static void job_abort(void *priv)
 	ctx->aborting = 1;
 }
 
-static void m2mtest_lock(void *priv)
-{
-	struct m2mtest_ctx *ctx = priv;
-	struct m2mtest_dev *dev = ctx->dev;
-	mutex_lock(&dev->dev_mutex);
-}
-
-static void m2mtest_unlock(void *priv)
-{
-	struct m2mtest_ctx *ctx = priv;
-	struct m2mtest_dev *dev = ctx->dev;
-	mutex_unlock(&dev->dev_mutex);
-}
-
-
 /* device_run() - prepares and starts the device
  *
  * This simulates all the immediate preparations required before starting
@@ -648,52 +633,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
 	return ret;
 }
 
-static int vidioc_reqbufs(struct file *file, void *priv,
-			  struct v4l2_requestbuffers *reqbufs)
-{
-	struct m2mtest_ctx *ctx = file2ctx(file);
-
-	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
-}
-
-static int vidioc_querybuf(struct file *file, void *priv,
-			   struct v4l2_buffer *buf)
-{
-	struct m2mtest_ctx *ctx = file2ctx(file);
-
-	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct m2mtest_ctx *ctx = file2ctx(file);
-
-	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct m2mtest_ctx *ctx = file2ctx(file);
-
-	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_streamon(struct file *file, void *priv,
-			   enum v4l2_buf_type type)
-{
-	struct m2mtest_ctx *ctx = file2ctx(file);
-
-	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
-}
-
-static int vidioc_streamoff(struct file *file, void *priv,
-			    enum v4l2_buf_type type)
-{
-	struct m2mtest_ctx *ctx = file2ctx(file);
-
-	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
-}
-
 static int m2mtest_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct m2mtest_ctx *ctx =
@@ -748,14 +687,14 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = {
 	.vidioc_try_fmt_vid_out	= vidioc_try_fmt_vid_out,
 	.vidioc_s_fmt_vid_out	= vidioc_s_fmt_vid_out,
 
-	.vidioc_reqbufs		= vidioc_reqbufs,
-	.vidioc_querybuf	= vidioc_querybuf,
+	.vidioc_reqbufs		= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
+	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
+	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
 
-	.vidioc_qbuf		= vidioc_qbuf,
-	.vidioc_dqbuf		= vidioc_dqbuf,
+	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,
 
-	.vidioc_streamon	= vidioc_streamon,
-	.vidioc_streamoff	= vidioc_streamoff,
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
@@ -821,24 +760,12 @@ static void m2mtest_buf_queue(struct vb2_buffer *vb)
 	v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
 }
 
-static void m2mtest_wait_prepare(struct vb2_queue *q)
-{
-	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
-	m2mtest_unlock(ctx);
-}
-
-static void m2mtest_wait_finish(struct vb2_queue *q)
-{
-	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
-	m2mtest_lock(ctx);
-}
-
 static struct vb2_ops m2mtest_qops = {
 	.queue_setup	 = m2mtest_queue_setup,
 	.buf_prepare	 = m2mtest_buf_prepare,
 	.buf_queue	 = m2mtest_buf_queue,
-	.wait_prepare	 = m2mtest_wait_prepare,
-	.wait_finish	 = m2mtest_wait_finish,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
@@ -853,6 +780,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
 	src_vq->ops = &m2mtest_qops;
 	src_vq->mem_ops = &vb2_vmalloc_memops;
 	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->dev->dev_mutex;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -865,6 +793,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
 	dst_vq->ops = &m2mtest_qops;
 	dst_vq->mem_ops = &vb2_vmalloc_memops;
 	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->dev->dev_mutex;
 
 	return vb2_queue_init(dst_vq);
 }
@@ -945,6 +874,7 @@ static int m2mtest_open(struct file *file)
 		kfree(ctx);
 		goto open_unlock;
 	}
+	ctx->fh.m2m_ctx = ctx->m2m_ctx;
 
 	v4l2_fh_add(&ctx->fh);
 	atomic_inc(&dev->num_inst);
@@ -1019,8 +949,6 @@ static struct v4l2_m2m_ops m2m_ops = {
 	.device_run	= device_run,
 	.job_ready	= job_ready,
 	.job_abort	= job_abort,
-	.lock		= m2mtest_lock,
-	.unlock		= m2mtest_unlock,
 };
 
 static int m2mtest_probe(struct platform_device *pdev)
-- 
1.7.9.5

Re: [PATCH RFC 2/7] mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers

[Philipp Zabel]

Hi Sylwester,

Am Freitag, den 13.09.2013, 14:56 +0200 schrieb Sylwester Nawrocki:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/platform/mem2mem_testdev.c |   94 ++++--------------------------
>  1 file changed, 11 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
> index 6a17676..73df44f 100644
> --- a/drivers/media/platform/mem2mem_testdev.c
> +++ b/drivers/media/platform/mem2mem_testdev.c
> @@ -359,21 +359,6 @@ static void job_abort(void *priv)
>  	ctx->aborting = 1;
>  }
>  
> -static void m2mtest_lock(void *priv)
> -{
> -	struct m2mtest_ctx *ctx = priv;
> -	struct m2mtest_dev *dev = ctx->dev;
> -	mutex_lock(&dev->dev_mutex);
> -}
> -
> -static void m2mtest_unlock(void *priv)
> -{
> -	struct m2mtest_ctx *ctx = priv;
> -	struct m2mtest_dev *dev = ctx->dev;
> -	mutex_unlock(&dev->dev_mutex);
> -}
> -
> -
>  /* device_run() - prepares and starts the device
>   *
>   * This simulates all the immediate preparations required before starting
> @@ -648,52 +633,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
>  	return ret;
>  }
>  
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -			  struct v4l2_requestbuffers *reqbufs)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -			   struct v4l2_buffer *buf)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> -			   enum v4l2_buf_type type)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> -			    enum v4l2_buf_type type)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int m2mtest_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct m2mtest_ctx *ctx =
> @@ -748,14 +687,14 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = {
>  	.vidioc_try_fmt_vid_out	= vidioc_try_fmt_vid_out,
>  	.vidioc_s_fmt_vid_out	= vidioc_s_fmt_vid_out,
>  
> -	.vidioc_reqbufs		= vidioc_reqbufs,
> -	.vidioc_querybuf	= vidioc_querybuf,
> +	.vidioc_reqbufs		= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
>  
> -	.vidioc_qbuf		= vidioc_qbuf,
> -	.vidioc_dqbuf		= vidioc_dqbuf,
> +	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,
>  
> -	.vidioc_streamon	= vidioc_streamon,
> -	.vidioc_streamoff	= vidioc_streamoff,
>  	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>  	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>  };
> @@ -821,24 +760,12 @@ static void m2mtest_buf_queue(struct vb2_buffer *vb)
>  	v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -static void m2mtest_wait_prepare(struct vb2_queue *q)
> -{
> -	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
> -	m2mtest_unlock(ctx);
> -}
> -
> -static void m2mtest_wait_finish(struct vb2_queue *q)
> -{
> -	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
> -	m2mtest_lock(ctx);
> -}
> -
>  static struct vb2_ops m2mtest_qops = {
>  	.queue_setup	 = m2mtest_queue_setup,
>  	.buf_prepare	 = m2mtest_buf_prepare,
>  	.buf_queue	 = m2mtest_buf_queue,
> -	.wait_prepare	 = m2mtest_wait_prepare,
> -	.wait_finish	 = m2mtest_wait_finish,
> +	.wait_prepare	 = vb2_ops_wait_prepare,
> +	.wait_finish	 = vb2_ops_wait_finish,
>  };
>  
>  static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> @@ -853,6 +780,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
>  	src_vq->ops = &m2mtest_qops;
>  	src_vq->mem_ops = &vb2_vmalloc_memops;
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->dev->dev_mutex;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -865,6 +793,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
>  	dst_vq->ops = &m2mtest_qops;
>  	dst_vq->mem_ops = &vb2_vmalloc_memops;
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->dev->dev_mutex;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> @@ -945,6 +874,7 @@ static int m2mtest_open(struct file *file)
>  		kfree(ctx);
>  		goto open_unlock;
>  	}
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;

Since you added m2m_ctx to v4l2_fh, why not drop ctx->m2m_ctx altogether
and always use ctx->fh.m2m_ctx instead?

>  	v4l2_fh_add(&ctx->fh);
>  	atomic_inc(&dev->num_inst);
> @@ -1019,8 +949,6 @@ static struct v4l2_m2m_ops m2m_ops = {
>  	.device_run	= device_run,
>  	.job_ready	= job_ready,
>  	.job_abort	= job_abort,
> -	.lock		= m2mtest_lock,
> -	.unlock		= m2mtest_unlock,
>  };
>  
>  static int m2mtest_probe(struct platform_device *pdev)

regards
Philipp

Re: [PATCH RFC 2/7] mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers

[Sylwester Nawrocki]

Hi Philipp,

On 09/13/2013 03:08 PM, Philipp Zabel wrote:
> Am Freitag, den 13.09.2013, 14:56 +0200 schrieb Sylwester Nawrocki:
[...]
>> @@ -865,6 +793,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
>>   	dst_vq->ops =&m2mtest_qops;
>>   	dst_vq->mem_ops =&vb2_vmalloc_memops;
>>   	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
>> +	dst_vq->lock =&ctx->dev->dev_mutex;
>>
>>   	return vb2_queue_init(dst_vq);
>>   }
>> @@ -945,6 +874,7 @@ static int m2mtest_open(struct file *file)
>>   		kfree(ctx);
>>   		goto open_unlock;
>>   	}
>> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>
> Since you added m2m_ctx to v4l2_fh, why not drop ctx->m2m_ctx altogether
> and always use ctx->fh.m2m_ctx instead?

Yes, that might make it a bit cleaner. I guess I wanted to minimize
the necessary changes. I'll amend that in all drivers in this series
for the next iteration, as they all look very similar. Thanks for the
suggestion.

--
Regards,
Sylwester

Re: [PATCH RFC 2/7] mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers

[Philipp Zabel]

Hi Sylwester,

Am Sonntag, den 15.09.2013, 22:58 +0200 schrieb Sylwester Nawrocki:
> Hi Philipp,
> 
> On 09/13/2013 03:08 PM, Philipp Zabel wrote:
> > Am Freitag, den 13.09.2013, 14:56 +0200 schrieb Sylwester Nawrocki:
> [...]
> >> @@ -865,6 +793,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
> >>   	dst_vq->ops =&m2mtest_qops;
> >>   	dst_vq->mem_ops =&vb2_vmalloc_memops;
> >>   	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> >> +	dst_vq->lock =&ctx->dev->dev_mutex;
> >>
> >>   	return vb2_queue_init(dst_vq);
> >>   }
> >> @@ -945,6 +874,7 @@ static int m2mtest_open(struct file *file)
> >>   		kfree(ctx);
> >>   		goto open_unlock;
> >>   	}
> >> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
> >
> > Since you added m2m_ctx to v4l2_fh, why not drop ctx->m2m_ctx altogether
> > and always use ctx->fh.m2m_ctx instead?
> 
> Yes, that might make it a bit cleaner. I guess I wanted to minimize
> the necessary changes.

You are right. Converting the coda driver, I notice that depending on
how many occurences of ctx->m2m_ctx there are, it might be better to do
this in a separate patch.

> I'll amend that in all drivers in this series
> for the next iteration, as they all look very similar. Thanks for the
> suggestion.

regards
Philipp

Re: [PATCH RFC 2/7] mem2mem_testdev: Use mem-to-mem ioctl and vb2 helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/media/platform/mem2mem_testdev.c |   94 ++++--------------------------
>  1 file changed, 11 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/media/platform/mem2mem_testdev.c b/drivers/media/platform/mem2mem_testdev.c
> index 6a17676..73df44f 100644
> --- a/drivers/media/platform/mem2mem_testdev.c
> +++ b/drivers/media/platform/mem2mem_testdev.c
> @@ -359,21 +359,6 @@ static void job_abort(void *priv)
>  	ctx->aborting = 1;
>  }
>  
> -static void m2mtest_lock(void *priv)
> -{
> -	struct m2mtest_ctx *ctx = priv;
> -	struct m2mtest_dev *dev = ctx->dev;
> -	mutex_lock(&dev->dev_mutex);
> -}
> -
> -static void m2mtest_unlock(void *priv)
> -{
> -	struct m2mtest_ctx *ctx = priv;
> -	struct m2mtest_dev *dev = ctx->dev;
> -	mutex_unlock(&dev->dev_mutex);
> -}
> -
> -
>  /* device_run() - prepares and starts the device
>   *
>   * This simulates all the immediate preparations required before starting
> @@ -648,52 +633,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
>  	return ret;
>  }
>  
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -			  struct v4l2_requestbuffers *reqbufs)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -			   struct v4l2_buffer *buf)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> -			   enum v4l2_buf_type type)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> -			    enum v4l2_buf_type type)
> -{
> -	struct m2mtest_ctx *ctx = file2ctx(file);
> -
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int m2mtest_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct m2mtest_ctx *ctx =
> @@ -748,14 +687,14 @@ static const struct v4l2_ioctl_ops m2mtest_ioctl_ops = {
>  	.vidioc_try_fmt_vid_out	= vidioc_try_fmt_vid_out,
>  	.vidioc_s_fmt_vid_out	= vidioc_s_fmt_vid_out,
>  
> -	.vidioc_reqbufs		= vidioc_reqbufs,
> -	.vidioc_querybuf	= vidioc_querybuf,
> +	.vidioc_reqbufs		= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
>  
> -	.vidioc_qbuf		= vidioc_qbuf,
> -	.vidioc_dqbuf		= vidioc_dqbuf,
> +	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,
>  
> -	.vidioc_streamon	= vidioc_streamon,
> -	.vidioc_streamoff	= vidioc_streamoff,
>  	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>  	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
>  };
> @@ -821,24 +760,12 @@ static void m2mtest_buf_queue(struct vb2_buffer *vb)
>  	v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -static void m2mtest_wait_prepare(struct vb2_queue *q)
> -{
> -	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
> -	m2mtest_unlock(ctx);
> -}
> -
> -static void m2mtest_wait_finish(struct vb2_queue *q)
> -{
> -	struct m2mtest_ctx *ctx = vb2_get_drv_priv(q);
> -	m2mtest_lock(ctx);
> -}
> -
>  static struct vb2_ops m2mtest_qops = {
>  	.queue_setup	 = m2mtest_queue_setup,
>  	.buf_prepare	 = m2mtest_buf_prepare,
>  	.buf_queue	 = m2mtest_buf_queue,
> -	.wait_prepare	 = m2mtest_wait_prepare,
> -	.wait_finish	 = m2mtest_wait_finish,
> +	.wait_prepare	 = vb2_ops_wait_prepare,
> +	.wait_finish	 = vb2_ops_wait_finish,
>  };
>  
>  static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq)
> @@ -853,6 +780,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
>  	src_vq->ops = &m2mtest_qops;
>  	src_vq->mem_ops = &vb2_vmalloc_memops;
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->dev->dev_mutex;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -865,6 +793,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds
>  	dst_vq->ops = &m2mtest_qops;
>  	dst_vq->mem_ops = &vb2_vmalloc_memops;
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->dev->dev_mutex;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> @@ -945,6 +874,7 @@ static int m2mtest_open(struct file *file)
>  		kfree(ctx);
>  		goto open_unlock;
>  	}
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>  	v4l2_fh_add(&ctx->fh);
>  	atomic_inc(&dev->num_inst);
> @@ -1019,8 +949,6 @@ static struct v4l2_m2m_ops m2m_ops = {
>  	.device_run	= device_run,
>  	.job_ready	= job_ready,
>  	.job_abort	= job_abort,
> -	.lock		= m2mtest_lock,
> -	.unlock		= m2mtest_unlock,
>  };
>  
>  static int m2mtest_probe(struct platform_device *pdev)
> 

[PATCH RFC 3/7] exynos4-is: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Simplify the FIMC mem-to-mem driver by using the m2m ioctl and vb2 helpers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos4-is/fimc-m2m.c |  119 +++-----------------------
 1 file changed, 14 insertions(+), 105 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
index 8d33b68..71ee871 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -226,24 +226,12 @@ static void fimc_buf_queue(struct vb2_buffer *vb)
 		v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
 }
 
-static void fimc_lock(struct vb2_queue *vq)
-{
-	struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
-	mutex_lock(&ctx->fimc_dev->lock);
-}
-
-static void fimc_unlock(struct vb2_queue *vq)
-{
-	struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
-	mutex_unlock(&ctx->fimc_dev->lock);
-}
-
 static struct vb2_ops fimc_qops = {
 	.queue_setup	 = fimc_queue_setup,
 	.buf_prepare	 = fimc_buf_prepare,
 	.buf_queue	 = fimc_buf_queue,
-	.wait_prepare	 = fimc_unlock,
-	.wait_finish	 = fimc_lock,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
 	.stop_streaming	 = stop_streaming,
 	.start_streaming = start_streaming,
 };
@@ -410,56 +398,6 @@ static int fimc_m2m_s_fmt_mplane(struct file *file, void *fh,
 	return 0;
 }
 
-static int fimc_m2m_reqbufs(struct file *file, void *fh,
-			    struct v4l2_requestbuffers *reqbufs)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
-}
-
-static int fimc_m2m_querybuf(struct file *file, void *fh,
-			     struct v4l2_buffer *buf)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
-}
-
-static int fimc_m2m_qbuf(struct file *file, void *fh,
-			 struct v4l2_buffer *buf)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int fimc_m2m_dqbuf(struct file *file, void *fh,
-			  struct v4l2_buffer *buf)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int fimc_m2m_expbuf(struct file *file, void *fh,
-			    struct v4l2_exportbuffer *eb)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
-}
-
-
-static int fimc_m2m_streamon(struct file *file, void *fh,
-			     enum v4l2_buf_type type)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
-}
-
-static int fimc_m2m_streamoff(struct file *file, void *fh,
-			    enum v4l2_buf_type type)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
-}
-
 static int fimc_m2m_cropcap(struct file *file, void *fh,
 			    struct v4l2_cropcap *cr)
 {
@@ -598,13 +536,13 @@ static const struct v4l2_ioctl_ops fimc_m2m_ioctl_ops = {
 	.vidioc_try_fmt_vid_out_mplane	= fimc_m2m_try_fmt_mplane,
 	.vidioc_s_fmt_vid_cap_mplane	= fimc_m2m_s_fmt_mplane,
 	.vidioc_s_fmt_vid_out_mplane	= fimc_m2m_s_fmt_mplane,
-	.vidioc_reqbufs			= fimc_m2m_reqbufs,
-	.vidioc_querybuf		= fimc_m2m_querybuf,
-	.vidioc_qbuf			= fimc_m2m_qbuf,
-	.vidioc_dqbuf			= fimc_m2m_dqbuf,
-	.vidioc_expbuf			= fimc_m2m_expbuf,
-	.vidioc_streamon		= fimc_m2m_streamon,
-	.vidioc_streamoff		= fimc_m2m_streamoff,
+	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
+	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
+	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
+	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
+	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
 	.vidioc_g_crop			= fimc_m2m_g_crop,
 	.vidioc_s_crop			= fimc_m2m_s_crop,
 	.vidioc_cropcap			= fimc_m2m_cropcap
@@ -624,6 +562,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->fimc_dev->lock;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -636,6 +575,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->fimc_dev->lock;
 
 	return vb2_queue_init(dst_vq);
 }
@@ -713,6 +653,7 @@ static int fimc_m2m_open(struct file *file)
 		ret = PTR_ERR(ctx->m2m_ctx);
 		goto error_c;
 	}
+	ctx->fh.m2m_ctx = ctx->m2m_ctx;
 
 	if (fimc->m2m.refcnt++ == 0)
 		set_bit(ST_M2M_RUN, &fimc->state);
@@ -760,45 +701,13 @@ static int fimc_m2m_release(struct file *file)
 	return 0;
 }
 
-static unsigned int fimc_m2m_poll(struct file *file,
-				  struct poll_table_struct *wait)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(file->private_data);
-	struct fimc_dev *fimc = ctx->fimc_dev;
-	int ret;
-
-	if (mutex_lock_interruptible(&fimc->lock))
-		return -ERESTARTSYS;
-
-	ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
-	mutex_unlock(&fimc->lock);
-
-	return ret;
-}
-
-
-static int fimc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct fimc_ctx *ctx = fh_to_ctx(file->private_data);
-	struct fimc_dev *fimc = ctx->fimc_dev;
-	int ret;
-
-	if (mutex_lock_interruptible(&fimc->lock))
-		return -ERESTARTSYS;
-
-	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
-	mutex_unlock(&fimc->lock);
-
-	return ret;
-}
-
 static const struct v4l2_file_operations fimc_m2m_fops = {
 	.owner		= THIS_MODULE,
 	.open		= fimc_m2m_open,
 	.release	= fimc_m2m_release,
-	.poll		= fimc_m2m_poll,
+	.poll		= v4l2_m2m_fop_poll,
 	.unlocked_ioctl	= video_ioctl2,
-	.mmap		= fimc_m2m_mmap,
+	.mmap		= v4l2_m2m_fop_mmap,
 };
 
 static struct v4l2_m2m_ops m2m_ops = {
-- 
1.7.9.5

Re: [PATCH RFC 3/7] exynos4-is: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the FIMC mem-to-mem driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/media/platform/exynos4-is/fimc-m2m.c |  119 +++-----------------------
>  1 file changed, 14 insertions(+), 105 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c b/drivers/media/platform/exynos4-is/fimc-m2m.c
> index 8d33b68..71ee871 100644
> --- a/drivers/media/platform/exynos4-is/fimc-m2m.c
> +++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
> @@ -226,24 +226,12 @@ static void fimc_buf_queue(struct vb2_buffer *vb)
>  		v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -static void fimc_lock(struct vb2_queue *vq)
> -{
> -	struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
> -	mutex_lock(&ctx->fimc_dev->lock);
> -}
> -
> -static void fimc_unlock(struct vb2_queue *vq)
> -{
> -	struct fimc_ctx *ctx = vb2_get_drv_priv(vq);
> -	mutex_unlock(&ctx->fimc_dev->lock);
> -}
> -
>  static struct vb2_ops fimc_qops = {
>  	.queue_setup	 = fimc_queue_setup,
>  	.buf_prepare	 = fimc_buf_prepare,
>  	.buf_queue	 = fimc_buf_queue,
> -	.wait_prepare	 = fimc_unlock,
> -	.wait_finish	 = fimc_lock,
> +	.wait_prepare	 = vb2_ops_wait_prepare,
> +	.wait_finish	 = vb2_ops_wait_finish,
>  	.stop_streaming	 = stop_streaming,
>  	.start_streaming = start_streaming,
>  };
> @@ -410,56 +398,6 @@ static int fimc_m2m_s_fmt_mplane(struct file *file, void *fh,
>  	return 0;
>  }
>  
> -static int fimc_m2m_reqbufs(struct file *file, void *fh,
> -			    struct v4l2_requestbuffers *reqbufs)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int fimc_m2m_querybuf(struct file *file, void *fh,
> -			     struct v4l2_buffer *buf)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int fimc_m2m_qbuf(struct file *file, void *fh,
> -			 struct v4l2_buffer *buf)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int fimc_m2m_dqbuf(struct file *file, void *fh,
> -			  struct v4l2_buffer *buf)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int fimc_m2m_expbuf(struct file *file, void *fh,
> -			    struct v4l2_exportbuffer *eb)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
> -}
> -
> -
> -static int fimc_m2m_streamon(struct file *file, void *fh,
> -			     enum v4l2_buf_type type)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int fimc_m2m_streamoff(struct file *file, void *fh,
> -			    enum v4l2_buf_type type)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int fimc_m2m_cropcap(struct file *file, void *fh,
>  			    struct v4l2_cropcap *cr)
>  {
> @@ -598,13 +536,13 @@ static const struct v4l2_ioctl_ops fimc_m2m_ioctl_ops = {
>  	.vidioc_try_fmt_vid_out_mplane	= fimc_m2m_try_fmt_mplane,
>  	.vidioc_s_fmt_vid_cap_mplane	= fimc_m2m_s_fmt_mplane,
>  	.vidioc_s_fmt_vid_out_mplane	= fimc_m2m_s_fmt_mplane,
> -	.vidioc_reqbufs			= fimc_m2m_reqbufs,
> -	.vidioc_querybuf		= fimc_m2m_querybuf,
> -	.vidioc_qbuf			= fimc_m2m_qbuf,
> -	.vidioc_dqbuf			= fimc_m2m_dqbuf,
> -	.vidioc_expbuf			= fimc_m2m_expbuf,
> -	.vidioc_streamon		= fimc_m2m_streamon,
> -	.vidioc_streamoff		= fimc_m2m_streamoff,
> +	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
> +	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
> +	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
>  	.vidioc_g_crop			= fimc_m2m_g_crop,
>  	.vidioc_s_crop			= fimc_m2m_s_crop,
>  	.vidioc_cropcap			= fimc_m2m_cropcap
> @@ -624,6 +562,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->mem_ops = &vb2_dma_contig_memops;
>  	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->fimc_dev->lock;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -636,6 +575,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
>  	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->fimc_dev->lock;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> @@ -713,6 +653,7 @@ static int fimc_m2m_open(struct file *file)
>  		ret = PTR_ERR(ctx->m2m_ctx);
>  		goto error_c;
>  	}
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>  	if (fimc->m2m.refcnt++ == 0)
>  		set_bit(ST_M2M_RUN, &fimc->state);
> @@ -760,45 +701,13 @@ static int fimc_m2m_release(struct file *file)
>  	return 0;
>  }
>  
> -static unsigned int fimc_m2m_poll(struct file *file,
> -				  struct poll_table_struct *wait)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(file->private_data);
> -	struct fimc_dev *fimc = ctx->fimc_dev;
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&fimc->lock))
> -		return -ERESTARTSYS;
> -
> -	ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> -	mutex_unlock(&fimc->lock);
> -
> -	return ret;
> -}
> -
> -
> -static int fimc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> -	struct fimc_ctx *ctx = fh_to_ctx(file->private_data);
> -	struct fimc_dev *fimc = ctx->fimc_dev;
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&fimc->lock))
> -		return -ERESTARTSYS;
> -
> -	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> -	mutex_unlock(&fimc->lock);
> -
> -	return ret;
> -}
> -
>  static const struct v4l2_file_operations fimc_m2m_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= fimc_m2m_open,
>  	.release	= fimc_m2m_release,
> -	.poll		= fimc_m2m_poll,
> +	.poll		= v4l2_m2m_fop_poll,
>  	.unlocked_ioctl	= video_ioctl2,
> -	.mmap		= fimc_m2m_mmap,
> +	.mmap		= v4l2_m2m_fop_mmap,
>  };
>  
>  static struct v4l2_m2m_ops m2m_ops = {
> 

[PATCH RFC 4/7] s5p-jpeg: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Simplify the driver by using the m2m ioctl and vb2 helpers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-jpeg/jpeg-core.c |  111 ++++-----------------------
 1 file changed, 13 insertions(+), 98 deletions(-)

diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index 88c5beb..66f7519 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -322,6 +322,7 @@ static int s5p_jpeg_open(struct file *file)
 		ret = PTR_ERR(ctx->m2m_ctx);
 		goto error;
 	}
+	ctx->fh.m2m_ctx = ctx->m2m_ctx;
 
 	ctx->out_q.fmt = out_fmt;
 	ctx->cap_q.fmt = s5p_jpeg_find_format(ctx->mode, V4L2_PIX_FMT_YUYV);
@@ -353,39 +354,13 @@ static int s5p_jpeg_release(struct file *file)
 	return 0;
 }
 
-static unsigned int s5p_jpeg_poll(struct file *file,
-				 struct poll_table_struct *wait)
-{
-	struct s5p_jpeg *jpeg = video_drvdata(file);
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
-	unsigned int res;
-
-	mutex_lock(&jpeg->lock);
-	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
-	mutex_unlock(&jpeg->lock);
-	return res;
-}
-
-static int s5p_jpeg_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct s5p_jpeg *jpeg = video_drvdata(file);
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
-	int ret;
-
-	if (mutex_lock_interruptible(&jpeg->lock))
-		return -ERESTARTSYS;
-	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
-	mutex_unlock(&jpeg->lock);
-	return ret;
-}
-
 static const struct v4l2_file_operations s5p_jpeg_fops = {
 	.owner		= THIS_MODULE,
 	.open		= s5p_jpeg_open,
 	.release	= s5p_jpeg_release,
-	.poll		= s5p_jpeg_poll,
+	.poll		= v4l2_m2m_fop_poll,
 	.unlocked_ioctl	= video_ioctl2,
-	.mmap		= s5p_jpeg_mmap,
+	.mmap		= v4l2_m2m_fop_mmap,
 };
 
 /*
@@ -793,53 +768,6 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, void *priv,
 	return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
 }
 
-static int s5p_jpeg_reqbufs(struct file *file, void *priv,
-			  struct v4l2_requestbuffers *reqbufs)
-{
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
-
-	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
-}
-
-static int s5p_jpeg_querybuf(struct file *file, void *priv,
-			   struct v4l2_buffer *buf)
-{
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
-
-	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
-}
-
-static int s5p_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
-
-	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int s5p_jpeg_dqbuf(struct file *file, void *priv,
-			  struct v4l2_buffer *buf)
-{
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
-
-	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int s5p_jpeg_streamon(struct file *file, void *priv,
-			   enum v4l2_buf_type type)
-{
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
-
-	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
-}
-
-static int s5p_jpeg_streamoff(struct file *file, void *priv,
-			    enum v4l2_buf_type type)
-{
-	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
-
-	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
-}
-
 static int s5p_jpeg_g_selection(struct file *file, void *priv,
 			 struct v4l2_selection *s)
 {
@@ -973,14 +901,13 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = {
 	.vidioc_s_fmt_vid_cap		= s5p_jpeg_s_fmt_vid_cap,
 	.vidioc_s_fmt_vid_out		= s5p_jpeg_s_fmt_vid_out,
 
-	.vidioc_reqbufs			= s5p_jpeg_reqbufs,
-	.vidioc_querybuf		= s5p_jpeg_querybuf,
-
-	.vidioc_qbuf			= s5p_jpeg_qbuf,
-	.vidioc_dqbuf			= s5p_jpeg_dqbuf,
+	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
+	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
+	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
 
-	.vidioc_streamon		= s5p_jpeg_streamon,
-	.vidioc_streamoff		= s5p_jpeg_streamoff,
+	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
 
 	.vidioc_g_selection		= s5p_jpeg_g_selection,
 };
@@ -1175,20 +1102,6 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
 		v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
 }
 
-static void s5p_jpeg_wait_prepare(struct vb2_queue *vq)
-{
-	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(vq);
-
-	mutex_unlock(&ctx->jpeg->lock);
-}
-
-static void s5p_jpeg_wait_finish(struct vb2_queue *vq)
-{
-	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(vq);
-
-	mutex_lock(&ctx->jpeg->lock);
-}
-
 static int s5p_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(q);
@@ -1212,8 +1125,8 @@ static struct vb2_ops s5p_jpeg_qops = {
 	.queue_setup		= s5p_jpeg_queue_setup,
 	.buf_prepare		= s5p_jpeg_buf_prepare,
 	.buf_queue		= s5p_jpeg_buf_queue,
-	.wait_prepare		= s5p_jpeg_wait_prepare,
-	.wait_finish		= s5p_jpeg_wait_finish,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
 	.start_streaming	= s5p_jpeg_start_streaming,
 	.stop_streaming		= s5p_jpeg_stop_streaming,
 };
@@ -1231,6 +1144,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->ops = &s5p_jpeg_qops;
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->jpeg->lock;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -1243,6 +1157,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->ops = &s5p_jpeg_qops;
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->jpeg->lock;
 
 	return vb2_queue_init(dst_vq);
 }
-- 
1.7.9.5

Re: [PATCH RFC 4/7] s5p-jpeg: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>


Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/media/platform/s5p-jpeg/jpeg-core.c |  111 ++++-----------------------
>  1 file changed, 13 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> index 88c5beb..66f7519 100644
> --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
> +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
> @@ -322,6 +322,7 @@ static int s5p_jpeg_open(struct file *file)
>  		ret = PTR_ERR(ctx->m2m_ctx);
>  		goto error;
>  	}
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>  	ctx->out_q.fmt = out_fmt;
>  	ctx->cap_q.fmt = s5p_jpeg_find_format(ctx->mode, V4L2_PIX_FMT_YUYV);
> @@ -353,39 +354,13 @@ static int s5p_jpeg_release(struct file *file)
>  	return 0;
>  }
>  
> -static unsigned int s5p_jpeg_poll(struct file *file,
> -				 struct poll_table_struct *wait)
> -{
> -	struct s5p_jpeg *jpeg = video_drvdata(file);
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
> -	unsigned int res;
> -
> -	mutex_lock(&jpeg->lock);
> -	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> -	mutex_unlock(&jpeg->lock);
> -	return res;
> -}
> -
> -static int s5p_jpeg_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> -	struct s5p_jpeg *jpeg = video_drvdata(file);
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(file->private_data);
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&jpeg->lock))
> -		return -ERESTARTSYS;
> -	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> -	mutex_unlock(&jpeg->lock);
> -	return ret;
> -}
> -
>  static const struct v4l2_file_operations s5p_jpeg_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= s5p_jpeg_open,
>  	.release	= s5p_jpeg_release,
> -	.poll		= s5p_jpeg_poll,
> +	.poll		= v4l2_m2m_fop_poll,
>  	.unlocked_ioctl	= video_ioctl2,
> -	.mmap		= s5p_jpeg_mmap,
> +	.mmap		= v4l2_m2m_fop_mmap,
>  };
>  
>  /*
> @@ -793,53 +768,6 @@ static int s5p_jpeg_s_fmt_vid_out(struct file *file, void *priv,
>  	return s5p_jpeg_s_fmt(fh_to_ctx(priv), f);
>  }
>  
> -static int s5p_jpeg_reqbufs(struct file *file, void *priv,
> -			  struct v4l2_requestbuffers *reqbufs)
> -{
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> -	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int s5p_jpeg_querybuf(struct file *file, void *priv,
> -			   struct v4l2_buffer *buf)
> -{
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int s5p_jpeg_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int s5p_jpeg_dqbuf(struct file *file, void *priv,
> -			  struct v4l2_buffer *buf)
> -{
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int s5p_jpeg_streamon(struct file *file, void *priv,
> -			   enum v4l2_buf_type type)
> -{
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int s5p_jpeg_streamoff(struct file *file, void *priv,
> -			    enum v4l2_buf_type type)
> -{
> -	struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
> -
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int s5p_jpeg_g_selection(struct file *file, void *priv,
>  			 struct v4l2_selection *s)
>  {
> @@ -973,14 +901,13 @@ static const struct v4l2_ioctl_ops s5p_jpeg_ioctl_ops = {
>  	.vidioc_s_fmt_vid_cap		= s5p_jpeg_s_fmt_vid_cap,
>  	.vidioc_s_fmt_vid_out		= s5p_jpeg_s_fmt_vid_out,
>  
> -	.vidioc_reqbufs			= s5p_jpeg_reqbufs,
> -	.vidioc_querybuf		= s5p_jpeg_querybuf,
> -
> -	.vidioc_qbuf			= s5p_jpeg_qbuf,
> -	.vidioc_dqbuf			= s5p_jpeg_dqbuf,
> +	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
>  
> -	.vidioc_streamon		= s5p_jpeg_streamon,
> -	.vidioc_streamoff		= s5p_jpeg_streamoff,
> +	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
>  
>  	.vidioc_g_selection		= s5p_jpeg_g_selection,
>  };
> @@ -1175,20 +1102,6 @@ static void s5p_jpeg_buf_queue(struct vb2_buffer *vb)
>  		v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -static void s5p_jpeg_wait_prepare(struct vb2_queue *vq)
> -{
> -	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(vq);
> -
> -	mutex_unlock(&ctx->jpeg->lock);
> -}
> -
> -static void s5p_jpeg_wait_finish(struct vb2_queue *vq)
> -{
> -	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(vq);
> -
> -	mutex_lock(&ctx->jpeg->lock);
> -}
> -
>  static int s5p_jpeg_start_streaming(struct vb2_queue *q, unsigned int count)
>  {
>  	struct s5p_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> @@ -1212,8 +1125,8 @@ static struct vb2_ops s5p_jpeg_qops = {
>  	.queue_setup		= s5p_jpeg_queue_setup,
>  	.buf_prepare		= s5p_jpeg_buf_prepare,
>  	.buf_queue		= s5p_jpeg_buf_queue,
> -	.wait_prepare		= s5p_jpeg_wait_prepare,
> -	.wait_finish		= s5p_jpeg_wait_finish,
> +	.wait_prepare		= vb2_ops_wait_prepare,
> +	.wait_finish		= vb2_ops_wait_finish,
>  	.start_streaming	= s5p_jpeg_start_streaming,
>  	.stop_streaming		= s5p_jpeg_stop_streaming,
>  };
> @@ -1231,6 +1144,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->ops = &s5p_jpeg_qops;
>  	src_vq->mem_ops = &vb2_dma_contig_memops;
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->jpeg->lock;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -1243,6 +1157,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	dst_vq->ops = &s5p_jpeg_qops;
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->jpeg->lock;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> 

[PATCH RFC 5/7] mx2-emmaprp: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Simplify the driver by using the m2m ioctl and vb2 helpers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/mx2_emmaprp.c |  108 ++++------------------------------
 1 file changed, 11 insertions(+), 97 deletions(-)

diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
index c690435..a531862 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -253,20 +253,6 @@ static void emmaprp_job_abort(void *priv)
 	v4l2_m2m_job_finish(pcdev->m2m_dev, ctx->m2m_ctx);
 }
 
-static void emmaprp_lock(void *priv)
-{
-	struct emmaprp_ctx *ctx = priv;
-	struct emmaprp_dev *pcdev = ctx->dev;
-	mutex_lock(&pcdev->dev_mutex);
-}
-
-static void emmaprp_unlock(void *priv)
-{
-	struct emmaprp_ctx *ctx = priv;
-	struct emmaprp_dev *pcdev = ctx->dev;
-	mutex_unlock(&pcdev->dev_mutex);
-}
-
 static inline void emmaprp_dump_regs(struct emmaprp_dev *pcdev)
 {
 	dprintk(pcdev,
@@ -617,52 +603,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
 	return vidioc_s_fmt(priv, f);
 }
 
-static int vidioc_reqbufs(struct file *file, void *priv,
-			  struct v4l2_requestbuffers *reqbufs)
-{
-	struct emmaprp_ctx *ctx = priv;
-
-	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
-}
-
-static int vidioc_querybuf(struct file *file, void *priv,
-			   struct v4l2_buffer *buf)
-{
-	struct emmaprp_ctx *ctx = priv;
-
-	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct emmaprp_ctx *ctx = priv;
-
-	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct emmaprp_ctx *ctx = priv;
-
-	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_streamon(struct file *file, void *priv,
-			   enum v4l2_buf_type type)
-{
-	struct emmaprp_ctx *ctx = priv;
-
-	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
-}
-
-static int vidioc_streamoff(struct file *file, void *priv,
-			    enum v4l2_buf_type type)
-{
-	struct emmaprp_ctx *ctx = priv;
-
-	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
-}
-
 static const struct v4l2_ioctl_ops emmaprp_ioctl_ops = {
 	.vidioc_querycap	= vidioc_querycap,
 
@@ -676,14 +616,13 @@ static const struct v4l2_ioctl_ops emmaprp_ioctl_ops = {
 	.vidioc_try_fmt_vid_out	= vidioc_try_fmt_vid_out,
 	.vidioc_s_fmt_vid_out	= vidioc_s_fmt_vid_out,
 
-	.vidioc_reqbufs		= vidioc_reqbufs,
-	.vidioc_querybuf	= vidioc_querybuf,
-
-	.vidioc_qbuf		= vidioc_qbuf,
-	.vidioc_dqbuf		= vidioc_dqbuf,
+	.vidioc_reqbufs		= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
+	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
+	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
 
-	.vidioc_streamon	= vidioc_streamon,
-	.vidioc_streamoff	= vidioc_streamoff,
+	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,
 };
 
 
@@ -767,6 +706,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->ops = &emmaprp_qops;
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->dev->dev_mutex;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -779,6 +719,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->ops = &emmaprp_qops;
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->dev->dev_mutex;
 
 	return vb2_queue_init(dst_vq);
 }
@@ -812,6 +753,7 @@ static int emmaprp_open(struct file *file)
 		kfree(ctx);
 		return ret;
 	}
+	/* TODO: Assign fh->m2m_ctx, needs conversion to struct v4l2_fh first */
 
 	clk_prepare_enable(pcdev->clk_emma_ipg);
 	clk_prepare_enable(pcdev->clk_emma_ahb);
@@ -841,39 +783,13 @@ static int emmaprp_release(struct file *file)
 	return 0;
 }
 
-static unsigned int emmaprp_poll(struct file *file,
-				 struct poll_table_struct *wait)
-{
-	struct emmaprp_dev *pcdev = video_drvdata(file);
-	struct emmaprp_ctx *ctx = file->private_data;
-	unsigned int res;
-
-	mutex_lock(&pcdev->dev_mutex);
-	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
-	mutex_unlock(&pcdev->dev_mutex);
-	return res;
-}
-
-static int emmaprp_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct emmaprp_dev *pcdev = video_drvdata(file);
-	struct emmaprp_ctx *ctx = file->private_data;
-	int ret;
-
-	if (mutex_lock_interruptible(&pcdev->dev_mutex))
-		return -ERESTARTSYS;
-	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
-	mutex_unlock(&pcdev->dev_mutex);
-	return ret;
-}
-
 static const struct v4l2_file_operations emmaprp_fops = {
 	.owner		= THIS_MODULE,
 	.open		= emmaprp_open,
 	.release	= emmaprp_release,
-	.poll		= emmaprp_poll,
+	.poll		= v4l2_m2m_fop_poll,
 	.unlocked_ioctl	= video_ioctl2,
-	.mmap		= emmaprp_mmap,
+	.mmap		= v4l2_m2m_fop_mmap,
 };
 
 static struct video_device emmaprp_videodev = {
@@ -888,8 +804,6 @@ static struct video_device emmaprp_videodev = {
 static struct v4l2_m2m_ops m2m_ops = {
 	.device_run	= emmaprp_device_run,
 	.job_abort	= emmaprp_job_abort,
-	.lock		= emmaprp_lock,
-	.unlock		= emmaprp_unlock,
 };
 
 static int emmaprp_probe(struct platform_device *pdev)
-- 
1.7.9.5

Re: [PATCH RFC 5/7] mx2-emmaprp: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/platform/mx2_emmaprp.c |  108 ++++------------------------------
>  1 file changed, 11 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/media/platform/mx2_emmaprp.c b/drivers/media/platform/mx2_emmaprp.c
> index c690435..a531862 100644
> --- a/drivers/media/platform/mx2_emmaprp.c
> +++ b/drivers/media/platform/mx2_emmaprp.c
> @@ -253,20 +253,6 @@ static void emmaprp_job_abort(void *priv)
>  	v4l2_m2m_job_finish(pcdev->m2m_dev, ctx->m2m_ctx);
>  }
>  
> -static void emmaprp_lock(void *priv)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -	struct emmaprp_dev *pcdev = ctx->dev;
> -	mutex_lock(&pcdev->dev_mutex);
> -}
> -
> -static void emmaprp_unlock(void *priv)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -	struct emmaprp_dev *pcdev = ctx->dev;
> -	mutex_unlock(&pcdev->dev_mutex);
> -}
> -
>  static inline void emmaprp_dump_regs(struct emmaprp_dev *pcdev)
>  {
>  	dprintk(pcdev,
> @@ -617,52 +603,6 @@ static int vidioc_s_fmt_vid_out(struct file *file, void *priv,
>  	return vidioc_s_fmt(priv, f);
>  }
>  
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -			  struct v4l2_requestbuffers *reqbufs)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -
> -	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -			   struct v4l2_buffer *buf)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> -			   enum v4l2_buf_type type)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> -			    enum v4l2_buf_type type)
> -{
> -	struct emmaprp_ctx *ctx = priv;
> -
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static const struct v4l2_ioctl_ops emmaprp_ioctl_ops = {
>  	.vidioc_querycap	= vidioc_querycap,
>  
> @@ -676,14 +616,13 @@ static const struct v4l2_ioctl_ops emmaprp_ioctl_ops = {
>  	.vidioc_try_fmt_vid_out	= vidioc_try_fmt_vid_out,
>  	.vidioc_s_fmt_vid_out	= vidioc_s_fmt_vid_out,
>  
> -	.vidioc_reqbufs		= vidioc_reqbufs,
> -	.vidioc_querybuf	= vidioc_querybuf,
> -
> -	.vidioc_qbuf		= vidioc_qbuf,
> -	.vidioc_dqbuf		= vidioc_dqbuf,
> +	.vidioc_reqbufs		= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf		= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf		= v4l2_m2m_ioctl_dqbuf,
>  
> -	.vidioc_streamon	= vidioc_streamon,
> -	.vidioc_streamoff	= vidioc_streamoff,
> +	.vidioc_streamon	= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff	= v4l2_m2m_ioctl_streamoff,
>  };
>  
>  
> @@ -767,6 +706,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->ops = &emmaprp_qops;
>  	src_vq->mem_ops = &vb2_dma_contig_memops;
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->dev->dev_mutex;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -779,6 +719,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	dst_vq->ops = &emmaprp_qops;
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->dev->dev_mutex;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> @@ -812,6 +753,7 @@ static int emmaprp_open(struct file *file)
>  		kfree(ctx);
>  		return ret;
>  	}
> +	/* TODO: Assign fh->m2m_ctx, needs conversion to struct v4l2_fh first */

What's the point of adding this patch if it won't work yet?

Regards,

	Hans

>  
>  	clk_prepare_enable(pcdev->clk_emma_ipg);
>  	clk_prepare_enable(pcdev->clk_emma_ahb);
> @@ -841,39 +783,13 @@ static int emmaprp_release(struct file *file)
>  	return 0;
>  }
>  
> -static unsigned int emmaprp_poll(struct file *file,
> -				 struct poll_table_struct *wait)
> -{
> -	struct emmaprp_dev *pcdev = video_drvdata(file);
> -	struct emmaprp_ctx *ctx = file->private_data;
> -	unsigned int res;
> -
> -	mutex_lock(&pcdev->dev_mutex);
> -	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> -	mutex_unlock(&pcdev->dev_mutex);
> -	return res;
> -}
> -
> -static int emmaprp_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> -	struct emmaprp_dev *pcdev = video_drvdata(file);
> -	struct emmaprp_ctx *ctx = file->private_data;
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&pcdev->dev_mutex))
> -		return -ERESTARTSYS;
> -	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> -	mutex_unlock(&pcdev->dev_mutex);
> -	return ret;
> -}
> -
>  static const struct v4l2_file_operations emmaprp_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= emmaprp_open,
>  	.release	= emmaprp_release,
> -	.poll		= emmaprp_poll,
> +	.poll		= v4l2_m2m_fop_poll,
>  	.unlocked_ioctl	= video_ioctl2,
> -	.mmap		= emmaprp_mmap,
> +	.mmap		= v4l2_m2m_fop_mmap,
>  };
>  
>  static struct video_device emmaprp_videodev = {
> @@ -888,8 +804,6 @@ static struct video_device emmaprp_videodev = {
>  static struct v4l2_m2m_ops m2m_ops = {
>  	.device_run	= emmaprp_device_run,
>  	.job_abort	= emmaprp_job_abort,
> -	.lock		= emmaprp_lock,
> -	.unlock		= emmaprp_unlock,
>  };
>  
>  static int emmaprp_probe(struct platform_device *pdev)
> 

Re: [PATCH RFC 5/7] mx2-emmaprp: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

On 09/30/2013 11:47 AM, Hans Verkuil wrote:
> On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
[...]
>> @@ -812,6 +753,7 @@ static int emmaprp_open(struct file *file)
>>   		kfree(ctx);
>>   		return ret;
>>   	}
>> +	/* TODO: Assign fh->m2m_ctx, needs conversion to struct v4l2_fh first */
>
> What's the point of adding this patch if it won't work yet?

Sorry, I shouldn't have included that commit in this series yet, that's my
oversight. And it didn't prove a good way to draw people's attention to
this RFC series either. ;)
Nevertheless, I have already written the missing patch and will be sending
v2 of this series shortly.

--
Thanks,
Sylwester

[PATCH RFC 6/7] exynos-gsc: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Simplify the driver by using the m2m ioctl and vb2 helpers.

TODO: Add setting of default initial format.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/exynos-gsc/gsc-core.h |   12 ---
 drivers/media/platform/exynos-gsc/gsc-m2m.c  |  109 ++++----------------------
 2 files changed, 16 insertions(+), 105 deletions(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
index cc19bba..1afad32 100644
--- a/drivers/media/platform/exynos-gsc/gsc-core.h
+++ b/drivers/media/platform/exynos-gsc/gsc-core.h
@@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, int irq)
 	writel(cfg, dev->regs + GSC_IRQ);
 }
 
-static inline void gsc_lock(struct vb2_queue *vq)
-{
-	struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
-	mutex_lock(&ctx->gsc_dev->lock);
-}
-
-static inline void gsc_unlock(struct vb2_queue *vq)
-{
-	struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
-	mutex_unlock(&ctx->gsc_dev->lock);
-}
-
 static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx)
 {
 	unsigned long flags;
diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index 40a73f7..4f5d6cb 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = {
 	.queue_setup	 = gsc_m2m_queue_setup,
 	.buf_prepare	 = gsc_m2m_buf_prepare,
 	.buf_queue	 = gsc_m2m_buf_queue,
-	.wait_prepare	 = gsc_unlock,
-	.wait_finish	 = gsc_lock,
+	.wait_prepare	 = vb2_ops_wait_prepare,
+	.wait_finish	 = vb2_ops_wait_finish,
 	.stop_streaming	 = gsc_m2m_stop_streaming,
 	.start_streaming = gsc_m2m_start_streaming,
 };
@@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh,
 	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
 }
 
-static int gsc_m2m_expbuf(struct file *file, void *fh,
-				struct v4l2_exportbuffer *eb)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
-}
-
-static int gsc_m2m_querybuf(struct file *file, void *fh,
-					struct v4l2_buffer *buf)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
-}
-
-static int gsc_m2m_qbuf(struct file *file, void *fh,
-			  struct v4l2_buffer *buf)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int gsc_m2m_dqbuf(struct file *file, void *fh,
-			   struct v4l2_buffer *buf)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int gsc_m2m_streamon(struct file *file, void *fh,
-			   enum v4l2_buf_type type)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(fh);
-
-	/* The source and target color format need to be set */
-	if (V4L2_TYPE_IS_OUTPUT(type)) {
-		if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx))
-			return -EINVAL;
-	} else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) {
-		return -EINVAL;
-	}
-
-	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
-}
-
-static int gsc_m2m_streamoff(struct file *file, void *fh,
-			    enum v4l2_buf_type type)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(fh);
-	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
-}
-
 /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */
 static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b)
 {
@@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = {
 	.vidioc_try_fmt_vid_out_mplane	= gsc_m2m_try_fmt_mplane,
 	.vidioc_s_fmt_vid_cap_mplane	= gsc_m2m_s_fmt_mplane,
 	.vidioc_s_fmt_vid_out_mplane	= gsc_m2m_s_fmt_mplane,
-	.vidioc_reqbufs			= gsc_m2m_reqbufs,
-	.vidioc_expbuf                  = gsc_m2m_expbuf,
-	.vidioc_querybuf		= gsc_m2m_querybuf,
-	.vidioc_qbuf			= gsc_m2m_qbuf,
-	.vidioc_dqbuf			= gsc_m2m_dqbuf,
-	.vidioc_streamon		= gsc_m2m_streamon,
-	.vidioc_streamoff		= gsc_m2m_streamoff,
+
+	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
+	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
+	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
+	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
+
+	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
 	.vidioc_g_selection		= gsc_m2m_g_selection,
 	.vidioc_s_selection		= gsc_m2m_s_selection
 };
@@ -588,6 +539,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->gsc_dev->lock;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -601,6 +553,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->gsc_dev->lock;
 
 	return vb2_queue_init(dst_vq);
 }
@@ -648,6 +601,7 @@ static int gsc_m2m_open(struct file *file)
 		ret = PTR_ERR(ctx->m2m_ctx);
 		goto error_ctrls;
 	}
+	ctx->fh.m2m_ctx = ctx->m2m_ctx;
 
 	if (gsc->m2m.refcnt++ == 0)
 		set_bit(ST_M2M_OPEN, &gsc->state);
@@ -691,44 +645,13 @@ static int gsc_m2m_release(struct file *file)
 	return 0;
 }
 
-static unsigned int gsc_m2m_poll(struct file *file,
-					struct poll_table_struct *wait)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
-	struct gsc_dev *gsc = ctx->gsc_dev;
-	int ret;
-
-	if (mutex_lock_interruptible(&gsc->lock))
-		return -ERESTARTSYS;
-
-	ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
-	mutex_unlock(&gsc->lock);
-
-	return ret;
-}
-
-static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
-	struct gsc_dev *gsc = ctx->gsc_dev;
-	int ret;
-
-	if (mutex_lock_interruptible(&gsc->lock))
-		return -ERESTARTSYS;
-
-	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
-	mutex_unlock(&gsc->lock);
-
-	return ret;
-}
-
 static const struct v4l2_file_operations gsc_m2m_fops = {
 	.owner		= THIS_MODULE,
 	.open		= gsc_m2m_open,
 	.release	= gsc_m2m_release,
-	.poll		= gsc_m2m_poll,
+	.poll		= v4l2_m2m_fop_poll,
 	.unlocked_ioctl	= video_ioctl2,
-	.mmap		= gsc_m2m_mmap,
+	.mmap		= v4l2_m2m_fop_mmap,
 };
 
 static struct v4l2_m2m_ops gsc_m2m_ops = {
-- 
1.7.9.5

Re: [PATCH RFC 6/7] exynos-gsc: Use mem-to-mem ioctl helpers

[Shaik Ameer Basha]

Hi Sylwester,

On Fri, Sep 13, 2013 at 6:26 PM, Sylwester Nawrocki
<s.nawrocki@samsung.com> wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
>
> TODO: Add setting of default initial format.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/platform/exynos-gsc/gsc-core.h |   12 ---
>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |  109 ++++----------------------
>  2 files changed, 16 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
> index cc19bba..1afad32 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
> @@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, int irq)
>         writel(cfg, dev->regs + GSC_IRQ);
>  }
>
> -static inline void gsc_lock(struct vb2_queue *vq)
> -{
> -       struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
> -       mutex_lock(&ctx->gsc_dev->lock);
> -}
> -
> -static inline void gsc_unlock(struct vb2_queue *vq)
> -{
> -       struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
> -       mutex_unlock(&ctx->gsc_dev->lock);
> -}
> -
>  static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx)
>  {
>         unsigned long flags;
> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> index 40a73f7..4f5d6cb 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> @@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = {
>         .queue_setup     = gsc_m2m_queue_setup,
>         .buf_prepare     = gsc_m2m_buf_prepare,
>         .buf_queue       = gsc_m2m_buf_queue,
> -       .wait_prepare    = gsc_unlock,
> -       .wait_finish     = gsc_lock,
> +       .wait_prepare    = vb2_ops_wait_prepare,
> +       .wait_finish     = vb2_ops_wait_finish,
>         .stop_streaming  = gsc_m2m_stop_streaming,
>         .start_streaming = gsc_m2m_start_streaming,
>  };
> @@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh,
>         return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
>  }
>
> -static int gsc_m2m_expbuf(struct file *file, void *fh,
> -                               struct v4l2_exportbuffer *eb)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
> -       return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
> -}
> -
> -static int gsc_m2m_querybuf(struct file *file, void *fh,
> -                                       struct v4l2_buffer *buf)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
> -       return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_qbuf(struct file *file, void *fh,
> -                         struct v4l2_buffer *buf)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
> -       return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_dqbuf(struct file *file, void *fh,
> -                          struct v4l2_buffer *buf)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
> -       return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_streamon(struct file *file, void *fh,
> -                          enum v4l2_buf_type type)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
> -
> -       /* The source and target color format need to be set */
> -       if (V4L2_TYPE_IS_OUTPUT(type)) {
> -               if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx))
> -                       return -EINVAL;
> -       } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) {
> -               return -EINVAL;
> -       }
> -
> -       return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int gsc_m2m_streamoff(struct file *file, void *fh,
> -                           enum v4l2_buf_type type)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
> -       return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */
>  static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b)
>  {
> @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = {
>         .vidioc_try_fmt_vid_out_mplane  = gsc_m2m_try_fmt_mplane,
>         .vidioc_s_fmt_vid_cap_mplane    = gsc_m2m_s_fmt_mplane,
>         .vidioc_s_fmt_vid_out_mplane    = gsc_m2m_s_fmt_mplane,
> -       .vidioc_reqbufs                 = gsc_m2m_reqbufs,
> -       .vidioc_expbuf                  = gsc_m2m_expbuf,
> -       .vidioc_querybuf                = gsc_m2m_querybuf,
> -       .vidioc_qbuf                    = gsc_m2m_qbuf,
> -       .vidioc_dqbuf                   = gsc_m2m_dqbuf,
> -       .vidioc_streamon                = gsc_m2m_streamon,
> -       .vidioc_streamoff               = gsc_m2m_streamoff,
> +
> +       .vidioc_reqbufs                 = v4l2_m2m_ioctl_reqbufs,

I think your intention was not to replace gsc_m2m_reqbufs() with
v4l2_m2m_ioctl_reqbufs().
you didn't remove the gsc_m2m_reqbufs() function :)

On top of that,  gsc_m2m_reqbufs() has some buffer count related checks.

Regards,
Shaik Ameer Basha

> +       .vidioc_querybuf                = v4l2_m2m_ioctl_querybuf,
> +       .vidioc_expbuf                  = v4l2_m2m_ioctl_expbuf,
> +       .vidioc_qbuf                    = v4l2_m2m_ioctl_qbuf,
> +       .vidioc_dqbuf                   = v4l2_m2m_ioctl_dqbuf,
> +
> +       .vidioc_streamon                = v4l2_m2m_ioctl_streamon,
> +       .vidioc_streamoff               = v4l2_m2m_ioctl_streamoff,
>         .vidioc_g_selection             = gsc_m2m_g_selection,
>         .vidioc_s_selection             = gsc_m2m_s_selection
>  };
> @@ -588,6 +539,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>         src_vq->mem_ops = &vb2_dma_contig_memops;
>         src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>         src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +       src_vq->lock = &ctx->gsc_dev->lock;
>
>         ret = vb2_queue_init(src_vq);
>         if (ret)
> @@ -601,6 +553,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>         dst_vq->mem_ops = &vb2_dma_contig_memops;
>         dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>         dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +       dst_vq->lock = &ctx->gsc_dev->lock;
>
>         return vb2_queue_init(dst_vq);
>  }
> @@ -648,6 +601,7 @@ static int gsc_m2m_open(struct file *file)
>                 ret = PTR_ERR(ctx->m2m_ctx);
>                 goto error_ctrls;
>         }
> +       ctx->fh.m2m_ctx = ctx->m2m_ctx;
>
>         if (gsc->m2m.refcnt++ == 0)
>                 set_bit(ST_M2M_OPEN, &gsc->state);
> @@ -691,44 +645,13 @@ static int gsc_m2m_release(struct file *file)
>         return 0;
>  }
>
> -static unsigned int gsc_m2m_poll(struct file *file,
> -                                       struct poll_table_struct *wait)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
> -       struct gsc_dev *gsc = ctx->gsc_dev;
> -       int ret;
> -
> -       if (mutex_lock_interruptible(&gsc->lock))
> -               return -ERESTARTSYS;
> -
> -       ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> -       mutex_unlock(&gsc->lock);
> -
> -       return ret;
> -}
> -
> -static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> -       struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
> -       struct gsc_dev *gsc = ctx->gsc_dev;
> -       int ret;
> -
> -       if (mutex_lock_interruptible(&gsc->lock))
> -               return -ERESTARTSYS;
> -
> -       ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> -       mutex_unlock(&gsc->lock);
> -
> -       return ret;
> -}
> -
>  static const struct v4l2_file_operations gsc_m2m_fops = {
>         .owner          = THIS_MODULE,
>         .open           = gsc_m2m_open,
>         .release        = gsc_m2m_release,
> -       .poll           = gsc_m2m_poll,
> +       .poll           = v4l2_m2m_fop_poll,
>         .unlocked_ioctl = video_ioctl2,
> -       .mmap           = gsc_m2m_mmap,
> +       .mmap           = v4l2_m2m_fop_mmap,
>  };
>
>  static struct v4l2_m2m_ops gsc_m2m_ops = {
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RFC 6/7] exynos-gsc: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Hi Shaik,

On 09/13/2013 04:12 PM, Shaik Ameer Basha wrote:
[...]
>> -static int gsc_m2m_streamon(struct file *file, void *fh,
>> -                          enum v4l2_buf_type type)
>> -{
>> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
>> -
>> -       /* The source and target color format need to be set */
>> -       if (V4L2_TYPE_IS_OUTPUT(type)) {
>> -               if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx))
>> -                       return -EINVAL;
>> -       } else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) {
>> -               return -EINVAL;
>> -       }
>> -
>> -       return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
>> -}
>> -
>> -static int gsc_m2m_streamoff(struct file *file, void *fh,
>> -                           enum v4l2_buf_type type)
>> -{
>> -       struct gsc_ctx *ctx = fh_to_ctx(fh);
>> -       return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
>> -}
>> -
>>   /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */
>>   static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b)
>>   {
>> @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = {
>>          .vidioc_try_fmt_vid_out_mplane  = gsc_m2m_try_fmt_mplane,
>>          .vidioc_s_fmt_vid_cap_mplane    = gsc_m2m_s_fmt_mplane,
>>          .vidioc_s_fmt_vid_out_mplane    = gsc_m2m_s_fmt_mplane,
>> -       .vidioc_reqbufs                 = gsc_m2m_reqbufs,
>> -       .vidioc_expbuf                  = gsc_m2m_expbuf,
>> -       .vidioc_querybuf                = gsc_m2m_querybuf,
>> -       .vidioc_qbuf                    = gsc_m2m_qbuf,
>> -       .vidioc_dqbuf                   = gsc_m2m_dqbuf,
>> -       .vidioc_streamon                = gsc_m2m_streamon,
>> -       .vidioc_streamoff               = gsc_m2m_streamoff,
>> +
>> +       .vidioc_reqbufs                 = v4l2_m2m_ioctl_reqbufs,
>
> I think your intention was not to replace gsc_m2m_reqbufs() with
> v4l2_m2m_ioctl_reqbufs().
> you didn't remove the gsc_m2m_reqbufs() function :)
>
> On top of that,  gsc_m2m_reqbufs() has some buffer count related checks.

Thanks for the review. Sorry, I actually left this patch halfway done.
There is some clean up required before we can actually benefit from those
m2m helpers. First of all the driver should have set valid default format
right when the video device is opened. Then the hack with *{SRC, DST}_FMT
flags should be removed.

The fact that the selection API on mem-to-mem video nodes and its
interaction with VIDIOC_S_FMT is not well defined doesn't of course help
here.

I thought I'd drop exynos-gsc from this series but I had a look at it and
it didn't take much time to make those cleanups, so there will be two more
patches for exynos-gsc, unfortunately not tested yet.

Regarding gsc_m2m_reqbufs(), it currently behaves incorrectly. It should
adjust reqbufs->count to a supported value, rather than returning EINVAL.

Moreover, the buffer count limit is currently 32 for both CAPTURE and OUTPUT
queue. I don't know when this number comes from, the driver always uses
DMA buffer descriptor 0 for all transactions (GSC_M2M_BUF_NUM). Maybe this
code was inherited from the initial BSP gsc-capture driver, where the buffer
masking feature was actually used.

Besides that, the number of requested buffer per vb2 buffer queue is
always being limited to VIDEO_MAX_FRAME in videobuf2, which is also 32.

So I think gsc_m2m_reqbufs() can be pretty much optimized, including
removal of an unused 'frame' variable, and we can safely replace it with
v4l2_m2m_ioctl_reqbufs().

--
Regards,
Sylwester

Re: [PATCH RFC 6/7] exynos-gsc: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> TODO: Add setting of default initial format.

So this patch can't be applied yet.

Other than that it looks good, but I won't ack it since it introduces a regression
as long as the TODO isn't addressed.

Regards,

	Hans

> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/platform/exynos-gsc/gsc-core.h |   12 ---
>  drivers/media/platform/exynos-gsc/gsc-m2m.c  |  109 ++++----------------------
>  2 files changed, 16 insertions(+), 105 deletions(-)
> 
> diff --git a/drivers/media/platform/exynos-gsc/gsc-core.h b/drivers/media/platform/exynos-gsc/gsc-core.h
> index cc19bba..1afad32 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-core.h
> +++ b/drivers/media/platform/exynos-gsc/gsc-core.h
> @@ -464,18 +464,6 @@ static inline void gsc_hw_clear_irq(struct gsc_dev *dev, int irq)
>  	writel(cfg, dev->regs + GSC_IRQ);
>  }
>  
> -static inline void gsc_lock(struct vb2_queue *vq)
> -{
> -	struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
> -	mutex_lock(&ctx->gsc_dev->lock);
> -}
> -
> -static inline void gsc_unlock(struct vb2_queue *vq)
> -{
> -	struct gsc_ctx *ctx = vb2_get_drv_priv(vq);
> -	mutex_unlock(&ctx->gsc_dev->lock);
> -}
> -
>  static inline bool gsc_ctx_state_is_set(u32 mask, struct gsc_ctx *ctx)
>  {
>  	unsigned long flags;
> diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> index 40a73f7..4f5d6cb 100644
> --- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
> +++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
> @@ -262,8 +262,8 @@ static struct vb2_ops gsc_m2m_qops = {
>  	.queue_setup	 = gsc_m2m_queue_setup,
>  	.buf_prepare	 = gsc_m2m_buf_prepare,
>  	.buf_queue	 = gsc_m2m_buf_queue,
> -	.wait_prepare	 = gsc_unlock,
> -	.wait_finish	 = gsc_lock,
> +	.wait_prepare	 = vb2_ops_wait_prepare,
> +	.wait_finish	 = vb2_ops_wait_finish,
>  	.stop_streaming	 = gsc_m2m_stop_streaming,
>  	.start_streaming = gsc_m2m_start_streaming,
>  };
> @@ -376,57 +376,6 @@ static int gsc_m2m_reqbufs(struct file *file, void *fh,
>  	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
>  }
>  
> -static int gsc_m2m_expbuf(struct file *file, void *fh,
> -				struct v4l2_exportbuffer *eb)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_expbuf(file, ctx->m2m_ctx, eb);
> -}
> -
> -static int gsc_m2m_querybuf(struct file *file, void *fh,
> -					struct v4l2_buffer *buf)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_qbuf(struct file *file, void *fh,
> -			  struct v4l2_buffer *buf)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_dqbuf(struct file *file, void *fh,
> -			   struct v4l2_buffer *buf)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int gsc_m2m_streamon(struct file *file, void *fh,
> -			   enum v4l2_buf_type type)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(fh);
> -
> -	/* The source and target color format need to be set */
> -	if (V4L2_TYPE_IS_OUTPUT(type)) {
> -		if (!gsc_ctx_state_is_set(GSC_SRC_FMT, ctx))
> -			return -EINVAL;
> -	} else if (!gsc_ctx_state_is_set(GSC_DST_FMT, ctx)) {
> -		return -EINVAL;
> -	}
> -
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int gsc_m2m_streamoff(struct file *file, void *fh,
> -			    enum v4l2_buf_type type)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(fh);
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  /* Return 1 if rectangle a is enclosed in rectangle b, or 0 otherwise. */
>  static int is_rectangle_enclosed(struct v4l2_rect *a, struct v4l2_rect *b)
>  {
> @@ -563,13 +512,15 @@ static const struct v4l2_ioctl_ops gsc_m2m_ioctl_ops = {
>  	.vidioc_try_fmt_vid_out_mplane	= gsc_m2m_try_fmt_mplane,
>  	.vidioc_s_fmt_vid_cap_mplane	= gsc_m2m_s_fmt_mplane,
>  	.vidioc_s_fmt_vid_out_mplane	= gsc_m2m_s_fmt_mplane,
> -	.vidioc_reqbufs			= gsc_m2m_reqbufs,
> -	.vidioc_expbuf                  = gsc_m2m_expbuf,
> -	.vidioc_querybuf		= gsc_m2m_querybuf,
> -	.vidioc_qbuf			= gsc_m2m_qbuf,
> -	.vidioc_dqbuf			= gsc_m2m_dqbuf,
> -	.vidioc_streamon		= gsc_m2m_streamon,
> -	.vidioc_streamoff		= gsc_m2m_streamoff,
> +
> +	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_expbuf			= v4l2_m2m_ioctl_expbuf,
> +	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
> +
> +	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
>  	.vidioc_g_selection		= gsc_m2m_g_selection,
>  	.vidioc_s_selection		= gsc_m2m_s_selection
>  };
> @@ -588,6 +539,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->mem_ops = &vb2_dma_contig_memops;
>  	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->gsc_dev->lock;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -601,6 +553,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
>  	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->gsc_dev->lock;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> @@ -648,6 +601,7 @@ static int gsc_m2m_open(struct file *file)
>  		ret = PTR_ERR(ctx->m2m_ctx);
>  		goto error_ctrls;
>  	}
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>  	if (gsc->m2m.refcnt++ == 0)
>  		set_bit(ST_M2M_OPEN, &gsc->state);
> @@ -691,44 +645,13 @@ static int gsc_m2m_release(struct file *file)
>  	return 0;
>  }
>  
> -static unsigned int gsc_m2m_poll(struct file *file,
> -					struct poll_table_struct *wait)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
> -	struct gsc_dev *gsc = ctx->gsc_dev;
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&gsc->lock))
> -		return -ERESTARTSYS;
> -
> -	ret = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> -	mutex_unlock(&gsc->lock);
> -
> -	return ret;
> -}
> -
> -static int gsc_m2m_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> -	struct gsc_ctx *ctx = fh_to_ctx(file->private_data);
> -	struct gsc_dev *gsc = ctx->gsc_dev;
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&gsc->lock))
> -		return -ERESTARTSYS;
> -
> -	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> -	mutex_unlock(&gsc->lock);
> -
> -	return ret;
> -}
> -
>  static const struct v4l2_file_operations gsc_m2m_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= gsc_m2m_open,
>  	.release	= gsc_m2m_release,
> -	.poll		= gsc_m2m_poll,
> +	.poll		= v4l2_m2m_fop_poll,
>  	.unlocked_ioctl	= video_ioctl2,
> -	.mmap		= gsc_m2m_mmap,
> +	.mmap		= v4l2_m2m_fop_mmap,
>  };
>  
>  static struct v4l2_m2m_ops gsc_m2m_ops = {
> 

Re: [PATCH RFC 6/7] exynos-gsc: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

On 09/30/2013 11:50 AM, Hans Verkuil wrote:
> On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
>> >  Simplify the driver by using the m2m ioctl and vb2 helpers.
>> >
>> >  TODO: Add setting of default initial format.
>
> So this patch can't be applied yet.

Indeed, it's just an RFC series, I wanted it to give an overview of
how much code can be eliminated with those helpers and posted this
series regardless of availability of the pre-requisite cleanups,
which I might or might not have time to prepare.

Anyway I've already written those missing patches and these may be
included in the final series, as soon as people Ack and test the
patches where I couldn't test myself.

> Other than that it looks good, but I won't ack it since it introduces a regression
> as long as the TODO isn't addressed.

Thanks, whole series updated to follow.

--
Regards,
Sylwester

[PATCH RFC 7/7] s5p-g2d: Use mem-to-mem ioctl helpers

[Sylwester Nawrocki]

Simplify the driver by using the m2m ioctl and vb2 helpers.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>
---
 drivers/media/platform/s5p-g2d/g2d.c |  103 ++++------------------------------
 1 file changed, 11 insertions(+), 92 deletions(-)

diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
index 553d87e..d59d546 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -139,7 +139,6 @@ static void g2d_buf_queue(struct vb2_buffer *vb)
 	v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
 }
 
-
 static struct vb2_ops g2d_qops = {
 	.queue_setup	= g2d_queue_setup,
 	.buf_prepare	= g2d_buf_prepare,
@@ -159,6 +158,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->mem_ops = &vb2_dma_contig_memops;
 	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	src_vq->lock = &ctx->dev->mutex;
 
 	ret = vb2_queue_init(src_vq);
 	if (ret)
@@ -171,6 +171,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	dst_vq->lock = &ctx->dev->mutex;
 
 	return vb2_queue_init(dst_vq);
 }
@@ -263,6 +264,7 @@ static int g2d_open(struct file *file)
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
 	file->private_data = &ctx->fh;
 	v4l2_fh_add(&ctx->fh);
+	ctx->fh.m2m_ctx = ctx->m2m_ctx;
 
 	g2d_setup_ctrls(ctx);
 
@@ -410,72 +412,6 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
 	return 0;
 }
 
-static unsigned int g2d_poll(struct file *file, struct poll_table_struct *wait)
-{
-	struct g2d_ctx *ctx = fh2ctx(file->private_data);
-	struct g2d_dev *dev = ctx->dev;
-	unsigned int res;
-
-	mutex_lock(&dev->mutex);
-	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
-	mutex_unlock(&dev->mutex);
-	return res;
-}
-
-static int g2d_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct g2d_ctx *ctx = fh2ctx(file->private_data);
-	struct g2d_dev *dev = ctx->dev;
-	int ret;
-
-	if (mutex_lock_interruptible(&dev->mutex))
-		return -ERESTARTSYS;
-	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
-	mutex_unlock(&dev->mutex);
-	return ret;
-}
-
-static int vidioc_reqbufs(struct file *file, void *priv,
-			struct v4l2_requestbuffers *reqbufs)
-{
-	struct g2d_ctx *ctx = priv;
-	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
-}
-
-static int vidioc_querybuf(struct file *file, void *priv,
-			struct v4l2_buffer *buf)
-{
-	struct g2d_ctx *ctx = priv;
-	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct g2d_ctx *ctx = priv;
-	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
-}
-
-static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
-{
-	struct g2d_ctx *ctx = priv;
-	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
-}
-
-
-static int vidioc_streamon(struct file *file, void *priv,
-					enum v4l2_buf_type type)
-{
-	struct g2d_ctx *ctx = priv;
-	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
-}
-
-static int vidioc_streamoff(struct file *file, void *priv,
-					enum v4l2_buf_type type)
-{
-	struct g2d_ctx *ctx = priv;
-	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
-}
-
 static int vidioc_cropcap(struct file *file, void *priv,
 					struct v4l2_cropcap *cr)
 {
@@ -551,20 +487,6 @@ static int vidioc_s_crop(struct file *file, void *prv, const struct v4l2_crop *c
 	return 0;
 }
 
-static void g2d_lock(void *prv)
-{
-	struct g2d_ctx *ctx = prv;
-	struct g2d_dev *dev = ctx->dev;
-	mutex_lock(&dev->mutex);
-}
-
-static void g2d_unlock(void *prv)
-{
-	struct g2d_ctx *ctx = prv;
-	struct g2d_dev *dev = ctx->dev;
-	mutex_unlock(&dev->mutex);
-}
-
 static void job_abort(void *prv)
 {
 	struct g2d_ctx *ctx = prv;
@@ -653,9 +575,9 @@ static const struct v4l2_file_operations g2d_fops = {
 	.owner		= THIS_MODULE,
 	.open		= g2d_open,
 	.release	= g2d_release,
-	.poll		= g2d_poll,
+	.poll		= v4l2_m2m_fop_poll,
 	.unlocked_ioctl	= video_ioctl2,
-	.mmap		= g2d_mmap,
+	.mmap		= v4l2_m2m_fop_mmap,
 };
 
 static const struct v4l2_ioctl_ops g2d_ioctl_ops = {
@@ -671,14 +593,13 @@ static const struct v4l2_ioctl_ops g2d_ioctl_ops = {
 	.vidioc_try_fmt_vid_out		= vidioc_try_fmt,
 	.vidioc_s_fmt_vid_out		= vidioc_s_fmt,
 
-	.vidioc_reqbufs			= vidioc_reqbufs,
-	.vidioc_querybuf		= vidioc_querybuf,
-
-	.vidioc_qbuf			= vidioc_qbuf,
-	.vidioc_dqbuf			= vidioc_dqbuf,
+	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
+	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
+	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
 
-	.vidioc_streamon		= vidioc_streamon,
-	.vidioc_streamoff		= vidioc_streamoff,
+	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
+	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
 
 	.vidioc_g_crop			= vidioc_g_crop,
 	.vidioc_s_crop			= vidioc_s_crop,
@@ -697,8 +618,6 @@ static struct video_device g2d_videodev = {
 static struct v4l2_m2m_ops g2d_m2m_ops = {
 	.device_run	= device_run,
 	.job_abort	= job_abort,
-	.lock		= g2d_lock,
-	.unlock		= g2d_unlock,
 };
 
 static const struct of_device_id exynos_g2d_match[];
-- 
1.7.9.5

Re: [PATCH RFC 7/7] s5p-g2d: Use mem-to-mem ioctl helpers

[Hans Verkuil]

On 09/13/2013 02:56 PM, Sylwester Nawrocki wrote:
> Simplify the driver by using the m2m ioctl and vb2 helpers.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Kyugmin Park <kyungmin.park@samsung.com>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/media/platform/s5p-g2d/g2d.c |  103 ++++------------------------------
>  1 file changed, 11 insertions(+), 92 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-g2d/g2d.c b/drivers/media/platform/s5p-g2d/g2d.c
> index 553d87e..d59d546 100644
> --- a/drivers/media/platform/s5p-g2d/g2d.c
> +++ b/drivers/media/platform/s5p-g2d/g2d.c
> @@ -139,7 +139,6 @@ static void g2d_buf_queue(struct vb2_buffer *vb)
>  	v4l2_m2m_buf_queue(ctx->m2m_ctx, vb);
>  }
>  
> -
>  static struct vb2_ops g2d_qops = {
>  	.queue_setup	= g2d_queue_setup,
>  	.buf_prepare	= g2d_buf_prepare,
> @@ -159,6 +158,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->mem_ops = &vb2_dma_contig_memops;
>  	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	src_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	src_vq->lock = &ctx->dev->mutex;
>  
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
> @@ -171,6 +171,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
>  	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	dst_vq->timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	dst_vq->lock = &ctx->dev->mutex;
>  
>  	return vb2_queue_init(dst_vq);
>  }
> @@ -263,6 +264,7 @@ static int g2d_open(struct file *file)
>  	v4l2_fh_init(&ctx->fh, video_devdata(file));
>  	file->private_data = &ctx->fh;
>  	v4l2_fh_add(&ctx->fh);
> +	ctx->fh.m2m_ctx = ctx->m2m_ctx;
>  
>  	g2d_setup_ctrls(ctx);
>  
> @@ -410,72 +412,6 @@ static int vidioc_s_fmt(struct file *file, void *prv, struct v4l2_format *f)
>  	return 0;
>  }
>  
> -static unsigned int g2d_poll(struct file *file, struct poll_table_struct *wait)
> -{
> -	struct g2d_ctx *ctx = fh2ctx(file->private_data);
> -	struct g2d_dev *dev = ctx->dev;
> -	unsigned int res;
> -
> -	mutex_lock(&dev->mutex);
> -	res = v4l2_m2m_poll(file, ctx->m2m_ctx, wait);
> -	mutex_unlock(&dev->mutex);
> -	return res;
> -}
> -
> -static int g2d_mmap(struct file *file, struct vm_area_struct *vma)
> -{
> -	struct g2d_ctx *ctx = fh2ctx(file->private_data);
> -	struct g2d_dev *dev = ctx->dev;
> -	int ret;
> -
> -	if (mutex_lock_interruptible(&dev->mutex))
> -		return -ERESTARTSYS;
> -	ret = v4l2_m2m_mmap(file, ctx->m2m_ctx, vma);
> -	mutex_unlock(&dev->mutex);
> -	return ret;
> -}
> -
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -			struct v4l2_requestbuffers *reqbufs)
> -{
> -	struct g2d_ctx *ctx = priv;
> -	return v4l2_m2m_reqbufs(file, ctx->m2m_ctx, reqbufs);
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -			struct v4l2_buffer *buf)
> -{
> -	struct g2d_ctx *ctx = priv;
> -	return v4l2_m2m_querybuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct g2d_ctx *ctx = priv;
> -	return v4l2_m2m_qbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
> -{
> -	struct g2d_ctx *ctx = priv;
> -	return v4l2_m2m_dqbuf(file, ctx->m2m_ctx, buf);
> -}
> -
> -
> -static int vidioc_streamon(struct file *file, void *priv,
> -					enum v4l2_buf_type type)
> -{
> -	struct g2d_ctx *ctx = priv;
> -	return v4l2_m2m_streamon(file, ctx->m2m_ctx, type);
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> -					enum v4l2_buf_type type)
> -{
> -	struct g2d_ctx *ctx = priv;
> -	return v4l2_m2m_streamoff(file, ctx->m2m_ctx, type);
> -}
> -
>  static int vidioc_cropcap(struct file *file, void *priv,
>  					struct v4l2_cropcap *cr)
>  {
> @@ -551,20 +487,6 @@ static int vidioc_s_crop(struct file *file, void *prv, const struct v4l2_crop *c
>  	return 0;
>  }
>  
> -static void g2d_lock(void *prv)
> -{
> -	struct g2d_ctx *ctx = prv;
> -	struct g2d_dev *dev = ctx->dev;
> -	mutex_lock(&dev->mutex);
> -}
> -
> -static void g2d_unlock(void *prv)
> -{
> -	struct g2d_ctx *ctx = prv;
> -	struct g2d_dev *dev = ctx->dev;
> -	mutex_unlock(&dev->mutex);
> -}
> -
>  static void job_abort(void *prv)
>  {
>  	struct g2d_ctx *ctx = prv;
> @@ -653,9 +575,9 @@ static const struct v4l2_file_operations g2d_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= g2d_open,
>  	.release	= g2d_release,
> -	.poll		= g2d_poll,
> +	.poll		= v4l2_m2m_fop_poll,
>  	.unlocked_ioctl	= video_ioctl2,
> -	.mmap		= g2d_mmap,
> +	.mmap		= v4l2_m2m_fop_mmap,
>  };
>  
>  static const struct v4l2_ioctl_ops g2d_ioctl_ops = {
> @@ -671,14 +593,13 @@ static const struct v4l2_ioctl_ops g2d_ioctl_ops = {
>  	.vidioc_try_fmt_vid_out		= vidioc_try_fmt,
>  	.vidioc_s_fmt_vid_out		= vidioc_s_fmt,
>  
> -	.vidioc_reqbufs			= vidioc_reqbufs,
> -	.vidioc_querybuf		= vidioc_querybuf,
> -
> -	.vidioc_qbuf			= vidioc_qbuf,
> -	.vidioc_dqbuf			= vidioc_dqbuf,
> +	.vidioc_reqbufs			= v4l2_m2m_ioctl_reqbufs,
> +	.vidioc_querybuf		= v4l2_m2m_ioctl_querybuf,
> +	.vidioc_qbuf			= v4l2_m2m_ioctl_qbuf,
> +	.vidioc_dqbuf			= v4l2_m2m_ioctl_dqbuf,
>  
> -	.vidioc_streamon		= vidioc_streamon,
> -	.vidioc_streamoff		= vidioc_streamoff,
> +	.vidioc_streamon		= v4l2_m2m_ioctl_streamon,
> +	.vidioc_streamoff		= v4l2_m2m_ioctl_streamoff,
>  
>  	.vidioc_g_crop			= vidioc_g_crop,
>  	.vidioc_s_crop			= vidioc_s_crop,
> @@ -697,8 +618,6 @@ static struct video_device g2d_videodev = {
>  static struct v4l2_m2m_ops g2d_m2m_ops = {
>  	.device_run	= device_run,
>  	.job_abort	= job_abort,
> -	.lock		= g2d_lock,
> -	.unlock		= g2d_unlock,
>  };
>  
>  static const struct of_device_id exynos_g2d_match[];
>