[PATCH v3 0/8] Add support for MFC v7 firmware

[PATCH v3 0/8] Add support for MFC v7 firmware

[Arun Kumar K]

This patch series adds MFC v7 firmware support to the Exynos
MFC driver. MFC v7 is present in 5420 SoC which has support
for VP8 encoding and many other features.

Changes from v2:
- Addressed review comments from Sylwester
http://www.mail-archive.com/linux-media@vger.kernel.org/msg63676.html
http://www.mail-archive.com/linux-media@vger.kernel.org/msg63677.html

Changes from v1:
- Addressed review comments from Hans and Sylwester
http://www.mail-archive.com/linux-media@vger.kernel.org/msg63148.html
http://www.mail-archive.com/linux-media@vger.kernel.org/msg63311.html
- Modified IS_MFCV6 macro to IS_MFCV6_PLUS to include v7 also

Arun Kumar K (7):
  [media] s5p-mfc: Update v6 encoder buffer sizes
  [media] s5p-mfc: Rename IS_MFCV6 macro
  [media] s5p-mfc: Add register definition file for MFC v7
  [media] s5p-mfc: Core support for MFC v7
  [media] s5p-mfc: Update driver for v7 firmware
  [media] V4L: Add VP8 encoder controls
  [media] s5p-mfc: Add support for VP8 encoder

Sylwester Nawrocki (1):
  [media] V4L: Add support for integer menu controls with standard menu
    items

 Documentation/DocBook/media/v4l/controls.xml       |  150 ++++++++++++++++++++
 .../devicetree/bindings/media/s5p-mfc.txt          |    1 +
 Documentation/video4linux/v4l2-controls.txt        |   21 +--
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h       |    4 +-
 drivers/media/platform/s5p-mfc/regs-mfc-v7.h       |   58 ++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |   32 +++++
 drivers/media/platform/s5p-mfc/s5p_mfc_cmd.c       |    2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c    |    3 +
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h    |   23 ++-
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c      |   12 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c       |   18 +--
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c       |  117 +++++++++++++--
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.c       |    2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c    |  143 +++++++++++++++++--
 drivers/media/v4l2-core/v4l2-ctrls.c               |   61 +++++++-
 include/uapi/linux/v4l2-controls.h                 |   29 +++-
 16 files changed, 622 insertions(+), 54 deletions(-)
 create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v7.h

-- 
1.7.9.5

[PATCH v3 1/8] [media] s5p-mfc: Update v6 encoder buffer sizes

[Arun Kumar K]

The patch updates few encoder buffer sizes for MFC v6.5
as per the udpdated user manual. The same buffer sizes
holds good for v7 firmware also.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
---
 drivers/media/platform/s5p-mfc/regs-mfc-v6.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
index 363a97c..2398cdf 100644
--- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h
@@ -374,9 +374,9 @@
 #define S5P_FIMV_NUM_PIXELS_IN_MB_COL_V6	16
 
 /* Buffer size requirements defined by hardware */
-#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h)	(((w) + 1) * ((h) + 1) * 8)
+#define S5P_FIMV_TMV_BUFFER_SIZE_V6(w, h)	(((w) + 1) * ((h) + 3) * 8)
 #define S5P_FIMV_ME_BUFFER_SIZE_V6(imw, imh, mbw, mbh) \
-	((DIV_ROUND_UP(imw, 64) *  DIV_ROUND_UP(imh, 64) * 256) + \
+	(((((imw + 127) / 64) * 16) *  DIV_ROUND_UP(imh, 64) * 256) + \
 	 (DIV_ROUND_UP((mbw) * (mbh), 32) * 16))
 #define S5P_FIMV_SCRATCH_BUF_SIZE_H264_DEC_V6(w, h)	(((w) * 192) + 64)
 #define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(w, h) \
-- 
1.7.9.5

[PATCH v3 2/8] [media] s5p-mfc: Rename IS_MFCV6 macro

[Arun Kumar K]

The MFC v6 specific code holds good for MFC v7 also as
the v7 version is a superset of v6 and the HW interface
remains more or less similar. This patch renames the macro
IS_MFCV6() to IS_MFCV6_PLUS() so that it can be used
for v7 also.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_cmd.c    |    2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |    2 +-
 drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c   |   12 ++++++------
 drivers/media/platform/s5p-mfc/s5p_mfc_dec.c    |   18 ++++++++++--------
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   16 +++++++++-------
 drivers/media/platform/s5p-mfc/s5p_mfc_opr.c    |    2 +-
 6 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd.c
index f0a41c9..242c033 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd.c
@@ -20,7 +20,7 @@ static struct s5p_mfc_hw_cmds *s5p_mfc_cmds;
 
 void s5p_mfc_init_hw_cmds(struct s5p_mfc_dev *dev)
 {
-	if (IS_MFCV6(dev))
+	if (IS_MFCV6_PLUS(dev))
 		s5p_mfc_cmds = s5p_mfc_init_hw_cmds_v6();
 	else
 		s5p_mfc_cmds = s5p_mfc_init_hw_cmds_v5();
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index ef4074c..d47016d 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -683,6 +683,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
 #define HAS_PORTNUM(dev)	(dev ? (dev->variant ? \
 				(dev->variant->port_num ? 1 : 0) : 0) : 0)
 #define IS_TWOPORT(dev)		(dev->variant->port_num == 2 ? 1 : 0)
-#define IS_MFCV6(dev)		(dev->variant->version >= 0x60 ? 1 : 0)
+#define IS_MFCV6_PLUS(dev)	(dev->variant->version >= 0x60 ? 1 : 0)
 
 #endif /* S5P_MFC_COMMON_H_ */
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
index dc1fc94..7cab684 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_ctrl.c
@@ -164,7 +164,7 @@ int s5p_mfc_reset(struct s5p_mfc_dev *dev)
 
 	mfc_debug_enter();
 
