[PATCH 00/12] CODA fixes and buffer allocation in REQBUFS

[PATCH 00/12] CODA fixes and buffer allocation in REQBUFS

[Philipp Zabel]

Hi,

this is a series of various fixes and a move of the per-context buffer
allocation into the REQBUFS call. Allocating the bitstream buffer only
after S_FMT allows at the same time to guarantee that the bitstream buffer
is always large enough to contain two worst-case compressed frames and
doesn't waste memory if a small format is selected.

Peter Seiderer (3):
  [media] coda: check kasprintf return value in coda_open
  [media] coda: fix double call to debugfs_remove
  [media] coda: free decoder bitstream buffer

Philipp Zabel (9):
  [media] coda: bitstream payload is unsigned
  [media] coda: use strlcpy instead of snprintf
  [media] coda: allocate per-context buffers from REQBUFS
  [media] coda: allocate bitstream buffer from REQBUFS, size depends on
    the format
  [media] coda: move parameter buffer in together with context buffer
    allocation
  [media] coda: remove duplicate error messages for buffer allocations
  [media] coda: fail to start streaming if userspace set invalid formats
  [media] coda: call SEQ_END when the first queue is stopped
  [media] coda: fix fill bitstream errors in nonstreaming case

 drivers/media/platform/coda/coda-bit.c    | 177 +++++++++++++++++++++---------
 drivers/media/platform/coda/coda-common.c | 104 +++++++++---------
 drivers/media/platform/coda/coda.h        |  13 +--
 3 files changed, 180 insertions(+), 114 deletions(-)

-- 
2.1.4

[PATCH 01/12] [media] coda: check kasprintf return value in coda_open

[Philipp Zabel]

From: Peter Seiderer <ps.report@gmx.net>

kasprintf might fail if free memory is low.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 6f32e6d..c81af1b 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1621,6 +1621,11 @@ static int coda_open(struct file *file)
 	set_bit(idx, &dev->instance_mask);
 
 	name = kasprintf(GFP_KERNEL, "context%d", idx);
+	if (!name) {
+		ret = -ENOMEM;
+		goto err_coda_name_init;
+	}
+
 	ctx->debugfs_entry = debugfs_create_dir(name, dev->debugfs_root);
 	kfree(name);
 
@@ -1735,6 +1740,7 @@ err_pm_get:
 	v4l2_fh_del(&ctx->fh);
 	v4l2_fh_exit(&ctx->fh);
 	clear_bit(ctx->idx, &dev->instance_mask);
+err_coda_name_init:
 err_coda_max:
 	kfree(ctx);
 	return ret;
-- 
2.1.4

[PATCH 02/12] [media] coda: fix double call to debugfs_remove

[Philipp Zabel]

From: Peter Seiderer <ps.report@gmx.net>

In coda_free_aux_buf() call debugfs_remove only if buffer entry
is valid (and therfore dentry is valid), double protect by
invalidating dentry value.

Fixes erroneous prematurely dealloc of debugfs caused by
incorrect reference count incrementing.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index c81af1b..37bbd57 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1215,8 +1215,9 @@ void coda_free_aux_buf(struct coda_dev *dev,
 				  buf->vaddr, buf->paddr);
 		buf->vaddr = NULL;
 		buf->size = 0;
+		debugfs_remove(buf->dentry);
+		buf->dentry = NULL;
 	}
-	debugfs_remove(buf->dentry);
 }
 
 static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
-- 
2.1.4

[PATCH 03/12] [media] coda: free decoder bitstream buffer

[Philipp Zabel]

From: Peter Seiderer <ps.report@gmx.net>

Fixes resource leak preventing repeatedly decoding.

Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 856b542..3801a37 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -36,6 +36,8 @@
 #define CODA_DEFAULT_GAMMA	4096
 #define CODA9_DEFAULT_GAMMA	24576	/* 0.75 * 32768 */
 
+static void coda_free_bitstream_buffer(struct coda_ctx *ctx);
+
 static inline int coda_is_initialized(struct coda_dev *dev)
 {
 	return coda_read(dev, CODA_REG_BIT_CUR_PC) != 0;
@@ -1322,6 +1324,7 @@ static void coda_bit_release(struct coda_ctx *ctx)
 	mutex_lock(&ctx->buffer_mutex);
 	coda_free_framebuffers(ctx);
 	coda_free_context_buffers(ctx);
+	coda_free_bitstream_buffer(ctx);
 	mutex_unlock(&ctx->buffer_mutex);
 }
 
-- 
2.1.4

[PATCH 04/12] [media] coda: bitstream payload is unsigned

[Philipp Zabel]

kfifo_len is unsigned int, return it as such.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 0c35cd5..499049f 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -284,7 +284,7 @@ const char *coda_product_name(int product);
 
 int coda_check_firmware(struct coda_dev *dev);
 
-static inline int coda_get_bitstream_payload(struct coda_ctx *ctx)
+static inline unsigned int coda_get_bitstream_payload(struct coda_ctx *ctx)
 {
 	return kfifo_len(&ctx->bitstream_fifo);
 }
-- 
2.1.4

[PATCH 05/12] [media] coda: use strlcpy instead of snprintf

[Philipp Zabel]

