[PATCH 00/11] CODA encoder/decoder device split

[PATCH 00/11] CODA encoder/decoder device split

[Philipp Zabel]

Hi,

the following patches add a few fixes and cleanups and split the
coda video4linux2 device into encoder and decoder.
Following the principle of least surprise, this way the format
enumeration on the output and capture sides is fixed and does
not change depending on whether the given instance is currently
configured as encoder or decoder.

regards
Philipp

Michael Olbrich (2):
  [media] coda: use CODA_MAX_FRAME_SIZE everywhere
  [media] coda: delay coda_fill_bitstream()

Philipp Zabel (9):
  [media] coda: fix CODA7541 hardware reset
  [media] coda: initialize hardware on pm runtime resume only if
    firmware available
  [media] coda: remove CAPTURE and OUTPUT caps
  [media] coda: remove VB2_USERPTR from queue io_modes
  [media] coda: lock capture frame size to output frame size when
    streaming
  [media] coda: split userspace interface into encoder and decoder
    device
  [media] coda: split format enumeration for encoder end decoder device
  [media] coda: default to h.264 decoder on invalid formats
  [media] coda: mark constant structures as such

 drivers/media/platform/coda.c | 316 ++++++++++++++++++++++++++----------------
 1 file changed, 193 insertions(+), 123 deletions(-)

-- 
2.0.1

[PATCH 01/11] [media] coda: fix CODA7541 hardware reset

[Philipp Zabel]

Do not try to read the CODA960 GDI status register on CODA7541.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 7e69eda..d5abb7c 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -350,19 +350,22 @@ static int coda_hw_reset(struct coda_ctx *ctx)
 
 	idx = coda_read(dev, CODA_REG_BIT_RUN_INDEX);
 
-	timeout = jiffies + msecs_to_jiffies(100);
-	coda_write(dev, 0x11, CODA9_GDI_BUS_CTRL);
-	while (coda_read(dev, CODA9_GDI_BUS_STATUS) != 0x77) {
-		if (time_after(jiffies, timeout))
-			return -ETIME;
-		cpu_relax();
+	if (dev->devtype->product == CODA_960) {
+		timeout = jiffies + msecs_to_jiffies(100);
+		coda_write(dev, 0x11, CODA9_GDI_BUS_CTRL);
+		while (coda_read(dev, CODA9_GDI_BUS_STATUS) != 0x77) {
+			if (time_after(jiffies, timeout))
+				return -ETIME;
+			cpu_relax();
+		}
 	}
 
 	ret = reset_control_reset(dev->rstc);
 	if (ret < 0)
 		return ret;
 
-	coda_write(dev, 0x00, CODA9_GDI_BUS_CTRL);
+	if (dev->devtype->product == CODA_960)
+		coda_write(dev, 0x00, CODA9_GDI_BUS_CTRL);
 	coda_write(dev, CODA_REG_BIT_BUSY_FLAG, CODA_REG_BIT_BUSY);
 	coda_write(dev, CODA_REG_RUN_ENABLE, CODA_REG_BIT_CODE_RUN);
 	ret = coda_wait_timeout(dev);
-- 
2.0.1

[PATCH 02/11] [media] coda: initialize hardware on pm runtime resume only if firmware available

[Philipp Zabel]

If no firmware was found and the coda module is unloaded, coda_runtime_resume
will be called without an allocated code buffer. Do not call coda_hw_init in
this case.

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index d5abb7c..10f9278 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -3904,7 +3904,7 @@ static int coda_runtime_resume(struct device *dev)
 	struct coda_dev *cdev = dev_get_drvdata(dev);
 	int ret = 0;
 
-	if (dev->pm_domain) {
+	if (dev->pm_domain && cdev->codebuf.vaddr) {
 		ret = coda_hw_init(cdev);
 		if (ret)
 			v4l2_err(&cdev->v4l2_dev, "HW initialization failed\n");
-- 
2.0.1

[PATCH 03/11] [media] coda: remove CAPTURE and OUTPUT caps

[Philipp Zabel]

This is a mem2mem driver, pure capture or output modes are not
supported.

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 10f9278..f52d17c 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -536,13 +536,7 @@ static int coda_querycap(struct file *file, void *priv,
 	strlcpy(cap->card, coda_product_name(ctx->dev->devtype->product),
 		sizeof(cap->card));
 	strlcpy(cap->bus_info, "platform:" CODA_NAME, sizeof(cap->bus_info));
-	/*
-	 * This is only a mem-to-mem video device. The capture and output
-	 * device capability flags are left only for backward compatibility
-	 * and are scheduled for removal.
-	 */
-	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT |
-			   V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
 	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
-- 
2.0.1

[PATCH 04/11] [media] coda: remove VB2_USERPTR from queue io_modes

[Philipp Zabel]

Disallow USERPTR buffers, videobuf2-dma-contig doesn't support them.

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index f52d17c..917727e 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -2865,7 +2865,7 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq,
 	int ret;
 
 	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
-	src_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
+	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
 	src_vq->drv_priv = ctx;
 	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	src_vq->ops = &coda_qops;
@@ -2878,7 +2878,7 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq,
 		return ret;
 
 	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
+	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
 	dst_vq->drv_priv = ctx;
 	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
 	dst_vq->ops = &coda_qops;
-- 
2.0.1

[PATCH 05/11] [media] coda: use CODA_MAX_FRAME_SIZE everywhere

[Philipp Zabel]

From: Michael Olbrich <m.olbrich@pengutronix.de>

Without this changing CODA_MAX_FRAME_SIZE to anything other than 0x100000
can break the bitstram handling

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 917727e..141ec29 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -3106,7 +3106,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) >= 0x100000 - 512)
+		if (coda_get_bitstream_payload(ctx) >= CODA_MAX_FRAME_SIZE - 512)
 			kfifo_init(&ctx->bitstream_fifo,
 				ctx->bitstream.vaddr, ctx->bitstream.size);
 	}
-- 
2.0.1

[PATCH 06/11] [media] coda: delay coda_fill_bitstream()

[Philipp Zabel]

From: Michael Olbrich <m.olbrich@pengutronix.de>

coda_fill_bitstream() calls v4l2_m2m_buf_done() which is no longer allowed
before streaming was started.
Delay coda_fill_bitstream() until coda_start_streaming() and explicitly set
'start_streaming_called' before calling coda_fill_bitstream()

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 141ec29..3d57986 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -1682,7 +1682,8 @@ static void coda_buf_queue(struct vb2_buffer *vb)
 		}
 		mutex_lock(&ctx->bitstream_mutex);
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb);
-		coda_fill_bitstream(ctx);
+		if (vb2_is_streaming(vb->vb2_queue))
+			coda_fill_bitstream(ctx);
 		mutex_unlock(&ctx->bitstream_mutex);
 	} else {
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb);
@@ -2272,6 +2273,15 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
 	q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
 		if (q_data_src->fourcc == V4L2_PIX_FMT_H264) {
+			struct vb2_queue *vq;
+			/* start_streaming_called must be set, for v4l2_m2m_buf_done() */
+			vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+			vq->start_streaming_called = 1;
+			/* copy the buffers that where queued before streamon */
+			mutex_lock(&ctx->bitstream_mutex);
+			coda_fill_bitstream(ctx);
+			mutex_unlock(&ctx->bitstream_mutex);
+
 			if (coda_get_bitstream_payload(ctx) < 512)
 				return -EINVAL;
 		} else {
-- 
2.0.1

[PATCH 07/11] [media] coda: lock capture frame size to output frame size when streaming

[Philipp Zabel]

As soon as the output queue is streaming, let try_fmt on the capture side
only allow the frame size that was set on the output side.

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 3d57986..6b659c8 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -721,6 +721,9 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
 					f->fmt.pix.pixelformat);
 		if (!codec)
 			return -EINVAL;