-	if (IS_MFCV6(dev)) {
+	if (IS_MFCV6_PLUS(dev)) {
 		/* Reset IP */
 		/*  except RISC, reset */
 		mfc_write(dev, 0xFEE, S5P_FIMV_MFC_RESET_V6);
@@ -213,7 +213,7 @@ int s5p_mfc_reset(struct s5p_mfc_dev *dev)
 
 static inline void s5p_mfc_init_memctrl(struct s5p_mfc_dev *dev)
 {
-	if (IS_MFCV6(dev)) {
+	if (IS_MFCV6_PLUS(dev)) {
 		mfc_write(dev, dev->bank1, S5P_FIMV_RISC_BASE_ADDRESS_V6);
 		mfc_debug(2, "Base Address : %08x\n", dev->bank1);
 	} else {
@@ -226,7 +226,7 @@ static inline void s5p_mfc_init_memctrl(struct s5p_mfc_dev *dev)
 
 static inline void s5p_mfc_clear_cmds(struct s5p_mfc_dev *dev)
 {
-	if (IS_MFCV6(dev)) {
+	if (IS_MFCV6_PLUS(dev)) {
 		/* Zero initialization should be done before RESET.
 		 * Nothing to do here. */
 	} else {
@@ -264,7 +264,7 @@ int s5p_mfc_init_hw(struct s5p_mfc_dev *dev)
 	s5p_mfc_clear_cmds(dev);
 	/* 3. Release reset signal to the RISC */
 	s5p_mfc_clean_dev_int_flags(dev);
-	if (IS_MFCV6(dev))
+	if (IS_MFCV6_PLUS(dev))
 		mfc_write(dev, 0x1, S5P_FIMV_RISC_ON_V6);
 	else
 		mfc_write(dev, 0x3ff, S5P_FIMV_SW_RESET);
@@ -301,7 +301,7 @@ int s5p_mfc_init_hw(struct s5p_mfc_dev *dev)
 		s5p_mfc_clock_off();
 		return -EIO;
 	}
-	if (IS_MFCV6(dev))
+	if (IS_MFCV6_PLUS(dev))
 		ver = mfc_read(dev, S5P_FIMV_FW_VERSION_V6);
 	else
 		ver = mfc_read(dev, S5P_FIMV_FW_VERSION);
@@ -380,7 +380,7 @@ int s5p_mfc_wakeup(struct s5p_mfc_dev *dev)
 		return ret;
 	}
 	/* 4. Release reset signal to the RISC */
-	if (IS_MFCV6(dev))
+	if (IS_MFCV6_PLUS(dev))
 		mfc_write(dev, 0x1, S5P_FIMV_RISC_ON_V6);
 	else
 		mfc_write(dev, 0x3ff, S5P_FIMV_SW_RESET);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
index 00b0703..56a1d3b 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_dec.c
@@ -382,7 +382,7 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
 			mfc_err("Unsupported format for source.\n");
 			return -EINVAL;
 		}
-		if (!IS_MFCV6(dev) && (fmt->fourcc == V4L2_PIX_FMT_VP8)) {
+		if (!IS_MFCV6_PLUS(dev) && (fmt->fourcc == V4L2_PIX_FMT_VP8)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
 		}
@@ -392,10 +392,11 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f)
 			mfc_err("Unsupported format for destination.\n");
 			return -EINVAL;
 		}
-		if (IS_MFCV6(dev) && (fmt->fourcc == V4L2_PIX_FMT_NV12MT)) {
+		if (IS_MFCV6_PLUS(dev) &&
+				(fmt->fourcc == V4L2_PIX_FMT_NV12MT)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
-		} else if (!IS_MFCV6(dev) &&
+		} else if (!IS_MFCV6_PLUS(dev) &&
 				(fmt->fourcc != V4L2_PIX_FMT_NV12MT)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
@@ -430,10 +431,11 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
 			mfc_err("Unsupported format for source.\n");
 			return -EINVAL;
 		}
-		if (!IS_MFCV6(dev) && (fmt->fourcc != V4L2_PIX_FMT_NV12MT)) {
+		if (!IS_MFCV6_PLUS(dev) &&
+				(fmt->fourcc != V4L2_PIX_FMT_NV12MT)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
-		} else if (IS_MFCV6(dev) &&
+		} else if (IS_MFCV6_PLUS(dev) &&
 				(fmt->fourcc == V4L2_PIX_FMT_NV12MT)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
@@ -457,7 +459,7 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
 		ret = -EINVAL;
 		goto out;
 	}
-	if (!IS_MFCV6(dev) && (fmt->fourcc == V4L2_PIX_FMT_VP8)) {
+	if (!IS_MFCV6_PLUS(dev) && (fmt->fourcc == V4L2_PIX_FMT_VP8)) {
 		mfc_err("Not supported format.\n");
 		return -EINVAL;
 	}
@@ -942,7 +944,7 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 		psize[0] = ctx->luma_size;
 		psize[1] = ctx->chroma_size;
 
-		if (IS_MFCV6(dev))
+		if (IS_MFCV6_PLUS(dev))
 			allocators[0] =
 				ctx->dev->alloc_ctx[MFC_BANK1_ALLOC_CTX];
 		else
@@ -1067,7 +1069,7 @@ static int s5p_mfc_stop_streaming(struct vb2_queue *q)
 		ctx->dpb_flush_flag = 1;
 		ctx->dec_dst_flag = 0;
 		spin_unlock_irqrestore(&dev->irqlock, flags);
-		if (IS_MFCV6(dev) && (ctx->state == MFCINST_RUNNING)) {
+		if (IS_MFCV6_PLUS(dev) && (ctx->state == MFCINST_RUNNING)) {
 			ctx->state = MFCINST_FLUSH;
 			set_work_bit_irqsave(ctx);
 			s5p_mfc_clean_ctx_int_flags(ctx);
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index 2549967..f734ccc 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -663,7 +663,7 @@ static int enc_post_seq_start(struct s5p_mfc_ctx *ctx)
 		spin_unlock_irqrestore(&dev->irqlock, flags);
 	}
 
-	if (!IS_MFCV6(dev)) {
+	if (!IS_MFCV6_PLUS(dev)) {
 		ctx->state = MFCINST_RUNNING;
 		if (s5p_mfc_ctx_ready(ctx))
 			set_work_bit_irqsave(ctx);
@@ -993,11 +993,11 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
 			return -EINVAL;
 		}
 
-		if (!IS_MFCV6(dev) &&
+		if (!IS_MFCV6_PLUS(dev) &&
 				(fmt->fourcc == V4L2_PIX_FMT_NV12MT_16X16)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
-		} else if (IS_MFCV6(dev) &&
+		} else if (IS_MFCV6_PLUS(dev) &&
 				(fmt->fourcc == V4L2_PIX_FMT_NV12MT)) {
 			mfc_err("Not supported format.\n");
 			return -EINVAL;
@@ -1072,7 +1072,7 @@ static int vidioc_reqbufs(struct file *file, void *priv,
 			return -EINVAL;
 		}
 
-		if (IS_MFCV6(dev)) {
+		if (IS_MFCV6_PLUS(dev)) {
 			/* Check for min encoder buffers */
 			if (ctx->pb_count &&
 				(reqbufs->count < ctx->pb_count)) {
@@ -1353,7 +1353,7 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
 				S5P_FIMV_ENC_PROFILE_H264_BASELINE;
 			break;
 		case V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE:
-			if (IS_MFCV6(dev))
+			if (IS_MFCV6_PLUS(dev))
 				p->codec.h264.profile =
 				S5P_FIMV_ENC_PROFILE_H264_CONSTRAINED_BASELINE;
 			else
@@ -1662,9 +1662,10 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 			*buf_count = 1;
 		if (*buf_count > MFC_MAX_BUFFERS)
 			*buf_count = MFC_MAX_BUFFERS;
+
 		psize[0] = ctx->luma_size;
 		psize[1] = ctx->chroma_size;
-		if (IS_MFCV6(dev)) {
+		if (IS_MFCV6_PLUS(dev)) {
 			allocators[0] =
 				ctx->dev->alloc_ctx[MFC_BANK1_ALLOC_CTX];
 			allocators[1] =
@@ -1773,7 +1774,8 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, unsigned int count)
 	struct s5p_mfc_ctx *ctx = fh_to_ctx(q->drv_priv);
 	struct s5p_mfc_dev *dev = ctx->dev;
 
-	if (IS_MFCV6(dev) && (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)) {
+	if (IS_MFCV6_PLUS(dev) &&
+			(q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)) {
 
 		if ((ctx->state == MFCINST_GOT_INST) &&
 			(dev->curr_ctx == ctx->num) && dev->hw_lock) {
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
index 10f8ac3..3c01c33 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr.c
@@ -21,7 +21,7 @@ static struct s5p_mfc_hw_ops *s5p_mfc_ops;
 
 void s5p_mfc_init_hw_ops(struct s5p_mfc_dev *dev)
 {
-	if (IS_MFCV6(dev)) {
+	if (IS_MFCV6_PLUS(dev)) {
 		s5p_mfc_ops = s5p_mfc_init_hw_ops_v6();
 		dev->warn_start = S5P_FIMV_ERR_WARNINGS_START_V6;
 	} else {
-- 
1.7.9.5

[PATCH v3 3/8] [media] s5p-mfc: Add register definition file for MFC v7

[Arun Kumar K]

The patch adds the register definition file for new firmware
version v7 for MFC. New firmware supports VP8 encoding along with
many other features.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/media/platform/s5p-mfc/regs-mfc-v7.h |   58 ++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 drivers/media/platform/s5p-mfc/regs-mfc-v7.h

diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h
new file mode 100644
index 0000000..24dba69
--- /dev/null
+++ b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h
@@ -0,0 +1,58 @@
+/*
+ * Register definition file for Samsung MFC V7.x Interface (FIMV) driver
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *		http://www.samsung.com/
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _REGS_MFC_V7_H
+#define _REGS_MFC_V7_H
+
+#include "regs-mfc-v6.h"
+
+/* Additional features of v7 */
+#define S5P_FIMV_CODEC_VP8_ENC_V7	25
+
+/* Additional registers for v7 */
+#define S5P_FIMV_D_INIT_BUFFER_OPTIONS_V7		0xf47c
+
+#define S5P_FIMV_E_SOURCE_FIRST_ADDR_V7			0xf9e0
+#define S5P_FIMV_E_SOURCE_SECOND_ADDR_V7		0xf9e4
+#define S5P_FIMV_E_SOURCE_THIRD_ADDR_V7			0xf9e8
+#define S5P_FIMV_E_SOURCE_FIRST_STRIDE_V7		0xf9ec
+#define S5P_FIMV_E_SOURCE_SECOND_STRIDE_V7		0xf9f0
+#define S5P_FIMV_E_SOURCE_THIRD_STRIDE_V7		0xf9f4
+
+#define S5P_FIMV_E_ENCODED_SOURCE_FIRST_ADDR_V7		0xfa70
+#define S5P_FIMV_E_ENCODED_SOURCE_SECOND_ADDR_V7	0xfa74
+
+#define S5P_FIMV_E_VP8_OPTIONS_V7			0xfdb0
+#define S5P_FIMV_E_VP8_FILTER_OPTIONS_V7		0xfdb4
+#define S5P_FIMV_E_VP8_GOLDEN_FRAME_OPTION_V7		0xfdb8
+#define S5P_FIMV_E_VP8_NUM_T_LAYER_V7			0xfdc4
+
+/* MFCv7 variant defines */
+#define MAX_FW_SIZE_V7			(SZ_1M)		/* 1MB */
+#define MAX_CPB_SIZE_V7			(3 * SZ_1M)	/* 3MB */
+#define MFC_VERSION_V7			0x72
+#define MFC_NUM_PORTS_V7		1
+
+/* MFCv7 Context buffer sizes */
+#define MFC_CTX_BUF_SIZE_V7		(30 * SZ_1K)	/*  30KB */
+#define MFC_H264_DEC_CTX_BUF_SIZE_V7	(2 * SZ_1M)	/*  2MB */
+#define MFC_OTHER_DEC_CTX_BUF_SIZE_V7	(20 * SZ_1K)	/*  20KB */
+#define MFC_H264_ENC_CTX_BUF_SIZE_V7	(100 * SZ_1K)	/* 100KB */
+#define MFC_OTHER_ENC_CTX_BUF_SIZE_V7	(10 * SZ_1K)	/*  10KB */
+
+/* Buffer size defines */
+#define S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V7(w, h) \
+			(SZ_1M + ((w) * 144) + (8192 * (h)) + 49216)
+
+#define S5P_FIMV_SCRATCH_BUF_SIZE_VP8_ENC_V7(w, h) \
+			(((w) * 48) + (((w) + 1) / 2 * 128) + 144 + 8192)
+
+#endif /*_REGS_MFC_V7_H*/
-- 
1.7.9.5

[PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7

[Arun Kumar K]

Adds variant data and core support for the MFC v7 firmware

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 .../devicetree/bindings/media/s5p-mfc.txt          |    1 +
 drivers/media/platform/s5p-mfc/s5p_mfc.c           |   32 ++++++++++++++++++++
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h    |    2 ++
 3 files changed, 35 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt
index 67ec3d4..cb9c5bc 100644
--- a/Documentation/devicetree/bindings/media/s5p-mfc.txt
+++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt
@@ -10,6 +10,7 @@ Required properties:
   - compatible : value should be either one among the following
 	(a) "samsung,mfc-v5" for MFC v5 present in Exynos4 SoCs
 	(b) "samsung,mfc-v6" for MFC v6 present in Exynos5 SoCs
+	(b) "samsung,mfc-v7" for MFC v7 present in Exynos5420 SoC
 
   - reg : Physical base address of the IP registers and length of memory
 	  mapped region.
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc.c b/drivers/media/platform/s5p-mfc/s5p_mfc.c
index d12faa6..d6be52f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c
@@ -1391,6 +1391,32 @@ static struct s5p_mfc_variant mfc_drvdata_v6 = {
 	.fw_name        = "s5p-mfc-v6.fw",
 };
 
+struct s5p_mfc_buf_size_v6 mfc_buf_size_v7 = {
+	.dev_ctx	= MFC_CTX_BUF_SIZE_V7,
+	.h264_dec_ctx	= MFC_H264_DEC_CTX_BUF_SIZE_V7,
+	.other_dec_ctx	= MFC_OTHER_DEC_CTX_BUF_SIZE_V7,
+	.h264_enc_ctx	= MFC_H264_ENC_CTX_BUF_SIZE_V7,
+	.other_enc_ctx	= MFC_OTHER_ENC_CTX_BUF_SIZE_V7,
+};
+
+struct s5p_mfc_buf_size buf_size_v7 = {
+	.fw	= MAX_FW_SIZE_V7,
+	.cpb	= MAX_CPB_SIZE_V7,
+	.priv	= &mfc_buf_size_v7,
+};
+
+struct s5p_mfc_buf_align mfc_buf_align_v7 = {
+	.base = 0,
+};
+
+static struct s5p_mfc_variant mfc_drvdata_v7 = {
+	.version	= MFC_VERSION_V7,
+	.port_num	= MFC_NUM_PORTS_V7,
+	.buf_size	= &buf_size_v7,
+	.buf_align	= &mfc_buf_align_v7,
+	.fw_name        = "s5p-mfc-v7.fw",
+};
+
 static struct platform_device_id mfc_driver_ids[] = {
 	{
 		.name = "s5p-mfc",
@@ -1401,6 +1427,9 @@ static struct platform_device_id mfc_driver_ids[] = {
 	}, {
 		.name = "s5p-mfc-v6",
 		.driver_data = (unsigned long)&mfc_drvdata_v6,
+	}, {
+		.name = "s5p-mfc-v7",
+		.driver_data = (unsigned long)&mfc_drvdata_v7,
 	},
 	{},
 };
@@ -1413,6 +1442,9 @@ static const struct of_device_id exynos_mfc_match[] = {
 	}, {
 		.compatible = "samsung,mfc-v6",
 		.data = &mfc_drvdata_v6,
+	}, {
+		.compatible = "samsung,mfc-v7",
+		.data = &mfc_drvdata_v7,
 	},
 	{},
 };
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index d47016d..17545d7 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -24,6 +24,7 @@
 #include <media/videobuf2-core.h>
 #include "regs-mfc.h"
 #include "regs-mfc-v6.h"
+#include "regs-mfc-v7.h"
 
 /* Definitions related to MFC memory */
 
@@ -684,5 +685,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
 				(dev->variant->port_num ? 1 : 0) : 0) : 0)
 #define IS_TWOPORT(dev)		(dev->variant->port_num == 2 ? 1 : 0)
 #define IS_MFCV6_PLUS(dev)	(dev->variant->version >= 0x60 ? 1 : 0)
+#define IS_MFCV7(dev)		(dev->variant->version >= 0x70 ? 1 : 0)
 
 #endif /* S5P_MFC_COMMON_H_ */
-- 
1.7.9.5

[PATCH v3 5/8] [media] s5p-mfc: Update driver for v7 firmware

[Arun Kumar K]

Firmware version v7 is mostly similar to v6 in terms
of hardware specific controls and commands. So the hardware
specific opr_v6 and cmd_v6 are re-used for v7 also. This patch
updates the v6 files to handle v7 version also.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   11 ++++-
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   53 +++++++++++++++++++----
 2 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index f734ccc..dd57b06 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -1663,8 +1663,15 @@ static int s5p_mfc_queue_setup(struct vb2_queue *vq,
 		if (*buf_count > MFC_MAX_BUFFERS)
 			*buf_count = MFC_MAX_BUFFERS;
 
-		psize[0] = ctx->luma_size;
-		psize[1] = ctx->chroma_size;
+		if (IS_MFCV7(dev)) {
+			/* MFCv7 needs pad bytes for input YUV */
+			psize[0] = ctx->luma_size + 256;
+			psize[1] = ctx->chroma_size + 128;
+		} else {
+			psize[0] = ctx->luma_size;
+			psize[1] = ctx->chroma_size;
+		}
+
 		if (IS_MFCV6_PLUS(dev)) {
 			allocators[0] =
 				ctx->dev->alloc_ctx[MFC_BANK1_ALLOC_CTX];
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index 7d4c5e1..7145ae5 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -80,6 +80,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 		ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
 			ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width, mb_height),
 			S5P_FIMV_TMV_BUFFER_ALIGN_V6);
+
 		ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
 				S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
 				S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
@@ -112,10 +113,18 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 			(ctx->mv_count * ctx->mv_size);
 		break;
 	case S5P_MFC_CODEC_MPEG4_DEC:
-		ctx->scratch_buf_size =
-			S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(
-					mb_width,
-					mb_height);
+		if (IS_MFCV7(dev)) {
+			ctx->scratch_buf_size =
+				S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V7(
+						mb_width,
+						mb_height);
+		} else {
+			ctx->scratch_buf_size =
+				S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(
+						mb_width,
+						mb_height);
+		}
+
 		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size,
 				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
 		ctx->bank1.size = ctx->scratch_buf_size;
@@ -453,8 +462,13 @@ static void s5p_mfc_set_enc_frame_buffer_v6(struct s5p_mfc_ctx *ctx,
 {
 	struct s5p_mfc_dev *dev = ctx->dev;
 
-	WRITEL(y_addr, S5P_FIMV_E_SOURCE_LUMA_ADDR_V6); /* 256B align */
-	WRITEL(c_addr, S5P_FIMV_E_SOURCE_CHROMA_ADDR_V6);
+	if (IS_MFCV7(dev)) {
+		WRITEL(y_addr, S5P_FIMV_E_SOURCE_FIRST_ADDR_V7);
+		WRITEL(c_addr, S5P_FIMV_E_SOURCE_SECOND_ADDR_V7);
+	} else {
+		WRITEL(y_addr, S5P_FIMV_E_SOURCE_LUMA_ADDR_V6);
+		WRITEL(c_addr, S5P_FIMV_E_SOURCE_CHROMA_ADDR_V6);
+	}
 
 	mfc_debug(2, "enc src y buf addr: 0x%08lx\n", y_addr);
 	mfc_debug(2, "enc src c buf addr: 0x%08lx\n", c_addr);
@@ -466,8 +480,13 @@ static void s5p_mfc_get_enc_frame_buffer_v6(struct s5p_mfc_ctx *ctx,
 	struct s5p_mfc_dev *dev = ctx->dev;
 	unsigned long enc_recon_y_addr, enc_recon_c_addr;
 
-	*y_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_LUMA_ADDR_V6);
-	*c_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_CHROMA_ADDR_V6);
+	if (IS_MFCV7(dev)) {
+		*y_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_FIRST_ADDR_V7);
+		*c_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_SECOND_ADDR_V7);
+	} else {
+		*y_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_LUMA_ADDR_V6);
+		*c_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_CHROMA_ADDR_V6);
+	}
 
 	enc_recon_y_addr = READL(S5P_FIMV_E_RECON_LUMA_DPB_ADDR_V6);
 	enc_recon_c_addr = READL(S5P_FIMV_E_RECON_CHROMA_DPB_ADDR_V6);
@@ -1166,6 +1185,12 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx)
 		reg |= (0x1 << S5P_FIMV_D_OPT_DDELAY_EN_SHIFT_V6);
 		WRITEL(ctx->display_delay, S5P_FIMV_D_DISPLAY_DELAY_V6);
 	}
+
+	if (IS_MFCV7(dev)) {
+		WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6);
+		reg = 0;
+	}
+
 	/* Setup loop filter, for decoding this is only valid for MPEG4 */
 	if (ctx->codec_mode == S5P_MFC_CODEC_MPEG4_DEC) {
 		mfc_debug(2, "Set loop filter to: %d\n",
@@ -1176,7 +1201,10 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx)
 	if (ctx->dst_fmt->fourcc == V4L2_PIX_FMT_NV12MT_16X16)
 		reg |= (0x1 << S5P_FIMV_D_OPT_TILE_MODE_SHIFT_V6);
 
-	WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6);
+	if (IS_MFCV7(dev))
+		WRITEL(reg, S5P_FIMV_D_INIT_BUFFER_OPTIONS_V7);
+	else
+		WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6);
 
 	/* 0: NV12(CbCr), 1: NV21(CrCb) */
 	if (ctx->dst_fmt->fourcc == V4L2_PIX_FMT_NV21M)
@@ -1184,6 +1212,7 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx)
 	else
 		WRITEL(0x0, S5P_FIMV_PIXEL_FORMAT_V6);
 
+
 	/* sei parse */
 	WRITEL(ctx->sei_fp_parse & 0x1, S5P_FIMV_D_SEI_ENABLE_V6);
 
@@ -1254,6 +1283,12 @@ static int s5p_mfc_init_encode_v6(struct s5p_mfc_ctx *ctx)
 		return -EINVAL;
 	}
 
+	/* Set stride lengths */
+	if (IS_MFCV7(dev)) {
+		WRITEL(ctx->img_width, S5P_FIMV_E_SOURCE_FIRST_STRIDE_V7);
+		WRITEL(ctx->img_width, S5P_FIMV_E_SOURCE_SECOND_STRIDE_V7);
+	}
+
 	WRITEL(ctx->inst_no, S5P_FIMV_INSTANCE_ID_V6);
 	s5p_mfc_hw_call(dev->mfc_cmds, cmd_host2risc, dev,
 			S5P_FIMV_CH_SEQ_HEADER_V6, NULL);
-- 
1.7.9.5

[PATCH v3 6/8] [media] V4L: Add support for integer menu controls with standard menu items

[Arun Kumar K]

From: Sylwester Nawrocki <s.nawrocki@samsung.com>

The patch modifies the helper function v4l2_ctrl_new_std_menu
to accept integer menu controls with standard menu items.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 Documentation/video4linux/v4l2-controls.txt |   21 ++++++++++----------
 drivers/media/v4l2-core/v4l2-ctrls.c        |   28 ++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt
index 676f873..e06e768 100644
--- a/Documentation/video4linux/v4l2-controls.txt
+++ b/Documentation/video4linux/v4l2-controls.txt
@@ -124,26 +124,27 @@ You add non-menu controls by calling v4l2_ctrl_new_std:
 			const struct v4l2_ctrl_ops *ops,
 			u32 id, s32 min, s32 max, u32 step, s32 def);
 
-Menu controls are added by calling v4l2_ctrl_new_std_menu:
+Menu and integer menu controls are added by calling v4l2_ctrl_new_std_menu:
 
 	struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
 			const struct v4l2_ctrl_ops *ops,
 			u32 id, s32 max, s32 skip_mask, s32 def);
 
-Or alternatively for integer menu controls, by calling v4l2_ctrl_new_int_menu:
+Menu controls with a driver specific menu are added by calling
+v4l2_ctrl_new_std_menu_items:
+
+       struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(
+                       struct v4l2_ctrl_handler *hdl,
+                       const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
+                       s32 skip_mask, s32 def, const char * const *qmenu);
+
+Integer menu controls with driver specific menu can be added by calling
+v4l2_ctrl_new_int_menu:
 
 	struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
 			const struct v4l2_ctrl_ops *ops,
 			u32 id, s32 max, s32 def, const s64 *qmenu_int);
 
-Standard menu controls with a driver specific menu are added by calling
-v4l2_ctrl_new_std_menu_items:
-
-	struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(
-		struct v4l2_ctrl_handler *hdl,
-		const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
-		s32 skip_mask, s32 def, const char * const *qmenu);
-
 These functions are typically called right after the v4l2_ctrl_handler_init:
 
 	static const s64 exp_bias_qmenu[] = {
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index fccd08b..e03a2e8 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -552,6 +552,20 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 }
 EXPORT_SYMBOL(v4l2_ctrl_get_menu);
 
+/*
+ * Returns NULL or an s64 type array containing the menu for given
+ * control ID. The total number of the menu items is returned in @len.
+ */
+const s64 const *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
+{
+	switch (id) {
+	default:
+		*len = 0;
+		return NULL;
+	};
+}
+EXPORT_SYMBOL(v4l2_ctrl_get_int_menu);
+
 /* Return the control name. */
 const char *v4l2_ctrl_get_name(u32 id)
 {
@@ -1712,20 +1726,28 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
 			const struct v4l2_ctrl_ops *ops,
 			u32 id, s32 max, s32 mask, s32 def)
 {
-	const char * const *qmenu = v4l2_ctrl_get_menu(id);
+	const char * const *qmenu = NULL;
+	const s64 *qmenu_int = NULL;
 	const char *name;
 	enum v4l2_ctrl_type type;
+	unsigned int qmenu_int_len;
 	s32 min;
 	s32 step;
 	u32 flags;
 
 	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
-	if (type != V4L2_CTRL_TYPE_MENU) {
+
+	if (type == V4L2_CTRL_TYPE_MENU)
+		qmenu = v4l2_ctrl_get_menu(id);
+	else if (type == V4L2_CTRL_TYPE_INTEGER_MENU)
+		qmenu_int = v4l2_ctrl_get_int_menu(id, &qmenu_int_len);
+
+	if ((!qmenu && !qmenu_int) || (qmenu_int && max > qmenu_int_len)) {
 		handler_set_err(hdl, -EINVAL);
 		return NULL;
 	}
 	return v4l2_ctrl_new(hdl, ops, id, name, type,
-			     0, max, mask, def, flags, qmenu, NULL, NULL);
+			     0, max, mask, def, flags, qmenu, qmenu_int, NULL);
 }
 EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
 
-- 
1.7.9.5

[PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Arun Kumar K]

This patch adds new V4L controls for VP8 encoding.

Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 Documentation/DocBook/media/v4l/controls.xml |  150 ++++++++++++++++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls.c         |   33 ++++++
 include/uapi/linux/v4l2-controls.h           |   29 ++++-
 3 files changed, 210 insertions(+), 2 deletions(-)

diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
index 8d7a779..736c991 100644
--- a/Documentation/DocBook/media/v4l/controls.xml
+++ b/Documentation/DocBook/media/v4l/controls.xml
@@ -3009,6 +3009,156 @@ in by the application. 0 = do not insert, 1 = insert packets.</entry>
 	  </tgroup>
 	</table>
       </section>
+
+    <section>
+      <title>VPX Control Reference</title>
+
+      <para>The VPX controls include controls for encoding parameters
+      of VPx video codec.</para>
+
+      <table pgwide="1" frame="none" id="vpx-control-id">
+      <title>VPX Control IDs</title>
+
+      <tgroup cols="4">
+        <colspec colname="c1" colwidth="1*" />
+        <colspec colname="c2" colwidth="6*" />
+        <colspec colname="c3" colwidth="2*" />
+        <colspec colname="c4" colwidth="6*" />
+        <spanspec namest="c1" nameend="c2" spanname="id" />
+        <spanspec namest="c2" nameend="c4" spanname="descr" />
+        <thead>
+          <row>
+            <entry spanname="id" align="left">ID</entry>
+            <entry align="left">Type</entry>
+          </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
+          </row>
+        </thead>
+        <tbody valign="top">
+          <row><entry></entry></row>
+
+	      <row><entry></entry></row>
+	      <row id="v4l2-vpx-num-partitions">
+		<entry spanname="id"><constant>V4L2_CID_VPX_NUM_PARTITIONS</constant></entry>
+		<entry>enum v4l2_vp8_num_partitions</entry>
+	      </row>
+	      <row><entry spanname="descr">The number of token partitions to use in VP8 encoder.
+Possible values are:</entry>
+	      </row>
+	      <row>
+		<entrytbl spanname="descr" cols="2">
+		  <tbody valign="top">
+		    <row>
+		      <entry><constant>V4L2_VPX_1_PARTITION</constant></entry>
+		      <entry>1 coefficient partition</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_VPX_2_PARTITIONS</constant></entry>
+		      <entry>2 partitions</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_VPX_4_PARTITIONS</constant></entry>
+		      <entry>4 partitions</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_VPX_8_PARTITIONS</constant></entry>
+		      <entry>8 partitions</entry>
+	            </row>
+                  </tbody>
+		</entrytbl>
+	      </row>
+
+	      <row><entry></entry></row>
+	      <row>
+		<entry spanname="id"><constant>V4L2_CID_VPX_IMD_DISABLE_4X4</constant></entry>
+		<entry>boolean</entry>
+	      </row>
+	      <row><entry spanname="descr">Setting this prevents intra 4x4 mode in the intra mode decision.</entry>
+	      </row>
+
+	      <row><entry></entry></row>
+	      <row id="v4l2-vpx-num-ref-frames">
+		<entry spanname="id"><constant>V4L2_CID_VPX_NUM_REF_FRAMES</constant></entry>
+		<entry>enum v4l2_vp8_num_ref_frames</entry>
+	      </row>
+	      <row><entry spanname="descr">The number of reference pictures for encoding P frames.
+Possible values are:</entry>
+	      </row>
+	      <row>
+		<entrytbl spanname="descr" cols="2">
+		  <tbody valign="top">
+		    <row>
+		      <entry><constant>V4L2_VPX_1_REF_FRAME</constant></entry>
+		      <entry>Last encoded frame will be searched</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_VPX_2_REF_FRAME</constant></entry>
+		      <entry>Two frames would be searched among last encoded frame, golden frame
+and altref frame. Encoder implementation can decide which two are chosen.</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_VPX_3_REF_FRAME</constant></entry>
+		      <entry>The last encoded frame, golden frame and altref frame will be searched.</entry>
+		    </row>
+                  </tbody>
+		</entrytbl>
+	      </row>
+
+	      <row><entry></entry></row>
+	      <row>
+		<entry spanname="id"><constant>V4L2_CID_VPX_FILTER_LEVEL</constant></entry>
+		<entry>integer</entry>
+	      </row>
+	      <row><entry spanname="descr">Indicates the loop filter level. The adjustment of loop
+filter level is done via a delta value against a baseline loop filter value.</entry>
+	      </row>
+
+	      <row><entry></entry></row>
+	      <row>
+		<entry spanname="id"><constant>V4L2_CID_VPX_FILTER_SHARPNESS</constant></entry>
+		<entry>integer</entry>
+	      </row>
+	      <row><entry spanname="descr">This parameter affects the loop filter. Anything above
+zero weakens the deblocking effect on loop filter.</entry>
+	      </row>
+
+	      <row><entry></entry></row>
+	      <row>
+		<entry spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD</constant></entry>
+		<entry>integer</entry>
+	      </row>
+	      <row><entry spanname="descr">Sets the refresh period for golden frame. Period is defined
+in number of frames. For a value of 'n', every nth frame will be taken as golden frame.</entry>
+	      </row>
+
+	      <row><entry></entry></row>
+	      <row id="v4l2-vpx-golden-frame-sel">
+		<entry spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_SEL</constant></entry>
+		<entry>enum v4l2_vp8_golden_frame_sel</entry>
+	      </row>
+	      <row><entry spanname="descr">Selects the golden frame for encoding.
+Possible values are:</entry>
+	      </row>
+	      <row>
+		<entrytbl spanname="descr" cols="2">
+		  <tbody valign="top">
+		    <row>
+		      <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_PREV</constant></entry>
+		      <entry>Use the last to last or (n-2)th frame as a golden frame. Current frame index being 'n'.</entry>
+		    </row>
+		    <row>
+		      <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD</constant></entry>
+		      <entry>Use the previous specific frame indicated by V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame</entry>
+		    </row>
+                  </tbody>
+		</entrytbl>
+	      </row>
+
+          <row><entry></entry></row>
+        </tbody>
+      </tgroup>
+      </table>
+
+      </section>
     </section>
 
     <section id="camera-controls">
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index e03a2e8..891f595 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -424,6 +424,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		NULL,
 	};
 
+	static const char * const vpx_golden_frame_sel[] = {
+		"Use Previous Frame",
+		"Use Previous Specific Frame",
+		NULL,
+	};
+
 	static const char * const flash_led_mode[] = {
 		"Off",
 		"Flash",
@@ -538,6 +544,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 		return mpeg_mpeg4_level;
 	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
 		return mpeg4_profile;
+	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
+		return vpx_golden_frame_sel;
 	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
 		return jpeg_chroma_subsampling;
 	case V4L2_CID_DV_TX_MODE:
@@ -552,13 +560,26 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
 }
 EXPORT_SYMBOL(v4l2_ctrl_get_menu);
 
+#define __v4l2_qmenu_int_len(arr, len) ({ *(len) = ARRAY_SIZE(arr); arr; })
 /*
  * Returns NULL or an s64 type array containing the menu for given
  * control ID. The total number of the menu items is returned in @len.
  */
 const s64 const *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
 {
+	static const s64 const qmenu_int_vpx_num_partitions[] = {
+		1, 2, 4, 8,
+	};
+
+	static const s64 const qmenu_int_vpx_num_ref_frames[] = {
+		1, 2, 3,
+	};
+
 	switch (id) {
+	case V4L2_CID_VPX_NUM_PARTITIONS:
+		return __v4l2_qmenu_int_len(qmenu_int_vpx_num_partitions, len);
+	case V4L2_CID_VPX_NUM_REF_FRAMES:
+		return __v4l2_qmenu_int_len(qmenu_int_vpx_num_ref_frames, len);
 	default:
 		*len = 0;
 		return NULL;
@@ -714,6 +735,15 @@ const char *v4l2_ctrl_get_name(u32 id)
 	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
 	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence Header";
 
+	/* VPX controls */
+	case V4L2_CID_VPX_NUM_PARTITIONS:			return "VPX Number of partitions";
+	case V4L2_CID_VPX_IMD_DISABLE_4X4:			return "VPX Intra mode decision disable";
+	case V4L2_CID_VPX_NUM_REF_FRAMES:			return "VPX No. of refs for P frame";
+	case V4L2_CID_VPX_FILTER_LEVEL:				return "VPX Loop filter level range";
+	case V4L2_CID_VPX_FILTER_SHARPNESS:			return "VPX Deblocking effect control";
+	case V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD:		return "VPX Golden frame refresh period";
+	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:			return "VPX Golden frame indicator";
+
 	/* CAMERA controls */
 	/* Keep the order of the 'case's the same as in videodev2.h! */
 	case V4L2_CID_CAMERA_CLASS:		return "Camera Controls";
@@ -928,6 +958,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 	case V4L2_CID_DV_RX_RGB_RANGE:
 	case V4L2_CID_TEST_PATTERN:
 	case V4L2_CID_TUNE_DEEMPHASIS:
+	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
 		*type = V4L2_CTRL_TYPE_MENU;
 		break;
 	case V4L2_CID_LINK_FREQ:
@@ -939,6 +970,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
 		break;
 	case V4L2_CID_ISO_SENSITIVITY:
 	case V4L2_CID_AUTO_EXPOSURE_BIAS:
+	case V4L2_CID_VPX_NUM_PARTITIONS:
+	case V4L2_CID_VPX_NUM_REF_FRAMES:
 		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
 		break;
 	case V4L2_CID_USER_CLASS:
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 69bd5bb..8d6ffc9 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -522,6 +522,33 @@ enum v4l2_mpeg_video_mpeg4_profile {
 };
 #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL		(V4L2_CID_MPEG_BASE+407)
 
+/*  Control IDs for VP8 streams
+ *  Though VP8 is not part of MPEG, adding it here as MPEG class is
+ *  already handling other video compression standards
+ */
+#define V4L2_CID_VPX_NUM_PARTITIONS		(V4L2_CID_MPEG_BASE+500)
+enum v4l2_vp8_num_partitions {
+	V4L2_VPX_1_PARTITION	= 0,
+	V4L2_VPX_2_PARTITIONS	= 1,
+	V4L2_VPX_4_PARTITIONS	= 2,
+	V4L2_VPX_8_PARTITIONS	= 3,
+};
+#define V4L2_CID_VPX_IMD_DISABLE_4X4		(V4L2_CID_MPEG_BASE+501)
+#define V4L2_CID_VPX_NUM_REF_FRAMES		(V4L2_CID_MPEG_BASE+502)
+enum v4l2_vp8_num_ref_frames {
+	V4L2_VPX_1_REF_FRAME	= 0,
+	V4L2_VPX_2_REF_FRAME	= 1,
+	V4L2_VPX_3_REF_FRAME	= 2,
+};
+#define V4L2_CID_VPX_FILTER_LEVEL		(V4L2_CID_MPEG_BASE+503)
+#define V4L2_CID_VPX_FILTER_SHARPNESS		(V4L2_CID_MPEG_BASE+504)
+#define V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD	(V4L2_CID_MPEG_BASE+505)
+#define V4L2_CID_VPX_GOLDEN_FRAME_SEL		(V4L2_CID_MPEG_BASE+506)
+enum v4l2_vp8_golden_frame_sel {
+	V4L2_VPX_GOLDEN_FRAME_USE_PREV		= 0,
+	V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD	= 1,
+};
+
 /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
 #define V4L2_CID_MPEG_CX2341X_BASE 				(V4L2_CTRL_CLASS_MPEG | 0x1000)
 #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE 	(V4L2_CID_MPEG_CX2341X_BASE+0)
@@ -590,7 +617,6 @@ enum v4l2_mpeg_mfc51_video_force_frame_type {
 #define V4L2_CID_MPEG_MFC51_VIDEO_H264_ADAPTIVE_RC_STATIC		(V4L2_CID_MPEG_MFC51_BASE+53)
 #define V4L2_CID_MPEG_MFC51_VIDEO_H264_NUM_REF_PIC_FOR_P		(V4L2_CID_MPEG_MFC51_BASE+54)
 
-
 /*  Camera class control IDs */
 
 #define V4L2_CID_CAMERA_CLASS_BASE 	(V4L2_CTRL_CLASS_CAMERA | 0x900)
@@ -818,7 +844,6 @@ enum v4l2_jpeg_chroma_subsampling {
 #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
 #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
 
-
 /*  DV-class control IDs defined by V4L2 */
 #define V4L2_CID_DV_CLASS_BASE			(V4L2_CTRL_CLASS_DV | 0x900)
 #define V4L2_CID_DV_CLASS			(V4L2_CTRL_CLASS_DV | 1)
-- 
1.7.9.5

[PATCH v3 8/8] [media] s5p-mfc: Add support for VP8 encoder

[Arun Kumar K]

MFC v7 supports VP8 encoding and this patch adds support
for it in the driver.

Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
---
 drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c |    3 +
 drivers/media/platform/s5p-mfc/s5p_mfc_common.h |   19 ++++-
 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   90 ++++++++++++++++++++++-
 drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   90 +++++++++++++++++++++++
 4 files changed, 200 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
index 5708fc3..db796c8 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_cmd_v6.c
@@ -108,6 +108,9 @@ static int s5p_mfc_open_inst_cmd_v6(struct s5p_mfc_ctx *ctx)
 	case S5P_MFC_CODEC_H263_ENC:
 		codec_type = S5P_FIMV_CODEC_H263_ENC_V6;
 		break;
+	case S5P_MFC_CODEC_VP8_ENC:
+		codec_type = S5P_FIMV_CODEC_VP8_ENC_V7;
+		break;
 	default:
 		codec_type = S5P_FIMV_CODEC_NONE_V6;
 	};
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
index 17545d7..6920b54 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
@@ -65,7 +65,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
 #define MFC_ENC_CAP_PLANE_COUNT	1
 #define MFC_ENC_OUT_PLANE_COUNT	2
 #define STUFF_BYTE		4
-#define MFC_MAX_CTRLS		70
+#define MFC_MAX_CTRLS		77
 
 #define S5P_MFC_CODEC_NONE		-1
 #define S5P_MFC_CODEC_H264_DEC		0
@@ -81,6 +81,7 @@ static inline dma_addr_t s5p_mfc_mem_cookie(void *a, void *b)
 #define S5P_MFC_CODEC_H264_MVC_ENC	21
 #define S5P_MFC_CODEC_MPEG4_ENC		22
 #define S5P_MFC_CODEC_H263_ENC		23
+#define S5P_MFC_CODEC_VP8_ENC		24
 
 #define S5P_MFC_R2H_CMD_EMPTY			0
 #define S5P_MFC_R2H_CMD_SYS_INIT_RET		1
@@ -409,6 +410,21 @@ struct s5p_mfc_mpeg4_enc_params {
 };
 
 /**
+ * struct s5p_mfc_vp8_enc_params - encoding parameters for vp8
+ */
+struct s5p_mfc_vp8_enc_params {
+	u8 imd_4x4;
+	enum v4l2_vp8_num_partitions num_partitions;
+	enum v4l2_vp8_num_ref_frames num_ref;
+	u8 filter_level;
+	u8 filter_sharpness;
+	u32 golden_frame_ref_period;
+	enum v4l2_vp8_golden_frame_sel golden_frame_sel;
+	u8 hier_layer;
+	u8 hier_layer_qp[3];
+};
+
+/**
  * struct s5p_mfc_enc_params - general encoding parameters
  */
 struct s5p_mfc_enc_params {
@@ -442,6 +458,7 @@ struct s5p_mfc_enc_params {
 	struct {
 		struct s5p_mfc_h264_enc_params h264;
 		struct s5p_mfc_mpeg4_enc_params mpeg4;
+		struct s5p_mfc_vp8_enc_params vp8;
 	} codec;
 
 };
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
index dd57b06..f07c39f 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
@@ -84,6 +84,13 @@ static struct s5p_mfc_fmt formats[] = {
 		.type		= MFC_FMT_ENC,
 		.num_planes	= 1,
 	},
+	{
+		.name		= "VP8 Encoded Stream",
+		.fourcc		= V4L2_PIX_FMT_VP8,
+		.codec_mode	= S5P_MFC_CODEC_VP8_ENC,
+		.type		= MFC_FMT_ENC,
+		.num_planes	= 1,
+	},
 };
 
 #define NUM_FORMATS ARRAY_SIZE(formats)
@@ -557,6 +564,60 @@ static struct mfc_control controls[] = {
 		.step = 1,
 		.default_value = 0,
 	},
+	{
+		.id = V4L2_CID_VPX_NUM_PARTITIONS,
+		.type = V4L2_CTRL_TYPE_INTEGER_MENU,
+		.maximum = V4L2_VPX_8_PARTITIONS,
+		.default_value = V4L2_VPX_1_PARTITION,
+		.menu_skip_mask = 0,
+	},
+	{
+		.id = V4L2_CID_VPX_IMD_DISABLE_4X4,
+		.type = V4L2_CTRL_TYPE_BOOLEAN,
+		.minimum = 0,
+		.maximum = 1,
+		.step = 1,
+		.default_value = 0,
+	},
+	{
+		.id = V4L2_CID_VPX_NUM_REF_FRAMES,
+		.type = V4L2_CTRL_TYPE_INTEGER_MENU,
+		.maximum = V4L2_VPX_2_REF_FRAME,
+		.default_value = V4L2_VPX_1_REF_FRAME,
+		.menu_skip_mask = 0,
+	},
+	{
+		.id = V4L2_CID_VPX_FILTER_LEVEL,
+		.type = V4L2_CTRL_TYPE_INTEGER,
+		.minimum = 0,
+		.maximum = 63,
+		.step = 1,
+		.default_value = 0,
+	},
+	{
+		.id = V4L2_CID_VPX_FILTER_SHARPNESS,
+		.type = V4L2_CTRL_TYPE_INTEGER,
+		.minimum = 0,
+		.maximum = 7,
+		.step = 1,
+		.default_value = 0,
+	},
+	{
+		.id = V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD,
+		.type = V4L2_CTRL_TYPE_INTEGER,
+		.minimum = 0,
+		.maximum = (1 << 16) - 1,
+		.step = 1,
+		.default_value = 0,
+	},
+	{
+		.id = V4L2_CID_VPX_GOLDEN_FRAME_SEL,
+		.type = V4L2_CTRL_TYPE_MENU,
+		.minimum = V4L2_VPX_GOLDEN_FRAME_USE_PREV,
+		.maximum = V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD,
+		.default_value = V4L2_VPX_GOLDEN_FRAME_USE_PREV,
+		.menu_skip_mask = 0,
+	},
 };
 
 #define NUM_CTRLS ARRAY_SIZE(controls)
@@ -965,6 +1026,10 @@ static int vidioc_s_fmt(struct file *file, void *priv, struct v4l2_format *f)
 			mfc_err("failed to set capture format\n");
 			return -EINVAL;
 		}
+		if (!IS_MFCV7(dev) && (fmt->fourcc == V4L2_PIX_FMT_VP8)) {
+			mfc_err("VP8 is supported only in MFC v7\n");
+			return -EINVAL;
+		}
 		ctx->state = MFCINST_INIT;
 		ctx->dst_fmt = fmt;
 		ctx->codec_mode = ctx->dst_fmt->codec_mode;
@@ -1482,6 +1547,27 @@ static int s5p_mfc_enc_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_MPEG_VIDEO_MPEG4_QPEL:
 		p->codec.mpeg4.quarter_pixel = ctrl->val;
 		break;
+	case V4L2_CID_VPX_NUM_PARTITIONS:
+		p->codec.vp8.num_partitions = ctrl->val;
+		break;
+	case V4L2_CID_VPX_IMD_DISABLE_4X4:
+		p->codec.vp8.imd_4x4 = ctrl->val;
+		break;
+	case V4L2_CID_VPX_NUM_REF_FRAMES:
+		p->codec.vp8.num_ref = ctrl->val;
+		break;
+	case V4L2_CID_VPX_FILTER_LEVEL:
+		p->codec.vp8.filter_level = ctrl->val;
+		break;
+	case V4L2_CID_VPX_FILTER_SHARPNESS:
+		p->codec.vp8.filter_sharpness = ctrl->val;
+		break;
+	case V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD:
+		p->codec.vp8.golden_frame_ref_period = ctrl->val;
+		break;
+	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
+		p->codec.vp8.golden_frame_sel = ctrl->val;
+		break;
 	default:
 		v4l2_err(&dev->v4l2_dev, "Invalid control, id=%d, val=%d\n",
 							ctrl->id, ctrl->val);
@@ -1936,7 +2022,9 @@ int s5p_mfc_enc_ctrls_setup(struct s5p_mfc_ctx *ctx)
 			ctx->ctrls[i] = v4l2_ctrl_new_custom(&ctx->ctrl_handler,
 					&cfg, NULL);
 		} else {
-			if (controls[i].type == V4L2_CTRL_TYPE_MENU) {
+			if ((controls[i].type == V4L2_CTRL_TYPE_MENU) ||
+				(controls[i].type ==
+					V4L2_CTRL_TYPE_INTEGER_MENU)) {
 				ctx->ctrls[i] = v4l2_ctrl_new_std_menu(
 					&ctx->ctrl_handler,
 					&s5p_mfc_enc_ctrl_ops, controls[i].id,
diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
index 7145ae5..e75c3bb 100644
--- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
+++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
@@ -188,6 +188,19 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct s5p_mfc_ctx *ctx)
 			ctx->chroma_dpb_size + ctx->me_buffer_size));
 		ctx->bank2.size = 0;
 		break;
+	case S5P_MFC_CODEC_VP8_ENC:
+		ctx->scratch_buf_size =
+			S5P_FIMV_SCRATCH_BUF_SIZE_VP8_ENC_V7(
+					mb_width,
+					mb_height);
+		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size,
+				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
+		ctx->bank1.size =
+			ctx->scratch_buf_size + ctx->tmv_buffer_size +
+			(ctx->pb_count * (ctx->luma_dpb_size +
+			ctx->chroma_dpb_size + ctx->me_buffer_size));
+		ctx->bank2.size = 0;
+		break;
 	default:
 		break;
 	}
@@ -237,6 +250,7 @@ static int s5p_mfc_alloc_instance_buffer_v6(struct s5p_mfc_ctx *ctx)
 		break;
 	case S5P_MFC_CODEC_MPEG4_ENC:
 	case S5P_MFC_CODEC_H263_ENC:
+	case S5P_MFC_CODEC_VP8_ENC:
 		ctx->ctx.size = buf_size->other_enc_ctx;
 		break;
 	default:
@@ -1159,6 +1173,80 @@ static int s5p_mfc_set_enc_params_h263(struct s5p_mfc_ctx *ctx)
 	return 0;
 }
 
+static int s5p_mfc_set_enc_params_vp8(struct s5p_mfc_ctx *ctx)
+{
+	struct s5p_mfc_dev *dev = ctx->dev;
+	struct s5p_mfc_enc_params *p = &ctx->enc_params;
+	struct s5p_mfc_vp8_enc_params *p_vp8 = &p->codec.vp8;
+	unsigned int reg = 0;
+	unsigned int val = 0;
+
+	mfc_debug_enter();
+
+	s5p_mfc_set_enc_params(ctx);
+
+	/* pictype : number of B */
+	reg = READL(S5P_FIMV_E_GOP_CONFIG_V6);
+	reg &= ~(0x3 << 16);
+	reg |= ((p->num_b_frame & 0x3) << 16);
+	WRITEL(reg, S5P_FIMV_E_GOP_CONFIG_V6);
+
+	/* profile & level */
+	reg = 0;
+	/** profile */
+	reg |= (0x1 << 4);
+	WRITEL(reg, S5P_FIMV_E_PICTURE_PROFILE_V6);
+
+	/* rate control config. */
+	reg = READL(S5P_FIMV_E_RC_CONFIG_V6);
+	/** macroblock level rate control */
+	reg &= ~(0x1 << 8);
+	reg |= ((p->rc_mb & 0x1) << 8);
+	WRITEL(reg, S5P_FIMV_E_RC_CONFIG_V6);
+
+	/* frame rate */
+	if (p->rc_frame && p->rc_framerate_num && p->rc_framerate_denom) {
+		reg = 0;
+		reg |= ((p->rc_framerate_num & 0xFFFF) << 16);
+		reg |= p->rc_framerate_denom & 0xFFFF;
+		WRITEL(reg, S5P_FIMV_E_RC_FRAME_RATE_V6);
+	}
+
+	/* vbv buffer size */
+	if (p->frame_skip_mode ==
+			V4L2_MPEG_MFC51_VIDEO_FRAME_SKIP_MODE_BUF_LIMIT) {
+		WRITEL(p->vbv_size & 0xFFFF, S5P_FIMV_E_VBV_BUFFER_SIZE_V6);
+
+		if (p->rc_frame)
+			WRITEL(p->vbv_delay, S5P_FIMV_E_VBV_INIT_DELAY_V6);
+	}
+
+	/* VP8 specific params */
+	reg = 0;
+	reg |= (p_vp8->imd_4x4 & 0x1) << 10;
+	switch (p_vp8->num_partitions) {
+	case V4L2_VPX_1_PARTITION:
+		val = 0;
+		break;
+	case V4L2_VPX_2_PARTITIONS:
+		val = 2;
+		break;
+	case V4L2_VPX_4_PARTITIONS:
+		val = 4;
+		break;
+	case V4L2_VPX_8_PARTITIONS:
+		val = 8;
+		break;
+	}
+	reg |= (val & 0xF) << 3;
+	reg |= (p_vp8->num_ref & 0x2);
+	WRITEL(reg, S5P_FIMV_E_VP8_OPTIONS_V7);
+
+	mfc_debug_leave();
+
+	return 0;
+}
+
 /* Initialize decoding */
 static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx)
 {
@@ -1277,6 +1365,8 @@ static int s5p_mfc_init_encode_v6(struct s5p_mfc_ctx *ctx)
 		s5p_mfc_set_enc_params_mpeg4(ctx);
 	else if (ctx->codec_mode == S5P_MFC_CODEC_H263_ENC)
 		s5p_mfc_set_enc_params_h263(ctx);
+	else if (ctx->codec_mode == S5P_MFC_CODEC_VP8_ENC)
+		s5p_mfc_set_enc_params_vp8(ctx);
 	else {
 		mfc_err("Unknown codec for encoding (%x).\n",
 			ctx->codec_mode);
-- 
1.7.9.5

Re: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7

[Sachin Kamat]

Hi Arun,

> @@ -684,5 +685,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
>                                 (dev->variant->port_num ? 1 : 0) : 0) : 0)
>  #define IS_TWOPORT(dev)                (dev->variant->port_num == 2 ? 1 : 0)
>  #define IS_MFCV6_PLUS(dev)     (dev->variant->version >= 0x60 ? 1 : 0)
> +#define IS_MFCV7(dev)          (dev->variant->version >= 0x70 ? 1 : 0)

Considering the definition and pattern, wouldn't it be appropriate to
call this  IS_MFCV7_PLUS?

---
With warm regards,
Sachin

RE: [PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Kamil Debski]

Hi Arun,

This patch sets looks very good.
Please check my comments below.

> From: Arun Kumar K [mailto:arun.kk@samsung.com]
> 
> This patch adds new V4L controls for VP8 encoding.
> 
> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/controls.xml |  150
> ++++++++++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c         |   33 ++++++
>  include/uapi/linux/v4l2-controls.h           |   29 ++++-
>  3 files changed, 210 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml
> b/Documentation/DocBook/media/v4l/controls.xml
> index 8d7a779..736c991 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -3009,6 +3009,156 @@ in by the application. 0 = do not insert, 1 =
> insert packets.</entry>
>  	  </tgroup>
>  	</table>
>        </section>
> +
> +    <section>
> +      <title>VPX Control Reference</title>
> +
> +      <para>The VPX controls include controls for encoding parameters
> +      of VPx video codec.</para>
> +
> +      <table pgwide="1" frame="none" id="vpx-control-id">
> +      <title>VPX Control IDs</title>
> +
> +      <tgroup cols="4">
> +        <colspec colname="c1" colwidth="1*" />
> +        <colspec colname="c2" colwidth="6*" />
> +        <colspec colname="c3" colwidth="2*" />
> +        <colspec colname="c4" colwidth="6*" />
> +        <spanspec namest="c1" nameend="c2" spanname="id" />
> +        <spanspec namest="c2" nameend="c4" spanname="descr" />
> +        <thead>
> +          <row>
> +            <entry spanname="id" align="left">ID</entry>
> +            <entry align="left">Type</entry>
> +          </row><row rowsep="1"><entry spanname="descr"
> align="left">Description</entry>
> +          </row>
> +        </thead>
> +        <tbody valign="top">
> +          <row><entry></entry></row>
> +
> +	      <row><entry></entry></row>
> +	      <row id="v4l2-vpx-num-partitions">
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_NUM_PARTITIONS</constant></entry>
> +		<entry>enum v4l2_vp8_num_partitions</entry>
> +	      </row>
> +	      <row><entry spanname="descr">The number of token partitions
> to use in VP8 encoder.
> +Possible values are:</entry>
> +	      </row>
> +	      <row>
> +		<entrytbl spanname="descr" cols="2">
> +		  <tbody valign="top">
> +		    <row>
> +
> <entry><constant>V4L2_VPX_1_PARTITION</constant></entry>
> +		      <entry>1 coefficient partition</entry>
> +		    </row>
> +		    <row>
> +
> <entry><constant>V4L2_VPX_2_PARTITIONS</constant></entry>
> +		      <entry>2 partitions</entry>
> +		    </row>
> +		    <row>
> +
> <entry><constant>V4L2_VPX_4_PARTITIONS</constant></entry>
> +		      <entry>4 partitions</entry>
> +		    </row>
> +		    <row>
> +
> <entry><constant>V4L2_VPX_8_PARTITIONS</constant></entry>
> +		      <entry>8 partitions</entry>
> +	            </row>
> +                  </tbody>
> +		</entrytbl>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_IMD_DISABLE_4X4</constant></entry>
> +		<entry>boolean</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Setting this prevents intra
> 4x4 mode in the intra mode decision.</entry>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row id="v4l2-vpx-num-ref-frames">
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_NUM_REF_FRAMES</constant></entry>
> +		<entry>enum v4l2_vp8_num_ref_frames</entry>
> +	      </row>
> +	      <row><entry spanname="descr">The number of reference
> pictures for encoding P frames.
> +Possible values are:</entry>
> +	      </row>
> +	      <row>
> +		<entrytbl spanname="descr" cols="2">
> +		  <tbody valign="top">
> +		    <row>
> +
> <entry><constant>V4L2_VPX_1_REF_FRAME</constant></entry>
> +		      <entry>Last encoded frame will be searched</entry>
> +		    </row>
> +		    <row>
> +
> <entry><constant>V4L2_VPX_2_REF_FRAME</constant></entry>
> +		      <entry>Two frames would be searched among last
> encoded frame,
> +golden frame and altref frame. Encoder implementation can decide which
> two are chosen.</entry>
> +		    </row>
> +		    <row>
> +
> <entry><constant>V4L2_VPX_3_REF_FRAME</constant></entry>
> +		      <entry>The last encoded frame, golden frame and
> altref frame will be searched.</entry>
> +		    </row>
> +                  </tbody>
> +		</entrytbl>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_FILTER_LEVEL</constant></entry>
> +		<entry>integer</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Indicates the loop filter
> level.
> +The adjustment of loop filter level is done via a delta value against
> a baseline loop filter value.</entry>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_FILTER_SHARPNESS</constant></entry>
> +		<entry>integer</entry>
> +	      </row>
> +	      <row><entry spanname="descr">This parameter affects the
> loop
> +filter. Anything above zero weakens the deblocking effect on loop
> filter.</entry>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD</constant>
> </entry>
> +		<entry>integer</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Sets the refresh period for
> golden
> +frame. Period is defined in number of frames. For a value of 'n',
> every nth frame will be taken as golden frame.</entry>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row id="v4l2-vpx-golden-frame-sel">
> +		<entry
> spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_SEL</constant></entry>
> +		<entry>enum v4l2_vp8_golden_frame_sel</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Selects the golden frame for
> encoding.
> +Possible values are:</entry>
> +	      </row>
> +	      <row>
> +		<entrytbl spanname="descr" cols="2">
> +		  <tbody valign="top">
> +		    <row>
> +
> <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_PREV</constant></entry>
> +		      <entry>Use the last to last or (n-2)th frame as a
> golden frame. Current frame index being 'n'.</entry>
> +		    </row>
> +		    <row>
> +
> <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD</constant></entry>
> +		      <entry>Use the previous specific frame indicated by
> V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame</entry>
> +		    </row>
> +                  </tbody>
> +		</entrytbl>
> +	      </row>
> +
> +          <row><entry></entry></row>
> +        </tbody>
> +      </tgroup>
> +      </table>
> +
> +      </section>
>      </section>
> 
>      <section id="camera-controls">
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-
> core/v4l2-ctrls.c
> index e03a2e8..891f595 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -424,6 +424,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		NULL,
>  	};
> 
> +	static const char * const vpx_golden_frame_sel[] = {
> +		"Use Previous Frame",
> +		"Use Previous Specific Frame",

"Use Previous Specific Frame" seems unclear. 
Maybe "Use Previous Golden Frame" or "Use Periodic Golden Frame"?
I'm not sure if I get the description right.

> +		NULL,
> +	};
> +
>  	static const char * const flash_led_mode[] = {
>  		"Off",
>  		"Flash",
> @@ -538,6 +544,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		return mpeg_mpeg4_level;
>  	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
>  		return mpeg4_profile;
> +	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
> +		return vpx_golden_frame_sel;
>  	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
>  		return jpeg_chroma_subsampling;
>  	case V4L2_CID_DV_TX_MODE:
> @@ -552,13 +560,26 @@ const char * const *v4l2_ctrl_get_menu(u32 id)  }
> EXPORT_SYMBOL(v4l2_ctrl_get_menu);
> 
> +#define __v4l2_qmenu_int_len(arr, len) ({ *(len) = ARRAY_SIZE(arr);
> +arr; })
>  /*
>   * Returns NULL or an s64 type array containing the menu for given
>   * control ID. The total number of the menu items is returned in @len.
>   */
>  const s64 const *v4l2_ctrl_get_int_menu(u32 id, u32 *len)  {
> +	static const s64 const qmenu_int_vpx_num_partitions[] = {
> +		1, 2, 4, 8,
> +	};
> +
> +	static const s64 const qmenu_int_vpx_num_ref_frames[] = {
> +		1, 2, 3,
> +	};
> +
>  	switch (id) {
> +	case V4L2_CID_VPX_NUM_PARTITIONS:
> +		return __v4l2_qmenu_int_len(qmenu_int_vpx_num_partitions,
> len);
> +	case V4L2_CID_VPX_NUM_REF_FRAMES:
> +		return __v4l2_qmenu_int_len(qmenu_int_vpx_num_ref_frames,
> len);
>  	default:
>  		*len = 0;
>  		return NULL;
> @@ -714,6 +735,15 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return
"Initial
> Delay for VBV Control";
>  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return
> "Repeat Sequence Header";
> 
> +	/* VPX controls */
> +	case V4L2_CID_VPX_NUM_PARTITIONS:			return "VPX
Number
> of partitions";
> +	case V4L2_CID_VPX_IMD_DISABLE_4X4:			return "VPX
Intra
> mode decision disable";
> +	case V4L2_CID_VPX_NUM_REF_FRAMES:			return "VPX
No. of
> refs for P frame";
> +	case V4L2_CID_VPX_FILTER_LEVEL:				return "VPX
> Loop filter level range";
> +	case V4L2_CID_VPX_FILTER_SHARPNESS:			return "VPX
> Deblocking effect control";
> +	case V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD:		return "VPX
> Golden frame refresh period";
> +	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:			return "VPX
Golden
> frame indicator";
> +
>  	/* CAMERA controls */
>  	/* Keep the order of the 'case's the same as in videodev2.h! */
>  	case V4L2_CID_CAMERA_CLASS:		return "Camera Controls";
> @@ -928,6 +958,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type,
>  	case V4L2_CID_DV_RX_RGB_RANGE:
>  	case V4L2_CID_TEST_PATTERN:
>  	case V4L2_CID_TUNE_DEEMPHASIS:
> +	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
>  	case V4L2_CID_LINK_FREQ:
> @@ -939,6 +970,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum
> v4l2_ctrl_type *type,
>  		break;
>  	case V4L2_CID_ISO_SENSITIVITY:
>  	case V4L2_CID_AUTO_EXPOSURE_BIAS:
> +	case V4L2_CID_VPX_NUM_PARTITIONS:
> +	case V4L2_CID_VPX_NUM_REF_FRAMES:
>  		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
>  		break;
>  	case V4L2_CID_USER_CLASS:
> diff --git a/include/uapi/linux/v4l2-controls.h
> b/include/uapi/linux/v4l2-controls.h
> index 69bd5bb..8d6ffc9 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -522,6 +522,33 @@ enum v4l2_mpeg_video_mpeg4_profile {  };
>  #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL
(V4L2_CID_MPEG_BASE+407)
> 
> +/*  Control IDs for VP8 streams
> + *  Though VP8 is not part of MPEG, adding it here as MPEG class is
> + *  already handling other video compression standards  */
> +#define V4L2_CID_VPX_NUM_PARTITIONS		(V4L2_CID_MPEG_BASE+500)
> +enum v4l2_vp8_num_partitions {
> +	V4L2_VPX_1_PARTITION	= 0,
> +	V4L2_VPX_2_PARTITIONS	= 1,
> +	V4L2_VPX_4_PARTITIONS	= 2,
> +	V4L2_VPX_8_PARTITIONS	= 3,
> +};
> +#define V4L2_CID_VPX_IMD_DISABLE_4X4		(V4L2_CID_MPEG_BASE+501)
> +#define V4L2_CID_VPX_NUM_REF_FRAMES		(V4L2_CID_MPEG_BASE+502)
> +enum v4l2_vp8_num_ref_frames {
> +	V4L2_VPX_1_REF_FRAME	= 0,
> +	V4L2_VPX_2_REF_FRAME	= 1,
> +	V4L2_VPX_3_REF_FRAME	= 2,
> +};
> +#define V4L2_CID_VPX_FILTER_LEVEL		(V4L2_CID_MPEG_BASE+503)
> +#define V4L2_CID_VPX_FILTER_SHARPNESS
(V4L2_CID_MPEG_BASE+504)
> +#define V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD	(V4L2_CID_MPEG_BASE+505)
> +#define V4L2_CID_VPX_GOLDEN_FRAME_SEL
(V4L2_CID_MPEG_BASE+506)
> +enum v4l2_vp8_golden_frame_sel {
> +	V4L2_VPX_GOLDEN_FRAME_USE_PREV		= 0,
> +	V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD	= 1,
> +};
> +
>  /*  MPEG-class control IDs specific to the CX2341x driver as defined
> by V4L2 */
>  #define V4L2_CID_MPEG_CX2341X_BASE
> 	(V4L2_CTRL_CLASS_MPEG | 0x1000)
>  #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE
> 	(V4L2_CID_MPEG_CX2341X_BASE+0)
> @@ -590,7 +617,6 @@ enum v4l2_mpeg_mfc51_video_force_frame_type {
>  #define V4L2_CID_MPEG_MFC51_VIDEO_H264_ADAPTIVE_RC_STATIC
> 	(V4L2_CID_MPEG_MFC51_BASE+53)
>  #define V4L2_CID_MPEG_MFC51_VIDEO_H264_NUM_REF_PIC_FOR_P
> 	(V4L2_CID_MPEG_MFC51_BASE+54)
> 
> -
>  /*  Camera class control IDs */
> 
>  #define V4L2_CID_CAMERA_CLASS_BASE 	(V4L2_CTRL_CLASS_CAMERA |
> 0x900)
> @@ -818,7 +844,6 @@ enum v4l2_jpeg_chroma_subsampling {
>  #define V4L2_CID_PIXEL_RATE
> 	(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
>  #define V4L2_CID_TEST_PATTERN
> 	(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
> 
> -
>  /*  DV-class control IDs defined by V4L2 */
>  #define V4L2_CID_DV_CLASS_BASE			(V4L2_CTRL_CLASS_DV
|
> 0x900)
>  #define V4L2_CID_DV_CLASS			(V4L2_CTRL_CLASS_DV | 1)
> --
> 1.7.9.5


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

RE: [PATCH v3 5/8] [media] s5p-mfc: Update driver for v7 firmware

[Kamil Debski]

Hi Arun,

This patch sets looks very good.
Please check my comments below.

> From: Arun Kumar K [mailto:arun.kk@samsung.com]
> 
> Firmware version v7 is mostly similar to v6 in terms of hardware
> specific controls and commands. So the hardware specific opr_v6 and
> cmd_v6 are re-used for v7 also. This patch updates the v6 files to
> handle v7 version also.
> 
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   11 ++++-
>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   53
> +++++++++++++++++++----
>  2 files changed, 53 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> index f734ccc..dd57b06 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> @@ -1663,8 +1663,15 @@ static int s5p_mfc_queue_setup(struct vb2_queue
> *vq,
>  		if (*buf_count > MFC_MAX_BUFFERS)
>  			*buf_count = MFC_MAX_BUFFERS;
> 
> -		psize[0] = ctx->luma_size;
> -		psize[1] = ctx->chroma_size;
> +		if (IS_MFCV7(dev)) {
> +			/* MFCv7 needs pad bytes for input YUV */
> +			psize[0] = ctx->luma_size + 256;
> +			psize[1] = ctx->chroma_size + 128;

Here, I would add some define name to avoid magic number.
Secondly, why wasn't this padding added in s5p_mfc_enc_calc_src_size_v6/v7?
enc_calc_src_size is called in s_fmt, so it seems the best place to adjust
buffer/plane sizes.

> +		} else {
> +			psize[0] = ctx->luma_size;
> +			psize[1] = ctx->chroma_size;
> +		}
> +
>  		if (IS_MFCV6_PLUS(dev)) {
>  			allocators[0] =
>  				ctx->dev->alloc_ctx[MFC_BANK1_ALLOC_CTX];
> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> index 7d4c5e1..7145ae5 100644
> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c
> @@ -80,6 +80,7 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct
> s5p_mfc_ctx *ctx)
>  		ctx->tmv_buffer_size = S5P_FIMV_NUM_TMV_BUFFERS_V6 *
>  			ALIGN(S5P_FIMV_TMV_BUFFER_SIZE_V6(mb_width,
> mb_height),
>  			S5P_FIMV_TMV_BUFFER_ALIGN_V6);
> +
>  		ctx->luma_dpb_size = ALIGN((mb_width * mb_height) *
>  				S5P_FIMV_LUMA_MB_TO_PIXEL_V6,
>  				S5P_FIMV_LUMA_DPB_BUFFER_ALIGN_V6);
> @@ -112,10 +113,18 @@ static int s5p_mfc_alloc_codec_buffers_v6(struct
> s5p_mfc_ctx *ctx)
>  			(ctx->mv_count * ctx->mv_size);
>  		break;
>  	case S5P_MFC_CODEC_MPEG4_DEC:
> -		ctx->scratch_buf_size =
> -			S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(
> -					mb_width,
> -					mb_height);
> +		if (IS_MFCV7(dev)) {
> +			ctx->scratch_buf_size =
> +				S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V7(
> +						mb_width,
> +						mb_height);
> +		} else {
> +			ctx->scratch_buf_size =
> +				S5P_FIMV_SCRATCH_BUF_SIZE_MPEG4_DEC_V6(
> +						mb_width,
> +						mb_height);
> +		}
> +
>  		ctx->scratch_buf_size = ALIGN(ctx->scratch_buf_size,
>  				S5P_FIMV_SCRATCH_BUFFER_ALIGN_V6);
>  		ctx->bank1.size = ctx->scratch_buf_size; @@ -453,8 +462,13
> @@ static void s5p_mfc_set_enc_frame_buffer_v6(struct s5p_mfc_ctx *ctx,
> {
>  	struct s5p_mfc_dev *dev = ctx->dev;
> 
> -	WRITEL(y_addr, S5P_FIMV_E_SOURCE_LUMA_ADDR_V6); /* 256B align */
> -	WRITEL(c_addr, S5P_FIMV_E_SOURCE_CHROMA_ADDR_V6);
> +	if (IS_MFCV7(dev)) {
> +		WRITEL(y_addr, S5P_FIMV_E_SOURCE_FIRST_ADDR_V7);
> +		WRITEL(c_addr, S5P_FIMV_E_SOURCE_SECOND_ADDR_V7);
> +	} else {
> +		WRITEL(y_addr, S5P_FIMV_E_SOURCE_LUMA_ADDR_V6);
> +		WRITEL(c_addr, S5P_FIMV_E_SOURCE_CHROMA_ADDR_V6);
> +	}
> 
>  	mfc_debug(2, "enc src y buf addr: 0x%08lx\n", y_addr);
>  	mfc_debug(2, "enc src c buf addr: 0x%08lx\n", c_addr); @@ -466,8
> +480,13 @@ static void s5p_mfc_get_enc_frame_buffer_v6(struct
> s5p_mfc_ctx *ctx,
>  	struct s5p_mfc_dev *dev = ctx->dev;
>  	unsigned long enc_recon_y_addr, enc_recon_c_addr;
> 
> -	*y_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_LUMA_ADDR_V6);
> -	*c_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_CHROMA_ADDR_V6);
> +	if (IS_MFCV7(dev)) {
> +		*y_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_FIRST_ADDR_V7);
> +		*c_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_SECOND_ADDR_V7);
> +	} else {
> +		*y_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_LUMA_ADDR_V6);
> +		*c_addr = READL(S5P_FIMV_E_ENCODED_SOURCE_CHROMA_ADDR_V6);
> +	}
> 
>  	enc_recon_y_addr = READL(S5P_FIMV_E_RECON_LUMA_DPB_ADDR_V6);
>  	enc_recon_c_addr = READL(S5P_FIMV_E_RECON_CHROMA_DPB_ADDR_V6);
> @@ -1166,6 +1185,12 @@ static int s5p_mfc_init_decode_v6(struct
> s5p_mfc_ctx *ctx)
>  		reg |= (0x1 << S5P_FIMV_D_OPT_DDELAY_EN_SHIFT_V6);
>  		WRITEL(ctx->display_delay, S5P_FIMV_D_DISPLAY_DELAY_V6);
>  	}
> +
> +	if (IS_MFCV7(dev)) {
> +		WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6);
> +		reg = 0;
> +	}
> +
>  	/* Setup loop filter, for decoding this is only valid for MPEG4
> */
>  	if (ctx->codec_mode == S5P_MFC_CODEC_MPEG4_DEC) {
>  		mfc_debug(2, "Set loop filter to: %d\n", @@ -1176,7
> +1201,10 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx)
>  	if (ctx->dst_fmt->fourcc == V4L2_PIX_FMT_NV12MT_16X16)
>  		reg |= (0x1 << S5P_FIMV_D_OPT_TILE_MODE_SHIFT_V6);
> 
> -	WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6);
> +	if (IS_MFCV7(dev))
> +		WRITEL(reg, S5P_FIMV_D_INIT_BUFFER_OPTIONS_V7);
> +	else
> +		WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6);
> 
>  	/* 0: NV12(CbCr), 1: NV21(CrCb) */
>  	if (ctx->dst_fmt->fourcc == V4L2_PIX_FMT_NV21M) @@ -1184,6
> +1212,7 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx)
>  	else
>  		WRITEL(0x0, S5P_FIMV_PIXEL_FORMAT_V6);
> 
> +
>  	/* sei parse */
>  	WRITEL(ctx->sei_fp_parse & 0x1, S5P_FIMV_D_SEI_ENABLE_V6);
> 
> @@ -1254,6 +1283,12 @@ static int s5p_mfc_init_encode_v6(struct
> s5p_mfc_ctx *ctx)
>  		return -EINVAL;
>  	}
> 
> +	/* Set stride lengths */
> +	if (IS_MFCV7(dev)) {
> +		WRITEL(ctx->img_width, S5P_FIMV_E_SOURCE_FIRST_STRIDE_V7);
> +		WRITEL(ctx->img_width, S5P_FIMV_E_SOURCE_SECOND_STRIDE_V7);
> +	}
> +
>  	WRITEL(ctx->inst_no, S5P_FIMV_INSTANCE_ID_V6);
>  	s5p_mfc_hw_call(dev->mfc_cmds, cmd_host2risc, dev,
>  			S5P_FIMV_CH_SEQ_HEADER_V6, NULL);
> --
> 1.7.9.5

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