There is no need to take the detour through a "%s" format string
to create a copy of a string.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 37bbd57..04130ea 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1908,8 +1908,7 @@ static int coda_register_device(struct coda_dev *dev, int i)
 	if (i >= dev->devtype->num_vdevs)
 		return -EINVAL;
 
-	snprintf(vfd->name, sizeof(vfd->name), "%s",
-		 dev->devtype->vdevs[i]->name);
+	strlcpy(vfd->name, dev->devtype->vdevs[i]->name, sizeof(vfd->name));
 	vfd->fops	= &coda_fops;
 	vfd->ioctl_ops	= &coda_ioctl_ops;
 	vfd->release	= video_device_release_empty,
-- 
2.1.4

[PATCH 06/12] [media] coda: allocate per-context buffers from REQBUFS

[Philipp Zabel]

Allocate the per-context buffers from REQBUFS instead in start_encoding or
start_decoding. This allows to stop and start streaming independently of
buffer (re)allocation

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c    | 55 ++++++++++++++++++++++++-------
 drivers/media/platform/coda/coda-common.c | 22 ++++++++++++-
 drivers/media/platform/coda/coda.h        |  1 +
 3 files changed, 66 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 3801a37..ce64c91 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -711,6 +711,27 @@ err_clk_per:
  * Encoder context operations
  */
 