+
+		f->fmt.pix.width = q_data_src->width;
+		f->fmt.pix.height = q_data_src->height;
 	} else {
 		/* Otherwise determine codec by encoded format, if possible */
 		codec = coda_find_codec(ctx->dev, V4L2_PIX_FMT_YUV420,
-- 
2.0.1

[PATCH 08/11] [media] coda: split userspace interface into encoder and decoder device

[Philipp Zabel]

Userspace has a hard time making sense of format enumerations on V4L2
mem2mem devices if there are restrictions on which input and output
formats can be used together. Alleviate the problem by splitting the
video4linux device into separate encoder and decoder devices which list
only raw formats on one side and only encoded formats on the other side.
With this patch, the instance type (encoder or decoder) is already
determined by the open file operation.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda.c | 167 ++++++++++++++++++++++++++++++------------
 1 file changed, 120 insertions(+), 47 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 6b659c8..4a159031 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -129,7 +129,7 @@ struct coda_aux_buf {
 
 struct coda_dev {
 	struct v4l2_device	v4l2_dev;
-	struct video_device	vfd;
+	struct video_device	vfd[2];
 	struct platform_device	*plat_dev;
 	const struct coda_devtype *devtype;
 
@@ -1580,14 +1580,22 @@ static void coda_set_tiled_map_type(struct coda_ctx *ctx, int tiled_map_type)
 
 static void set_default_params(struct coda_ctx *ctx)
 {
+	u32 src_fourcc, dst_fourcc;
 	int max_w;
 	int max_h;
 
-	ctx->codec = &ctx->dev->devtype->codecs[0];
+	if (ctx->inst_type == CODA_INST_ENCODER) {
+		src_fourcc = V4L2_PIX_FMT_YUV420;
+		dst_fourcc = V4L2_PIX_FMT_H264;
+	} else {
+		src_fourcc = V4L2_PIX_FMT_H264;
+		dst_fourcc = V4L2_PIX_FMT_YUV420;
+	}
+	ctx->codec = coda_find_codec(ctx->dev, src_fourcc, dst_fourcc);
 	max_w = ctx->codec->max_w;
 	max_h = ctx->codec->max_h;
 
-	ctx->params.codec_mode = CODA_MODE_INVALID;
+	ctx->params.codec_mode = ctx->codec->mode;
 	ctx->colorspace = V4L2_COLORSPACE_REC709;
 	ctx->params.framerate = 30;
 	ctx->aborting = 0;
@@ -1597,12 +1605,19 @@ static void set_default_params(struct coda_ctx *ctx)
 	ctx->q_data[V4L2_M2M_DST].fourcc = ctx->codec->dst_fourcc;
 	ctx->q_data[V4L2_M2M_SRC].width = max_w;
 	ctx->q_data[V4L2_M2M_SRC].height = max_h;
-	ctx->q_data[V4L2_M2M_SRC].bytesperline = max_w;
-	ctx->q_data[V4L2_M2M_SRC].sizeimage = (max_w * max_h * 3) / 2;
 	ctx->q_data[V4L2_M2M_DST].width = max_w;
 	ctx->q_data[V4L2_M2M_DST].height = max_h;
-	ctx->q_data[V4L2_M2M_DST].bytesperline = 0;
-	ctx->q_data[V4L2_M2M_DST].sizeimage = CODA_MAX_FRAME_SIZE;
+	if (ctx->codec->src_fourcc == V4L2_PIX_FMT_YUV420) {
+		ctx->q_data[V4L2_M2M_SRC].bytesperline = max_w;
+		ctx->q_data[V4L2_M2M_SRC].sizeimage = (max_w * max_h * 3) / 2;
+		ctx->q_data[V4L2_M2M_DST].bytesperline = 0;
+		ctx->q_data[V4L2_M2M_DST].sizeimage = CODA_MAX_FRAME_SIZE;
+	} else {
+		ctx->q_data[V4L2_M2M_SRC].bytesperline = 0;
+		ctx->q_data[V4L2_M2M_SRC].sizeimage = CODA_MAX_FRAME_SIZE;
+		ctx->q_data[V4L2_M2M_DST].bytesperline = max_w;
+		ctx->q_data[V4L2_M2M_DST].sizeimage = (max_w * max_h * 3) / 2;
+	}
 	ctx->q_data[V4L2_M2M_SRC].rect.width = max_w;
 	ctx->q_data[V4L2_M2M_SRC].rect.height = max_h;
 	ctx->q_data[V4L2_M2M_DST].rect.width = max_w;
@@ -2293,11 +2308,6 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
 		}
 
 		ctx->streamon_out = 1;
-
-		if (coda_format_is_yuv(q_data_src->fourcc))
-			ctx->inst_type = CODA_INST_ENCODER;
-		else
-			ctx->inst_type = CODA_INST_DECODER;
 	} else {
 		if (count < 1)
 			return -EINVAL;
@@ -2719,7 +2729,7 @@ static void coda_stop_streaming(struct vb2_queue *q)
 	}
 }
 
-static struct vb2_ops coda_qops = {
+static const struct vb2_ops coda_qops = {
 	.queue_setup		= coda_queue_setup,
 	.buf_prepare		= coda_buf_prepare,
 	.buf_queue		= coda_buf_queue,
@@ -2871,35 +2881,55 @@ static int coda_ctrls_setup(struct coda_ctx *ctx)
 	return v4l2_ctrl_handler_setup(&ctx->ctrls);
 }
 
-static int coda_queue_init(void *priv, struct vb2_queue *src_vq,
-		      struct vb2_queue *dst_vq)
+static int coda_queue_init(struct coda_ctx *ctx, struct vb2_queue *vq)
+{
+	vq->drv_priv = ctx;
+	vq->ops = &coda_qops;
+	vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
+	vq->lock = &ctx->dev->dev_mutex;
+
+	return vb2_queue_init(vq);
+}
+
+static int coda_encoder_queue_init(void *priv, struct vb2_queue *src_vq,
+				   struct vb2_queue *dst_vq)
 {
-	struct coda_ctx *ctx = priv;
 	int ret;
 
 	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
 	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
-	src_vq->drv_priv = ctx;
-	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
-	src_vq->ops = &coda_qops;
 	src_vq->mem_ops = &vb2_dma_contig_memops;
-	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-	src_vq->lock = &ctx->dev->dev_mutex;
 
-	ret = vb2_queue_init(src_vq);
+	ret = coda_queue_init(priv, src_vq);
 	if (ret)
 		return ret;
 
 	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
-	dst_vq->drv_priv = ctx;
-	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
-	dst_vq->ops = &coda_qops;
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
-	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-	dst_vq->lock = &ctx->dev->dev_mutex;
 
-	return vb2_queue_init(dst_vq);
+	return coda_queue_init(priv, dst_vq);
+}
+
+static int coda_decoder_queue_init(void *priv, struct vb2_queue *src_vq,
+				   struct vb2_queue *dst_vq)
+{
+	int ret;
+
+	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
+	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
+	src_vq->mem_ops = &vb2_dma_contig_memops;
+
+	ret = coda_queue_init(priv, src_vq);
+	if (ret)
+		return ret;
+
+	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
+	dst_vq->mem_ops = &vb2_dma_contig_memops;
+
+	return coda_queue_init(priv, dst_vq);
 }
 
 static int coda_next_free_instance(struct coda_dev *dev)
@@ -2913,8 +2943,10 @@ static int coda_next_free_instance(struct coda_dev *dev)
 	return idx;
 }
 
-static int coda_open(struct file *file)
+static int coda_open(struct file *file, enum coda_inst_type inst_type)
 {
+	int (*queue_init)(void *priv, struct vb2_queue *src_vq,
+			  struct vb2_queue *dst_vq);
 	struct coda_dev *dev = video_drvdata(file);
 	struct coda_ctx *ctx = NULL;
 	char *name;
@@ -2936,6 +2968,7 @@ static int coda_open(struct file *file)
 	ctx->debugfs_entry = debugfs_create_dir(name, dev->debugfs_root);
 	kfree(name);
 
+	ctx->inst_type = inst_type;
 	init_completion(&ctx->completion);
 	INIT_WORK(&ctx->pic_run_work, coda_pic_run_work);
 	INIT_WORK(&ctx->seq_end_work, coda_seq_end_work);
@@ -2969,8 +3002,11 @@ static int coda_open(struct file *file)
 		goto err_clk_ahb;
 
 	set_default_params(ctx);
-	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx,
-					 &coda_queue_init);
+	if (inst_type == CODA_INST_ENCODER)
+		queue_init = coda_encoder_queue_init;
+	else
+		queue_init = coda_decoder_queue_init;
+	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, queue_init);
 	if (IS_ERR(ctx->fh.m2m_ctx)) {
 		ret = PTR_ERR(ctx->fh.m2m_ctx);
 
@@ -3041,6 +3077,16 @@ err_coda_max:
 	return ret;
 }
 
+static int coda_encoder_open(struct file *file)
+{
+	return coda_open(file, CODA_INST_ENCODER);
+}
+
+static int coda_decoder_open(struct file *file)
+{
+	return coda_open(file, CODA_INST_DECODER);
+}
+
 static int coda_release(struct file *file)
 {
 	struct coda_dev *dev = video_drvdata(file);
@@ -3085,9 +3131,18 @@ static int coda_release(struct file *file)
 	return 0;
 }
 
-static const struct v4l2_file_operations coda_fops = {
+static const struct v4l2_file_operations coda_encoder_fops = {
 	.owner		= THIS_MODULE,
-	.open		= coda_open,
+	.open		= coda_encoder_open,
+	.release	= coda_release,
+	.poll		= v4l2_m2m_fop_poll,
+	.unlocked_ioctl	= video_ioctl2,
+	.mmap		= v4l2_m2m_fop_mmap,
+};
+
+static const struct v4l2_file_operations coda_decoder_fops = {
+	.owner		= THIS_MODULE,
+	.open		= coda_decoder_open,
 	.release	= coda_release,
 	.poll		= v4l2_m2m_fop_poll,
 	.unlocked_ioctl	= video_ioctl2,
@@ -3570,6 +3625,17 @@ err_clk_per:
 	return ret;
 }
 
+static int coda_register_device(struct coda_dev *dev, struct video_device *vfd)
+{
+	vfd->release	= video_device_release_empty,
+	vfd->lock	= &dev->dev_mutex;
+	vfd->v4l2_dev	= &dev->v4l2_dev;
+	vfd->vfl_dir	= VFL_DIR_M2M;
+	video_set_drvdata(vfd, dev);
+
+	return video_register_device(vfd, VFL_TYPE_GRABBER, 0);
+}
+
 static void coda_fw_callback(const struct firmware *fw, void *context)
 {
 	struct coda_dev *dev = context;
@@ -3626,15 +3692,6 @@ static void coda_fw_callback(const struct firmware *fw, void *context)
 			return;
 	}
 
-	dev->vfd.fops	= &coda_fops,
-	dev->vfd.ioctl_ops	= &coda_ioctl_ops;
-	dev->vfd.release	= video_device_release_empty,
-	dev->vfd.lock	= &dev->dev_mutex;
-	dev->vfd.v4l2_dev	= &dev->v4l2_dev;
-	dev->vfd.vfl_dir	= VFL_DIR_M2M;
-	snprintf(dev->vfd.name, sizeof(dev->vfd.name), "%s", CODA_NAME);
-	video_set_drvdata(&dev->vfd, dev);
-
 	dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
 	if (IS_ERR(dev->alloc_ctx)) {
 		v4l2_err(&dev->v4l2_dev, "Failed to alloc vb2 context\n");
@@ -3647,13 +3704,28 @@ static void coda_fw_callback(const struct firmware *fw, void *context)
 		goto rel_ctx;
 	}
 
-	ret = video_register_device(&dev->vfd, VFL_TYPE_GRABBER, 0);
+	dev->vfd[0].fops      = &coda_encoder_fops,
+	dev->vfd[0].ioctl_ops = &coda_ioctl_ops;
+	snprintf(dev->vfd[0].name, sizeof(dev->vfd[0].name), "coda-encoder");
+	ret = coda_register_device(dev, &dev->vfd[0]);
 	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
+		v4l2_err(&dev->v4l2_dev,
+			 "Failed to register encoder video device\n");
 		goto rel_m2m;
 	}
-	v4l2_info(&dev->v4l2_dev, "codec registered as /dev/video%d\n",
-		  dev->vfd.num);
+
+	dev->vfd[1].fops      = &coda_decoder_fops,
+	dev->vfd[1].ioctl_ops = &coda_ioctl_ops;
+	snprintf(dev->vfd[1].name, sizeof(dev->vfd[1].name), "coda-decoder");
+	ret = coda_register_device(dev, &dev->vfd[1]);
+	if (ret) {
+		v4l2_err(&dev->v4l2_dev,
+			 "Failed to register decoder video device\n");
+		goto rel_m2m;
+	}
+
+	v4l2_info(&dev->v4l2_dev, "codec registered as /dev/video[%d-%d]\n",
+		  dev->vfd[0].num, dev->vfd[1].num);
 
 	return;
 
@@ -3887,7 +3959,8 @@ static int coda_remove(struct platform_device *pdev)
 {
 	struct coda_dev *dev = platform_get_drvdata(pdev);
 
-	video_unregister_device(&dev->vfd);
+	video_unregister_device(&dev->vfd[0]);
+	video_unregister_device(&dev->vfd[1]);
 	if (dev->m2m_dev)
 		v4l2_m2m_release(dev->m2m_dev);
 	pm_runtime_disable(&pdev->dev);
-- 
2.0.1

[PATCH 09/11] [media] coda: split format enumeration for encoder end decoder device

[Philipp Zabel]

Let the decoder capture side and encoder output side only list
uncompressed formats, and the decoder output and encoder capture
side only list compressed formats.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 drivers/media/platform/coda.c | 57 +++++++++++++++----------------------------
 1 file changed, 19 insertions(+), 38 deletions(-)

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index 4a159031..e63226b 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -542,8 +542,8 @@ static int coda_querycap(struct file *file, void *priv,
 	return 0;
 }
 
-static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
-			enum v4l2_buf_type type, int src_fourcc)
+static int coda_enum_fmt(struct file *file, void *priv,
+			 struct v4l2_fmtdesc *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
 	struct coda_codec *codecs = ctx->dev->devtype->codecs;
@@ -552,11 +552,19 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
 	int num_codecs = ctx->dev->devtype->num_codecs;
 	int num_formats = ARRAY_SIZE(coda_formats);
 	int i, k, num = 0;
+	bool yuv;
+
+	if (ctx->inst_type == CODA_INST_ENCODER)
+		yuv = (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT);
+	else
+		yuv = (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
 	for (i = 0; i < num_formats; i++) {
-		/* Both uncompressed formats are always supported */
-		if (coda_format_is_yuv(formats[i].fourcc) &&
-		    !coda_format_is_yuv(src_fourcc)) {
+		/* Skip either raw or compressed formats */
+		if (yuv != coda_format_is_yuv(formats[i].fourcc))
+			continue;
+		/* All uncompressed formats are always supported */
+		if (yuv) {
 			if (num == f->index)
 				break;
 			++num;
@@ -564,12 +572,10 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
 		}
 		/* Compressed formats may be supported, check the codec list */
 		for (k = 0; k < num_codecs; k++) {
-			/* if src_fourcc is set, only consider matching codecs */
-			if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-			    formats[i].fourcc == codecs[k].dst_fourcc &&
-			    (!src_fourcc || src_fourcc == codecs[k].src_fourcc))
+			if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
+			    formats[i].fourcc == codecs[k].dst_fourcc)
 				break;
-			if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
+			if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
 			    formats[i].fourcc == codecs[k].src_fourcc)
 				break;
 		}
@@ -584,7 +590,7 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
 		fmt = &formats[i];
 		strlcpy(f->description, fmt->name, sizeof(f->description));
 		f->pixelformat = fmt->fourcc;
-		if (!coda_format_is_yuv(fmt->fourcc))
+		if (!yuv)
 			f->flags |= V4L2_FMT_FLAG_COMPRESSED;
 		return 0;
 	}
@@ -593,31 +599,6 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
 	return -EINVAL;
 }
 