Re: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7

[Arun Kumar K]

Hi Sachin,

On Tue, Jun 25, 2013 at 4:54 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> Hi Arun,
>
>> @@ -684,5 +685,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
>>                                 (dev->variant->port_num ? 1 : 0) : 0) : 0)
>>  #define IS_TWOPORT(dev)                (dev->variant->port_num == 2 ? 1 : 0)
>>  #define IS_MFCV6_PLUS(dev)     (dev->variant->version >= 0x60 ? 1 : 0)
>> +#define IS_MFCV7(dev)          (dev->variant->version >= 0x70 ? 1 : 0)
>
> Considering the definition and pattern, wouldn't it be appropriate to
> call this  IS_MFCV7_PLUS?
>

We are still not sure about MFCv8 if it can re-use v7 stuff or not.

Regards
Arun

Re: [PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Hans Verkuil]

On Tue June 25 2013 12:57:14 Arun Kumar K wrote:
> This patch adds new V4L controls for VP8 encoding.
> 

FYI: I plan on reviewing this as soon as I have some time (should be this
weekend at the latest).

Regards,

	Hans

Re: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7

[Sachin Kamat]

Hi Arun,

On 26 June 2013 12:18, Arun Kumar K <arunkk.samsung@gmail.com> wrote:
> Hi Sachin,
>
> On Tue, Jun 25, 2013 at 4:54 PM, Sachin Kamat <sachin.kamat@linaro.org> wrote:
>> Hi Arun,
>>
>>> @@ -684,5 +685,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
>>>                                 (dev->variant->port_num ? 1 : 0) : 0) : 0)
>>>  #define IS_TWOPORT(dev)                (dev->variant->port_num == 2 ? 1 : 0)
>>>  #define IS_MFCV6_PLUS(dev)     (dev->variant->version >= 0x60 ? 1 : 0)
>>> +#define IS_MFCV7(dev)          (dev->variant->version >= 0x70 ? 1 : 0)
>>
>> Considering the definition and pattern, wouldn't it be appropriate to
>> call this  IS_MFCV7_PLUS?
>>
>
> We are still not sure about MFCv8 if it can re-use v7 stuff or not.
>