+static int coda_encoder_reqbufs(struct coda_ctx *ctx,
+				struct v4l2_requestbuffers *rb)
+{
+	struct coda_q_data *q_data_src;
+	int ret;
+
+	if (rb->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+		return 0;
+
+	if (rb->count) {
+		q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+		ret = coda_alloc_context_buffers(ctx, q_data_src);
+		if (ret < 0)
+			return ret;
+	} else {
+		coda_free_context_buffers(ctx);
+	}
+
+	return 0;
+}
+
 static int coda_start_encoding(struct coda_ctx *ctx)
 {
 	struct coda_dev *dev = ctx->dev;
@@ -727,11 +748,6 @@ static int coda_start_encoding(struct coda_ctx *ctx)
 	q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 	dst_fourcc = q_data_dst->fourcc;
 
-	/* Allocate per-instance buffers */
-	ret = coda_alloc_context_buffers(ctx, q_data_src);
-	if (ret < 0)
-		return ret;
-
 	buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 	bitstream_buf = vb2_dma_contig_plane_dma_addr(buf, 0);
 	bitstream_size = q_data_dst->sizeimage;
@@ -1313,7 +1329,6 @@ static void coda_seq_end_work(struct work_struct *work)
 		ctx->bitstream.vaddr, ctx->bitstream.size);
 
 	coda_free_framebuffers(ctx);
-	coda_free_context_buffers(ctx);
 
 	mutex_unlock(&dev->coda_mutex);
 	mutex_unlock(&ctx->buffer_mutex);
@@ -1330,6 +1345,7 @@ static void coda_bit_release(struct coda_ctx *ctx)
 
 const struct coda_context_ops coda_bit_encode_ops = {
 	.queue_init = coda_encoder_queue_init,
+	.reqbufs = coda_encoder_reqbufs,
 	.start_streaming = coda_start_encoding,
 	.prepare_run = coda_prepare_encode,
 	.finish_run = coda_finish_encode,
@@ -1341,6 +1357,27 @@ const struct coda_context_ops coda_bit_encode_ops = {
  * Decoder context operations
  */
 
+static int coda_decoder_reqbufs(struct coda_ctx *ctx,
+				struct v4l2_requestbuffers *rb)
+{
+	struct coda_q_data *q_data_src;
+	int ret;
+
+	if (rb->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
+		return 0;
+
+	if (rb->count) {
+		q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+		ret = coda_alloc_context_buffers(ctx, q_data_src);
+		if (ret < 0)
+			return ret;
+	} else {
+		coda_free_context_buffers(ctx);
+	}
+
+	return 0;
+}
+
 static int __coda_start_decoding(struct coda_ctx *ctx)
 {
 	struct coda_q_data *q_data_src, *q_data_dst;
@@ -1359,11 +1396,6 @@ static int __coda_start_decoding(struct coda_ctx *ctx)
 	src_fourcc = q_data_src->fourcc;
 	dst_fourcc = q_data_dst->fourcc;
 
-	/* Allocate per-instance buffers */
-	ret = coda_alloc_context_buffers(ctx, q_data_src);
-	if (ret < 0)
-		return ret;
-
 	coda_write(dev, ctx->parabuf.paddr, CODA_REG_BIT_PARA_BUF_ADDR);
 
 	/* Update coda bitstream read and write pointers from kfifo */
@@ -1909,6 +1941,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 
 const struct coda_context_ops coda_bit_decode_ops = {
 	.queue_init = coda_decoder_queue_init,
+	.reqbufs = coda_decoder_reqbufs,
 	.start_streaming = coda_start_decoding,
 	.prepare_run = coda_prepare_decode,
 	.finish_run = coda_finish_decode,
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 04130ea..0026e3a 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -696,6 +696,26 @@ static int coda_s_fmt_vid_out(struct file *file, void *priv,
 	return coda_s_fmt(ctx, &f_cap);
 }
 
+static int coda_reqbufs(struct file *file, void *priv,
+			struct v4l2_requestbuffers *rb)
+{
+	struct coda_ctx *ctx = fh_to_ctx(priv);
+	int ret;
+
+	ret = v4l2_m2m_reqbufs(file, ctx->fh.m2m_ctx, rb);
+	if (ret)
+		return ret;
+
+	/*
+	 * Allow to allocate instance specific per-context buffers, such as
+	 * bitstream ringbuffer, slice buffer, work buffer, etc. if needed.
+	 */
+	if (rb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT && ctx->ops->reqbufs)
+		return ctx->ops->reqbufs(ctx, rb);
+
+	return 0;
+}
+
 static int coda_qbuf(struct file *file, void *priv,
 		     struct v4l2_buffer *buf)
 {
@@ -841,7 +861,7 @@ static const struct v4l2_ioctl_ops coda_ioctl_ops = {
 	.vidioc_try_fmt_vid_out	= coda_try_fmt_vid_out,
 	.vidioc_s_fmt_vid_out	= coda_s_fmt_vid_out,
 
-	.vidioc_reqbufs		= v4l2_m2m_ioctl_reqbufs,
+	.vidioc_reqbufs		= coda_reqbufs,
 	.vidioc_querybuf	= v4l2_m2m_ioctl_querybuf,
 
 	.vidioc_qbuf		= coda_qbuf,
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 499049f..57d070c 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -178,6 +178,7 @@ struct coda_ctx;
 struct coda_context_ops {
 	int (*queue_init)(void *priv, struct vb2_queue *src_vq,
 			  struct vb2_queue *dst_vq);
+	int (*reqbufs)(struct coda_ctx *ctx, struct v4l2_requestbuffers *rb);
 	int (*start_streaming)(struct coda_ctx *ctx);
 	int (*prepare_run)(struct coda_ctx *ctx);
 	void (*finish_run)(struct coda_ctx *ctx);
-- 
2.1.4

[PATCH 07/12] [media] coda: allocate bitstream buffer from REQBUFS, size depends on the format

[Philipp Zabel]

Allocating the bitstream buffer only when the format is set allows to guarantee
that at least two frames fit into the bitstream buffer. For small frame sizes
a smaller bitstream buffer can be allocated. Since the bitstream buffer size now
depends on the format, replace CODA_MAX_FRAME_SIZE with ctx->bitstream.size
where appropriate and remove the now unused constant.
Since REQBUFS can be called multiple times, but the format can't be changed
unless REQBUFS 0 was called before, we can just keep the allocated context and
bitstream buffers if REQBUFS is called multiple times with a non-zero buffer
count.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c    | 82 +++++++++++++++++++++----------
 drivers/media/platform/coda/coda-common.c | 22 ---------
 drivers/media/platform/coda/coda.h        |  1 -
 3 files changed, 55 insertions(+), 50 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index ce64c91..5aa8d87 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -15,6 +15,7 @@
 #include <linux/clk.h>
 #include <linux/irqreturn.h>
 #include <linux/kernel.h>
+#include <linux/log2.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
@@ -391,21 +392,7 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 	if (dev->devtype->product == CODA_DX6)
 		return 0;
 
-	if (ctx->psbuf.vaddr) {
-		v4l2_err(&dev->v4l2_dev, "psmembuf still allocated\n");
-		return -EBUSY;
-	}
-	if (ctx->slicebuf.vaddr) {
-		v4l2_err(&dev->v4l2_dev, "slicebuf still allocated\n");
-		return -EBUSY;
-	}
-	if (ctx->workbuf.vaddr) {
-		v4l2_err(&dev->v4l2_dev, "context buffer still allocated\n");
-		ret = -EBUSY;
-		return -ENOMEM;
-	}
-
-	if (q_data->fourcc == V4L2_PIX_FMT_H264) {
+	if (!ctx->slicebuf.vaddr && q_data->fourcc == V4L2_PIX_FMT_H264) {
 		/* worst case slice size */
 		size = (DIV_ROUND_UP(q_data->width, 16) *
 			DIV_ROUND_UP(q_data->height, 16)) * 3200 / 8 + 512;
@@ -419,7 +406,7 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 		}
 	}
 
-	if (dev->devtype->product == CODA_7541) {
+	if (!ctx->psbuf.vaddr && dev->devtype->product == CODA_7541) {
 		ret = coda_alloc_context_buf(ctx, &ctx->psbuf,
 					     CODA7_PS_BUF_SIZE, "psbuf");
 		if (ret < 0) {
@@ -429,16 +416,19 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 		}
 	}
 
-	size = dev->devtype->workbuf_size;
-	if (dev->devtype->product == CODA_960 &&
-	    q_data->fourcc == V4L2_PIX_FMT_H264)
-		size += CODA9_PS_SAVE_SIZE;
-	ret = coda_alloc_context_buf(ctx, &ctx->workbuf, size, "workbuf");
-	if (ret < 0) {
-		v4l2_err(&dev->v4l2_dev,
-			 "failed to allocate %d byte context buffer",
-			 ctx->workbuf.size);
-		goto err;
+	if (!ctx->workbuf.vaddr) {
+		size = dev->devtype->workbuf_size;
+		if (dev->devtype->product == CODA_960 &&
+		    q_data->fourcc == V4L2_PIX_FMT_H264)
+			size += CODA9_PS_SAVE_SIZE;
+		ret = coda_alloc_context_buf(ctx, &ctx->workbuf, size,
+					     "workbuf");
+		if (ret < 0) {
+			v4l2_err(&dev->v4l2_dev,
+				 "failed to allocate %d byte context buffer",
+				 ctx->workbuf.size);
+			goto err;
+		}
 	}
 
 	return 0;
@@ -1357,6 +1347,38 @@ const struct coda_context_ops coda_bit_encode_ops = {
  * Decoder context operations
  */
 
+static int coda_alloc_bitstream_buffer(struct coda_ctx *ctx,
+				       struct coda_q_data *q_data)
+{
+	if (ctx->bitstream.vaddr)
+		return 0;
+
+	ctx->bitstream.size = roundup_pow_of_two(q_data->sizeimage * 2);
+	ctx->bitstream.vaddr = dma_alloc_writecombine(
+			&ctx->dev->plat_dev->dev, ctx->bitstream.size,
+			&ctx->bitstream.paddr, GFP_KERNEL);
+	if (!ctx->bitstream.vaddr) {
+		v4l2_err(&ctx->dev->v4l2_dev,
+			 "failed to allocate bitstream ringbuffer");
+		return -ENOMEM;
+	}
+	kfifo_init(&ctx->bitstream_fifo,
+		   ctx->bitstream.vaddr, ctx->bitstream.size);
+
+	return 0;
+}
+
+static void coda_free_bitstream_buffer(struct coda_ctx *ctx)
+{
+	if (ctx->bitstream.vaddr == NULL)
+		return;
+
+	dma_free_writecombine(&ctx->dev->plat_dev->dev, ctx->bitstream.size,
+			      ctx->bitstream.vaddr, ctx->bitstream.paddr);
+	ctx->bitstream.vaddr = NULL;
+	kfifo_init(&ctx->bitstream_fifo, NULL, 0);
+}
+
 static int coda_decoder_reqbufs(struct coda_ctx *ctx,
 				struct v4l2_requestbuffers *rb)
 {
@@ -1371,7 +1393,13 @@ static int coda_decoder_reqbufs(struct coda_ctx *ctx,
 		ret = coda_alloc_context_buffers(ctx, q_data_src);
 		if (ret < 0)
 			return ret;
+		ret = coda_alloc_bitstream_buffer(ctx, q_data_src);
+		if (ret < 0) {
+			coda_free_context_buffers(ctx);
+			return ret;
+		}
 	} else {
+		coda_free_bitstream_buffer(ctx);
 		coda_free_context_buffers(ctx);
 	}
 
@@ -1739,7 +1767,7 @@ static void coda_finish_decode(struct coda_ctx *ctx)
 	 * by up to 512 bytes
 	 */
 	if (ctx->bit_stream_param & CODA_BIT_STREAM_END_FLAG) {
-		if (coda_get_bitstream_payload(ctx) >= CODA_MAX_FRAME_SIZE - 512)
+		if (coda_get_bitstream_payload(ctx) >= ctx->bitstream.size - 512)
 			kfifo_init(&ctx->bitstream_fifo,
 				ctx->bitstream.vaddr, ctx->bitstream.size);
 	}
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 0026e3a..b34a0db 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1716,20 +1716,6 @@ static int coda_open(struct file *file)
 			goto err_dma_alloc;
 		}
 	}
-	if (ctx->use_bit && ctx->inst_type == CODA_INST_DECODER) {
-		ctx->bitstream.size = CODA_MAX_FRAME_SIZE;
-		ctx->bitstream.vaddr = dma_alloc_writecombine(
-				&dev->plat_dev->dev, ctx->bitstream.size,
-				&ctx->bitstream.paddr, GFP_KERNEL);
-		if (!ctx->bitstream.vaddr) {
-			v4l2_err(&dev->v4l2_dev,
-				 "failed to allocate bitstream ringbuffer");
-			ret = -ENOMEM;
-			goto err_dma_writecombine;
-		}
-	}
-	kfifo_init(&ctx->bitstream_fifo,
-		ctx->bitstream.vaddr, ctx->bitstream.size);
 	mutex_init(&ctx->bitstream_mutex);
 	mutex_init(&ctx->buffer_mutex);
 	INIT_LIST_HEAD(&ctx->buffer_meta_list);
@@ -1743,10 +1729,6 @@ static int coda_open(struct file *file)
 
 	return 0;
 
-err_dma_writecombine:
-	if (ctx->dev->devtype->product == CODA_DX6)
-		coda_free_aux_buf(dev, &ctx->workbuf);
-	coda_free_aux_buf(dev, &ctx->parabuf);
 err_dma_alloc:
 	v4l2_ctrl_handler_free(&ctx->ctrls);
 err_ctrls_setup:
@@ -1791,10 +1773,6 @@ static int coda_release(struct file *file)
 	list_del(&ctx->list);
 	coda_unlock(ctx);
 
-	if (ctx->bitstream.vaddr) {
-		dma_free_writecombine(&dev->plat_dev->dev, ctx->bitstream.size,
-			ctx->bitstream.vaddr, ctx->bitstream.paddr);
-	}
 	if (ctx->dev->devtype->product == CODA_DX6)
 		coda_free_aux_buf(dev, &ctx->workbuf);
 
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 57d070c..01d940c 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -26,7 +26,6 @@
 #include "coda_regs.h"
 
 #define CODA_MAX_FRAMEBUFFERS	8
-#define CODA_MAX_FRAME_SIZE	0x100000
 #define FMO_SLICE_SAVE_BUF_SIZE	(32)
 
 enum {
-- 
2.1.4

[PATCH 08/12] [media] coda: move parameter buffer in together with context buffer allocation

[Philipp Zabel]

The parameter buffer is a per-context buffer, so we can allocate and free it
together with the other context buffers during REQBUFS.
Since this was the last context buffer allocated in coda-common.c, we can now
move coda_alloc_context_buf into coda-bit.c.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c    | 21 ++++++++++++++++++++-
 drivers/media/platform/coda/coda-common.c | 12 ------------
 drivers/media/platform/coda/coda.h        |  7 -------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 5aa8d87..0073f5a 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -31,6 +31,7 @@
 
 #include "coda.h"
 
+#define CODA_PARA_BUF_SIZE	(10 * 1024)
 #define CODA7_PS_BUF_SIZE	0x28000
 #define CODA9_PS_SAVE_SIZE	(512 * 1024)
 
@@ -300,6 +301,14 @@ static void coda_parabuf_write(struct coda_ctx *ctx, int index, u32 value)
 		p[index ^ 1] = value;
 }
 
+static inline int coda_alloc_context_buf(struct coda_ctx *ctx,
+					 struct coda_aux_buf *buf, size_t size,
+					 const char *name)
+{
+	return coda_alloc_aux_buf(ctx->dev, buf, size, name, ctx->debugfs_entry);
+}
+
+
 static void coda_free_framebuffers(struct coda_ctx *ctx)
 {
 	int i;
@@ -380,6 +389,7 @@ static void coda_free_context_buffers(struct coda_ctx *ctx)
 	coda_free_aux_buf(dev, &ctx->psbuf);
 	if (dev->devtype->product != CODA_DX6)
 		coda_free_aux_buf(dev, &ctx->workbuf);
+	coda_free_aux_buf(dev, &ctx->parabuf);
 }
 
 static int coda_alloc_context_buffers(struct coda_ctx *ctx,
@@ -389,6 +399,15 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 	size_t size;
 	int ret;
 
+	if (!ctx->parabuf.vaddr) {
+		ret = coda_alloc_context_buf(ctx, &ctx->parabuf,
+					     CODA_PARA_BUF_SIZE, "parabuf");
+		if (ret < 0) {
+			v4l2_err(&dev->v4l2_dev, "failed to allocate parabuf");
+			return ret;
+		}
+	}
+
 	if (dev->devtype->product == CODA_DX6)
 		return 0;
 
@@ -402,7 +421,7 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 			v4l2_err(&dev->v4l2_dev,
 				 "failed to allocate %d byte slice buffer",
 				 ctx->slicebuf.size);
-			return ret;
+			goto err;
 		}
 	}
 
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index b34a0db..172805b 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -46,7 +46,6 @@
 #define CODADX6_MAX_INSTANCES	4
 #define CODA_MAX_FORMATS	4
 
-#define CODA_PARA_BUF_SIZE	(10 * 1024)
 #define CODA_ISRAM_SIZE	(2048 * 2)
 
 #define MIN_W 176
@@ -1708,14 +1707,6 @@ static int coda_open(struct file *file)
 
 	ctx->fh.ctrl_handler = &ctx->ctrls;
 
-	if (ctx->use_bit) {
-		ret = coda_alloc_context_buf(ctx, &ctx->parabuf,
-					     CODA_PARA_BUF_SIZE, "parabuf");
-		if (ret < 0) {
-			v4l2_err(&dev->v4l2_dev, "failed to allocate parabuf");
-			goto err_dma_alloc;
-		}
-	}
 	mutex_init(&ctx->bitstream_mutex);
 	mutex_init(&ctx->buffer_mutex);
 	INIT_LIST_HEAD(&ctx->buffer_meta_list);
@@ -1729,8 +1720,6 @@ static int coda_open(struct file *file)
 
 	return 0;
 
-err_dma_alloc:
-	v4l2_ctrl_handler_free(&ctx->ctrls);
 err_ctrls_setup:
 	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 err_ctx_init:
@@ -1776,7 +1765,6 @@ static int coda_release(struct file *file)
 	if (ctx->dev->devtype->product == CODA_DX6)
 		coda_free_aux_buf(dev, &ctx->workbuf);
 
-	coda_free_aux_buf(dev, &ctx->parabuf);
 	v4l2_ctrl_handler_free(&ctx->ctrls);
 	clk_disable_unprepare(dev->clk_ahb);
 	clk_disable_unprepare(dev->clk_per);
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 01d940c..2b59e16 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -249,13 +249,6 @@ int coda_alloc_aux_buf(struct coda_dev *dev, struct coda_aux_buf *buf,
 		       size_t size, const char *name, struct dentry *parent);
 void coda_free_aux_buf(struct coda_dev *dev, struct coda_aux_buf *buf);
 
-static inline int coda_alloc_context_buf(struct coda_ctx *ctx,
-					 struct coda_aux_buf *buf, size_t size,
-					 const char *name)
-{
-	return coda_alloc_aux_buf(ctx->dev, buf, size, name, ctx->debugfs_entry);
-}
-
 int coda_encoder_queue_init(void *priv, struct vb2_queue *src_vq,
 			    struct vb2_queue *dst_vq);
 int coda_decoder_queue_init(void *priv, struct vb2_queue *src_vq,
-- 
2.1.4

[PATCH 09/12] [media] coda: remove duplicate error messages for buffer allocations

[Philipp Zabel]

coda_alloc_aux_buf already prints an error, no need to print duplicate
error messages all over the place.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-bit.c    | 21 ++++-----------------
 drivers/media/platform/coda/coda-common.c | 12 +++---------
 2 files changed, 7 insertions(+), 26 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 0073f5a..2304158 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -402,10 +402,8 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 	if (!ctx->parabuf.vaddr) {
 		ret = coda_alloc_context_buf(ctx, &ctx->parabuf,
 					     CODA_PARA_BUF_SIZE, "parabuf");
-		if (ret < 0) {
-			v4l2_err(&dev->v4l2_dev, "failed to allocate parabuf");
+		if (ret < 0)
 			return ret;
-		}
 	}
 
 	if (dev->devtype->product == CODA_DX6)
@@ -417,22 +415,15 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 			DIV_ROUND_UP(q_data->height, 16)) * 3200 / 8 + 512;
 		ret = coda_alloc_context_buf(ctx, &ctx->slicebuf, size,
 					     "slicebuf");
-		if (ret < 0) {
-			v4l2_err(&dev->v4l2_dev,
-				 "failed to allocate %d byte slice buffer",
-				 ctx->slicebuf.size);
+		if (ret < 0)
 			goto err;
-		}
 	}
 
 	if (!ctx->psbuf.vaddr && dev->devtype->product == CODA_7541) {
 		ret = coda_alloc_context_buf(ctx, &ctx->psbuf,
 					     CODA7_PS_BUF_SIZE, "psbuf");
-		if (ret < 0) {
-			v4l2_err(&dev->v4l2_dev,
-				 "failed to allocate psmem buffer");
+		if (ret < 0)
 			goto err;
-		}
 	}
 
 	if (!ctx->workbuf.vaddr) {
@@ -442,12 +433,8 @@ static int coda_alloc_context_buffers(struct coda_ctx *ctx,
 			size += CODA9_PS_SAVE_SIZE;
 		ret = coda_alloc_context_buf(ctx, &ctx->workbuf, size,
 					     "workbuf");
-		if (ret < 0) {
-			v4l2_err(&dev->v4l2_dev,
-				 "failed to allocate %d byte context buffer",
-				 ctx->workbuf.size);
+		if (ret < 0)
 			goto err;
-		}
 	}
 
 	return 0;
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 172805b..b42ccfc 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1925,10 +1925,8 @@ static void coda_fw_callback(const struct firmware *fw, void *context)
 	/* allocate auxiliary per-device code buffer for the BIT processor */
 	ret = coda_alloc_aux_buf(dev, &dev->codebuf, fw->size, "codebuf",
 				 dev->debugfs_root);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "failed to allocate code buffer\n");
+	if (ret < 0)
 		goto put_pm;
-	}
 
 	/* Copy the whole firmware image to the code buffer */
 	memcpy(dev->codebuf.vaddr, fw->data, fw->size);
@@ -2166,20 +2164,16 @@ static int coda_probe(struct platform_device *pdev)
 		ret = coda_alloc_aux_buf(dev, &dev->workbuf,
 					 dev->devtype->workbuf_size, "workbuf",
 					 dev->debugfs_root);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "failed to allocate work buffer\n");
+		if (ret < 0)
 			goto err_v4l2_register;
-		}
 	}
 
 	if (dev->devtype->tempbuf_size) {
 		ret = coda_alloc_aux_buf(dev, &dev->tempbuf,
 					 dev->devtype->tempbuf_size, "tempbuf",
 					 dev->debugfs_root);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "failed to allocate temp buffer\n");
+		if (ret < 0)
 			goto err_v4l2_register;
-		}
 	}
 
 	dev->iram.size = dev->devtype->iram_size;
-- 
2.1.4

[PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index b42ccfc..4441179 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
 	if (!(ctx->streamon_out & ctx->streamon_cap))
 		return 0;
 
+	q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
+	if ((q_data_src->width != q_data_dst->width &&
+	     round_up(q_data_src->width, 16) != q_data_dst->width) ||
+	    (q_data_src->height != q_data_dst->height &&
+	     round_up(q_data_src->height, 16) != q_data_dst->height)) {
+		v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
+			 q_data_src->width, q_data_src->height,
+			 q_data_dst->width, q_data_dst->height);
+		ret = -EINVAL;
+		goto err;
+	}
+
 	/* Allow BIT decoder device_run with no new buffers queued */
 	if (ctx->inst_type == CODA_INST_DECODER && ctx->use_bit)
 		v4l2_m2m_set_src_buffered(ctx->fh.m2m_ctx, true);
 
 	ctx->gopcounter = ctx->params.gop_size - 1;
-	q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
 	ctx->codec = coda_find_codec(ctx->dev, q_data_src->fourcc,
 				     q_data_dst->fourcc);
-- 
2.1.4

[PATCH 11/12] [media] coda: call SEQ_END when the first queue is stopped

[Philipp Zabel]

This allows to stop and restart the output queue to start a new sequence
while keeping the capture queue running. Before, sequence end would only
be issued if both output and capture queue were stopped and the sequence
start issued when reenabling the output queue would fail.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda/coda-common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 4441179..54c972f 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1339,6 +1339,9 @@ static void coda_stop_streaming(struct vb2_queue *q)
 	struct coda_ctx *ctx = vb2_get_drv_priv(q);
 	struct coda_dev *dev = ctx->dev;
 	struct vb2_buffer *buf;
+	bool stop;
+
+	stop = ctx->streamon_out && ctx->streamon_cap;
 
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
 		v4l2_dbg(1, coda_debug, &dev->v4l2_dev,
@@ -1363,7 +1366,7 @@ static void coda_stop_streaming(struct vb2_queue *q)
 			v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
 	}
 
-	if (!ctx->streamon_out && !ctx->streamon_cap) {
+	if (stop) {
 		struct coda_buffer_meta *meta;
 
 		if (ctx->ops->seq_end_work) {
-- 
2.1.4

[PATCH 12/12] [media] coda: fix fill bitstream errors in nonstreaming case

[Philipp Zabel]

From: Philipp Zabel <philipp.zabel@gmail.com>

When queueing a buffer into the bitstream fails, it has to be requeued
in the videobuf2 queue before streaming starts, but while streaming it
should be returned to userspace with an error.

Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/media/platform/coda/coda-bit.c    | 11 +++++++----
 drivers/media/platform/coda/coda-common.c |  6 +++---
 drivers/media/platform/coda/coda.h        |  2 +-
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/coda/coda-bit.c b/drivers/media/platform/coda/coda-bit.c
index 2304158..d39789d 100644
--- a/drivers/media/platform/coda/coda-bit.c
+++ b/drivers/media/platform/coda/coda-bit.c
@@ -218,7 +218,7 @@ static bool coda_bitstream_try_queue(struct coda_ctx *ctx,
 	return true;
 }
 
-void coda_fill_bitstream(struct coda_ctx *ctx)
+void coda_fill_bitstream(struct coda_ctx *ctx, bool streaming)
 {
 	struct vb2_buffer *src_buf;
 	struct coda_buffer_meta *meta;
@@ -239,9 +239,12 @@ void coda_fill_bitstream(struct coda_ctx *ctx)
 		if (ctx->codec->src_fourcc == V4L2_PIX_FMT_JPEG &&
 		    !coda_jpeg_check_buffer(ctx, src_buf)) {
 			v4l2_err(&ctx->dev->v4l2_dev,
-				 "dropping invalid JPEG frame\n");
+				 "dropping invalid JPEG frame %d\n",
+				 ctx->qsequence);
 			src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
-			v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
+			v4l2_m2m_buf_done(src_buf, streaming ?
+					  VB2_BUF_STATE_ERROR :
+					  VB2_BUF_STATE_QUEUED);
 			continue;
 		}
 
@@ -1648,7 +1651,7 @@ static int coda_prepare_decode(struct coda_ctx *ctx)
 
 	/* Try to copy source buffer contents into the bitstream ringbuffer */
 	mutex_lock(&ctx->bitstream_mutex);
-	coda_fill_bitstream(ctx);
+	coda_fill_bitstream(ctx, true);
 	mutex_unlock(&ctx->bitstream_mutex);
 
 	if (coda_get_bitstream_payload(ctx) < 512 &&
diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 54c972f..5e159da 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1192,7 +1192,7 @@ static void coda_buf_queue(struct vb2_buffer *vb)
 		mutex_lock(&ctx->bitstream_mutex);
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb);
 		if (vb2_is_streaming(vb->vb2_queue))
-			coda_fill_bitstream(ctx);
+			coda_fill_bitstream(ctx, true);
 		mutex_unlock(&ctx->bitstream_mutex);
 	} else {
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb);
@@ -1252,9 +1252,9 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
 		if (q_data_src->fourcc == V4L2_PIX_FMT_H264 ||
 		    (q_data_src->fourcc == V4L2_PIX_FMT_JPEG &&
 		     ctx->dev->devtype->product == CODA_7541)) {
-			/* copy the buffers that where queued before streamon */
+			/* copy the buffers that were queued before streamon */
 			mutex_lock(&ctx->bitstream_mutex);
-			coda_fill_bitstream(ctx);
+			coda_fill_bitstream(ctx, false);
 			mutex_unlock(&ctx->bitstream_mutex);
 
 			if (coda_get_bitstream_payload(ctx) < 512) {
diff --git a/drivers/media/platform/coda/coda.h b/drivers/media/platform/coda/coda.h
index 2b59e16..970f0b3 100644
--- a/drivers/media/platform/coda/coda.h
+++ b/drivers/media/platform/coda/coda.h
@@ -256,7 +256,7 @@ int coda_decoder_queue_init(void *priv, struct vb2_queue *src_vq,
 
 int coda_hw_reset(struct coda_ctx *ctx);
 
-void coda_fill_bitstream(struct coda_ctx *ctx);
+void coda_fill_bitstream(struct coda_ctx *ctx, bool streaming);
 
 void coda_set_gdi_regs(struct coda_ctx *ctx);
 
-- 
2.1.4

Re: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats

[Jean-Michel Hautbois]

Hi Philipp,

2015-02-23 16:20 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda/coda-common.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index b42ccfc..4441179 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
>         if (!(ctx->streamon_out & ctx->streamon_cap))
>                 return 0;
>
> +       q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +       if ((q_data_src->width != q_data_dst->width &&
> +            round_up(q_data_src->width, 16) != q_data_dst->width) ||
> +           (q_data_src->height != q_data_dst->height &&
> +            round_up(q_data_src->height, 16) != q_data_dst->height)) {
> +               v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
> +                        q_data_src->width, q_data_src->height,
> +                        q_data_dst->width, q_data_dst->height);
> +               ret = -EINVAL;
> +               goto err;
> +       }
> +

Shouldn't the driver check on queues related to encoding or decoding only ?
We don't need to set correct width/height from userspace if we are
encoding, or it should be done by s_fmt itself.

Thanks,
JM

RE: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats

[Kamil Debski]

Hi Philipp,

> From: Jean-Michel Hautbois [mailto:jhautbois@gmail.com]
> Sent: Friday, February 27, 2015 10:00 AM
> To: Philipp Zabel
> Cc: Kamil Debski; Peter Seiderer; Linux Media Mailing List; Sascha
> Hauer
> Subject: Re: [PATCH 10/12] [media] coda: fail to start streaming if
> userspace set invalid formats
> 
> Hi Philipp,
> 
> 2015-02-23 16:20 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:

Could you add a description to this patch?
Also, please remember about this patch https://patchwork.linuxtv.org/patch/28016.
It also needs a description.

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland

> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/media/platform/coda/coda-common.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c
> > b/drivers/media/platform/coda/coda-common.c
> > index b42ccfc..4441179 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct
> vb2_queue *q, unsigned int count)
> >         if (!(ctx->streamon_out & ctx->streamon_cap))
> >                 return 0;
> >
> > +       q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> > +       if ((q_data_src->width != q_data_dst->width &&
> > +            round_up(q_data_src->width, 16) != q_data_dst->width) ||
> > +           (q_data_src->height != q_data_dst->height &&
> > +            round_up(q_data_src->height, 16) != q_data_dst->height))
> {
> > +               v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
> > +                        q_data_src->width, q_data_src->height,
> > +                        q_data_dst->width, q_data_dst->height);
> > +               ret = -EINVAL;
> > +               goto err;
> > +       }
> > +
> 
> Shouldn't the driver check on queues related to encoding or decoding
> only ?
> We don't need to set correct width/height from userspace if we are
> encoding, or it should be done by s_fmt itself.
> 
> Thanks,
> JM

Re: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats

[Philipp Zabel]

Am Freitag, den 27.02.2015, 09:59 +0100 schrieb Jean-Michel Hautbois:
> Hi Philipp,
> 
> 2015-02-23 16:20 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> >  drivers/media/platform/coda/coda-common.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> > index b42ccfc..4441179 100644
> > --- a/drivers/media/platform/coda/coda-common.c
> > +++ b/drivers/media/platform/coda/coda-common.c
> > @@ -1282,12 +1282,23 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
> >         if (!(ctx->streamon_out & ctx->streamon_cap))
> >                 return 0;
> >
> > +       q_data_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
> > +       if ((q_data_src->width != q_data_dst->width &&
> > +            round_up(q_data_src->width, 16) != q_data_dst->width) ||
> > +           (q_data_src->height != q_data_dst->height &&
> > +            round_up(q_data_src->height, 16) != q_data_dst->height)) {
> > +               v4l2_err(v4l2_dev, "can't convert %dx%d to %dx%d\n",
> > +                        q_data_src->width, q_data_src->height,
> > +                        q_data_dst->width, q_data_dst->height);
> > +               ret = -EINVAL;
> > +               goto err;
> > +       }
> > +
> 
> Shouldn't the driver check on queues related to encoding or decoding only ?
> We don't need to set correct width/height from userspace if we are
> encoding, or it should be done by s_fmt itself.

Good point, the notes from the V4L2 codec API session during ELCE2014
say:
   "the coded format is always the master; changing format on it
    changes the set of supported formats on the other queue;"

Since an unsupported format shouldn't be selected, this implies that
S_FMT on an encoder capture queue should possibly change the format on
the output queue at the same time.
If I enforce compatible output and capture formats during S_FMT,
this check is indeed not needed.

regards
Philipp

Re: [PATCH 10/12] [media] coda: fail to start streaming if userspace set invalid formats

[Philipp Zabel]

Hi Kamil,

Am Mittwoch, den 18.03.2015, 11:44 +0100 schrieb Kamil Debski:
> Hi Philipp,
> 
> > From: Jean-Michel Hautbois [mailto:jhautbois@gmail.com]
> > Sent: Friday, February 27, 2015 10:00 AM
> > To: Philipp Zabel
> > Cc: Kamil Debski; Peter Seiderer; Linux Media Mailing List; Sascha
> > Hauer
> > Subject: Re: [PATCH 10/12] [media] coda: fail to start streaming if
> > userspace set invalid formats
> > 
> > Hi Philipp,
> > 
> > 2015-02-23 16:20 GMT+01:00 Philipp Zabel <p.zabel@pengutronix.de>:
> 
> Could you add a description to this patch?

I think I'll have to solve this differently.

> Also, please remember about this patch https://patchwork.linuxtv.org/patch/28016.
> It also needs a description.

Thank you for the reminder.

regards
Philipp