-static int coda_enum_fmt_vid_cap(struct file *file, void *priv,
-				 struct v4l2_fmtdesc *f)
-{
-	struct coda_ctx *ctx = fh_to_ctx(priv);
-	struct vb2_queue *src_vq;
-	struct coda_q_data *q_data_src;
-
-	/* If the source format is already fixed, only list matching formats */
-	src_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
-	if (vb2_is_streaming(src_vq)) {
-		q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
-
-		return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_CAPTURE,
-				q_data_src->fourcc);
-	}
-
-	return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0);
-}
-
-static int coda_enum_fmt_vid_out(struct file *file, void *priv,
-				 struct v4l2_fmtdesc *f)
-{
-	return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_OUTPUT, 0);
-}
-
 static int coda_g_fmt(struct file *file, void *priv,
 		      struct v4l2_format *f)
 {
@@ -972,12 +953,12 @@ static int coda_subscribe_event(struct v4l2_fh *fh,
 static const struct v4l2_ioctl_ops coda_ioctl_ops = {
 	.vidioc_querycap	= coda_querycap,
 
-	.vidioc_enum_fmt_vid_cap = coda_enum_fmt_vid_cap,
+	.vidioc_enum_fmt_vid_cap = coda_enum_fmt,
 	.vidioc_g_fmt_vid_cap	= coda_g_fmt,
 	.vidioc_try_fmt_vid_cap	= coda_try_fmt_vid_cap,
 	.vidioc_s_fmt_vid_cap	= coda_s_fmt_vid_cap,
 
-	.vidioc_enum_fmt_vid_out = coda_enum_fmt_vid_out,
+	.vidioc_enum_fmt_vid_out = coda_enum_fmt,
 	.vidioc_g_fmt_vid_out	= coda_g_fmt,
 	.vidioc_try_fmt_vid_out	= coda_try_fmt_vid_out,
 	.vidioc_s_fmt_vid_out	= coda_s_fmt_vid_out,
-- 
2.0.1

[PATCH 10/11] [media] coda: default to h.264 decoder on invalid formats

[Philipp Zabel]

If the user provides an invalid format, let the decoder device
default to h.264.

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index e63226b..ed5fa4c 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -685,7 +685,7 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
-	struct coda_codec *codec;
+	struct coda_codec *codec = NULL;
 	struct vb2_queue *src_vq;
 	int ret;
 
@@ -738,6 +738,12 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv,
 	/* Determine codec by encoded format, returns NULL if raw or invalid */
 	codec = coda_find_codec(ctx->dev, f->fmt.pix.pixelformat,
 				V4L2_PIX_FMT_YUV420);
+	if (!codec && ctx->inst_type == CODA_INST_DECODER) {
+		codec = coda_find_codec(ctx->dev, V4L2_PIX_FMT_H264,
+					V4L2_PIX_FMT_YUV420);
+		if (!codec)
+			return -EINVAL;
+	}
 
 	if (!f->fmt.pix.colorspace)
 		f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
-- 
2.0.1

[PATCH 11/11] [media] coda: mark constant structures as such

[Philipp Zabel]

The format and codec lists and the ops structures are read-only.
Mark them as const.

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

diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
index ed5fa4c..b644f2b 100644
--- a/drivers/media/platform/coda.c
+++ b/drivers/media/platform/coda.c
@@ -102,7 +102,7 @@ struct coda_codec {
 struct coda_devtype {
 	char			*firmware;
 	enum coda_product	product;
-	struct coda_codec	*codecs;
+	const struct coda_codec	*codecs;
 	unsigned int		num_codecs;
 	size_t			workbuf_size;
 	size_t			tempbuf_size;
@@ -225,7 +225,7 @@ struct coda_ctx {
 	u32				sequence_offset;
 	struct coda_q_data		q_data[2];
 	enum coda_inst_type		inst_type;
-	struct coda_codec		*codec;
+	const struct coda_codec		*codec;
 	enum v4l2_colorspace		colorspace;
 	struct coda_params		params;
 	struct v4l2_ctrl_handler	ctrls;
@@ -390,7 +390,7 @@ static struct coda_q_data *get_q_data(struct coda_ctx *ctx,
 /*
  * Array of all formats supported by any version of Coda:
  */
-static struct coda_fmt coda_formats[] = {
+static const struct coda_fmt coda_formats[] = {
 	{
 		.name = "YUV 4:2:0 Planar, YCbCr",
 		.fourcc = V4L2_PIX_FMT_YUV420,
@@ -419,19 +419,19 @@ static struct coda_fmt coda_formats[] = {
  *  i.MX6  -> coda960
  * Use V4L2_PIX_FMT_YUV420 as placeholder for all supported YUV 4:2:0 variants
  */
-static struct coda_codec codadx6_codecs[] = {
+static const struct coda_codec codadx6_codecs[] = {
 	CODA_CODEC(CODADX6_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_H264,  720, 576),
 	CODA_CODEC(CODADX6_MODE_ENCODE_MP4,  V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_MPEG4, 720, 576),
 };
 
-static struct coda_codec coda7_codecs[] = {
+static const struct coda_codec coda7_codecs[] = {
 	CODA_CODEC(CODA7_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_H264,   1280, 720),
 	CODA_CODEC(CODA7_MODE_ENCODE_MP4,  V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_MPEG4,  1280, 720),
 	CODA_CODEC(CODA7_MODE_DECODE_H264, V4L2_PIX_FMT_H264,   V4L2_PIX_FMT_YUV420, 1920, 1080),
 	CODA_CODEC(CODA7_MODE_DECODE_MP4,  V4L2_PIX_FMT_MPEG4,  V4L2_PIX_FMT_YUV420, 1920, 1080),
 };
 
-static struct coda_codec coda9_codecs[] = {
+static const struct coda_codec coda9_codecs[] = {
 	CODA_CODEC(CODA9_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_H264,   1920, 1080),
 	CODA_CODEC(CODA9_MODE_ENCODE_MP4,  V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_MPEG4,  1920, 1080),
 	CODA_CODEC(CODA9_MODE_DECODE_H264, V4L2_PIX_FMT_H264,   V4L2_PIX_FMT_YUV420, 1920, 1080),
@@ -458,10 +458,10 @@ static u32 coda_format_normalize_yuv(u32 fourcc)
 	return coda_format_is_yuv(fourcc) ? V4L2_PIX_FMT_YUV420 : fourcc;
 }
 
-static struct coda_codec *coda_find_codec(struct coda_dev *dev, int src_fourcc,
-					  int dst_fourcc)
+static const struct coda_codec *coda_find_codec(struct coda_dev *dev,
+						int src_fourcc, int dst_fourcc)
 {
-	struct coda_codec *codecs = dev->devtype->codecs;
+	const struct coda_codec *codecs = dev->devtype->codecs;
 	int num_codecs = dev->devtype->num_codecs;
 	int k;
 
@@ -483,10 +483,10 @@ static struct coda_codec *coda_find_codec(struct coda_dev *dev, int src_fourcc,
 }
 
 static void coda_get_max_dimensions(struct coda_dev *dev,
-				    struct coda_codec *codec,
+				    const struct coda_codec *codec,
 				    int *max_w, int *max_h)
 {
-	struct coda_codec *codecs = dev->devtype->codecs;
+	const struct coda_codec *codecs = dev->devtype->codecs;
 	int num_codecs = dev->devtype->num_codecs;
 	unsigned int w, h;
 	int k;
@@ -546,9 +546,9 @@ static int coda_enum_fmt(struct file *file, void *priv,
 			 struct v4l2_fmtdesc *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
-	struct coda_codec *codecs = ctx->dev->devtype->codecs;
-	struct coda_fmt *formats = coda_formats;
-	struct coda_fmt *fmt;
+	const struct coda_codec *codecs = ctx->dev->devtype->codecs;
+	const struct coda_fmt *formats = coda_formats;
+	const struct coda_fmt *fmt;
 	int num_codecs = ctx->dev->devtype->num_codecs;
 	int num_formats = ARRAY_SIZE(coda_formats);
 	int i, k, num = 0;
@@ -621,7 +621,7 @@ static int coda_g_fmt(struct file *file, void *priv,
 	return 0;
 }
 
-static int coda_try_fmt(struct coda_ctx *ctx, struct coda_codec *codec,
+static int coda_try_fmt(struct coda_ctx *ctx, const struct coda_codec *codec,
 			struct v4l2_format *f)
 {
 	struct coda_dev *dev = ctx->dev;
@@ -685,7 +685,7 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
-	struct coda_codec *codec = NULL;
+	const struct coda_codec *codec = NULL;
 	struct vb2_queue *src_vq;
 	int ret;
 
@@ -733,7 +733,7 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct coda_ctx *ctx = fh_to_ctx(priv);
-	struct coda_codec *codec;
+	const struct coda_codec *codec;
 
 	/* Determine codec by encoded format, returns NULL if raw or invalid */
 	codec = coda_find_codec(ctx->dev, f->fmt.pix.pixelformat,
@@ -1531,7 +1531,7 @@ static void coda_unlock(void *m2m_priv)
 	mutex_unlock(&pcdev->dev_mutex);
 }
 
-static struct v4l2_m2m_ops coda_m2m_ops = {
+static const struct v4l2_m2m_ops coda_m2m_ops = {
 	.device_run	= coda_device_run,
 	.job_ready	= coda_job_ready,
 	.job_abort	= coda_job_abort,
@@ -2805,7 +2805,7 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl)
 	return 0;
 }
 
-static struct v4l2_ctrl_ops coda_ctrl_ops = {
+static const struct v4l2_ctrl_ops coda_ctrl_ops = {
 	.s_ctrl = coda_s_ctrl,
 };
 
-- 
2.0.1

Re: [PATCH 03/11] [media] coda: remove CAPTURE and OUTPUT caps

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> This is a mem2mem driver, pure capture or output modes are not
> supported.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

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

Regards,

	Hans

> ---
>  drivers/media/platform/coda.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 10f9278..f52d17c 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -536,13 +536,7 @@ static int coda_querycap(struct file *file, void *priv,
>  	strlcpy(cap->card, coda_product_name(ctx->dev->devtype->product),
>  		sizeof(cap->card));
>  	strlcpy(cap->bus_info, "platform:" CODA_NAME, sizeof(cap->bus_info));
> -	/*
> -	 * This is only a mem-to-mem video device. The capture and output
> -	 * device capability flags are left only for backward compatibility
> -	 * and are scheduled for removal.
> -	 */
> -	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_VIDEO_OUTPUT |
> -			   V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
> +	cap->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
>  	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
>  
>  	return 0;
> 

Re: [PATCH 04/11] [media] coda: remove VB2_USERPTR from queue io_modes

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> Disallow USERPTR buffers, videobuf2-dma-contig doesn't support them.

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

Regards,

	Hans

> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index f52d17c..917727e 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -2865,7 +2865,7 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq,
>  	int ret;
>  
>  	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> -	src_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
> +	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
>  	src_vq->drv_priv = ctx;
>  	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	src_vq->ops = &coda_qops;
> @@ -2878,7 +2878,7 @@ static int coda_queue_init(void *priv, struct vb2_queue *src_vq,
>  		return ret;
>  
>  	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP | VB2_USERPTR;
> +	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
>  	dst_vq->drv_priv = ctx;
>  	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
>  	dst_vq->ops = &coda_qops;
> 

Re: [PATCH 06/11] [media] coda: delay coda_fill_bitstream()

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> From: Michael Olbrich <m.olbrich@pengutronix.de>
> 
> coda_fill_bitstream() calls v4l2_m2m_buf_done() which is no longer allowed
> before streaming was started.
> Delay coda_fill_bitstream() until coda_start_streaming() and explicitly set
> 'start_streaming_called' before calling coda_fill_bitstream()
> 
> Signed-off-by: Michael Olbrich <m.olbrich@pengutronix.de>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 141ec29..3d57986 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -1682,7 +1682,8 @@ static void coda_buf_queue(struct vb2_buffer *vb)
>  		}
>  		mutex_lock(&ctx->bitstream_mutex);
>  		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb);
> -		coda_fill_bitstream(ctx);
> +		if (vb2_is_streaming(vb->vb2_queue))
> +			coda_fill_bitstream(ctx);
>  		mutex_unlock(&ctx->bitstream_mutex);
>  	} else {
>  		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vb);
> @@ -2272,6 +2273,15 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
>  	q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
>  	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
>  		if (q_data_src->fourcc == V4L2_PIX_FMT_H264) {
> +			struct vb2_queue *vq;
> +			/* start_streaming_called must be set, for v4l2_m2m_buf_done() */
> +			vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> +			vq->start_streaming_called = 1;

Why set start_streaming_called to 1? It is set before calling start_streaming.
This is a recent change in videobuf2-core.c though.

BTW, you should test with CONFIG_VIDEO_ADV_DEBUG on and force start_streaming
errors to check whether vb2_buffer_done(buf, VB2_BUF_STATE_DEQUEUED) is called
for the queued buffers in case of start_streaming failure.

With that config option vb2 will complain about unbalanced vb2 operations.

I strongly suspect this code does not play well with this.

BTW, isn't it time to split up the coda driver? Over 3000 lines...

Regards,

	Hans

> +			/* copy the buffers that where queued before streamon */
> +			mutex_lock(&ctx->bitstream_mutex);
> +			coda_fill_bitstream(ctx);
> +			mutex_unlock(&ctx->bitstream_mutex);
> +
>  			if (coda_get_bitstream_payload(ctx) < 512)
>  				return -EINVAL;
>  		} else {
> 

Re: [PATCH 07/11] [media] coda: lock capture frame size to output frame size when streaming

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> As soon as the output queue is streaming, let try_fmt on the capture side
> only allow the frame size that was set on the output side.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

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

Regards,

	Hans

> ---
>  drivers/media/platform/coda.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 3d57986..6b659c8 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -721,6 +721,9 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
>  					f->fmt.pix.pixelformat);
>  		if (!codec)
>  			return -EINVAL;
> +
> +		f->fmt.pix.width = q_data_src->width;
> +		f->fmt.pix.height = q_data_src->height;
>  	} else {
>  		/* Otherwise determine codec by encoded format, if possible */
>  		codec = coda_find_codec(ctx->dev, V4L2_PIX_FMT_YUV420,
> 

Re: [PATCH 08/11] [media] coda: split userspace interface into encoder and decoder device

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> Userspace has a hard time making sense of format enumerations on V4L2
> mem2mem devices if there are restrictions on which input and output
> formats can be used together. Alleviate the problem by splitting the
> video4linux device into separate encoder and decoder devices which list
> only raw formats on one side and only encoded formats on the other side.
> With this patch, the instance type (encoder or decoder) is already
> determined by the open file operation.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

Nice! This is indeed a much better solution.

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

Regards,

	Hans

> ---
>  drivers/media/platform/coda.c | 167 ++++++++++++++++++++++++++++++------------
>  1 file changed, 120 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 6b659c8..4a159031 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -129,7 +129,7 @@ struct coda_aux_buf {
>  
>  struct coda_dev {
>  	struct v4l2_device	v4l2_dev;
> -	struct video_device	vfd;
> +	struct video_device	vfd[2];
>  	struct platform_device	*plat_dev;
>  	const struct coda_devtype *devtype;
>  
> @@ -1580,14 +1580,22 @@ static void coda_set_tiled_map_type(struct coda_ctx *ctx, int tiled_map_type)
>  
>  static void set_default_params(struct coda_ctx *ctx)
>  {
> +	u32 src_fourcc, dst_fourcc;
>  	int max_w;
>  	int max_h;
>  
> -	ctx->codec = &ctx->dev->devtype->codecs[0];
> +	if (ctx->inst_type == CODA_INST_ENCODER) {
> +		src_fourcc = V4L2_PIX_FMT_YUV420;
> +		dst_fourcc = V4L2_PIX_FMT_H264;
> +	} else {
> +		src_fourcc = V4L2_PIX_FMT_H264;
> +		dst_fourcc = V4L2_PIX_FMT_YUV420;
> +	}
> +	ctx->codec = coda_find_codec(ctx->dev, src_fourcc, dst_fourcc);
>  	max_w = ctx->codec->max_w;
>  	max_h = ctx->codec->max_h;
>  
> -	ctx->params.codec_mode = CODA_MODE_INVALID;
> +	ctx->params.codec_mode = ctx->codec->mode;
>  	ctx->colorspace = V4L2_COLORSPACE_REC709;
>  	ctx->params.framerate = 30;
>  	ctx->aborting = 0;
> @@ -1597,12 +1605,19 @@ static void set_default_params(struct coda_ctx *ctx)
>  	ctx->q_data[V4L2_M2M_DST].fourcc = ctx->codec->dst_fourcc;
>  	ctx->q_data[V4L2_M2M_SRC].width = max_w;
>  	ctx->q_data[V4L2_M2M_SRC].height = max_h;
> -	ctx->q_data[V4L2_M2M_SRC].bytesperline = max_w;
> -	ctx->q_data[V4L2_M2M_SRC].sizeimage = (max_w * max_h * 3) / 2;
>  	ctx->q_data[V4L2_M2M_DST].width = max_w;
>  	ctx->q_data[V4L2_M2M_DST].height = max_h;
> -	ctx->q_data[V4L2_M2M_DST].bytesperline = 0;
> -	ctx->q_data[V4L2_M2M_DST].sizeimage = CODA_MAX_FRAME_SIZE;
> +	if (ctx->codec->src_fourcc == V4L2_PIX_FMT_YUV420) {
> +		ctx->q_data[V4L2_M2M_SRC].bytesperline = max_w;
> +		ctx->q_data[V4L2_M2M_SRC].sizeimage = (max_w * max_h * 3) / 2;
> +		ctx->q_data[V4L2_M2M_DST].bytesperline = 0;
> +		ctx->q_data[V4L2_M2M_DST].sizeimage = CODA_MAX_FRAME_SIZE;
> +	} else {
> +		ctx->q_data[V4L2_M2M_SRC].bytesperline = 0;
> +		ctx->q_data[V4L2_M2M_SRC].sizeimage = CODA_MAX_FRAME_SIZE;
> +		ctx->q_data[V4L2_M2M_DST].bytesperline = max_w;
> +		ctx->q_data[V4L2_M2M_DST].sizeimage = (max_w * max_h * 3) / 2;
> +	}
>  	ctx->q_data[V4L2_M2M_SRC].rect.width = max_w;
>  	ctx->q_data[V4L2_M2M_SRC].rect.height = max_h;
>  	ctx->q_data[V4L2_M2M_DST].rect.width = max_w;
> @@ -2293,11 +2308,6 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
>  		}
>  
>  		ctx->streamon_out = 1;
> -
> -		if (coda_format_is_yuv(q_data_src->fourcc))
> -			ctx->inst_type = CODA_INST_ENCODER;
> -		else
> -			ctx->inst_type = CODA_INST_DECODER;
>  	} else {
>  		if (count < 1)
>  			return -EINVAL;
> @@ -2719,7 +2729,7 @@ static void coda_stop_streaming(struct vb2_queue *q)
>  	}
>  }
>  
> -static struct vb2_ops coda_qops = {
> +static const struct vb2_ops coda_qops = {
>  	.queue_setup		= coda_queue_setup,
>  	.buf_prepare		= coda_buf_prepare,
>  	.buf_queue		= coda_buf_queue,
> @@ -2871,35 +2881,55 @@ static int coda_ctrls_setup(struct coda_ctx *ctx)
>  	return v4l2_ctrl_handler_setup(&ctx->ctrls);
>  }
>  
> -static int coda_queue_init(void *priv, struct vb2_queue *src_vq,
> -		      struct vb2_queue *dst_vq)
> +static int coda_queue_init(struct coda_ctx *ctx, struct vb2_queue *vq)
> +{
> +	vq->drv_priv = ctx;
> +	vq->ops = &coda_qops;
> +	vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> +	vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> +	vq->lock = &ctx->dev->dev_mutex;
> +
> +	return vb2_queue_init(vq);
> +}
> +
> +static int coda_encoder_queue_init(void *priv, struct vb2_queue *src_vq,
> +				   struct vb2_queue *dst_vq)
>  {
> -	struct coda_ctx *ctx = priv;
>  	int ret;
>  
>  	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
>  	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> -	src_vq->drv_priv = ctx;
> -	src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> -	src_vq->ops = &coda_qops;
>  	src_vq->mem_ops = &vb2_dma_contig_memops;
> -	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> -	src_vq->lock = &ctx->dev->dev_mutex;
>  
> -	ret = vb2_queue_init(src_vq);
> +	ret = coda_queue_init(priv, src_vq);
>  	if (ret)
>  		return ret;
>  
>  	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>  	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> -	dst_vq->drv_priv = ctx;
> -	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
> -	dst_vq->ops = &coda_qops;
>  	dst_vq->mem_ops = &vb2_dma_contig_memops;
> -	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> -	dst_vq->lock = &ctx->dev->dev_mutex;
>  
> -	return vb2_queue_init(dst_vq);
> +	return coda_queue_init(priv, dst_vq);
> +}
> +
> +static int coda_decoder_queue_init(void *priv, struct vb2_queue *src_vq,
> +				   struct vb2_queue *dst_vq)
> +{
> +	int ret;
> +
> +	src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT;
> +	src_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> +	src_vq->mem_ops = &vb2_dma_contig_memops;
> +
> +	ret = coda_queue_init(priv, src_vq);
> +	if (ret)
> +		return ret;
> +
> +	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
> +	dst_vq->mem_ops = &vb2_dma_contig_memops;
> +
> +	return coda_queue_init(priv, dst_vq);
>  }
>  
>  static int coda_next_free_instance(struct coda_dev *dev)
> @@ -2913,8 +2943,10 @@ static int coda_next_free_instance(struct coda_dev *dev)
>  	return idx;
>  }
>  
> -static int coda_open(struct file *file)
> +static int coda_open(struct file *file, enum coda_inst_type inst_type)
>  {
> +	int (*queue_init)(void *priv, struct vb2_queue *src_vq,
> +			  struct vb2_queue *dst_vq);
>  	struct coda_dev *dev = video_drvdata(file);
>  	struct coda_ctx *ctx = NULL;
>  	char *name;
> @@ -2936,6 +2968,7 @@ static int coda_open(struct file *file)
>  	ctx->debugfs_entry = debugfs_create_dir(name, dev->debugfs_root);
>  	kfree(name);
>  
> +	ctx->inst_type = inst_type;
>  	init_completion(&ctx->completion);
>  	INIT_WORK(&ctx->pic_run_work, coda_pic_run_work);
>  	INIT_WORK(&ctx->seq_end_work, coda_seq_end_work);
> @@ -2969,8 +3002,11 @@ static int coda_open(struct file *file)
>  		goto err_clk_ahb;
>  
>  	set_default_params(ctx);
> -	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx,
> -					 &coda_queue_init);
> +	if (inst_type == CODA_INST_ENCODER)
> +		queue_init = coda_encoder_queue_init;
> +	else
> +		queue_init = coda_decoder_queue_init;
> +	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx, queue_init);
>  	if (IS_ERR(ctx->fh.m2m_ctx)) {
>  		ret = PTR_ERR(ctx->fh.m2m_ctx);
>  
> @@ -3041,6 +3077,16 @@ err_coda_max:
>  	return ret;
>  }
>  
> +static int coda_encoder_open(struct file *file)
> +{
> +	return coda_open(file, CODA_INST_ENCODER);
> +}
> +
> +static int coda_decoder_open(struct file *file)
> +{
> +	return coda_open(file, CODA_INST_DECODER);
> +}
> +
>  static int coda_release(struct file *file)
>  {
>  	struct coda_dev *dev = video_drvdata(file);
> @@ -3085,9 +3131,18 @@ static int coda_release(struct file *file)
>  	return 0;
>  }
>  
> -static const struct v4l2_file_operations coda_fops = {
> +static const struct v4l2_file_operations coda_encoder_fops = {
>  	.owner		= THIS_MODULE,
> -	.open		= coda_open,
> +	.open		= coda_encoder_open,
> +	.release	= coda_release,
> +	.poll		= v4l2_m2m_fop_poll,
> +	.unlocked_ioctl	= video_ioctl2,
> +	.mmap		= v4l2_m2m_fop_mmap,
> +};
> +
> +static const struct v4l2_file_operations coda_decoder_fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= coda_decoder_open,
>  	.release	= coda_release,
>  	.poll		= v4l2_m2m_fop_poll,
>  	.unlocked_ioctl	= video_ioctl2,
> @@ -3570,6 +3625,17 @@ err_clk_per:
>  	return ret;
>  }
>  
> +static int coda_register_device(struct coda_dev *dev, struct video_device *vfd)
> +{
> +	vfd->release	= video_device_release_empty,
> +	vfd->lock	= &dev->dev_mutex;
> +	vfd->v4l2_dev	= &dev->v4l2_dev;
> +	vfd->vfl_dir	= VFL_DIR_M2M;
> +	video_set_drvdata(vfd, dev);
> +
> +	return video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> +}
> +
>  static void coda_fw_callback(const struct firmware *fw, void *context)
>  {
>  	struct coda_dev *dev = context;
> @@ -3626,15 +3692,6 @@ static void coda_fw_callback(const struct firmware *fw, void *context)
>  			return;
>  	}
>  
> -	dev->vfd.fops	= &coda_fops,
> -	dev->vfd.ioctl_ops	= &coda_ioctl_ops;
> -	dev->vfd.release	= video_device_release_empty,
> -	dev->vfd.lock	= &dev->dev_mutex;
> -	dev->vfd.v4l2_dev	= &dev->v4l2_dev;
> -	dev->vfd.vfl_dir	= VFL_DIR_M2M;
> -	snprintf(dev->vfd.name, sizeof(dev->vfd.name), "%s", CODA_NAME);
> -	video_set_drvdata(&dev->vfd, dev);
> -
>  	dev->alloc_ctx = vb2_dma_contig_init_ctx(&pdev->dev);
>  	if (IS_ERR(dev->alloc_ctx)) {
>  		v4l2_err(&dev->v4l2_dev, "Failed to alloc vb2 context\n");
> @@ -3647,13 +3704,28 @@ static void coda_fw_callback(const struct firmware *fw, void *context)
>  		goto rel_ctx;
>  	}
>  
> -	ret = video_register_device(&dev->vfd, VFL_TYPE_GRABBER, 0);
> +	dev->vfd[0].fops      = &coda_encoder_fops,
> +	dev->vfd[0].ioctl_ops = &coda_ioctl_ops;
> +	snprintf(dev->vfd[0].name, sizeof(dev->vfd[0].name), "coda-encoder");
> +	ret = coda_register_device(dev, &dev->vfd[0]);
>  	if (ret) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
> +		v4l2_err(&dev->v4l2_dev,
> +			 "Failed to register encoder video device\n");
>  		goto rel_m2m;
>  	}
> -	v4l2_info(&dev->v4l2_dev, "codec registered as /dev/video%d\n",
> -		  dev->vfd.num);
> +
> +	dev->vfd[1].fops      = &coda_decoder_fops,
> +	dev->vfd[1].ioctl_ops = &coda_ioctl_ops;
> +	snprintf(dev->vfd[1].name, sizeof(dev->vfd[1].name), "coda-decoder");
> +	ret = coda_register_device(dev, &dev->vfd[1]);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev,
> +			 "Failed to register decoder video device\n");
> +		goto rel_m2m;
> +	}
> +
> +	v4l2_info(&dev->v4l2_dev, "codec registered as /dev/video[%d-%d]\n",
> +		  dev->vfd[0].num, dev->vfd[1].num);
>  
>  	return;
>  
> @@ -3887,7 +3959,8 @@ static int coda_remove(struct platform_device *pdev)
>  {
>  	struct coda_dev *dev = platform_get_drvdata(pdev);
>  
> -	video_unregister_device(&dev->vfd);
> +	video_unregister_device(&dev->vfd[0]);
> +	video_unregister_device(&dev->vfd[1]);
>  	if (dev->m2m_dev)
>  		v4l2_m2m_release(dev->m2m_dev);
>  	pm_runtime_disable(&pdev->dev);
> 

Re: [PATCH 09/11] [media] coda: split format enumeration for encoder end decoder device

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> Let the decoder capture side and encoder output side only list
> uncompressed formats, and the decoder output and encoder capture
> side only list compressed formats.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

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

Regards,

	Hans

> ---
>  drivers/media/platform/coda.c | 57 +++++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index 4a159031..e63226b 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -542,8 +542,8 @@ static int coda_querycap(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
> -			enum v4l2_buf_type type, int src_fourcc)
> +static int coda_enum_fmt(struct file *file, void *priv,
> +			 struct v4l2_fmtdesc *f)
>  {
>  	struct coda_ctx *ctx = fh_to_ctx(priv);
>  	struct coda_codec *codecs = ctx->dev->devtype->codecs;
> @@ -552,11 +552,19 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
>  	int num_codecs = ctx->dev->devtype->num_codecs;
>  	int num_formats = ARRAY_SIZE(coda_formats);
>  	int i, k, num = 0;
> +	bool yuv;
> +
> +	if (ctx->inst_type == CODA_INST_ENCODER)
> +		yuv = (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT);
> +	else
> +		yuv = (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE);
>  
>  	for (i = 0; i < num_formats; i++) {
> -		/* Both uncompressed formats are always supported */
> -		if (coda_format_is_yuv(formats[i].fourcc) &&
> -		    !coda_format_is_yuv(src_fourcc)) {
> +		/* Skip either raw or compressed formats */
> +		if (yuv != coda_format_is_yuv(formats[i].fourcc))
> +			continue;
> +		/* All uncompressed formats are always supported */
> +		if (yuv) {
>  			if (num == f->index)
>  				break;
>  			++num;
> @@ -564,12 +572,10 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
>  		}
>  		/* Compressed formats may be supported, check the codec list */
>  		for (k = 0; k < num_codecs; k++) {
> -			/* if src_fourcc is set, only consider matching codecs */
> -			if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> -			    formats[i].fourcc == codecs[k].dst_fourcc &&
> -			    (!src_fourcc || src_fourcc == codecs[k].src_fourcc))
> +			if (f->type == V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> +			    formats[i].fourcc == codecs[k].dst_fourcc)
>  				break;
> -			if (type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
> +			if (f->type == V4L2_BUF_TYPE_VIDEO_OUTPUT &&
>  			    formats[i].fourcc == codecs[k].src_fourcc)
>  				break;
>  		}
> @@ -584,7 +590,7 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
>  		fmt = &formats[i];
>  		strlcpy(f->description, fmt->name, sizeof(f->description));
>  		f->pixelformat = fmt->fourcc;
> -		if (!coda_format_is_yuv(fmt->fourcc))
> +		if (!yuv)
>  			f->flags |= V4L2_FMT_FLAG_COMPRESSED;
>  		return 0;
>  	}
> @@ -593,31 +599,6 @@ static int enum_fmt(void *priv, struct v4l2_fmtdesc *f,
>  	return -EINVAL;
>  }
>  
> -static int coda_enum_fmt_vid_cap(struct file *file, void *priv,
> -				 struct v4l2_fmtdesc *f)
> -{
> -	struct coda_ctx *ctx = fh_to_ctx(priv);
> -	struct vb2_queue *src_vq;
> -	struct coda_q_data *q_data_src;
> -
> -	/* If the source format is already fixed, only list matching formats */
> -	src_vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> -	if (vb2_is_streaming(src_vq)) {
> -		q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> -
> -		return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_CAPTURE,
> -				q_data_src->fourcc);
> -	}
> -
> -	return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_CAPTURE, 0);
> -}
> -
> -static int coda_enum_fmt_vid_out(struct file *file, void *priv,
> -				 struct v4l2_fmtdesc *f)
> -{
> -	return enum_fmt(priv, f, V4L2_BUF_TYPE_VIDEO_OUTPUT, 0);
> -}
> -
>  static int coda_g_fmt(struct file *file, void *priv,
>  		      struct v4l2_format *f)
>  {
> @@ -972,12 +953,12 @@ static int coda_subscribe_event(struct v4l2_fh *fh,
>  static const struct v4l2_ioctl_ops coda_ioctl_ops = {
>  	.vidioc_querycap	= coda_querycap,
>  
> -	.vidioc_enum_fmt_vid_cap = coda_enum_fmt_vid_cap,
> +	.vidioc_enum_fmt_vid_cap = coda_enum_fmt,
>  	.vidioc_g_fmt_vid_cap	= coda_g_fmt,
>  	.vidioc_try_fmt_vid_cap	= coda_try_fmt_vid_cap,
>  	.vidioc_s_fmt_vid_cap	= coda_s_fmt_vid_cap,
>  
> -	.vidioc_enum_fmt_vid_out = coda_enum_fmt_vid_out,
> +	.vidioc_enum_fmt_vid_out = coda_enum_fmt,
>  	.vidioc_g_fmt_vid_out	= coda_g_fmt,
>  	.vidioc_try_fmt_vid_out	= coda_try_fmt_vid_out,
>  	.vidioc_s_fmt_vid_out	= coda_s_fmt_vid_out,
> 

Re: [PATCH 11/11] [media] coda: mark constant structures as such

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> The format and codec lists and the ops structures are read-only.
> Mark them as const.
> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>

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

Regards,

	Hans

> ---
>  drivers/media/platform/coda.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index ed5fa4c..b644f2b 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -102,7 +102,7 @@ struct coda_codec {
>  struct coda_devtype {
>  	char			*firmware;
>  	enum coda_product	product;
> -	struct coda_codec	*codecs;
> +	const struct coda_codec	*codecs;
>  	unsigned int		num_codecs;
>  	size_t			workbuf_size;
>  	size_t			tempbuf_size;
> @@ -225,7 +225,7 @@ struct coda_ctx {
>  	u32				sequence_offset;
>  	struct coda_q_data		q_data[2];
>  	enum coda_inst_type		inst_type;
> -	struct coda_codec		*codec;
> +	const struct coda_codec		*codec;
>  	enum v4l2_colorspace		colorspace;
>  	struct coda_params		params;
>  	struct v4l2_ctrl_handler	ctrls;
> @@ -390,7 +390,7 @@ static struct coda_q_data *get_q_data(struct coda_ctx *ctx,
>  /*
>   * Array of all formats supported by any version of Coda:
>   */
> -static struct coda_fmt coda_formats[] = {
> +static const struct coda_fmt coda_formats[] = {
>  	{
>  		.name = "YUV 4:2:0 Planar, YCbCr",
>  		.fourcc = V4L2_PIX_FMT_YUV420,
> @@ -419,19 +419,19 @@ static struct coda_fmt coda_formats[] = {
>   *  i.MX6  -> coda960
>   * Use V4L2_PIX_FMT_YUV420 as placeholder for all supported YUV 4:2:0 variants
>   */
> -static struct coda_codec codadx6_codecs[] = {
> +static const struct coda_codec codadx6_codecs[] = {
>  	CODA_CODEC(CODADX6_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_H264,  720, 576),
>  	CODA_CODEC(CODADX6_MODE_ENCODE_MP4,  V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_MPEG4, 720, 576),
>  };
>  
> -static struct coda_codec coda7_codecs[] = {
> +static const struct coda_codec coda7_codecs[] = {
>  	CODA_CODEC(CODA7_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_H264,   1280, 720),
>  	CODA_CODEC(CODA7_MODE_ENCODE_MP4,  V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_MPEG4,  1280, 720),
>  	CODA_CODEC(CODA7_MODE_DECODE_H264, V4L2_PIX_FMT_H264,   V4L2_PIX_FMT_YUV420, 1920, 1080),
>  	CODA_CODEC(CODA7_MODE_DECODE_MP4,  V4L2_PIX_FMT_MPEG4,  V4L2_PIX_FMT_YUV420, 1920, 1080),
>  };
>  
> -static struct coda_codec coda9_codecs[] = {
> +static const struct coda_codec coda9_codecs[] = {
>  	CODA_CODEC(CODA9_MODE_ENCODE_H264, V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_H264,   1920, 1080),
>  	CODA_CODEC(CODA9_MODE_ENCODE_MP4,  V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_MPEG4,  1920, 1080),
>  	CODA_CODEC(CODA9_MODE_DECODE_H264, V4L2_PIX_FMT_H264,   V4L2_PIX_FMT_YUV420, 1920, 1080),
> @@ -458,10 +458,10 @@ static u32 coda_format_normalize_yuv(u32 fourcc)
>  	return coda_format_is_yuv(fourcc) ? V4L2_PIX_FMT_YUV420 : fourcc;
>  }
>  
> -static struct coda_codec *coda_find_codec(struct coda_dev *dev, int src_fourcc,
> -					  int dst_fourcc)
> +static const struct coda_codec *coda_find_codec(struct coda_dev *dev,
> +						int src_fourcc, int dst_fourcc)
>  {
> -	struct coda_codec *codecs = dev->devtype->codecs;
> +	const struct coda_codec *codecs = dev->devtype->codecs;
>  	int num_codecs = dev->devtype->num_codecs;
>  	int k;
>  
> @@ -483,10 +483,10 @@ static struct coda_codec *coda_find_codec(struct coda_dev *dev, int src_fourcc,
>  }
>  
>  static void coda_get_max_dimensions(struct coda_dev *dev,
> -				    struct coda_codec *codec,
> +				    const struct coda_codec *codec,
>  				    int *max_w, int *max_h)
>  {
> -	struct coda_codec *codecs = dev->devtype->codecs;
> +	const struct coda_codec *codecs = dev->devtype->codecs;
>  	int num_codecs = dev->devtype->num_codecs;
>  	unsigned int w, h;
>  	int k;
> @@ -546,9 +546,9 @@ static int coda_enum_fmt(struct file *file, void *priv,
>  			 struct v4l2_fmtdesc *f)
>  {
>  	struct coda_ctx *ctx = fh_to_ctx(priv);
> -	struct coda_codec *codecs = ctx->dev->devtype->codecs;
> -	struct coda_fmt *formats = coda_formats;
> -	struct coda_fmt *fmt;
> +	const struct coda_codec *codecs = ctx->dev->devtype->codecs;
> +	const struct coda_fmt *formats = coda_formats;
> +	const struct coda_fmt *fmt;
>  	int num_codecs = ctx->dev->devtype->num_codecs;
>  	int num_formats = ARRAY_SIZE(coda_formats);
>  	int i, k, num = 0;
> @@ -621,7 +621,7 @@ static int coda_g_fmt(struct file *file, void *priv,
>  	return 0;
>  }
>  
> -static int coda_try_fmt(struct coda_ctx *ctx, struct coda_codec *codec,
> +static int coda_try_fmt(struct coda_ctx *ctx, const struct coda_codec *codec,
>  			struct v4l2_format *f)
>  {
>  	struct coda_dev *dev = ctx->dev;
> @@ -685,7 +685,7 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
>  				struct v4l2_format *f)
>  {
>  	struct coda_ctx *ctx = fh_to_ctx(priv);
> -	struct coda_codec *codec = NULL;
> +	const struct coda_codec *codec = NULL;
>  	struct vb2_queue *src_vq;
>  	int ret;
>  
> @@ -733,7 +733,7 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv,
>  				struct v4l2_format *f)
>  {
>  	struct coda_ctx *ctx = fh_to_ctx(priv);
> -	struct coda_codec *codec;
> +	const struct coda_codec *codec;
>  
>  	/* Determine codec by encoded format, returns NULL if raw or invalid */
>  	codec = coda_find_codec(ctx->dev, f->fmt.pix.pixelformat,
> @@ -1531,7 +1531,7 @@ static void coda_unlock(void *m2m_priv)
>  	mutex_unlock(&pcdev->dev_mutex);
>  }
>  
> -static struct v4l2_m2m_ops coda_m2m_ops = {
> +static const struct v4l2_m2m_ops coda_m2m_ops = {
>  	.device_run	= coda_device_run,
>  	.job_ready	= coda_job_ready,
>  	.job_abort	= coda_job_abort,
> @@ -2805,7 +2805,7 @@ static int coda_s_ctrl(struct v4l2_ctrl *ctrl)
>  	return 0;
>  }
>  
> -static struct v4l2_ctrl_ops coda_ctrl_ops = {
> +static const struct v4l2_ctrl_ops coda_ctrl_ops = {
>  	.s_ctrl = coda_s_ctrl,
>  };
>  
> 

Re: [PATCH 10/11] [media] coda: default to h.264 decoder on invalid formats

[Hans Verkuil]

On 07/17/2014 06:05 PM, Philipp Zabel wrote:
> If the user provides an invalid format, let the decoder device
> default to h.264.

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

Regards,

	Hans

> 
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>  drivers/media/platform/coda.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/coda.c b/drivers/media/platform/coda.c
> index e63226b..ed5fa4c 100644
> --- a/drivers/media/platform/coda.c
> +++ b/drivers/media/platform/coda.c
> @@ -685,7 +685,7 @@ static int coda_try_fmt_vid_cap(struct file *file, void *priv,
>  				struct v4l2_format *f)
>  {
>  	struct coda_ctx *ctx = fh_to_ctx(priv);
> -	struct coda_codec *codec;
> +	struct coda_codec *codec = NULL;
>  	struct vb2_queue *src_vq;
>  	int ret;
>  
> @@ -738,6 +738,12 @@ static int coda_try_fmt_vid_out(struct file *file, void *priv,
>  	/* Determine codec by encoded format, returns NULL if raw or invalid */
>  	codec = coda_find_codec(ctx->dev, f->fmt.pix.pixelformat,
>  				V4L2_PIX_FMT_YUV420);
> +	if (!codec && ctx->inst_type == CODA_INST_DECODER) {
> +		codec = coda_find_codec(ctx->dev, V4L2_PIX_FMT_H264,
> +					V4L2_PIX_FMT_YUV420);
> +		if (!codec)
> +			return -EINVAL;
> +	}
>  
>  	if (!f->fmt.pix.colorspace)
>  		f->fmt.pix.colorspace = V4L2_COLORSPACE_REC709;
> 

Re: [PATCH 06/11] [media] coda: delay coda_fill_bitstream()

[Philipp Zabel]

Hi Hans,

thank you for the review.

Am Donnerstag, den 17.07.2014, 18:17 +0200 schrieb Hans Verkuil:
> > @@ -2272,6 +2273,15 @@ static int coda_start_streaming(struct vb2_queue *q, unsigned int count)
> >  	q_data_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> >  	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> >  		if (q_data_src->fourcc == V4L2_PIX_FMT_H264) {
> > +			struct vb2_queue *vq;
> > +			/* start_streaming_called must be set, for v4l2_m2m_buf_done() */
> > +			vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
> > +			vq->start_streaming_called = 1;
> 
> Why set start_streaming_called to 1? It is set before calling start_streaming.
> This is a recent change in videobuf2-core.c though.

Because I hadn't seen "[media] v4l: vb2: Fix stream start and buffer
completion race" yet. I'll update this patch.

> BTW, you should test with CONFIG_VIDEO_ADV_DEBUG on and force start_streaming
> errors to check whether vb2_buffer_done(buf, VB2_BUF_STATE_DEQUEUED) is called
> for the queued buffers in case of start_streaming failure.
> 
> With that config option vb2 will complain about unbalanced vb2 operations.
>
> I strongly suspect this code does not play well with this.

Yes, I will fix this.

> BTW, isn't it time to split up the coda driver? Over 3000 lines...

Indeed. This is also on my list.

regards
Philipp