OK. In that case probably we can restrict the definition to
(dev->variant->version == 0x70 ? 1 : 0).


-- 
With warm regards,
Sachin

RE: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7

[Kamil Debski]

Hi,

> -----Original Message-----
> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
> Sent: Wednesday, June 26, 2013 9:05 AM
> To: Arun Kumar K
> Cc: Arun Kumar K; LMML; Kamil Debski; jtp.park@samsung.com; Sylwester
> Nawrocki; Hans Verkuil; avnd.kiran@samsung.com
> Subject: Re: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7
> 
> Hi Arun,
> 
> On 26 June 2013 12:18, Arun Kumar K <arunkk.samsung@gmail.com> wrote:
> > Hi Sachin,
> >
> > On Tue, Jun 25, 2013 at 4:54 PM, Sachin Kamat
> <sachin.kamat@linaro.org> wrote:
> >> Hi Arun,
> >>
> >>> @@ -684,5 +685,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx
> *ctx);
> >>>                                 (dev->variant->port_num ? 1 : 0) :
> 0) : 0)
> >>>  #define IS_TWOPORT(dev)                (dev->variant->port_num ==
> 2 ? 1 : 0)
> >>>  #define IS_MFCV6_PLUS(dev)     (dev->variant->version >= 0x60 ? 1 :
> 0)
> >>> +#define IS_MFCV7(dev)          (dev->variant->version >= 0x70 ? 1 :
> 0)
> >>
> >> Considering the definition and pattern, wouldn't it be appropriate
> to
> >> call this  IS_MFCV7_PLUS?
> >>
> >
> > We are still not sure about MFCv8 if it can re-use v7 stuff or not.
> >
> 
> OK. In that case probably we can restrict the definition to (dev-
> >variant->version == 0x70 ? 1 : 0).
> 
> 

Guys, I think that simple ((dev->variant->version & 0xF0) == 0x70) would
cover
every 7.x version. Same could apply to versions 6.x and 5.x. 
Then instead of using IS_MFCV6_PLUS(dev) one would use IS_MFCV6(dev) ||
IS_MFCV7(dev).
This is a bit longer, but if version 8 will be totally different from v7
then it is
much better to use v6||v7 instead of v6_plus.

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

Re: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7

[Arun Kumar K]

Hi Kamil,

On Wed, Jun 26, 2013 at 1:50 PM, Kamil Debski <k.debski@samsung.com> wrote:
> Hi,
>
>> -----Original Message-----
>> From: Sachin Kamat [mailto:sachin.kamat@linaro.org]
>> Sent: Wednesday, June 26, 2013 9:05 AM
>> To: Arun Kumar K
>> Cc: Arun Kumar K; LMML; Kamil Debski; jtp.park@samsung.com; Sylwester
>> Nawrocki; Hans Verkuil; avnd.kiran@samsung.com
>> Subject: Re: [PATCH v3 4/8] [media] s5p-mfc: Core support for MFC v7
>>
>> Hi Arun,
>>
>> On 26 June 2013 12:18, Arun Kumar K <arunkk.samsung@gmail.com> wrote:
>> > Hi Sachin,
>> >
>> > On Tue, Jun 25, 2013 at 4:54 PM, Sachin Kamat
>> <sachin.kamat@linaro.org> wrote:
>> >> Hi Arun,
>> >>
>> >>> @@ -684,5 +685,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx
>> *ctx);
>> >>>                                 (dev->variant->port_num ? 1 : 0) :
>> 0) : 0)
>> >>>  #define IS_TWOPORT(dev)                (dev->variant->port_num ==
>> 2 ? 1 : 0)
>> >>>  #define IS_MFCV6_PLUS(dev)     (dev->variant->version >= 0x60 ? 1 :
>> 0)
>> >>> +#define IS_MFCV7(dev)          (dev->variant->version >= 0x70 ? 1 :
>> 0)
>> >>
>> >> Considering the definition and pattern, wouldn't it be appropriate
>> to
>> >> call this  IS_MFCV7_PLUS?
>> >>
>> >
>> > We are still not sure about MFCv8 if it can re-use v7 stuff or not.
>> >
>>
>> OK. In that case probably we can restrict the definition to (dev-
>> >variant->version == 0x70 ? 1 : 0).
>>
>>
>
> Guys, I think that simple ((dev->variant->version & 0xF0) == 0x70) would
> cover
> every 7.x version. Same could apply to versions 6.x and 5.x.
> Then instead of using IS_MFCV6_PLUS(dev) one would use IS_MFCV6(dev) ||
> IS_MFCV7(dev).
> This is a bit longer, but if version 8 will be totally different from v7
> then it is
> much better to use v6||v7 instead of v6_plus.
>

If v8 was known to be different from v7, then we can go without second
thoughts about v6 || v7 check. But we cannot confirm that until v8 is
released. There is a good possibility of v6_plus macro holding good
for v8 also.
So as per current understanding, we can restrict v7 macro to only v7.x FW.
When v8 is released, we will change the name and usage of this macro if needed.
Is that ok?

Regards
Arun

Re: [PATCH v3 5/8] [media] s5p-mfc: Update driver for v7 firmware

[Arun Kumar K]

Hi Kamil,

On Tue, Jun 25, 2013 at 7:33 PM, Kamil Debski <k.debski@samsung.com> wrote:
> Hi Arun,
>
> This patch sets looks very good.
> Please check my comments below.
>
>> From: Arun Kumar K [mailto:arun.kk@samsung.com]
>>
>> Firmware version v7 is mostly similar to v6 in terms of hardware
>> specific controls and commands. So the hardware specific opr_v6 and
>> cmd_v6 are re-used for v7 also. This patch updates the v6 files to
>> handle v7 version also.
>>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
>>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c    |   11 ++++-
>>  drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c |   53
>> +++++++++++++++++++----
>>  2 files changed, 53 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
>> b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
>> index f734ccc..dd57b06 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
>> @@ -1663,8 +1663,15 @@ static int s5p_mfc_queue_setup(struct vb2_queue
>> *vq,
>>               if (*buf_count > MFC_MAX_BUFFERS)
>>                       *buf_count = MFC_MAX_BUFFERS;
>>
>> -             psize[0] = ctx->luma_size;
>> -             psize[1] = ctx->chroma_size;
>> +             if (IS_MFCV7(dev)) {
>> +                     /* MFCv7 needs pad bytes for input YUV */
>> +                     psize[0] = ctx->luma_size + 256;
>> +                     psize[1] = ctx->chroma_size + 128;
>
> Here, I would add some define name to avoid magic number.
> Secondly, why wasn't this padding added in s5p_mfc_enc_calc_src_size_v6/v7?
> enc_calc_src_size is called in s_fmt, so it seems the best place to adjust
> buffer/plane sizes.
>

Yes that seems to be the right place to make this change. Will change it
in next version.

Regards
Arun

Re: [PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Arun Kumar K]

Hi Kamil,

>> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
>> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
>> @@ -424,6 +424,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>>               NULL,
>>       };
>>
>> +     static const char * const vpx_golden_frame_sel[] = {
>> +             "Use Previous Frame",
>> +             "Use Previous Specific Frame",
>
> "Use Previous Specific Frame" seems unclear.
> Maybe "Use Previous Golden Frame" or "Use Periodic Golden Frame"?
> I'm not sure if I get the description right.
>

"Use Periodic Golden Frame" seems more reasonable. WIll change it.

Regards
Arun

Re: [PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Arun Kumar K]

Hi Hans,

On Wed, Jun 26, 2013 at 12:23 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On Tue June 25 2013 12:57:14 Arun Kumar K wrote:
>> This patch adds new V4L controls for VP8 encoding.
>>
>
> FYI: I plan on reviewing this as soon as I have some time (should be this
> weekend at the latest).
>

Thank you. Will wait for you review before posting next version.

Regards
Arun

Re: [PATCH v3 6/8] [media] V4L: Add support for integer menu controls with standard menu items

[Hans Verkuil]

On Tue June 25 2013 12:57:13 Arun Kumar K wrote:
> From: Sylwester Nawrocki <s.nawrocki@samsung.com>
> 
> The patch modifies the helper function v4l2_ctrl_new_std_menu
> to accept integer menu controls with standard menu items.
> 
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>

After fixing the very minor correction below you have my ack:

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

Regards,

	Hans

> ---
>  Documentation/video4linux/v4l2-controls.txt |   21 ++++++++++----------
>  drivers/media/v4l2-core/v4l2-ctrls.c        |   28 ++++++++++++++++++++++++---
>  2 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/video4linux/v4l2-controls.txt b/Documentation/video4linux/v4l2-controls.txt
> index 676f873..e06e768 100644
> --- a/Documentation/video4linux/v4l2-controls.txt
> +++ b/Documentation/video4linux/v4l2-controls.txt
> @@ -124,26 +124,27 @@ You add non-menu controls by calling v4l2_ctrl_new_std:
>  			const struct v4l2_ctrl_ops *ops,
>  			u32 id, s32 min, s32 max, u32 step, s32 def);
>  
> -Menu controls are added by calling v4l2_ctrl_new_std_menu:
> +Menu and integer menu controls are added by calling v4l2_ctrl_new_std_menu:
>  
>  	struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
>  			const struct v4l2_ctrl_ops *ops,
>  			u32 id, s32 max, s32 skip_mask, s32 def);
>  
> -Or alternatively for integer menu controls, by calling v4l2_ctrl_new_int_menu:
> +Menu controls with a driver specific menu are added by calling
> +v4l2_ctrl_new_std_menu_items:
> +
> +       struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(
> +                       struct v4l2_ctrl_handler *hdl,
> +                       const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
> +                       s32 skip_mask, s32 def, const char * const *qmenu);
> +
> +Integer menu controls with driver specific menu can be added by calling

s/with driver/with a driver/

> +v4l2_ctrl_new_int_menu:
>  
>  	struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct v4l2_ctrl_handler *hdl,
>  			const struct v4l2_ctrl_ops *ops,
>  			u32 id, s32 max, s32 def, const s64 *qmenu_int);
>  
> -Standard menu controls with a driver specific menu are added by calling
> -v4l2_ctrl_new_std_menu_items:
> -
> -	struct v4l2_ctrl *v4l2_ctrl_new_std_menu_items(
> -		struct v4l2_ctrl_handler *hdl,
> -		const struct v4l2_ctrl_ops *ops, u32 id, s32 max,
> -		s32 skip_mask, s32 def, const char * const *qmenu);
> -
>  These functions are typically called right after the v4l2_ctrl_handler_init:
>  
>  	static const s64 exp_bias_qmenu[] = {
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index fccd08b..e03a2e8 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -552,6 +552,20 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_get_menu);
>  
> +/*
> + * Returns NULL or an s64 type array containing the menu for given
> + * control ID. The total number of the menu items is returned in @len.
> + */
> +const s64 const *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
> +{
> +	switch (id) {
> +	default:
> +		*len = 0;
> +		return NULL;
> +	};
> +}
> +EXPORT_SYMBOL(v4l2_ctrl_get_int_menu);
> +
>  /* Return the control name. */
>  const char *v4l2_ctrl_get_name(u32 id)
>  {
> @@ -1712,20 +1726,28 @@ struct v4l2_ctrl *v4l2_ctrl_new_std_menu(struct v4l2_ctrl_handler *hdl,
>  			const struct v4l2_ctrl_ops *ops,
>  			u32 id, s32 max, s32 mask, s32 def)
>  {
> -	const char * const *qmenu = v4l2_ctrl_get_menu(id);
> +	const char * const *qmenu = NULL;
> +	const s64 *qmenu_int = NULL;
>  	const char *name;
>  	enum v4l2_ctrl_type type;
> +	unsigned int qmenu_int_len;
>  	s32 min;
>  	s32 step;
>  	u32 flags;
>  
>  	v4l2_ctrl_fill(id, &name, &type, &min, &max, &step, &def, &flags);
> -	if (type != V4L2_CTRL_TYPE_MENU) {
> +
> +	if (type == V4L2_CTRL_TYPE_MENU)
> +		qmenu = v4l2_ctrl_get_menu(id);
> +	else if (type == V4L2_CTRL_TYPE_INTEGER_MENU)
> +		qmenu_int = v4l2_ctrl_get_int_menu(id, &qmenu_int_len);
> +
> +	if ((!qmenu && !qmenu_int) || (qmenu_int && max > qmenu_int_len)) {
>  		handler_set_err(hdl, -EINVAL);
>  		return NULL;
>  	}
>  	return v4l2_ctrl_new(hdl, ops, id, name, type,
> -			     0, max, mask, def, flags, qmenu, NULL, NULL);
> +			     0, max, mask, def, flags, qmenu, qmenu_int, NULL);
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_new_std_menu);
>  
> 

Re: [PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Hans Verkuil]

Hi Arun,

As promised, here is my review.

I have been thinking a bit more about whether or not a VPX control class
should be added, and in my opinion it shouldn't. These controls should be
part of the MPEG control class, as the VPX encoder shares a lot of general
encoding parameters, just like h264 and mpeg4.

It is unfortunate that all the defines contain the MPEG name, and I take
the blame for that since I came up with these defines originally.

That said, there are some things that can be done to make it less confusing:

- Clearly state in v4l2-controls.h and v4l2-ctrls.c that the MPEG controls
  are really Codec Controls, so not MPEG specific, and that the 'MPEG' part of
  the define is historical.

- Currently the V4L2_CID_MPEG_CLASS name in v4l2-ctrls.c is "MPEG Encoder Controls".
  This should be changed to "Codec Controls", since the controls in this class are
  neither MPEG specific, nor are they encoder specific as there are also controls
  related to the decoder.

- Update the DocBook section for the MPEG controls accordingly: change 'MPEG' in
  the text to 'Codec' and add a note explaining why all the defines are prefixed
  with V4L2_CID_MPEG/V4L2_MPEG instead of _CODEC.

I did toy with the idea of adding aliases in v4l2-controls.h replacing MPEG with
CODEC, but that really is too messy. I think if you can take care of the three
points mentioned above we should be OK.

This also means that in this patch the V4L2_CID_VPX_ prefix changes to
V4L2_CID_MPEG_VIDEO_VPX_ as that is consistent with the current naming convention
in v4l2-controls.h: V4L2_CID_MPEG_VIDEO_H264_ASO, V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL.

Enums use V4L2_MPEG_VIDEO_VPX_ prefix.

Yes, I know, this will make the names quite a bit longer, but it is important for
consistency. Codecs are likely to have lots of controls since there are lots of
knobs you can tweak. So using a systematic naming scheme will prevent it from
descending into chaos...

On Tue June 25 2013 12:57:14 Arun Kumar K wrote:
> This patch adds new V4L controls for VP8 encoding.
> 
> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
> ---
>  Documentation/DocBook/media/v4l/controls.xml |  150 ++++++++++++++++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c         |   33 ++++++
>  include/uapi/linux/v4l2-controls.h           |   29 ++++-
>  3 files changed, 210 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
> index 8d7a779..736c991 100644
> --- a/Documentation/DocBook/media/v4l/controls.xml
> +++ b/Documentation/DocBook/media/v4l/controls.xml
> @@ -3009,6 +3009,156 @@ in by the application. 0 = do not insert, 1 = insert packets.</entry>
>  	  </tgroup>
>  	</table>
>        </section>
> +
> +    <section>
> +      <title>VPX Control Reference</title>
> +
> +      <para>The VPX controls include controls for encoding parameters
> +      of VPx video codec.</para>
> +
> +      <table pgwide="1" frame="none" id="vpx-control-id">
> +      <title>VPX Control IDs</title>
> +
> +      <tgroup cols="4">
> +        <colspec colname="c1" colwidth="1*" />
> +        <colspec colname="c2" colwidth="6*" />
> +        <colspec colname="c3" colwidth="2*" />
> +        <colspec colname="c4" colwidth="6*" />
> +        <spanspec namest="c1" nameend="c2" spanname="id" />
> +        <spanspec namest="c2" nameend="c4" spanname="descr" />
> +        <thead>
> +          <row>
> +            <entry spanname="id" align="left">ID</entry>
> +            <entry align="left">Type</entry>
> +          </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
> +          </row>
> +        </thead>
> +        <tbody valign="top">
> +          <row><entry></entry></row>
> +
> +	      <row><entry></entry></row>
> +	      <row id="v4l2-vpx-num-partitions">
> +		<entry spanname="id"><constant>V4L2_CID_VPX_NUM_PARTITIONS</constant></entry>
> +		<entry>enum v4l2_vp8_num_partitions</entry>
> +	      </row>
> +	      <row><entry spanname="descr">The number of token partitions to use in VP8 encoder.
> +Possible values are:</entry>
> +	      </row>
> +	      <row>
> +		<entrytbl spanname="descr" cols="2">
> +		  <tbody valign="top">
> +		    <row>
> +		      <entry><constant>V4L2_VPX_1_PARTITION</constant></entry>
> +		      <entry>1 coefficient partition</entry>
> +		    </row>
> +		    <row>
> +		      <entry><constant>V4L2_VPX_2_PARTITIONS</constant></entry>
> +		      <entry>2 partitions</entry>

Add 'coefficient' for the other cases as well in the description. At least, I think
this should be '2 coefficient partitions'.

> +		    </row>
> +		    <row>
> +		      <entry><constant>V4L2_VPX_4_PARTITIONS</constant></entry>
> +		      <entry>4 partitions</entry>
> +		    </row>
> +		    <row>
> +		      <entry><constant>V4L2_VPX_8_PARTITIONS</constant></entry>
> +		      <entry>8 partitions</entry>
> +	            </row>
> +                  </tbody>
> +		</entrytbl>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry spanname="id"><constant>V4L2_CID_VPX_IMD_DISABLE_4X4</constant></entry>
> +		<entry>boolean</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Setting this prevents intra 4x4 mode in the intra mode decision.</entry>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row id="v4l2-vpx-num-ref-frames">
> +		<entry spanname="id"><constant>V4L2_CID_VPX_NUM_REF_FRAMES</constant></entry>
> +		<entry>enum v4l2_vp8_num_ref_frames</entry>
> +	      </row>
> +	      <row><entry spanname="descr">The number of reference pictures for encoding P frames.
> +Possible values are:</entry>
> +	      </row>
> +	      <row>
> +		<entrytbl spanname="descr" cols="2">
> +		  <tbody valign="top">
> +		    <row>
> +		      <entry><constant>V4L2_VPX_1_REF_FRAME</constant></entry>
> +		      <entry>Last encoded frame will be searched</entry>
> +		    </row>
> +		    <row>
> +		      <entry><constant>V4L2_VPX_2_REF_FRAME</constant></entry>
> +		      <entry>Two frames would be searched among last encoded frame, golden frame

s/would/will/
s/among/among the/
s/golden/the golden/

> +and altref frame. Encoder implementation can decide which two are chosen.</entry>

s/altref/the altref/
s/Encoder/The encoder/
s/can/will/

Perhaps instead of writing 'altref' it should be 'alternate reference'? (At least, I assume
that's what altref is short for).

> +		    </row>
> +		    <row>
> +		      <entry><constant>V4L2_VPX_3_REF_FRAME</constant></entry>
> +		      <entry>The last encoded frame, golden frame and altref frame will be searched.</entry>

s/golden/the golden/
s/altref/the altref/

> +		    </row>
> +                  </tbody>
> +		</entrytbl>
> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry spanname="id"><constant>V4L2_CID_VPX_FILTER_LEVEL</constant></entry>
> +		<entry>integer</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Indicates the loop filter level. The adjustment of loop

s/of loop/of the loop/

> +filter level is done via a delta value against a baseline loop filter value.</entry>

Is that baseline loop filter value implementation specific, or is it defined by the standard?

> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry spanname="id"><constant>V4L2_CID_VPX_FILTER_SHARPNESS</constant></entry>
> +		<entry>integer</entry>
> +	      </row>
> +	      <row><entry spanname="descr">This parameter affects the loop filter. Anything above
> +zero weakens the deblocking effect on loop filter.</entry>

s/loop/the loop/

> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row>
> +		<entry spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD</constant></entry>
> +		<entry>integer</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Sets the refresh period for golden frame. Period is defined

s/golden/the golden/
s/Period/The period/

> +in number of frames. For a value of 'n', every nth frame will be taken as golden frame.</entry>

So for a period of, say, 4, what does that mean in practice? For example, I start encoding and
give you the first 8 frames: 0, 1, 2, 3, 4, 5, 6 and 7.

Will frames 0 and 4 be marked as golden frames, or 3 and 7? Your documentation suggests the
latter, but I'm not really sure that is what you meant.

> +	      </row>
> +
> +	      <row><entry></entry></row>
> +	      <row id="v4l2-vpx-golden-frame-sel">
> +		<entry spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_SEL</constant></entry>
> +		<entry>enum v4l2_vp8_golden_frame_sel</entry>
> +	      </row>
> +	      <row><entry spanname="descr">Selects the golden frame for encoding.
> +Possible values are:</entry>
> +	      </row>
> +	      <row>
> +		<entrytbl spanname="descr" cols="2">
> +		  <tbody valign="top">
> +		    <row>
> +		      <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_PREV</constant></entry>
> +		      <entry>Use the last to last or (n-2)th frame as a golden frame. Current frame index being 'n'.</entry>

"last to last" doesn't parse. Just use:

"Use the (n-2)th frame as a golden frame, the current frame index being 'n'."

That's unambiguous.

> +		    </row>
> +		    <row>
> +		      <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD</constant></entry>
> +		      <entry>Use the previous specific frame indicated by V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD as a golden frame</entry>

s/golden frame/golden frame./

> +		    </row>
> +                  </tbody>
> +		</entrytbl>
> +	      </row>
> +
> +          <row><entry></entry></row>
> +        </tbody>
> +      </tgroup>
> +      </table>
> +
> +      </section>
>      </section>
>  
>      <section id="camera-controls">
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index e03a2e8..891f595 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -424,6 +424,12 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		NULL,
>  	};
>  
> +	static const char * const vpx_golden_frame_sel[] = {
> +		"Use Previous Frame",
> +		"Use Previous Specific Frame",
> +		NULL,
> +	};
> +
>  	static const char * const flash_led_mode[] = {
>  		"Off",
>  		"Flash",
> @@ -538,6 +544,8 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  		return mpeg_mpeg4_level;
>  	case V4L2_CID_MPEG_VIDEO_MPEG4_PROFILE:
>  		return mpeg4_profile;
> +	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
> +		return vpx_golden_frame_sel;
>  	case V4L2_CID_JPEG_CHROMA_SUBSAMPLING:
>  		return jpeg_chroma_subsampling;
>  	case V4L2_CID_DV_TX_MODE:
> @@ -552,13 +560,26 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
>  }
>  EXPORT_SYMBOL(v4l2_ctrl_get_menu);
>  
> +#define __v4l2_qmenu_int_len(arr, len) ({ *(len) = ARRAY_SIZE(arr); arr; })
>  /*
>   * Returns NULL or an s64 type array containing the menu for given
>   * control ID. The total number of the menu items is returned in @len.
>   */
>  const s64 const *v4l2_ctrl_get_int_menu(u32 id, u32 *len)
>  {
> +	static const s64 const qmenu_int_vpx_num_partitions[] = {
> +		1, 2, 4, 8,
> +	};
> +
> +	static const s64 const qmenu_int_vpx_num_ref_frames[] = {
> +		1, 2, 3,
> +	};
> +
>  	switch (id) {
> +	case V4L2_CID_VPX_NUM_PARTITIONS:
> +		return __v4l2_qmenu_int_len(qmenu_int_vpx_num_partitions, len);
> +	case V4L2_CID_VPX_NUM_REF_FRAMES:
> +		return __v4l2_qmenu_int_len(qmenu_int_vpx_num_ref_frames, len);
>  	default:
>  		*len = 0;
>  		return NULL;
> @@ -714,6 +735,15 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_MPEG_VIDEO_VBV_DELAY:			return "Initial Delay for VBV Control";
>  	case V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER:		return "Repeat Sequence Header";
>  
> +	/* VPX controls */
> +	case V4L2_CID_VPX_NUM_PARTITIONS:			return "VPX Number of partitions";

The naming should use consistent upper case rules like the other controls. So
this becomes: "VPX Number of Partitions", etc.

> +	case V4L2_CID_VPX_IMD_DISABLE_4X4:			return "VPX Intra mode decision disable";
> +	case V4L2_CID_VPX_NUM_REF_FRAMES:			return "VPX No. of refs for P frame";
> +	case V4L2_CID_VPX_FILTER_LEVEL:				return "VPX Loop filter level range";
> +	case V4L2_CID_VPX_FILTER_SHARPNESS:			return "VPX Deblocking effect control";
> +	case V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD:		return "VPX Golden frame refresh period";
> +	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:			return "VPX Golden frame indicator";

Same for all these controls.

> +
>  	/* CAMERA controls */
>  	/* Keep the order of the 'case's the same as in videodev2.h! */
>  	case V4L2_CID_CAMERA_CLASS:		return "Camera Controls";
> @@ -928,6 +958,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_DV_RX_RGB_RANGE:
>  	case V4L2_CID_TEST_PATTERN:
>  	case V4L2_CID_TUNE_DEEMPHASIS:
> +	case V4L2_CID_VPX_GOLDEN_FRAME_SEL:
>  		*type = V4L2_CTRL_TYPE_MENU;
>  		break;
>  	case V4L2_CID_LINK_FREQ:
> @@ -939,6 +970,8 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  		break;
>  	case V4L2_CID_ISO_SENSITIVITY:
>  	case V4L2_CID_AUTO_EXPOSURE_BIAS:
> +	case V4L2_CID_VPX_NUM_PARTITIONS:
> +	case V4L2_CID_VPX_NUM_REF_FRAMES:
>  		*type = V4L2_CTRL_TYPE_INTEGER_MENU;
>  		break;
>  	case V4L2_CID_USER_CLASS:
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 69bd5bb..8d6ffc9 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -522,6 +522,33 @@ enum v4l2_mpeg_video_mpeg4_profile {
>  };
>  #define V4L2_CID_MPEG_VIDEO_MPEG4_QPEL		(V4L2_CID_MPEG_BASE+407)
>  
> +/*  Control IDs for VP8 streams
> + *  Though VP8 is not part of MPEG, adding it here as MPEG class is
> + *  already handling other video compression standards
> + */
> +#define V4L2_CID_VPX_NUM_PARTITIONS		(V4L2_CID_MPEG_BASE+500)
> +enum v4l2_vp8_num_partitions {
> +	V4L2_VPX_1_PARTITION	= 0,
> +	V4L2_VPX_2_PARTITIONS	= 1,
> +	V4L2_VPX_4_PARTITIONS	= 2,
> +	V4L2_VPX_8_PARTITIONS	= 3,
> +};
> +#define V4L2_CID_VPX_IMD_DISABLE_4X4		(V4L2_CID_MPEG_BASE+501)
> +#define V4L2_CID_VPX_NUM_REF_FRAMES		(V4L2_CID_MPEG_BASE+502)
> +enum v4l2_vp8_num_ref_frames {
> +	V4L2_VPX_1_REF_FRAME	= 0,
> +	V4L2_VPX_2_REF_FRAME	= 1,
> +	V4L2_VPX_3_REF_FRAME	= 2,
> +};
> +#define V4L2_CID_VPX_FILTER_LEVEL		(V4L2_CID_MPEG_BASE+503)
> +#define V4L2_CID_VPX_FILTER_SHARPNESS		(V4L2_CID_MPEG_BASE+504)
> +#define V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD	(V4L2_CID_MPEG_BASE+505)
> +#define V4L2_CID_VPX_GOLDEN_FRAME_SEL		(V4L2_CID_MPEG_BASE+506)
> +enum v4l2_vp8_golden_frame_sel {
> +	V4L2_VPX_GOLDEN_FRAME_USE_PREV		= 0,
> +	V4L2_VPX_GOLDEN_FRAME_USE_REF_PERIOD	= 1,
> +};
> +
>  /*  MPEG-class control IDs specific to the CX2341x driver as defined by V4L2 */
>  #define V4L2_CID_MPEG_CX2341X_BASE 				(V4L2_CTRL_CLASS_MPEG | 0x1000)
>  #define V4L2_CID_MPEG_CX2341X_VIDEO_SPATIAL_FILTER_MODE 	(V4L2_CID_MPEG_CX2341X_BASE+0)
> @@ -590,7 +617,6 @@ enum v4l2_mpeg_mfc51_video_force_frame_type {
>  #define V4L2_CID_MPEG_MFC51_VIDEO_H264_ADAPTIVE_RC_STATIC		(V4L2_CID_MPEG_MFC51_BASE+53)
>  #define V4L2_CID_MPEG_MFC51_VIDEO_H264_NUM_REF_PIC_FOR_P		(V4L2_CID_MPEG_MFC51_BASE+54)
>  
> -
>  /*  Camera class control IDs */
>  
>  #define V4L2_CID_CAMERA_CLASS_BASE 	(V4L2_CTRL_CLASS_CAMERA | 0x900)
> @@ -818,7 +844,6 @@ enum v4l2_jpeg_chroma_subsampling {
>  #define V4L2_CID_PIXEL_RATE			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 2)
>  #define V4L2_CID_TEST_PATTERN			(V4L2_CID_IMAGE_PROC_CLASS_BASE + 3)
>  
> -
>  /*  DV-class control IDs defined by V4L2 */
>  #define V4L2_CID_DV_CLASS_BASE			(V4L2_CTRL_CLASS_DV | 0x900)
>  #define V4L2_CID_DV_CLASS			(V4L2_CTRL_CLASS_DV | 1)
> 

Regards,

	Hans

Re: [PATCH v3 7/8] [media] V4L: Add VP8 encoder controls

[Arun Kumar K]

Hi Hans

On Fri, Jun 28, 2013 at 7:55 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> Hi Arun,
>
> As promised, here is my review.
>

Thank you :). Sorry for the delay in response.

> I have been thinking a bit more about whether or not a VPX control class
> should be added, and in my opinion it shouldn't. These controls should be
> part of the MPEG control class, as the VPX encoder shares a lot of general
> encoding parameters, just like h264 and mpeg4.
>
> It is unfortunate that all the defines contain the MPEG name, and I take
> the blame for that since I came up with these defines originally.
>
> That said, there are some things that can be done to make it less confusing:
>
> - Clearly state in v4l2-controls.h and v4l2-ctrls.c that the MPEG controls
>   are really Codec Controls, so not MPEG specific, and that the 'MPEG' part of
>   the define is historical.

Ok will do that.

>
> - Currently the V4L2_CID_MPEG_CLASS name in v4l2-ctrls.c is "MPEG Encoder Controls".
>   This should be changed to "Codec Controls", since the controls in this class are
>   neither MPEG specific, nor are they encoder specific as there are also controls
>   related to the decoder.
>
> - Update the DocBook section for the MPEG controls accordingly: change 'MPEG' in
>   the text to 'Codec' and add a note explaining why all the defines are prefixed
>   with V4L2_CID_MPEG/V4L2_MPEG instead of _CODEC.
>

Ok will do these changes.

> I did toy with the idea of adding aliases in v4l2-controls.h replacing MPEG with
> CODEC, but that really is too messy. I think if you can take care of the three
> points mentioned above we should be OK.
>
> This also means that in this patch the V4L2_CID_VPX_ prefix changes to
> V4L2_CID_MPEG_VIDEO_VPX_ as that is consistent with the current naming convention
> in v4l2-controls.h: V4L2_CID_MPEG_VIDEO_H264_ASO, V4L2_CID_MPEG_VIDEO_MPEG4_LEVEL.
>

Ok.

> Enums use V4L2_MPEG_VIDEO_VPX_ prefix.
>
> Yes, I know, this will make the names quite a bit longer, but it is important for
> consistency. Codecs are likely to have lots of controls since there are lots of
> knobs you can tweak. So using a systematic naming scheme will prevent it from
> descending into chaos...
>
> On Tue June 25 2013 12:57:14 Arun Kumar K wrote:
>> This patch adds new V4L controls for VP8 encoding.
>>
>> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
>>  Documentation/DocBook/media/v4l/controls.xml |  150 ++++++++++++++++++++++++++
>>  drivers/media/v4l2-core/v4l2-ctrls.c         |   33 ++++++
>>  include/uapi/linux/v4l2-controls.h           |   29 ++++-
>>  3 files changed, 210 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/DocBook/media/v4l/controls.xml b/Documentation/DocBook/media/v4l/controls.xml
>> index 8d7a779..736c991 100644
>> --- a/Documentation/DocBook/media/v4l/controls.xml
>> +++ b/Documentation/DocBook/media/v4l/controls.xml
>> @@ -3009,6 +3009,156 @@ in by the application. 0 = do not insert, 1 = insert packets.</entry>
>>         </tgroup>
>>       </table>
>>        </section>
>> +
>> +    <section>
>> +      <title>VPX Control Reference</title>
>> +
>> +      <para>The VPX controls include controls for encoding parameters
>> +      of VPx video codec.</para>
>> +
>> +      <table pgwide="1" frame="none" id="vpx-control-id">
>> +      <title>VPX Control IDs</title>
>> +
>> +      <tgroup cols="4">
>> +        <colspec colname="c1" colwidth="1*" />
>> +        <colspec colname="c2" colwidth="6*" />
>> +        <colspec colname="c3" colwidth="2*" />
>> +        <colspec colname="c4" colwidth="6*" />
>> +        <spanspec namest="c1" nameend="c2" spanname="id" />
>> +        <spanspec namest="c2" nameend="c4" spanname="descr" />
>> +        <thead>
>> +          <row>
>> +            <entry spanname="id" align="left">ID</entry>
>> +            <entry align="left">Type</entry>
>> +          </row><row rowsep="1"><entry spanname="descr" align="left">Description</entry>
>> +          </row>
>> +        </thead>
>> +        <tbody valign="top">
>> +          <row><entry></entry></row>
>> +
>> +           <row><entry></entry></row>
>> +           <row id="v4l2-vpx-num-partitions">
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_NUM_PARTITIONS</constant></entry>
>> +             <entry>enum v4l2_vp8_num_partitions</entry>
>> +           </row>
>> +           <row><entry spanname="descr">The number of token partitions to use in VP8 encoder.
>> +Possible values are:</entry>
>> +           </row>
>> +           <row>
>> +             <entrytbl spanname="descr" cols="2">
>> +               <tbody valign="top">
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_1_PARTITION</constant></entry>
>> +                   <entry>1 coefficient partition</entry>
>> +                 </row>
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_2_PARTITIONS</constant></entry>
>> +                   <entry>2 partitions</entry>
>
> Add 'coefficient' for the other cases as well in the description. At least, I think
> this should be '2 coefficient partitions'.
>

Ok.

>> +                 </row>
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_4_PARTITIONS</constant></entry>
>> +                   <entry>4 partitions</entry>
>> +                 </row>
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_8_PARTITIONS</constant></entry>
>> +                   <entry>8 partitions</entry>
>> +                 </row>
>> +                  </tbody>
>> +             </entrytbl>
>> +           </row>
>> +
>> +           <row><entry></entry></row>
>> +           <row>
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_IMD_DISABLE_4X4</constant></entry>
>> +             <entry>boolean</entry>
>> +           </row>
>> +           <row><entry spanname="descr">Setting this prevents intra 4x4 mode in the intra mode decision.</entry>
>> +           </row>
>> +
>> +           <row><entry></entry></row>
>> +           <row id="v4l2-vpx-num-ref-frames">
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_NUM_REF_FRAMES</constant></entry>
>> +             <entry>enum v4l2_vp8_num_ref_frames</entry>
>> +           </row>
>> +           <row><entry spanname="descr">The number of reference pictures for encoding P frames.
>> +Possible values are:</entry>
>> +           </row>
>> +           <row>
>> +             <entrytbl spanname="descr" cols="2">
>> +               <tbody valign="top">
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_1_REF_FRAME</constant></entry>
>> +                   <entry>Last encoded frame will be searched</entry>
>> +                 </row>
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_2_REF_FRAME</constant></entry>
>> +                   <entry>Two frames would be searched among last encoded frame, golden frame
>
> s/would/will/
> s/among/among the/
> s/golden/the golden/
>
>> +and altref frame. Encoder implementation can decide which two are chosen.</entry>
>
> s/altref/the altref/
> s/Encoder/The encoder/
> s/can/will/
>
> Perhaps instead of writing 'altref' it should be 'alternate reference'? (At least, I assume
> that's what altref is short for).
>

Yes altref is the shortform for alternate reference. But the name altref is used
everywhere in the VP8 standard.

>> +                 </row>
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_3_REF_FRAME</constant></entry>
>> +                   <entry>The last encoded frame, golden frame and altref frame will be searched.</entry>
>
> s/golden/the golden/
> s/altref/the altref/
>
>> +                 </row>
>> +                  </tbody>
>> +             </entrytbl>
>> +           </row>
>> +
>> +           <row><entry></entry></row>
>> +           <row>
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_FILTER_LEVEL</constant></entry>
>> +             <entry>integer</entry>
>> +           </row>
>> +           <row><entry spanname="descr">Indicates the loop filter level. The adjustment of loop
>
> s/of loop/of the loop/
>
>> +filter level is done via a delta value against a baseline loop filter value.</entry>
>
> Is that baseline loop filter value implementation specific, or is it defined by the standard?
>

It is defined by the standard.

>> +           </row>
>> +
>> +           <row><entry></entry></row>
>> +           <row>
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_FILTER_SHARPNESS</constant></entry>
>> +             <entry>integer</entry>
>> +           </row>
>> +           <row><entry spanname="descr">This parameter affects the loop filter. Anything above
>> +zero weakens the deblocking effect on loop filter.</entry>
>
> s/loop/the loop/
>
>> +           </row>
>> +
>> +           <row><entry></entry></row>
>> +           <row>
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_REF_PERIOD</constant></entry>
>> +             <entry>integer</entry>
>> +           </row>
>> +           <row><entry spanname="descr">Sets the refresh period for golden frame. Period is defined
>
> s/golden/the golden/
> s/Period/The period/
>
>> +in number of frames. For a value of 'n', every nth frame will be taken as golden frame.</entry>
>
> So for a period of, say, 4, what does that mean in practice? For example, I start encoding and
> give you the first 8 frames: 0, 1, 2, 3, 4, 5, 6 and 7.
>
> Will frames 0 and 4 be marked as golden frames, or 3 and 7? Your documentation suggests the
> latter, but I'm not really sure that is what you meant.
>

Yes it was bit ambiguous. For the set of 8 frames you mentioned, 0 and
4 are golden frames.
Will make it more clear.


>> +           </row>
>> +
>> +           <row><entry></entry></row>
>> +           <row id="v4l2-vpx-golden-frame-sel">
>> +             <entry spanname="id"><constant>V4L2_CID_VPX_GOLDEN_FRAME_SEL</constant></entry>
>> +             <entry>enum v4l2_vp8_golden_frame_sel</entry>
>> +           </row>
>> +           <row><entry spanname="descr">Selects the golden frame for encoding.
>> +Possible values are:</entry>
>> +           </row>
>> +           <row>
>> +             <entrytbl spanname="descr" cols="2">
>> +               <tbody valign="top">
>> +                 <row>
>> +                   <entry><constant>V4L2_VPX_GOLDEN_FRAME_USE_PREV</constant></entry>
>> +                   <entry>Use the last to last or (n-2)th frame as a golden frame. Current frame index being 'n'.</entry>
>
> "last to last" doesn't parse. Just use:
>
> "Use the (n-2)th frame as a golden frame, the current frame index being 'n'."
>
> That's unambiguous.

Ok.

Thanks and Regards
Arun